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

[Ayan Kumar Halder]

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.hal...@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;