On 13/3/18 9:44 am, Alex Williamson wrote:
> 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,
We can do that too, this just means that we will keep doing v2.3 masking
tests and possibly other things for SRIOV VFs when we do not need to as
dev-irq==0 anyway. Not a big deal though (was not a problem before), are
you going to post this as a patch or you want me to do this? 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;
>
--
Alexey
