Re: Fail to boot Dom0 on Xen Arm64 after "dma-mapping: bypass indirect calls for dma-direct"
On Thu, 17 Jan 2019, Christoph Hellwig wrote: > On Thu, Jan 17, 2019 at 11:43:49AM +, Julien Grall wrote: > > Looking at the change for arm64, you will always call dma-direct API. In > > previous Linux version, xen-swiotlb will call dev->archdata.dev_dma_ops (a > > copy of dev->dma_ops before setting Xen DMA ops) if not NULL. Does it mean > > we expect dev->dma_ops to always be NULL and hence using dma-direct API? > > The way I understood the code from inspecting it and sking the > maintainers a few askings is that for DOM0 we always use xen-swiotlb > as the actual dma_map_ops, but then use the functions in page-coherent.h > only to deal with cache maintainance, so it should be safe. Yes, what you wrote is correct, the stuff in include/xen/arm/page-coherent.h is only to deal with cache maintenance, specifically any cache maintenance operations that might be required by map/unmap_page, dma_sync_single_for_cpu/device. It looks like it is safe today to make the dma_direct calls directly, especially considering that on arm64 it looks like the only other option is iommu_dma_ops which has never been used with Xen so far (the IOMMU has not been exposed to guests yet). On arm32 we don't have this problem because dev->dma_ops is always != NULL with MMU enable (required on Xen), right? So the patch looks fine, I only have an optional suggestion to add a check on dma_ops being unset. I'll reply to the patch. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: Fail to boot Dom0 on Xen Arm64 after "dma-mapping: bypass indirect calls for dma-direct"
On Thu, Jan 17, 2019 at 11:43:49AM +, Julien Grall wrote: > Looking at the change for arm64, you will always call dma-direct API. In > previous Linux version, xen-swiotlb will call dev->archdata.dev_dma_ops (a > copy of dev->dma_ops before setting Xen DMA ops) if not NULL. Does it mean > we expect dev->dma_ops to always be NULL and hence using dma-direct API? The way I understood the code from inspecting it and sking the maintainers a few askings is that for DOM0 we always use xen-swiotlb as the actual dma_map_ops, but then use the functions in page-coherent.h only to deal with cache maintainance, so it should be safe. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: Fail to boot Dom0 on Xen Arm64 after "dma-mapping: bypass indirect calls for dma-direct"
(+ Leo Yan who reported a similar issues on xen-devel) Hi Christoph, On 16/01/2019 18:19, Christoph Hellwig wrote: Does this fix your problem? Thank you for the patch. This allows me to boot Dom0 on Juno r2. My knowledge of DMA is quite limited, so I may have missed something. Looking at the change for arm64, you will always call dma-direct API. In previous Linux version, xen-swiotlb will call dev->archdata.dev_dma_ops (a copy of dev->dma_ops before setting Xen DMA ops) if not NULL. Does it mean we expect dev->dma_ops to always be NULL and hence using dma-direct API? Cheers, -- Julien Grall ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: Fail to boot Dom0 on Xen Arm64 after "dma-mapping: bypass indirect calls for dma-direct"
Does this fix your problem?
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/xen/page-coherent.h
b/arch/arm64/include/asm/xen/page-coherent.h
index b3ef061d8b74..61006385a437 100644
--- a/arch/arm64/include/asm/xen/page-coherent.h
+++ b/arch/arm64/include/asm/xen/page-coherent.h
@@ -1 +1,84 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_ARM64_XEN_PAGE_COHERENT_H
+#define _ASM_ARM64_XEN_PAGE_COHERENT_H
+
+#include
+#include
#include
+
+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 dma_direct_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)
+{
+ dma_direct_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 =
+ (1mmap(dev, vma, cpu_addr,
Fail to boot Dom0 on Xen Arm64 after "dma-mapping: bypass indirect calls for dma-direct"
Hi, I made an attempt to boot Linux 5.0-rc2 as Dom0 on Xen Arm64 and got the following splat: [4.074264] Unable to handle kernel NULL pointer dereference at virtual address [4.083074] Mem abort info: [4.085870] ESR = 0x9604 [4.089050] Exception class = DABT (current EL), IL = 32 bits [4.095065] SET = 0, FnV = 0 [4.098138] EA = 0, S1PTW = 0 [4.101405] Data abort info: [4.104362] ISV = 0, ISS = 0x0004 [4.108289] CM = 0, WnR = 0 [4.111328] user pgtable: 4k pages, 48-bit VAs, pgdp = (ptrval) [4.118025] [] pgd= [4.123058] Internal error: Oops: 9604 [#1] PREEMPT SMP [4.128590] Modules linked in: [4.131727] CPU: 4 PID: 1 Comm: swapper/0 Not tainted 5.0.0-rc2-00154-gc5dbed6dcf60 #1191 [4.139997] Hardware name: ARM Juno development board (r2) (DT) [4.145995] pstate: 2005 (nzCv daif -PAN -UAO) [4.150876] pc : xen_swiotlb_alloc_coherent+0x64/0x1e8 [4.156092] lr : dma_alloc_attrs+0xf4/0x110 [4.160359] sp : 1003b960 [4.163743] x29: 1003b960 x28: 112cfbb4 [4.169137] x27: 116ed000 x26: 1003ba90 [4.174533] x25: 10d6c350 x24: 0005 [4.179937] x23: 0002 x22: 0001f000 [4.185323] x21: x20: 8008b904b0b0 [4.190727] x19: 114d4000 x18: 14033fff [4.196113] x17: x16: [4.201509] x15: 4000 x14: 110fc000 [4.206903] x13: 114dd000 x12: 0068 [4.212299] x11: 0001 x10: [4.217693] x9 : x8 : 8008b9b05b00 [4.223089] x7 : x6 : [4.228483] x5 : 106d4c38 x4 : [4.233879] x3 : 006000c0 x2 : 1003ba90 [4.239274] x1 : 0002 x0 : [4.244671] Process swapper/0 (pid: 1, stack limit = 0x(ptrval)) [4.251456] Call trace: [4.253985] xen_swiotlb_alloc_coherent+0x64/0x1e8 [4.258857] dma_alloc_attrs+0xf4/0x110 [4.262774] dmam_alloc_attrs+0x64/0xb8 [4.266691] sil24_port_start+0x6c/0x100 [4.270704] ata_host_start.part.10+0x104/0x210 [4.275304] ata_host_activate+0x64/0x148 [4.279392] sil24_init_one+0x1ac/0x2b0 [4.283312] pci_device_probe+0xdc/0x168 [4.287313] really_probe+0x1f0/0x298 [4.291054] driver_probe_device+0x58/0x100 [4.295316] __driver_attach+0xdc/0xe0 [4.299145] bus_for_each_dev+0x74/0xc8 [4.303061] driver_attach+0x20/0x28 [4.306716] bus_add_driver+0x1b0/0x220 [4.310641] driver_register+0x60/0x110 [4.314549] __pci_register_driver+0x58/0x68 [4.318902] sil24_pci_driver_init+0x20/0x28 [4.323251] do_one_initcall+0xb8/0x348 [4.327166] kernel_init_freeable+0x3cc/0x474 [4.331606] kernel_init+0x10/0x100 [4.335171] ret_from_fork+0x10/0x1c [4.338829] Code: f941fa80 aa1503e4 aa1a03e2 aa1703e1 (f945) [4.345028] ---[ end trace 277757f27697a72b ]--- The bisector fingered the following commit: commit 356da6d0cde3323236977fce54c1f9612a742036 Author: Christoph Hellwig Date: Thu Dec 6 13:39:32 2018 -0800 dma-mapping: bypass indirect calls for dma-direct Avoid expensive indirect calls in the fast path DMA mapping operations by directly calling the dma_direct_* ops if we are using the directly mapped DMA operations. Signed-off-by: Christoph Hellwig Acked-by: Jesper Dangaard Brouer Tested-by: Jesper Dangaard Brouer Tested-by: Tony Luck Discussing with Robin, it looks like that the current wrappers in for Xen (see include/xen/arm/page-coherent.h) are not able to cope with direct calls. Those wrappers are used by swiotlb to call the underlying DMA ops of the device. They are unable to cope with NULL (aka direct-mapping), hence the NULL dereference crash. Do you have any suggestion how this should be fixed? Best regards, -- Julien Grall ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
