Re: [PATCH v5 6/8] add cpu_test_interrupt()/cpu_set_interrupt() helpers and use them tree wide

[Igor Mammedov <imammedo@redhat.com>]

On Mon, 25 Aug 2025 23:28:22 +0800
Zhao Liu <zhao1.liu@intel.com> 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
>