Hi Phil,
On 7/25/25 10:18, Philippe Mathieu-Daudé wrote:
Hi Gustavo,
On 25/7/25 03:47, Gustavo Romero wrote:
Clean up the definitions of NSW and NSA fields in the VTCR register.
These two fields are already defined properly using FIELD() so they are
actually duplications. Also, define the NSW and NSA fields in the
VSTCR register using FIELD() and remove their definitions based on VTCR
fields.
Signed-off-by: Gustavo Romero <gustavo.rom...@linaro.org>
---
target/arm/internals.h | 8 +++-----
target/arm/ptw.c | 8 ++++----
2 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index c4765e4489..052f7b641c 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -113,11 +113,6 @@ FIELD(DBGWCR, WT, 20, 1)
FIELD(DBGWCR, MASK, 24, 5)
FIELD(DBGWCR, SSCE, 29, 1)
-#define VTCR_NSW (1u << 29)
-#define VTCR_NSA (1u << 30)
-#define VSTCR_SW VTCR_NSW
-#define VSTCR_SA VTCR_NSA
-
/* Bit definitions for CPACR (AArch32 only) */
FIELD(CPACR, CP10, 20, 2)
FIELD(CPACR, CP11, 22, 2)
@@ -220,6 +215,9 @@ FIELD(VTCR, NSA, 30, 1)
FIELD(VTCR, DS, 32, 1)
FIELD(VTCR, SL2, 33, 1)
+FIELD(VSTCR, SW, 29, 1)
+FIELD(VSTCR, SA, 30, 1)
+
#define HCRX_ENAS0 (1ULL << 0)
#define HCRX_ENALS (1ULL << 1)
#define HCRX_ENASR (1ULL << 2)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 561bf2678e..ed5c728eab 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -193,9 +193,9 @@ static ARMMMUIdx ptw_idx_for_stage_2(CPUARMState *env,
ARMMMUIdx stage2idx)
return ARMMMUIdx_Phys_Realm;
case ARMSS_Secure:
if (stage2idx == ARMMMUIdx_Stage2_S) {
- s2walk_secure = !(env->cp15.vstcr_el2 & VSTCR_SW);
+ s2walk_secure = !(env->cp15.vstcr_el2 & R_VSTCR_SW_MASK);
FYI register API provides helper macros:
s2walk_secure = !FIELD_EX32(env->cp15.vstcr_el2, VSTCR, SW);
Do you know which form is currently preferred? I see that
R_<REGNAME>_<FIELD>_MASK is used a lot, .e.g, in helper.c.
Also, even tho the SW field in VSTCR is <= 31, VSTCR is a 64-bit register, so
should I really use FIELD_EX32 (which works) or FIELD_EX64 (my first thought would
be to use it, i.e. based on the register size, not the field)?
Thanks.
Cheers,
Gustavo