[PATCH 2/2] hvf: only update sysreg from owning thread

Mads Ynddal posted 2 patches 7 months, 2 weeks ago
Maintainers: Cameron Esfahani <dirty@apple.com>, Roman Bolshakov <rbolshakov@ddn.com>, Phil Dennis-Jordan <phil@philjordan.eu>, Alexander Graf <agraf@csgraf.de>, Peter Maydell <peter.maydell@linaro.org>
[PATCH 2/2] hvf: only update sysreg from owning thread
Posted by Mads Ynddal 7 months, 2 weeks ago
From: Mads Ynddal <m.ynddal@samsung.com>

hv_vcpu_set_sys_reg should only be called from the owning thread of the
vCPU, so to avoid crashes, the call to hvf_update_guest_debug is
dispatched to the individual threads.

Tested-by: Daniel Gomez <da.gomez@samsung.com>
Signed-off-by: Mads Ynddal <m.ynddal@samsung.com>
---
 accel/hvf/hvf-all.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/accel/hvf/hvf-all.c b/accel/hvf/hvf-all.c
index d404e01ade..3fc65d6b23 100644
--- a/accel/hvf/hvf-all.c
+++ b/accel/hvf/hvf-all.c
@@ -58,8 +58,13 @@ int hvf_sw_breakpoints_active(CPUState *cpu)
     return !QTAILQ_EMPTY(&hvf_state->hvf_sw_breakpoints);
 }
 
-int hvf_update_guest_debug(CPUState *cpu)
+static void do_hvf_update_guest_debug(CPUState *cpu, run_on_cpu_data arg)
 {
     hvf_arch_update_guest_debug(cpu);
+}
+
+int hvf_update_guest_debug(CPUState *cpu)
+{
+    run_on_cpu(cpu, do_hvf_update_guest_debug, RUN_ON_CPU_NULL);
     return 0;
 }
-- 
2.48.1
Re: [PATCH 2/2] hvf: only update sysreg from owning thread
Posted by Peter Maydell 6 months, 2 weeks ago
On Wed, 2 Apr 2025 at 14:52, Mads Ynddal <mads@ynddal.dk> wrote:
>
> From: Mads Ynddal <m.ynddal@samsung.com>
>
> hv_vcpu_set_sys_reg should only be called from the owning thread of the
> vCPU, so to avoid crashes, the call to hvf_update_guest_debug is
> dispatched to the individual threads.
>
> Tested-by: Daniel Gomez <da.gomez@samsung.com>
> Signed-off-by: Mads Ynddal <m.ynddal@samsung.com>
> ---
>  accel/hvf/hvf-all.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/accel/hvf/hvf-all.c b/accel/hvf/hvf-all.c
> index d404e01ade..3fc65d6b23 100644
> --- a/accel/hvf/hvf-all.c
> +++ b/accel/hvf/hvf-all.c
> @@ -58,8 +58,13 @@ int hvf_sw_breakpoints_active(CPUState *cpu)
>      return !QTAILQ_EMPTY(&hvf_state->hvf_sw_breakpoints);
>  }
>
> -int hvf_update_guest_debug(CPUState *cpu)
> +static void do_hvf_update_guest_debug(CPUState *cpu, run_on_cpu_data arg)
>  {
>      hvf_arch_update_guest_debug(cpu);
> +}
> +
> +int hvf_update_guest_debug(CPUState *cpu)
> +{
> +    run_on_cpu(cpu, do_hvf_update_guest_debug, RUN_ON_CPU_NULL);
>      return 0;
>  }

run_on_cpu() implicitly drops the BQL, so it is a potential
really nasty footgun in this kind of situation where callers
to update_guest_debug aren't expecting the BQL to be dropped
on them. But we already use run_on_cpu() in the KVM equivalent
kvm_update_guest_debug(), so presumably it's also OK here...

-- PMM
Re: [PATCH 2/2] hvf: only update sysreg from owning thread
Posted by Alex Bennée 7 months ago
Mads Ynddal <mads@ynddal.dk> writes:

> From: Mads Ynddal <m.ynddal@samsung.com>
>
> hv_vcpu_set_sys_reg should only be called from the owning thread of the
> vCPU, so to avoid crashes, the call to hvf_update_guest_debug is
> dispatched to the individual threads.
>
> Tested-by: Daniel Gomez <da.gomez@samsung.com>
> Signed-off-by: Mads Ynddal <m.ynddal@samsung.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro