Re: [Xen-devel] Regression PCI passthrough from 4.5.5 to 4.6.0-rc1

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Aug 14, 2017 at 02:08:56PM +0200, Andreas Kinzler wrote:
> On Mon, 14 Aug 2017 13:56:58 +0200, Roger Pau Monné <roger.pau@xxxxxxxxxx>
> wrote:
> > > > I defined XEN_PT_LOGGING_ENABLED in xen_pt.h as requested without the
> > > > "hack" patch. Log is attached. Does it help?
> > > It tells me that there's nothing unexpected on that side. As I think I
> > > had indicated before, we really need to see both sides (qemu and
> > > hypervisor), as part of the MSI-X handling lives in Xen. And for the
> > > hypervisor side it is unlikely that we'll be able to get away without
> > > a debugging patch. I am intending to make such available to you in
> > > case you can't do so yourself, but I can't currently predict when I'll
> > > get to it.
> > I think the problem is that pci_msi_conf_write_intercept is failing to
> > unmask the entries when MSI-X is enabled with entries already
> > configured, but this will require some debugging patch as Jan said.
> > Following the MSI-X code is quite complicated, this split brain
> > between Xen and QEMU makes it quite hard. I can try to come up with a
> > patch later.
> 
> I can try some debug patches although my workload is very high at the
> moment. It would help me quite a bit if the debug patches were suitable for
> the stable 4.8 tree.

Hello,

Could you please try the patch below and paste the output you get on
the Xen console?

Jan, AFAICT (but I have to admit it's not easy to follow the code at
all), the following series of events will cause the MSIX entries to
not be unmasked:

1. Guest configures the MSIX table entries and unmasks each of them.
2. Guest enables MSIX.

This will cause the entries to remain masked, because QEMU will only
register the PIRQs and bind them when the MSI-X enable bit is set,
instead of doing it for each write to the MSIX table.

I guess one way to solve this would be to force QEMU to call
xen_pt_msix_update_one in pci_msix_write once the entry is unmasked,
even if MSIX is not enabled. I can prepare a patch for that.

This doesn't happen with Linux/FreeBSD because both of them enabled
MSIX first and then configure the table entries and unmask them.

Roger.

---8<---
diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c
index d81c5d47c6..d7c64dcd90 100644
--- a/xen/arch/x86/hvm/vmsi.c
+++ b/xen/arch/x86/hvm/vmsi.c
@@ -330,6 +330,8 @@ static int msixtbl_write(struct vcpu *v, unsigned long 
address,
 
     ASSERT(msi_desc == desc->msi_desc);
    
+    printk("%smasking entry %#x\n",
+           (val & PCI_MSIX_VECTOR_BITMASK) ? "" : "un", nr_entry);
     guest_mask_msi_irq(desc, !!(val & PCI_MSIX_VECTOR_BITMASK));
 
 unlock:
@@ -431,6 +433,9 @@ static void add_msixtbl_entry(struct domain *d,
     entry->gtable = (unsigned long) gtable;
 
     list_add_rcu(&entry->list, &d->arch.hvm_domain.msixtbl_list);
+
+    printk("%04x:%02x:%02x.%u added to msixtbl list\n", pdev->seg, pdev->bus,
+            PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
 }
 
 static void free_msixtbl_entry(struct rcu_head *rcu)
@@ -511,8 +516,12 @@ out:
                  (gtable + msi_desc->msi_attrib.entry_nr *
                            PCI_MSIX_ENTRY_SIZE +
                   PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET) )
+            {
+                printk("msixtbl_pt_register: detected attempt to write to 
vector ctrl (entry %#x)\n",
+                       msi_desc->msi_attrib.entry_nr);
                 v->arch.hvm_vcpu.hvm_io.msix_unmask_address =
                     v->arch.hvm_vcpu.hvm_io.msix_snoop_address;
+            }
         }
     }
 
@@ -621,6 +630,7 @@ void msix_write_completion(struct vcpu *v)
         return;
 
     v->arch.hvm_vcpu.hvm_io.msix_unmask_address = 0;
+    printk("Detected MSI-X unmask in write completion\n");
     if ( msixtbl_write(v, ctrl_address, 4, 0) != X86EMUL_OKAY )
         gdprintk(XENLOG_WARNING, "MSI-X write completion failure\n");
 }
diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index 2c38adb1b1..f36919d1c3 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -980,6 +980,8 @@ static int msix_capability_init(struct pci_dev *dev,
 
         list_add_tail(&entry->list, &dev->msi_list);
         *desc = entry;
+        printk("%04x:%02x:%02x.%u added entry %#x to msi_list\n",
+               seg, bus, slot, func, msi->entry_nr);
     }
 
     if ( !msix->used_entries )
@@ -1297,6 +1299,18 @@ int pci_msi_conf_write_intercept(struct pci_dev *pdev, 
unsigned int reg,
             if ( reg != msix_control_reg(pos) || size != 2 )
                 return -EACCES;
 
+                printk("MSIX ctrl write. Enabled: %d Maskall: %d. "
+                       "Configured entries:\n",
+                       !!(*data & PCI_MSIX_FLAGS_ENABLE),
+                       !!(*data & PCI_MSIX_FLAGS_MASKALL));
+                list_for_each_entry( entry, &pdev->msi_list, list )
+                {
+                    printk("%#x host_masked: %d guest_masked: %d\n",
+                           entry->msi_attrib.entry_nr,
+                           entry->msi_attrib.host_masked,
+                           entry->msi_attrib.guest_masked);
+                }
+
             pdev->msix->guest_maskall = !!(*data & PCI_MSIX_FLAGS_MASKALL);
             if ( pdev->msix->host_maskall )
                 *data |= PCI_MSIX_FLAGS_MASKALL;


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