| Message ID | 20260923171116.31276-18-philmd@oss.qualcomm.com |
|---|---|
| State | New |
| Headers | show |
| Series | accel/tcg: Make halt-to-exec transition explicit and remove cpu_exec_halt | expand |
On 9/23/26 10:11, Philippe Mathieu-Daudé wrote: > x86_cpu_has_work() may run without the BQL, but currently reads > CPUState::interrupt_request directly. Other threads update this > field with qatomic_or() in cpu_set_interrupt(), so the direct > read races with an atomic access. > > Load the interrupt request with qatomic_load_acquire(), matching > cpu_test_interrupt(). This provides the acquire ordering required > before x86_cpu_pending_interrupt() inspects the CPU state. > > Suggested-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com> > --- > target/i386/cpu.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index b97f144aea5..ee1aa3a5a70 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -10640,7 +10640,10 @@ int x86_cpu_pending_interrupt(const CPUState *cs, int interrupt_request) > > static bool x86_cpu_has_work(CPUState *cs) > { > - return x86_cpu_pending_interrupt(cs, cs->interrupt_request) != 0; > + uint32_t pending_interrupts; > + > + pending_interrupts = qatomic_load_acquire(&cs->interrupt_request); > + return x86_cpu_pending_interrupt(cs, pending_interrupts) != 0; > } > #endif /* !CONFIG_USER_ONLY */ > For the change overall, Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Because of -ftrivial-auto-var-init=zero, I am now in general against uninitialized local variables, if at all possible, everywhere. I know you've added the local variable due to wrapping, and split the initialization for the same reason. In this case, perhaps best to just use a shorter variable name: uint32_t ir = qatomic_load_acquire(&cs->interrupt_request); return x86_cpu_pending_interrupt(cs, ir) != 0; but in others, just wrap the initialization: uint32_t pending_interrupts = qatomic_load_acquire(&cs->interrupt_request); r~
On Wed, Sep 23, 2026 at 07:11:11PM +0200, Philippe Mathieu-Daudé wrote: > Date: Wed, 23 Sep 2026 19:11:11 +0200 > From: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com> > Subject: [PATCH v3 17/21] target/i386: Use an acquire load for > interrupt_request > X-Mailer: git-send-email 2.53.0 > > x86_cpu_has_work() may run without the BQL, but currently reads > CPUState::interrupt_request directly. Other threads update this > field with qatomic_or() in cpu_set_interrupt(), so the direct > read races with an atomic access. > > Load the interrupt request with qatomic_load_acquire(), matching > cpu_test_interrupt(). This provides the acquire ordering required > before x86_cpu_pending_interrupt() inspects the CPU state. > > Suggested-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com> > --- > target/i386/cpu.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) Reviewed-by: Zhao Liu <zhao1.liu@intel.com> (and I agree with Richard's comment :-)) Regards, Zhao
diff --git a/target/i386/cpu.c b/target/i386/cpu.c index b97f144aea5..ee1aa3a5a70 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -10640,7 +10640,10 @@ int x86_cpu_pending_interrupt(const CPUState *cs, int interrupt_request) static bool x86_cpu_has_work(CPUState *cs) { - return x86_cpu_pending_interrupt(cs, cs->interrupt_request) != 0; + uint32_t pending_interrupts; + + pending_interrupts = qatomic_load_acquire(&cs->interrupt_request); + return x86_cpu_pending_interrupt(cs, pending_interrupts) != 0; } #endif /* !CONFIG_USER_ONLY */
x86_cpu_has_work() may run without the BQL, but currently reads CPUState::interrupt_request directly. Other threads update this field with qatomic_or() in cpu_set_interrupt(), so the direct read races with an atomic access. Load the interrupt request with qatomic_load_acquire(), matching cpu_test_interrupt(). This provides the acquire ordering required before x86_cpu_pending_interrupt() inspects the CPU state. Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com> --- target/i386/cpu.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)