Jean-Philippe Brucker <jean-philippe.brucker@xxxxxxx> writes: > On 22/06/17 18:05, Jean-Philippe Brucker wrote: >> + switch (cap.type) { >> + case PCI_CAP_ID_MSIX: >> + ret = vfio_pci_add_cap(vdev, &cap, info->offset, pos); >> + if (ret) { >> + dev_warn(vdev, "failed to read capability structure %x", >> + cap.type); >> + return ret; >> + } >> + >> + pdev->msi.pos = pos; >> + pdev->irq_type = VFIO_PCI_IRQ_MSIX; > > A few more issues noticed when testing this code with an IGB endpoint on > AMD Seattle: > > * Selecting only one irq_type seems like the wrong thing to do. If the > device has an MSI-X capability, then kvmtool will only initialize it and > ignore INTx altogether. But if the virtual irqchip doesn't support MSIs, > then the guest will fall back to INTx and fail to receive any interrupt. Ah right! I'd missed the fact that we were choosing one interrupt type support for the device. That explains why I couldn't get legacy interrupts to work with passthrough for a device advertising MSI/-X. > > * If we force selection of INTx over MSI-X, then kvmtool will not treat > the MSI-X BAR as special, attempt to map it, and get rejected by VFIO. > > * The next patch relies on the order of capabilities in the config space > to decide whether to use MSI-X or MSIs, which isn't nice. Same as before, > the discarded capability will be advertised to the guest but won't work. > > So I think irq_type should instead be a bitfield, and we should emulate > all available INTx/MSI/MSI-X capabilities, then let the guest choose what > to use. Makes sense. I'll look out for the next update. Thanks for looking into this. Punit > > Thanks, > Jean