diff mbox series

[v3,07/10] xen/arm: Always access SCTLR_EL2 using READ/WRITE_SYSREG()

Message ID 20210505074308.11016-8-michal.orzel@arm.com (mailing list archive)
State New
Headers show
Series arm64: Get rid of READ/WRITE_SYSREG32 | expand

Commit Message

Michal Orzel May 5, 2021, 7:43 a.m. UTC
The Armv8 specification describes the system register as a 64-bit value
on AArch64 and 32-bit value on AArch32 (same as ARMv7).

Unfortunately, Xen is accessing the system registers using
READ/WRITE_SYSREG32() which means the top 32-bit are clobbered.

This is only a latent bug so far because Xen will not yet use the top
32-bit.

There is also no change in behavior because arch/arm/arm64/head.S will
initialize SCTLR_EL2 to a sane value with the top 32-bit zeroed.

Signed-off-by: Michal Orzel <michal.orzel@arm.com>
---
Changes since v2:
-Modify the commit message
Changes since v1:
-Update commit message with SCTLR_EL2 analysis
---
 xen/arch/arm/mm.c    | 2 +-
 xen/arch/arm/traps.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Julien Grall May 5, 2021, 6:07 p.m. UTC | #1
Hi Michal,

On 05/05/2021 08:43, Michal Orzel wrote:
> The Armv8 specification describes the system register as a 64-bit value
> on AArch64 and 32-bit value on AArch32 (same as ARMv7).
> 
> Unfortunately, Xen is accessing the system registers using
> READ/WRITE_SYSREG32() which means the top 32-bit are clobbered.
> 
> This is only a latent bug so far because Xen will not yet use the top
> 32-bit.
> 
> There is also no change in behavior because arch/arm/arm64/head.S will
> initialize SCTLR_EL2 to a sane value with the top 32-bit zeroed.
> 
> Signed-off-by: Michal Orzel <michal.orzel@arm.com>

Acked-by: Julien Grall <jgrall@amazon.com>

Cheers,
diff mbox series

Patch

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 59f8a3f15f..0e07335291 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -613,7 +613,7 @@  void __init remove_early_mappings(void)
  */
 static void xen_pt_enforce_wnx(void)
 {
-    WRITE_SYSREG32(READ_SYSREG32(SCTLR_EL2) | SCTLR_Axx_ELx_WXN, SCTLR_EL2);
+    WRITE_SYSREG(READ_SYSREG(SCTLR_EL2) | SCTLR_Axx_ELx_WXN, SCTLR_EL2);
     /*
      * The TLBs may cache SCTLR_EL2.WXN. So ensure it is synchronized
      * before flushing the TLBs.
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index c7acdb2087..e7384381cc 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -915,7 +915,7 @@  static void _show_registers(const struct cpu_user_regs *regs,
     printk(" VTTBR_EL2: %016"PRIx64"\n", ctxt->vttbr_el2);
     printk("\n");
 
-    printk(" SCTLR_EL2: %08"PRIx32"\n", READ_SYSREG32(SCTLR_EL2));
+    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");