Re: [PATCH] target/arm: fix s2mmu input size check

[Keisuke Iida]

Address Translation Fault is triggered when PA size set by VTCR_EL2.PS is less than IPA size set by VTCR_EL2.T0SZ on the guest. (e.g. vtcr_el2.PS = 1 && vtcr_el2.T0SZ = 25. PA size is 36bit, and IPA size is 39bit.)

        ps = FIELD_EX64(cpu->isar.id_aa64mmfr0, ID_AA64MMFR0, PARANGE);
        ps = MIN(ps, param.ps);
        assert(ps < ARRAY_SIZE(pamax_map));
        outputsize = pamax_map[ps];

When 'param.ps' determined by VTCR_EL2.PS less than 'ps', 'outputsize' is set to PA address by VTCR_EL2.PS.

--
Keisuke Iida

On 2022/05/05 17:20, Peter Maydell wrote:

> On Thu, 5 May 2022 at 01:40, <mkei@sfc.wide.ad.jp> wrote:
> > From: Keisuke Iida <mkei@sfc.wide.ad.jp>
> >
> > The maximum IPA size('inputsize') is constrained by the implemented PA size that is
> > specified by ID_AA64MMFR0_EL1.PARange. Please reference Arm Architecture Reference
> > Manual for A-profile architecture "Supported IPA size" on page D5-4788.
> >
> > Signed-off-by: Keisuke Iida <mkei@sfc.wide.ad.jp>
> > ---
> >  target/arm/helper.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/target/arm/helper.c b/target/arm/helper.c
> > index 5a244c3ed9..868e7a2c0b 100644
> > --- a/target/arm/helper.c
> > +++ b/target/arm/helper.c
> > @@ -11116,7 +11116,7 @@ static bool check_s2_mmu_setup(ARMCPU *cpu, bool is_aa64, int level,
> >          }
> >
> >          /* Inputsize checks.  */
> > -        if (inputsize > outputsize &&
> > +        if (inputsize > arm_pamax(cpu) &&
> >              (arm_el_is_aa64(&cpu->env, 1) || inputsize > 40)) {
> >              /* This is CONSTRAINED UNPREDICTABLE and we choose to fault.  */
> >              return false;
> >       
> Can you give an example, eg a test case, where you see wrong
> behaviour? The 'outputsize' variable in this function is
> passed in from the caller get_phys_addr_lpae(), where (for
> an AArch64 guest) it is indeed constrained to the value
> of ID_AA64MMFR0.PARange:
>
>         /*
>          * Bound PS by PARANGE to find the effective output address size.
>          * ID_AA64MMFR0 is a read-only register so values outside of the
>          * supported mappings can be considered an implementation error.
>          */
>         ps = FIELD_EX64(cpu->isar.id_aa64mmfr0, ID_AA64MMFR0, PARANGE);
>         ps = MIN(ps, param.ps);
>         assert(ps < ARRAY_SIZE(pamax_map));
>         outputsize = pamax_map[ps];
>
>
> thanks
> -- PMM
>