arm: force flag recalculation when messing with DAIF

[RFC PATCH] arm: force flag recalculation when messing with DAIF

Posted by Alex Bennée 2 years, 3 months ago

The recently introduced debug tests in kvm-unit-tests exposed an error
in our handling of singlestep cause by stale hflags. This is caught by
--enable-debug-tcg when running the tests.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Andrew Jones <drjones@redhat.com>
---
 target/arm/helper-a64.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/arm/helper-a64.c b/target/arm/helper-a64.c
index d6a6fd73d9..7cf953b1e6 100644
--- a/target/arm/helper-a64.c
+++ b/target/arm/helper-a64.c
@@ -83,12 +83,14 @@ void HELPER(msr_i_daifset)(CPUARMState *env, uint32_t imm)
 {
     daif_check(env, 0x1e, imm, GETPC());
     env->daif |= (imm << 6) & PSTATE_DAIF;
+    arm_rebuild_hflags(env);
 }
 
 void HELPER(msr_i_daifclear)(CPUARMState *env, uint32_t imm)
 {
     daif_check(env, 0x1f, imm, GETPC());
     env->daif &= ~((imm << 6) & PSTATE_DAIF);
+    arm_rebuild_hflags(env);
 }
 
 /* Convert a softfloat float_relation_ (as returned by
-- 
2.30.2

Re: [RFC PATCH] arm: force flag recalculation when messing with DAIF

Posted by Richard Henderson 2 years, 2 months ago

On 2/2/22 23:23, Alex Bennée wrote:
> The recently introduced debug tests in kvm-unit-tests exposed an error
> in our handling of singlestep cause by stale hflags. This is caught by
> --enable-debug-tcg when running the tests.

You could mention that PSTATE_D is used by aa64_generate_debug_exceptions.

> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Andrew Jones <drjones@redhat.com>
> ---
>   target/arm/helper-a64.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/target/arm/helper-a64.c b/target/arm/helper-a64.c
> index d6a6fd73d9..7cf953b1e6 100644
> --- a/target/arm/helper-a64.c
> +++ b/target/arm/helper-a64.c
> @@ -83,12 +83,14 @@ void HELPER(msr_i_daifset)(CPUARMState *env, uint32_t imm)
>   {
>       daif_check(env, 0x1e, imm, GETPC());
>       env->daif |= (imm << 6) & PSTATE_DAIF;
> +    arm_rebuild_hflags(env);
>   }
>   
>   void HELPER(msr_i_daifclear)(CPUARMState *env, uint32_t imm)
>   {
>       daif_check(env, 0x1f, imm, GETPC());
>       env->daif &= ~((imm << 6) & PSTATE_DAIF);
> +    arm_rebuild_hflags(env);
>   }

You could call helper_rebuild_hflags_a64 directly, since we know that we must be in a64 
state to get here.  Regardless,

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [RFC PATCH] arm: force flag recalculation when messing with DAIF

Posted by Peter Maydell 2 years, 2 months ago

On Wed, 2 Feb 2022 at 12:24, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> The recently introduced debug tests in kvm-unit-tests exposed an error
> in our handling of singlestep cause by stale hflags. This is caught by
> --enable-debug-tcg when running the tests.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Andrew Jones <drjones@redhat.com>
> ---
>  target/arm/helper-a64.c | 2 ++
>  1 file changed, 2 insertions(+)



Applied to target-arm.next, thanks.

-- PMM

Re: [RFC PATCH] arm: force flag recalculation when messing with DAIF

Posted by Andrew Jones 2 years, 3 months ago

On Wed, Feb 02, 2022 at 12:23:53PM +0000, Alex Bennée wrote:
> The recently introduced debug tests in kvm-unit-tests exposed an error
> in our handling of singlestep cause by stale hflags. This is caught by
> --enable-debug-tcg when running the tests.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Andrew Jones <drjones@redhat.com>

s/Cc: Andrew/Reported-by: Andrew/

and now also

Tested-by: Andrew Jones <drjones@redhat.com>

Thanks,
drew

> ---
>  target/arm/helper-a64.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/target/arm/helper-a64.c b/target/arm/helper-a64.c
> index d6a6fd73d9..7cf953b1e6 100644
> --- a/target/arm/helper-a64.c
> +++ b/target/arm/helper-a64.c
> @@ -83,12 +83,14 @@ void HELPER(msr_i_daifset)(CPUARMState *env, uint32_t imm)
>  {
>      daif_check(env, 0x1e, imm, GETPC());
>      env->daif |= (imm << 6) & PSTATE_DAIF;
> +    arm_rebuild_hflags(env);
>  }
>  
>  void HELPER(msr_i_daifclear)(CPUARMState *env, uint32_t imm)
>  {
>      daif_check(env, 0x1f, imm, GETPC());
>      env->daif &= ~((imm << 6) & PSTATE_DAIF);
> +    arm_rebuild_hflags(env);
>  }
>  
>  /* Convert a softfloat float_relation_ (as returned by
> -- 
> 2.30.2
>