Hi Peter,
On 7/25/25 11:47, Peter Maydell wrote:
On Fri, 25 Jul 2025 at 15:38, Gustavo Romero <gustavo.rom...@linaro.org> wrote:
Hi Phil,
On 7/25/25 10:18, Philippe Mathieu-Daudé wrote:
Hi Gustavo,
On 25/7/25 03:47, Gustavo Romero wrote:
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.
Where a mask is the most simple or useful way to get what
you want, it's fine to use R_*_MASK directly; for instance
in this patch
(env->cp15.vstcr_el2 & (R_VSTCR_SA_MASK | R_VSTCR_SW_MASK)
is better than
(FIELD_EX64(env->cp15.vstcr_el2, VSTCR, SA) ||
FIELD_EX64(env->cp15.vstcr_el2, VSTCR, SW))
and
value |= R_FOO_BAR_MASK;
seems simpler than
value = FIELD_DP64(value, FOO, BAR, 1);
I think (though this is to some extent a matter of personal taste).
I also prefer the mask for the most simple cases, like in 1 bit masks and where
the code becomes more succinct.
Where we're using the mask to test whether a single bit field
is set or not, you could do it either way. I think in new code
I would probably use the FIELD_EX64 macro, but a test against
the mask value is fine too.
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)?
Use FIELD_EX64 for working on 64-bit values, yes.
Thanks for the clarifications.
Personally I don't feel strongly about any of this, so I
would be happy taking this patch the way it is.
-- PMM
OK, thanks.
Cheers,
Gustavo
