Re: [RFC PATCH] target/arm: hold bql when SEV sets the event_register

["Alex Bennée" <alex.bennee@linaro.org>]

Alex Bennée <alex.bennee@linaro.org> writes:

> There is a race in qemu_process_cpu_events and sleeping on halt_cond
> where we can miss the setting of the event_register which would cause
> arm_cpu_has_work to report we shouldn't sleep. As by definition the
> bql has to be held when we go to sleep against halt_cond guard the
> kicking of the cpus by the SEV helper with the BQL to ensure things
> are properly serialised.
>
> The other places that set event_register are fine as they are either
> in the same vCPU context or are already serialised by the BQL.
>
> Also use qemu_cond_wait_bql like the other halt_cond users.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4660

This also fixes 4670 

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  system/cpus.c              | 2 +-
>  target/arm/tcg/op_helper.c | 3 +++
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/system/cpus.c b/system/cpus.c
> index e11a5aab6a6..985732217fb 100644
> --- a/system/cpus.c
> +++ b/system/cpus.c
> @@ -412,7 +412,7 @@ void qemu_process_cpu_events(CPUState *cpu)
>              slept = true;
>              qemu_plugin_vcpu_idle_cb(cpu);
>          }
> -        qemu_cond_wait(cpu->halt_cond, &bql);
> +        qemu_cond_wait_bql(cpu->halt_cond);
>      }
>      if (slept) {
>          qemu_plugin_vcpu_resume_cb(cpu);
> diff --git a/target/arm/tcg/op_helper.c b/target/arm/tcg/op_helper.c
> index af56eff9f18..7c1fdab79d7 100644
> --- a/target/arm/tcg/op_helper.c
> +++ b/target/arm/tcg/op_helper.c
> @@ -471,6 +471,9 @@ void HELPER(wfit)(CPUARMState *env, uint32_t rd)
>  void HELPER(sev)(CPUARMState *env)
>  {
>      CPUState *cs = env_cpu(env);
> +
> +    BQL_LOCK_GUARD();
> +
>      CPU_FOREACH(cs) {
>          ARMCPU *target_cpu = ARM_CPU(cs);
>          target_cpu->env.event_register = true;

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro