On Fri, 9 Mar 2018 12:17:36 +1100
Alexey Kardashevskiy <aik@xxxxxxxxx> wrote:
> Commit 2170dd04316e ("vfio-pci: Mask INTx if a device is not capabable
> of enabling it") is causing 'Masking broken INTx support' messages
> every time when a PCI without INTx support is enabled. This message is
> intended to appear when a device known for broken INTx is being enabled.
> However the message also appears on IOV virtual functions where INTx
> cannot possibly be enabled so saying that the "broken" support is masked
> is not correct.
>
> This changes the message to only appear when the device advertises INTx
> (via PCI_INTERRUPT_PIN != 0) but does not implement PCI 2.3 INTx masking.
>
> Fixes: 2170dd04316e ("vfio-pci: Mask INTx if a device is not capabable of enabling it")
> Signed-off-by: Alexey Kardashevskiy <aik@xxxxxxxxx>
> ---
> drivers/vfio/pci/vfio_pci.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index b0f7594..7f2ec47 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -245,7 +245,13 @@ static int vfio_pci_enable(struct vfio_pci_device *vdev)
>
> if (likely(!nointxmask)) {
> if (vfio_pci_nointx(pdev)) {
> - dev_info(&pdev->dev, "Masking broken INTx support\n");
> + u8 pin = 0;
> +
> + pci_read_config_byte(vdev->pdev, PCI_INTERRUPT_PIN,
> + &pin);
> + if (pin)
> + dev_info(&pdev->dev,
> + "Masking broken INTx support\n");
> vdev->nointx = true;
> pci_intx(pdev, 0);
> } else
Why don't we fix it at the location of the bug rather than after the
fact? I don't see any reason to invoke the nointx handling for devices
that don't actually support INTx. Thanks,
Alex
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index ad18ed266dc0..34d463a0b4a5 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -192,6 +192,8 @@ static void vfio_pci_disable(struct vfio_pci_device *vdev);
*/
static bool vfio_pci_nointx(struct pci_dev *pdev)
{
+ u8 pin;
+
switch (pdev->vendor) {
case PCI_VENDOR_ID_INTEL:
switch (pdev->device) {
@@ -207,7 +209,9 @@ static bool vfio_pci_nointx(struct pci_dev *pdev)
}
}
- if (!pdev->irq)
+ pci_read_config_byte(pdev, PCI_INTERRUPT_PIN, &pin);
+
+ if (pin && !pdev->irq)
return true;
return false;
