On Thu, 25 Sept 2025 at 12:07, Philippe Mathieu-Daudé <[email protected]> wrote:
>
> On 16/9/25 16:22, Richard Henderson wrote:
> > Hoist the computation of key into the caller, where
> > state is a known constant.
> > @@ -7587,18 +7582,22 @@ static void add_cpreg_to_hashtable_aa64(ARMCPU
> > *cpu, const ARMCPRegInfo *r,
> > */
> > ARMCPRegInfo nxs_ri = *r;
> > g_autofree char *name = g_strdup_printf("%sNXS", r->name);
> > + uint32_t nxs_key;
> >
> > assert(nxs_ri.crn < 0xf);
> > nxs_ri.crn++;
> > + nxs_key = key + (1 << CP_REG_ARM64_SYSREG_CRN_SHIFT);
>
> This is new but not mentioned. While the CRN bit is know to be 0,
> we usually use '|' to set a bit, not '+'. Preferably using '|':
I thought so too at first glance -- but what we're doing here
is adding one to crn (there's a comment in this function that's
just outside the context of the diff that explains this).
Since crn is both in the reginfo field and also encoded into
the key, we need to increment both the crn and the bitfield
inside the key. As it happens, at the moment all the regdefs
with ARM_CP_ADD_TLBI_NXS have crn == 8 and so whether we add
or OR makes no difference, but conceptually the addition is
correct.
I have added a comment
/* Also increment the CRN field inside the key value */
to hopefully make it a bit clearer that we're doing an
increment operation here.
thanks
-- PMM
