* [PATCH] target/arm: fix s2mmu input size check
@ 2022-05-05 3:12 mkei
2022-05-05 16:13 ` Richard Henderson
0 siblings, 1 reply; 6+ messages in thread
From: mkei @ 2022-05-05 3:12 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell, Keisuke Iida
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] target/arm: fix s2mmu input size check
2022-05-05 3:12 [PATCH] target/arm: fix s2mmu input size check mkei
@ 2022-05-05 16:13 ` Richard Henderson
2022-05-07 14:09 ` Keisuke Iida
0 siblings, 1 reply; 6+ messages in thread
From: Richard Henderson @ 2022-05-05 16:13 UTC (permalink / raw)
To: mkei, qemu-devel; +Cc: qemu-arm, Peter Maydell
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~
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] target/arm: fix s2mmu input size check
2022-05-05 16:13 ` Richard Henderson
@ 2022-05-07 14:09 ` Keisuke Iida
0 siblings, 0 replies; 6+ messages in thread
From: Keisuke Iida @ 2022-05-07 14:09 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-arm, Peter Maydell
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] target/arm: fix s2mmu input size check
@ 2022-05-05 0:40 mkei
2022-05-05 8:20 ` Peter Maydell
0 siblings, 1 reply; 6+ messages in thread
From: mkei @ 2022-05-05 0:40 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell, Keisuke Iida
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] target/arm: fix s2mmu input size check
2022-05-05 0:40 mkei
@ 2022-05-05 8:20 ` Peter Maydell
2022-05-05 12:30 ` Keisuke Iida
0 siblings, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2022-05-05 8:20 UTC (permalink / raw)
To: mkei; +Cc: qemu-devel, qemu-arm
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] target/arm: fix s2mmu input size check
2022-05-05 8:20 ` Peter Maydell
@ 2022-05-05 12:30 ` Keisuke Iida
0 siblings, 0 replies; 6+ messages in thread
From: Keisuke Iida @ 2022-05-05 12:30 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel, qemu-arm
[-- Attachment #1: Type: text/plain, Size: 2399 bytes --]
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
[-- Attachment #2: Type: text/html, Size: 3099 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-05-07 14:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-05-05 3:12 [PATCH] target/arm: fix s2mmu input size check mkei
2022-05-05 16:13 ` Richard Henderson
2022-05-07 14:09 ` Keisuke Iida
-- strict thread matches above, loose matches on Subject: below --
2022-05-05 0:40 mkei
2022-05-05 8:20 ` Peter Maydell
2022-05-05 12:30 ` Keisuke Iida
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).