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

[Peter Maydell]

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