Regression PCI passthrough from 4.5.5 to 4.6.0-rc1

Commit Message

Roger Pau Monné Aug. 15, 2017, 9:55 a.m. UTC

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é <[e-mail]>
> 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<---

Comments

Andreas Kinzler Aug. 17, 2017, 5:36 p.m. UTC | #1

On Tue, 15 Aug 2017 11:55:10 +0200, Roger Pau Monné <[e-mail]>  
wrote:
> Could you please try the patch below and paste the output you get on
> the Xen console?

Output is in attached file. Does it help?

Regards Andreas
(XEN) MSIX ctrl write. Enabled: 0 Maskall: 0. Configured entries:
(XEN) MSIX ctrl write. Enabled: 0 Maskall: 0. Configured entries:
(XEN) MSIX ctrl write. Enabled: 0 Maskall: 0. Configured entries:
(XEN) MSIX ctrl write. Enabled: 0 Maskall: 0. Configured entries:
(XEN) 0000:06:00.0 added entry 0 to msi_list
(XEN) 0000:06:00.0 added entry 0x1 to msi_list
(XEN) 0000:06:00.0 added entry 0x2 to msi_list
(XEN) MSIX ctrl write. Enabled: 1 Maskall: 1. Configured entries:
(XEN) 0 host_masked: 1 guest_masked: 1
(XEN) 0x1 host_masked: 1 guest_masked: 1
(XEN) 0x2 host_masked: 1 guest_masked: 1
(XEN) MSIX ctrl write. Enabled: 1 Maskall: 0. Configured entries:
(XEN) 0 host_masked: 1 guest_masked: 1
(XEN) 0x1 host_masked: 1 guest_masked: 1
(XEN) 0x2 host_masked: 1 guest_masked: 1
(XEN) MSIX ctrl write. Enabled: 0 Maskall: 0. Configured entries:
(XEN) 0000:06:00.1 added entry 0 to msi_list
(XEN) 0000:06:00.1 added entry 0x1 to msi_list
(XEN) 0000:06:00.1 added entry 0x2 to msi_list
(XEN) MSIX ctrl write. Enabled: 1 Maskall: 1. Configured entries:
(XEN) 0 host_masked: 1 guest_masked: 1
(XEN) 0x1 host_masked: 1 guest_masked: 1
(XEN) 0x2 host_masked: 1 guest_masked: 1
(XEN) MSIX ctrl write. Enabled: 1 Maskall: 0. Configured entries:
(XEN) 0 host_masked: 1 guest_masked: 1
(XEN) 0x1 host_masked: 1 guest_masked: 1
(XEN) 0x2 host_masked: 1 guest_masked: 1
(XEN) MSIX ctrl write. Enabled: 0 Maskall: 0. Configured entries:
(XEN) MSIX ctrl write. Enabled: 0 Maskall: 0. Configured entries:
(XEN) 0000:02:00.0 added entry 0 to msi_list
(XEN) 0000:02:00.0 added to msixtbl list
(XEN) 0000:02:00.0 added entry 0x1 to msi_list
(XEN) 0000:02:00.0 added entry 0x2 to msi_list
(XEN) 0000:02:00.0 added entry 0x3 to msi_list
(XEN) 0000:02:00.0 added entry 0x4 to msi_list
(XEN) 0000:02:00.0 added entry 0x5 to msi_list
(XEN) 0000:02:00.0 added entry 0x6 to msi_list
(XEN) 0000:02:00.0 added entry 0x7 to msi_list
(XEN) 0000:02:00.0 added entry 0x8 to msi_list
(XEN) 0000:02:00.0 added entry 0x9 to msi_list
(XEN) 0000:02:00.0 added entry 0xa to msi_list
(XEN) 0000:02:00.0 added entry 0xb to msi_list
(XEN) 0000:02:00.0 added entry 0xc to msi_list
(XEN) 0000:02:00.0 added entry 0xd to msi_list
(XEN) 0000:02:00.0 added entry 0xe to msi_list
(XEN) MSIX ctrl write. Enabled: 1 Maskall: 0. Configured entries:
(XEN) 0 host_masked: 0 guest_masked: 1
(XEN) 0x1 host_masked: 0 guest_masked: 1
(XEN) 0x2 host_masked: 0 guest_masked: 1
(XEN) 0x3 host_masked: 0 guest_masked: 1
(XEN) 0x4 host_masked: 0 guest_masked: 1
(XEN) 0x5 host_masked: 0 guest_masked: 1
(XEN) 0x6 host_masked: 0 guest_masked: 1
(XEN) 0x7 host_masked: 0 guest_masked: 1
(XEN) 0x8 host_masked: 0 guest_masked: 1
(XEN) 0x9 host_masked: 0 guest_masked: 1
(XEN) 0xa host_masked: 0 guest_masked: 1
(XEN) 0xb host_masked: 0 guest_masked: 1
(XEN) 0xc host_masked: 0 guest_masked: 1
(XEN) 0xd host_masked: 0 guest_masked: 1
(XEN) 0xe host_masked: 0 guest_masked: 1

Jan Beulich Aug. 18, 2017, 7:16 a.m. UTC | #2

>>> On 17.08.17 at 19:36, <[e-mail]> wrote:
> On Tue, 15 Aug 2017 11:55:10 +0200, Roger Pau Monné <[e-mail]>  wrote:
>> Could you please try the patch below and paste the output you get on
>> the Xen console?
> 
> Output is in attached file. Does it help?

For the moment I'll defer to Roger, as he wrote the debugging patch.
If I wanted to look at this, I'd really wish to see a matching pair of
hypervisor and qemu logs (i.e. from the same VM instance).

Jan

Patch

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;