Re: Regression PCI passthrough from 4.5.5 to 4.6.0-rc1

["Roger Pau Monné" <roger.pau@citrix.com>]

[-- Attachment #1: Type: text/plain, Size: 1047 bytes --]

On Mon, Aug 21, 2017 at 04:18:56PM +0100, Roger Pau Monné wrote:
> On Mon, Aug 21, 2017 at 09:14:45AM -0600, Jan Beulich wrote:
> > >>> On 21.08.17 at 16:49, <roger.pau@citrix.com> wrote:
> > > Another option is to (ab)use the msi.gflags field to add another flag
> > > in order to signal Xen whether the interrupt should be unmasked. This
> > > is in line with what you suggest below.
> > 
> > From a brief look it looks like this would be doable, but the way these
> > flags are being communicated is rather ugly (the values used here
> > aren't part of the public interface, and hence it wasn't immediately
> > clear whether using one of the unused bits would be an option, but
> > it looks like it is).
> 
> Yes, it's not pretty... Last used bit is 15, hence bit 16 could be
> used to signal to Xen whether the interrupt should be unmasked after
> binding. I have a half-drafted patch, will finish it now.

Hello,

Andreas, could you please give a try to the attached two patches? One
is for Xen and the other one is for QEMU.

Thanks, Roger.

[-- Attachment #2: xen.patch --]
[-- Type: text/plain, Size: 2638 bytes --]

diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/io.c
index ffeaf70be6..7e63007fbf 100644
--- a/xen/drivers/passthrough/io.c
+++ b/xen/drivers/passthrough/io.c
@@ -343,13 +343,16 @@ int pt_irq_create_bind(
         uint8_t dest, dest_mode, delivery_mode;
         int dest_vcpu_id;
         const struct vcpu *vcpu;
+        uint32_t gflags = pt_irq_bind->u.msi.gflags & ~VMSI_UNMASKED;
+        struct irq_desc *desc;
+        unsigned long flags;
 
         if ( !(pirq_dpci->flags & HVM_IRQ_DPCI_MAPPED) )
         {
             pirq_dpci->flags = HVM_IRQ_DPCI_MAPPED | HVM_IRQ_DPCI_MACH_MSI |
                                HVM_IRQ_DPCI_GUEST_MSI;
             pirq_dpci->gmsi.gvec = pt_irq_bind->u.msi.gvec;
-            pirq_dpci->gmsi.gflags = pt_irq_bind->u.msi.gflags;
+            pirq_dpci->gmsi.gflags = gflags;
             /*
              * 'pt_irq_create_bind' can be called after 'pt_irq_destroy_bind'.
              * The 'pirq_cleanup_check' which would free the structure is only
@@ -402,13 +405,13 @@ int pt_irq_create_bind(
 
             /* If pirq is already mapped as vmsi, update guest data/addr. */
             if ( pirq_dpci->gmsi.gvec != pt_irq_bind->u.msi.gvec ||
-                 pirq_dpci->gmsi.gflags != pt_irq_bind->u.msi.gflags )
+                 pirq_dpci->gmsi.gflags != gflags )
             {
                 /* Directly clear pending EOIs before enabling new MSI info. */
                 pirq_guest_eoi(info);
 
                 pirq_dpci->gmsi.gvec = pt_irq_bind->u.msi.gvec;
-                pirq_dpci->gmsi.gflags = pt_irq_bind->u.msi.gflags;
+                pirq_dpci->gmsi.gflags = gflags;
             }
         }
         /* Calculate dest_vcpu_id for MSI-type pirq migration. */
@@ -439,6 +442,13 @@ int pt_irq_create_bind(
             pi_update_irte(vcpu ? &vcpu->arch.hvm_vmx.pi_desc : NULL,
                            info, pirq_dpci->gmsi.gvec);
 
+        desc = irq_to_desc(info->arch.irq);
+        ASSERT(desc);
+
+        spin_lock_irqsave(&desc->lock, flags);
+        guest_mask_msi_irq(desc, !(pt_irq_bind->u.msi.gflags & VMSI_UNMASKED));
+        spin_unlock_irqrestore(&desc->lock, flags);
+
         break;
     }
 
diff --git a/xen/include/xen/hvm/irq.h b/xen/include/xen/hvm/irq.h
index 0d2c72c109..bdc843fb9a 100644
--- a/xen/include/xen/hvm/irq.h
+++ b/xen/include/xen/hvm/irq.h
@@ -56,6 +56,7 @@ struct dev_intx_gsi_link {
 #define VMSI_DM_MASK      0x200
 #define VMSI_DELIV_MASK   0x7000
 #define VMSI_TRIG_MODE    0x8000
+#define VMSI_UNMASKED     0x10000
 
 #define GFLAGS_SHIFT_RH             8
 #define GFLAGS_SHIFT_DELIV_MODE     12

[-- Attachment #3: qemu.patch --]
[-- Type: text/plain, Size: 1851 bytes --]

diff --git a/hw/xen/xen_pt_msi.c b/hw/xen/xen_pt_msi.c
index ff9a79f5d2..971441dbab 100644
--- a/hw/xen/xen_pt_msi.c
+++ b/hw/xen/xen_pt_msi.c
@@ -24,6 +24,7 @@
 #define XEN_PT_GFLAGS_SHIFT_DM             9
 #define XEN_PT_GFLAGSSHIFT_DELIV_MODE     12
 #define XEN_PT_GFLAGSSHIFT_TRG_MODE       15
+#define XEN_PT_GFLAGSSHIFT_UNMASKED       16
 
 #define latch(fld) latch[PCI_MSIX_ENTRY_##fld / sizeof(uint32_t)]
 
@@ -155,7 +156,8 @@ static int msi_msix_update(XenPCIPassthroughState *s,
                            int pirq,
                            bool is_msix,
                            int msix_entry,
-                           int *old_pirq)
+                           int *old_pirq,
+                           bool masked)
 {
     PCIDevice *d = &s->dev;
     uint8_t gvec = msi_vector(data);
@@ -171,6 +173,8 @@ static int msi_msix_update(XenPCIPassthroughState *s,
         table_addr = s->msix->mmio_base_addr;
     }
 
+    gflags |= masked ? 0 : XEN_PT_GFLAGSSHIFT_UNMASKED;
+
     rc = xc_domain_update_msi_irq(xen_xc, xen_domid, gvec,
                                   pirq, gflags, table_addr);
 
@@ -274,7 +278,7 @@ int xen_pt_msi_update(XenPCIPassthroughState *s)
 {
     XenPTMSI *msi = s->msi;
     return msi_msix_update(s, msi_addr64(msi), msi->data, msi->pirq,
-                           false, 0, &msi->pirq);
+                           false, 0, &msi->pirq, false);
 }
 
 void xen_pt_msi_disable(XenPCIPassthroughState *s)
@@ -355,7 +359,8 @@ static int xen_pt_msix_update_one(XenPCIPassthroughState *s, int entry_nr,
     }
 
     rc = msi_msix_update(s, entry->addr, entry->data, pirq, true,
-                         entry_nr, &entry->pirq);
+                         entry_nr, &entry->pirq,
+                         vec_ctrl & PCI_MSIX_ENTRY_CTRL_MASKBIT);
 
     if (!rc) {
         entry->updated = false;

[-- Attachment #4: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel