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

Re: [Xen-devel] [PATCH 04/20] xen/arm: Rework HSCTLR_BASE



Hello Julien,

On 22.04.19 19:49, Julien Grall wrote:
The current value of HSCTLR_BASE for Arm64 is pretty wrong. It would
actually turn on SCTLR_EL2.nAA (bit 6) on hardware implementing
ARMv8.4-LSE.

Furthermore, the documentation of what is cleared/set in SCTLR_EL2 is
also not correct and looks like to be a verbatim copy from Arm32.

HSCTLR_BASE is replaced with a bunch of per-architecture new defines
helping to understand better what is the initialie value for
SCTLR_EL2/HSCTLR.

Note the defines *_CLEAR are only used to check the state of each bits
are known.

Lastly, the documentation is dropped from arm{32,64}/head.S as it would
be pretty easy to get out-of-sync with the definitions.

Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
---
  xen/arch/arm/arm32/head.S       | 12 +---------
  xen/arch/arm/arm64/head.S       | 10 +-------
  xen/include/asm-arm/processor.h | 52 ++++++++++++++++++++++++++++++++++++++++-
  3 files changed, 53 insertions(+), 21 deletions(-)

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index 5ef7e5a2f3..8a98607459 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -234,17 +234,7 @@ cpu_init_done:
          ldr   r0, 
=(TCR_RES1|TCR_SH0_IS|TCR_ORGN0_WBWA|TCR_IRGN0_WBWA|TCR_T0SZ(0))
          mcr   CP32(r0, HTCR)
- /*
-         * Set up the HSCTLR:
-         * Exceptions in LE ARM,
-         * Low-latency IRQs disabled,
-         * Write-implies-XN disabled (for now),
-         * D-cache disabled (for now),
-         * I-cache enabled,
-         * Alignment checking enabled,
-         * MMU translation disabled (for now).
-         */
-        ldr   r0, =(HSCTLR_BASE|SCTLR_AXX_A)
+        ldr   r0, =HSCTLR_SET
          mcr   CP32(r0, HSCTLR)
/*
diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index 8a6be3352e..4fe904c51d 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -363,15 +363,7 @@ skip_bss:
msr tcr_el2, x0 - /* Set up the SCTLR_EL2:
-         * Exceptions in LE ARM,
-         * Low-latency IRQs disabled,
-         * Write-implies-XN disabled (for now),
-         * D-cache disabled (for now),
-         * I-cache enabled,
-         * Alignment checking disabled,
-         * MMU translation disabled (for now). */
-        ldr   x0, =(HSCTLR_BASE)
+        ldr   x0, =SCTLR_EL2_SET
          msr   SCTLR_EL2, x0
/* Ensure that any exceptions encountered at EL2
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index 1a143fb6a3..6dac500e40 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -122,6 +122,9 @@
  #define SCTLR_A32_ELx_TE    _BITUL(30)
  #define SCTLR_A32_ELx_FI    _BITUL(21)
+/* Common bits for SCTLR_ELx for Arm64 */
+#define SCTLR_A64_ELx_SA    _BITUL(3)
+
  /* Common bits for SCTLR_ELx on all architectures */
  #define SCTLR_Axx_ELx_EE    _BITUL(25)
  #define SCTLR_Axx_ELx_WXN   _BITUL(19)
@@ -130,7 +133,54 @@
  #define SCTLR_Axx_ELx_A     _BITUL(1)
  #define SCTLR_Axx_ELx_M     _BITUL(0)
-#define HSCTLR_BASE _AC(0x30c51878,U)
+#ifdef CONFIG_ARM_32
+
+#define HSCTLR_RES1     (_BITUL(3)  | _BITUL(4)  | _BITUL(5)  | _BITUL(6)  |\
+                         _BITUL(11) | _BITUL(16) | _BITUL(18) | _BITUL(22) |\
+                         _BITUL(23) | _BITUL(28) | _BITUL(29))
+
+#define HSCTLR_RES0     (_BITUL(7)  | _BITUL(8)  | _BITUL(9)  | _BITUL(10) |\
+                         _BITUL(13) | _BITUL(14) | _BITUL(15) | _BITUL(17) |\
+                         _BITUL(20) | _BITUL(24) | _BITUL(26) | _BITUL(27) |\
+                         _BITUL(31))
+
+/* Initial value for HSCTLR */
+#define HSCTLR_SET      (HSCTLR_RES1    | SCTLR_Axx_ELx_A   | SCTLR_Axx_ELx_I)
+
+#define HSCTLR_CLEAR    (HSCTLR_RES0        | SCTLR_Axx_ELx_M   |\
+                         SCTLR_Axx_ELx_C    | SCTLR_Axx_ELx_WXN |\
+                         SCTLR_A32_ELx_FI   | SCTLR_Axx_ELx_EE  |\
+                         SCTLR_A32_ELx_TE)
+
+#if (HSCTLR_SET ^ HSCTLR_CLEAR) != 0xffffffffU
+#error "Inconsistent HSCTLR set/clear bits"
+#endif
+
+#else
+
+#define SCTLR_EL2_RES1  (_BITUL(4)  | _BITUL(5)  | _BITUL(11) | _BITUL(16) |\
+                         _BITUL(18) | _BITUL(22) | _BITUL(23) | _BITUL(28) |\
+                         _BITUL(29))
+
+#define SCTLR_EL2_RES0  (_BITUL(6)  | _BITUL(7)  | _BITUL(8)  | _BITUL(9)  |\
+                         _BITUL(10) | _BITUL(13) | _BITUL(14) | _BITUL(15) |\
+                         _BITUL(17) | _BITUL(20) | _BITUL(21) | _BITUL(24) |\
+                         _BITUL(26) | _BITUL(27) | _BITUL(30) | _BITUL(31) |\
+                         (0xffffffffULL << 32))
+
+/* Initial value for SCTLR_EL2 */
+#define SCTLR_EL2_SET   (SCTLR_EL2_RES1     | SCTLR_A64_ELx_SA  |\
+                         SCTLR_Axx_ELx_I)
+
+#define SCTLR_EL2_CLEAR (SCTLR_EL2_RES0     | SCTLR_Axx_ELx_M   |\
+                         SCTLR_Axx_ELx_A    | SCTLR_Axx_ELx_C   |\
+                         SCTLR_Axx_ELx_WXN  | SCTLR_Axx_ELx_EE)
+
+#if (SCTLR_EL2_SET ^ SCTLR_EL2_CLEAR) != 0xffffffffffffffffUL
+#error "Inconsistent SCTLR_EL2 set/clear bits"
+#endif
+
+#endif
/* HCR Hyp Configuration Register */
  #define HCR_RW          (_AC(1,UL)<<31) /* Register Width, ARM64 only */


Resolution of the dispute with Jan about [PATCH 01/20] is required first.

--
Sincerely,
Andrii Anisov.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

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