Re: [Qemu-devel] [PATCH for-2.8] pci-assign: sync MSI/MSI-X cap and table with PCIDevice

[Peter Xu]

CC Limin.

Limin, could you please help test this patch as well to see whether it
can solve your issue? This one is slightly cleaner IMHO.

Thanks,

On Thu, Nov 24, 2016 at 06:25:13PM +0800, Peter Xu wrote:
> Since commit e1d4fb2d ("kvm-irqchip: x86: add msi route notify fn"),
> kvm_irqchip_add_msi_route() starts to use pci_get_msi_message() to fetch
> MSI info. This requires that we setup MSI related fields in PCIDevice.
> For most devices, that won't be a problem, as long as we are using
> general interfaces like msi_init()/msix_init().
> 
> However, for pci-assign devices, MSI/MSI-X is treated differently - PCI
> assign devices are maintaining its own MSI table and cap information in
> AssignedDevice struct. however that's not synced up with PCIDevice's
> fields. That will leads to pci_get_msi_message() failed to find correct
> MSI capability, even with an NULL msix_table.
> 
> A quick fix is to sync up the two places: both the capability bits and
> table address for MSI/MSI-X.
> 
> Reported-by: Changlimin <address@hidden>
> Cc: address@hidden
> Fixes: e1d4fb2d ("kvm-irqchip: x86: add msi route notify fn")
> Signed-off-by: Peter Xu <address@hidden>
> 
> ---
> Do we still support pci-assign?
> ---
>  hw/i386/kvm/pci-assign.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
> index 8238fbc..df9748d 100644
> --- a/hw/i386/kvm/pci-assign.c
> +++ b/hw/i386/kvm/pci-assign.c
> @@ -1251,6 +1251,7 @@ static int assigned_device_pci_cap_init(PCIDevice 
> *pci_dev, Error **errp)
>              error_propagate(errp, local_err);
>              return -ENOTSUP;
>          }
> +        dev->dev.cap_present |= QEMU_PCI_CAP_MSI;
>          dev->cap.available |= ASSIGNED_DEVICE_CAP_MSI;
>          /* Only 32-bit/no-mask currently supported */
>          ret = pci_add_capability2(pci_dev, PCI_CAP_ID_MSI, pos, 10,
> @@ -1285,6 +1286,7 @@ static int assigned_device_pci_cap_init(PCIDevice 
> *pci_dev, Error **errp)
>              error_propagate(errp, local_err);
>              return -ENOTSUP;
>          }
> +        dev->dev.cap_present |= QEMU_PCI_CAP_MSIX;
>          dev->cap.available |= ASSIGNED_DEVICE_CAP_MSIX;
>          ret = pci_add_capability2(pci_dev, PCI_CAP_ID_MSIX, pos, 12,
>                                    &local_err);
> @@ -1648,6 +1650,7 @@ static void 
> assigned_dev_register_msix_mmio(AssignedDevice *dev, Error **errp)
>          dev->msix_table = NULL;
>          return;
>      }
> +    dev->dev.msix_table = dev->msix_table;
>  
>      assigned_dev_msix_reset(dev);
>  
> @@ -1665,6 +1668,7 @@ static void 
> assigned_dev_unregister_msix_mmio(AssignedDevice *dev)
>          error_report("error unmapping msix_table! %s", strerror(errno));
>      }
>      dev->msix_table = NULL;
> +    dev->dev.msix_table = NULL;
>  }
>  
>  static const VMStateDescription vmstate_assigned_device = {
> -- 
> 2.7.4
> 

-- peterx