| Message ID | alpine.DEB.2.02.1602151009090.27008@kaball.uk.xensource.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Feb 15, 2016 at 10:15:12AM +0000, Stefano Stabellini wrote: > On Sat, 13 Feb 2016, Michael S. Tsirkin wrote: > > commit 428c3ece97179557f2753071fb0ca97a03437267 ("fix MSI injection on Xen") > > inadvertently enabled the xen-specific logic unconditionally. > > Limit it to only when xen is enabled. > > Additionally, msix data should be read with pci_get_log > > since the format is pci little-endian. > > > > Reported-by: "Daniel P. Berrange" <berrange@redhat.com> > > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > Thanks Daniel for finding the issue and thanks Michael for fixing my > bug, sorry about that. > > > > hw/pci/msix.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/hw/pci/msix.c b/hw/pci/msix.c > > index eb4ef11..537fdba 100644 > > --- a/hw/pci/msix.c > > +++ b/hw/pci/msix.c > > @@ -80,10 +80,10 @@ static void msix_clr_pending(PCIDevice *dev, int vector) > > static bool msix_vector_masked(PCIDevice *dev, unsigned int vector, bool fmask) > > { > > unsigned offset = vector * PCI_MSIX_ENTRY_SIZE; > > - uint32_t *data = (uint32_t *)&dev->msix_table[offset + PCI_MSIX_ENTRY_DATA]; > > + uint8_t *data = &dev->msix_table[offset + PCI_MSIX_ENTRY_DATA]; > > /* MSIs on Xen can be remapped into pirqs. In those cases, masking > > * and unmasking go through the PV evtchn path. */ > > - if (xen_is_pirq_msi(*data)) { > > + if (xen_enabled() && xen_is_pirq_msi(pci_get_long(data))) { > > return false; > > } > > return fmask || dev->msix_table[offset + PCI_MSIX_ENTRY_VECTOR_CTRL] & > > I think this is good, but moving the xen_enabled() check inside > xen_is_pirq_msi is even be better, so that we cover all call sites at > once. Feel free to post a patch on top. > > diff --git a/xen-hvm.c b/xen-hvm.c > index 039680a..991f6b7 100644 > --- a/xen-hvm.c > +++ b/xen-hvm.c > @@ -151,7 +151,8 @@ int xen_is_pirq_msi(uint32_t msi_data) > /* If vector is 0, the msi is remapped into a pirq, passed as > * dest_id. > */ > - return ((msi_data & MSI_DATA_VECTOR_MASK) >> MSI_DATA_VECTOR_SHIFT) == 0; > + return xen_enabled() && > + ((msi_data & MSI_DATA_VECTOR_MASK) >> MSI_DATA_VECTOR_SHIFT) == 0; > } > > void xen_hvm_inject_msi(uint64_t addr, uint32_t data)
diff --git a/xen-hvm.c b/xen-hvm.c index 039680a..991f6b7 100644 --- a/xen-hvm.c +++ b/xen-hvm.c @@ -151,7 +151,8 @@ int xen_is_pirq_msi(uint32_t msi_data) /* If vector is 0, the msi is remapped into a pirq, passed as * dest_id. */ - return ((msi_data & MSI_DATA_VECTOR_MASK) >> MSI_DATA_VECTOR_SHIFT) == 0; + return xen_enabled() && + ((msi_data & MSI_DATA_VECTOR_MASK) >> MSI_DATA_VECTOR_SHIFT) == 0; } void xen_hvm_inject_msi(uint64_t addr, uint32_t data)