Fail to boot Dom0 on Xen Arm64 after "dma-mapping: bypass indirect calls for dma-direct"

Fail to boot Dom0 on Xen Arm64 after "dma-mapping: bypass indirect calls for dma-direct"

[Julien Grall]

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 0000000000000000
[    4.083074] Mem abort info:
[    4.085870]   ESR = 0x96000004
[    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 = 0x00000004
[    4.108289]   CM = 0, WnR = 0
[    4.111328] user pgtable: 4k pages, 48-bit VAs, pgdp = (____ptrval____)
[    4.118025] [0000000000000000] pgd=0000000000000000
[    4.123058] Internal error: Oops: 96000004 [#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: 20000005 (nzCv daif -PAN -UAO)
[    4.150876] pc : xen_swiotlb_alloc_coherent+0x64/0x1e8
[    4.156092] lr : dma_alloc_attrs+0xf4/0x110
[    4.160359] sp : ffff00001003b960
[    4.163743] x29: ffff00001003b960 x28: ffff0000112cfbb4
[    4.169137] x27: ffff0000116ed000 x26: ffff00001003ba90
[    4.174533] x25: ffff000010d6c350 x24: 0000000000000005
[    4.179937] x23: 0000000000020000 x22: 000000000001f000
[    4.185323] x21: 0000000000000000 x20: ffff8008b904b0b0
[    4.190727] x19: ffff0000114d4000 x18: ffff000014033fff
[    4.196113] x17: 0000000000000000 x16: 0000000000000000
[    4.201509] x15: 0000000000004000 x14: ffff0000110fc000
[    4.206903] x13: ffff0000114dd000 x12: 0000000000000068
[    4.212299] x11: 0000000000000001 x10: 0000000000000000
[    4.217693] x9 : 0000000000000000 x8 : ffff8008b9b05b00
[    4.223089] x7 : 0000000000000000 x6 : 0000000000000000
[    4.228483] x5 : ffff0000106d4c38 x4 : 0000000000000000
[    4.233879] x3 : 00000000006000c0 x2 : ffff00001003ba90
[    4.239274] x1 : 0000000000020000 x0 : 0000000000000000
[    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 (f9400005)
[    4.345028] ---[ end trace 277757f27697a72b ]---

The bisector fingered the following commit:

commit 356da6d0cde3323236977fce54c1f9612a742036
Author: Christoph Hellwig <hch@lst.de>
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 <hch@lst.de>
    Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
    Tested-by: Jesper Dangaard Brouer <brouer@redhat.com>
    Tested-by: Tony Luck <tony.luck@intel.com>

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

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

Re: Fail to boot Dom0 on Xen Arm64 after "dma-mapping: bypass indirect calls for dma-direct"

[Christoph Hellwig]

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 <linux/dma-mapping.h>
+#include <asm/page.h>
 #include <xen/arm/page-coherent.h>
+
+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<<compound_order(page)) * XEN_PFN_PER_PAGE;
+	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(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 <linux/dma-mapping.h>
+#include <asm/page.h>
 #include <xen/arm/page-coherent.h>
+
+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 =
+		(1<<compound_order(page)) * XEN_PFN_PER_PAGE;
+	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)
+		dma_direct_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))
+		dma_direct_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))
+		dma_direct_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))
+		dma_direct_sync_single_for_device(hwdev, handle, size, dir);
+	else
+		__xen_dma_sync_single_for_device(hwdev, handle, size, dir);
+}
+
+#endif /* _ASM_ARM64_XEN_PAGE_COHERENT_H */
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 989cf872b98c..bb7888429be6 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -645,7 +645,7 @@ xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma,
 		     void *cpu_addr, dma_addr_t dma_addr, size_t size,
 		     unsigned long attrs)
 {
-#if defined(CONFIG_ARM) || defined(CONFIG_ARM64)
+#ifdef CONFIG_ARM
 	if (xen_get_dma_ops(dev)->mmap)
 		return xen_get_dma_ops(dev)->mmap(dev, vma, cpu_addr,
 						    dma_addr, size, attrs);
@@ -662,7 +662,7 @@ xen_swiotlb_get_sgtable(struct device *dev, struct sg_table *sgt,
 			void *cpu_addr, dma_addr_t handle, size_t size,
 			unsigned long attrs)
 {
-#if defined(CONFIG_ARM) || defined(CONFIG_ARM64)
+#ifdef CONFIG_ARM
 	if (xen_get_dma_ops(dev)->get_sgtable) {
 #if 0
 	/*
diff --git a/include/xen/arm/page-coherent.h b/include/xen/arm/page-coherent.h
index 59a260712a56..e57ca55643b1 100644
--- a/include/xen/arm/page-coherent.h
+++ b/include/xen/arm/page-coherent.h
@@ -1,17 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0 */
-#ifndef _ASM_ARM_XEN_PAGE_COHERENT_H
-#define _ASM_ARM_XEN_PAGE_COHERENT_H
-
-#include <asm/page.h>
-#include <asm/dma-mapping.h>
-#include <linux/dma-mapping.h>
-
-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);
-}
+#ifndef _XEN_ARMPAGE_COHERENT_H
+#define _XEN_ARMPAGE_COHERENT_H
 
 void __xen_dma_map_page(struct device *hwdev, struct page *page,
 	     dma_addr_t dev_addr, unsigned long offset, size_t size,
@@ -21,87 +10,7 @@ void __xen_dma_unmap_page(struct device *hwdev, dma_addr_t handle,
 		unsigned long attrs);
 void __xen_dma_sync_single_for_cpu(struct device *hwdev,
 		dma_addr_t handle, size_t size, enum dma_data_direction dir);
-
 void __xen_dma_sync_single_for_device(struct device *hwdev,
 		dma_addr_t handle, size_t size, enum dma_data_direction dir);
 
-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<<compound_order(page)) * XEN_PFN_PER_PAGE;
-	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(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 */
+#endif /* _XEN_ARMPAGE_COHERENT_H */

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

Re: Fail to boot Dom0 on Xen Arm64 after "dma-mapping: bypass indirect calls for dma-direct"

[Julien Grall]

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

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

Re: Fail to boot Dom0 on Xen Arm64 after "dma-mapping: bypass indirect calls for dma-direct"

[Christoph Hellwig]

On Thu, Jan 17, 2019 at 11:43:49AM +0000, 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.

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

Re: Fail to boot Dom0 on Xen Arm64 after "dma-mapping: bypass indirect calls for dma-direct"

[Stefano Stabellini]

On Thu, 17 Jan 2019, Christoph Hellwig wrote:
> On Thu, Jan 17, 2019 at 11:43:49AM +0000, 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. 

Re: Fail to boot Dom0 on Xen Arm64 after "dma-mapping: bypass indirect calls for dma-direct"

[Stefano Stabellini]

On Thu, 17 Jan 2019, Christoph Hellwig wrote:
> On Thu, Jan 17, 2019 at 11:43:49AM +0000, 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. 

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