Re: [PATCH v3 06/10] introduce cpu_test_interrupt() that will replace open coded checks

[Peter Xu <peterx@redhat.com>]

On Fri, Aug 08, 2025 at 02:01:33PM +0200, Igor Mammedov wrote:
> the helper forms load-acquire/store-release pair with
> tcg_handle_interrupt/generic_handle_interrupt and can be used
> for checking interrupts outside of BQL
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  include/hw/core/cpu.h     | 12 ++++++++++++
>  accel/tcg/tcg-accel-ops.c |  3 ++-
>  system/cpus.c             |  3 ++-
>  3 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 5eaf41a566..d0460c01cf 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -942,6 +942,18 @@ 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_pc:
>   * @cpu: The CPU to set the program counter for.
> diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
> index 3b0d7d298e..02c7600bb7 100644
> --- a/accel/tcg/tcg-accel-ops.c
> +++ b/accel/tcg/tcg-accel-ops.c
> @@ -97,7 +97,8 @@ static void tcg_cpu_reset_hold(CPUState *cpu)
>  /* mask must never be zero, except for A20 change call */
>  void tcg_handle_interrupt(CPUState *cpu, int mask)
>  {
> -    cpu->interrupt_request |= mask;
> +    qatomic_store_release(&cpu->interrupt_request,
> +        cpu->interrupt_request | mask);
>  
>      /*
>       * If called from iothread context, wake the target cpu in
> diff --git a/system/cpus.c b/system/cpus.c
> index 256723558d..8e543488c3 100644
> --- a/system/cpus.c
> +++ b/system/cpus.c
> @@ -256,7 +256,8 @@ int64_t cpus_get_elapsed_ticks(void)
>  
>  void generic_handle_interrupt(CPUState *cpu, int mask)
>  {
> -    cpu->interrupt_request |= mask;
> +    qatomic_store_release(&cpu->interrupt_request,
> +        cpu->interrupt_request | mask);
>  
>      if (!qemu_cpu_is_self(cpu)) {
>          qemu_cpu_kick(cpu);
> -- 
> 2.47.1
> 

Besides the two writters mentioned above, I still see some more:

*** accel/tcg/user-exec.c:
cpu_interrupt[52]              cpu->interrupt_request |= mask;
*** hw/intc/s390_flic.c:
qemu_s390_flic_notify[193]     cs->interrupt_request |= CPU_INTERRUPT_HARD;
*** hw/openrisc/cputimer.c:
openrisc_timer_cb[108]         cs->interrupt_request |= CPU_INTERRUPT_TIMER;
*** target/arm/helper.c:
arm_cpu_do_interrupt[9150]     cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
*** target/i386/tcg/system/svm_helper.c:
helper_vmrun[406]              cs->interrupt_request |= CPU_INTERRUPT_VIRQ;

Do they better as well be converted to use store_release too?

The other nitpick is this patch introduces the reader helper but without
using it.

It can be squashed into the other patch where the reader helper will be
applied tree-wide.  IMHO that would be better explaining the helper on its
own.

Thanks,

-- 
Peter Xu