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

2017-11-02 Thread Pasi Kärkkäinen 
On Thu, Nov 02, 2017 at 05:11:17PM +, Roger Pau Monné wrote:
> On Thu, Nov 02, 2017 at 07:02:49PM +0200, Pasi Kärkkäinen wrote:
> > On Thu, Aug 24, 2017 at 09:23:16AM +0100, Roger Pau Monné wrote:
> > > On Wed, Aug 23, 2017 at 07:13:00PM +0200, Andreas Kinzler wrote:
> > > > > > > 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.
> > > > > Andreas, could you please give a try to the attached two patches? One
> > > > > is for Xen and the other one is for QEMU.
> > > > 
> > > > Seems to work after I fixed a bug ;-)
> > > > 
> > > > -gflags |= masked ? 0 : XEN_PT_GFLAGSSHIFT_UNMASKED;
> > > > +gflags |= masked ? 0 : (1 << XEN_PT_GFLAGSSHIFT_UNMASKED);
> > > > 
> > > > Please let Jan and/or others review the patches.
> > > 
> > > Thanks. I would like to add your tested-by if possible, since I'm not
> > > able to trigger this behavior myself.
> > > 
> > 
> > Hmm.. did these patches get merged / acked? 
> 
> Yes, both the QEMU and the Xen sides have been merged.
> 

Great, thanks a lot!


> Roger.


-- Pasi


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

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

2017-11-02 Thread Roger Pau Monné 
On Thu, Nov 02, 2017 at 07:02:49PM +0200, Pasi Kärkkäinen wrote:
> On Thu, Aug 24, 2017 at 09:23:16AM +0100, Roger Pau Monné wrote:
> > On Wed, Aug 23, 2017 at 07:13:00PM +0200, Andreas Kinzler wrote:
> > > > > > 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.
> > > > Andreas, could you please give a try to the attached two patches? One
> > > > is for Xen and the other one is for QEMU.
> > > 
> > > Seems to work after I fixed a bug ;-)
> > > 
> > > -gflags |= masked ? 0 : XEN_PT_GFLAGSSHIFT_UNMASKED;
> > > +gflags |= masked ? 0 : (1 << XEN_PT_GFLAGSSHIFT_UNMASKED);
> > > 
> > > Please let Jan and/or others review the patches.
> > 
> > Thanks. I would like to add your tested-by if possible, since I'm not
> > able to trigger this behavior myself.
> > 
> 
> Hmm.. did these patches get merged / acked? 

Yes, both the QEMU and the Xen sides have been merged.

Roger.

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

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

2017-11-02 Thread Pasi Kärkkäinen 
On Thu, Aug 24, 2017 at 09:23:16AM +0100, Roger Pau Monné wrote:
> On Wed, Aug 23, 2017 at 07:13:00PM +0200, Andreas Kinzler wrote:
> > > > > 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.
> > > Andreas, could you please give a try to the attached two patches? One
> > > is for Xen and the other one is for QEMU.
> > 
> > Seems to work after I fixed a bug ;-)
> > 
> > -gflags |= masked ? 0 : XEN_PT_GFLAGSSHIFT_UNMASKED;
> > +gflags |= masked ? 0 : (1 << XEN_PT_GFLAGSSHIFT_UNMASKED);
> > 
> > Please let Jan and/or others review the patches.
> 
> Thanks. I would like to add your tested-by if possible, since I'm not
> able to trigger this behavior myself.
> 

Hmm.. did these patches get merged / acked? 


Thanks,

-- Pasi

> Roger.
> 


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

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

2017-08-24 Thread Roger Pau Monné 
On Wed, Aug 23, 2017 at 07:13:00PM +0200, Andreas Kinzler wrote:
> > > > 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.
> > Andreas, could you please give a try to the attached two patches? One
> > is for Xen and the other one is for QEMU.
> 
> Seems to work after I fixed a bug ;-)
> 
> -gflags |= masked ? 0 : XEN_PT_GFLAGSSHIFT_UNMASKED;
> +gflags |= masked ? 0 : (1 << XEN_PT_GFLAGSSHIFT_UNMASKED);
> 
> Please let Jan and/or others review the patches.

Thanks. I would like to add your tested-by if possible, since I'm not
able to trigger this behavior myself.

Roger.

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

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

2017-08-23 Thread Andreas Kinzler 
> 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.

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


Seems to work after I fixed a bug ;-)

-gflags |= masked ? 0 : XEN_PT_GFLAGSSHIFT_UNMASKED;
+gflags |= masked ? 0 : (1 << XEN_PT_GFLAGSSHIFT_UNMASKED);

Please let Jan and/or others review the patches.

Regards Andreas

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

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

2017-08-22 Thread Roger Pau Monné 
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,  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.
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 ? >arch.hvm_vmx.pi_desc : NULL,
info, pirq_dpci->gmsi.gvec);
 
+desc = irq_to_desc(info->arch.irq);
+ASSERT(desc);
+
+spin_lock_irqsave(>lock, flags);
+guest_mask_msi_irq(desc, !(pt_irq_bind->u.msi.gflags & VMSI_UNMASKED));
+spin_unlock_irqrestore(>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_MODE0x8000
+#define VMSI_UNMASKED 0x1
 
 #define GFLAGS_SHIFT_RH 8
 #define GFLAGS_SHIFT_DELIV_MODE 12
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 = >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,

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

2017-08-21 Thread Roger Pau Monné 
On Mon, Aug 21, 2017 at 09:14:45AM -0600, Jan Beulich wrote:
> >>> On 21.08.17 at 16:49,  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.

Thanks, Roger.

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

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

2017-08-21 Thread Jan Beulich 
>>> On 21.08.17 at 16:49,  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).

Jan


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

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

2017-08-21 Thread Roger Pau Monné 
On Mon, Aug 21, 2017 at 06:22:05AM -0600, Jan Beulich wrote:
> >>> On 21.08.17 at 11:46,  wrote:
> > Xen never detects the MSIX table entry unmask because it happens
> > before the MSIX is bound to the guest, so the Xen msixtbl handlers are
> > not aware of this memory region.
> > 
> > The two possible solutions I see to this are:
> > 
> >  - Make QEMU setup the vectors when the table entries are unmasked,
> >even if MSIX is not enabled.
> >  - Provide an hypercall so QEMU can unmask MSIX vectors on behalf of
> >the guest. This would be used to unmask the entries if MSIX is
> >enabled with table entries already unmasked.
> 
> Neither sounds right. As long as MSI-X is disabled (or the maskall
> bit set), the contents of the table entries is meaningless. It is not
> correct to assign any meaning to them in that state.

Right, they only become relevant when MSI-X is enabled, the maskall
bit is unset and the entry control register mask bit is also unset.

> And qemu
> should not be unmasking the entries on behalf of the guest either.
> It ought to be possible for Xen to know the state of the mask bits
> at the time of binding the interrupts.

AFAICT QEMU will only bound the interrupts when they are unmasked, so
the semantics of XEN_DOMCTL_bind_pt_irq could be changed so that MSI
interrupts are unmasked when bound, but that will change the current
behavior.

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.

Roger.

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

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

2017-08-21 Thread Jan Beulich 
>>> On 21.08.17 at 14:32,  wrote:
>> >  - Make QEMU setup the vectors when the table entries are unmasked,
>>>even if MSIX is not enabled.
>>>  - Provide an hypercall so QEMU can unmask MSIX vectors on behalf of
>>>the guest. This would be used to unmask the entries if MSIX is
>>>enabled with table entries already unmasked.
>> Neither sounds right. As long as MSI-X is disabled (or the maskall
>> bit set), the contents of the table entries is meaningless. It is not
>> correct to assign any meaning to them in that state. And qemu
>> should not be unmasking the entries on behalf of the guest either.
>> It ought to be possible for Xen to know the state of the mask bits
>> at the time of binding the interrupts. After all, with a new hypercall
>> added like you propose it, there would still be a window in time
>> where neither setting (masked or unmasked) would be correct in
>> Xen's internal recording of state. Quite possibly the only solution
>> is for qemu to communicate the intended state of the mask bit
>> while binding the interrupt.
> 
> as someone who is not familiar with MSI-X stuff, I still have one obvious  
> question: Why was all of this not a problem before the commit I bisected  
> down? Everything did work before and to be honest there is a "business"  
> side to my question: Since that patch it has actually been broken for at  
> least 3 major Xen releaes (4.6-4.8). Does that make any sense? For the  
> reputation of Xen?

For the reputation of Xen this is not nice, I agree. But this being an
open source project, you would have been free to contribute a fix
in all of this time. In no case is it, imo, appropriate to demand an
immediate solution here - if there's a business aspect for you and
you don't feel capable of analyzing and fixing the issue yourself,
you always have the option of paying someone who is.

The fact that things had worked before that change does _not_
mean the change was bad. It's is far more likely for things to have
worked out of pure luck.

Jan


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

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

2017-08-21 Thread Andreas Kinzler 

 - Make QEMU setup the vectors when the table entries are unmasked,
   even if MSIX is not enabled.
 - Provide an hypercall so QEMU can unmask MSIX vectors on behalf of
   the guest. This would be used to unmask the entries if MSIX is
   enabled with table entries already unmasked.

Neither sounds right. As long as MSI-X is disabled (or the maskall
bit set), the contents of the table entries is meaningless. It is not
correct to assign any meaning to them in that state. And qemu
should not be unmasking the entries on behalf of the guest either.
It ought to be possible for Xen to know the state of the mask bits
at the time of binding the interrupts. After all, with a new hypercall
added like you propose it, there would still be a window in time
where neither setting (masked or unmasked) would be correct in
Xen's internal recording of state. Quite possibly the only solution
is for qemu to communicate the intended state of the mask bit
while binding the interrupt.


Hello Jan + Roger,

as someone who is not familiar with MSI-X stuff, I still have one obvious  
question: Why was all of this not a problem before the commit I bisected  
down? Everything did work before and to be honest there is a "business"  
side to my question: Since that patch it has actually been broken for at  
least 3 major Xen releaes (4.6-4.8). Does that make any sense? For the  
reputation of Xen?


Regards Andreas

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

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

2017-08-21 Thread Jan Beulich 
>>> On 21.08.17 at 11:46,  wrote:
> Xen never detects the MSIX table entry unmask because it happens
> before the MSIX is bound to the guest, so the Xen msixtbl handlers are
> not aware of this memory region.
> 
> The two possible solutions I see to this are:
> 
>  - Make QEMU setup the vectors when the table entries are unmasked,
>even if MSIX is not enabled.
>  - Provide an hypercall so QEMU can unmask MSIX vectors on behalf of
>the guest. This would be used to unmask the entries if MSIX is
>enabled with table entries already unmasked.

Neither sounds right. As long as MSI-X is disabled (or the maskall
bit set), the contents of the table entries is meaningless. It is not
correct to assign any meaning to them in that state. And qemu
should not be unmasking the entries on behalf of the guest either.
It ought to be possible for Xen to know the state of the mask bits
at the time of binding the interrupts. After all, with a new hypercall
added like you propose it, there would still be a window in time
where neither setting (masked or unmasked) would be correct in
Xen's internal recording of state. Quite possibly the only solution
is for qemu to communicate the intended state of the mask bit
while binding the interrupt.

Jan


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

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

2017-08-21 Thread Roger Pau Monné 
On Mon, Aug 21, 2017 at 11:09:01AM +0200, Andreas Kinzler wrote:
> Hello Roger & Jan,
> 
> > > > 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?
> > Yes, thanks. I'm quite sure the problem is that MSIX table entries are
> > being unmasked before MSIX is enabled, and so Xen is not able to snoop
> > those writes.
> > Just to confirm, can you try the following two debug patches? One is
> > for the hypervisor, the other is for QEMU.
> 
> Attached ZIP file contains hypervisor and qemu log.

Thanks, so the guest is indeed unmasking the MSIX table entries before
MSIX is enabled, and QEMU only registers the entries with Xen once MSIX
is enabled:

Trying to interleave the Xen and QEMU log correctly, I think the
following is the current flow:

[00:05.0] pci_msix_write: Write to MSIX table entry 0 CTRL, masked: 0
[...]
[00:05.0] xen_pt_msixctrl_reg_write: Enabling MSIX, setting up entries
[00:05.0] xen_pt_msix_update_one: Setting up MSIX vector 0 PIRQ: -1 Masked: 0
[00:05.0] msi_msix_setup: Mapping PIRQ for MSIX entry 0
[00:05.0] msi_msix_update: Updating MSI-X with pirq 55 gvec 0x81 gflags 0x1301 
(entry: 0)
(XEN) :02:00.0 added entry 0 to msi_list
(XEN) :02:00.0 added to msixtbl list
[...]
[00:05.0] xen_pt_msixctrl_reg_write: enable MSI-X
(XEN) MSIX ctrl write. Enabled: 1 Maskall: 0. Configured entries:
(XEN) 0 host_masked: 0 guest_masked: 1

Xen never detects the MSIX table entry unmask because it happens
before the MSIX is bound to the guest, so the Xen msixtbl handlers are
not aware of this memory region.

The two possible solutions I see to this are:

 - Make QEMU setup the vectors when the table entries are unmasked,
   even if MSIX is not enabled.
 - Provide an hypercall so QEMU can unmask MSIX vectors on behalf of
   the guest. This would be used to unmask the entries if MSIX is
   enabled with table entries already unmasked.

The patch I've sent earlier (which I'm also attaching to this email)
implements the first solution for QEMU.

Can you please give it a try?

Thanks.
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;
 
 /*
-- 
2.11.0 (Apple Git-81)

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

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

2017-08-18 Thread Roger Pau Monné 
On Thu, Aug 17, 2017 at 07:36:20PM +0200, Andreas Kinzler wrote:
> On Tue, 15 Aug 2017 11:55:10 +0200, Roger Pau Monné 
> 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?

Yes, thanks. I'm quite sure the problem is that MSIX table entries are
being unmasked before MSIX is enabled, and so Xen is not able to snoop
those writes.

Just to confirm, can you try the following two debug patches? One is
for the hypervisor, the other is for QEMU.

Thanks, Roger.
diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c
index a36692c313..87caef300a 100644
--- a/xen/arch/x86/hvm/vmsi.c
+++ b/xen/arch/x86/hvm/vmsi.c
@@ -329,6 +329,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:
@@ -430,6 +432,9 @@ static void add_msixtbl_entry(struct domain *d,
 entry->gtable = (unsigned long) gtable;
 
 list_add_rcu(>list, >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)
@@ -510,8 +515,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;
+}
 }
 }
 
@@ -619,6 +628,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 77998f4fb3..0ca31c22e2 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(>list, >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, >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;
diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index 191d9caea1..c296cf682c 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -10,6 +10,7 @@ void xen_pt_log(const PCIDevice *d, const char *f, ...) 
GCC_FMT_ATTR(2, 3);
 
 #define XEN_PT_ERR(d, _f, _a...) xen_pt_log(d, "%s: Error: "_f, __func__, ##_a)
 
+#define XEN_PT_LOGGING_ENABLED 1
 #ifdef XEN_PT_LOGGING_ENABLED
 #  define XEN_PT_LOG(d, _f, _a...)  xen_pt_log(d, "%s: " _f, __func__, ##_a)
 #  define XEN_PT_WARN(d, _f, _a...) \
diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
index 1f04ec5eec..893ac06a80 100644
--- a/hw/xen/xen_pt_config_init.c
+++ b/hw/xen/xen_pt_config_init.c
@@ -1482,6 +1482,7 @@ static int 
xen_pt_msixctrl_reg_write(XenPCIPassthroughState *s,
 /* update MSI-X */
 if ((*val & PCI_MSIX_FLAGS_ENABLE)
 && !(*val & PCI_MSIX_FLAGS_MASKALL)) {
+XEN_PT_LOG(>dev, "Enabling MSIX, setting up entries\n");
 xen_pt_msix_update(s);
 } else if (!(*val & PCI_MSIX_FLAGS_ENABLE) && s->msix->enabled) {
 xen_pt_msix_disable(s);
diff --git a/hw/xen/xen_pt_msi.c b/hw/xen/xen_pt_msi.c
index dfb8d64654..2662eb1940 100644
--- a/hw/xen/xen_pt_msi.c
+++ b/hw/xen/xen_pt_msi.c
@@ -132,8 +132,9 @@ static int msi_msix_setup(XenPCIPassthroughState *s,
 
 if (is_msix) {

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

2017-08-18 Thread Jan Beulich 
>>> On 17.08.17 at 19:36,  wrote:
> On Tue, 15 Aug 2017 11:55:10 +0200, Roger Pau Monné   
> 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

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

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

2017-08-17 Thread Andreas Kinzler 
On Tue, 15 Aug 2017 11:55:10 +0200, Roger Pau Monné   
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) :06:00.0 added entry 0 to msi_list
(XEN) :06:00.0 added entry 0x1 to msi_list
(XEN) :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) :06:00.1 added entry 0 to msi_list
(XEN) :06:00.1 added entry 0x1 to msi_list
(XEN) :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) :02:00.0 added entry 0 to msi_list
(XEN) :02:00.0 added to msixtbl list
(XEN) :02:00.0 added entry 0x1 to msi_list
(XEN) :02:00.0 added entry 0x2 to msi_list
(XEN) :02:00.0 added entry 0x3 to msi_list
(XEN) :02:00.0 added entry 0x4 to msi_list
(XEN) :02:00.0 added entry 0x5 to msi_list
(XEN) :02:00.0 added entry 0x6 to msi_list
(XEN) :02:00.0 added entry 0x7 to msi_list
(XEN) :02:00.0 added entry 0x8 to msi_list
(XEN) :02:00.0 added entry 0x9 to msi_list
(XEN) :02:00.0 added entry 0xa to msi_list
(XEN) :02:00.0 added entry 0xb to msi_list
(XEN) :02:00.0 added entry 0xc to msi_list
(XEN) :02:00.0 added entry 0xd to msi_list
(XEN) :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
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

2017-08-15 Thread Roger Pau Monné 
On Tue, Aug 15, 2017 at 06:12:32AM -0600, Jan Beulich wrote:
> >>> On 15.08.17 at 13:00,  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é 
> >> > 
> >> > 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

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

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

2017-08-15 Thread Jan Beulich 
>>> On 15.08.17 at 13:00,  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é 
>> > 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

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

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

2017-08-15 Thread Roger Pau Monné 
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é 
> > 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.

---8<---
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;
 
 /*


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

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

2017-08-15 Thread Roger Pau Monné 
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é 
> 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(>list, >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(>list, >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, >msi_list, list )
+{
+printk("%#x host_masked: %d guest_masked: %d\n",
+   entry->msi_attrib.entry_nr,
+

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

2017-08-14 Thread Andreas Kinzler 
On Mon, 14 Aug 2017 13:56:58 +0200, Roger Pau Monné   
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.


Regards Andreas

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

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

2017-08-14 Thread Roger Pau Monné 
On Mon, Aug 14, 2017 at 05:44:00AM -0600, Jan Beulich wrote:
> >>> On 14.08.17 at 13:31,  wrote:
> > On Mon, 31 Jul 2017 12:12:45 +0200, Jan Beulich  wrote:
> > "Andreas Kinzler"  07/17/17 6:32 PM >>>
> > Jan, I still have access to the hardware so perhaps we can finally  
> > solve
> > this problem.
>  Feel free to go ahead; I'll be on vacation for the next three weeks.
> >>> Perhaps we can shortcut debugging a bit because I looked through the
> >>> patches of XenServer 7.2 and found the attached patch. Now I tried it  
> >>> and
> >>> it seems to solve all the problems. Does that patch look good to you,  
> >>> too?
> >> Iirc the patch had even been submitted once, and rejected as being not
> >> generally correct (i.e. it cures a symptom rather than the cause). What
> >> we'd need to know is the order of actions the guest takes which ought to
> >> result in the vector getting unmasked, but doesn't in reality.
> > 
> > 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.

Roger.

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

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

2017-08-14 Thread Jan Beulich 
>>> On 14.08.17 at 13:31,  wrote:
> On Mon, 31 Jul 2017 12:12:45 +0200, Jan Beulich  wrote:
> "Andreas Kinzler"  07/17/17 6:32 PM >>>
> Jan, I still have access to the hardware so perhaps we can finally  
> solve
> this problem.
 Feel free to go ahead; I'll be on vacation for the next three weeks.
>>> Perhaps we can shortcut debugging a bit because I looked through the
>>> patches of XenServer 7.2 and found the attached patch. Now I tried it  
>>> and
>>> it seems to solve all the problems. Does that patch look good to you,  
>>> too?
>> Iirc the patch had even been submitted once, and rejected as being not
>> generally correct (i.e. it cures a symptom rather than the cause). What
>> we'd need to know is the order of actions the guest takes which ought to
>> result in the vector getting unmasked, but doesn't in reality.
> 
> 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.

Jan


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

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

2017-08-14 Thread Andreas Kinzler 

On Mon, 31 Jul 2017 12:12:45 +0200, Jan Beulich  wrote:

"Andreas Kinzler"  07/17/17 6:32 PM >>>
Jan, I still have access to the hardware so perhaps we can finally  
solve

this problem.

Feel free to go ahead; I'll be on vacation for the next three weeks.

Perhaps we can shortcut debugging a bit because I looked through the
patches of XenServer 7.2 and found the attached patch. Now I tried it  
and
it seems to solve all the problems. Does that patch look good to you,  
too?

Iirc the patch had even been submitted once, and rejected as being not
generally correct (i.e. it cures a symptom rather than the cause). What
we'd need to know is the order of actions the guest takes which ought to
result in the vector getting unmasked, but doesn't in reality.


I defined XEN_PT_LOGGING_ENABLED in xen_pt.h as requested without the  
"hack" patch. Log is attached. Does it help?


Regards Andreas[00:05.0] xen_pt_realize: Assigning real physical device 02:00.0 to devfn 0x28
[00:05.0] xen_pt_register_regions: IO region 0 registered (size=0x0100 
base_addr=0xe000 type: 0x1)
[00:05.0] xen_pt_register_regions: IO region 1 registered (size=0x4000 
base_addr=0xdf2c type: 0x4)
[00:05.0] xen_pt_register_regions: IO region 3 registered (size=0x0004 
base_addr=0xdf28 type: 0x4)
[00:05.0] xen_pt_register_regions: Expansion ROM registered (size=0x0008 
base_addr=0xdf20)
[00:05.0] xen_pt_config_reg_init: Offset 0x000e mismatch! Emulated=0x0080, 
host=0x, syncing to 0x0080.
[00:05.0] xen_pt_config_reg_init: Offset 0x0010 mismatch! Emulated=0x, 
host=0xe001, syncing to 0xe001.
[00:05.0] xen_pt_config_reg_init: Offset 0x0014 mismatch! Emulated=0x, 
host=0xdf2c0004, syncing to 0xdf2c0004.
[00:05.0] xen_pt_config_reg_init: Offset 0x001c mismatch! Emulated=0x, 
host=0xdf280004, syncing to 0xdf280004.
[00:05.0] xen_pt_config_reg_init: Offset 0x0052 mismatch! Emulated=0x, 
host=0x0603, syncing to 0x0603.
[00:05.0] xen_pt_config_reg_init: Offset 0x00aa mismatch! Emulated=0x, 
host=0x0080, syncing to 0x0080.
[00:05.0] xen_pt_config_reg_init: Offset 0x006c mismatch! Emulated=0x, 
host=0x10008025, syncing to 0x8025.
[00:05.0] xen_pt_config_reg_init: Offset 0x007a mismatch! Emulated=0x, 
host=0x1042, syncing to 0x1042.
[00:05.0] xen_pt_msix_init: get MSI-X table BAR base 0xdf2c
[00:05.0] xen_pt_msix_init: table_off = 0x2000, total_entries = 15
[00:05.0] xen_pt_msix_init: mapping physical MSI-X table to 0x7fc1bb3e9000
[00:05.0] xen_pt_config_reg_init: Offset 0x00c2 mismatch! Emulated=0x, 
host=0x000e, syncing to 0x000e.
[00:05.0] xen_pt_pci_intx: intx=1
[00:05.0] xen_pt_realize: Real physical device 02:00.0 registered successfully
[00:05.0] msi_msix_update: Updating MSI-X with pirq 55 gvec 0x71 gflags 0x1301 
(entry: 0)
[00:05.0] msi_msix_update: Updating MSI-X with pirq 54 gvec 0xa1 gflags 0x1302 
(entry: 0x1)
[00:05.0] msi_msix_update: Updating MSI-X with pirq 53 gvec 0xa1 gflags 0x1304 
(entry: 0x2)
[00:05.0] msi_msix_update: Updating MSI-X with pirq 52 gvec 0xa1 gflags 0x1308 
(entry: 0x3)
[00:05.0] msi_msix_update: Updating MSI-X with pirq 51 gvec 0x61 gflags 0x1301 
(entry: 0x4)
[00:05.0] msi_msix_update: Updating MSI-X with pirq 50 gvec 0x71 gflags 0x1302 
(entry: 0x5)
[00:05.0] msi_msix_update: Updating MSI-X with pirq 49 gvec 0x71 gflags 0x1304 
(entry: 0x6)
[00:05.0] msi_msix_update: Updating MSI-X with pirq 48 gvec 0x71 gflags 0x1308 
(entry: 0x7)
[00:05.0] msi_msix_update: Updating MSI-X with pirq 47 gvec 0xb2 gflags 0x1301 
(entry: 0x8)
[00:05.0] msi_msix_update: Updating MSI-X with pirq 46 gvec 0x61 gflags 0x1302 
(entry: 0x9)
[00:05.0] msi_msix_update: Updating MSI-X with pirq 45 gvec 0x61 gflags 0x1304 
(entry: 0xa)
[00:05.0] msi_msix_update: Updating MSI-X with pirq 44 gvec 0x61 gflags 0x1308 
(entry: 0xb)
[00:05.0] msi_msix_update: Updating MSI-X with pirq 43 gvec 0xa2 gflags 0x1301 
(entry: 0xc)
[00:05.0] msi_msix_update: Updating MSI-X with pirq 42 gvec 0xb2 gflags 0x1302 
(entry: 0xd)
[00:05.0] msi_msix_update: Updating MSI-X with pirq 41 gvec 0xb2 gflags 0x1304 
(entry: 0xe)
[00:05.0] xen_pt_msixctrl_reg_write: enable MSI-X
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

2017-07-31 Thread Jan Beulich 
>>> "Andreas Kinzler"  07/17/17 6:32 PM >>>
>>> Jan, I still have access to the hardware so perhaps we can finally solve
>>> this problem.
>> Feel free to go ahead; I'll be on vacation for the next three weeks.
>
>Perhaps we can shortcut debugging a bit because I looked through the  
>patches of XenServer 7.2 and found the attached patch. Now I tried it and  
>it seems to solve all the problems. Does that patch look good to you, too?

Iirc the patch had even been submitted once, and rejected as being not
generally correct (i.e. it cures a symptom rather than the cause). What
we'd need to know is the order of actions the guest takes which ought to
result in the vector getting unmasked, but doesn't in reality.

Jan


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

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

2017-07-21 Thread Pasi Kärkkäinen 
Hi,

On Mon, Jul 17, 2017 at 06:32:42PM +0200, Andreas Kinzler wrote:
> Hello Jan, Pasi, all
> 
> >>Jan, I still have access to the hardware so perhaps we can finally solve
> >>this problem.
> >Feel free to go ahead; I'll be on vacation for the next three weeks.
> 
> Perhaps we can shortcut debugging a bit because I looked through the patches
> of XenServer 7.2 and found the attached patch. Now I tried it and it seems
> to solve all the problems. Does that patch look good to you, too?
>

I think Jan is on vacation/offline currently.

Can someone else comment about the patch? Can it be upstreamed? Sounds like it 
fixes an actual bug/problem.


> Regards Andreas


Thanks,

-- Pasi


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

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

2017-07-17 Thread Andreas Kinzler 

Hello Jan, Pasi, all


Jan, I still have access to the hardware so perhaps we can finally solve
this problem.

Feel free to go ahead; I'll be on vacation for the next three weeks.


Perhaps we can shortcut debugging a bit because I looked through the  
patches of XenServer 7.2 and found the attached patch. Now I tried it and  
it seems to solve all the problems. Does that patch look good to you, too?


Regards Andreas


xen-dont-assume-msix-is-masked.patch
Description: Binary data
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

2017-07-14 Thread Jan Beulich 
>>> On 10.07.17 at 18:56,  wrote:
> Hello Jan, Pasi, all
> 
>>> I noticed that PCI passthrough for an LSI SAS HBA 9211 did not longer work 
> (at least under Windows) when using Xen 4.8.1.
>>> I then bisected through various released versions and finally I narrowed it 
> down to
>>> 4.5.5 (with qemu from Xen 4.6.5) -> working
>>> 4.6.0-rc1 (with qemu from Xen 4.6.5) -> no longer working
>> So can you please bisect which exact commit between Xen 4.5 and 4.6 causes 
> the problem?
> 
> Initially I did not bisect through unstable code as I expected to hit broken 
> intermediate snapshots. But finally it worked.
> 
> I found that commit 
> http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=ad28e42bd1d28d746988ed716 
> 54e8aa670629753 is causing the problem (x86/MSI: track host and guest masking 
> separately) which appeared on the mailing list already:
> 
> https://lists.xenproject.org/archives/html/xen-devel/2015-06/msg03923.html 
> https://lists.xenproject.org/archives/html/xen-devel/2016-04/msg00028.html 
> 
> Commit 
> http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=84d6add5593d865736831d150 
> da7c38588f669f6 does not fix it.

So it's possibly a different issue from at least what Sander had seen,
as that change has been tested by him successfully.

> Attached is some first debug info.

What I first of all notice is that there's no interrupt at all which is
bound to either Dom1 or Dom2 in the debug key output. qemu
logs may therefore also be necessary, likely even with verbosity
first increased in the sources (by #define-ing
XEN_PT_LOGGING_ENABLED in xen_pt.h prior to its first use).

It further looks like it's Windows you're having the problem with.
Unless Windows provides you with some kind of diagnostics,
could you check with Linux (allowing us to also see the kernel
log)?

Finally, it would help if you could do all debugging on at least 4.9,
preferably even master. Among other benefits that'll exclude
there potentially being some other change missing from the older
tree.

> Jan, I still have access to the hardware so perhaps we can finally solve 
> this problem.

Feel free to go ahead; I'll be on vacation for the next three weeks.

Jan


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

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

2017-07-09 Thread Pasi Kärkkäinen 
On Fri, Jul 07, 2017 at 06:39:03PM +0200, Andreas Kinzler wrote:
> Hello,
> 
> I noticed that PCI passthrough for an LSI SAS HBA 9211 did not longer work 
> (at least under Windows) when using Xen 4.8.1.
> I then bisected through various released versions and finally I narrowed it 
> down to
> 
> 4.5.5 (with qemu from Xen 4.6.5) -> working
> 4.6.0-rc1 (with qemu from Xen 4.6.5) -> no longer working
>

So can you please bisect which exact commit between Xen 4.5 and 4.6 causes the 
problem? 


> 4.6.4, 4.7.2, 4.8.1 -> no longer working
> 
> dom0 kernel is 4.8.17 but that should not matter.
> 
> PCI passthrough is still working when initializing the controller BIOS in 
> Seabios, but when Windows starts
> it is stuck in an endless loop of spinning dots (see first seconds of 
> https://www.youtube.com/watch?v=3za5fsfYftQ).
> My guess is that is has something to do with PCI enumeration.
> 
> Any ideas? Help?
> 
> Regards Andreas
> 


Thanks,

-- Pasi


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

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

2017-07-07 Thread Andreas Kinzler 

Hello,

I noticed that PCI passthrough for an LSI SAS HBA 9211 did not longer work (at 
least under Windows) when using Xen 4.8.1.
I then bisected through various released versions and finally I narrowed it 
down to

4.5.5 (with qemu from Xen 4.6.5) -> working
4.6.0-rc1 (with qemu from Xen 4.6.5) -> no longer working
4.6.4, 4.7.2, 4.8.1 -> no longer working

dom0 kernel is 4.8.17 but that should not matter.

PCI passthrough is still working when initializing the controller BIOS in 
Seabios, but when Windows starts
it is stuck in an endless loop of spinning dots (see first seconds of 
https://www.youtube.com/watch?v=3za5fsfYftQ).
My guess is that is has something to do with PCI enumeration.

Any ideas? Help?

Regards Andreas

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