Re: [Xen-devel] [PATCH v11 2/6] VT-d: Introduce new fields in msi_desc to track binding with guest interrupt

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 29.03.17 at 07:11, <chao.gao@xxxxxxxxx> wrote:
> v11:
> - Commit message changes.
> - Add code to clear the two new fields introduced in this patch.

The latter appears to contradict ...

> --- a/xen/arch/x86/msi.c
> +++ b/xen/arch/x86/msi.c
> @@ -578,6 +578,7 @@ static struct msi_desc *alloc_msi_entry(unsigned int nr)
>          entry[nr].dev = NULL;
>          entry[nr].irq = -1;
>          entry[nr].remap_index = -1;
> +        entry[nr].pi_desc = NULL;
>      }

... only pi_desc being cleared here. I think the code is fine here (as
you use gvec only when pi_desc is non-NULL), but please try to
avoid giving imprecise information in the future.

> --- a/xen/drivers/passthrough/io.c
> +++ b/xen/drivers/passthrough/io.c
> @@ -618,6 +618,8 @@ int pt_irq_destroy_bind(
>          else
>              what = "bogus";
>      }
> +    else if ( iommu_intpost && pirq && pirq_dpci->gmsi.posted )
> +        pi_update_irte(NULL, pirq, 0);

The more I look at this passing of a NULL vCPU pointer, the less
I like it: If the pointer was used for anything other than
retrieving pi_desc, this would be actively dangerous. I think the
function would better be passed a const struct pi_desc * instead.

> --- a/xen/include/asm-x86/msi.h
> +++ b/xen/include/asm-x86/msi.h
> @@ -118,6 +118,8 @@ struct msi_desc {
>       struct msi_msg msg;             /* Last set MSI message */
>  
>       int remap_index;                /* index in interrupt remapping table */
> +     const struct pi_desc *pi_desc;  /* pointer to posted descriptor */
> +     uint8_t gvec;                   /* guest vector. valid when pi_desc 
> isn't NULL */
>  };

Please avoid adding further holes in this structure, by moving
gvec right after msi_attrib (I'll also queue a patch to move
remap_index up ahead of msg).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel