| Message ID | 20170815110024.7grrprg7fg6gg3z3@MacBook-Pro-de-Roger.local (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 15.08.17 at 13:00, <roger.pau@citrix.com> wrote: > On Tue, Aug 15, 2017 at 10:55:10AM +0100, Roger Pau Monné wrote: >> 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@citrix.com> >> > 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. > > So here is the patch for QEMU, if you don't mind giving it a try. I'm > not really sure this is correct, since it will force Xen to enable > MSIX in order to configure the entries, while the guest will still > have MSIX disabled, but might we worth giving it a try. But that's not a debugging patch then, but one trying a (guessed?) solution. A debugging patch would not alter functionality (unless a clear problem with a clear solution was found), but rather log state in an extended fashion. Jan Jan
On Tue, Aug 15, 2017 at 06:12:32AM -0600, Jan Beulich wrote: > >>> On 15.08.17 at 13:00, <roger.pau@citrix.com> wrote: > > On Tue, Aug 15, 2017 at 10:55:10AM +0100, Roger Pau Monné wrote: > >> 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@citrix.com> > >> > 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. > > > > So here is the patch for QEMU, if you don't mind giving it a try. I'm > > not really sure this is correct, since it will force Xen to enable > > MSIX in order to configure the entries, while the guest will still > > have MSIX disabled, but might we worth giving it a try. > > But that's not a debugging patch then, but one trying a (guessed?) > solution. A debugging patch would not alter functionality (unless a > clear problem with a clear solution was found), but rather log state > in an extended fashion. I've sent the debugging patch in a first email [0], just thought I could also sent this one so it's probably easier for Andreas to test both of them in a row (but not together). Roger. [0] https://lists.xenproject.org/archives/html/xen-devel/2017-08/msg01578.html
diff --git a/hw/xen/xen_pt_msi.c b/hw/xen/xen_pt_msi.c index ff9a79f5d2..dfb8d64654 100644 --- a/hw/xen/xen_pt_msi.c +++ b/hw/xen/xen_pt_msi.c @@ -451,8 +451,12 @@ static void pci_msix_write(void *opaque, hwaddr addr, } entry->updated = true; - } else if (msix->enabled && entry->updated && - !(val & PCI_MSIX_ENTRY_CTRL_MASKBIT)) { + } else if (entry->updated && !(val & PCI_MSIX_ENTRY_CTRL_MASKBIT)) { + /* + * NB: always register the entries with Xen when the write to the MSIX + * table happens, or else Xen won't be able to correctly snoop the + * entry control register write, and will fails to unmask the vector. + */ const volatile uint32_t *vec_ctrl; /*