diff mbox series

[XEN,v1,4/4] xen/arm: traps.c: Enclose VMSA specific registers within CONFIG_MMU

Message ID 20230911135942.791206-5-ayan.kumar.halder@amd.com (mailing list archive)
State Superseded
Headers show
Series xen/arm: Split MMU code as the prepration of MPU work form Arm32 | expand

Commit Message

Ayan Kumar Halder Sept. 11, 2023, 1:59 p.m. UTC 
The VMSA specific registers (ie TCR, TTBR0, TTBR1, VTTBR, etc) are valid when
MMU is used, thus we can enclose them with CONFIG_MMU.

Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
---
 xen/arch/arm/traps.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

Comments

Penny Zheng Sept. 14, 2023, 5:48 a.m. UTC | #1 
Hi, Ayan

On 2023/9/11 21:59, Ayan Kumar Halder wrote:
> The VMSA specific registers (ie TCR, TTBR0, TTBR1, VTTBR, etc) are valid when
> MMU is used, thus we can enclose them with CONFIG_MMU.
> 
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
> ---
>   xen/arch/arm/traps.c | 17 +++++++++++++++--
>   1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 46c9a4031b..83522fcc5a 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -698,8 +698,10 @@ static void __do_trap_serror(struct cpu_user_regs *regs, bool guest)
>   struct reg_ctxt {
>       /* Guest-side state */
>       register_t sctlr_el1;
> +#ifdef CONFIG_MMU
>       register_t tcr_el1;
>       uint64_t ttbr0_el1, ttbr1_el1;
> +#endif
>   #ifdef CONFIG_ARM_32
>       uint32_t dfsr, ifsr;
>       uint32_t dfar, ifar;
> @@ -801,9 +803,11 @@ static void show_registers_32(const struct cpu_user_regs *regs,
>       if ( guest_mode_on )
>       {
>           printk("     SCTLR: %"PRIregister"\n", ctxt->sctlr_el1);
> +#ifdef CONFIG_MMU
>           printk("       TCR: %"PRIregister"\n", ctxt->tcr_el1);
>           printk("     TTBR0: %016"PRIx64"\n", ctxt->ttbr0_el1);
>           printk("     TTBR1: %016"PRIx64"\n", ctxt->ttbr1_el1);
> +#endif

FWIS, on v8R-AArch64, with MPU on EL2, we could still support VMSA on 
EL1, so registers like TTBR0_EL1/TCR_EL1 are valid.
See ARM DDI 0600B.a Table G1-2 Alphabetical index of modified AArch64 
System registers (continued) for detailed info.

>           printk("      IFAR: %08"PRIx32", IFSR: %08"PRIx32"\n"
>                  "      DFAR: %08"PRIx32", DFSR: %08"PRIx32"\n",
>   #ifdef CONFIG_ARM_64
> @@ -873,9 +877,11 @@ static void show_registers_64(const struct cpu_user_regs *regs,
>           printk("   FAR_EL1: %016"PRIx64"\n", ctxt->far);
>           printk("\n");
>           printk(" SCTLR_EL1: %"PRIregister"\n", ctxt->sctlr_el1);
> +#ifdef CONFIG_MMU
>           printk("   TCR_EL1: %"PRIregister"\n", ctxt->tcr_el1);
>           printk(" TTBR0_EL1: %016"PRIx64"\n", ctxt->ttbr0_el1);
>           printk(" TTBR1_EL1: %016"PRIx64"\n", ctxt->ttbr1_el1);
> +#endif
>           printk("\n");
>       }
>   }
> @@ -907,13 +913,15 @@ static void _show_registers(const struct cpu_user_regs *regs,
>           show_registers_32(regs, ctxt, guest_mode_on, v);
>   #endif
>       }
> +#ifdef CONFIG_MMU
>       printk("  VTCR_EL2: %"PRIregister"\n", READ_SYSREG(VTCR_EL2));
>       printk(" VTTBR_EL2: %016"PRIx64"\n", ctxt->vttbr_el2);
> +    printk(" TTBR0_EL2: %016"PRIx64"\n", READ_SYSREG64(TTBR0_EL2));
>       printk("\n");
> +#endif
>   
>       printk(" SCTLR_EL2: %"PRIregister"\n", READ_SYSREG(SCTLR_EL2));
>       printk("   HCR_EL2: %"PRIregister"\n", READ_SYSREG(HCR_EL2));
> -    printk(" TTBR0_EL2: %016"PRIx64"\n", READ_SYSREG64(TTBR0_EL2));
>       printk("\n");
>       printk("   ESR_EL2: %"PRIregister"\n", regs->hsr);
>       printk(" HPFAR_EL2: %"PRIregister"\n", READ_SYSREG(HPFAR_EL2));
> @@ -931,9 +939,13 @@ void show_registers(const struct cpu_user_regs *regs)
>   {
>       struct reg_ctxt ctxt;
>       ctxt.sctlr_el1 = READ_SYSREG(SCTLR_EL1);
> +#ifdef CONFIG_MMU
>       ctxt.tcr_el1 = READ_SYSREG(TCR_EL1);
>       ctxt.ttbr0_el1 = READ_SYSREG64(TTBR0_EL1);
>       ctxt.ttbr1_el1 = READ_SYSREG64(TTBR1_EL1);
> +    ctxt.vttbr_el2 = READ_SYSREG64(VTTBR_EL2);
> +#endif
> +
>   #ifdef CONFIG_ARM_32
>       ctxt.dfar = READ_CP32(DFAR);
>       ctxt.ifar = READ_CP32(IFAR);
> @@ -945,7 +957,6 @@ void show_registers(const struct cpu_user_regs *regs)
>       if ( guest_mode(regs) && is_32bit_domain(current->domain) )
>           ctxt.ifsr32_el2 = READ_SYSREG(IFSR32_EL2);
>   #endif
> -    ctxt.vttbr_el2 = READ_SYSREG64(VTTBR_EL2);
>   
>       _show_registers(regs, &ctxt, guest_mode(regs), current);
>   }
> @@ -954,9 +965,11 @@ void vcpu_show_registers(const struct vcpu *v)
>   {
>       struct reg_ctxt ctxt;
>       ctxt.sctlr_el1 = v->arch.sctlr;
> +#ifdef CONFIG_MMU
>       ctxt.tcr_el1 = v->arch.ttbcr;
>       ctxt.ttbr0_el1 = v->arch.ttbr0;
>       ctxt.ttbr1_el1 = v->arch.ttbr1;
> +#endif
>   #ifdef CONFIG_ARM_32
>       ctxt.dfar = v->arch.dfar;
>       ctxt.ifar = v->arch.ifar;
Ayan Kumar Halder Sept. 14, 2023, 8:47 a.m. UTC | #2 
On 14/09/2023 06:48, Penny Zheng wrote:
> Hi, Ayan
Hi Penny,
>
> On 2023/9/11 21:59, Ayan Kumar Halder wrote:
>> The VMSA specific registers (ie TCR, TTBR0, TTBR1, VTTBR, etc) are 
>> valid when
>> MMU is used, thus we can enclose them with CONFIG_MMU.
>>
>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
>> ---
>>   xen/arch/arm/traps.c | 17 +++++++++++++++--
>>   1 file changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
>> index 46c9a4031b..83522fcc5a 100644
>> --- a/xen/arch/arm/traps.c
>> +++ b/xen/arch/arm/traps.c
>> @@ -698,8 +698,10 @@ static void __do_trap_serror(struct 
>> cpu_user_regs *regs, bool guest)
>>   struct reg_ctxt {
>>       /* Guest-side state */
>>       register_t sctlr_el1;
>> +#ifdef CONFIG_MMU
>>       register_t tcr_el1;
>>       uint64_t ttbr0_el1, ttbr1_el1;
>> +#endif
>>   #ifdef CONFIG_ARM_32
>>       uint32_t dfsr, ifsr;
>>       uint32_t dfar, ifar;
>> @@ -801,9 +803,11 @@ static void show_registers_32(const struct 
>> cpu_user_regs *regs,
>>       if ( guest_mode_on )
>>       {
>>           printk("     SCTLR: %"PRIregister"\n", ctxt->sctlr_el1);
>> +#ifdef CONFIG_MMU
>>           printk("       TCR: %"PRIregister"\n", ctxt->tcr_el1);
>>           printk("     TTBR0: %016"PRIx64"\n", ctxt->ttbr0_el1);
>>           printk("     TTBR1: %016"PRIx64"\n", ctxt->ttbr1_el1);
>> +#endif
>
> FWIS, on v8R-AArch64, with MPU on EL2, we could still support VMSA on 
> EL1, so registers like TTBR0_EL1/TCR_EL1 are valid.
> See ARM DDI 0600B.a Table G1-2 Alphabetical index of modified AArch64 
> System registers (continued) for detailed info.

Actually, you are correct. And, we should not be using CONFIG_MMU to 
enclose VMSA EL1 registers. I am ok to drop this patch.

However, refer G1.3.7  "ID_AA64MMFR0_EL1, AArch64 Memory Model Feature 
Register 0"

When MSA_frac, bits [55:52] == 0b0001, then there is no support for 
VMSAv8-64. IIUC, VMSA EL1 registers will not be present.

If this is correct, then we might need to have AARCH64_V8R specific 
config as ID_AA64MMFR0_EL1 can be interpreted for this configuration only.

Appreciate your comments.

- Ayan

>
>>           printk("      IFAR: %08"PRIx32", IFSR: %08"PRIx32"\n"
>>                  "      DFAR: %08"PRIx32", DFSR: %08"PRIx32"\n",
>>   #ifdef CONFIG_ARM_64
>> @@ -873,9 +877,11 @@ static void show_registers_64(const struct 
>> cpu_user_regs *regs,
>>           printk("   FAR_EL1: %016"PRIx64"\n", ctxt->far);
>>           printk("\n");
>>           printk(" SCTLR_EL1: %"PRIregister"\n", ctxt->sctlr_el1);
>> +#ifdef CONFIG_MMU
>>           printk("   TCR_EL1: %"PRIregister"\n", ctxt->tcr_el1);
>>           printk(" TTBR0_EL1: %016"PRIx64"\n", ctxt->ttbr0_el1);
>>           printk(" TTBR1_EL1: %016"PRIx64"\n", ctxt->ttbr1_el1);
>> +#endif
>>           printk("\n");
>>       }
>>   }
>> @@ -907,13 +913,15 @@ static void _show_registers(const struct 
>> cpu_user_regs *regs,
>>           show_registers_32(regs, ctxt, guest_mode_on, v);
>>   #endif
>>       }
>> +#ifdef CONFIG_MMU
>>       printk("  VTCR_EL2: %"PRIregister"\n", READ_SYSREG(VTCR_EL2));
>>       printk(" VTTBR_EL2: %016"PRIx64"\n", ctxt->vttbr_el2);
>> +    printk(" TTBR0_EL2: %016"PRIx64"\n", READ_SYSREG64(TTBR0_EL2));
>>       printk("\n");
>> +#endif
>>         printk(" SCTLR_EL2: %"PRIregister"\n", READ_SYSREG(SCTLR_EL2));
>>       printk("   HCR_EL2: %"PRIregister"\n", READ_SYSREG(HCR_EL2));
>> -    printk(" TTBR0_EL2: %016"PRIx64"\n", READ_SYSREG64(TTBR0_EL2));
>>       printk("\n");
>>       printk("   ESR_EL2: %"PRIregister"\n", regs->hsr);
>>       printk(" HPFAR_EL2: %"PRIregister"\n", READ_SYSREG(HPFAR_EL2));
>> @@ -931,9 +939,13 @@ void show_registers(const struct cpu_user_regs 
>> *regs)
>>   {
>>       struct reg_ctxt ctxt;
>>       ctxt.sctlr_el1 = READ_SYSREG(SCTLR_EL1);
>> +#ifdef CONFIG_MMU
>>       ctxt.tcr_el1 = READ_SYSREG(TCR_EL1);
>>       ctxt.ttbr0_el1 = READ_SYSREG64(TTBR0_EL1);
>>       ctxt.ttbr1_el1 = READ_SYSREG64(TTBR1_EL1);
>> +    ctxt.vttbr_el2 = READ_SYSREG64(VTTBR_EL2);
>> +#endif
>> +
>>   #ifdef CONFIG_ARM_32
>>       ctxt.dfar = READ_CP32(DFAR);
>>       ctxt.ifar = READ_CP32(IFAR);
>> @@ -945,7 +957,6 @@ void show_registers(const struct cpu_user_regs 
>> *regs)
>>       if ( guest_mode(regs) && is_32bit_domain(current->domain) )
>>           ctxt.ifsr32_el2 = READ_SYSREG(IFSR32_EL2);
>>   #endif
>> -    ctxt.vttbr_el2 = READ_SYSREG64(VTTBR_EL2);
>>         _show_registers(regs, &ctxt, guest_mode(regs), current);
>>   }
>> @@ -954,9 +965,11 @@ void vcpu_show_registers(const struct vcpu *v)
>>   {
>>       struct reg_ctxt ctxt;
>>       ctxt.sctlr_el1 = v->arch.sctlr;
>> +#ifdef CONFIG_MMU
>>       ctxt.tcr_el1 = v->arch.ttbcr;
>>       ctxt.ttbr0_el1 = v->arch.ttbr0;
>>       ctxt.ttbr1_el1 = v->arch.ttbr1;
>> +#endif
>>   #ifdef CONFIG_ARM_32
>>       ctxt.dfar = v->arch.dfar;
>>       ctxt.ifar = v->arch.ifar;
diff mbox series

Patch

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 46c9a4031b..83522fcc5a 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -698,8 +698,10 @@  static void __do_trap_serror(struct cpu_user_regs *regs, bool guest)
 struct reg_ctxt {
     /* Guest-side state */
     register_t sctlr_el1;
+#ifdef CONFIG_MMU
     register_t tcr_el1;
     uint64_t ttbr0_el1, ttbr1_el1;
+#endif
 #ifdef CONFIG_ARM_32
     uint32_t dfsr, ifsr;
     uint32_t dfar, ifar;
@@ -801,9 +803,11 @@  static void show_registers_32(const struct cpu_user_regs *regs,
     if ( guest_mode_on )
     {
         printk("     SCTLR: %"PRIregister"\n", ctxt->sctlr_el1);
+#ifdef CONFIG_MMU
         printk("       TCR: %"PRIregister"\n", ctxt->tcr_el1);
         printk("     TTBR0: %016"PRIx64"\n", ctxt->ttbr0_el1);
         printk("     TTBR1: %016"PRIx64"\n", ctxt->ttbr1_el1);
+#endif
         printk("      IFAR: %08"PRIx32", IFSR: %08"PRIx32"\n"
                "      DFAR: %08"PRIx32", DFSR: %08"PRIx32"\n",
 #ifdef CONFIG_ARM_64
@@ -873,9 +877,11 @@  static void show_registers_64(const struct cpu_user_regs *regs,
         printk("   FAR_EL1: %016"PRIx64"\n", ctxt->far);
         printk("\n");
         printk(" SCTLR_EL1: %"PRIregister"\n", ctxt->sctlr_el1);
+#ifdef CONFIG_MMU
         printk("   TCR_EL1: %"PRIregister"\n", ctxt->tcr_el1);
         printk(" TTBR0_EL1: %016"PRIx64"\n", ctxt->ttbr0_el1);
         printk(" TTBR1_EL1: %016"PRIx64"\n", ctxt->ttbr1_el1);
+#endif
         printk("\n");
     }
 }
@@ -907,13 +913,15 @@  static void _show_registers(const struct cpu_user_regs *regs,
         show_registers_32(regs, ctxt, guest_mode_on, v);
 #endif
     }
+#ifdef CONFIG_MMU
     printk("  VTCR_EL2: %"PRIregister"\n", READ_SYSREG(VTCR_EL2));
     printk(" VTTBR_EL2: %016"PRIx64"\n", ctxt->vttbr_el2);
+    printk(" TTBR0_EL2: %016"PRIx64"\n", READ_SYSREG64(TTBR0_EL2));
     printk("\n");
+#endif
 
     printk(" SCTLR_EL2: %"PRIregister"\n", READ_SYSREG(SCTLR_EL2));
     printk("   HCR_EL2: %"PRIregister"\n", READ_SYSREG(HCR_EL2));
-    printk(" TTBR0_EL2: %016"PRIx64"\n", READ_SYSREG64(TTBR0_EL2));
     printk("\n");
     printk("   ESR_EL2: %"PRIregister"\n", regs->hsr);
     printk(" HPFAR_EL2: %"PRIregister"\n", READ_SYSREG(HPFAR_EL2));
@@ -931,9 +939,13 @@  void show_registers(const struct cpu_user_regs *regs)
 {
     struct reg_ctxt ctxt;
     ctxt.sctlr_el1 = READ_SYSREG(SCTLR_EL1);
+#ifdef CONFIG_MMU
     ctxt.tcr_el1 = READ_SYSREG(TCR_EL1);
     ctxt.ttbr0_el1 = READ_SYSREG64(TTBR0_EL1);
     ctxt.ttbr1_el1 = READ_SYSREG64(TTBR1_EL1);
+    ctxt.vttbr_el2 = READ_SYSREG64(VTTBR_EL2);
+#endif
+
 #ifdef CONFIG_ARM_32
     ctxt.dfar = READ_CP32(DFAR);
     ctxt.ifar = READ_CP32(IFAR);
@@ -945,7 +957,6 @@  void show_registers(const struct cpu_user_regs *regs)
     if ( guest_mode(regs) && is_32bit_domain(current->domain) )
         ctxt.ifsr32_el2 = READ_SYSREG(IFSR32_EL2);
 #endif
-    ctxt.vttbr_el2 = READ_SYSREG64(VTTBR_EL2);
 
     _show_registers(regs, &ctxt, guest_mode(regs), current);
 }
@@ -954,9 +965,11 @@  void vcpu_show_registers(const struct vcpu *v)
 {
     struct reg_ctxt ctxt;
     ctxt.sctlr_el1 = v->arch.sctlr;
+#ifdef CONFIG_MMU
     ctxt.tcr_el1 = v->arch.ttbcr;
     ctxt.ttbr0_el1 = v->arch.ttbr0;
     ctxt.ttbr1_el1 = v->arch.ttbr1;
+#endif
 #ifdef CONFIG_ARM_32
     ctxt.dfar = v->arch.dfar;
     ctxt.ifar = v->arch.ifar;