Re: [Xen-devel] [PATCH] x86/pt: skip setup of posted format IRTE when gvec is 0

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 30.04.19 at 07:19, <chao.gao@xxxxxxxxx> wrote:
> When testing with an UP guest with a pass-thru device with vt-d pi
> enabled in host, we observed that guest couldn't receive interrupts
> from that pass-thru device. Dumping IRTE, we found the corresponding
> IRTE is set to posted format with "vector" field as 0.
> 
> We would fall into this issue when guest used the pirq format of MSI
> (see the comment xen_msi_compose_msg() in linux kernel). As 'dest_id'
> is repurposed, skip migration which is based on 'dest_id'.

I've gone through all uses of gvec, and I couldn't find any existing
special casing of it being zero. I assume this is actually communication
between the kernel and qemu, in which case I'd like to see an
explanation of why the issue needs to be addressed in Xen rather
than qemu. Otherwise, if I've overlooked something, would you
mind pointing out where such special casing lives in Xen?

In any event it doesn't look correct to skip migration altogether in
that case. I'd rather expect it to require getting done differently.
After all there still is a (CPU, vector) tuple associated with that
{,p}IRQ if it's not posted, and hvm_migrate_pirq() is a no-op if it is
posted.

Finally it hasn't become clear to me why this is a UP-guest issue
only.

> --- a/xen/drivers/passthrough/io.c
> +++ b/xen/drivers/passthrough/io.c
> @@ -413,34 +413,52 @@ int pt_irq_create_bind(
>                  pirq_dpci->gmsi.gflags = gflags;
>              }
>          }
> -        /* Calculate dest_vcpu_id for MSI-type pirq migration. */
> -        dest = MASK_EXTR(pirq_dpci->gmsi.gflags,
> -                         XEN_DOMCTL_VMSI_X86_DEST_ID_MASK);
> -        dest_mode = pirq_dpci->gmsi.gflags & XEN_DOMCTL_VMSI_X86_DM_MASK;
> -        delivery_mode = MASK_EXTR(pirq_dpci->gmsi.gflags,
> -                                  XEN_DOMCTL_VMSI_X86_DELIV_MASK);
> -
> -        dest_vcpu_id = hvm_girq_dest_2_vcpu_id(d, dest, dest_mode);
> -        pirq_dpci->gmsi.dest_vcpu_id = dest_vcpu_id;
> -        spin_unlock(&d->event_lock);
> -
> -        pirq_dpci->gmsi.posted = false;
> -        vcpu = (dest_vcpu_id >= 0) ? d->vcpu[dest_vcpu_id] : NULL;
> -        if ( iommu_intpost )
> +        /*
> +         * Migrate pirq and create posted format IRTE only if we know the 
> gmsi's
> +         * dest_id and vector.
> +         */
> +        if ( pirq_dpci->gmsi.gvec )

If we're to go with this hypervisor side change, please insert a
blank line above the comment.

Jan



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