MSI/MSIX issues and bugs

["Ayaz Abdulla" <AAbdulla@nvidia.com>]

I am working on MSI/MSIX support described in PCI 3.0 specification and
found some bugs in the linux kernel code.

1) If a device does not expose PCI Express capability, the MSI code
should still disable INTx for the device since you are switching to a
different interrupt model - in functions enable_msi_mode() and
disable_msi_mode().

2) If a device supports MSI mask mode (PCI_MSI_FLAGS_MASKBIT), the
expression to set the flag is wrong:

    ie. entry->msi_attrib.maskbit = is_mask_bit_support(control);

    This will return 0x100 but maskbit field is only 1 bit. It should be
the following:

    entry->msi_attrib.maskbit = (is_mask_bit_support(control) ? 1 : 0);

3) Same bug when using is_64bit_address() macro followed by
msi_mask_bits_reg() macro:

    #define msi_mask_bits_reg(base, is64bit) \
     ( (is64bit == 1) ? base+PCI_MSI_MASK_BIT : base+PCI_MSI_MASK_BIT-4)

    is_64bit_address() will return 0x80, which is passed as is64bit
parameter. It should be the following:

    #define msi_mask_bits_reg(base, is64bit) \
     ( (is64bit != 0) ? base+PCI_MSI_MASK_BIT : base+PCI_MSI_MASK_BIT-4)

4) If a device supports both MSI and MSIX, you can not switch between
them on subsequent driver loads. In either case, when the
pci_disable_msi(x) functions are called, the vector is not free'd. If
you reload the driver in the other mode it will complain that you are
already in the previous mode. This might have been intended design not
to free the vector or msi_desc entry, but it should be allowed to switch
between the different modes without a reboot.

5) The PCI_MSIX_XXX defines in msi.h should be within
include/linux/pci.h alongside the other PCI config space defines.


Regards,
Ayaz
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/