Re: [Xen-devel] [PATCH v9 8/8] VT-d: Add copy_irte_{to, from}_irt for updating irte

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 27.02.17 at 02:45, <chao.gao@xxxxxxxxx> wrote:
> We used structure assignment to update irte which was not safe when a
> interrupt happend during update. It is better to update IRTE atomically
> through cmpxchg16b(). When cmpxchg16b is not supported, two 64-bit write
> operation can atomically update IRTE when only one the high dword or
> low dword is intented to be changed.

s/dword/qword/ in the last sentence.

> --- a/xen/drivers/passthrough/vtd/intremap.c
> +++ b/xen/drivers/passthrough/vtd/intremap.c
> @@ -169,6 +169,37 @@ bool_t __init iommu_supports_eim(void)
>      return 1;
>  }
>  
> +static inline void copy_irte_from_irt(struct iremap_entry *dst,
> +                                      struct iremap_entry *src)
> +{
> +    *dst = *src;
> +}
> +
> +static void copy_irte_to_irt(struct iremap_entry *dst, struct iremap_entry 
> *src)

For both functions the source side wants to be pointer to const.

> +{
> +    if ( cpu_has_cx16 )
> +    {
> +        __uint128_t ret;
> +        struct iremap_entry old_ire;
> +
> +        copy_irte_from_irt(&old_ire, dst);
> +        ret = cmpxchg16b(dst, &old_ire, src);
> +
> +        /*
> +         * In the above, we use cmpxchg16 to atomically update the 128-bit
> +         * IRTE, and the hardware cannot update the IRTE behind us, so
> +         * the return value of cmpxchg16 should be the same as old_ire.
> +         * This ASSERT validate it.
> +         */
> +        ASSERT(ret == old_ire.val);
> +    }
> +    else
> +    {
> +        dst->lo = src->lo;
> +        dst->hi = src->hi;

This doesn't exactly match the description: You nevertheless write
both halves, which makes the update non-atomic. But with the earlier
patch adjusted to document the conditions under which this is safe,
this may be fine here too. Just make the patch description match
reality then please.

Jan


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