From: Alex Williamson <alex@shazbot.org>
To: Christos Longros <chris.longros@gmail.com>
Cc: "Cédric Le Goater" <clg@redhat.com>,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
alex@shazbot.org
Subject: Re: [PATCH v2] vfio/pci: sanitize bogus INTx interrupt pin values
Date: Wed, 1 Apr 2026 16:59:20 -0600 [thread overview]
Message-ID: <20260401165920.373e7584@shazbot.org> (raw)
In-Reply-To: <20260328230126.73230-1-chris.longros@gmail.com>
On Sun, 29 Mar 2026 00:01:26 +0100
Christos Longros <chris.longros@gmail.com> wrote:
> Some PCI devices may report out-of-range interrupt pin values in
> config space (e.g., 0xFF when the device is in an error state).
> The VFIO PCI config virtualization layer passes these values through
> to userspace, causing QEMU to crash with an assertion failure in
> pci_irq_handler() when it computes irq_num = pin - 1, which exceeds
> PCI_NUM_PINS (4).
>
> The existing code already handles bogus VF interrupt pins (set to 0
> per SR-IOV spec 3.4.1.18), but physical functions with out-of-range
> pin values are not caught. Extend the condition that clears the
> virtualized interrupt pin to also cover values outside 1-4.
The description has taken quite a change in direction between v1 and
v2, which makes me wonder what's actually happening here. v1 suggested
this was an issue with a specific RTL NIC where the pin register reads
as 0xFF, but proposes a generic solution. v2 proposes the same
solution, but drops the reference to the RTL NIC, instead suggesting
that a device "in an error state" might have this trait.
If the device is in error state and the PIN register reads as 0xFF,
does all of config space read as 0xFF? Was the PIN register valid
before? How did it get into an "error state".
If this has only actually been observed with this RTL NIC, is it
really an error state or is the hardware non-spec compliant, trying to
indicate lack of INTx support with 0xFF rather than 0x0.
It would be surprising if a device in an error state was only broken
relative to the INTx PIN register, and exposing a broken device only
masking the INTx support seems incomplete. If we're dealing with a
specific broken device, maybe the right answer is a quirk to set
pdev->irq to zero.
Please elaborate on what's actually happening here. Thanks,
Alex
> Signed-off-by: Christos Longros <chris.longros@gmail.com>
> ---
> drivers/vfio/pci/vfio_pci_config.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
> index b4e39253f..ed75c1cc3 100644
> --- a/drivers/vfio/pci/vfio_pci_config.c
> +++ b/drivers/vfio/pci/vfio_pci_config.c
> @@ -1829,8 +1829,17 @@ int vfio_config_init(struct vfio_pci_core_device *vdev)
> cpu_to_le16(PCI_COMMAND_MEMORY);
> }
>
> + /*
> + * Sanitize bogus interrupt pin values. Valid pins are 1 (INTA)
> + * through 4 (INTD); anything else disables legacy interrupts.
> + */
> + if (vconfig[PCI_INTERRUPT_PIN] > 4)
> + pci_info(pdev, "Bogus INTx pin %d, disabling INTx virtualization\n",
> + vconfig[PCI_INTERRUPT_PIN]);
> +
> if (!IS_ENABLED(CONFIG_VFIO_PCI_INTX) || vdev->nointx ||
> - !vdev->pdev->irq || vdev->pdev->irq == IRQ_NOTCONNECTED)
> + !vdev->pdev->irq || vdev->pdev->irq == IRQ_NOTCONNECTED ||
> + vconfig[PCI_INTERRUPT_PIN] > 4)
> vconfig[PCI_INTERRUPT_PIN] = 0;
>
> ret = vfio_cap_init(vdev);
next prev parent reply other threads:[~2026-04-01 22:59 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-28 21:58 [PATCH] vfio/pci: sanitize bogus INTx interrupt pin values Christos Longros
2026-03-28 23:01 ` [PATCH v2] " Christos Longros
2026-04-01 22:59 ` Alex Williamson [this message]
2026-04-04 18:14 ` Christos Longros
2026-04-10 16:53 ` Alex Williamson
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=20260401165920.373e7584@shazbot.org \
--to=alex@shazbot.org \
--cc=chris.longros@gmail.com \
--cc=clg@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/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