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

mkei@sfc.wide.ad.jp posted 1 patch 2 years ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220505031234.20349-1-mkei@sfc.wide.ad.jp
Maintainers: Peter Maydell <peter.maydell@linaro.org>
target/arm/helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] target/arm: fix s2mmu input size check
Posted by mkei@sfc.wide.ad.jp 2 years ago
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;
-- 
2.34.1
Re: [PATCH] target/arm: fix s2mmu input size check
Posted by Richard Henderson 2 years ago
On 5/4/22 22:12, 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) &&


This is incorrect -- arm_pamax has already been taken into account in computing 
outputsize.  There are many more constraints than just this.

You need to have a look at the computation of ps and tsz in aa64_va_parameters, and then 
the computation of outputsize near the beginning of get_phys_addr_lpae, which takes 
arm_pamax into account by bounding ps against ID_AA64MMFR0.PARANGE, and pamax_map.

What problem are you encountering?


r~
Re: [PATCH] target/arm: fix s2mmu input size check
Posted by Keisuke Iida 2 years ago
On 2022/05/06 1:13, Richard Henderson wrote:
> On 5/4/22 22:12, 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) &&
>
>
> This is incorrect -- arm_pamax has already been taken into account in 
> computing outputsize.  There are many more constraints than just this.
>
> You need to have a look at the computation of ps and tsz in 
> aa64_va_parameters, and then the computation of outputsize near the 
> beginning of get_phys_addr_lpae, which takes arm_pamax into account by 
> bounding ps against ID_AA64MMFR0.PARANGE, and pamax_map.
>
> What problem are you encountering?
>
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.)

In this case, inputsize = 39 and outputsize = 36, so inputsize > 
outputsize is true and a fault is triggered.

This behavior means that the effective minimum VTCR_EL2.T0SZ value 
depends on VTCR_EL2.PS set by guest.

Is this the expected behavior for qemu?

As a side note, this does not happen before qemu 6.2.

>
> r~

--

Keisuke Iida <mkei@sfc.wide.ad.jp>