Re: [PATCH] arm64/xen: fix xen-swiotlb cache flushing
On Mon, Jan 21, 2019 at 03:56:29PM -0800, Stefano Stabellini wrote: > > Where should we pick this up? I could pick it up through the dma-mapping > > tree given that is where the problem is introduced, but the Xen or arm64 > > trees would also fit. > > I am happy for you to carry it in the dma-mapping tree, especially if > you have other fixes to send. Otherwise, let me know. Thanks, I've queued it up for Linus. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] arm64/xen: fix xen-swiotlb cache flushing
On Sat, 19 Jan 2019, Christoph Hellwig wrote: > [full quote deleted, please take a little more care when quoting] > > On Fri, Jan 18, 2019 at 04:44:23PM -0800, Stefano Stabellini wrote: > > > #ifdef CONFIG_XEN > > > - if (xen_initial_domain()) { > > > - dev->archdata.dev_dma_ops = dev->dma_ops; > > > + if (xen_initial_domain()) > > > dev->dma_ops = xen_dma_ops; > > > - } > > > #endif > > > } > > > > This is an optional suggestion, but it would be nice to add a check on > > dev->dma_ops being unset here, something like: > > > > #ifdef CONFIG_XEN > > if (xen_initial_domain()) { > > if (dev->dma_ops != NULL) > > warning/error > > dev->dma_ops = xen_dma_ops; > > } > > > > Does it make sense? > > Well, no such check existed before, so this probably should be a > separate patch if we care enough. I have a series for 5.1 pending > that moves the IOMMU handling to the comment code which will make > the ops assginment a lot cleaner, and I guess I could fold such > a check in that. Doing it now will just create churn as it would > have to get reworked anyway Fine by me, thank you! > > Reviewed-by: Stefano Stabellini > > Where should we pick this up? I could pick it up through the dma-mapping > tree given that is where the problem is introduced, but the Xen or arm64 > trees would also fit. I am happy for you to carry it in the dma-mapping tree, especially if you have other fixes to send. Otherwise, let me know. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] arm64/xen: fix xen-swiotlb cache flushing
[full quote deleted, please take a little more care when quoting] On Fri, Jan 18, 2019 at 04:44:23PM -0800, Stefano Stabellini wrote: > > #ifdef CONFIG_XEN > > - if (xen_initial_domain()) { > > - dev->archdata.dev_dma_ops = dev->dma_ops; > > + if (xen_initial_domain()) > > dev->dma_ops = xen_dma_ops; > > - } > > #endif > > } > > This is an optional suggestion, but it would be nice to add a check on > dev->dma_ops being unset here, something like: > > #ifdef CONFIG_XEN > if (xen_initial_domain()) { > if (dev->dma_ops != NULL) > warning/error > dev->dma_ops = xen_dma_ops; > } > > Does it make sense? Well, no such check existed before, so this probably should be a separate patch if we care enough. I have a series for 5.1 pending that moves the IOMMU handling to the comment code which will make the ops assginment a lot cleaner, and I guess I could fold such a check in that. Doing it now will just create churn as it would have to get reworked anyway > Reviewed-by: Stefano Stabellini Where should we pick this up? I could pick it up through the dma-mapping tree given that is where the problem is introduced, but the Xen or arm64 trees would also fit. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] arm64/xen: fix xen-swiotlb cache flushing
On Thu, 17 Jan 2019, Christoph Hellwig wrote: > Xen-swiotlb hooks into the arm/arm64 arch code through a copy of the > DMA mapping operations stored in the struct device arch data. > > Switching arm64 to use the direct calls for the merged DMA direct / > swiotlb code broke this scheme. Replace the indirect calls with > direct-calls in xen-swiotlb as well to fix this problem. > > Fixes: 356da6d0cd ("dma-mapping: bypass indirect calls for dma-direct") > Reported-by: Julien Grall > Signed-off-by: Christoph Hellwig > --- > arch/arm/include/asm/xen/page-coherent.h | 94 + > arch/arm64/include/asm/device.h| 3 - > arch/arm64/include/asm/xen/page-coherent.h | 76 + > arch/arm64/mm/dma-mapping.c| 4 +- > drivers/xen/swiotlb-xen.c | 4 +- > include/xen/arm/page-coherent.h| 97 +- > 6 files changed, 176 insertions(+), 102 deletions(-) > > diff --git a/arch/arm/include/asm/xen/page-coherent.h > b/arch/arm/include/asm/xen/page-coherent.h > index b3ef061d8b74..2c403e7c782d 100644 > --- a/arch/arm/include/asm/xen/page-coherent.h > +++ b/arch/arm/include/asm/xen/page-coherent.h > @@ -1 +1,95 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _ASM_ARM_XEN_PAGE_COHERENT_H > +#define _ASM_ARM_XEN_PAGE_COHERENT_H > + > +#include > +#include > #include > + > +static inline const struct dma_map_ops *xen_get_dma_ops(struct device *dev) > +{ > + if (dev && dev->archdata.dev_dma_ops) > + return dev->archdata.dev_dma_ops; > + return get_arch_dma_ops(NULL); > +} > + > +static inline void *xen_alloc_coherent_pages(struct device *hwdev, size_t > size, > + dma_addr_t *dma_handle, gfp_t flags, unsigned long attrs) > +{ > + return xen_get_dma_ops(hwdev)->alloc(hwdev, size, dma_handle, flags, > attrs); > +} > + > +static inline void xen_free_coherent_pages(struct device *hwdev, size_t size, > + void *cpu_addr, dma_addr_t dma_handle, unsigned long attrs) > +{ > + xen_get_dma_ops(hwdev)->free(hwdev, size, cpu_addr, dma_handle, attrs); > +} > + > +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< + bool local = (page_pfn <= dev_pfn) && > + (dev_pfn - page_pfn < compound_pages); > + > + /* > + * 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. > + */ > + if (local) > + xen_get_dma_ops(hwdev)->map_page(hwdev, page, offset, size, > dir, attrs); > + else > + __xen_dma_map_page(hwdev, page, dev_addr, offset, size, dir, > attrs); > +} > + > +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)) { > + if (xen_get_dma_ops(hwdev)->unmap_page) > + xen_get_dma_ops(hwdev)->unmap_page(hwdev, handle, size, > dir, attrs); > + } else > + __xen_dma_unmap_page(hwdev, handle, size, dir, attrs); > +} > + > +static inline void xen_dma_sync_single_for_cpu(struct device *hwdev, > + dma_addr_t handle, size_t size, enum dma_data_direction dir) > +{ > + unsigned long pfn = PFN_DOWN(handle); > + if (pfn_valid(pfn)) { > + if (xen_get_dma_ops(hwdev)->sync_single_for_cpu) > + xen_get_dma_ops(hwdev)->sync_single_for_cpu(hwdev, > handle, size, dir); > + } else > + __xen_dma_sync_single_for_cpu(hwdev, handle, size, dir); > +} > + > +static inline void xen_dma_sync_single_for_device(struct device *hwdev, > + dma_addr_t handle, size_t size, enum dma_data_direction dir) > +{ > + unsigned long pfn = PFN_DOWN(handle); > + if (pfn_valid(pfn)) { > + if (xen_get_dma_ops(hwde
[PATCH] arm64/xen: fix xen-swiotlb cache flushing
Xen-swiotlb hooks into the arm/arm64 arch code through a copy of the DMA mapping operations stored in the struct device arch data. Switching arm64 to use the direct calls for the merged DMA direct / swiotlb code broke this scheme. Replace the indirect calls with direct-calls in xen-swiotlb as well to fix this problem. Fixes: 356da6d0cd ("dma-mapping: bypass indirect calls for dma-direct") Reported-by: Julien Grall Signed-off-by: Christoph Hellwig --- arch/arm/include/asm/xen/page-coherent.h | 94 + arch/arm64/include/asm/device.h| 3 - arch/arm64/include/asm/xen/page-coherent.h | 76 + arch/arm64/mm/dma-mapping.c| 4 +- drivers/xen/swiotlb-xen.c | 4 +- include/xen/arm/page-coherent.h| 97 +- 6 files changed, 176 insertions(+), 102 deletions(-) diff --git a/arch/arm/include/asm/xen/page-coherent.h b/arch/arm/include/asm/xen/page-coherent.h index b3ef061d8b74..2c403e7c782d 100644 --- a/arch/arm/include/asm/xen/page-coherent.h +++ b/arch/arm/include/asm/xen/page-coherent.h @@ -1 +1,95 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _ASM_ARM_XEN_PAGE_COHERENT_H +#define _ASM_ARM_XEN_PAGE_COHERENT_H + +#include +#include #include + +static inline const struct dma_map_ops *xen_get_dma_ops(struct device *dev) +{ + if (dev && dev->archdata.dev_dma_ops) + return dev->archdata.dev_dma_ops; + return get_arch_dma_ops(NULL); +} + +static inline void *xen_alloc_coherent_pages(struct device *hwdev, size_t size, + dma_addr_t *dma_handle, gfp_t flags, unsigned long attrs) +{ + return xen_get_dma_ops(hwdev)->alloc(hwdev, size, dma_handle, flags, attrs); +} + +static inline void xen_free_coherent_pages(struct device *hwdev, size_t size, + void *cpu_addr, dma_addr_t dma_handle, unsigned long attrs) +{ + xen_get_dma_ops(hwdev)->free(hwdev, size, cpu_addr, dma_handle, attrs); +} + +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 = + (1unmap_page) + xen_get_dma_ops(hwdev)->unmap_page(hwdev, handle, size, dir, attrs); + } else + __xen_dma_unmap_page(hwdev, handle, size, dir, attrs); +} + +static inline void xen_dma_sync_single_for_cpu(struct device *hwdev, + dma_addr_t handle, size_t size, enum dma_data_direction dir) +{ + unsigned long pfn = PFN_DOWN(handle); + if (pfn_valid(pfn)) { + if (xen_get_dma_ops(hwdev)->sync_single_for_cpu) + xen_get_dma_ops(hwdev)->sync_single_for_cpu(hwdev, handle, size, dir); + } else + __xen_dma_sync_single_for_cpu(hwdev, handle, size, dir); +} + +static inline void xen_dma_sync_single_for_device(struct device *hwdev, + dma_addr_t handle, size_t size, enum dma_data_direction dir) +{ + unsigned long pfn = PFN_DOWN(handle); + if (pfn_valid(pfn)) { + if (xen_get_dma_ops(hwdev)->sync_single_for_device) + xen_get_dma_ops(hwdev)->sync_single_for_device(hwdev, handle, size, dir); + } else + __xen_dma_sync_single_for_device(hwdev, handle, size, dir); +} + +#endif /* _ASM_ARM_XEN_PAGE_COHERENT_H */ diff --git a/arch/arm64/include/asm/device.h b/arch/arm64/include/asm/device.h index 3dd3d664c5c5..4658c937e173 100644 --- a/arch/arm64/include/asm/device.h +++ b/arch/arm64/include/asm/device.h @@ -20,9 +20,6 @@ struct dev_archdata { #ifdef CONFIG_IOMMU_API void *iommu;/* private IOMMU data */ #endif -#ifdef CONFIG_XEN - const struct dma_map_ops *dev_dma_ops; -#endif }; struct pdev_archdata { diff --git a/arch/arm64/include/asm/xen/page-coherent.h b/arch/arm64/include/asm/xen/page-coherent.h inde