Re: [Xen-devel] [PATCH 07/11] swiotlb-xen: use the same foreign page check everywhere

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 26 Aug 2019, Christoph Hellwig wrote:
> xen_dma_map_page uses a different and more complicated check for foreign
> pages than the other three cache maintainance helpers.  Switch it to the
> simpler pfn_valid method a well, and document the scheme with a single
> improved comment in xen_dma_map_page.
> 
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> ---
>  include/xen/arm/page-coherent.h | 31 +++++++++----------------------
>  1 file changed, 9 insertions(+), 22 deletions(-)
> 
> diff --git a/include/xen/arm/page-coherent.h b/include/xen/arm/page-coherent.h
> index 0e244f4fec1a..07c104dbc21f 100644
> --- a/include/xen/arm/page-coherent.h
> +++ b/include/xen/arm/page-coherent.h
> @@ -41,23 +41,17 @@ static inline void xen_dma_map_page(struct device *hwdev, 
> struct page *page,
>            dma_addr_t dev_addr, unsigned long offset, size_t size,
>            enum dma_data_direction dir, unsigned long attrs)
>  {
> -     unsigned long page_pfn = page_to_xen_pfn(page);
> -     unsigned long dev_pfn = XEN_PFN_DOWN(dev_addr);
> -     unsigned long compound_pages =
> -             (1<<compound_order(page)) * XEN_PFN_PER_PAGE;
> -     bool local = (page_pfn <= dev_pfn) &&
> -             (dev_pfn - page_pfn < compound_pages);
> +     unsigned long pfn = PFN_DOWN(dev_addr);
>  
>       /*
> -      * Dom0 is mapped 1:1, while the Linux page can span across
> -      * multiple Xen pages, it's not possible for it to contain a
> -      * mix of local and foreign Xen pages. So if the first xen_pfn
> -      * == mfn the page is local otherwise it's a foreign page
> -      * grant-mapped in dom0. If the page is local we can safely
> -      * call the native dma_ops function, otherwise we call the xen
> -      * specific function.
> +      * Dom0 is mapped 1:1, and while the Linux page can span across multiple
> +      * Xen pages, it is not possible for it to contain a mix of local and
> +      * foreign Xen pages.  Calling pfn_valid on a foreign mfn will always
> +      * return false, so if pfn_valid returns true the pages is local and we
> +      * can use the native dma-direct functions, otherwise we call the Xen
> +      * specific version.
>        */
> -     if (local)
> +     if (pfn_valid(pfn))
>               dma_direct_map_page(hwdev, page, offset, size, dir, attrs);
>       else
>               __xen_dma_map_page(hwdev, page, dev_addr, offset, size, dir, 
> attrs);
> @@ -67,14 +61,7 @@ static inline void xen_dma_unmap_page(struct device 
> *hwdev, dma_addr_t handle,
>               size_t size, enum dma_data_direction dir, unsigned long attrs)
>  {
>       unsigned long pfn = PFN_DOWN(handle);
> -     /*
> -      * Dom0 is mapped 1:1, while the Linux page can be spanned accross
> -      * multiple Xen page, it's not possible to have a mix of local and
> -      * foreign Xen page. Dom0 is mapped 1:1, so calling pfn_valid on a
> -      * foreign mfn will always return false. If the page is local we can
> -      * safely call the native dma_ops function, otherwise we call the xen
> -      * specific function.
> -      */
> +
>       if (pfn_valid(pfn))
>               dma_direct_unmap_page(hwdev, handle, size, dir, attrs);
>       else
> -- 
> 2.20.1
> 

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