| Message ID | 20200424134024.12543-1-lorenzo.pieralisi@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kvmtool] vfio: fix multi-MSI vector handling | expand |
Hi Lorenzo, On 2020-04-24 14:40, Lorenzo Pieralisi wrote: > A PCI device with a MSI capability enabling Multiple MSI messages > (through the Multiple Message Enable field in the Message Control > register[6:4]) is expected to drive the Message Data lower bits (number > determined by the number of selected vectors) to generate the > corresponding MSI messages writes on the PCI bus. > > Therefore, KVM expects the MSI data lower bits (a number of > bits that depend on bits [6:4] of the Message Control > register - which in turn control the number of vectors > allocated) to be set-up by kvmtool while programming the > MSI IRQ routing entries to make sure the MSI entries can > actually be demultiplexed by KVM and IRQ routes set-up > accordingly so that when an actual HW fires KVM can > route it to the correct entry in the interrupt controller > (and set-up a correct passthrough route for directly > injected interrupt). > > Current kvmtool code does not set-up Message data entries > correctly for multi-MSI vectors - the data field is left > as programmed in the MSI capability by the guest for all > vector entries, triggering IRQs misrouting. > > Fix it. > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > Cc: Will Deacon <will@kernel.org> > Cc: Julien Thierry <julien.thierry.kdev@gmail.com> > --- > vfio/pci.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/vfio/pci.c b/vfio/pci.c > index 76e24c1..b43e522 100644 > --- a/vfio/pci.c > +++ b/vfio/pci.c > @@ -434,6 +434,12 @@ static void vfio_pci_msi_cap_write(struct kvm > *kvm, struct vfio_device *vdev, > > for (i = 0; i < nr_vectors; i++) { > entry = &pdev->msi.entries[i]; > + > + if (nr_vectors > 1) { > + msg.data &= ~(nr_vectors - 1); > + msg.data |= i; > + } > + This matches my own understanding of how MultiMSI works. Small nit: you don't need to check the condition for the number of vectors, as this expression is valid for any nr_vectors that is a power of 2 (as required by the spec). > entry->config.msg = msg; > vfio_pci_update_msi_entry(kvm, vdev, entry); > } FWIW: Acked-by: Marc Zyngier <maz@kernel.org> M.
On Fri, Apr 24, 2020 at 02:54:37PM +0100, Marc Zyngier wrote: > On 2020-04-24 14:40, Lorenzo Pieralisi wrote: > > diff --git a/vfio/pci.c b/vfio/pci.c > > index 76e24c1..b43e522 100644 > > --- a/vfio/pci.c > > +++ b/vfio/pci.c > > @@ -434,6 +434,12 @@ static void vfio_pci_msi_cap_write(struct kvm > > *kvm, struct vfio_device *vdev, > > > > for (i = 0; i < nr_vectors; i++) { > > entry = &pdev->msi.entries[i]; > > + > > + if (nr_vectors > 1) { > > + msg.data &= ~(nr_vectors - 1); > > + msg.data |= i; > > + } > > + > > This matches my own understanding of how MultiMSI works. > Small nit: you don't need to check the condition for the number > of vectors, as this expression is valid for any nr_vectors > that is a power of 2 (as required by the spec). > > > entry->config.msg = msg; > > vfio_pci_update_msi_entry(kvm, vdev, entry); > > } > > FWIW: > > Acked-by: Marc Zyngier <maz@kernel.org> Cheers guys. Lorenzo -- if you send a new version, I can pick it up straight away. Will
diff --git a/vfio/pci.c b/vfio/pci.c index 76e24c1..b43e522 100644 --- a/vfio/pci.c +++ b/vfio/pci.c @@ -434,6 +434,12 @@ static void vfio_pci_msi_cap_write(struct kvm *kvm, struct vfio_device *vdev, for (i = 0; i < nr_vectors; i++) { entry = &pdev->msi.entries[i]; + + if (nr_vectors > 1) { + msg.data &= ~(nr_vectors - 1); + msg.data |= i; + } + entry->config.msg = msg; vfio_pci_update_msi_entry(kvm, vdev, entry); }
A PCI device with a MSI capability enabling Multiple MSI messages (through the Multiple Message Enable field in the Message Control register[6:4]) is expected to drive the Message Data lower bits (number determined by the number of selected vectors) to generate the corresponding MSI messages writes on the PCI bus. Therefore, KVM expects the MSI data lower bits (a number of bits that depend on bits [6:4] of the Message Control register - which in turn control the number of vectors allocated) to be set-up by kvmtool while programming the MSI IRQ routing entries to make sure the MSI entries can actually be demultiplexed by KVM and IRQ routes set-up accordingly so that when an actual HW fires KVM can route it to the correct entry in the interrupt controller (and set-up a correct passthrough route for directly injected interrupt). Current kvmtool code does not set-up Message data entries correctly for multi-MSI vectors - the data field is left as programmed in the MSI capability by the guest for all vector entries, triggering IRQs misrouting. Fix it. Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Cc: Will Deacon <will@kernel.org> Cc: Julien Thierry <julien.thierry.kdev@gmail.com> --- vfio/pci.c | 6 ++++++ 1 file changed, 6 insertions(+)