Re: [Xen-devel] [PATCH] xen: arm: Fixing ttbcr (TCR_EL1 for AArch64) size.

[Pranavkumar Sawargaonkar <psawargaonkar@xxxxxxx>]

On Tue, Dec 3, 2013 at 10:57 PM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> On Tue, 2013-12-03 at 21:50 +0530, Pranavkumar Sawargaonkar wrote:
>> > Acked-by: Julien Grall <julien.grall@xxxxxxxxxx>
>
>> Thanks for spotting other changes.
>> Patch looks good for me.
>
> Thanks. May I translate that into an Acked-by?
Yes, i missed adding acked by in last mail :)
Acked-by: Pranavkumar Sawargaonkar <pranavkumar@xxxxxxxxxx>


>
> Actually my 64-bit userspace build test wasn't actually building (just
> installing, oops!) so I missed out
> updating ./tools/include/xen-foreign/reference.size and xenctx. v2
> below.
>
> -------8<-----------------
>
> From f49eae50daf4c6be11abbb43c2a74e3643798044 Mon Sep 17 00:00:00 2001
> From: Ian Campbell <ian.campbell@xxxxxxxxxx>
> 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 <pranavkumar@xxxxxxxxxx>
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> Acked-by: Julien Grall <julien.grall@xxxxxxxxxx>
> Cc: Anup Patel <anup.patel@xxxxxxxxxx>
> Cc: patches@xxxxxxxxxx
> Cc: patches@xxxxxxx
> ---
> v2: Update foreign header reference sizes, fix xenctx.
> ---
>  tools/include/xen-foreign/reference.size |    2 +-
>  tools/xentrace/xenctx.c                  |    2 +-
>  xen/arch/arm/traps.c                     |   11 ++++++-----
>  xen/include/asm-arm/domain.h             |    2 +-
>  xen/include/public/arch-arm.h            |    6 ++++--
>  5 files changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/tools/include/xen-foreign/reference.size 
> b/tools/include/xen-foreign/reference.size
> index b3347b4..60ee262 100644
> --- a/tools/include/xen-foreign/reference.size
> +++ b/tools/include/xen-foreign/reference.size
> @@ -5,7 +5,7 @@ start_info                |       -       -    1112    1168
>  trap_info                 |       -       -       8      16
>  cpu_user_regs             |       -       -      68     200
>  vcpu_guest_core_regs      |     304     304       -       -
> -vcpu_guest_context        |     336     336    2800    5168
> +vcpu_guest_context        |     344     344    2800    5168
>  arch_vcpu_info            |       0       0      24      16
>  vcpu_time_info            |      32      32      32      32
>  vcpu_info                 |      48      48      64      64
> diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c
> index ba502fd..7275a00 100644
> --- a/tools/xentrace/xenctx.c
> +++ b/tools/xentrace/xenctx.c
> @@ -578,7 +578,7 @@ static void print_ctx(vcpu_guest_context_any_t *ctx_any)
>  #endif
>
>      printf("SCTLR: %08"PRIx32"\n", ctx->sctlr);
> -    printf("TTBCR: %08"PRIx32"\n", ctx->ttbcr);
> +    printf("TTBCR: %016"PRIx64"\n", ctx->ttbcr);
>      printf("TTBR0: %016"PRIx64"\n", ctx->ttbr0);
>      printf("TTBR1: %016"PRIx64"\n", ctx->ttbr1);
>  }
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 8144b2b..458128e 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");
> @@ -1257,14 +1258,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 {
>  };
> --
> 1.7.10.4
>
>
>
> CONFIDENTIALITY NOTICE: This e-mail message, including any attachments,
> is for the sole use of the intended recipient(s) and contains information
> that is confidential and proprietary to Applied Micro Circuits Corporation or 
> its subsidiaries.
> It is to be used solely for the purpose of furthering the parties' business 
> relationship.
> All unauthorized review, use, disclosure or distribution is prohibited.
> If you are not the intended recipient, please contact the sender by reply 
> e-mail
> and destroy all copies of the original message.
>

-
Pranav

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel