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

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

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 ? &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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel