[PATCH] target/arm: Restrict translation disabled alignment check to VMSA

[PATCH] target/arm: Restrict translation disabled alignment check to VMSA

[Richard Henderson]

For cpus using PMSA, when the MPU is disabled, the default memory
type is Normal, Non-cachable.

Fixes: 59754f85ed3 ("target/arm: Do memory type alignment check when translation disabled")
Reported-by: Clément Chigot <chigot@adacore.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---

Since v9 will likely be tagged tomorrow without this fixed,
Cc: qemu-stable@nongnu.org

---
 target/arm/tcg/hflags.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/target/arm/tcg/hflags.c b/target/arm/tcg/hflags.c
index 5da1b0fc1d..66de30b828 100644
--- a/target/arm/tcg/hflags.c
+++ b/target/arm/tcg/hflags.c
@@ -38,8 +38,16 @@ static bool aprofile_require_alignment(CPUARMState *env, int el, uint64_t sctlr)
     }
 
     /*
-     * If translation is disabled, then the default memory type is
-     * Device(-nGnRnE) instead of Normal, which requires that alignment
+     * With PMSA, when the MPU is disabled, all memory types in the
+     * default map is Normal.
+     */
+    if (arm_feature(env, ARM_FEATURE_PMSA)) {
+        return false;
+    }
+
+    /*
+     * With VMSA, if translation is disabled, then the default memory type
+     * is Device(-nGnRnE) instead of Normal, which requires that alignment
      * be enforced.  Since this affects all ram, it is most efficient
      * to handle this during translation.
      */
-- 
2.34.1

Re: [PATCH] target/arm: Restrict translation disabled alignment check to VMSA

[Richard Henderson]

On 4/22/24 10:07, Richard Henderson wrote:
> For cpus using PMSA, when the MPU is disabled, the default memory
> type is Normal, Non-cachable.
> 
> Fixes: 59754f85ed3 ("target/arm: Do memory type alignment check when translation disabled")
> Reported-by: Clément Chigot <chigot@adacore.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> 
> Since v9 will likely be tagged tomorrow without this fixed,
> Cc: qemu-stable@nongnu.org
> 
> ---
>   target/arm/tcg/hflags.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/target/arm/tcg/hflags.c b/target/arm/tcg/hflags.c
> index 5da1b0fc1d..66de30b828 100644
> --- a/target/arm/tcg/hflags.c
> +++ b/target/arm/tcg/hflags.c
> @@ -38,8 +38,16 @@ static bool aprofile_require_alignment(CPUARMState *env, int el, uint64_t sctlr)
>       }
>   
>       /*
> -     * If translation is disabled, then the default memory type is
> -     * Device(-nGnRnE) instead of Normal, which requires that alignment
> +     * With PMSA, when the MPU is disabled, all memory types in the
> +     * default map is Normal.
> +     */
> +    if (arm_feature(env, ARM_FEATURE_PMSA)) {
> +        return false;
> +    }
> +
> +    /*
> +     * With VMSA, if translation is disabled, then the default memory type
> +     * is Device(-nGnRnE) instead of Normal, which requires that alignment
>        * be enforced.  Since this affects all ram, it is most efficient
>        * to handle this during translation.
>        */

Oh, I meant to add: since the armv7 manual has both VMSA and PMSA sections, and the 
language about default Device type and alignment traps, is in the VMSA section.
This will at least fix our two r-profile cpus.

r~

Re: [PATCH] target/arm: Restrict translation disabled alignment check to VMSA

[Philippe Mathieu-Daudé]

On 22/4/24 19:09, Richard Henderson wrote:
> On 4/22/24 10:07, Richard Henderson wrote:
>> For cpus using PMSA, when the MPU is disabled, the default memory
>> type is Normal, Non-cachable.
>>
>> Fixes: 59754f85ed3 ("target/arm: Do memory type alignment check when 
>> translation disabled")
>> Reported-by: Clément Chigot <chigot@adacore.com>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>
>> Since v9 will likely be tagged tomorrow without this fixed,
>> Cc: qemu-stable@nongnu.org
>>
>> ---
>>   target/arm/tcg/hflags.c | 12 ++++++++++--
>>   1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/arm/tcg/hflags.c b/target/arm/tcg/hflags.c
>> index 5da1b0fc1d..66de30b828 100644
>> --- a/target/arm/tcg/hflags.c
>> +++ b/target/arm/tcg/hflags.c
>> @@ -38,8 +38,16 @@ static bool aprofile_require_alignment(CPUARMState 
>> *env, int el, uint64_t sctlr)
>>       }
>>       /*
>> -     * If translation is disabled, then the default memory type is
>> -     * Device(-nGnRnE) instead of Normal, which requires that alignment
>> +     * With PMSA, when the MPU is disabled, all memory types in the
>> +     * default map is Normal.
>> +     */
>> +    if (arm_feature(env, ARM_FEATURE_PMSA)) {
>> +        return false;
>> +    }
>> +
>> +    /*
>> +     * With VMSA, if translation is disabled, then the default memory 
>> type
>> +     * is Device(-nGnRnE) instead of Normal, which requires that 
>> alignment
>>        * be enforced.  Since this affects all ram, it is most efficient
>>        * to handle this during translation.
>>        */
> 
> Oh, I meant to add: since the armv7 manual has both VMSA and PMSA 
> sections, and the language about default Device type and alignment 
> traps, is in the VMSA section.

To the best of my knowledge,
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH] target/arm: Restrict translation disabled alignment check to VMSA

[Clément Chigot]

On Mon, Apr 22, 2024 at 11:02 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> On 22/4/24 19:09, Richard Henderson wrote:
> > On 4/22/24 10:07, Richard Henderson wrote:
> >> For cpus using PMSA, when the MPU is disabled, the default memory
> >> type is Normal, Non-cachable.
> >>
> >> Fixes: 59754f85ed3 ("target/arm: Do memory type alignment check when
> >> translation disabled")
> >> Reported-by: Clément Chigot <chigot@adacore.com>
> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> >> ---
> >>
> >> Since v9 will likely be tagged tomorrow without this fixed,
> >> Cc: qemu-stable@nongnu.org
> >>
> >> ---
> >>   target/arm/tcg/hflags.c | 12 ++++++++++--
> >>   1 file changed, 10 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/target/arm/tcg/hflags.c b/target/arm/tcg/hflags.c
> >> index 5da1b0fc1d..66de30b828 100644
> >> --- a/target/arm/tcg/hflags.c
> >> +++ b/target/arm/tcg/hflags.c
> >> @@ -38,8 +38,16 @@ static bool aprofile_require_alignment(CPUARMState
> >> *env, int el, uint64_t sctlr)
> >>       }
> >>       /*
> >> -     * If translation is disabled, then the default memory type is
> >> -     * Device(-nGnRnE) instead of Normal, which requires that alignment
> >> +     * With PMSA, when the MPU is disabled, all memory types in the
> >> +     * default map is Normal.
> >> +     */
> >> +    if (arm_feature(env, ARM_FEATURE_PMSA)) {
> >> +        return false;
> >> +    }
> >> +
> >> +    /*
> >> +     * With VMSA, if translation is disabled, then the default memory
> >> type
> >> +     * is Device(-nGnRnE) instead of Normal, which requires that
> >> alignment
> >>        * be enforced.  Since this affects all ram, it is most efficient
> >>        * to handle this during translation.
> >>        */
> >
> > Oh, I meant to add: since the armv7 manual has both VMSA and PMSA
> > sections, and the language about default Device type and alignment
> > traps, is in the VMSA section.
>
> To the best of my knowledge,
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Thanks for the patch.

Tested-by: Clément Chigot <chigot@adacore.com>

Re: [PATCH] target/arm: Restrict translation disabled alignment check to VMSA

[Peter Maydell]

On Mon, 22 Apr 2024 at 18:07, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> For cpus using PMSA, when the MPU is disabled, the default memory
> type is Normal, Non-cachable.
>
> Fixes: 59754f85ed3 ("target/arm: Do memory type alignment check when translation disabled")
> Reported-by: Clément Chigot <chigot@adacore.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>
> Since v9 will likely be tagged tomorrow without this fixed,
> Cc: qemu-stable@nongnu.org

Applied to target-arm.next (with the cc:stable added in and a couple of
trivial tweaks to the comment/commit message), thanks.

-- PMM

[PATCH 0/5] tcg: Misc improvements

[Richard Henderson]

One patch to allow two output operands from gvec expansion,
to be used by target/arm for updating QC.

One patch to record the result of the generic breakpoint
search so target translators do not need to repeat it.

Three small optimization patches.


r~


Richard Henderson (5):
  tcg: Add write_aofs to GVecGen3i
  tcg/i386: Simplify immediate 8-bit logical vector shifts
  tcg/i386: Optimize setcond of TST{EQ,NE} with 0xffffffff
  tcg/optimize: Optimize setcond with zmask
  accel/tcg: Introduce CF_BP_PAGE

 include/exec/translation-block.h |   1 +
 include/tcg/tcg-op-gvec-common.h |   2 +
 accel/tcg/cpu-exec.c             |   2 +-
 tcg/optimize.c                   | 110 +++++++++++++++++++++++++++++++
 tcg/tcg-op-gvec.c                |  30 ++++++---
 tcg/i386/tcg-target.c.inc        |  78 ++++++++--------------
 6 files changed, 165 insertions(+), 58 deletions(-)

-- 
2.34.1

[PATCH] target/arm: Restrict translation disabled alignment check to VMSA

[Richard Henderson]

For cpus using PMSA, when the MPU is disabled, the default memory
type is Normal, Non-cachable.

Fixes: 59754f85ed3 ("target/arm: Do memory type alignment check when translation disabled")
Reported-by: Clément Chigot <chigot@adacore.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---

Since v9 will likely be tagged tomorrow without this fixed,
Cc: qemu-stable@nongnu.org

---
 target/arm/tcg/hflags.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/target/arm/tcg/hflags.c b/target/arm/tcg/hflags.c
index 5da1b0fc1d..66de30b828 100644
--- a/target/arm/tcg/hflags.c
+++ b/target/arm/tcg/hflags.c
@@ -38,8 +38,16 @@ static bool aprofile_require_alignment(CPUARMState *env, int el, uint64_t sctlr)
     }
 
     /*
-     * If translation is disabled, then the default memory type is
-     * Device(-nGnRnE) instead of Normal, which requires that alignment
+     * With PMSA, when the MPU is disabled, all memory types in the
+     * default map is Normal.
+     */
+    if (arm_feature(env, ARM_FEATURE_PMSA)) {
+        return false;
+    }
+
+    /*
+     * With VMSA, if translation is disabled, then the default memory type
+     * is Device(-nGnRnE) instead of Normal, which requires that alignment
      * be enforced.  Since this affects all ram, it is most efficient
      * to handle this during translation.
      */
-- 
2.34.1

Re: [PATCH] target/arm: Restrict translation disabled alignment check to VMSA

[Philippe Mathieu-Daudé]

On 24/4/24 19:09, Richard Henderson wrote:
> For cpus using PMSA, when the MPU is disabled, the default memory
> type is Normal, Non-cachable.
> 
> Fixes: 59754f85ed3 ("target/arm: Do memory type alignment check when translation disabled")
> Reported-by: Clément Chigot <chigot@adacore.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> 
> Since v9 will likely be tagged tomorrow without this fixed,
> Cc: qemu-stable@nongnu.org
> 
> ---
>   target/arm/tcg/hflags.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/target/arm/tcg/hflags.c b/target/arm/tcg/hflags.c
> index 5da1b0fc1d..66de30b828 100644
> --- a/target/arm/tcg/hflags.c
> +++ b/target/arm/tcg/hflags.c
> @@ -38,8 +38,16 @@ static bool aprofile_require_alignment(CPUARMState *env, int el, uint64_t sctlr)
>       }
>   
>       /*
> -     * If translation is disabled, then the default memory type is
> -     * Device(-nGnRnE) instead of Normal, which requires that alignment
> +     * With PMSA, when the MPU is disabled, all memory types in the
> +     * default map is Normal.
> +     */
> +    if (arm_feature(env, ARM_FEATURE_PMSA)) {
> +        return false;
> +    }
> +
> +    /*
> +     * With VMSA, if translation is disabled, then the default memory type
> +     * is Device(-nGnRnE) instead of Normal, which requires that alignment
>        * be enforced.  Since this affects all ram, it is most efficient
>        * to handle this during translation.
>        */

This one is in target-arm.next:
https://lore.kernel.org/qemu-devel/CAFEAcA98UrBLsAXKzLSkUnC2G_RZd56veqUkSGSttoADfkEKGA@mail.gmail.com/

Re: [PATCH] target/arm: Restrict translation disabled alignment check to VMSA

[Richard Henderson]

On 5/3/24 07:58, Philippe Mathieu-Daudé wrote:
> On 24/4/24 19:09, Richard Henderson wrote:
>> For cpus using PMSA, when the MPU is disabled, the default memory
>> type is Normal, Non-cachable.
>>
>> Fixes: 59754f85ed3 ("target/arm: Do memory type alignment check when translation disabled")
>> Reported-by: Clément Chigot <chigot@adacore.com>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>
>> Since v9 will likely be tagged tomorrow without this fixed,
>> Cc: qemu-stable@nongnu.org
>>
>> ---
>>   target/arm/tcg/hflags.c | 12 ++++++++++--
>>   1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/arm/tcg/hflags.c b/target/arm/tcg/hflags.c
>> index 5da1b0fc1d..66de30b828 100644
>> --- a/target/arm/tcg/hflags.c
>> +++ b/target/arm/tcg/hflags.c
>> @@ -38,8 +38,16 @@ static bool aprofile_require_alignment(CPUARMState *env, int el, 
>> uint64_t sctlr)
>>       }
>>       /*
>> -     * If translation is disabled, then the default memory type is
>> -     * Device(-nGnRnE) instead of Normal, which requires that alignment
>> +     * With PMSA, when the MPU is disabled, all memory types in the
>> +     * default map is Normal.
>> +     */
>> +    if (arm_feature(env, ARM_FEATURE_PMSA)) {
>> +        return false;
>> +    }
>> +
>> +    /*
>> +     * With VMSA, if translation is disabled, then the default memory type
>> +     * is Device(-nGnRnE) instead of Normal, which requires that alignment
>>        * be enforced.  Since this affects all ram, it is most efficient
>>        * to handle this during translation.
>>        */
> 
> This one is in target-arm.next:
> https://lore.kernel.org/qemu-devel/CAFEAcA98UrBLsAXKzLSkUnC2G_RZd56veqUkSGSttoADfkEKGA@mail.gmail.com/

Yes, that was a stray patch that accidentally got re-posted with this series.


r~