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).
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.
Personally I don't feel strongly about any of this, so I
would be happy taking this patch the way it is.
-- PMM
