Re: [Xen-devel] [PATCH v2] x86/passthrough: fix migration of MSI when using posted interrupts

2019-11-14 Thread Tian, Kevin 


> -Original Message-
> From: Roger Pau Monné [mailto:roger@citrix.com]
> Sent: Friday, November 8, 2019 6:20 PM
> To: Tian, Kevin 
> Cc: Joe Jin ; Jan Beulich ;
> Andrew Cooper ; xen-
> de...@lists.xenproject.org; Juergen Gross ; Wei Liu
> 
> Subject: Re: [Xen-devel] [PATCH v2] x86/passthrough: fix migration of MSI
> when using posted interrupts
> 
> On Fri, Nov 08, 2019 at 02:25:05AM +, Tian, Kevin wrote:
> > > From: Roger Pau Monné [mailto:roger@citrix.com]
> > > Sent: Monday, November 4, 2019 5:47 PM
> > >
> > > On Sat, Nov 02, 2019 at 07:48:06AM +, Tian, Kevin wrote:
> > > > > From: Roger Pau Monné [mailto:roger@citrix.com]
> > > > > Sent: Thursday, October 31, 2019 11:23 PM
> > > > >
> > > > > On Thu, Oct 31, 2019 at 07:52:23AM -0700, Joe Jin wrote:
> > > > > > On 10/31/19 1:01 AM, Jan Beulich wrote:
> > > > > > > On 30.10.2019 19:01, Joe Jin wrote:
> > > > > > >> On 10/30/19 10:23 AM, Roger Pau Monné wrote:
> > > > > > >>> On Wed, Oct 30, 2019 at 09:38:16AM -0700, Joe Jin wrote:
> > > > > > >>>> On 10/30/19 1:24 AM, Roger Pau Monné wrote:
> > > > > > >>>>> Can you try to add the following debug patch on top of the
> > > existing
> > > > > > >>>>> one and report the output that you get on the Xen console?
> > > > > > >>>>
> > > > > > >>>> Applied debug patch and run the test again, not of any log
> > > printed,
> > > > > > >>>> attached Xen log on serial console, seems pi_update_irte()
> not
> > > been
> > > > > > >>>> called for iommu_intpost was false.
> > > > > > >>>
> > > > > > >>> I have to admit I'm lost at this point. Does it mean the 
> > > > > > >>> original
> > > > > > >>> issue had nothing to do with posted interrupts?
> > > > > > >>
> > > > > > >> Looks when inject irq by vlapic_set_irq(), it checked by
> > > > > > >> hvm_funcs.deliver_posted_intr rather than iommu_intpost:
> > > > > > >>
> > > > > > >>  176 if ( hvm_funcs.deliver_posted_intr )
> > > > > > >>  177 hvm_funcs.deliver_posted_intr(target, vec);
> > > > > > >>
> > > > > > >> And deliver_posted_intr() would be there, when vmx enabled:
> > > > > > >>
> > > > > > >> (XEN) HVM: VMX enabled
> > > > > > >> (XEN) HVM: Hardware Assisted Paging (HAP) detected
> > > > > > >> (XEN) HVM: HAP page sizes: 4kB, 2MB, 1GB
> > > > > > >
> > > > > > > I can't see the connection. start_vmx() has
> > > > > > >
> > > > > > > if ( cpu_has_vmx_posted_intr_processing )
> > > > > > > {
> > > > > > > alloc_direct_apic_vector(_intr_vector,
> > > > > pi_notification_interrupt);
> > > > > > > if ( iommu_intpost )
> > > > > > > alloc_direct_apic_vector(_wakeup_vector,
> > > > > pi_wakeup_interrupt);
> > > > > > >
> > > > > > > vmx_function_table.deliver_posted_intr =
> > > vmx_deliver_posted_intr;
> > > > > > > vmx_function_table.sync_pir_to_irr = 
> > > > > > > vmx_sync_pir_to_irr;
> > > > > > > vmx_function_table.test_pir= vmx_test_pir;
> > > > > > > }
> > > > > > >
> > > > > > > i.e. the hook is present only when posted interrupts are
> > > > > > > available in general. I.e. also with just CPU-side posted
> > > > > > > interrupts, yes, which gets confirmed by your "apicv=0"
> > > > > > > test. Yet with just CPU-side posted interrupts I'm
> > > > > > > struggling again to understand your original problem
> > > > > > > description, and the need to fiddle with IOMMU side code.
> > > > > >
> > > > > > Yes, on my test env, cpu_has_vmx_posted_intr_processing == true
> &&
> > > > > iommu_intpost == false,
> > > > > > with this, poste

Re: [Xen-devel] [PATCH v2] x86/passthrough: fix migration of MSI when using posted interrupts

2019-11-08 Thread Roger Pau Monné 
On Fri, Nov 08, 2019 at 02:25:05AM +, Tian, Kevin wrote:
> > From: Roger Pau Monné [mailto:roger@citrix.com]
> > Sent: Monday, November 4, 2019 5:47 PM
> > 
> > On Sat, Nov 02, 2019 at 07:48:06AM +, Tian, Kevin wrote:
> > > > From: Roger Pau Monné [mailto:roger@citrix.com]
> > > > Sent: Thursday, October 31, 2019 11:23 PM
> > > >
> > > > On Thu, Oct 31, 2019 at 07:52:23AM -0700, Joe Jin wrote:
> > > > > On 10/31/19 1:01 AM, Jan Beulich wrote:
> > > > > > On 30.10.2019 19:01, Joe Jin wrote:
> > > > > >> On 10/30/19 10:23 AM, Roger Pau Monné wrote:
> > > > > >>> On Wed, Oct 30, 2019 at 09:38:16AM -0700, Joe Jin wrote:
> > > > >  On 10/30/19 1:24 AM, Roger Pau Monné wrote:
> > > > > > Can you try to add the following debug patch on top of the
> > existing
> > > > > > one and report the output that you get on the Xen console?
> > > > > 
> > > > >  Applied debug patch and run the test again, not of any log
> > printed,
> > > > >  attached Xen log on serial console, seems pi_update_irte() not
> > been
> > > > >  called for iommu_intpost was false.
> > > > > >>>
> > > > > >>> I have to admit I'm lost at this point. Does it mean the original
> > > > > >>> issue had nothing to do with posted interrupts?
> > > > > >>
> > > > > >> Looks when inject irq by vlapic_set_irq(), it checked by
> > > > > >> hvm_funcs.deliver_posted_intr rather than iommu_intpost:
> > > > > >>
> > > > > >>  176 if ( hvm_funcs.deliver_posted_intr )
> > > > > >>  177 hvm_funcs.deliver_posted_intr(target, vec);
> > > > > >>
> > > > > >> And deliver_posted_intr() would be there, when vmx enabled:
> > > > > >>
> > > > > >> (XEN) HVM: VMX enabled
> > > > > >> (XEN) HVM: Hardware Assisted Paging (HAP) detected
> > > > > >> (XEN) HVM: HAP page sizes: 4kB, 2MB, 1GB
> > > > > >
> > > > > > I can't see the connection. start_vmx() has
> > > > > >
> > > > > > if ( cpu_has_vmx_posted_intr_processing )
> > > > > > {
> > > > > > alloc_direct_apic_vector(_intr_vector,
> > > > pi_notification_interrupt);
> > > > > > if ( iommu_intpost )
> > > > > > alloc_direct_apic_vector(_wakeup_vector,
> > > > pi_wakeup_interrupt);
> > > > > >
> > > > > > vmx_function_table.deliver_posted_intr =
> > vmx_deliver_posted_intr;
> > > > > > vmx_function_table.sync_pir_to_irr = 
> > > > > > vmx_sync_pir_to_irr;
> > > > > > vmx_function_table.test_pir= vmx_test_pir;
> > > > > > }
> > > > > >
> > > > > > i.e. the hook is present only when posted interrupts are
> > > > > > available in general. I.e. also with just CPU-side posted
> > > > > > interrupts, yes, which gets confirmed by your "apicv=0"
> > > > > > test. Yet with just CPU-side posted interrupts I'm
> > > > > > struggling again to understand your original problem
> > > > > > description, and the need to fiddle with IOMMU side code.
> > > > >
> > > > > Yes, on my test env, cpu_has_vmx_posted_intr_processing == true &&
> > > > iommu_intpost == false,
> > > > > with this, posted interrupts been enabled.
> > > >
> > > > I'm still quite lost. My reading of the Intel VT-d spec is that the
> > > > posted interrupt descriptor (which contains the PIRR) is used in
> > > > conjunction with a posted interrupt remapping entry in the iommu, so
> > > > that interrupts get recorded in the PIRR and later synced by the
> > > > hypervisor into the vlapic IRR when resuming the virtual CPU.
> > >
> > > there are two parts. Intel first implements CPU posted interrupt,
> > > which allows one CPU to post IPI into non-root context in another
> > > CPU through posted interrupt descriptor. Later VT-d posted
> > > interrupt comes, which use interrupt remapping entry and the
> > > same posted interrupt descriptor (using more fields) to convert
> > > a device interrupt into a posted interrupt. The posting process is
> > > same on the dest CPU, regardless of whether it's from another CPU
> > > or a device.
> > 
> > Thanks for the description.
> > 
> > So the problem reported by Jin happens when using CPU posted
> > interrupts but not VT-d posted interrupts, in which case there
> > shouldn't be a need to sync PIRR with IRR when interrupts from a
> > passthrough device are reconfigured, because interrupts from that
> > device shouldn't end up signaled in PIRR because VT-d posted
> > interrupts is not being used.
> > 
> > Do interrupts from passthrough devices end up signaled in the posted
> > interrupt descriptor PIRR field when not using VT-d posted
> > interrupts but using CPU posted interrupts?
> 
> No. If VT-d posted interrupt is disabled, interrupts from passthrough
> devices don't go through posted interrupt descriptor. But after hypervisor
> serves the interrupt and when it decides to inject a virtual interrupt into
> the guest, PIRR will be updated if CPU posted interrupt is enabled.

Oh, I see. vmx_deliver_posted_intr which is called regardless of
whether VT-d posted interrupts are enabled or not does set the

Re: [Xen-devel] [PATCH v2] x86/passthrough: fix migration of MSI when using posted interrupts

2019-11-07 Thread Tian, Kevin 
> From: Roger Pau Monné [mailto:roger@citrix.com]
> Sent: Monday, November 4, 2019 5:47 PM
> 
> On Sat, Nov 02, 2019 at 07:48:06AM +, Tian, Kevin wrote:
> > > From: Roger Pau Monné [mailto:roger@citrix.com]
> > > Sent: Thursday, October 31, 2019 11:23 PM
> > >
> > > On Thu, Oct 31, 2019 at 07:52:23AM -0700, Joe Jin wrote:
> > > > On 10/31/19 1:01 AM, Jan Beulich wrote:
> > > > > On 30.10.2019 19:01, Joe Jin wrote:
> > > > >> On 10/30/19 10:23 AM, Roger Pau Monné wrote:
> > > > >>> On Wed, Oct 30, 2019 at 09:38:16AM -0700, Joe Jin wrote:
> > > >  On 10/30/19 1:24 AM, Roger Pau Monné wrote:
> > > > > Can you try to add the following debug patch on top of the
> existing
> > > > > one and report the output that you get on the Xen console?
> > > > 
> > > >  Applied debug patch and run the test again, not of any log
> printed,
> > > >  attached Xen log on serial console, seems pi_update_irte() not
> been
> > > >  called for iommu_intpost was false.
> > > > >>>
> > > > >>> I have to admit I'm lost at this point. Does it mean the original
> > > > >>> issue had nothing to do with posted interrupts?
> > > > >>
> > > > >> Looks when inject irq by vlapic_set_irq(), it checked by
> > > > >> hvm_funcs.deliver_posted_intr rather than iommu_intpost:
> > > > >>
> > > > >>  176 if ( hvm_funcs.deliver_posted_intr )
> > > > >>  177 hvm_funcs.deliver_posted_intr(target, vec);
> > > > >>
> > > > >> And deliver_posted_intr() would be there, when vmx enabled:
> > > > >>
> > > > >> (XEN) HVM: VMX enabled
> > > > >> (XEN) HVM: Hardware Assisted Paging (HAP) detected
> > > > >> (XEN) HVM: HAP page sizes: 4kB, 2MB, 1GB
> > > > >
> > > > > I can't see the connection. start_vmx() has
> > > > >
> > > > > if ( cpu_has_vmx_posted_intr_processing )
> > > > > {
> > > > > alloc_direct_apic_vector(_intr_vector,
> > > pi_notification_interrupt);
> > > > > if ( iommu_intpost )
> > > > > alloc_direct_apic_vector(_wakeup_vector,
> > > pi_wakeup_interrupt);
> > > > >
> > > > > vmx_function_table.deliver_posted_intr =
> vmx_deliver_posted_intr;
> > > > > vmx_function_table.sync_pir_to_irr = vmx_sync_pir_to_irr;
> > > > > vmx_function_table.test_pir= vmx_test_pir;
> > > > > }
> > > > >
> > > > > i.e. the hook is present only when posted interrupts are
> > > > > available in general. I.e. also with just CPU-side posted
> > > > > interrupts, yes, which gets confirmed by your "apicv=0"
> > > > > test. Yet with just CPU-side posted interrupts I'm
> > > > > struggling again to understand your original problem
> > > > > description, and the need to fiddle with IOMMU side code.
> > > >
> > > > Yes, on my test env, cpu_has_vmx_posted_intr_processing == true &&
> > > iommu_intpost == false,
> > > > with this, posted interrupts been enabled.
> > >
> > > I'm still quite lost. My reading of the Intel VT-d spec is that the
> > > posted interrupt descriptor (which contains the PIRR) is used in
> > > conjunction with a posted interrupt remapping entry in the iommu, so
> > > that interrupts get recorded in the PIRR and later synced by the
> > > hypervisor into the vlapic IRR when resuming the virtual CPU.
> >
> > there are two parts. Intel first implements CPU posted interrupt,
> > which allows one CPU to post IPI into non-root context in another
> > CPU through posted interrupt descriptor. Later VT-d posted
> > interrupt comes, which use interrupt remapping entry and the
> > same posted interrupt descriptor (using more fields) to convert
> > a device interrupt into a posted interrupt. The posting process is
> > same on the dest CPU, regardless of whether it's from another CPU
> > or a device.
> 
> Thanks for the description.
> 
> So the problem reported by Jin happens when using CPU posted
> interrupts but not VT-d posted interrupts, in which case there
> shouldn't be a need to sync PIRR with IRR when interrupts from a
> passthrough device are reconfigured, because interrupts from that
> device shouldn't end up signaled in PIRR because VT-d posted
> interrupts is not being used.
> 
> Do interrupts from passthrough devices end up signaled in the posted
> interrupt descriptor PIRR field when not using VT-d posted
> interrupts but using CPU posted interrupts?

No. If VT-d posted interrupt is disabled, interrupts from passthrough
devices don't go through posted interrupt descriptor. But after hypervisor
serves the interrupt and when it decides to inject a virtual interrupt into
the guest, PIRR will be updated if CPU posted interrupt is enabled.

> 
> From my reading of your description above when using CPU posted
> interrupts only the vectors signaled in the PIRR field should belong
> to IPIs from other vCPUs?
> 

I didn't understand your question.

Thanks
Kevin

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

Re: [Xen-devel] [PATCH v2] x86/passthrough: fix migration of MSI when using posted interrupts

2019-11-04 Thread Roger Pau Monné 
On Sat, Nov 02, 2019 at 07:48:06AM +, Tian, Kevin wrote:
> > From: Roger Pau Monné [mailto:roger@citrix.com]
> > Sent: Thursday, October 31, 2019 11:23 PM
> > 
> > On Thu, Oct 31, 2019 at 07:52:23AM -0700, Joe Jin wrote:
> > > On 10/31/19 1:01 AM, Jan Beulich wrote:
> > > > On 30.10.2019 19:01, Joe Jin wrote:
> > > >> On 10/30/19 10:23 AM, Roger Pau Monné wrote:
> > > >>> On Wed, Oct 30, 2019 at 09:38:16AM -0700, Joe Jin wrote:
> > >  On 10/30/19 1:24 AM, Roger Pau Monné wrote:
> > > > Can you try to add the following debug patch on top of the existing
> > > > one and report the output that you get on the Xen console?
> > > 
> > >  Applied debug patch and run the test again, not of any log printed,
> > >  attached Xen log on serial console, seems pi_update_irte() not been
> > >  called for iommu_intpost was false.
> > > >>>
> > > >>> I have to admit I'm lost at this point. Does it mean the original
> > > >>> issue had nothing to do with posted interrupts?
> > > >>
> > > >> Looks when inject irq by vlapic_set_irq(), it checked by
> > > >> hvm_funcs.deliver_posted_intr rather than iommu_intpost:
> > > >>
> > > >>  176 if ( hvm_funcs.deliver_posted_intr )
> > > >>  177 hvm_funcs.deliver_posted_intr(target, vec);
> > > >>
> > > >> And deliver_posted_intr() would be there, when vmx enabled:
> > > >>
> > > >> (XEN) HVM: VMX enabled
> > > >> (XEN) HVM: Hardware Assisted Paging (HAP) detected
> > > >> (XEN) HVM: HAP page sizes: 4kB, 2MB, 1GB
> > > >
> > > > I can't see the connection. start_vmx() has
> > > >
> > > > if ( cpu_has_vmx_posted_intr_processing )
> > > > {
> > > > alloc_direct_apic_vector(_intr_vector,
> > pi_notification_interrupt);
> > > > if ( iommu_intpost )
> > > > alloc_direct_apic_vector(_wakeup_vector,
> > pi_wakeup_interrupt);
> > > >
> > > > vmx_function_table.deliver_posted_intr = 
> > > > vmx_deliver_posted_intr;
> > > > vmx_function_table.sync_pir_to_irr = vmx_sync_pir_to_irr;
> > > > vmx_function_table.test_pir= vmx_test_pir;
> > > > }
> > > >
> > > > i.e. the hook is present only when posted interrupts are
> > > > available in general. I.e. also with just CPU-side posted
> > > > interrupts, yes, which gets confirmed by your "apicv=0"
> > > > test. Yet with just CPU-side posted interrupts I'm
> > > > struggling again to understand your original problem
> > > > description, and the need to fiddle with IOMMU side code.
> > >
> > > Yes, on my test env, cpu_has_vmx_posted_intr_processing == true &&
> > iommu_intpost == false,
> > > with this, posted interrupts been enabled.
> > 
> > I'm still quite lost. My reading of the Intel VT-d spec is that the
> > posted interrupt descriptor (which contains the PIRR) is used in
> > conjunction with a posted interrupt remapping entry in the iommu, so
> > that interrupts get recorded in the PIRR and later synced by the
> > hypervisor into the vlapic IRR when resuming the virtual CPU.
> 
> there are two parts. Intel first implements CPU posted interrupt,
> which allows one CPU to post IPI into non-root context in another
> CPU through posted interrupt descriptor. Later VT-d posted 
> interrupt comes, which use interrupt remapping entry and the
> same posted interrupt descriptor (using more fields) to convert
> a device interrupt into a posted interrupt. The posting process is
> same on the dest CPU, regardless of whether it's from another CPU
> or a device. 

Thanks for the description.

So the problem reported by Jin happens when using CPU posted
interrupts but not VT-d posted interrupts, in which case there
shouldn't be a need to sync PIRR with IRR when interrupts from a
passthrough device are reconfigured, because interrupts from that
device shouldn't end up signaled in PIRR because VT-d posted
interrupts is not being used.

Do interrupts from passthrough devices end up signaled in the posted
interrupt descriptor PIRR field when not using VT-d posted
interrupts but using CPU posted interrupts?

From my reading of your description above when using CPU posted
interrupts only the vectors signaled in the PIRR field should belong
to IPIs from other vCPUs?

Thanks, Roger.

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

Re: [Xen-devel] [PATCH v2] x86/passthrough: fix migration of MSI when using posted interrupts

2019-11-02 Thread Tian, Kevin 
> From: Roger Pau Monné [mailto:roger@citrix.com]
> Sent: Thursday, October 31, 2019 11:23 PM
> 
> On Thu, Oct 31, 2019 at 07:52:23AM -0700, Joe Jin wrote:
> > On 10/31/19 1:01 AM, Jan Beulich wrote:
> > > On 30.10.2019 19:01, Joe Jin wrote:
> > >> On 10/30/19 10:23 AM, Roger Pau Monné wrote:
> > >>> On Wed, Oct 30, 2019 at 09:38:16AM -0700, Joe Jin wrote:
> >  On 10/30/19 1:24 AM, Roger Pau Monné wrote:
> > > Can you try to add the following debug patch on top of the existing
> > > one and report the output that you get on the Xen console?
> > 
> >  Applied debug patch and run the test again, not of any log printed,
> >  attached Xen log on serial console, seems pi_update_irte() not been
> >  called for iommu_intpost was false.
> > >>>
> > >>> I have to admit I'm lost at this point. Does it mean the original
> > >>> issue had nothing to do with posted interrupts?
> > >>
> > >> Looks when inject irq by vlapic_set_irq(), it checked by
> > >> hvm_funcs.deliver_posted_intr rather than iommu_intpost:
> > >>
> > >>  176 if ( hvm_funcs.deliver_posted_intr )
> > >>  177 hvm_funcs.deliver_posted_intr(target, vec);
> > >>
> > >> And deliver_posted_intr() would be there, when vmx enabled:
> > >>
> > >> (XEN) HVM: VMX enabled
> > >> (XEN) HVM: Hardware Assisted Paging (HAP) detected
> > >> (XEN) HVM: HAP page sizes: 4kB, 2MB, 1GB
> > >
> > > I can't see the connection. start_vmx() has
> > >
> > > if ( cpu_has_vmx_posted_intr_processing )
> > > {
> > > alloc_direct_apic_vector(_intr_vector,
> pi_notification_interrupt);
> > > if ( iommu_intpost )
> > > alloc_direct_apic_vector(_wakeup_vector,
> pi_wakeup_interrupt);
> > >
> > > vmx_function_table.deliver_posted_intr = vmx_deliver_posted_intr;
> > > vmx_function_table.sync_pir_to_irr = vmx_sync_pir_to_irr;
> > > vmx_function_table.test_pir= vmx_test_pir;
> > > }
> > >
> > > i.e. the hook is present only when posted interrupts are
> > > available in general. I.e. also with just CPU-side posted
> > > interrupts, yes, which gets confirmed by your "apicv=0"
> > > test. Yet with just CPU-side posted interrupts I'm
> > > struggling again to understand your original problem
> > > description, and the need to fiddle with IOMMU side code.
> >
> > Yes, on my test env, cpu_has_vmx_posted_intr_processing == true &&
> iommu_intpost == false,
> > with this, posted interrupts been enabled.
> 
> I'm still quite lost. My reading of the Intel VT-d spec is that the
> posted interrupt descriptor (which contains the PIRR) is used in
> conjunction with a posted interrupt remapping entry in the iommu, so
> that interrupts get recorded in the PIRR and later synced by the
> hypervisor into the vlapic IRR when resuming the virtual CPU.

there are two parts. Intel first implements CPU posted interrupt,
which allows one CPU to post IPI into non-root context in another
CPU through posted interrupt descriptor. Later VT-d posted 
interrupt comes, which use interrupt remapping entry and the
same posted interrupt descriptor (using more fields) to convert
a device interrupt into a posted interrupt. The posting process is
same on the dest CPU, regardless of whether it's from another CPU
or a device. 

> 
> How is the PIRR filled if there's no interrupt remapping entry
> pointing to it?
> 
> I have to admit I'm not super-familiar with the implementation in Xen,
> so it's likely I'm missing something.
> 
> Roger.

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

Re: [Xen-devel] [PATCH v2] x86/passthrough: fix migration of MSI when using posted interrupts

2019-10-31 Thread Roger Pau Monné 
On Thu, Oct 31, 2019 at 07:52:23AM -0700, Joe Jin wrote:
> On 10/31/19 1:01 AM, Jan Beulich wrote:
> > On 30.10.2019 19:01, Joe Jin wrote:
> >> On 10/30/19 10:23 AM, Roger Pau Monné wrote:
> >>> On Wed, Oct 30, 2019 at 09:38:16AM -0700, Joe Jin wrote:
>  On 10/30/19 1:24 AM, Roger Pau Monné wrote:
> > Can you try to add the following debug patch on top of the existing
> > one and report the output that you get on the Xen console?
> 
>  Applied debug patch and run the test again, not of any log printed,
>  attached Xen log on serial console, seems pi_update_irte() not been
>  called for iommu_intpost was false.
> >>>
> >>> I have to admit I'm lost at this point. Does it mean the original
> >>> issue had nothing to do with posted interrupts?
> >>
> >> Looks when inject irq by vlapic_set_irq(), it checked by
> >> hvm_funcs.deliver_posted_intr rather than iommu_intpost:
> >>
> >>  176 if ( hvm_funcs.deliver_posted_intr )
> >>  177 hvm_funcs.deliver_posted_intr(target, vec);
> >>
> >> And deliver_posted_intr() would be there, when vmx enabled:
> >>
> >> (XEN) HVM: VMX enabled
> >> (XEN) HVM: Hardware Assisted Paging (HAP) detected
> >> (XEN) HVM: HAP page sizes: 4kB, 2MB, 1GB
> > 
> > I can't see the connection. start_vmx() has
> > 
> > if ( cpu_has_vmx_posted_intr_processing )
> > {
> > alloc_direct_apic_vector(_intr_vector, 
> > pi_notification_interrupt);
> > if ( iommu_intpost )
> > alloc_direct_apic_vector(_wakeup_vector, 
> > pi_wakeup_interrupt);
> > 
> > vmx_function_table.deliver_posted_intr = vmx_deliver_posted_intr;
> > vmx_function_table.sync_pir_to_irr = vmx_sync_pir_to_irr;
> > vmx_function_table.test_pir= vmx_test_pir;
> > }
> > 
> > i.e. the hook is present only when posted interrupts are
> > available in general. I.e. also with just CPU-side posted
> > interrupts, yes, which gets confirmed by your "apicv=0"
> > test. Yet with just CPU-side posted interrupts I'm
> > struggling again to understand your original problem
> > description, and the need to fiddle with IOMMU side code.
> 
> Yes, on my test env, cpu_has_vmx_posted_intr_processing == true && 
> iommu_intpost == false,
> with this, posted interrupts been enabled.

I'm still quite lost. My reading of the Intel VT-d spec is that the
posted interrupt descriptor (which contains the PIRR) is used in
conjunction with a posted interrupt remapping entry in the iommu, so
that interrupts get recorded in the PIRR and later synced by the
hypervisor into the vlapic IRR when resuming the virtual CPU.

How is the PIRR filled if there's no interrupt remapping entry
pointing to it?

I have to admit I'm not super-familiar with the implementation in Xen,
so it's likely I'm missing something.

Roger.

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

Re: [Xen-devel] [PATCH v2] x86/passthrough: fix migration of MSI when using posted interrupts

2019-10-31 Thread Joe Jin 
On 10/31/19 7:56 AM, Jan Beulich wrote:
> On 31.10.2019 15:52, Joe Jin wrote:
>> On 10/31/19 1:01 AM, Jan Beulich wrote:
>>> On 30.10.2019 19:01, Joe Jin wrote:
 On 10/30/19 10:23 AM, Roger Pau Monné wrote:
> On Wed, Oct 30, 2019 at 09:38:16AM -0700, Joe Jin wrote:
>> On 10/30/19 1:24 AM, Roger Pau Monné wrote:
>>> Can you try to add the following debug patch on top of the existing
>>> one and report the output that you get on the Xen console?
>>
>> Applied debug patch and run the test again, not of any log printed,
>> attached Xen log on serial console, seems pi_update_irte() not been
>> called for iommu_intpost was false.
>
> I have to admit I'm lost at this point. Does it mean the original
> issue had nothing to do with posted interrupts?

 Looks when inject irq by vlapic_set_irq(), it checked by
 hvm_funcs.deliver_posted_intr rather than iommu_intpost:

  176 if ( hvm_funcs.deliver_posted_intr )
  177 hvm_funcs.deliver_posted_intr(target, vec);

 And deliver_posted_intr() would be there, when vmx enabled:

 (XEN) HVM: VMX enabled
 (XEN) HVM: Hardware Assisted Paging (HAP) detected
 (XEN) HVM: HAP page sizes: 4kB, 2MB, 1GB
>>>
>>> I can't see the connection. start_vmx() has
>>>
>>> if ( cpu_has_vmx_posted_intr_processing )
>>> {
>>> alloc_direct_apic_vector(_intr_vector, 
>>> pi_notification_interrupt);
>>> if ( iommu_intpost )
>>> alloc_direct_apic_vector(_wakeup_vector, 
>>> pi_wakeup_interrupt);
>>>
>>> vmx_function_table.deliver_posted_intr = vmx_deliver_posted_intr;
>>> vmx_function_table.sync_pir_to_irr = vmx_sync_pir_to_irr;
>>> vmx_function_table.test_pir= vmx_test_pir;
>>> }
>>>
>>> i.e. the hook is present only when posted interrupts are
>>> available in general. I.e. also with just CPU-side posted
>>> interrupts, yes, which gets confirmed by your "apicv=0"
>>> test. Yet with just CPU-side posted interrupts I'm
>>> struggling again to understand your original problem
>>> description, and the need to fiddle with IOMMU side code.
>>
>> Yes, on my test env, cpu_has_vmx_posted_intr_processing == true && 
>> iommu_intpost == false,
>> with this, posted interrupts been enabled.
> 
> And what's the theory then again for needing to modify IOMMU
> code in this case?

Idea is when dev msix vector been updated, we need to let vcpu know to avoid
to lost interrupt. Not sure we can do this or other path?

Thanks,
Joe



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

Re: [Xen-devel] [PATCH v2] x86/passthrough: fix migration of MSI when using posted interrupts

2019-10-31 Thread Jan Beulich 
On 31.10.2019 15:52, Joe Jin wrote:
> On 10/31/19 1:01 AM, Jan Beulich wrote:
>> On 30.10.2019 19:01, Joe Jin wrote:
>>> On 10/30/19 10:23 AM, Roger Pau Monné wrote:
 On Wed, Oct 30, 2019 at 09:38:16AM -0700, Joe Jin wrote:
> On 10/30/19 1:24 AM, Roger Pau Monné wrote:
>> Can you try to add the following debug patch on top of the existing
>> one and report the output that you get on the Xen console?
>
> Applied debug patch and run the test again, not of any log printed,
> attached Xen log on serial console, seems pi_update_irte() not been
> called for iommu_intpost was false.

 I have to admit I'm lost at this point. Does it mean the original
 issue had nothing to do with posted interrupts?
>>>
>>> Looks when inject irq by vlapic_set_irq(), it checked by
>>> hvm_funcs.deliver_posted_intr rather than iommu_intpost:
>>>
>>>  176 if ( hvm_funcs.deliver_posted_intr )
>>>  177 hvm_funcs.deliver_posted_intr(target, vec);
>>>
>>> And deliver_posted_intr() would be there, when vmx enabled:
>>>
>>> (XEN) HVM: VMX enabled
>>> (XEN) HVM: Hardware Assisted Paging (HAP) detected
>>> (XEN) HVM: HAP page sizes: 4kB, 2MB, 1GB
>>
>> I can't see the connection. start_vmx() has
>>
>> if ( cpu_has_vmx_posted_intr_processing )
>> {
>> alloc_direct_apic_vector(_intr_vector, 
>> pi_notification_interrupt);
>> if ( iommu_intpost )
>> alloc_direct_apic_vector(_wakeup_vector, pi_wakeup_interrupt);
>>
>> vmx_function_table.deliver_posted_intr = vmx_deliver_posted_intr;
>> vmx_function_table.sync_pir_to_irr = vmx_sync_pir_to_irr;
>> vmx_function_table.test_pir= vmx_test_pir;
>> }
>>
>> i.e. the hook is present only when posted interrupts are
>> available in general. I.e. also with just CPU-side posted
>> interrupts, yes, which gets confirmed by your "apicv=0"
>> test. Yet with just CPU-side posted interrupts I'm
>> struggling again to understand your original problem
>> description, and the need to fiddle with IOMMU side code.
> 
> Yes, on my test env, cpu_has_vmx_posted_intr_processing == true && 
> iommu_intpost == false,
> with this, posted interrupts been enabled.

And what's the theory then again for needing to modify IOMMU
code in this case?

Jan

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

Re: [Xen-devel] [PATCH v2] x86/passthrough: fix migration of MSI when using posted interrupts

2019-10-31 Thread Joe Jin 
On 10/31/19 1:01 AM, Jan Beulich wrote:
> On 30.10.2019 19:01, Joe Jin wrote:
>> On 10/30/19 10:23 AM, Roger Pau Monné wrote:
>>> On Wed, Oct 30, 2019 at 09:38:16AM -0700, Joe Jin wrote:
 On 10/30/19 1:24 AM, Roger Pau Monné wrote:
> Can you try to add the following debug patch on top of the existing
> one and report the output that you get on the Xen console?

 Applied debug patch and run the test again, not of any log printed,
 attached Xen log on serial console, seems pi_update_irte() not been
 called for iommu_intpost was false.
>>>
>>> I have to admit I'm lost at this point. Does it mean the original
>>> issue had nothing to do with posted interrupts?
>>
>> Looks when inject irq by vlapic_set_irq(), it checked by
>> hvm_funcs.deliver_posted_intr rather than iommu_intpost:
>>
>>  176 if ( hvm_funcs.deliver_posted_intr )
>>  177 hvm_funcs.deliver_posted_intr(target, vec);
>>
>> And deliver_posted_intr() would be there, when vmx enabled:
>>
>> (XEN) HVM: VMX enabled
>> (XEN) HVM: Hardware Assisted Paging (HAP) detected
>> (XEN) HVM: HAP page sizes: 4kB, 2MB, 1GB
> 
> I can't see the connection. start_vmx() has
> 
> if ( cpu_has_vmx_posted_intr_processing )
> {
> alloc_direct_apic_vector(_intr_vector, 
> pi_notification_interrupt);
> if ( iommu_intpost )
> alloc_direct_apic_vector(_wakeup_vector, pi_wakeup_interrupt);
> 
> vmx_function_table.deliver_posted_intr = vmx_deliver_posted_intr;
> vmx_function_table.sync_pir_to_irr = vmx_sync_pir_to_irr;
> vmx_function_table.test_pir= vmx_test_pir;
> }
> 
> i.e. the hook is present only when posted interrupts are
> available in general. I.e. also with just CPU-side posted
> interrupts, yes, which gets confirmed by your "apicv=0"
> test. Yet with just CPU-side posted interrupts I'm
> struggling again to understand your original problem
> description, and the need to fiddle with IOMMU side code.

Yes, on my test env, cpu_has_vmx_posted_intr_processing == true && 
iommu_intpost == false,
with this, posted interrupts been enabled.

Thanks,
Joe

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

Re: [Xen-devel] [PATCH v2] x86/passthrough: fix migration of MSI when using posted interrupts

2019-10-31 Thread Jan Beulich 
On 30.10.2019 19:01, Joe Jin wrote:
> On 10/30/19 10:23 AM, Roger Pau Monné wrote:
>> On Wed, Oct 30, 2019 at 09:38:16AM -0700, Joe Jin wrote:
>>> On 10/30/19 1:24 AM, Roger Pau Monné wrote:
 Can you try to add the following debug patch on top of the existing
 one and report the output that you get on the Xen console?
>>>
>>> Applied debug patch and run the test again, not of any log printed,
>>> attached Xen log on serial console, seems pi_update_irte() not been
>>> called for iommu_intpost was false.
>>
>> I have to admit I'm lost at this point. Does it mean the original
>> issue had nothing to do with posted interrupts?
> 
> Looks when inject irq by vlapic_set_irq(), it checked by
> hvm_funcs.deliver_posted_intr rather than iommu_intpost:
> 
>  176 if ( hvm_funcs.deliver_posted_intr )
>  177 hvm_funcs.deliver_posted_intr(target, vec);
> 
> And deliver_posted_intr() would be there, when vmx enabled:
> 
> (XEN) HVM: VMX enabled
> (XEN) HVM: Hardware Assisted Paging (HAP) detected
> (XEN) HVM: HAP page sizes: 4kB, 2MB, 1GB

I can't see the connection. start_vmx() has

if ( cpu_has_vmx_posted_intr_processing )
{
alloc_direct_apic_vector(_intr_vector, 
pi_notification_interrupt);
if ( iommu_intpost )
alloc_direct_apic_vector(_wakeup_vector, pi_wakeup_interrupt);

vmx_function_table.deliver_posted_intr = vmx_deliver_posted_intr;
vmx_function_table.sync_pir_to_irr = vmx_sync_pir_to_irr;
vmx_function_table.test_pir= vmx_test_pir;
}

i.e. the hook is present only when posted interrupts are
available in general. I.e. also with just CPU-side posted
interrupts, yes, which gets confirmed by your "apicv=0"
test. Yet with just CPU-side posted interrupts I'm
struggling again to understand your original problem
description, and the need to fiddle with IOMMU side code.

Jan

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

Re: [Xen-devel] [PATCH v2] x86/passthrough: fix migration of MSI when using posted interrupts

2019-10-30 Thread Joe Jin 
On 10/30/19 10:23 AM, Roger Pau Monné wrote:
> On Wed, Oct 30, 2019 at 09:38:16AM -0700, Joe Jin wrote:
>> On 10/30/19 1:24 AM, Roger Pau Monné wrote:
>>> Can you try to add the following debug patch on top of the existing
>>> one and report the output that you get on the Xen console?
>>
>> Applied debug patch and run the test again, not of any log printed,
>> attached Xen log on serial console, seems pi_update_irte() not been
>> called for iommu_intpost was false.
> 
> I have to admit I'm lost at this point. Does it mean the original
> issue had nothing to do with posted interrupts?

Looks when inject irq by vlapic_set_irq(), it checked by
hvm_funcs.deliver_posted_intr rather than iommu_intpost:

 176 if ( hvm_funcs.deliver_posted_intr )
 177 hvm_funcs.deliver_posted_intr(target, vec);

And deliver_posted_intr() would be there, when vmx enabled:

(XEN) HVM: VMX enabled
(XEN) HVM: Hardware Assisted Paging (HAP) detected
(XEN) HVM: HAP page sizes: 4kB, 2MB, 1GB

So original issue still used posted interrupts?

> 
> Where you booting using iommu=intpost in your previous tests? Note
> that posted interrupts is not enabled by default according to the
> command line documentation.
> 

No, from xen command line you can see I only had iommu=1 for xen.

> Can you confirm whether you also see weird behavior without using
> posted interrupts, and can you turn posted interrupts on and give the
> patch a try?

I disabled apicv, looks posted interrupts been disables as well, then
I could not reproduced it anymore:

(XEN) Command line: placeholder dom0_mem=max:3456M allowsuperpage 
dom0_vcpus_pin=numa dom0_max_vcpus=4 crashkernel=512M@1024M 
iommu=verbose,debug,force,intremap,intpost hvm_debug=832 guest_loglvl=all 
com1=115200,8n1 console=com1 conring_size=1m console_to_ring apicv=0
...
(XEN) Initing memory sharing.
(XEN) Intel VT-d iommu 0 supported page sizes: 4kB, 2MB, 1GB.
(XEN) Intel VT-d iommu 1 supported page sizes: 4kB, 2MB, 1GB.
(XEN) Intel VT-d Snoop Control enabled.
(XEN) Intel VT-d Dom0 DMA Passthrough not enabled.
(XEN) Intel VT-d Queued Invalidation enabled.
(XEN) Intel VT-d Interrupt Remapping enabled.
(XEN) Intel VT-d Posted Interrupt not enabled.
(XEN) Intel VT-d Shared EPT tables enabled.
...
(XEN) VMX: Supported advanced features:
(XEN)  - APIC MMIO access virtualisation
(XEN)  - APIC TPR shadow
(XEN)  - Extended Page Tables (EPT)
(XEN)  - Virtual-Processor Identifiers (VPID)
(XEN)  - Virtual NMI
(XEN)  - MSR direct-access bitmap
(XEN)  - Unrestricted Guest
(XEN)  - VMCS shadowing
(XEN)  - VM Functions
(XEN) HVM: ASIDs enabled.
(XEN) HVM: VMX enabled

Thanks,
Joe


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

Re: [Xen-devel] [PATCH v2] x86/passthrough: fix migration of MSI when using posted interrupts

2019-10-30 Thread Roger Pau Monné 
On Wed, Oct 30, 2019 at 09:38:16AM -0700, Joe Jin wrote:
> On 10/30/19 1:24 AM, Roger Pau Monné wrote:
> > Can you try to add the following debug patch on top of the existing
> > one and report the output that you get on the Xen console?
> 
> Applied debug patch and run the test again, not of any log printed,
> attached Xen log on serial console, seems pi_update_irte() not been
> called for iommu_intpost was false.

I have to admit I'm lost at this point. Does it mean the original
issue had nothing to do with posted interrupts?

Where you booting using iommu=intpost in your previous tests? Note
that posted interrupts is not enabled by default according to the
command line documentation.

Can you confirm whether you also see weird behavior without using
posted interrupts, and can you turn posted interrupts on and give the
patch a try?

Thanks, Roger.

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

Re: [Xen-devel] [PATCH v2] x86/passthrough: fix migration of MSI when using posted interrupts

2019-10-30 Thread Joe Jin 
On 10/30/19 1:24 AM, Roger Pau Monné wrote:
> Can you try to add the following debug patch on top of the existing
> one and report the output that you get on the Xen console?

Applied debug patch and run the test again, not of any log printed,
attached Xen log on serial console, seems pi_update_irte() not been
called for iommu_intpost was false.

Thanks,
Joe
(XEN) Xen version 4.12.2-pre (root@) (gcc (GCC) 4.4.7 20120313 (Red Hat 
4.4.7-23.0.1)) debug=n  Tue Oct 29 02:43:40 PDT 2019
(XEN) Latest ChangeSet: 
(XEN) Bootloader: GRUB 2.02~beta2   
(XEN) Command line: placeholder dom0_mem=max:3456M allowsuperpage 
dom0_vcpus_pin=numa dom0_max_vcpus=4 crashkernel=512M@1024M iommu=1 
hvm_debug=832 guest_loglvl=all com1=115200,8n1 console=com1 conring_size=1m 
console_to_ring 
(XEN) Xen image load base address: 0x7720   
(XEN) Video information:
(XEN)  VGA is text mode 80x25, font 8x16
(XEN)  VBE/DDC methods: none; EDID transfer time: 0 seconds 
(XEN)  EDID info not retrieved because no DDC retrieval method detected 
(XEN) Disc information: 
(XEN)  Found 2 MBR signatures   
(XEN)  Found 2 EDD information structures   
(XEN) Xen-e820 RAM map: 
(XEN)   - 0009b000 (usable) 
(XEN)  0009b000 - 000a (reserved)   
(XEN)  000e - 0010 (reserved)   
(XEN)  0010 - 77928000 (usable) 
(XEN)  77928000 - 79356000 (reserved)  
(XEN)  79356000 - 79391000 (ACPI data)
(XEN)  79391000 - 7990 (ACPI NVS)
(XEN)  7990 - 7bd4d000 (reserved)
(XEN)  7bd4d000 - 7bd58000 (usable)
(XEN)  7bd58000 - 7bd59000 (reserved)
(XEN)  7bd59000 - 7bd5c000 (usable)
(XEN)  7bd5c000 - 7bd5d000 (reserved)
(XEN)  7bd5d000 - 7bd5e000 (usable)
(XEN)  7bd5e000 - 7bde4000 (reserved)
(XEN)  7bde4000 - 7c00 (usable)
(XEN)  8000 - 9000 (reserved)
(XEN)  fed1c000 - fed2 (reserved)
(XEN)  ff00 - 0001 (reserved)
(XEN)  0001 - 00208000 (usable)
(XEN) Kdump: 512MB (524288kB) at 0x4000
(XEN) ACPI: RSDP 000F0530, 0024 (r2 ORACLE)
(XEN) ACPI: XSDT 7936C0B0, 00E4 (r1 ORACLE X5-2 30130200 AMI 10013)
(XEN) ACPI: FACP 7937F608, 010C (r5 ORACLE X5-2 30130200 AMI 10013)
(XEN) ACPI: DSDT 7936C228, 133DE (r2 ORACLE X5-2 30130200 INTL 20091013)
(XEN) ACPI: FACS 798FDF80, 0040
(XEN) ACPI: APIC 7937F718, 0224 (r3 ORACLE X5-2 30130200 AMI 10013)
(XEN) ACPI: FPDT 7937F940, 0044 (r1 ORACLE X5-2 30130200 AMI 10013)
(XEN) ACPI: FIDT 7937F988, 009C (r1 ORACLE X5-2 30130200 AMI 10013)
(XEN) ACPI: SPMI 7937FA28, 0041 (r5 ORACLE X5-2 30130200 AMI.0)
(XEN) ACPI: MCFG 7937FA70, 003C (r1 ORACLE X5-2 30130200 MSFT   97)
(XEN) ACPI: OEMS 7937FAB0, 07DC (r1 ORACLE X5-2 30130200 ORCL1)
(XEN) ACPI: UEFI 79380290, 0042 (r1 ORACLE X5-2 30130200 0)
(XEN) ACPI: BDAT 793802D8, 0030 (r1 ORACLE X5-2 30130200 INTL 20091013)
(XEN) ACPI: HPET 79380308, 0038 (r1 ORACLE X5-2 30130200 INTL 20091013)
(XEN) ACPI: MSCT 79380340, 0090 (r1 ORACLE X5-2 30130200 INTL 20091013)
(XEN) ACPI: PCCT 793803D0, 006E (r1 ORACLE X5-2 30130200 INTL 20091013)
(XEN) ACPI: PMCT 79380440, 0064 (r1 ORACLE X5-2 30130200 INTL 20091013)
(XEN) ACPI: PMTT 793804A8, 0268 (r1 ORACLE X5-2 30130200 INTL 20091013)
(XEN) ACPI: SLIT 79380710, 0030 (r1 ORACLE X5-2 30130200 INTL 20091013)
(XEN) ACPI: SRAT 79380740, 0E58 (r3 ORACLE X5-2 30130200 INTL 20091013)
(XEN) ACPI: WDDT 79381598, 0040 (r1 ORACLE X5-2 30130200 INTL 20091013)
(XEN) ACPI: SSDT 793815D8, ED2F (r1 ORACLEPmMgt 30130200 INTL 20120913)
(XEN) ACPI: OEMP 79390308, 0158 (r1 ORACLE X5-2 30130200 ORCL1)
(XEN) ACPI: DMAR 79390460, 0148 (r1 ORACLE X5-2 30130200 INTL 20091013)
(XEN) ACPI: HEST 793905A8, 013C (r1 ORACLE X5-2 30130200 INTL1)
(XEN) ACPI: BERT 793906E8, 0030 (r1 ORACLE X5-2 30130200 INTL1)
(XEN) ACPI: ERST 79390718, 0230 (r1 ORACLE X5-2 30130200 INTL1)
(XEN) ACPI: EINJ 79390948, 0130 (r1 ORACLE X5-2 30130200 INTL1)
(XEN) System RAM: 130938MB (134081464kB)
(XEN) Domain heap initialised DMA width 32 bits
(XEN) Allocated

Re: [Xen-devel] [PATCH v2] x86/passthrough: fix migration of MSI when using posted interrupts

2019-10-30 Thread Roger Pau Monné 
On Tue, Oct 29, 2019 at 05:20:18PM -0700, Joe Jin wrote:
> Hi Roger & Jan,
> 
> I got my test env back, and back the patch to stable-4.12, run same
> test, I still seen original issue, guest kernel printed error:
> 
>  kernel:do_IRQ: 20.114 No irq handler for vector (irq -1)
> 
> After that, pass through infiniband VF stopped to work.

Thanks for the testing, TBH I'm not sure what's wrong here, since I
intended my proposed patch to be functionally equivalent to your first
proposed fix.

> My patch as below, please check:

The patch LGTM.

Can you try to add the following debug patch on top of the existing
one and report the output that you get on the Xen console?

---8<---
diff --git a/xen/drivers/passthrough/vtd/intremap.c 
b/xen/drivers/passthrough/vtd/intremap.c
index 07c1c1627a..91a1dde131 100644
--- a/xen/drivers/passthrough/vtd/intremap.c
+++ b/xen/drivers/passthrough/vtd/intremap.c
@@ -977,7 +977,13 @@ int pi_update_irte(const struct pi_desc *pi_desc, const 
struct pirq *pirq,
 
 rc = msi_msg_write_remap_rte(msi_desc, _desc->msg);
 if ( !rc && prev )
+{
+ printk("sync PIRR on vcpu#%u\n", prev->vcpu_id);
  vlapic_sync_pir_to_irr(prev);
+}
+else
+ printk("not syncing PIRR rc: %d vcpu#%u\n",
+rc, prev ? prev->vcpu_id : -1);
 
 return rc;
 }

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

Re: [Xen-devel] [PATCH v2] x86/passthrough: fix migration of MSI when using posted interrupts

2019-10-29 Thread Joe Jin 
Hi Roger & Jan,

I got my test env back, and back the patch to stable-4.12, run same
test, I still seen original issue, guest kernel printed error:

 kernel:do_IRQ: 20.114 No irq handler for vector (irq -1)

After that, pass through infiniband VF stopped to work.

My patch as below, please check:

diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index a1a43cd792..2d175d2a00 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -111,6 +111,12 @@ static void vlapic_clear_irr(int vector, struct vlapic 
*vlapic)
 vlapic_clear_vector(vector, >regs->data[APIC_IRR]);
 }
 
+void vlapic_sync_pir_to_irr(struct vcpu *v)
+{
+if ( hvm_funcs.sync_pir_to_irr )
+hvm_funcs.sync_pir_to_irr(v);
+}
+
 static int vlapic_find_highest_irr(struct vlapic *vlapic)
 {
 if ( hvm_funcs.sync_pir_to_irr )
diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/io.c
index 4290c7c710..b628adea4c 100644
--- a/xen/drivers/passthrough/io.c
+++ b/xen/drivers/passthrough/io.c
@@ -341,7 +341,7 @@ int pt_irq_create_bind(
 {
 uint8_t dest, delivery_mode;
 bool dest_mode;
-int dest_vcpu_id;
+int dest_vcpu_id, prev_vcpu_id = -1;
 const struct vcpu *vcpu;
 uint32_t gflags = pt_irq_bind->u.msi.gflags &
   ~XEN_DOMCTL_VMSI_X86_UNMASKED;
@@ -411,6 +411,7 @@ int pt_irq_create_bind(
 
 pirq_dpci->gmsi.gvec = pt_irq_bind->u.msi.gvec;
 pirq_dpci->gmsi.gflags = gflags;
+prev_vcpu_id = pirq_dpci->gmsi.dest_vcpu_id;
 }
 }
 /* Calculate dest_vcpu_id for MSI-type pirq migration. */
@@ -432,7 +433,10 @@ int pt_irq_create_bind(
 vcpu = vector_hashing_dest(d, dest, dest_mode,
pirq_dpci->gmsi.gvec);
 if ( vcpu )
+{
 pirq_dpci->gmsi.posted = true;
+pirq_dpci->gmsi.dest_vcpu_id = vcpu->vcpu_id;
+}
 }
 if ( vcpu && iommu_enabled )
 hvm_migrate_pirq(pirq_dpci, vcpu);
@@ -440,7 +444,8 @@ int pt_irq_create_bind(
 /* Use interrupt posting if it is supported. */
 if ( iommu_intpost )
 pi_update_irte(vcpu ? >arch.hvm.vmx.pi_desc : NULL,
-   info, pirq_dpci->gmsi.gvec);
+   info, pirq_dpci->gmsi.gvec,
+   prev_vcpu_id >= 0 ? d->vcpu[prev_vcpu_id] : NULL);
 
 if ( pt_irq_bind->u.msi.gflags & XEN_DOMCTL_VMSI_X86_UNMASKED )
 {
@@ -729,7 +734,9 @@ int pt_irq_destroy_bind(
 what = "bogus";
 }
 else if ( pirq_dpci && pirq_dpci->gmsi.posted )
-pi_update_irte(NULL, pirq, 0);
+pi_update_irte(NULL, pirq, 0,
+   pirq_dpci->gmsi.dest_vcpu_id >= 0
+   ? d->vcpu[pirq_dpci->gmsi.dest_vcpu_id] : NULL);
 
 if ( pirq_dpci && (pirq_dpci->flags & HVM_IRQ_DPCI_MAPPED) &&
  list_empty(_dpci->digl_list) )
diff --git a/xen/drivers/passthrough/vtd/intremap.c 
b/xen/drivers/passthrough/vtd/intremap.c
index c9927e4706..d788a4b9e7 100644
--- a/xen/drivers/passthrough/vtd/intremap.c
+++ b/xen/drivers/passthrough/vtd/intremap.c
@@ -961,12 +961,13 @@ void iommu_disable_x2apic_IR(void)
 disable_qinval(drhd->iommu);
 }
 
+#ifdef CONFIG_HVM
 /*
  * This function is used to update the IRTE for posted-interrupt
  * when guest changes MSI/MSI-X information.
  */
 int pi_update_irte(const struct pi_desc *pi_desc, const struct pirq *pirq,
-const uint8_t gvec)
+const uint8_t gvec, struct vcpu *prev)
 {
 struct irq_desc *desc;
 struct msi_desc *msi_desc;
@@ -979,8 +980,8 @@ int pi_update_irte(const struct pi_desc *pi_desc, const 
struct pirq *pirq,
 msi_desc = desc->msi_desc;
 if ( !msi_desc )
 {
-rc = -ENODEV;
-goto unlock_out;
+spin_unlock_irq(>lock);
+return -ENODEV;
 }
 msi_desc->pi_desc = pi_desc;
 msi_desc->gvec = gvec;
@@ -989,10 +990,10 @@ int pi_update_irte(const struct pi_desc *pi_desc, const 
struct pirq *pirq,
 
 ASSERT(pcidevs_locked());
 
-return msi_msg_write_remap_rte(msi_desc, _desc->msg);
-
- unlock_out:
-spin_unlock_irq(>lock);
+rc = msi_msg_write_remap_rte(msi_desc, _desc->msg);
+if ( !rc && prev )
+ vlapic_sync_pir_to_irr(prev);
 
 return rc;
 }
+#endif
diff --git a/xen/include/asm-x86/hvm/vlapic.h b/xen/include/asm-x86/hvm/vlapic.h
index dde66b4f0f..b0017d1dae 100644
--- a/xen/include/asm-x86/hvm/vlapic.h
+++ b/xen/include/asm-x86/hvm/vlapic.h
@@ -150,4 +150,6 @@ bool_t vlapic_match_dest(
 const struct vlapic *target, const struct vlapic *source,
 int short_hand, uint32_t dest, bool_t dest_mode);
 
+void vlapic_sync_pir_to_irr(struct vcpu *v);
+
 #endif /* __ASM_X86_HVM_VLAPIC_H__ */
diff --git a/xen/include/asm-x86/iommu.h b/xen/include/asm-x86/iommu.h
index 8dc392473d..32bfa23648 100644
---

Re: [Xen-devel] [PATCH v2] x86/passthrough: fix migration of MSI when using posted interrupts

2019-10-14 Thread Joe Jin 
On 10/9/19 6:35 AM, Jan Beulich wrote:
> On 09.10.2019 14:52, Roger Pau Monne wrote:
>> When using posted interrupts and the guest migrates MSI from vCPUs Xen
>> needs to flush any pending PIRR vectors on the previous vCPU, or else
>> those vectors could get wrongly injected at a later point when the MSI
>> fields are already updated.
>>
>> Rename sync_pir_to_irr to vlapic_sync_pir_to_irr and export it so it
>> can be called when updating the posted interrupt descriptor field in
>> pi_update_irte. While there also remove the unlock_out from
>> pi_update_irte, it's used in a single goto and removing it makes the
>> function smaller.
>>
>> Note that PIRR is synced to IRR both in pt_irq_destroy_bind and
>> pt_irq_create_bind when the interrupt delivery data is being updated.
>>
>> Also store the vCPU ID in multi-destination mode when using posted
>> interrupts and the interrupt is bound to a single vCPU in order for
>> posted interrupts to be used.
>>
>> While there guard pi_update_irte with CONFIG_HVM since it's only used
>> with HVM guests.
>>
>> Reported-by: Joe Jin 
>> Signed-off-by: Roger Pau Monné 
> 
> Reviewed-by: Jan Beulich 
> 
> Like for the other patch I'd prefer to wait a little with committing
> (even if the VT-d ack appeared quickly) until hopefully a Tested-by
> could be provided.

My test env has not been fixed yet, not sure when it can be fixed, once
it available I'll test it.

Thanks,
Joe


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

Re: [Xen-devel] [PATCH v2] x86/passthrough: fix migration of MSI when using posted interrupts

2019-10-09 Thread Jan Beulich 
On 09.10.2019 14:52, Roger Pau Monne wrote:
> When using posted interrupts and the guest migrates MSI from vCPUs Xen
> needs to flush any pending PIRR vectors on the previous vCPU, or else
> those vectors could get wrongly injected at a later point when the MSI
> fields are already updated.
> 
> Rename sync_pir_to_irr to vlapic_sync_pir_to_irr and export it so it
> can be called when updating the posted interrupt descriptor field in
> pi_update_irte. While there also remove the unlock_out from
> pi_update_irte, it's used in a single goto and removing it makes the
> function smaller.
> 
> Note that PIRR is synced to IRR both in pt_irq_destroy_bind and
> pt_irq_create_bind when the interrupt delivery data is being updated.
> 
> Also store the vCPU ID in multi-destination mode when using posted
> interrupts and the interrupt is bound to a single vCPU in order for
> posted interrupts to be used.
> 
> While there guard pi_update_irte with CONFIG_HVM since it's only used
> with HVM guests.
> 
> Reported-by: Joe Jin 
> Signed-off-by: Roger Pau Monné 

Reviewed-by: Jan Beulich 

Like for the other patch I'd prefer to wait a little with committing
(even if the VT-d ack appeared quickly) until hopefully a Tested-by
could be provided.

Jan

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

[Xen-devel] [PATCH v2] x86/passthrough: fix migration of MSI when using posted interrupts

2019-10-09 Thread Roger Pau Monne 
When using posted interrupts and the guest migrates MSI from vCPUs Xen
needs to flush any pending PIRR vectors on the previous vCPU, or else
those vectors could get wrongly injected at a later point when the MSI
fields are already updated.

Rename sync_pir_to_irr to vlapic_sync_pir_to_irr and export it so it
can be called when updating the posted interrupt descriptor field in
pi_update_irte. While there also remove the unlock_out from
pi_update_irte, it's used in a single goto and removing it makes the
function smaller.

Note that PIRR is synced to IRR both in pt_irq_destroy_bind and
pt_irq_create_bind when the interrupt delivery data is being updated.

Also store the vCPU ID in multi-destination mode when using posted
interrupts and the interrupt is bound to a single vCPU in order for
posted interrupts to be used.

While there guard pi_update_irte with CONFIG_HVM since it's only used
with HVM guests.

Reported-by: Joe Jin 
Signed-off-by: Roger Pau Monné 
---
Cc: Joe Jin 
Cc: Juergen Gross 
---
I would like to see a bug fix for this issue in 4.13. The fix here only
affects posted interrupts, hence I think the risk of breaking anything
else is low.
---
Changes since v1:
 - Store the vcpu id also in multi-dest mode if the interrupt is bound
   to a vcpu for posted delivery.
 - s/#if/#ifdef/.
---
 xen/arch/x86/hvm/vlapic.c  |  6 +++---
 xen/drivers/passthrough/io.c   | 13 ++---
 xen/drivers/passthrough/vtd/intremap.c | 15 ---
 xen/include/asm-x86/hvm/vlapic.h   |  2 ++
 xen/include/asm-x86/iommu.h|  2 +-
 5 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index 9466258d6f..d255ad8db7 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -106,7 +106,7 @@ static void vlapic_clear_irr(int vector, struct vlapic 
*vlapic)
 vlapic_clear_vector(vector, >regs->data[APIC_IRR]);
 }
 
-static void sync_pir_to_irr(struct vcpu *v)
+void vlapic_sync_pir_to_irr(struct vcpu *v)
 {
 if ( hvm_funcs.sync_pir_to_irr )
 alternative_vcall(hvm_funcs.sync_pir_to_irr, v);
@@ -114,7 +114,7 @@ static void sync_pir_to_irr(struct vcpu *v)
 
 static int vlapic_find_highest_irr(struct vlapic *vlapic)
 {
-sync_pir_to_irr(vlapic_vcpu(vlapic));
+vlapic_sync_pir_to_irr(vlapic_vcpu(vlapic));
 
 return vlapic_find_highest_vector(>regs->data[APIC_IRR]);
 }
@@ -1493,7 +1493,7 @@ static int lapic_save_regs(struct vcpu *v, 
hvm_domain_context_t *h)
 if ( !has_vlapic(v->domain) )
 return 0;
 
-sync_pir_to_irr(v);
+vlapic_sync_pir_to_irr(v);
 
 return hvm_save_entry(LAPIC_REGS, v->vcpu_id, h, vcpu_vlapic(v)->regs);
 }
diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/io.c
index b292e79382..5bf1877726 100644
--- a/xen/drivers/passthrough/io.c
+++ b/xen/drivers/passthrough/io.c
@@ -341,7 +341,7 @@ int pt_irq_create_bind(
 {
 uint8_t dest, delivery_mode;
 bool dest_mode;
-int dest_vcpu_id;
+int dest_vcpu_id, prev_vcpu_id = -1;
 const struct vcpu *vcpu;
 uint32_t gflags = pt_irq_bind->u.msi.gflags &
   ~XEN_DOMCTL_VMSI_X86_UNMASKED;
@@ -411,6 +411,7 @@ int pt_irq_create_bind(
 
 pirq_dpci->gmsi.gvec = pt_irq_bind->u.msi.gvec;
 pirq_dpci->gmsi.gflags = gflags;
+prev_vcpu_id = pirq_dpci->gmsi.dest_vcpu_id;
 }
 }
 /* Calculate dest_vcpu_id for MSI-type pirq migration. */
@@ -432,7 +433,10 @@ int pt_irq_create_bind(
 vcpu = vector_hashing_dest(d, dest, dest_mode,
pirq_dpci->gmsi.gvec);
 if ( vcpu )
+{
 pirq_dpci->gmsi.posted = true;
+pirq_dpci->gmsi.dest_vcpu_id = vcpu->vcpu_id;
+}
 }
 if ( vcpu && is_iommu_enabled(d) )
 hvm_migrate_pirq(pirq_dpci, vcpu);
@@ -440,7 +444,8 @@ int pt_irq_create_bind(
 /* Use interrupt posting if it is supported. */
 if ( iommu_intpost )
 pi_update_irte(vcpu ? >arch.hvm.vmx.pi_desc : NULL,
-   info, pirq_dpci->gmsi.gvec);
+   info, pirq_dpci->gmsi.gvec,
+   prev_vcpu_id >= 0 ? d->vcpu[prev_vcpu_id] : NULL);
 
 if ( pt_irq_bind->u.msi.gflags & XEN_DOMCTL_VMSI_X86_UNMASKED )
 {
@@ -729,7 +734,9 @@ int pt_irq_destroy_bind(
 what = "bogus";
 }
 else if ( pirq_dpci && pirq_dpci->gmsi.posted )
-pi_update_irte(NULL, pirq, 0);
+pi_update_irte(NULL, pirq, 0,
+   pirq_dpci->gmsi.dest_vcpu_id >= 0
+   ? d->vcpu[pirq_dpci->gmsi.dest_vcpu_id] : NULL);
 
 if ( pirq_dpci && (pirq_dpci->flags & HVM_IRQ_DPCI_MAPPED) &&
  list_empty(_dpci->digl_list) )
diff --git a/xen/drivers/passthrough/vtd/intremap.c