Hi Eric,
On 7/20/23 01:46, Eric Auger wrote:
Hi Shaoqin,
On 7/19/23 05:19, Shaoqin Huang wrote:
Currently some fields in SCTLR_EL1 don't define a name and directly used
in the SCTLR_EL1_RES1, that's not good now since these fields have been
functional and have a name.
According to the ARM DDI 0487J.a, define the name related to these
fields.
Suggested-by: Alexandru Elisei <alexandru.elisei@xxxxxxx>
Signed-off-by: Shaoqin Huang <shahuang@xxxxxxxxxx>
---
lib/arm64/asm/sysreg.h | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/lib/arm64/asm/sysreg.h b/lib/arm64/asm/sysreg.h
index c7f529d..9c68698 100644
--- a/lib/arm64/asm/sysreg.h
+++ b/lib/arm64/asm/sysreg.h
@@ -80,17 +80,26 @@ asm(
#define ICC_GRPEN1_EL1 sys_reg(3, 0, 12, 12, 7)
/* System Control Register (SCTLR_EL1) bits */
+#define SCTLR_EL1_LSMAOE _BITUL(29)
+#define SCTLR_EL1_NTLSMD _BITUL(28)
#define SCTLR_EL1_EE _BITUL(25)
+#define SCTLR_EL1_SPAN _BITUL(23)
+#define SCTLR_EL1_EIS _BITUL(22)
+#define SCTLR_EL1_TSCXT _BITUL(20)
#define SCTLR_EL1_WXN _BITUL(19)
#define SCTLR_EL1_I _BITUL(12)
+#define SCTLR_EL1_EOS _BITUL(11)
+#define SCTLR_EL1_SED _BITUL(8)
+#define SCTLR_EL1_ITD _BITUL(7)
#define SCTLR_EL1_SA0 _BITUL(4)
#define SCTLR_EL1_SA _BITUL(3)
#define SCTLR_EL1_C _BITUL(2)
#define SCTLR_EL1_A _BITUL(1)
#define SCTLR_EL1_M _BITUL(0)
-#define SCTLR_EL1_RES1 (_BITUL(7) | _BITUL(8) | _BITUL(11) | _BITUL(20) | \
- _BITUL(22) | _BITUL(23) | _BITUL(28) | _BITUL(29))
+#define SCTLR_EL1_RES1 (SCTLR_EL1_ITD | SCTLR_EL1_SED | SCTLR_EL1_EOS | \
+ SCTLR_EL1_TSCXT | SCTLR_EL1_EIS | SCTLR_EL1_SPAN | \
+ SCTLR_EL1_NTLSMD | SCTLR_EL1_LSMAOE)
#define INIT_SCTLR_EL1_MMU_OFF \
SCTLR_EL1_RES1
The change looks good to me (although _BITULL remark still holds).
Independently on this patch the _RES1 terminology looks odd to me. For
example ESO bit is RES1 only if FEAT_ExS is not implemented. Maybe I
misunderstand why it was named that way but to me RES1 means another
thing. If confirmed we could simply drop SCTLR_EL1_RES1 which is not
used elsewhere and directly define INIT_SCTLR_EL1_MMU_OF.
The SCTLR_EL1_RES1 was originally defined by commit 10b65ce ("arm64:
Configure SCTLR_EL1 at boot"). It seems at that time the BIT(11), yes
the EOS bit is undefined, and must be reserved for 1, but now it has a
definition which is EOS. It should have no effect if set EOS bit to 1
since its originally res to 1.
SCTLR_EL1_RES1 can simply be dropped since it has no other user.
Thanks,
Shaoqin
Thanks
Eric
--
Shaoqin
