On Tue, 27 Oct 2026 at 12:01, Mohamed Mediouni <[email protected]> wrote:
>
> When a vCPU reset happens, we should also
> reset the state of the GIC CPU interface.
>
> Otherwise, reboots might not work as expected
> during some circumstances.
>
> Signed-off-by: Mohamed Mediouni <[email protected]>
> @@ -629,6 +640,7 @@ static void hvf_gicv3_set_irq(void *opaque, int irq, int
> level)
> static void hvf_gicv3_icc_reset(CPUARMState *env, const ARMCPRegInfo *ri)
> {
> GICv3CPUState *c;
> + run_on_cpu_data data;
>
> c = env->gicv3state;
> c->icc_pmr_el1 = 0;
> @@ -649,6 +661,9 @@ static void hvf_gicv3_icc_reset(CPUARMState *env, const
> ARMCPRegInfo *ri)
> c->icc_sre_el1 = 0x7;
> memset(c->icc_apr, 0, sizeof(c->icc_apr));
> memset(c->icc_igrpen, 0, sizeof(c->icc_igrpen));
> +
> + data.host_ptr = env->gicv3state;
> + run_on_cpu(env_cpu(env), hvf_gicv3_put_cpu, data);
This isn't safe to call here. run_on_cpu() has an extremely
nasty beartrap built in, which is that it will unlock the BQL
while it is waiting for the queued work to complete. The
code that calls these register reset functions doesn't
expect to have the BQL dropped underneath it, and there
might be various race conditions that happen if it does.
I think the existing use of run_on_cpu() in hvf_gicv3_put()
is also broken for a similar reason.
Put another way, run_on_cpu() is extremely hard to use
safely; I think you can only really use it if you're calling
it directly from code that was itself called with the BQL
initially not held, and which is therefore dealing with the
BQL directly and can be careful not to assume that state
that it looked at before the run_on_cpu() is still valid
after run_on_cpu() returns; and if you do use it like that
you probably want clear commentary warning about the
potential problems.
thanks
-- PMM
