Re: [GIT PULL] virtio: fixes, features
From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2022-10-12 13:28:46
Also in:
kvm, linux-pci, lkml, netdev, virtualization
Subsystem:
pci subsystem, the rest · Maintainers:
Bjorn Helgaas, Linus Torvalds
[ Cc += Bjorn & linux-pci ] "Michael S. Tsirkin" [off-list ref] writes:
On Wed, Oct 12, 2022 at 05:21:24PM +1100, Michael Ellerman wrote:quoted
"Michael S. Tsirkin" [off-list ref] writes:
...
quoted hunk ↗ jump to hunk
quoted
quoted
---------------------------------------------------------------- virtio: fixes, features 9k mtu perf improvements vdpa feature provisioning virtio blk SECURE ERASE support Fixes, cleanups all over the place. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> ---------------------------------------------------------------- Alvaro Karsz (1): virtio_blk: add SECURE ERASE command support Angus Chen (1): virtio_pci: don't try to use intxif pin is zeroThis commit breaks virtio_pci for me on powerpc, when running as a qemu guest. vp_find_vqs() bails out because pci_dev->pin == 0. But pci_dev->irq is populated correctly, so vp_find_vqs_intx() would succeed if we called it - which is what the code used to do. I think this happens because pci_dev->pin is not populated in pci_assign_irq(). I would absolutely believe this is bug in our PCI code, but I think it may also affect other platforms that use of_irq_parse_and_map_pci().How about fixing this in of_irq_parse_and_map_pci then? Something like the below maybe?diff --git a/drivers/pci/of.c b/drivers/pci/of.c index 196834ed44fe..504c4d75c83f 100644 --- a/drivers/pci/of.c +++ b/drivers/pci/of.c@@ -446,6 +446,8 @@ static int of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args * if (pin == 0) return -ENODEV; + pdev->pin = pin; + /* Local interrupt-map in the device node? Use it! */ if (of_get_property(dn, "interrupt-map", NULL)) { pin = pci_swizzle_interrupt_pin(pdev, pin);
That doesn't fix it in all cases, because there's an early return if there's a struct device_node associated with the pci_dev, before we even read the pin. Also the pci_dev is const, and removing the const would propagate to a few other places. The other obvious place to fix it would be in pci_assign_irq(), as below. That fixes this bug for me, but is otherwise very lightly tested. cheers
diff --git a/drivers/pci/setup-irq.c b/drivers/pci/setup-irq.c
index cc7d26b015f3..0135413b33af 100644
--- a/drivers/pci/setup-irq.c
+++ b/drivers/pci/setup-irq.c@@ -22,6 +22,15 @@ void pci_assign_irq(struct pci_dev *dev) int irq = 0; struct pci_host_bridge *hbrg = pci_find_host_bridge(dev->bus); + /* Make sure dev->pin is populated */ + pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin); + + /* Cope with illegal. */ + if (pin > 4) + pin = 1; + + dev->pin = pin; + if (!(hbrg->map_irq)) { pci_dbg(dev, "runtime IRQ mapping not provided by arch\n"); return;
@@ -34,11 +43,6 @@ void pci_assign_irq(struct pci_dev *dev) * time the interrupt line passes through a PCI-PCI bridge we must * apply the swizzle function. */ - pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin); - /* Cope with illegal. */ - if (pin > 4) - pin = 1; - if (pin) { /* Follow the chain of bridges, swizzling as we go. */ if (hbrg->swizzle_irq)