On Mon, 25 Aug 2025 23:28:22 +0800
Zhao Liu <[email protected]> wrote:
> Hi Igor,
>
> > diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> > index 5eaf41a566..1dee9d4c76 100644
> > --- a/include/hw/core/cpu.h
> > +++ b/include/hw/core/cpu.h
> > @@ -942,6 +942,31 @@ CPUState *cpu_by_arch_id(int64_t id);
> >
> > void cpu_interrupt(CPUState *cpu, int mask);
> >
> > +/**
> > + * cpu_test_interrupt:
> > + * @cpu: The CPU to check interrupt(s) on.
> > + * @mask: The interrupts to check.
> > + *
> > + * Checks if any of interrupts in @mask are pending on @cpu.
> > + */
> > +static inline bool cpu_test_interrupt(CPUState *cpu, int mask)
> > +{
> > + return qatomic_load_acquire(&cpu->interrupt_request) & mask;
> > +}
> > +
> > +/**
> > + * cpu_set_interrupt:
> > + * @cpu: The CPU to set pending interrupt(s) on.
> > + * @mask: The interrupts to set.
> > + *
> > + * Sets interrupts in @mask as pending on @cpu.
> > + */
> > +static inline void cpu_set_interrupt(CPUState *cpu, int mask)
> > +{
> > + qatomic_store_release(&cpu->interrupt_request,
> > + cpu->interrupt_request | mask);
>
> It seems the read access of cpu->interrupt_request is not atomic, should
> we also protect it by qatomic_read(cpu->interrupt_request)? like
>
> qatomic_store_release(&cpu->interrupt_request,
> qatomic_read(cpu->interrupt_request) | mask)
it's not necessary according to doc:
- ``qatomic_store_release()``, which guarantees the STORE to appear to
happen, ...,
after all the LOAD or STORE operations specified before.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
that includes 'cpu->interrupt_request | mask' part
>
> or futher,
>
> qatomic_fetch_or(&cpu->interrupt_request, mask)
that would work as well but it also could be more expensive than
qatomic_store_release()
>
> > +}
> > +
>
> Thanks,
> Zhao
>
