[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 10/10] arm64: Change type of hsr, cpsr, spsr_el1 to uint64_t



Hi,

On 05/05/2021 08:43, Michal Orzel wrote:
AArch64 registers are 64bit whereas AArch32 registers
are 32bit or 64bit. MSR/MRS are expecting 64bit values thus
we should get rid of helpers READ/WRITE_SYSREG32
in favour of using READ/WRITE_SYSREG.
We should also use register_t type when reading sysregs
which can correspond to uint64_t or uint32_t.
Even though many AArch64 registers have upper 32bit reserved
it does not mean that they can't be widen in the future.

Modify type of hsr, cpsr, spsr_el1 to uint64_t.
Previously we relied on the padding after SPSR_EL1.
As we removed the padding, modify the union to be 64bit so we don't corrupt 
SPSR_FIQ.
No need to modify the assembly code becuase the accesses were based on 64bit

s/becuase/because/

registers as there was a 32bit padding after SPSR_EL1.

Remove 32bit padding in cpu_user_regs before spsr_fiq
as it is no longer needed due to upper union being 64bit now.
Add 64bit padding in cpu_user_regs before spsr_el1
because offset of spsr_el1 must be a multiple of 8.

Change type of cpsr to uint64_t in the public outside interface
"public/arch-arm.h" to allow ABI compatibility between 32bit and 64bit.
Increment XEN_DOMCTL_INTERFACE_VERSION.

Change type of cpsr to uint64_t in the public outside interface
"public/vm_event.h" to allow ABI compatibility between 32bit and 64bit.
[...]

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index e7384381cc..c8f9773566 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -546,7 +546,7 @@ void inject_undef64_exception(struct cpu_user_regs *regs, 
int instr_len)
          PSR_IRQ_MASK | PSR_DBG_MASK;
      regs->pc = handler;
- WRITE_SYSREG32(esr.bits, ESR_EL1);
+    WRITE_SYSREG(esr.bits, ESR_EL1);
  }
/* Inject an abort exception into a 64 bit guest */
@@ -580,7 +580,7 @@ static void inject_abt64_exception(struct cpu_user_regs 
*regs,
      regs->pc = handler;
WRITE_SYSREG(addr, FAR_EL1);
-    WRITE_SYSREG32(esr.bits, ESR_EL1);
+    WRITE_SYSREG(esr.bits, ESR_EL1);
  }
static void inject_dabt64_exception(struct cpu_user_regs *regs,
@@ -717,7 +717,7 @@ struct reg_ctxt {
      uint64_t vttbr_el2;
  };
-static const char *mode_string(uint32_t cpsr)
+static const char *mode_string(register_t cpsr)
  {
      uint32_t mode;
      static const char *mode_strings[] = {
@@ -756,14 +756,16 @@ static void show_registers_32(const struct cpu_user_regs 
*regs,
  #ifdef CONFIG_ARM_64
      BUG_ON( ! (regs->cpsr & PSR_MODE_BIT) );
      printk("PC:     %08"PRIx32"\n", regs->pc32);
+    printk("CPSR:   %016"PRIx64" MODE:%s\n", regs->cpsr,
+           mode_string(regs->cpsr));
Why do you now need to duplicate this line? Can't we use PRIregister you did everywhere else a register is printed?

  #else
      printk("PC:     %08"PRIx32, regs->pc);
      if ( !guest_mode )
          printk(" %pS", _p(regs->pc));
      printk("\n");
-#endif
      printk("CPSR:   %08"PRIx32" MODE:%s\n", regs->cpsr,
             mode_string(regs->cpsr));
+#endif

[...]

diff --git a/xen/include/asm-arm/arm64/processor.h 
b/xen/include/asm-arm/arm64/processor.h
index 81dfc5e615..0e86079cbb 100644
--- a/xen/include/asm-arm/arm64/processor.h
+++ b/xen/include/asm-arm/arm64/processor.h
@@ -63,18 +63,19 @@ struct cpu_user_regs
/* Return address and mode */
      __DECL_REG(pc,           pc32);             /* ELR_EL2 */
-    uint32_t cpsr;                              /* SPSR_EL2 */
-    uint32_t hsr;                               /* ESR_EL2 */
+    uint64_t cpsr;                              /* SPSR_EL2 */
+    uint64_t hsr;                               /* ESR_EL2 */
+
+    /* Offset of spsr_el1 must be a multiple of 8 */

I am guessing you are saying it should be 8-byte aligned, right? If so, the field before is a 64-bit value, therefore the offset should already be a multiple of 8. Did I miss anything?

+    uint64_t pad0;
/* Outer guest frame only from here on... */ union {
-        uint32_t spsr_el1;       /* AArch64 */
+        uint64_t spsr_el1;       /* AArch64 */
          uint32_t spsr_svc;       /* AArch32 */
      };
- uint32_t pad1; /* Doubleword-align the user half of the frame */
-
      /* AArch32 guests only */
      uint32_t spsr_fiq, spsr_irq, spsr_und, spsr_abt;
diff --git a/xen/include/asm-arm/hsr.h b/xen/include/asm-arm/hsr.h
index 29d4531f40..9b91b28c48 100644
--- a/xen/include/asm-arm/hsr.h
+++ b/xen/include/asm-arm/hsr.h
@@ -16,7 +16,7 @@ enum dabt_size {
  };
union hsr {
-    uint32_t bits;
+    register_t bits;
      struct {
          unsigned long iss:25;  /* Instruction Specific Syndrome */
          unsigned long len:1;   /* Instruction length */
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index 713fd65317..64a2ca30da 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -267,10 +267,10 @@ struct vcpu_guest_core_regs
/* Return address and mode */
      __DECL_REG(pc64,         pc32);             /* ELR_EL2 */
-    uint32_t cpsr;                              /* SPSR_EL2 */
+    uint64_t cpsr;                              /* SPSR_EL2 */
union {
-        uint32_t spsr_el1;       /* AArch64 */
+        uint64_t spsr_el1;       /* AArch64 */
          uint32_t spsr_svc;       /* AArch32 */
      };
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 4dbf107785..d576bfabd6 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -38,7 +38,7 @@
  #include "hvm/save.h"
  #include "memory.h"
-#define XEN_DOMCTL_INTERFACE_VERSION 0x00000013
+#define XEN_DOMCTL_INTERFACE_VERSION 0x00000014
/*
   * NB. xen_domctl.domain is an IN/OUT parameter for this operation.
diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h
index 36135ba4f1..bb003d21d0 100644
--- a/xen/include/public/vm_event.h
+++ b/xen/include/public/vm_event.h
@@ -266,8 +266,7 @@ struct vm_event_regs_arm {
      uint64_t ttbr1;
      uint64_t ttbcr;
      uint64_t pc;
-    uint32_t cpsr;
-    uint32_t _pad;
+    uint64_t cpsr;
  };
/*


--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.