* Re: [PATCH v1] s390x/pci: Fix interrupt forwarding disable for interpreted devices
2026-05-21 18:29 [PATCH v1] s390x/pci: Fix interrupt forwarding disable for interpreted devices Farhan Ali
@ 2026-05-22 18:47 ` Omar Elghoul
2026-05-22 18:50 ` Matthew Rosato
2026-05-29 14:02 ` Cornelia Huck
2 siblings, 0 replies; 4+ messages in thread
From: Omar Elghoul @ 2026-05-22 18:47 UTC (permalink / raw)
To: Farhan Ali, qemu-s390x, qemu-devel
Cc: mjrosato, farman, cohuck, qemu-stable, Niklas Schnelle
On 5/21/26 2:29 PM, Farhan Ali wrote:
> Remove the FH_MASK_ENABLE check when disabling interrupt forwarding
> during device reset. This check was broken for the default case in
> the switch statement above, preventing proper cleanup of interrupt
> forwarding.
>
> The pbdev->aif check in s390_pci_kvm_aif_disable() already guards
> against double-disabling of interrupt forwarding.
I tested this patch with PCI mlx, ISM, and NVMe devices in combinations of
guest kernels, including and excluding the patch that originally exposed
this issue, and have found it to be resolved. I did not find any new
regressions with kernels that don't expose this issue either.
Tested-by: Omar Elghoul <oelghoul@linux.ibm.com>
Thanks.
>
> Cc: qemu-stable@nongnu.org
> Reported-by: Niklas Schnelle <schnelle@linux.ibm.com>
> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
> ---
> hw/s390x/s390-pci-bus.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index 3665aba106..2f7b214891 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -1542,7 +1542,7 @@ static void s390_pci_device_reset(DeviceState *dev)
> break;
> }
>
> - if (pbdev->interp && (pbdev->fh & FH_MASK_ENABLE)) {
> + if (pbdev->interp) {
> /* Interpreted devices were using interrupt forwarding */
> s390_pci_kvm_aif_disable(pbdev);
> } else if (pbdev->summary_ind) {
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v1] s390x/pci: Fix interrupt forwarding disable for interpreted devices
2026-05-21 18:29 [PATCH v1] s390x/pci: Fix interrupt forwarding disable for interpreted devices Farhan Ali
2026-05-22 18:47 ` Omar Elghoul
@ 2026-05-22 18:50 ` Matthew Rosato
2026-05-29 14:02 ` Cornelia Huck
2 siblings, 0 replies; 4+ messages in thread
From: Matthew Rosato @ 2026-05-22 18:50 UTC (permalink / raw)
To: Farhan Ali, qemu-s390x, qemu-devel
Cc: farman, cohuck, qemu-stable, Niklas Schnelle
On 5/21/26 2:29 PM, Farhan Ali wrote:
> Remove the FH_MASK_ENABLE check when disabling interrupt forwarding
> during device reset. This check was broken for the default case in
> the switch statement above, preventing proper cleanup of interrupt
> forwarding.
>
> The pbdev->aif check in s390_pci_kvm_aif_disable() already guards
> against double-disabling of interrupt forwarding.
>
> Cc: qemu-stable@nongnu.org
> Reported-by: Niklas Schnelle <schnelle@linux.ibm.com>
> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
Thanks Farhan (and Omar for testing).
This issue was exposed by recent changes to the kernel but both the
initial implementation of AIF + the attempts at avoiding duplicate
registration failed to catch this case, which we can now encounter
easily with the new code in the guest kernel.
The goal of the enabled function handle check here was intended to
avoid double-deregistration, but in fact we can get here when the
function handle is already disabled but AIF was never cleaned up in
response to guest activity (e.g. guest is not required to issue the
corresponding MPCIFC to clean it up).
Prior to the introduction of pbdev->aif, this check did help
prevent instances of double-deregistration but was not sufficient
to also catch the case where the function was already disabled but
AIF was never cleaned up. So I think it's been broken from the
beginning (d0bc7091c2) but it was not safe to remove this check until
we introduced pbdev->aif (07b2c8e034), which allows us to reliably tell
whether or not aif cleanup still needs to happen during reset.
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
> ---
> hw/s390x/s390-pci-bus.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index 3665aba106..2f7b214891 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -1542,7 +1542,7 @@ static void s390_pci_device_reset(DeviceState *dev)
> break;
> }
>
> - if (pbdev->interp && (pbdev->fh & FH_MASK_ENABLE)) {
> + if (pbdev->interp) {
> /* Interpreted devices were using interrupt forwarding */
> s390_pci_kvm_aif_disable(pbdev);
> } else if (pbdev->summary_ind) {
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v1] s390x/pci: Fix interrupt forwarding disable for interpreted devices
2026-05-21 18:29 [PATCH v1] s390x/pci: Fix interrupt forwarding disable for interpreted devices Farhan Ali
2026-05-22 18:47 ` Omar Elghoul
2026-05-22 18:50 ` Matthew Rosato
@ 2026-05-29 14:02 ` Cornelia Huck
2 siblings, 0 replies; 4+ messages in thread
From: Cornelia Huck @ 2026-05-29 14:02 UTC (permalink / raw)
To: Farhan Ali, qemu-s390x, qemu-devel
Cc: mjrosato, farman, alifm, qemu-stable, Niklas Schnelle
On Thu, May 21 2026, Farhan Ali <alifm@linux.ibm.com> wrote:
> Remove the FH_MASK_ENABLE check when disabling interrupt forwarding
> during device reset. This check was broken for the default case in
> the switch statement above, preventing proper cleanup of interrupt
> forwarding.
>
> The pbdev->aif check in s390_pci_kvm_aif_disable() already guards
> against double-disabling of interrupt forwarding.
>
> Cc: qemu-stable@nongnu.org
> Reported-by: Niklas Schnelle <schnelle@linux.ibm.com>
> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
> ---
> hw/s390x/s390-pci-bus.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index 3665aba106..2f7b214891 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -1542,7 +1542,7 @@ static void s390_pci_device_reset(DeviceState *dev)
> break;
> }
>
> - if (pbdev->interp && (pbdev->fh & FH_MASK_ENABLE)) {
> + if (pbdev->interp) {
> /* Interpreted devices were using interrupt forwarding */
> s390_pci_kvm_aif_disable(pbdev);
> } else if (pbdev->summary_ind) {
Thanks, queued to s390-next.
^ permalink raw reply [flat|nested] 4+ messages in thread