fix MSI injection on Xen

On Xen MSIs can be remapped into pirqs, which are a type of event
channels. It's mostly for the benefit of PCI passthrough devices, to
avoid the overhead of interacting with the emulated lapic.

However remapping interrupts and MSIs is also supported for emulated
devices, such as the e1000 and virtio-net.

When an interrupt or an MSI is remapped into a pirq, masking and
unmasking is done by masking and unmasking the event channel. The
masking bit on the PCI config space or MSI-X table should be ignored,
but it isn't at the moment.

As a consequence emulated devices which use MSI or MSI-X, such as
virtio-net, don't work properly (the guest doesn't receive any
notifications). The mechanism was working properly when xen_apic was
introduced, but I haven't narrowed down which commit in particular is
causing the regression.

Fix the issue by ignoring the masking bit for MSI and MSI-X which have
been remapped into pirqs.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

diff --git a/hw/pci/msi.c b/hw/pci/msi.c
index 1d25b629ea875185582bb08738d72cad35aa0378..b7b61b397b726874e8fbfa9d22e2a71e95728b7d 100644 (file)
--- a/hw/pci/msi.c
+++ b/hw/pci/msi.c
@@ -19,6 +19,7 @@
  */
 
 #include "hw/pci/msi.h"
+#include "hw/xen/xen.h"
 #include "qemu/range.h"
 
 /* PCI_MSI_ADDRESS_LO */
@@ -253,13 +254,19 @@ void msi_reset(PCIDevice *dev)
 static bool msi_is_masked(const PCIDevice *dev, unsigned int vector)
 {
     uint16_t flags = pci_get_word(dev->config + msi_flags_off(dev));
-    uint32_t mask;
+    uint32_t mask, data;
+    bool msi64bit = flags & PCI_MSI_FLAGS_64BIT;
     assert(vector < PCI_MSI_VECTORS_MAX);
 
     if (!(flags & PCI_MSI_FLAGS_MASKBIT)) {
         return false;
     }
 
+    data = pci_get_word(dev->config + msi_data_off(dev, msi64bit));
+    if (xen_is_pirq_msi(data)) {
+        return false;
+    }
+
     mask = pci_get_long(dev->config +
                         msi_mask_off(dev, flags & PCI_MSI_FLAGS_64BIT));
     return mask & (1U << vector);

diff --git a/hw/pci/msix.c b/hw/pci/msix.c
index 24de2605fbac4a2dae61e1ada7c531ff677eeb88..3c605130e737e09b13f1b7d051035d68ac521a5f 100644 (file)
--- a/hw/pci/msix.c
+++ b/hw/pci/msix.c
@@ -18,6 +18,7 @@
 #include "hw/pci/msi.h"
 #include "hw/pci/msix.h"
 #include "hw/pci/pci.h"
+#include "hw/xen/xen.h"
 #include "qemu/range.h"
 
 #define MSIX_CAP_LENGTH 12
@@ -77,8 +78,15 @@ 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 + PCI_MSIX_ENTRY_VECTOR_CTRL;
-    return fmask || dev->msix_table[offset] & PCI_MSIX_ENTRY_CTRL_MASKBIT;
+    unsigned offset = vector * PCI_MSIX_ENTRY_SIZE;
+    uint32_t *data = (uint32_t *)&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)) {
+        return false;
+    }
+    return fmask || dev->msix_table[offset + PCI_MSIX_ENTRY_VECTOR_CTRL] &
+        PCI_MSIX_ENTRY_CTRL_MASKBIT;
 }
 
 bool msix_is_masked(PCIDevice *dev, unsigned int vector)

diff --git a/hw/xen/xen_pt_msi.c b/hw/xen/xen_pt_msi.c
index 263e0514a23b66b0f6f41111db272934db257b85..301ad4c3ce26da2fb1fdf6000bf934235d8d89e8 100644 (file)
--- a/hw/xen/xen_pt_msi.c
+++ b/hw/xen/xen_pt_msi.c
@@ -103,9 +103,7 @@ static int msi_msix_setup(XenPCIPassthroughState *s,
 
     assert((!is_msix && msix_entry == 0) || is_msix);
 
-    if (gvec == 0) {
-        /* if gvec is 0, the guest is asking for a particular pirq that
-         * is passed as dest_id */
+    if (xen_is_pirq_msi(data)) {
         *ppirq = msi_ext_dest_id(addr >> 32) | msi_dest_id(addr);
         if (!*ppirq) {
             /* this probably identifies an misconfiguration of the guest,

diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index b0ed04caa966e6572a81f269e5f2b64478c7de3f..fcd85b5c0525fb545064820a31bd332acd928e6d 100644 (file)
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -33,6 +33,7 @@ void xen_piix3_set_irq(void *opaque, int irq_num, int level);
 void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int len);
 void xen_hvm_inject_msi(uint64_t addr, uint32_t data);
 void xen_cmos_set_s3_resume(void *opaque, int irq, int level);
+int xen_is_pirq_msi(uint32_t msi_data);
 
 qemu_irq *xen_interrupt_controller_init(void);
 

diff --git a/xen-hvm-stub.c b/xen-hvm-stub.c
index 2d98696e72ba12bbbb2a6a317e1b372b244ccadf..d2969aa66f278b6c23c18e5fca7a8821d9b5b98c 100644 (file)
--- a/xen-hvm-stub.c
+++ b/xen-hvm-stub.c
@@ -34,6 +34,11 @@ void xen_cmos_set_s3_resume(void *opaque, int irq, int level)
 {
 }
 
+int xen_is_pirq_msi(uint32_t msi_data)
+{
+    return 0;
+}
+
 void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr)
 {
 }

diff --git a/xen-hvm.c b/xen-hvm.c
index 3a7fd5885a2ce23ca3a3a4015ef30255bb5e16a9..4a215e3f0a5124d962747826c8d4d95c5f81b8b1 100644 (file)
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -12,6 +12,7 @@
 
 #include "hw/pci/pci.h"
 #include "hw/i386/pc.h"
+#include "hw/i386/apic-msidef.h"
 #include "hw/xen/xen_common.h"
 #include "hw/xen/xen_backend.h"
 #include "qmp-commands.h"
@@ -156,6 +157,14 @@ void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int len)
     }
 }
 
+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;
+}
+
 void xen_hvm_inject_msi(uint64_t addr, uint32_t data)
 {
     xen_xc_hvm_inject_msi(xen_xc, xen_domid, addr, data);