qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Halil Pasic <pasic@linux.ibm.com>
To: Jaehoon Kim <jhkim@linux.ibm.com>
Cc: qemu-devel@nongnu.org, qemu-s390x@nongnu.org,
	mjrosato@linux.ibm.com, farman@linux.ibm.com,
	borntraeger@linux.ibm.com, thuth@redhat.com,
	richard.henderson@linaro.org, david@redhat.com,
	iii@linux.ibm.com, Halil Pasic <pasic@linux.ibm.com>
Subject: Re: [PATCH v1] s390x/pci: fix interrupt blocking by returning only the device's summary bit
Date: Wed, 8 Oct 2025 12:43:00 +0200	[thread overview]
Message-ID: <20251008124300.4b391ad9.pasic@linux.ibm.com> (raw)
In-Reply-To: <20251001154004.71917-1-jhkim@linux.ibm.com>

On Wed,  1 Oct 2025 10:39:38 -0500
Jaehoon Kim <jhkim@linux.ibm.com> wrote:

> Previously, set_ind_atomic() returned the entire byte containing
> multiple summary bits. This meant that if any other summary bit in the
> byte was set, interrupt injection could be incorrectly blocked, even
> when the current device's summary bit was not set. As a result, the
> guest could remain blocked after I/O completion during FIO tests.
> 
> This patch replaces set_ind_atomic() with set_ind_bit_atomic(), which
> returns true if the bit was set by this function, and false if it was
> already set or mapping failed. Interrupts are now blocked only when
> the device's own summary bit was not previously set, avoiding
> unintended blocking when multiple PCI summary bits exist within the
> same byte.
> 
> Signed-off-by: Jaehoon Kim <jhkim@linux.ibm.com>
> ---
>  hw/s390x/s390-pci-bus.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index f87d2748b6..e8e41c8a9a 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -652,7 +652,16 @@ static const PCIIOMMUOps s390_iommu_ops = {
>      .get_address_space = s390_pci_dma_iommu,
>  };
>  
> -static uint8_t set_ind_atomic(uint64_t ind_loc, uint8_t to_be_set)
> +/**
> + * set_ind_bit_atomic - Atomically set a bit in an indicator
> + *
> + * @ind_loc:   Address of the indicator
> + * @to_be_set: Bit to set
> + *
> + * Returns true if the bit was set by this function, false if it was
> + * already set or mapping failed.
> + */
> +static bool set_ind_bit_atomic(uint64_t ind_loc, uint8_t to_be_set)

Unlike for example set_bit() in the Linux kernel which works with
a bit number, this one works with basically a mask in which only that
single bit (or possibly multiple bits) is set which needs to be set in
the target byte as well. I'm not sure if bit nr based would lead
to simplification or not, just wanted to point this out.

Reviewed-by: Halil Pasic <pasic@linux.ibm.com>


      parent reply	other threads:[~2025-10-08 10:43 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-01 15:39 [PATCH v1] s390x/pci: fix interrupt blocking by returning only the device's summary bit Jaehoon Kim
2025-10-01 20:00 ` Matthew Rosato
2025-10-02  7:54 ` Christian Borntraeger
2025-10-02 13:58 ` Eric Farman
2025-10-08 10:43 ` Halil Pasic [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251008124300.4b391ad9.pasic@linux.ibm.com \
    --to=pasic@linux.ibm.com \
    --cc=borntraeger@linux.ibm.com \
    --cc=david@redhat.com \
    --cc=farman@linux.ibm.com \
    --cc=iii@linux.ibm.com \
    --cc=jhkim@linux.ibm.com \
    --cc=mjrosato@linux.ibm.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-s390x@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).