From patchwork Tue Dec 3 15:40:34 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Campbell X-Patchwork-Id: 21968 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-qa0-f69.google.com (mail-qa0-f69.google.com [209.85.216.69]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id E6BCF202DA for ; Tue, 3 Dec 2013 15:40:40 +0000 (UTC) Received: by mail-qa0-f69.google.com with SMTP id ii20sf14223466qab.8 for ; Tue, 03 Dec 2013 07:40:40 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:message-id:subject:from:to:cc:date :in-reply-to:references:organization:mime-version:x-original-sender :x-original-authentication-results:precedence:mailing-list:list-id :list-post:list-help:list-archive:list-unsubscribe:content-type :content-transfer-encoding; bh=82FRB8oqKpCcqASkBIml/9ySzhOnDIKgBi1tLjnXKvw=; b=FTRCyfxKwOtBqY1OkwfASIvtafvnNJnnpbo8foijFK6yy69oBPyqmEK2vns8stB7sc x6TW3YDqK0BRBiFOEsV6olO8zeckj4nRgvC6nBH7OPxUiaozZ4WcJyaWm/DcgDVk5WSa 2ZTzVREf9WXT5fU+eqSxLay6HRc+HVAwFIUhZ3F/NOQ7hQXd9l1qy7+ENiJdzsNUYibx 08hS5fnJi1pJDkJ3niMV1RBc615zAgeSjSJIOr2WOGlfyD5/D8gOShzuJI97i923YnG6 NupI/gNSPxKRVF2aWKgGV8Nx2g9KgHw2/PNmOULYA+1YhjUOyJbIrL2104lQBWWy3oZS UpFA== X-Gm-Message-State: ALoCoQk9acmUYVXcPyGVOSClbUC7LKz3jczyRz/dDKovb9Dg9e5/wVmfnGCV1AJ8jzAtv3TieYPK X-Received: by 10.224.24.65 with SMTP id u1mr23822115qab.6.1386085240075; Tue, 03 Dec 2013 07:40:40 -0800 (PST) X-BeenThere: patchwork-forward@linaro.org Received: by 10.49.24.174 with SMTP id v14ls3736169qef.74.gmail; Tue, 03 Dec 2013 07:40:39 -0800 (PST) X-Received: by 10.52.120.11 with SMTP id ky11mr5111821vdb.28.1386085239953; Tue, 03 Dec 2013 07:40:39 -0800 (PST) Received: from mail-vc0-f170.google.com (mail-vc0-f170.google.com [209.85.220.170]) by mx.google.com with ESMTPS id eb1si2413336vec.30.2013.12.03.07.40.39 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 03 Dec 2013 07:40:39 -0800 (PST) Received-SPF: neutral (google.com: 209.85.220.170 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.220.170; Received: by mail-vc0-f170.google.com with SMTP id ht10so10216025vcb.1 for ; Tue, 03 Dec 2013 07:40:39 -0800 (PST) X-Received: by 10.52.227.233 with SMTP id sd9mr87500vdc.53.1386085239734; Tue, 03 Dec 2013 07:40:39 -0800 (PST) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patches@linaro.org Received: by 10.220.174.196 with SMTP id u4csp217939vcz; Tue, 3 Dec 2013 07:40:39 -0800 (PST) X-Received: by 10.224.57.68 with SMTP id b4mr127341494qah.63.1386085239156; Tue, 03 Dec 2013 07:40:39 -0800 (PST) Received: from SMTP.CITRIX.COM (smtp.citrix.com. [66.165.176.89]) by mx.google.com with ESMTPS id y5si4476756qat.153.2013.12.03.07.40.37 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Tue, 03 Dec 2013 07:40:39 -0800 (PST) Received-SPF: pass (google.com: domain of Ian.Campbell@citrix.com designates 66.165.176.89 as permitted sender) client-ip=66.165.176.89; X-IronPort-AV: E=Sophos;i="4.93,818,1378857600"; d="scan'208";a="80138405" Received: from accessns.citrite.net (HELO FTLPEX01CL01.citrite.net) ([10.9.154.239]) by FTLPIPO01.CITRIX.COM with ESMTP; 03 Dec 2013 15:40:36 +0000 Received: from [10.80.2.80] (10.80.2.80) by FTLPEX01CL01.citrite.net (10.13.107.78) with Microsoft SMTP Server id 14.2.342.4; Tue, 3 Dec 2013 10:40:35 -0500 Message-ID: <1386085234.13256.49.camel@kazak.uk.xensource.com> Subject: Re: [Xen-devel] [PATCH] xen: arm: Fixing ttbcr (TCR_EL1 for AArch64) size. From: Ian Campbell To: Pranavkumar Sawargaonkar CC: Anup Patel , , , , , Date: Tue, 3 Dec 2013 15:40:34 +0000 In-Reply-To: <1386078453.13256.35.camel@kazak.uk.xensource.com> References: <1386072224-4478-1-git-send-email-pranavkumar@linaro.org> <1386078453.13256.35.camel@kazak.uk.xensource.com> Organization: Citrix Systems, Inc. X-Mailer: Evolution 3.4.4-3 MIME-Version: 1.0 X-Originating-IP: [10.80.2.80] X-DLP: MIA1 X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: ian.campbell@citrix.com X-Original-Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.220.170 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Precedence: list Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org List-ID: X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , On Tue, 2013-12-03 at 13:47 +0000, Ian Campbell wrote: > On Tue, 2013-12-03 at 17:33 +0530, Pranavkumar Sawargaonkar wrote: > > This patch fixes size of ttbcr register (TCR_EL1 in case of AArch64) > > and it's programming considering size in case of context switch. > > > > Currently ttbcr is defined as 32b register but for AArch64 TCR_EL1 > > size is 64b. > > Thanks for tracking this down. > > > Signed-off-by: Anup Patel > > Signed-off-by: Pranavkumar Sawargaonkar > [...] > > diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h > > index d5cae2e..2aa4443 100644 > > --- a/xen/include/asm-arm/domain.h > > +++ b/xen/include/asm-arm/domain.h > > @@ -165,7 +165,11 @@ struct arch_vcpu > > > > /* MMU */ > > register_t vbar; > > +#ifdef CONFIG_ARM_32 > > uint32_t ttbcr; > > +#else > > + uint64_t ttbcr; > > +#endif > > I think that actually only this hunk is required. Actually, it turns out there a few other places where ttbcr is incorrectly stored in a 32-bit variable. Including one which needed some careful consideration before it was safe to change. Here is what I came up with. -------------8<--------------- >From 832544695c39a7d3f98e73381010eaab491982cc Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Tue, 3 Dec 2013 15:13:36 +0000 Subject: [PATCH] xen: arm: TCR_EL1 is 64-bit on arm64 Storing it in a 32-bit variable in struct arch_vcpu caused breakage over context switch. There were also several other places which stored this as the 32-bit value. Update them all. The "struct vcpu_guest_context" case needs special consideration. This struct is in theory is exposed to guests, via the VCPUOP_initialise hypercall. However as discussed in http://lists.xen.org/archives/html/xen-devel/2013-10/msg00912.html this isn't really a guest visible interface since ARM uses PSCI for VCPU bringup (VCPUOP_initialise simply isn't available) The other users of this interface are the domctls, which are not a stable API. Therefore while fixing the ttbcr size also surround the struct in ifdefs to restrict the struct to the hypervisor and the tools only (omitting the extra complexity of renaming as I suggested in the referenced thread) NB TCR_EL1 on arm64 is known as TTBCR on arm32, hence the apparent naming inconsistencies. Spotted-by: Pranavkumar Sawargaonkar Signed-off-by: Ian Campbell Cc: Anup Patel Cc: patches@linaro.org Cc: patches@apm.com Acked-by: Julien Grall --- xen/arch/arm/traps.c | 11 ++++++----- xen/include/asm-arm/domain.h | 2 +- xen/include/public/arch-arm.h | 6 ++++-- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index 53eb0cf..b2725df 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -330,7 +330,8 @@ static void inject_undef64_exception(struct cpu_user_regs *regs, int instr_len) struct reg_ctxt { /* Guest-side state */ - uint32_t sctlr_el1, tcr_el1; + uint32_t sctlr_el1; + register_t tcr_el1; uint64_t ttbr0_el1, ttbr1_el1; #ifdef CONFIG_ARM_32 uint32_t dfsr, ifsr; @@ -433,7 +434,7 @@ static void show_registers_32(struct cpu_user_regs *regs, if ( guest_mode ) { printk(" SCTLR: %08"PRIx32"\n", ctxt->sctlr_el1); - printk(" TCR: %08"PRIx32"\n", ctxt->tcr_el1); + printk(" TCR: %08"PRIregister"\n", ctxt->tcr_el1); printk(" TTBR0: %016"PRIx64"\n", ctxt->ttbr0_el1); printk(" TTBR1: %016"PRIx64"\n", ctxt->ttbr1_el1); printk(" IFAR: %08"PRIx32", IFSR: %08"PRIx32"\n" @@ -505,7 +506,7 @@ static void show_registers_64(struct cpu_user_regs *regs, printk(" FAR_EL1: %016"PRIx64"\n", ctxt->far); printk("\n"); printk(" SCTLR_EL1: %08"PRIx32"\n", ctxt->sctlr_el1); - printk(" TCR_EL1: %08"PRIx32"\n", ctxt->tcr_el1); + printk(" TCR_EL1: %08"PRIregister"\n", ctxt->tcr_el1); printk(" TTBR0_EL1: %016"PRIx64"\n", ctxt->ttbr0_el1); printk(" TTBR1_EL1: %016"PRIx64"\n", ctxt->ttbr1_el1); printk("\n"); @@ -1261,14 +1262,14 @@ static void do_sysreg(struct cpu_user_regs *regs, void dump_guest_s1_walk(struct domain *d, vaddr_t addr) { - uint32_t ttbcr = READ_SYSREG32(TCR_EL1); + register_t ttbcr = READ_SYSREG(TCR_EL1); uint64_t ttbr0 = READ_SYSREG64(TTBR0_EL1); paddr_t paddr; uint32_t offset; uint32_t *first = NULL, *second = NULL; printk("dom%d VA 0x%08"PRIvaddr"\n", d->domain_id, addr); - printk(" TTBCR: 0x%08"PRIx32"\n", ttbcr); + printk(" TTBCR: 0x%08"PRIregister"\n", ttbcr); printk(" TTBR0: 0x%016"PRIx64" = 0x%"PRIpaddr"\n", ttbr0, p2m_lookup(d, ttbr0 & PAGE_MASK)); diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h index d5cae2e..8ebee3e 100644 --- a/xen/include/asm-arm/domain.h +++ b/xen/include/asm-arm/domain.h @@ -165,7 +165,7 @@ struct arch_vcpu /* MMU */ register_t vbar; - uint32_t ttbcr; + register_t ttbcr; uint64_t ttbr0, ttbr1; uint32_t dacr; /* 32-bit guests only */ diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h index cb41ddc..475cb4a 100644 --- a/xen/include/public/arch-arm.h +++ b/xen/include/public/arch-arm.h @@ -278,6 +278,7 @@ typedef uint64_t xen_pfn_t; typedef uint64_t xen_ulong_t; #define PRI_xen_ulong PRIx64 +#if defined(__XEN__) || defined(__XEN_TOOLS__) struct vcpu_guest_context { #define _VGCF_online 0 #define VGCF_online (1<<_VGCF_online) @@ -285,11 +286,12 @@ struct vcpu_guest_context { struct vcpu_guest_core_regs user_regs; /* Core CPU registers */ - uint32_t sctlr, ttbcr; - uint64_t ttbr0, ttbr1; + uint32_t sctlr; + uint64_t ttbcr, ttbr0, ttbr1; }; typedef struct vcpu_guest_context vcpu_guest_context_t; DEFINE_XEN_GUEST_HANDLE(vcpu_guest_context_t); +#endif struct arch_vcpu_info { };