2019-08-26 14:40:20

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 07/11] swiotlb-xen: use the same foreign page check everywhere

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 <[email protected]>
---
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


2019-08-29 22:48:14

by Stefano Stabellini

[permalink] [raw]
Subject: Re: [PATCH 07/11] swiotlb-xen: use the same foreign page check everywhere

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 <[email protected]>

Reviewed-by: Stefano Stabellini <[email protected]>


> ---
> 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
>