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

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

On Fri, Nov 08, 2019 at 02:25:05AM +0000, Tian, Kevin wrote:
> > From: Roger Pau Monné [mailto:roger.pau@xxxxxxxxxx]
> > Sent: Monday, November 4, 2019 5:47 PM
> > 
> > On Sat, Nov 02, 2019 at 07:48:06AM +0000, Tian, Kevin wrote:
> > > > From: Roger Pau Monné [mailto:roger.pau@xxxxxxxxxx]
> > > > 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(&posted_intr_vector,
> > > > pi_notification_interrupt);
> > > > > >         if ( iommu_intpost )
> > > > > >             alloc_direct_apic_vector(&pi_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 vector
in the PIRR, so we do need to sync the PIRR with the IRR even when CPU
only posted interrupts are used.

May I ask why this is done that way? When VT-d posted interrupts are
not used wouldn't it be simpler to just set the vector in the IRR
directly instead of setting it in the PIRR and later syncing the PIRR
with IRR?

Thanks, Roger.

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