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

[Jan Beulich]

On 09.10.2019 12:20, Roger Pau Monné  wrote:
> On Wed, Oct 09, 2019 at 12:08:50PM +0200, Jan Beulich wrote:
>> Considering that hvm_girq_dest_2_vcpu_id() isn't really inexpensive,
>> subsequent cleanup may then involve avoiding to call this function
>> if we'd overwrite .dest_vcpu_id as per above anyway.
> 
> Hm, I see. I would leave that for a further optimization.

Sure, hence me saying "subsequent cleanup".

Jan

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

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

[Roger Pau Monné]

On Wed, Oct 09, 2019 at 12:08:50PM +0200, Jan Beulich wrote:
> On 09.10.2019 11:05, Roger Pau Monne wrote:
> > @@ -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;
> 
> If this and ...
> 
> > @@ -440,7 +441,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);
> 
> ... this are to be reliable, then - as explained to Joe already
> in the earlier discussion - I think you need to update
> pirq_dpci->gmsi.dest_vcpu_id in a code section a few lines up
> from here (such that it would be reliable the next time we come
> here) like this:
> 
> @@ -xxx,7 +yyy,10 @@
>  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);
> 
> This ought to be fine because so far .dest_vcpu_id has a consumer
> only in the non-posted case (in hvm_migrate_pirq()).

Oh, I see. dest_vcpu_id is only valid for non-multidest, but when
using posted interrupts Xen selects a fixed destination vcpu even in
multidest, so we can take advantage of posted interrupts at the price
of always delivering to the same vcpu.

> 
> Considering that hvm_girq_dest_2_vcpu_id() isn't really inexpensive,
> subsequent cleanup may then involve avoiding to call this function
> if we'd overwrite .dest_vcpu_id as per above anyway.

Hm, I see. I would leave that for a further optimization.

Thanks, Roger.

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

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

[Jan Beulich]

On 09.10.2019 11:05, Roger Pau Monne wrote:
> @@ -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;

If this and ...

> @@ -440,7 +441,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);

... this are to be reliable, then - as explained to Joe already
in the earlier discussion - I think you need to update
pirq_dpci->gmsi.dest_vcpu_id in a code section a few lines up
from here (such that it would be reliable the next time we come
here) like this:

@@ -xxx,7 +yyy,10 @@
 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);

This ought to be fine because so far .dest_vcpu_id has a consumer
only in the non-posted case (in hvm_migrate_pirq()).

Considering that hvm_girq_dest_2_vcpu_id() isn't really inexpensive,
subsequent cleanup may then involve avoiding to call this function
if we'd overwrite .dest_vcpu_id as per above anyway.

> --- a/xen/drivers/passthrough/vtd/intremap.c
> +++ b/xen/drivers/passthrough/vtd/intremap.c
> @@ -946,12 +946,13 @@ void intel_iommu_disable_eim(void)
>  disable_qinval(drhd->iommu);
>  }
>  
> +#if CONFIG_HVM

#ifdef please.

Jan

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