Re: [PATCH 37/82] target/arm: Convert regime_is_user from switch to table

[Peter Maydell <peter.maydell@linaro.org>]

On Fri, 1 Aug 2025 at 04:54, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 7/31/25 07:21, Pierrick Bouvier wrote:
> >>   #include "mmuidx-internal.h"
> >> -#define EL(X)  ((X << R_MMUIDXINFO_EL_SHIFT) | R_MMUIDXINFO_ELVALID_MASK)
> >> +#define EL(X)  ((X << R_MMUIDXINFO_EL_SHIFT) | R_MMUIDXINFO_ELVALID_MASK | \
> >> +                ((X == 0) << R_MMUIDXINFO_USER_SHIFT))
> >>   #define REL(X) ((X << R_MMUIDXINFO_REL_SHIFT) | R_MMUIDXINFO_RELVALID_MASK)
> >>   #define R2     R_MMUIDXINFO_2RANGES_MASK
> >>   #define PAN    R_MMUIDXINFO_PAN_MASK
> >> +#define USER   R_MMUIDXINFO_USER_MASK
> >>   const uint32_t arm_mmuidx_table[ARM_MMU_IDX_M + 8] = {
> >>       /*
> >> @@ -33,7 +35,7 @@ const uint32_t arm_mmuidx_table[ARM_MMU_IDX_M + 8] = {
> >>       [ARMMMUIdx_Stage2_S]        = REL(2),
> >>       [ARMMMUIdx_Stage2]          = REL(2),
> >> -    [ARMMMUIdx_Stage1_E0]       = REL(1) | R2,
> >> +    [ARMMMUIdx_Stage1_E0]       = REL(1) | R2 | USER,
> >>       [ARMMMUIdx_Stage1_E1]       = REL(1) | R2,
> >>       [ARMMMUIdx_Stage1_E1_PAN]   = REL(1) | R2 | PAN,
> >
> > Maybe I missed something, but what about other entries that were initially treated in the
> > switch?
> > - ARMMMUIdx_E.0_0
> > - ARMMMUIdx_M*User
>
> See the change to EL().
>
> I'm not sure why ARMMMUIdx_Stage1_* is excluded from arm_mmu_idx_to_el(), but I don't
> change that in this patch series.

It's always been that way through various refactorings
of the mmu index, back to commit c1e3781090b9d36 when
the function was added.

In practice we only use arm_mmu_idx_to_el() to get back to
the EL from the MMU index that we put into the TB flags.
So we know it's always one of the "complete translation"
index values, not a Stage1-only, Stage2-only or Phys index.

My guess is I originally put in the assert that enforced
that you don't call it with either a Stage2-only or a
Stage1-only mmuidx because I knew that couldn't happen and
it meant I could implement the idx-to-EL code for the
valid cases as "mmu_idx & 3" and didn't need to add
extra code to handle a Stage1-only index the function
would never see.

thanks
-- PMM