[PATCH] target/arm: Kick the other vCPUs on SEV under TCG round-robin

[PATCH] target/arm: Kick the other vCPUs on SEV under TCG round-robin

[Philippe Mathieu-Daudé]

In round-robin (single-thread) TCG, all vCPUs share one
host thread, so qemu_cpu_is_self() is true for every vCPU.

The SEV helper used it to skip the kick of the issuing vCPU,
and so skipped it for every other vCPU too: a vCPU sleeping
in WFE is only woken when the RR kick timer fires.

Compare against the issuing vCPU instead.

Fixes: d238858bff60 ("target/arm: Implement WFE, SEV and SEVONPEND")
Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
---
 target/arm/tcg/op_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/tcg/op_helper.c b/target/arm/tcg/op_helper.c
index 6a673e5ae6e8..173dd0e5151a 100644
--- a/target/arm/tcg/op_helper.c
+++ b/target/arm/tcg/op_helper.c
@@ -470,11 +470,11 @@ void HELPER(wfit)(CPUARMState *env, uint32_t rd)
 
 void HELPER(sev)(CPUARMState *env)
 {
-    CPUState *cs = env_cpu(env);
+    CPUState *self = env_cpu(env), *cs;
     CPU_FOREACH(cs) {
         ARMCPU *target_cpu = ARM_CPU(cs);
         target_cpu->env.event_register = true;
-        if (!qemu_cpu_is_self(cs)) {
+        if (cs != self) {
             qemu_cpu_kick(cs);
         }
     }
-- 
2.55.0

Re: [PATCH] target/arm: Kick the other vCPUs on SEV under TCG round-robin

[Philippe Mathieu-Daudé]

On 9/11/26 15:23, Philippe Mathieu-Daudé wrote:
> In round-robin (single-thread) TCG, all vCPUs share one
> host thread, so qemu_cpu_is_self() is true for every vCPU.
> 
> The SEV helper used it to skip the kick of the issuing vCPU,
> and so skipped it for every other vCPU too: a vCPU sleeping
> in WFE is only woken when the RR kick timer fires.
> 
> Compare against the issuing vCPU instead.
> 
> Fixes: d238858bff60 ("target/arm: Implement WFE, SEV and SEVONPEND")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
> ---
>   target/arm/tcg/op_helper.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/arm/tcg/op_helper.c b/target/arm/tcg/op_helper.c
> index 6a673e5ae6e8..173dd0e5151a 100644
> --- a/target/arm/tcg/op_helper.c
> +++ b/target/arm/tcg/op_helper.c
> @@ -470,11 +470,11 @@ void HELPER(wfit)(CPUARMState *env, uint32_t rd)
>   
>   void HELPER(sev)(CPUARMState *env)
>   {
> -    CPUState *cs = env_cpu(env);
> +    CPUState *self = env_cpu(env), *cs;

While rebasing I noticed I did that 1 week before latest ARM PR
and this conflicts with c5ec1bfad103 ("target/arm: don't pass env
to the sev helper"), then I noticed the previous 8def1f30d5d4
("target/arm: hold bql when SEV sets the event_register") which is
a distinct fix. This patch is still needed but need this to be
squashed now:

    -    CPUState *self = env_cpu(env), *cs;
    +    CPUState *self = current_cpu, *cs;

Or even simpler, check @current_cpu in place (no @self local var).

>       CPU_FOREACH(cs) {
>           ARMCPU *target_cpu = ARM_CPU(cs);
>           target_cpu->env.event_register = true;
> -        if (!qemu_cpu_is_self(cs)) {
> +        if (cs != self) {
>               qemu_cpu_kick(cs);
>           }
>       }

Re: [PATCH] target/arm: Kick the other vCPUs on SEV under TCG round-robin

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@oss.qualcomm.com> writes:

> In round-robin (single-thread) TCG, all vCPUs share one
> host thread, so qemu_cpu_is_self() is true for every vCPU.

This makes me think we should check other cases of qemu_cpu_is_self. For
example:

  void icount_handle_interrupt(CPUState *cpu, int mask)
  {
      int old_mask = cpu->interrupt_request;

      tcg_handle_interrupt(cpu, mask);
      if (qemu_cpu_is_self(cpu) &&
          !cpu->neg.can_do_io
          && (mask & ~old_mask) != 0) {
          cpu_abort(cpu, "Raised interrupt while not in I/O function");
      }
  }

Besides current_cpu does change even in rr so why I we testing against
thread rather than CPUState?

>
> The SEV helper used it to skip the kick of the issuing vCPU,
> and so skipped it for every other vCPU too: a vCPU sleeping
> in WFE is only woken when the RR kick timer fires.
>
> Compare against the issuing vCPU instead.
>
> Fixes: d238858bff60 ("target/arm: Implement WFE, SEV and SEVONPEND")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
> ---
>  target/arm/tcg/op_helper.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/arm/tcg/op_helper.c b/target/arm/tcg/op_helper.c
> index 6a673e5ae6e8..173dd0e5151a 100644
> --- a/target/arm/tcg/op_helper.c
> +++ b/target/arm/tcg/op_helper.c
> @@ -470,11 +470,11 @@ void HELPER(wfit)(CPUARMState *env, uint32_t rd)
>  
>  void HELPER(sev)(CPUARMState *env)
>  {
> -    CPUState *cs = env_cpu(env);
> +    CPUState *self = env_cpu(env), *cs;
>      CPU_FOREACH(cs) {
>          ARMCPU *target_cpu = ARM_CPU(cs);
>          target_cpu->env.event_register = true;
> -        if (!qemu_cpu_is_self(cs)) {
> +        if (cs != self) {
>              qemu_cpu_kick(cs);
>          }
>      }

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro