Re: [Patch RFC 10/13] vt-d: Held on the removed page until the Device-TLB flush is completed.

[Julien Grall <julien.grall@citrix.com>]

Hi Quan,

On 16/09/2015 14:24, Quan Xu wrote:
> Signed-off-by: Quan Xu <quan.xu@intel.com>
> ---
>   xen/common/memory.c | 16 +++++++++++++++-
>   1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/xen/common/memory.c b/xen/common/memory.c
> index 61bb94c..4b2def5 100644
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -253,7 +253,21 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
>
>       guest_physmap_remove_page(d, gmfn, mfn, 0);
>
> -    put_page(page);
> +#ifdef HAS_PASSTHROUGH
> +    /*
> +     * The page freed from the domain should be on held, until the
> +     * Device-TLB flush is completed. The page previously associated
> +     * with the freed portion of GPA should not be reallocated for
> +     * another purpose until the appropriate invalidations have been
> +     * performed. Otherwise, the original page owner can still access
> +     * freed page though DMA.
> +     */
> +    if ( need_iommu(d) && QI_FLUSHING(d) && !d->is_dying )
> +        qi_hold_page(d, page);

qi_hold_page is defined in drivers/passthrough/vtd/iommu.c which is only 
compiled for x86.

Which means that this call will break compilation on ARM. Also, AMD 
iommu should never call this code.

IHMO this should be moved in x86 specific code. Although, if you plan to 
keep it in common code, you need to at least add a new IOMMU ops.

> +    else
> +#endif
> +        put_page(page);
> +
>       put_gfn(d, gmfn);
>
>       return 1;
>

Regards,

-- 
Julien Grall