Hi all,
this patch series enables xen-swiotlb on arm and arm64.
It has been heavily reworked compared to the previous versions in order
to achieve better performances and to address review comments.
We are not using dma_mark_clean to ensure coherency anymore. We call the
platform implementation of map_page and unmap_page.
We assume that dom0 has been mapped 1:1 (physical address ==
machine address), that is what Xen on ARM currently does.
As a consequence we only use the swiotlb to handle dma
requests involving pages corresponding to grant refs. Obviously these
pages cannot be part of the 1:1 because they belong to another domain.
To improve code readability this version of the series introduces a
"p2m" for arm and arm64, implemented using two red-black trees. The p2m
tracks physical to machine and machine to physical mappings. Given that
we assume a 1:1 mapping in dom0, the only entries in both trees are
grant refs that have been mapped in dom0.
Another consequence of the new approach is that the hypercalls we were
previously introducing (XENMEM_exchange_and_pin, XENMEM_pin,
XENMEM_unpin) are not needed anymore.
This latest version doesn't need any hypervisor changes anymore.
Cheers,
Stefano
Changes in v8:
- use __phys_to_pfn and __pfn_to_phys in phys_to_dma and dma_to_phys;
- cast 0 to dma_addr_t in the definition of DMA_ERROR_CODE;
- move pfn_to_mfn and mfn_to_pfn to page.h as static inline functions;
- no need to walk the two p2m trees if phys_to_mach.rb_node is NULL;
- correctly handle multipage p2m entries;
- substitute the p2m spin_lock with a rwlock;
- assume dom0 is mapped 1:1, no need to call XENMEM_exchange in
xen_create_contiguous_region;
- add two simple performance improvements for swiotlb-xen.
Changes in v7:
- dma_mark_clean: switch to an empty implementation (we are going to use
the platform map_page and unmap_page to ensure coherency);
- dma_capable: use coherent_dma_mask if dma_mask hasn't been
allocated;
- introduce a p2m for xen on arm and arm64;
- call __set_phys_to_machine_multi from xen_create_contiguous_region to
update the p2m;
- remove XENMEM_unpin and XENMEM_exchange_and_pin;
- set nr_exchanged to 0 before calling the hypercall;
- return xen_dma_ops only if we are the initial domain;
- rename __get_dma_ops to __generic_dma_ops;
- call __generic_dma_ops(hwdev)->alloc/free on arm64 too;
- introduce xen_dma_map/unmap_page and xen_dma_sync_single_for_cpu/device;
- use xen_dma_map/unmap_page, xen_dma_sync_single_for_cpu/device in
swiotlb-xen;
- use dev_warn instead of pr_warn;
- remove the extra autotranslate check in biomerge.c;
- call set_phys_to_machine after mapping grant refs in gnttab_map_refs;
- call set_phys_to_machine after unmapping grant refs in gnttab_unmap_refs;
- allow __set_phys_to_machine to be called for autotranslate guests on
x86.
Changes in v6:
- check for dev->dma_mask being NULL in dma_capable;
- update the comments and the hypercalls structures;
- add a xen_dma_info entry to the rbtree in xen_swiotlb_alloc_coherent
to keep track of the new mapping. Free the entry in xen_swiotlb_free_coherent;
- rename xen_dma_seg to dma_info in xen_swiotlb_alloc/free_coherent to
avoid confusions;
- introduce and export xen_dma_ops;
- call xen_mm_init from as arch_initcall;
- call __get_dma_ops to get the native dma_ops pointer on arm;
- do not merge biovecs;
- add single page optimization: pin the page rather than bouncing.
Changes in v5:
- dropped the first two patches, already in the Xen tree;
- implement dma_mark_clean using dmac_flush_range on arm;
- add "arm64: define DMA_ERROR_CODE"
- better comment for XENMEM_exchange_and_pin return codes;
- fix xen_dma_add_entry error path;
- remove the spin_lock: the red-black tree is not modified at run time;
- add "swiotlb-xen: introduce xen_swiotlb_set_dma_mask";
- add "xen: introduce xen_alloc/free_coherent_pages";
- add "swiotlb-xen: use xen_alloc/free_coherent_pages";
- add "swiotlb: don't assume that io_tlb_start-io_tlb_end is coherent".
Changes in v4:
- rename XENMEM_get_dma_buf to XENMEM_exchange_and_pin;
- rename XENMEM_put_dma_buf to XENMEM_unpin;
- improve the documentation of the new hypercalls;
- add a note about out.address_bits for XENMEM_exchange;
- code style fixes;
- add err_out label in xen_dma_add_entry;
- remove INVALID_ADDRESS, use DMA_ERROR_CODE instead;
- add in-code comments regarding the usage of xen_dma_seg[0].dma_addr.
Changes in v3:
- add a patch to compile SWIOTLB without CONFIG_NEED_SG_DMA_LENGTH;
- add a patch to compile SWIOTLB_XEN without CONFIG_NEED_SG_DMA_LENGTH;
- arm/dma_capable: do not treat dma_mask as a limit;
- arm/dmabounce: keep using arm_dma_ops;
- add missing __init in xen_early_init declaration;
- many code style and name changes in swiotlb-xen.c;
- improve error checks in xen_dma_add_entry;
- warn on XENMEM_put_dma_buf failures.
Changes in v2:
- fixed a couple of errors in xen_bus_to_phys, xen_phys_to_bus and
xen_swiotlb_fixup.
Julien Grall (1):
ASoC: Samsung: Rename dma_ops by samsung_dma_ops
Stefano Stabellini (18):
arm: make SWIOTLB available
arm64: define DMA_ERROR_CODE
arm/xen,arm64/xen: introduce p2m
xen/x86: allow __set_phys_to_machine for autotranslate guests
xen: make xen_create_contiguous_region return the dma address
xen/arm,arm64: enable SWIOTLB_XEN
swiotlb-xen: introduce xen_swiotlb_set_dma_mask
arm/xen: get_dma_ops: return xen_dma_ops if we are running as xen_initial_domain
arm64/xen: get_dma_ops: return xen_dma_ops if we are running as xen_initial_domain
xen: introduce xen_alloc/free_coherent_pages
swiotlb-xen: use xen_alloc/free_coherent_pages
xen: introduce xen_dma_map/unmap_page and xen_dma_sync_single_for_cpu/device
swiotlb-xen: use xen_dma_map/unmap_page, xen_dma_sync_single_for_cpu/device
swiotlb: print a warning when the swiotlb is full
arm,arm64: do not always merge biovec if we are running on Xen
grant-table: call set_phys_to_machine after mapping grant refs
swiotlb-xen: no need to do execute two bus_to_phys translations in a row
swiotlb-xen: static inline xen_phys_to_bus, xen_bus_to_phys, xen_virt_to_bus and range_straddles_page_boundary
arch/arm/Kconfig | 7 +
arch/arm/include/asm/dma-mapping.h | 50 +++++++-
arch/arm/include/asm/io.h | 8 +
arch/arm/include/asm/xen/hypervisor.h | 2 +
arch/arm/include/asm/xen/page-coherent.h | 50 +++++++
arch/arm/include/asm/xen/page.h | 50 ++++++-
arch/arm/xen/Makefile | 2 +-
arch/arm/xen/mm.c | 65 +++++++++
arch/arm/xen/p2m.c | 208 ++++++++++++++++++++++++++++
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/dma-mapping.h | 14 ++-
arch/arm64/include/asm/io.h | 9 ++
arch/arm64/include/asm/xen/page-coherent.h | 47 +++++++
arch/arm64/xen/Makefile | 2 +-
arch/ia64/include/asm/xen/page-coherent.h | 38 +++++
arch/x86/include/asm/xen/page-coherent.h | 38 +++++
arch/x86/xen/mmu.c | 11 +-
arch/x86/xen/p2m.c | 6 +-
drivers/xen/Kconfig | 1 -
drivers/xen/grant-table.c | 17 ++-
drivers/xen/swiotlb-xen.c | 116 ++++++++++++----
include/xen/swiotlb-xen.h | 2 +
include/xen/xen-ops.h | 7 +-
lib/swiotlb.c | 1 +
sound/soc/samsung/dma.c | 4 +-
25 files changed, 704 insertions(+), 52 deletions(-)
create mode 100644 arch/arm/include/asm/xen/page-coherent.h
create mode 100644 arch/arm/xen/mm.c
create mode 100644 arch/arm/xen/p2m.c
create mode 100644 arch/arm64/include/asm/xen/page-coherent.h
create mode 100644 arch/ia64/include/asm/xen/page-coherent.h
create mode 100644 arch/x86/include/asm/xen/page-coherent.h
git://git.kernel.org/pub/scm/linux/kernel/git/sstabellini/xen.git swiotlb-xen-8
Xen on arm and arm64 needs SWIOTLB_XEN: when running on Xen we need to
program the hardware with mfns rather than pfns for dma addresses.
Remove SWIOTLB_XEN dependency on X86 and PCI and make XEN select
SWIOTLB_XEN on arm and arm64.
At the moment always rely on swiotlb-xen, but when Xen starts supporting
hardware IOMMUs we'll be able to avoid it conditionally on the presence
of an IOMMU on the platform.
Implement xen_create_contiguous_region on arm and arm64: for the moment
we assume that dom0 has been mapped 1:1 (physical addresses == machine
addresses) therefore we don't need to call XENMEM_exchange. Simply
return the physical address as dma address.
Initialize the xen-swiotlb from xen_early_init (before the native
dma_ops are initialized), set xen_dma_ops to &xen_swiotlb_dma_ops.
Signed-off-by: Stefano Stabellini <[email protected]>
Changes in v8:
- assume dom0 is mapped 1:1, no need to call XENMEM_exchange.
Changes in v7:
- call __set_phys_to_machine_multi from xen_create_contiguous_region and
xen_destroy_contiguous_region to update the P2M;
- don't call XENMEM_unpin, it has been removed;
- call XENMEM_exchange instead of XENMEM_exchange_and_pin;
- set nr_exchanged to 0 before calling the hypercall.
Changes in v6:
- introduce and export xen_dma_ops;
- call xen_mm_init from as arch_initcall.
Changes in v4:
- remove redefinition of DMA_ERROR_CODE;
- update the code to use XENMEM_exchange_and_pin and XENMEM_unpin;
- add a note about hardware IOMMU in the commit message.
Changes in v3:
- code style changes;
- warn on XENMEM_put_dma_buf failures.
---
arch/arm/Kconfig | 1 +
arch/arm/include/asm/xen/hypervisor.h | 2 +
arch/arm/include/asm/xen/page.h | 1 +
arch/arm/xen/Makefile | 2 +-
arch/arm/xen/mm.c | 64 +++++++++++++++++++++++++++++++++
arch/arm64/Kconfig | 1 +
arch/arm64/xen/Makefile | 2 +-
drivers/xen/Kconfig | 1 -
drivers/xen/swiotlb-xen.c | 15 ++++++++
9 files changed, 86 insertions(+), 3 deletions(-)
create mode 100644 arch/arm/xen/mm.c
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index c0bfb33..2c9d112 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1848,6 +1848,7 @@ config XEN
depends on CPU_V7 && !CPU_V6
depends on !GENERIC_ATOMIC64
select ARM_PSCI
+ select SWIOTLB_XEN
help
Say Y if you want to run Linux in a Virtual Machine on Xen on ARM.
diff --git a/arch/arm/include/asm/xen/hypervisor.h b/arch/arm/include/asm/xen/hypervisor.h
index d7ab99a..1317ee4 100644
--- a/arch/arm/include/asm/xen/hypervisor.h
+++ b/arch/arm/include/asm/xen/hypervisor.h
@@ -16,4 +16,6 @@ static inline enum paravirt_lazy_mode paravirt_get_lazy_mode(void)
return PARAVIRT_LAZY_NONE;
}
+extern struct dma_map_ops *xen_dma_ops;
+
#endif /* _ASM_ARM_XEN_HYPERVISOR_H */
diff --git a/arch/arm/include/asm/xen/page.h b/arch/arm/include/asm/xen/page.h
index d1b5dd5..5d0e4c5 100644
--- a/arch/arm/include/asm/xen/page.h
+++ b/arch/arm/include/asm/xen/page.h
@@ -6,6 +6,7 @@
#include <linux/pfn.h>
#include <linux/types.h>
+#include <linux/dma-mapping.h>
#include <xen/xen.h>
#include <xen/interface/grant_table.h>
diff --git a/arch/arm/xen/Makefile b/arch/arm/xen/Makefile
index 21e6ff5..1296952 100644
--- a/arch/arm/xen/Makefile
+++ b/arch/arm/xen/Makefile
@@ -1 +1 @@
-obj-y := enlighten.o hypercall.o grant-table.o p2m.o
+obj-y := enlighten.o hypercall.o grant-table.o p2m.o mm.o
diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
new file mode 100644
index 0000000..d56b8c6
--- /dev/null
+++ b/arch/arm/xen/mm.c
@@ -0,0 +1,64 @@
+#include <linux/bootmem.h>
+#include <linux/gfp.h>
+#include <linux/export.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/dma-mapping.h>
+#include <linux/vmalloc.h>
+#include <linux/swiotlb.h>
+
+#include <xen/xen.h>
+#include <xen/interface/memory.h>
+#include <xen/swiotlb-xen.h>
+
+#include <asm/cacheflush.h>
+#include <asm/xen/page.h>
+#include <asm/xen/hypercall.h>
+#include <asm/xen/interface.h>
+
+int xen_create_contiguous_region(unsigned long vstart, unsigned int order,
+ unsigned int address_bits,
+ dma_addr_t *dma_handle)
+{
+ if (!xen_initial_domain())
+ return -EINVAL;
+
+ /* we assume that dom0 is mapped 1:1 for now */
+ *dma_handle = virt_to_phys(pstart);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(xen_create_contiguous_region);
+
+void xen_destroy_contiguous_region(unsigned long vstart, unsigned int order)
+{
+ return;
+}
+EXPORT_SYMBOL_GPL(xen_destroy_contiguous_region);
+
+struct dma_map_ops *xen_dma_ops;
+EXPORT_SYMBOL_GPL(xen_dma_ops);
+
+static struct dma_map_ops xen_swiotlb_dma_ops = {
+ .mapping_error = xen_swiotlb_dma_mapping_error,
+ .alloc = xen_swiotlb_alloc_coherent,
+ .free = xen_swiotlb_free_coherent,
+ .sync_single_for_cpu = xen_swiotlb_sync_single_for_cpu,
+ .sync_single_for_device = xen_swiotlb_sync_single_for_device,
+ .sync_sg_for_cpu = xen_swiotlb_sync_sg_for_cpu,
+ .sync_sg_for_device = xen_swiotlb_sync_sg_for_device,
+ .map_sg = xen_swiotlb_map_sg_attrs,
+ .unmap_sg = xen_swiotlb_unmap_sg_attrs,
+ .map_page = xen_swiotlb_map_page,
+ .unmap_page = xen_swiotlb_unmap_page,
+ .dma_supported = xen_swiotlb_dma_supported,
+};
+
+int __init xen_mm_init(void)
+{
+ if (!xen_initial_domain())
+ return 0;
+ xen_swiotlb_init(1, false);
+ xen_dma_ops = &xen_swiotlb_dma_ops;
+ return 0;
+}
+arch_initcall(xen_mm_init);
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 9737e97..aa1f6fb 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -209,6 +209,7 @@ config XEN_DOM0
config XEN
bool "Xen guest support on ARM64 (EXPERIMENTAL)"
depends on ARM64 && OF
+ select SWIOTLB_XEN
help
Say Y if you want to run Linux in a Virtual Machine on Xen on ARM64.
diff --git a/arch/arm64/xen/Makefile b/arch/arm64/xen/Makefile
index cd866b0..74a8d87 100644
--- a/arch/arm64/xen/Makefile
+++ b/arch/arm64/xen/Makefile
@@ -1,2 +1,2 @@
-xen-arm-y += $(addprefix ../../arm/xen/, enlighten.o grant-table.o p2m.o)
+xen-arm-y += $(addprefix ../../arm/xen/, enlighten.o grant-table.o p2m.o mm.o)
obj-y := xen-arm.o hypercall.o
diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index 9e02d60..7e83688 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -140,7 +140,6 @@ config XEN_GRANT_DEV_ALLOC
config SWIOTLB_XEN
def_bool y
- depends on PCI && X86
select SWIOTLB
config XEN_TMEM
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index b72f31c..f0fc1a4 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -42,12 +42,27 @@
#include <xen/page.h>
#include <xen/xen-ops.h>
#include <xen/hvc-console.h>
+#include <asm/dma-mapping.h>
/*
* Used to do a quick range check in swiotlb_tbl_unmap_single and
* swiotlb_tbl_sync_single_*, to see if the memory was in fact allocated by this
* API.
*/
+#ifndef CONFIG_X86
+static unsigned long dma_alloc_coherent_mask(struct device *dev,
+ gfp_t gfp)
+{
+ unsigned long dma_mask = 0;
+
+ dma_mask = dev->coherent_dma_mask;
+ if (!dma_mask)
+ dma_mask = (gfp & GFP_DMA) ? DMA_BIT_MASK(24) : DMA_BIT_MASK(32);
+
+ return dma_mask;
+}
+#endif
+
static char *xen_io_tlb_start, *xen_io_tlb_end;
static unsigned long xen_io_tlb_nslabs;
/*
--
1.7.2.5
Allow __set_phys_to_machine to be called for autotranslate guests.
It can be used to keep track of phys_to_machine changes, however we
don't do anything with the information at the moment.
Signed-off-by: Stefano Stabellini <[email protected]>
---
arch/x86/xen/p2m.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/xen/p2m.c b/arch/x86/xen/p2m.c
index 95fb2aa..59d9e70 100644
--- a/arch/x86/xen/p2m.c
+++ b/arch/x86/xen/p2m.c
@@ -798,10 +798,10 @@ bool __set_phys_to_machine(unsigned long pfn, unsigned long mfn)
{
unsigned topidx, mididx, idx;
- if (unlikely(xen_feature(XENFEAT_auto_translated_physmap))) {
- BUG_ON(pfn != mfn && mfn != INVALID_P2M_ENTRY);
+ /* don't track P2M changes in autotranslate guests */
+ if (unlikely(xen_feature(XENFEAT_auto_translated_physmap)))
return true;
- }
+
if (unlikely(pfn >= MAX_P2M_PFN)) {
BUG_ON(mfn != INVALID_P2M_ENTRY);
return true;
--
1.7.2.5
We can't simply override arm_dma_ops with xen_dma_ops because devices
are allowed to have their own dma_ops and they take precedence over
arm_dma_ops. When running on Xen as initial domain, we always want
xen_dma_ops to be the one in use.
Signed-off-by: Stefano Stabellini <[email protected]>
Suggested-by: Catalin Marinas <[email protected]>
CC: [email protected]
CC: [email protected]
Changes in v7:
- return xen_dma_ops only if we are the initial domain;
- rename __get_dma_ops to __generic_dma_ops.
---
arch/arm/include/asm/dma-mapping.h | 13 ++++++++++++-
1 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h
index 01b5a3d..f5945d4 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -12,17 +12,28 @@
#include <asm/memory.h>
#include <asm/cacheflush.h>
+#include <xen/xen.h>
+#include <asm/xen/hypervisor.h>
+
#define DMA_ERROR_CODE (~0)
extern struct dma_map_ops arm_dma_ops;
extern struct dma_map_ops arm_coherent_dma_ops;
-static inline struct dma_map_ops *get_dma_ops(struct device *dev)
+static inline struct dma_map_ops *__generic_dma_ops(struct device *dev)
{
if (dev && dev->archdata.dma_ops)
return dev->archdata.dma_ops;
return &arm_dma_ops;
}
+static inline struct dma_map_ops *get_dma_ops(struct device *dev)
+{
+ if (xen_initial_domain())
+ return xen_dma_ops;
+ else
+ return __generic_dma_ops(dev);
+}
+
static inline void set_dma_ops(struct device *dev, struct dma_map_ops *ops)
{
BUG_ON(!dev);
--
1.7.2.5
xen_swiotlb_alloc_coherent needs to allocate a coherent buffer for cpu
and devices. On native x86 is sufficient to call __get_free_pages in
order to get a coherent buffer, while on ARM (and potentially ARM64) we
need to call the native dma_ops->alloc implementation.
Introduce xen_alloc_coherent_pages to abstract the arch specific buffer
allocation.
Similarly introduce xen_free_coherent_pages to free a coherent buffer:
on x86 is simply a call to free_pages while on ARM and ARM64 is
arm_dma_ops.free.
Signed-off-by: Stefano Stabellini <[email protected]>
Changes in v7:
- rename __get_dma_ops to __generic_dma_ops;
- call __generic_dma_ops(hwdev)->alloc/free on arm64 too.
Changes in v6:
- call __get_dma_ops to get the native dma_ops pointer on arm.
---
arch/arm/include/asm/xen/page-coherent.h | 22 ++++++++++++++++++++++
arch/arm64/include/asm/xen/page-coherent.h | 22 ++++++++++++++++++++++
arch/ia64/include/asm/xen/page-coherent.h | 24 ++++++++++++++++++++++++
arch/x86/include/asm/xen/page-coherent.h | 24 ++++++++++++++++++++++++
4 files changed, 92 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/include/asm/xen/page-coherent.h
create mode 100644 arch/arm64/include/asm/xen/page-coherent.h
create mode 100644 arch/ia64/include/asm/xen/page-coherent.h
create mode 100644 arch/x86/include/asm/xen/page-coherent.h
diff --git a/arch/arm/include/asm/xen/page-coherent.h b/arch/arm/include/asm/xen/page-coherent.h
new file mode 100644
index 0000000..c4d843c
--- /dev/null
+++ b/arch/arm/include/asm/xen/page-coherent.h
@@ -0,0 +1,22 @@
+#ifndef _ASM_ARM_XEN_PAGE_COHERENT_H
+#define _ASM_ARM_XEN_PAGE_COHERENT_H
+
+#include <asm/page.h>
+#include <linux/dma-attrs.h>
+#include <linux/dma-mapping.h>
+
+static inline void *xen_alloc_coherent_pages(struct device *hwdev, size_t size,
+ dma_addr_t *dma_handle, gfp_t flags,
+ struct dma_attrs *attrs)
+{
+ return __generic_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,
+ struct dma_attrs *attrs)
+{
+ __generic_dma_ops(hwdev)->free(hwdev, size, cpu_addr, dma_handle, attrs);
+}
+
+#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
new file mode 100644
index 0000000..3b4f029
--- /dev/null
+++ b/arch/arm64/include/asm/xen/page-coherent.h
@@ -0,0 +1,22 @@
+#ifndef _ASM_ARM64_XEN_PAGE_COHERENT_H
+#define _ASM_ARM64_XEN_PAGE_COHERENT_H
+
+#include <asm/page.h>
+#include <linux/dma-attrs.h>
+#include <linux/dma-mapping.h>
+
+static inline void *xen_alloc_coherent_pages(struct device *hwdev, size_t size,
+ dma_addr_t *dma_handle, gfp_t flags,
+ struct dma_attrs *attrs)
+{
+ return __generic_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,
+ struct dma_attrs *attrs)
+{
+ __generic_dma_ops(hwdev)->free(hwdev, size, cpu_addr, dma_handle, attrs);
+}
+
+#endif /* _ASM_ARM64_XEN_PAGE_COHERENT_H */
diff --git a/arch/ia64/include/asm/xen/page-coherent.h b/arch/ia64/include/asm/xen/page-coherent.h
new file mode 100644
index 0000000..37b929c
--- /dev/null
+++ b/arch/ia64/include/asm/xen/page-coherent.h
@@ -0,0 +1,24 @@
+#ifndef _ASM_IA64_XEN_PAGE_COHERENT_H
+#define _ASM_IA64_XEN_PAGE_COHERENT_H
+
+#include <asm/page.h>
+#include <linux/dma-attrs.h>
+#include <linux/dma-mapping.h>
+
+static inline void *xen_alloc_coherent_pages(struct device *hwdev, size_t size,
+ dma_addr_t *dma_handle, gfp_t flags,
+ struct dma_attrs *attrs)
+{
+ void *vstart = (void*)__get_free_pages(flags, get_order(size));
+ *dma_handle = virt_to_phys(vstart);
+ return vstart;
+}
+
+static inline void xen_free_coherent_pages(struct device *hwdev, size_t size,
+ void *cpu_addr, dma_addr_t dma_handle,
+ struct dma_attrs *attrs)
+{
+ free_pages((unsigned long) cpu_addr, get_order(size));
+}
+
+#endif /* _ASM_IA64_XEN_PAGE_COHERENT_H */
diff --git a/arch/x86/include/asm/xen/page-coherent.h b/arch/x86/include/asm/xen/page-coherent.h
new file mode 100644
index 0000000..31de2e0
--- /dev/null
+++ b/arch/x86/include/asm/xen/page-coherent.h
@@ -0,0 +1,24 @@
+#ifndef _ASM_X86_XEN_PAGE_COHERENT_H
+#define _ASM_X86_XEN_PAGE_COHERENT_H
+
+#include <asm/page.h>
+#include <linux/dma-attrs.h>
+#include <linux/dma-mapping.h>
+
+static inline void *xen_alloc_coherent_pages(struct device *hwdev, size_t size,
+ dma_addr_t *dma_handle, gfp_t flags,
+ struct dma_attrs *attrs)
+{
+ void *vstart = (void*)__get_free_pages(flags, get_order(size));
+ *dma_handle = virt_to_phys(vstart);
+ return vstart;
+}
+
+static inline void xen_free_coherent_pages(struct device *hwdev, size_t size,
+ void *cpu_addr, dma_addr_t dma_handle,
+ struct dma_attrs *attrs)
+{
+ free_pages((unsigned long) cpu_addr, get_order(size));
+}
+
+#endif /* _ASM_X86_XEN_PAGE_COHERENT_H */
--
1.7.2.5
Introduce physical to machine and machine to physical tracking
mechanisms based on rbtrees for arm/xen and arm64/xen.
We need it because any guests on ARM are an autotranslate guests,
therefore a physical address is potentially different from a machine
address. When programming a device to do DMA, we need to be
extra-careful to use machine addresses rather than physical addresses to
program the device. Therefore we need to know the physical to machine
mappings.
For the moment we assume that dom0 starts with a 1:1 physical to machine
mapping, in other words physical addresses correspond to machine
addresses. However when mapping a foreign grant reference, obviously the
1:1 model doesn't work anymore. So at the very least we need to be able
to track grant mappings.
We need locking to protect accesses to the two trees.
Signed-off-by: Stefano Stabellini <[email protected]>
Changes in v8:
- move pfn_to_mfn and mfn_to_pfn to page.h as static inline functions;
- no need to walk the tree if phys_to_mach.rb_node is NULL;
- correctly handle multipage p2m entries;
- substitute the spin_lock with a rwlock.
---
arch/arm/include/asm/xen/page.h | 49 ++++++++--
arch/arm/xen/Makefile | 2 +-
arch/arm/xen/p2m.c | 208 +++++++++++++++++++++++++++++++++++++++
arch/arm64/xen/Makefile | 2 +-
4 files changed, 252 insertions(+), 9 deletions(-)
create mode 100644 arch/arm/xen/p2m.c
diff --git a/arch/arm/include/asm/xen/page.h b/arch/arm/include/asm/xen/page.h
index 359a7b5..d1b5dd5 100644
--- a/arch/arm/include/asm/xen/page.h
+++ b/arch/arm/include/asm/xen/page.h
@@ -7,11 +7,10 @@
#include <linux/pfn.h>
#include <linux/types.h>
+#include <xen/xen.h>
#include <xen/interface/grant_table.h>
-#define pfn_to_mfn(pfn) (pfn)
#define phys_to_machine_mapping_valid(pfn) (1)
-#define mfn_to_pfn(mfn) (mfn)
#define mfn_to_virt(m) (__va(mfn_to_pfn(m) << PAGE_SHIFT))
#define pte_mfn pte_pfn
@@ -32,6 +31,44 @@ typedef struct xpaddr {
#define INVALID_P2M_ENTRY (~0UL)
+unsigned long __pfn_to_mfn(unsigned long pfn);
+unsigned long __mfn_to_pfn(unsigned long mfn);
+extern struct rb_root phys_to_mach;
+
+static inline unsigned long pfn_to_mfn(unsigned long pfn)
+{
+ unsigned long mfn;
+
+ if (phys_to_mach.rb_node != NULL) {
+ mfn = __pfn_to_mfn(pfn);
+ if (mfn != INVALID_P2M_ENTRY)
+ return mfn;
+ }
+
+ if (xen_initial_domain())
+ return pfn;
+ else
+ return INVALID_P2M_ENTRY;
+}
+
+static inline unsigned long mfn_to_pfn(unsigned long mfn)
+{
+ unsigned long pfn;
+
+ if (phys_to_mach.rb_node != NULL) {
+ pfn = __mfn_to_pfn(mfn);
+ if (pfn != INVALID_P2M_ENTRY)
+ return pfn;
+ }
+
+ if (xen_initial_domain())
+ return mfn;
+ else
+ return INVALID_P2M_ENTRY;
+}
+
+#define mfn_to_local_pfn(mfn) mfn_to_pfn(mfn)
+
static inline xmaddr_t phys_to_machine(xpaddr_t phys)
{
unsigned offset = phys.paddr & ~PAGE_MASK;
@@ -76,11 +113,9 @@ static inline int m2p_remove_override(struct page *page, bool clear_pte)
return 0;
}
-static inline bool __set_phys_to_machine(unsigned long pfn, unsigned long mfn)
-{
- BUG_ON(pfn != mfn && mfn != INVALID_P2M_ENTRY);
- return true;
-}
+bool __set_phys_to_machine(unsigned long pfn, unsigned long mfn);
+bool __set_phys_to_machine_multi(unsigned long pfn, unsigned long mfn,
+ unsigned long nr_pages);
static inline bool set_phys_to_machine(unsigned long pfn, unsigned long mfn)
{
diff --git a/arch/arm/xen/Makefile b/arch/arm/xen/Makefile
index 4384103..21e6ff5 100644
--- a/arch/arm/xen/Makefile
+++ b/arch/arm/xen/Makefile
@@ -1 +1 @@
-obj-y := enlighten.o hypercall.o grant-table.o
+obj-y := enlighten.o hypercall.o grant-table.o p2m.o
diff --git a/arch/arm/xen/p2m.c b/arch/arm/xen/p2m.c
new file mode 100644
index 0000000..5df4a9a
--- /dev/null
+++ b/arch/arm/xen/p2m.c
@@ -0,0 +1,208 @@
+#include <linux/bootmem.h>
+#include <linux/gfp.h>
+#include <linux/export.h>
+#include <linux/rwlock.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/dma-mapping.h>
+#include <linux/vmalloc.h>
+#include <linux/swiotlb.h>
+
+#include <xen/xen.h>
+#include <xen/interface/memory.h>
+#include <xen/swiotlb-xen.h>
+
+#include <asm/cacheflush.h>
+#include <asm/xen/page.h>
+#include <asm/xen/hypercall.h>
+#include <asm/xen/interface.h>
+
+struct xen_p2m_entry {
+ unsigned long pfn;
+ unsigned long mfn;
+ unsigned long nr_pages;
+ struct rb_node rbnode_mach;
+ struct rb_node rbnode_phys;
+};
+
+rwlock_t p2m_lock;
+struct rb_root phys_to_mach = RB_ROOT;
+static struct rb_root mach_to_phys = RB_ROOT;
+
+static int xen_add_phys_to_mach_entry(struct xen_p2m_entry *new)
+{
+ struct rb_node **link = &phys_to_mach.rb_node;
+ struct rb_node *parent = NULL;
+ struct xen_p2m_entry *entry;
+ int rc = 0;
+
+ while (*link) {
+ parent = *link;
+ entry = rb_entry(parent, struct xen_p2m_entry, rbnode_phys);
+
+ if (new->mfn == entry->mfn)
+ goto err_out;
+ if (new->pfn == entry->pfn)
+ goto err_out;
+
+ if (new->pfn < entry->pfn)
+ link = &(*link)->rb_left;
+ else
+ link = &(*link)->rb_right;
+ }
+ rb_link_node(&new->rbnode_phys, parent, link);
+ rb_insert_color(&new->rbnode_phys, &phys_to_mach);
+ goto out;
+
+err_out:
+ rc = -EINVAL;
+ pr_warn("%s: cannot add pfn=%pa -> mfn=%pa: pfn=%pa -> mfn=%pa already exists\n",
+ __func__, &new->pfn, &new->mfn, &entry->pfn, &entry->mfn);
+out:
+ return rc;
+}
+
+unsigned long __pfn_to_mfn(unsigned long pfn)
+{
+ struct rb_node *n = phys_to_mach.rb_node;
+ struct xen_p2m_entry *entry;
+ unsigned long irqflags;
+
+ read_lock_irqsave(&p2m_lock, irqflags);
+ while (n) {
+ entry = rb_entry(n, struct xen_p2m_entry, rbnode_phys);
+ if (entry->pfn <= pfn &&
+ entry->pfn + entry->nr_pages > pfn) {
+ read_unlock_irqrestore(&p2m_lock, irqflags);
+ return entry->mfn + (pfn - entry->pfn);
+ }
+ if (pfn < entry->pfn)
+ n = n->rb_left;
+ else
+ n = n->rb_right;
+ }
+ read_unlock_irqrestore(&p2m_lock, irqflags);
+
+ return INVALID_P2M_ENTRY;
+}
+EXPORT_SYMBOL_GPL(__pfn_to_mfn);
+
+static int xen_add_mach_to_phys_entry(struct xen_p2m_entry *new)
+{
+ struct rb_node **link = &mach_to_phys.rb_node;
+ struct rb_node *parent = NULL;
+ struct xen_p2m_entry *entry;
+ int rc = 0;
+
+ while (*link) {
+ parent = *link;
+ entry = rb_entry(parent, struct xen_p2m_entry, rbnode_mach);
+
+ if (new->mfn == entry->mfn)
+ goto err_out;
+ if (new->pfn == entry->pfn)
+ goto err_out;
+
+ if (new->mfn < entry->mfn)
+ link = &(*link)->rb_left;
+ else
+ link = &(*link)->rb_right;
+ }
+ rb_link_node(&new->rbnode_mach, parent, link);
+ rb_insert_color(&new->rbnode_mach, &mach_to_phys);
+ goto out;
+
+err_out:
+ rc = -EINVAL;
+ pr_warn("%s: cannot add pfn=%pa -> mfn=%pa: pfn=%pa -> mfn=%pa already exists\n",
+ __func__, &new->pfn, &new->mfn, &entry->pfn, &entry->mfn);
+out:
+ return rc;
+}
+
+unsigned long __mfn_to_pfn(unsigned long mfn)
+{
+ struct rb_node *n = mach_to_phys.rb_node;
+ struct xen_p2m_entry *entry;
+ unsigned long irqflags;
+
+ read_lock_irqsave(&p2m_lock, irqflags);
+ while (n) {
+ entry = rb_entry(n, struct xen_p2m_entry, rbnode_mach);
+ if (entry->mfn <= mfn &&
+ entry->mfn + entry->nr_pages > mfn) {
+ read_unlock_irqrestore(&p2m_lock, irqflags);
+ return entry->pfn + (mfn - entry->mfn);
+ }
+ if (mfn < entry->mfn)
+ n = n->rb_left;
+ else
+ n = n->rb_right;
+ }
+ read_unlock_irqrestore(&p2m_lock, irqflags);
+
+ return INVALID_P2M_ENTRY;
+}
+EXPORT_SYMBOL_GPL(__mfn_to_pfn);
+
+bool __set_phys_to_machine_multi(unsigned long pfn,
+ unsigned long mfn, unsigned long nr_pages)
+{
+ int rc;
+ unsigned long irqflags;
+ struct xen_p2m_entry *p2m_entry;
+ struct rb_node *n = phys_to_mach.rb_node;
+
+ if (mfn == INVALID_P2M_ENTRY) {
+ write_lock_irqsave(&p2m_lock, irqflags);
+ while (n) {
+ p2m_entry = rb_entry(n, struct xen_p2m_entry, rbnode_phys);
+ if (p2m_entry->pfn <= pfn &&
+ p2m_entry->pfn + p2m_entry->nr_pages > pfn) {
+ rb_erase(&p2m_entry->rbnode_mach, &mach_to_phys);
+ rb_erase(&p2m_entry->rbnode_phys, &phys_to_mach);
+ write_unlock_irqrestore(&p2m_lock, irqflags);
+ kfree(p2m_entry);
+ return true;
+ }
+ if (pfn < p2m_entry->pfn)
+ n = n->rb_left;
+ else
+ n = n->rb_right;
+ }
+ write_unlock_irqrestore(&p2m_lock, irqflags);
+ return true;
+ }
+
+ p2m_entry = kzalloc(sizeof(struct xen_p2m_entry), GFP_NOWAIT);
+ if (!p2m_entry) {
+ pr_warn("cannot allocate xen_p2m_entry\n");
+ return false;
+ }
+ p2m_entry->pfn = pfn;
+ p2m_entry->nr_pages = nr_pages;
+ p2m_entry->mfn = mfn;
+
+ write_lock_irqsave(&p2m_lock, irqflags);
+ if ((rc = xen_add_phys_to_mach_entry(p2m_entry) < 0) ||
+ (rc = xen_add_mach_to_phys_entry(p2m_entry) < 0)) {
+ write_unlock_irqrestore(&p2m_lock, irqflags);
+ return false;
+ }
+ write_unlock_irqrestore(&p2m_lock, irqflags);
+ return true;
+}
+EXPORT_SYMBOL_GPL(__set_phys_to_machine_multi);
+
+bool __set_phys_to_machine(unsigned long pfn, unsigned long mfn)
+{
+ return __set_phys_to_machine_multi(pfn, mfn, 1);
+}
+EXPORT_SYMBOL_GPL(__set_phys_to_machine);
+
+int p2m_init(void)
+{
+ rwlock_init(&p2m_lock);
+ return 0;
+}
+arch_initcall(p2m_init);
diff --git a/arch/arm64/xen/Makefile b/arch/arm64/xen/Makefile
index be24040..cd866b0 100644
--- a/arch/arm64/xen/Makefile
+++ b/arch/arm64/xen/Makefile
@@ -1,2 +1,2 @@
-xen-arm-y += $(addprefix ../../arm/xen/, enlighten.o grant-table.o)
+xen-arm-y += $(addprefix ../../arm/xen/, enlighten.o grant-table.o p2m.o)
obj-y := xen-arm.o hypercall.o
--
1.7.2.5
IOMMU_HELPER is needed because SWIOTLB calls iommu_is_span_boundary,
provided by lib/iommu_helper.c.
Signed-off-by: Stefano Stabellini <[email protected]>
Reviewed-by: Konrad Rzeszutek Wilk <[email protected]>
CC: [email protected]
CC: [email protected]
Changes in v8:
- use __phys_to_pfn and __pfn_to_phys.
Changes in v7:
- dma_mark_clean: empty implementation;
- in dma_capable use coherent_dma_mask if dma_mask hasn't been
allocated.
Changes in v6:
- check for dev->dma_mask being NULL in dma_capable.
Changes in v5:
- implement dma_mark_clean using dmac_flush_range.
Changes in v3:
- dma_capable: do not treat dma_mask as a limit;
- remove SWIOTLB dependency on NEED_SG_DMA_LENGTH.
---
arch/arm/Kconfig | 6 +++++
arch/arm/include/asm/dma-mapping.h | 37 ++++++++++++++++++++++++++++++++++++
2 files changed, 43 insertions(+), 0 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index ba412e0..c0bfb33 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1832,6 +1832,12 @@ config CC_STACKPROTECTOR
neutralized via a kernel panic.
This feature requires gcc version 4.2 or above.
+config SWIOTLB
+ def_bool y
+
+config IOMMU_HELPER
+ def_bool SWIOTLB
+
config XEN_DOM0
def_bool y
depends on XEN
diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h
index 5b579b9..01b5a3d 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -10,6 +10,7 @@
#include <asm-generic/dma-coherent.h>
#include <asm/memory.h>
+#include <asm/cacheflush.h>
#define DMA_ERROR_CODE (~0)
extern struct dma_map_ops arm_dma_ops;
@@ -86,6 +87,42 @@ static inline dma_addr_t virt_to_dma(struct device *dev, void *addr)
}
#endif
+static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
+{
+ unsigned int offset = paddr & ~PAGE_MASK;
+ return pfn_to_dma(dev, __phys_to_pfn(paddr)) + offset;
+}
+
+static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t dev_addr)
+{
+ unsigned int offset = dev_addr & ~PAGE_MASK;
+ return __pfn_to_phys(dma_to_pfn(dev, dev_addr)) + offset;
+}
+
+static inline bool dma_capable(struct device *dev, dma_addr_t addr, size_t size)
+{
+ u64 limit, mask;
+
+ if (dev->dma_mask)
+ mask = *dev->dma_mask;
+ else
+ mask = dev->coherent_dma_mask;
+
+ if (mask == 0)
+ return 0;
+
+ limit = (mask + 1) & ~mask;
+ if (limit && size > limit)
+ return 0;
+
+ if ((addr | (addr + size - 1)) & ~mask)
+ return 0;
+
+ return 1;
+}
+
+static inline void dma_mark_clean(void *addr, size_t size) { }
+
/*
* DMA errors are defined by all-bits-set in the DMA address.
*/
--
1.7.2.5
We can't simply override arm_dma_ops with xen_dma_ops because devices
are allowed to have their own dma_ops and they take precedence over
arm_dma_ops. When running on Xen as initial domain, we always want
xen_dma_ops to be the one in use.
Signed-off-by: Stefano Stabellini <[email protected]>
Acked-by: Catalin Marinas <[email protected]>
CC: [email protected]
Changes in v7:
- return xen_dma_ops only if we are the initial domain;
- rename __get_dma_ops to __generic_dma_ops.
---
arch/arm64/include/asm/dma-mapping.h | 13 ++++++++++++-
1 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/arch/arm64/include/asm/dma-mapping.h b/arch/arm64/include/asm/dma-mapping.h
index 59206b1..fd0c0c0 100644
--- a/arch/arm64/include/asm/dma-mapping.h
+++ b/arch/arm64/include/asm/dma-mapping.h
@@ -23,12 +23,15 @@
#include <asm-generic/dma-coherent.h>
+#include <xen/xen.h>
+#include <asm/xen/hypervisor.h>
+
#define ARCH_HAS_DMA_GET_REQUIRED_MASK
#define DMA_ERROR_CODE (~(dma_addr_t)0)
extern struct dma_map_ops *dma_ops;
-static inline struct dma_map_ops *get_dma_ops(struct device *dev)
+static inline struct dma_map_ops *__generic_dma_ops(struct device *dev)
{
if (unlikely(!dev) || !dev->archdata.dma_ops)
return dma_ops;
@@ -36,6 +39,14 @@ static inline struct dma_map_ops *get_dma_ops(struct device *dev)
return dev->archdata.dma_ops;
}
+static inline struct dma_map_ops *get_dma_ops(struct device *dev)
+{
+ if (xen_initial_domain())
+ return xen_dma_ops;
+ else
+ return __generic_dma_ops(dev);
+}
+
#include <asm-generic/dma-mapping-common.h>
static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
--
1.7.2.5
Modify xen_create_contiguous_region to return the dma address of the
newly contiguous buffer.
Signed-off-by: Stefano Stabellini <[email protected]>
Acked-by: Konrad Rzeszutek Wilk <[email protected]>
Reviewed-by: David Vrabel <[email protected]>
Changes in v4:
- use virt_to_machine instead of virt_to_bus.
---
arch/x86/xen/mmu.c | 4 +++-
drivers/xen/swiotlb-xen.c | 6 +++---
include/xen/xen-ops.h | 3 ++-
3 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
index fdc3ba2..6c34d7c 100644
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -2329,7 +2329,8 @@ static int xen_exchange_memory(unsigned long extents_in, unsigned int order_in,
}
int xen_create_contiguous_region(unsigned long vstart, unsigned int order,
- unsigned int address_bits)
+ unsigned int address_bits,
+ dma_addr_t *dma_handle)
{
unsigned long *in_frames = discontig_frames, out_frame;
unsigned long flags;
@@ -2368,6 +2369,7 @@ int xen_create_contiguous_region(unsigned long vstart, unsigned int order,
spin_unlock_irqrestore(&xen_reservation_lock, flags);
+ *dma_handle = virt_to_machine(vstart).maddr;
return success ? 0 : -ENOMEM;
}
EXPORT_SYMBOL_GPL(xen_create_contiguous_region);
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 1b2277c..b72f31c 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -126,6 +126,7 @@ xen_swiotlb_fixup(void *buf, size_t size, unsigned long nslabs)
{
int i, rc;
int dma_bits;
+ dma_addr_t dma_handle;
dma_bits = get_order(IO_TLB_SEGSIZE << IO_TLB_SHIFT) + PAGE_SHIFT;
@@ -137,7 +138,7 @@ xen_swiotlb_fixup(void *buf, size_t size, unsigned long nslabs)
rc = xen_create_contiguous_region(
(unsigned long)buf + (i << IO_TLB_SHIFT),
get_order(slabs << IO_TLB_SHIFT),
- dma_bits);
+ dma_bits, &dma_handle);
} while (rc && dma_bits++ < max_dma_bits);
if (rc)
return rc;
@@ -294,11 +295,10 @@ xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
*dma_handle = dev_addr;
else {
if (xen_create_contiguous_region(vstart, order,
- fls64(dma_mask)) != 0) {
+ fls64(dma_mask), dma_handle) != 0) {
free_pages(vstart, order);
return NULL;
}
- *dma_handle = virt_to_machine(ret).maddr;
}
memset(ret, 0, size);
return ret;
diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
index d6fe062..9ef704d 100644
--- a/include/xen/xen-ops.h
+++ b/include/xen/xen-ops.h
@@ -20,7 +20,8 @@ int xen_setup_shutdown_event(void);
extern unsigned long *xen_contiguous_bitmap;
int xen_create_contiguous_region(unsigned long vstart, unsigned int order,
- unsigned int address_bits);
+ unsigned int address_bits,
+ dma_addr_t *dma_handle);
void xen_destroy_contiguous_region(unsigned long vstart, unsigned int order);
--
1.7.2.5
Implement xen_swiotlb_set_dma_mask, use it for set_dma_mask on arm.
Signed-off-by: Stefano Stabellini <[email protected]>
---
arch/arm/xen/mm.c | 1 +
drivers/xen/swiotlb-xen.c | 12 ++++++++++++
include/xen/swiotlb-xen.h | 2 ++
3 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
index d56b8c6..0d69b87 100644
--- a/arch/arm/xen/mm.c
+++ b/arch/arm/xen/mm.c
@@ -51,6 +51,7 @@ static struct dma_map_ops xen_swiotlb_dma_ops = {
.map_page = xen_swiotlb_map_page,
.unmap_page = xen_swiotlb_unmap_page,
.dma_supported = xen_swiotlb_dma_supported,
+ .set_dma_mask = xen_swiotlb_set_dma_mask,
};
int __init xen_mm_init(void)
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index f0fc1a4..d8ef0bf 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -608,3 +608,15 @@ xen_swiotlb_dma_supported(struct device *hwdev, u64 mask)
return xen_virt_to_bus(xen_io_tlb_end - 1) <= mask;
}
EXPORT_SYMBOL_GPL(xen_swiotlb_dma_supported);
+
+int
+xen_swiotlb_set_dma_mask(struct device *dev, u64 dma_mask)
+{
+ if (!dev->dma_mask || !xen_swiotlb_dma_supported(dev, dma_mask))
+ return -EIO;
+
+ *dev->dma_mask = dma_mask;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(xen_swiotlb_set_dma_mask);
diff --git a/include/xen/swiotlb-xen.h b/include/xen/swiotlb-xen.h
index de8bcc6..7b64465 100644
--- a/include/xen/swiotlb-xen.h
+++ b/include/xen/swiotlb-xen.h
@@ -55,4 +55,6 @@ xen_swiotlb_dma_mapping_error(struct device *hwdev, dma_addr_t dma_addr);
extern int
xen_swiotlb_dma_supported(struct device *hwdev, u64 mask);
+extern int
+xen_swiotlb_set_dma_mask(struct device *dev, u64 dma_mask);
#endif /* __LINUX_SWIOTLB_XEN_H */
--
1.7.2.5
Signed-off-by: Stefano Stabellini <[email protected]>
Acked-by: Catalin Marinas <[email protected]>
CC: [email protected]
Changes in v8:
- cast to dma_addr_t before returning.
---
arch/arm64/include/asm/dma-mapping.h | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/arch/arm64/include/asm/dma-mapping.h b/arch/arm64/include/asm/dma-mapping.h
index 8d18100..59206b1 100644
--- a/arch/arm64/include/asm/dma-mapping.h
+++ b/arch/arm64/include/asm/dma-mapping.h
@@ -25,6 +25,7 @@
#define ARCH_HAS_DMA_GET_REQUIRED_MASK
+#define DMA_ERROR_CODE (~(dma_addr_t)0)
extern struct dma_map_ops *dma_ops;
static inline struct dma_map_ops *get_dma_ops(struct device *dev)
--
1.7.2.5
xen_unmap_single calls xen_bus_to_phys, then passes dev_addr to
is_xen_swiotlb_buffer that calls xen_bus_to_phys again.
xen_bus_to_phys is potentially a slow operation that might involve
walking a tree.
We can avoid calling xen_bus_to_phys twice by removing the
is_xen_swiotlb_buffer call and doing the check directly in
xen_unmap_single using the physical address.
Signed-off-by: Stefano Stabellini <[email protected]>
---
drivers/xen/swiotlb-xen.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 4d50058..6d9ca61 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -426,8 +426,8 @@ static void xen_unmap_single(struct device *hwdev, dma_addr_t dev_addr,
xen_dma_unmap_page(hwdev, paddr, size, dir, attrs);
- /* NOTE: We use dev_addr here, not paddr! */
- if (is_xen_swiotlb_buffer(dev_addr)) {
+ if (paddr >= virt_to_phys(xen_io_tlb_start) &&
+ paddr < virt_to_phys(xen_io_tlb_end)) {
swiotlb_tbl_unmap_single(hwdev, paddr, size, dir);
return;
}
--
1.7.2.5
This is similar to what it is done on X86: biovecs are prevented from merging
otherwise every dma requests would be forced to bounce on the swiotlb buffer.
Signed-off-by: Stefano Stabellini <[email protected]>
Changes in v7:
- remove the extra autotranslate check in biomerge.c.
---
arch/arm/include/asm/io.h | 8 ++++++++
arch/arm64/include/asm/io.h | 9 +++++++++
2 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
index d070741..c45effc 100644
--- a/arch/arm/include/asm/io.h
+++ b/arch/arm/include/asm/io.h
@@ -24,9 +24,11 @@
#ifdef __KERNEL__
#include <linux/types.h>
+#include <linux/blk_types.h>
#include <asm/byteorder.h>
#include <asm/memory.h>
#include <asm-generic/pci_iomap.h>
+#include <xen/xen.h>
/*
* ISA I/O bus memory addresses are 1:1 with the physical address.
@@ -372,6 +374,12 @@ extern void pci_iounmap(struct pci_dev *dev, void __iomem *addr);
#define BIOVEC_MERGEABLE(vec1, vec2) \
((bvec_to_phys((vec1)) + (vec1)->bv_len) == bvec_to_phys((vec2)))
+extern bool xen_biovec_phys_mergeable(const struct bio_vec *vec1,
+ const struct bio_vec *vec2);
+#define BIOVEC_PHYS_MERGEABLE(vec1, vec2) \
+ (__BIOVEC_PHYS_MERGEABLE(vec1, vec2) && \
+ (!xen_domain() || xen_biovec_phys_mergeable(vec1, vec2)))
+
#ifdef CONFIG_MMU
#define ARCH_HAS_VALID_PHYS_ADDR_RANGE
extern int valid_phys_addr_range(phys_addr_t addr, size_t size);
diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
index 1d12f89..c163287b 100644
--- a/arch/arm64/include/asm/io.h
+++ b/arch/arm64/include/asm/io.h
@@ -22,11 +22,14 @@
#ifdef __KERNEL__
#include <linux/types.h>
+#include <linux/blk_types.h>
#include <asm/byteorder.h>
#include <asm/barrier.h>
#include <asm/pgtable.h>
+#include <xen/xen.h>
+
/*
* Generic IO read/write. These perform native-endian accesses.
*/
@@ -263,5 +266,11 @@ extern int devmem_is_allowed(unsigned long pfn);
*/
#define xlate_dev_kmem_ptr(p) p
+extern bool xen_biovec_phys_mergeable(const struct bio_vec *vec1,
+ const struct bio_vec *vec2);
+#define BIOVEC_PHYS_MERGEABLE(vec1, vec2) \
+ (__BIOVEC_PHYS_MERGEABLE(vec1, vec2) && \
+ (!xen_domain() || xen_biovec_phys_mergeable(vec1, vec2)))
+
#endif /* __KERNEL__ */
#endif /* __ASM_IO_H */
--
1.7.2.5
When mapping/unmapping grant refs, call
set_phys_to_machine to update the P2M with the new mappings for
autotranslate guests.
Signed-off-by: Stefano Stabellini <[email protected]>
---
drivers/xen/grant-table.c | 17 +++++++++++++++--
1 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index 04cdeb8..a36cb03 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -49,6 +49,7 @@
#include <xen/grant_table.h>
#include <xen/interface/memory.h>
#include <xen/hvc-console.h>
+#include <xen/swiotlb-xen.h>
#include <asm/xen/hypercall.h>
#include <asm/xen/interface.h>
@@ -889,8 +890,15 @@ int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops,
gnttab_retry_eagain_gop(GNTTABOP_map_grant_ref, map_ops + i,
&map_ops[i].status, __func__);
- if (xen_feature(XENFEAT_auto_translated_physmap))
+ if (xen_feature(XENFEAT_auto_translated_physmap)) {
+ for (i = 0; i < count; i++) {
+ if (map_ops[i].status)
+ continue;
+ set_phys_to_machine(map_ops[i].host_addr >> PAGE_SHIFT,
+ map_ops[i].dev_bus_addr >> PAGE_SHIFT);
+ }
return ret;
+ }
if (!in_interrupt() && paravirt_get_lazy_mode() == PARAVIRT_LAZY_NONE) {
arch_enter_lazy_mmu_mode();
@@ -933,8 +941,13 @@ int gnttab_unmap_refs(struct gnttab_unmap_grant_ref *unmap_ops,
if (ret)
return ret;
- if (xen_feature(XENFEAT_auto_translated_physmap))
+ if (xen_feature(XENFEAT_auto_translated_physmap)) {
+ for (i = 0; i < count; i++) {
+ set_phys_to_machine(unmap_ops[i].host_addr >> PAGE_SHIFT,
+ INVALID_P2M_ENTRY);
+ }
return ret;
+ }
if (!in_interrupt() && paravirt_get_lazy_mode() == PARAVIRT_LAZY_NONE) {
arch_enter_lazy_mmu_mode();
--
1.7.2.5
Use xen_alloc_coherent_pages and xen_free_coherent_pages to allocate or
free coherent pages.
We need to be careful handling the pointer returned by
xen_alloc_coherent_pages, because on ARM the pointer is not equal to
phys_to_virt(*dma_handle). In fact virt_to_phys only works for kernel
direct mapped RAM memory.
In ARM case the pointer could be an ioremap address, therefore passing
it to virt_to_phys would give you another physical address that doesn't
correspond to it.
Make xen_create_contiguous_region take a phys_addr_t as start parameter to
avoid the virt_to_phys calls which would be incorrect.
Changes in v6:
- remove extra spaces.
Signed-off-by: Stefano Stabellini <[email protected]>
Reviewed-by: Konrad Rzeszutek Wilk <[email protected]>
---
arch/arm/xen/mm.c | 6 +++---
arch/x86/xen/mmu.c | 7 +++++--
drivers/xen/swiotlb-xen.c | 31 +++++++++++++++++++++----------
include/xen/xen-ops.h | 4 ++--
4 files changed, 31 insertions(+), 17 deletions(-)
diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
index 0d69b87..b0e77de 100644
--- a/arch/arm/xen/mm.c
+++ b/arch/arm/xen/mm.c
@@ -16,7 +16,7 @@
#include <asm/xen/hypercall.h>
#include <asm/xen/interface.h>
-int xen_create_contiguous_region(unsigned long vstart, unsigned int order,
+int xen_create_contiguous_region(phys_addr_t pstart, unsigned int order,
unsigned int address_bits,
dma_addr_t *dma_handle)
{
@@ -24,12 +24,12 @@ int xen_create_contiguous_region(unsigned long vstart, unsigned int order,
return -EINVAL;
/* we assume that dom0 is mapped 1:1 for now */
- *dma_handle = virt_to_phys(pstart);
+ *dma_handle = pstart;
return 0;
}
EXPORT_SYMBOL_GPL(xen_create_contiguous_region);
-void xen_destroy_contiguous_region(unsigned long vstart, unsigned int order)
+void xen_destroy_contiguous_region(phys_addr_t pstart, unsigned int order)
{
return;
}
diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
index 6c34d7c..8830883 100644
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -2328,13 +2328,14 @@ static int xen_exchange_memory(unsigned long extents_in, unsigned int order_in,
return success;
}
-int xen_create_contiguous_region(unsigned long vstart, unsigned int order,
+int xen_create_contiguous_region(phys_addr_t pstart, unsigned int order,
unsigned int address_bits,
dma_addr_t *dma_handle)
{
unsigned long *in_frames = discontig_frames, out_frame;
unsigned long flags;
int success;
+ unsigned long vstart = (unsigned long)phys_to_virt(pstart);
/*
* Currently an auto-translated guest will not perform I/O, nor will
@@ -2374,11 +2375,12 @@ int xen_create_contiguous_region(unsigned long vstart, unsigned int order,
}
EXPORT_SYMBOL_GPL(xen_create_contiguous_region);
-void xen_destroy_contiguous_region(unsigned long vstart, unsigned int order)
+void xen_destroy_contiguous_region(phys_addr_t pstart, unsigned int order)
{
unsigned long *out_frames = discontig_frames, in_frame;
unsigned long flags;
int success;
+ unsigned long vstart;
if (xen_feature(XENFEAT_auto_translated_physmap))
return;
@@ -2386,6 +2388,7 @@ void xen_destroy_contiguous_region(unsigned long vstart, unsigned int order)
if (unlikely(order > MAX_CONTIG_ORDER))
return;
+ vstart = (unsigned long)phys_to_virt(pstart);
memset((void *) vstart, 0, PAGE_SIZE << order);
spin_lock_irqsave(&xen_reservation_lock, flags);
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index d8ef0bf..189b8db 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -43,6 +43,7 @@
#include <xen/xen-ops.h>
#include <xen/hvc-console.h>
#include <asm/dma-mapping.h>
+#include <asm/xen/page-coherent.h>
/*
* Used to do a quick range check in swiotlb_tbl_unmap_single and
* swiotlb_tbl_sync_single_*, to see if the memory was in fact allocated by this
@@ -142,6 +143,7 @@ xen_swiotlb_fixup(void *buf, size_t size, unsigned long nslabs)
int i, rc;
int dma_bits;
dma_addr_t dma_handle;
+ phys_addr_t p = virt_to_phys(buf);
dma_bits = get_order(IO_TLB_SEGSIZE << IO_TLB_SHIFT) + PAGE_SHIFT;
@@ -151,7 +153,7 @@ xen_swiotlb_fixup(void *buf, size_t size, unsigned long nslabs)
do {
rc = xen_create_contiguous_region(
- (unsigned long)buf + (i << IO_TLB_SHIFT),
+ p + (i << IO_TLB_SHIFT),
get_order(slabs << IO_TLB_SHIFT),
dma_bits, &dma_handle);
} while (rc && dma_bits++ < max_dma_bits);
@@ -279,7 +281,6 @@ xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
void *ret;
int order = get_order(size);
u64 dma_mask = DMA_BIT_MASK(32);
- unsigned long vstart;
phys_addr_t phys;
dma_addr_t dev_addr;
@@ -294,8 +295,12 @@ xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
if (dma_alloc_from_coherent(hwdev, size, dma_handle, &ret))
return ret;
- vstart = __get_free_pages(flags, order);
- ret = (void *)vstart;
+ /* On ARM this function returns an ioremap'ped virtual address for
+ * which virt_to_phys doesn't return the corresponding physical
+ * address. In fact on ARM virt_to_phys only works for kernel direct
+ * mapped RAM memory. Also see comment below.
+ */
+ ret = xen_alloc_coherent_pages(hwdev, size, dma_handle, flags, attrs);
if (!ret)
return ret;
@@ -303,15 +308,19 @@ xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
if (hwdev && hwdev->coherent_dma_mask)
dma_mask = dma_alloc_coherent_mask(hwdev, flags);
- phys = virt_to_phys(ret);
+ /* At this point dma_handle is the physical address, next we are
+ * going to set it to the machine address.
+ * Do not use virt_to_phys(ret) because on ARM it doesn't correspond
+ * to *dma_handle. */
+ phys = *dma_handle;
dev_addr = xen_phys_to_bus(phys);
if (((dev_addr + size - 1 <= dma_mask)) &&
!range_straddles_page_boundary(phys, size))
*dma_handle = dev_addr;
else {
- if (xen_create_contiguous_region(vstart, order,
+ if (xen_create_contiguous_region(phys, order,
fls64(dma_mask), dma_handle) != 0) {
- free_pages(vstart, order);
+ xen_free_coherent_pages(hwdev, size, ret, (dma_addr_t)phys, attrs);
return NULL;
}
}
@@ -334,13 +343,15 @@ xen_swiotlb_free_coherent(struct device *hwdev, size_t size, void *vaddr,
if (hwdev && hwdev->coherent_dma_mask)
dma_mask = hwdev->coherent_dma_mask;
- phys = virt_to_phys(vaddr);
+ /* do not use virt_to_phys because on ARM it doesn't return you the
+ * physical address */
+ phys = xen_bus_to_phys(dev_addr);
if (((dev_addr + size - 1 > dma_mask)) ||
range_straddles_page_boundary(phys, size))
- xen_destroy_contiguous_region((unsigned long)vaddr, order);
+ xen_destroy_contiguous_region(phys, order);
- free_pages((unsigned long)vaddr, order);
+ xen_free_coherent_pages(hwdev, size, vaddr, (dma_addr_t)phys, attrs);
}
EXPORT_SYMBOL_GPL(xen_swiotlb_free_coherent);
diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
index 9ef704d..fb2ea8f 100644
--- a/include/xen/xen-ops.h
+++ b/include/xen/xen-ops.h
@@ -19,11 +19,11 @@ void xen_arch_resume(void);
int xen_setup_shutdown_event(void);
extern unsigned long *xen_contiguous_bitmap;
-int xen_create_contiguous_region(unsigned long vstart, unsigned int order,
+int xen_create_contiguous_region(phys_addr_t pstart, unsigned int order,
unsigned int address_bits,
dma_addr_t *dma_handle);
-void xen_destroy_contiguous_region(unsigned long vstart, unsigned int order);
+void xen_destroy_contiguous_region(phys_addr_t pstart, unsigned int order);
struct vm_area_struct;
int xen_remap_domain_mfn_range(struct vm_area_struct *vma,
--
1.7.2.5
Call xen_dma_map_page, xen_dma_unmap_page, xen_dma_sync_single_for_cpu,
xen_dma_sync_single_for_device from swiotlb-xen to ensure cpu/device
coherency of the pages used for DMA, including the ones belonging to the
swiotlb buffer.
Signed-off-by: Stefano Stabellini <[email protected]>
---
drivers/xen/swiotlb-xen.c | 39 +++++++++++++++++++++++++++++++--------
1 files changed, 31 insertions(+), 8 deletions(-)
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 189b8db..4221cb5 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -378,8 +378,13 @@ dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
* buffering it.
*/
if (dma_capable(dev, dev_addr, size) &&
- !range_straddles_page_boundary(phys, size) && !swiotlb_force)
+ !range_straddles_page_boundary(phys, size) && !swiotlb_force) {
+ /* we are not interested in the dma_addr returned by
+ * xen_dma_map_page, only in the potential cache flushes executed
+ * by the function. */
+ xen_dma_map_page(dev, page, offset, size, dir, attrs);
return dev_addr;
+ }
/*
* Oh well, have to allocate and map a bounce buffer.
@@ -388,6 +393,8 @@ dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
if (map == SWIOTLB_MAP_ERROR)
return DMA_ERROR_CODE;
+ xen_dma_map_page(dev, pfn_to_page(map >> PAGE_SHIFT),
+ map & ~PAGE_MASK, size, dir, attrs);
dev_addr = xen_phys_to_bus(map);
/*
@@ -410,12 +417,15 @@ EXPORT_SYMBOL_GPL(xen_swiotlb_map_page);
* whatever the device wrote there.
*/
static void xen_unmap_single(struct device *hwdev, dma_addr_t dev_addr,
- size_t size, enum dma_data_direction dir)
+ size_t size, enum dma_data_direction dir,
+ struct dma_attrs *attrs)
{
phys_addr_t paddr = xen_bus_to_phys(dev_addr);
BUG_ON(dir == DMA_NONE);
+ xen_dma_unmap_page(hwdev, paddr, size, dir, attrs);
+
/* NOTE: We use dev_addr here, not paddr! */
if (is_xen_swiotlb_buffer(dev_addr)) {
swiotlb_tbl_unmap_single(hwdev, paddr, size, dir);
@@ -438,7 +448,7 @@ void xen_swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
size_t size, enum dma_data_direction dir,
struct dma_attrs *attrs)
{
- xen_unmap_single(hwdev, dev_addr, size, dir);
+ xen_unmap_single(hwdev, dev_addr, size, dir, attrs);
}
EXPORT_SYMBOL_GPL(xen_swiotlb_unmap_page);
@@ -461,11 +471,15 @@ xen_swiotlb_sync_single(struct device *hwdev, dma_addr_t dev_addr,
BUG_ON(dir == DMA_NONE);
+ if (target == SYNC_FOR_CPU)
+ xen_dma_sync_single_for_cpu(hwdev, paddr, size, dir);
+
/* NOTE: We use dev_addr here, not paddr! */
- if (is_xen_swiotlb_buffer(dev_addr)) {
+ if (is_xen_swiotlb_buffer(dev_addr))
swiotlb_tbl_sync_single(hwdev, paddr, size, dir, target);
- return;
- }
+
+ if (target == SYNC_FOR_DEVICE)
+ xen_dma_sync_single_for_cpu(hwdev, paddr, size, dir);
if (dir != DMA_FROM_DEVICE)
return;
@@ -536,8 +550,17 @@ xen_swiotlb_map_sg_attrs(struct device *hwdev, struct scatterlist *sgl,
return DMA_ERROR_CODE;
}
sg->dma_address = xen_phys_to_bus(map);
- } else
+ } else {
+ /* we are not interested in the dma_addr returned by
+ * xen_dma_map_page, only in the potential cache flushes executed
+ * by the function. */
+ xen_dma_map_page(hwdev, pfn_to_page(paddr >> PAGE_SHIFT),
+ paddr & ~PAGE_MASK,
+ sg->length,
+ dir,
+ attrs);
sg->dma_address = dev_addr;
+ }
sg_dma_len(sg) = sg->length;
}
return nelems;
@@ -559,7 +582,7 @@ xen_swiotlb_unmap_sg_attrs(struct device *hwdev, struct scatterlist *sgl,
BUG_ON(dir == DMA_NONE);
for_each_sg(sgl, sg, nelems, i)
- xen_unmap_single(hwdev, sg->dma_address, sg_dma_len(sg), dir);
+ xen_unmap_single(hwdev, sg->dma_address, sg_dma_len(sg), dir, attrs);
}
EXPORT_SYMBOL_GPL(xen_swiotlb_unmap_sg_attrs);
--
1.7.2.5
This functions are small and called frequently. Static inline them.
Signed-off-by: Stefano Stabellini <[email protected]>
---
drivers/xen/swiotlb-xen.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 6d9ca61..9045517 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -72,17 +72,17 @@ static unsigned long xen_io_tlb_nslabs;
static u64 start_dma_addr;
-static dma_addr_t xen_phys_to_bus(phys_addr_t paddr)
+static inline dma_addr_t xen_phys_to_bus(phys_addr_t paddr)
{
return phys_to_machine(XPADDR(paddr)).maddr;
}
-static phys_addr_t xen_bus_to_phys(dma_addr_t baddr)
+static inline phys_addr_t xen_bus_to_phys(dma_addr_t baddr)
{
return machine_to_phys(XMADDR(baddr)).paddr;
}
-static dma_addr_t xen_virt_to_bus(void *address)
+static inline dma_addr_t xen_virt_to_bus(void *address)
{
return xen_phys_to_bus(virt_to_phys(address));
}
@@ -105,7 +105,7 @@ static int check_pages_physically_contiguous(unsigned long pfn,
return 1;
}
-static int range_straddles_page_boundary(phys_addr_t p, size_t size)
+static inline int range_straddles_page_boundary(phys_addr_t p, size_t size)
{
unsigned long pfn = PFN_DOWN(p);
unsigned int offset = p & ~PAGE_MASK;
--
1.7.2.5
Signed-off-by: Stefano Stabellini <[email protected]>
Changes in v7:
- use dev_warn instead of pr_warn.
---
drivers/xen/swiotlb-xen.c | 1 +
lib/swiotlb.c | 1 +
2 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 4221cb5..4d50058 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -542,6 +542,7 @@ xen_swiotlb_map_sg_attrs(struct device *hwdev, struct scatterlist *sgl,
sg->length,
dir);
if (map == SWIOTLB_MAP_ERROR) {
+ dev_warn(hwdev, "swiotlb buffer is full\n");
/* Don't panic here, we expect map_sg users
to do proper error handling. */
xen_swiotlb_unmap_sg_attrs(hwdev, sgl, i, dir,
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 4e8686c..cdc051e 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -502,6 +502,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *hwdev,
not_found:
spin_unlock_irqrestore(&io_tlb_lock, flags);
+ dev_warn(hwdev, "swiotlb buffer is full\n");
return SWIOTLB_MAP_ERROR;
found:
spin_unlock_irqrestore(&io_tlb_lock, flags);
--
1.7.2.5
From: Julien Grall <[email protected]>
The commit "arm: introduce a global dma_ops pointer" introduce compilation issue
when CONFIG_SND_SOC_SAMSUNG is enabled.
sound/soc/samsung/dma.c:345:27: error: conflicting types for 'dma_ops'
/local/home/julien/works/arndale/linux/arch/arm/include/asm/dma-mapping.h:16:28:
note: previous declaration of 'dma_ops' was here
Signed-off-by: Julien Grall <[email protected]>
Signed-off-by: Stefano Stabellini <[email protected]>
---
sound/soc/samsung/dma.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/samsung/dma.c b/sound/soc/samsung/dma.c
index 21b7926..827c0d8 100644
--- a/sound/soc/samsung/dma.c
+++ b/sound/soc/samsung/dma.c
@@ -341,7 +341,7 @@ static int dma_mmap(struct snd_pcm_substream *substream,
runtime->dma_bytes);
}
-static struct snd_pcm_ops dma_ops = {
+static struct snd_pcm_ops samsung_dma_ops = {
.open = dma_open,
.close = dma_close,
.ioctl = snd_pcm_lib_ioctl,
@@ -428,7 +428,7 @@ out:
}
static struct snd_soc_platform_driver samsung_asoc_platform = {
- .ops = &dma_ops,
+ .ops = &samsung_dma_ops,
.pcm_new = dma_new,
.pcm_free = dma_free_dma_buffers,
};
--
1.7.2.5
Introduce xen_dma_map_page, xen_dma_unmap_page,
xen_dma_sync_single_for_cpu and xen_dma_sync_single_for_device.
They have empty implementations on x86 and ia64 but they call the
corresponding platform dma_ops function on arm and arm64.
Signed-off-by: Stefano Stabellini <[email protected]>
---
arch/arm/include/asm/xen/page-coherent.h | 28 ++++++++++++++++++++++++++++
arch/arm64/include/asm/xen/page-coherent.h | 25 +++++++++++++++++++++++++
arch/ia64/include/asm/xen/page-coherent.h | 14 ++++++++++++++
arch/x86/include/asm/xen/page-coherent.h | 14 ++++++++++++++
4 files changed, 81 insertions(+), 0 deletions(-)
diff --git a/arch/arm/include/asm/xen/page-coherent.h b/arch/arm/include/asm/xen/page-coherent.h
index c4d843c..532cffe 100644
--- a/arch/arm/include/asm/xen/page-coherent.h
+++ b/arch/arm/include/asm/xen/page-coherent.h
@@ -19,4 +19,32 @@ static inline void xen_free_coherent_pages(struct device *hwdev, size_t size,
__generic_dma_ops(hwdev)->free(hwdev, size, cpu_addr, dma_handle, attrs);
}
+static inline dma_addr_t xen_dma_map_page(struct device *hwdev, struct page *page,
+ unsigned long offset, size_t size, enum dma_data_direction dir,
+ struct dma_attrs *attrs)
+{
+ return __generic_dma_ops(hwdev)->map_page(hwdev, page, 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,
+ struct dma_attrs *attrs)
+{
+ if (__generic_dma_ops(hwdev)->unmap_page)
+ __generic_dma_ops(hwdev)->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)
+{
+ if (__generic_dma_ops(hwdev)->sync_single_for_cpu)
+ __generic_dma_ops(hwdev)->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)
+{
+ if (__generic_dma_ops(hwdev)->sync_single_for_device)
+ __generic_dma_ops(hwdev)->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 3b4f029..5abe098 100644
--- a/arch/arm64/include/asm/xen/page-coherent.h
+++ b/arch/arm64/include/asm/xen/page-coherent.h
@@ -19,4 +19,29 @@ static inline void xen_free_coherent_pages(struct device *hwdev, size_t size,
__generic_dma_ops(hwdev)->free(hwdev, size, cpu_addr, dma_handle, attrs);
}
+static inline dma_addr_t xen_dma_map_page(struct device *hwdev, struct page *page,
+ unsigned long offset, size_t size, enum dma_data_direction dir,
+ struct dma_attrs *attrs)
+{
+ return __generic_dma_ops(hwdev)->map_page(hwdev, page, 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,
+ struct dma_attrs *attrs)
+{
+ __generic_dma_ops(hwdev)->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)
+{
+ __generic_dma_ops(hwdev)->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)
+{
+ __generic_dma_ops(hwdev)->sync_single_for_device(hwdev, handle, size, dir);
+}
#endif /* _ASM_ARM64_XEN_PAGE_COHERENT_H */
diff --git a/arch/ia64/include/asm/xen/page-coherent.h b/arch/ia64/include/asm/xen/page-coherent.h
index 37b929c..ed2e5ed 100644
--- a/arch/ia64/include/asm/xen/page-coherent.h
+++ b/arch/ia64/include/asm/xen/page-coherent.h
@@ -21,4 +21,18 @@ static inline void xen_free_coherent_pages(struct device *hwdev, size_t size,
free_pages((unsigned long) cpu_addr, get_order(size));
}
+static inline dma_addr_t xen_dma_map_page(struct device *hwdev, struct page *page,
+ unsigned long offset, size_t size, enum dma_data_direction dir,
+ struct dma_attrs *attrs) { return page_to_phys(page); }
+
+static inline void xen_dma_unmap_page(struct device *hwdev, dma_addr_t handle,
+ size_t size, enum dma_data_direction dir,
+ struct dma_attrs *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) { }
+
+static inline void xen_dma_sync_single_for_device(struct device *hwdev,
+ dma_addr_t handle, size_t size, enum dma_data_direction dir) { }
+
#endif /* _ASM_IA64_XEN_PAGE_COHERENT_H */
diff --git a/arch/x86/include/asm/xen/page-coherent.h b/arch/x86/include/asm/xen/page-coherent.h
index 31de2e0..3ddc2c2 100644
--- a/arch/x86/include/asm/xen/page-coherent.h
+++ b/arch/x86/include/asm/xen/page-coherent.h
@@ -21,4 +21,18 @@ static inline void xen_free_coherent_pages(struct device *hwdev, size_t size,
free_pages((unsigned long) cpu_addr, get_order(size));
}
+static inline dma_addr_t xen_dma_map_page(struct device *hwdev, struct page *page,
+ unsigned long offset, size_t size, enum dma_data_direction dir,
+ struct dma_attrs *attrs) { return page_to_phys(page); }
+
+static inline void xen_dma_unmap_page(struct device *hwdev, dma_addr_t handle,
+ size_t size, enum dma_data_direction dir,
+ struct dma_attrs *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) { }
+
+static inline void xen_dma_sync_single_for_device(struct device *hwdev,
+ dma_addr_t handle, size_t size, enum dma_data_direction dir) { }
+
#endif /* _ASM_X86_XEN_PAGE_COHERENT_H */
--
1.7.2.5
Russell,
are you OK with this patch?
On Thu, 17 Oct 2013, Stefano Stabellini wrote:
> We can't simply override arm_dma_ops with xen_dma_ops because devices
> are allowed to have their own dma_ops and they take precedence over
> arm_dma_ops. When running on Xen as initial domain, we always want
> xen_dma_ops to be the one in use.
>
> Signed-off-by: Stefano Stabellini <[email protected]>
> Suggested-by: Catalin Marinas <[email protected]>
> CC: [email protected]
> CC: [email protected]
>
> Changes in v7:
> - return xen_dma_ops only if we are the initial domain;
> - rename __get_dma_ops to __generic_dma_ops.
> ---
> arch/arm/include/asm/dma-mapping.h | 13 ++++++++++++-
> 1 files changed, 12 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h
> index 01b5a3d..f5945d4 100644
> --- a/arch/arm/include/asm/dma-mapping.h
> +++ b/arch/arm/include/asm/dma-mapping.h
> @@ -12,17 +12,28 @@
> #include <asm/memory.h>
> #include <asm/cacheflush.h>
>
> +#include <xen/xen.h>
> +#include <asm/xen/hypervisor.h>
> +
> #define DMA_ERROR_CODE (~0)
> extern struct dma_map_ops arm_dma_ops;
> extern struct dma_map_ops arm_coherent_dma_ops;
>
> -static inline struct dma_map_ops *get_dma_ops(struct device *dev)
> +static inline struct dma_map_ops *__generic_dma_ops(struct device *dev)
> {
> if (dev && dev->archdata.dma_ops)
> return dev->archdata.dma_ops;
> return &arm_dma_ops;
> }
>
> +static inline struct dma_map_ops *get_dma_ops(struct device *dev)
> +{
> + if (xen_initial_domain())
> + return xen_dma_ops;
> + else
> + return __generic_dma_ops(dev);
> +}
> +
> static inline void set_dma_ops(struct device *dev, struct dma_map_ops *ops)
> {
> BUG_ON(!dev);
> --
> 1.7.2.5
>
Russell, Catalin,
are you OK with this patch?
It shouldn't have any impact when running on native, only on Xen.
On Thu, 17 Oct 2013, Stefano Stabellini wrote:
> This is similar to what it is done on X86: biovecs are prevented from merging
> otherwise every dma requests would be forced to bounce on the swiotlb buffer.
>
> Signed-off-by: Stefano Stabellini <[email protected]>
>
>
> Changes in v7:
> - remove the extra autotranslate check in biomerge.c.
> ---
> arch/arm/include/asm/io.h | 8 ++++++++
> arch/arm64/include/asm/io.h | 9 +++++++++
> 2 files changed, 17 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
> index d070741..c45effc 100644
> --- a/arch/arm/include/asm/io.h
> +++ b/arch/arm/include/asm/io.h
> @@ -24,9 +24,11 @@
> #ifdef __KERNEL__
>
> #include <linux/types.h>
> +#include <linux/blk_types.h>
> #include <asm/byteorder.h>
> #include <asm/memory.h>
> #include <asm-generic/pci_iomap.h>
> +#include <xen/xen.h>
>
> /*
> * ISA I/O bus memory addresses are 1:1 with the physical address.
> @@ -372,6 +374,12 @@ extern void pci_iounmap(struct pci_dev *dev, void __iomem *addr);
> #define BIOVEC_MERGEABLE(vec1, vec2) \
> ((bvec_to_phys((vec1)) + (vec1)->bv_len) == bvec_to_phys((vec2)))
>
> +extern bool xen_biovec_phys_mergeable(const struct bio_vec *vec1,
> + const struct bio_vec *vec2);
> +#define BIOVEC_PHYS_MERGEABLE(vec1, vec2) \
> + (__BIOVEC_PHYS_MERGEABLE(vec1, vec2) && \
> + (!xen_domain() || xen_biovec_phys_mergeable(vec1, vec2)))
> +
> #ifdef CONFIG_MMU
> #define ARCH_HAS_VALID_PHYS_ADDR_RANGE
> extern int valid_phys_addr_range(phys_addr_t addr, size_t size);
> diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
> index 1d12f89..c163287b 100644
> --- a/arch/arm64/include/asm/io.h
> +++ b/arch/arm64/include/asm/io.h
> @@ -22,11 +22,14 @@
> #ifdef __KERNEL__
>
> #include <linux/types.h>
> +#include <linux/blk_types.h>
>
> #include <asm/byteorder.h>
> #include <asm/barrier.h>
> #include <asm/pgtable.h>
>
> +#include <xen/xen.h>
> +
> /*
> * Generic IO read/write. These perform native-endian accesses.
> */
> @@ -263,5 +266,11 @@ extern int devmem_is_allowed(unsigned long pfn);
> */
> #define xlate_dev_kmem_ptr(p) p
>
> +extern bool xen_biovec_phys_mergeable(const struct bio_vec *vec1,
> + const struct bio_vec *vec2);
> +#define BIOVEC_PHYS_MERGEABLE(vec1, vec2) \
> + (__BIOVEC_PHYS_MERGEABLE(vec1, vec2) && \
> + (!xen_domain() || xen_biovec_phys_mergeable(vec1, vec2)))
> +
> #endif /* __KERNEL__ */
> #endif /* __ASM_IO_H */
> --
> 1.7.2.5
>
On Fri, Oct 18, 2013 at 01:23:59PM +0100, Stefano Stabellini wrote:
> are you OK with this patch?
>
> It shouldn't have any impact when running on native, only on Xen.
I don't have an opinion on what it does but since it only affects Xen,
it's fine by me.
Acked-by: Catalin Marinas <[email protected]>
On Fri, Oct 18, 2013 at 01:22:18PM +0100, Stefano Stabellini wrote:
> Russell,
> are you OK with this patch?
Only concern is why the change is soo large.
> > -static inline struct dma_map_ops *get_dma_ops(struct device *dev)
> > +static inline struct dma_map_ops *__generic_dma_ops(struct device *dev)
> > {
> > if (dev && dev->archdata.dma_ops)
> > return dev->archdata.dma_ops;
> > return &arm_dma_ops;
> > }
> >
> > +static inline struct dma_map_ops *get_dma_ops(struct device *dev)
> > +{
> > + if (xen_initial_domain())
> > + return xen_dma_ops;
> > + else
> > + return __generic_dma_ops(dev);
> > +}
> > +
What's wrong with:
static inline struct dma_map_ops *get_dma_ops(struct device *dev)
{
+ if (xen_initial_domain())
+ return xen_dma_ops;
if (dev && dev->archdata.dma_ops)
return dev->archdata.dma_ops;
return &arm_dma_ops;
}
On Fri, 18 Oct 2013, Russell King - ARM Linux wrote:
> On Fri, Oct 18, 2013 at 01:22:18PM +0100, Stefano Stabellini wrote:
> > Russell,
> > are you OK with this patch?
>
> Only concern is why the change is soo large.
>
> > > -static inline struct dma_map_ops *get_dma_ops(struct device *dev)
> > > +static inline struct dma_map_ops *__generic_dma_ops(struct device *dev)
> > > {
> > > if (dev && dev->archdata.dma_ops)
> > > return dev->archdata.dma_ops;
> > > return &arm_dma_ops;
> > > }
> > >
> > > +static inline struct dma_map_ops *get_dma_ops(struct device *dev)
> > > +{
> > > + if (xen_initial_domain())
> > > + return xen_dma_ops;
> > > + else
> > > + return __generic_dma_ops(dev);
> > > +}
> > > +
>
> What's wrong with:
>
> static inline struct dma_map_ops *get_dma_ops(struct device *dev)
> {
> + if (xen_initial_domain())
> + return xen_dma_ops;
> if (dev && dev->archdata.dma_ops)
> return dev->archdata.dma_ops;
> return &arm_dma_ops;
> }
xen_dma_ops functions might have to call back the native implementation.
With the above there is no way for a xen_dma_ops function to get to
archdata.dma_ops or arm_dma_ops.
This is the reason why this patch introduces __generic_dma_ops.
On Fri, Oct 18, 2013 at 04:46:00PM +0100, Stefano Stabellini wrote:
> On Fri, 18 Oct 2013, Russell King - ARM Linux wrote:
> > On Fri, Oct 18, 2013 at 01:22:18PM +0100, Stefano Stabellini wrote:
> > > Russell,
> > > are you OK with this patch?
> >
> > Only concern is why the change is soo large.
> >
> > > > -static inline struct dma_map_ops *get_dma_ops(struct device *dev)
> > > > +static inline struct dma_map_ops *__generic_dma_ops(struct device *dev)
> > > > {
> > > > if (dev && dev->archdata.dma_ops)
> > > > return dev->archdata.dma_ops;
> > > > return &arm_dma_ops;
> > > > }
> > > >
> > > > +static inline struct dma_map_ops *get_dma_ops(struct device *dev)
> > > > +{
> > > > + if (xen_initial_domain())
> > > > + return xen_dma_ops;
> > > > + else
> > > > + return __generic_dma_ops(dev);
> > > > +}
> > > > +
> >
> > What's wrong with:
> >
> > static inline struct dma_map_ops *get_dma_ops(struct device *dev)
> > {
> > + if (xen_initial_domain())
> > + return xen_dma_ops;
> > if (dev && dev->archdata.dma_ops)
> > return dev->archdata.dma_ops;
> > return &arm_dma_ops;
> > }
>
>
> xen_dma_ops functions might have to call back the native implementation.
> With the above there is no way for a xen_dma_ops function to get to
> archdata.dma_ops or arm_dma_ops.
> This is the reason why this patch introduces __generic_dma_ops.
Ah, that's fine then, but having that described in the commit message
would be useful.
Acked-by: Russell King <[email protected]>
Thanks.
Russell,
this is the last ARM patch of the series that needs review.
It is just adding three new static inlines needed by SWIOTLB and
IOMMU_HELPER.
If you are OK with the patch I am going to add the lot to linux-next via
the Xen tree.
On Thu, 17 Oct 2013, Stefano Stabellini wrote:
> IOMMU_HELPER is needed because SWIOTLB calls iommu_is_span_boundary,
> provided by lib/iommu_helper.c.
>
> Signed-off-by: Stefano Stabellini <[email protected]>
> Reviewed-by: Konrad Rzeszutek Wilk <[email protected]>
> CC: [email protected]
> CC: [email protected]
>
>
> Changes in v8:
> - use __phys_to_pfn and __pfn_to_phys.
>
> Changes in v7:
> - dma_mark_clean: empty implementation;
> - in dma_capable use coherent_dma_mask if dma_mask hasn't been
> allocated.
>
> Changes in v6:
> - check for dev->dma_mask being NULL in dma_capable.
>
> Changes in v5:
> - implement dma_mark_clean using dmac_flush_range.
>
> Changes in v3:
> - dma_capable: do not treat dma_mask as a limit;
> - remove SWIOTLB dependency on NEED_SG_DMA_LENGTH.
> ---
> arch/arm/Kconfig | 6 +++++
> arch/arm/include/asm/dma-mapping.h | 37 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 43 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index ba412e0..c0bfb33 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1832,6 +1832,12 @@ config CC_STACKPROTECTOR
> neutralized via a kernel panic.
> This feature requires gcc version 4.2 or above.
>
> +config SWIOTLB
> + def_bool y
> +
> +config IOMMU_HELPER
> + def_bool SWIOTLB
> +
> config XEN_DOM0
> def_bool y
> depends on XEN
> diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h
> index 5b579b9..01b5a3d 100644
> --- a/arch/arm/include/asm/dma-mapping.h
> +++ b/arch/arm/include/asm/dma-mapping.h
> @@ -10,6 +10,7 @@
>
> #include <asm-generic/dma-coherent.h>
> #include <asm/memory.h>
> +#include <asm/cacheflush.h>
>
> #define DMA_ERROR_CODE (~0)
> extern struct dma_map_ops arm_dma_ops;
> @@ -86,6 +87,42 @@ static inline dma_addr_t virt_to_dma(struct device *dev, void *addr)
> }
> #endif
>
> +static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
> +{
> + unsigned int offset = paddr & ~PAGE_MASK;
> + return pfn_to_dma(dev, __phys_to_pfn(paddr)) + offset;
> +}
> +
> +static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t dev_addr)
> +{
> + unsigned int offset = dev_addr & ~PAGE_MASK;
> + return __pfn_to_phys(dma_to_pfn(dev, dev_addr)) + offset;
> +}
> +
> +static inline bool dma_capable(struct device *dev, dma_addr_t addr, size_t size)
> +{
> + u64 limit, mask;
> +
> + if (dev->dma_mask)
> + mask = *dev->dma_mask;
> + else
> + mask = dev->coherent_dma_mask;
> +
> + if (mask == 0)
> + return 0;
> +
> + limit = (mask + 1) & ~mask;
> + if (limit && size > limit)
> + return 0;
> +
> + if ((addr | (addr + size - 1)) & ~mask)
> + return 0;
> +
> + return 1;
> +}
> +
> +static inline void dma_mark_clean(void *addr, size_t size) { }
> +
> /*
> * DMA errors are defined by all-bits-set in the DMA address.
> */
> --
> 1.7.2.5
>
On Fri, 18 Oct 2013, Russell King - ARM Linux wrote:
> On Fri, Oct 18, 2013 at 04:46:00PM +0100, Stefano Stabellini wrote:
> > On Fri, 18 Oct 2013, Russell King - ARM Linux wrote:
> > > On Fri, Oct 18, 2013 at 01:22:18PM +0100, Stefano Stabellini wrote:
> > > > Russell,
> > > > are you OK with this patch?
> > >
> > > Only concern is why the change is soo large.
> > >
> > > > > -static inline struct dma_map_ops *get_dma_ops(struct device *dev)
> > > > > +static inline struct dma_map_ops *__generic_dma_ops(struct device *dev)
> > > > > {
> > > > > if (dev && dev->archdata.dma_ops)
> > > > > return dev->archdata.dma_ops;
> > > > > return &arm_dma_ops;
> > > > > }
> > > > >
> > > > > +static inline struct dma_map_ops *get_dma_ops(struct device *dev)
> > > > > +{
> > > > > + if (xen_initial_domain())
> > > > > + return xen_dma_ops;
> > > > > + else
> > > > > + return __generic_dma_ops(dev);
> > > > > +}
> > > > > +
> > >
> > > What's wrong with:
> > >
> > > static inline struct dma_map_ops *get_dma_ops(struct device *dev)
> > > {
> > > + if (xen_initial_domain())
> > > + return xen_dma_ops;
> > > if (dev && dev->archdata.dma_ops)
> > > return dev->archdata.dma_ops;
> > > return &arm_dma_ops;
> > > }
> >
> >
> > xen_dma_ops functions might have to call back the native implementation.
> > With the above there is no way for a xen_dma_ops function to get to
> > archdata.dma_ops or arm_dma_ops.
> > This is the reason why this patch introduces __generic_dma_ops.
>
> Ah, that's fine then, but having that described in the commit message
> would be useful.
>
> Acked-by: Russell King <[email protected]>
>
> Thanks.
Thank you. I'll add a note to the commit message.
On Thu, Oct 17, 2013 at 06:43:33PM +0100, Stefano Stabellini wrote:
> xen_unmap_single calls xen_bus_to_phys, then passes dev_addr to
> is_xen_swiotlb_buffer that calls xen_bus_to_phys again.
It does?
I see it call mfn_to_local_pfn which calls pfn_to_mfn and
get_phys_to_machine (which I presume is going to be a slow
operation on ARM).
> xen_bus_to_phys is potentially a slow operation that might involve
> walking a tree.
> We can avoid calling xen_bus_to_phys twice by removing the
> is_xen_swiotlb_buffer call and doing the check directly in
> xen_unmap_single using the physical address.
>
> Signed-off-by: Stefano Stabellini <[email protected]>
Have you tested this with a PFN that is in a foreign domain?
I am bit uncomfortable with this - I recall when developing this
that I had hit some odd cases of doing DMA of a page to a guest
and it not always working. It was the case of not detecting whether
the pfn was foreign or not. Sadly I can't recall the details.
I would think that this problem would also be present with ARM?
But perhaps all the new changes in the pfn_to_mfn that had lately
been done have fixed this?
> ---
> drivers/xen/swiotlb-xen.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index 4d50058..6d9ca61 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -426,8 +426,8 @@ static void xen_unmap_single(struct device *hwdev, dma_addr_t dev_addr,
>
> xen_dma_unmap_page(hwdev, paddr, size, dir, attrs);
>
> - /* NOTE: We use dev_addr here, not paddr! */
> - if (is_xen_swiotlb_buffer(dev_addr)) {
> + if (paddr >= virt_to_phys(xen_io_tlb_start) &&
> + paddr < virt_to_phys(xen_io_tlb_end)) {
> swiotlb_tbl_unmap_single(hwdev, paddr, size, dir);
> return;
> }
> --
> 1.7.2.5
>
On Thu, Oct 17, 2013 at 06:43:32PM +0100, Stefano Stabellini wrote:
> When mapping/unmapping grant refs, call
> set_phys_to_machine to update the P2M with the new mappings for
> autotranslate guests.
Heh. That sounds so odd (p2m + xlat).
Can you add a sentence saying that this is a mostly a nop on x86 please?
(Well, the call to set_phys_to_machine does end up with this check:
if (unlikely(xen_feature(XENFEAT_auto_translated_physmap))) {
BUG_ON(pfn != mfn && mfn != INVALID_P2M_ENTRY);
return true;
}
But that should be a fairly small amount of code.
>
> Signed-off-by: Stefano Stabellini <[email protected]>
> ---
> drivers/xen/grant-table.c | 17 +++++++++++++++--
> 1 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
> index 04cdeb8..a36cb03 100644
> --- a/drivers/xen/grant-table.c
> +++ b/drivers/xen/grant-table.c
> @@ -49,6 +49,7 @@
> #include <xen/grant_table.h>
> #include <xen/interface/memory.h>
> #include <xen/hvc-console.h>
> +#include <xen/swiotlb-xen.h>
> #include <asm/xen/hypercall.h>
> #include <asm/xen/interface.h>
>
> @@ -889,8 +890,15 @@ int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops,
> gnttab_retry_eagain_gop(GNTTABOP_map_grant_ref, map_ops + i,
> &map_ops[i].status, __func__);
>
> - if (xen_feature(XENFEAT_auto_translated_physmap))
> + if (xen_feature(XENFEAT_auto_translated_physmap)) {
> + for (i = 0; i < count; i++) {
> + if (map_ops[i].status)
> + continue;
> + set_phys_to_machine(map_ops[i].host_addr >> PAGE_SHIFT,
> + map_ops[i].dev_bus_addr >> PAGE_SHIFT);
> + }
> return ret;
> + }
>
> if (!in_interrupt() && paravirt_get_lazy_mode() == PARAVIRT_LAZY_NONE) {
> arch_enter_lazy_mmu_mode();
> @@ -933,8 +941,13 @@ int gnttab_unmap_refs(struct gnttab_unmap_grant_ref *unmap_ops,
> if (ret)
> return ret;
>
> - if (xen_feature(XENFEAT_auto_translated_physmap))
> + if (xen_feature(XENFEAT_auto_translated_physmap)) {
> + for (i = 0; i < count; i++) {
> + set_phys_to_machine(unmap_ops[i].host_addr >> PAGE_SHIFT,
> + INVALID_P2M_ENTRY);
> + }
> return ret;
> + }
>
> if (!in_interrupt() && paravirt_get_lazy_mode() == PARAVIRT_LAZY_NONE) {
> arch_enter_lazy_mmu_mode();
> --
> 1.7.2.5
>
On Thu, Oct 17, 2013 at 06:43:28PM +0100, Stefano Stabellini wrote:
> Call xen_dma_map_page, xen_dma_unmap_page, xen_dma_sync_single_for_cpu,
> xen_dma_sync_single_for_device from swiotlb-xen to ensure cpu/device
> coherency of the pages used for DMA, including the ones belonging to the
> swiotlb buffer.
You lost me.
Isn't it the driver's responsibility to do this?
Looking at what 'xen_dma_map_page()' does for x86 it looks to add an extra
call - page_to_phys - and we ignore it here.
>
> Signed-off-by: Stefano Stabellini <[email protected]>
> ---
> drivers/xen/swiotlb-xen.c | 39 +++++++++++++++++++++++++++++++--------
> 1 files changed, 31 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index 189b8db..4221cb5 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -378,8 +378,13 @@ dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
> * buffering it.
> */
> if (dma_capable(dev, dev_addr, size) &&
> - !range_straddles_page_boundary(phys, size) && !swiotlb_force)
> + !range_straddles_page_boundary(phys, size) && !swiotlb_force) {
> + /* we are not interested in the dma_addr returned by
> + * xen_dma_map_page, only in the potential cache flushes executed
> + * by the function. */
> + xen_dma_map_page(dev, page, offset, size, dir, attrs);
> return dev_addr;
> + }
>
> /*
> * Oh well, have to allocate and map a bounce buffer.
> @@ -388,6 +393,8 @@ dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
> if (map == SWIOTLB_MAP_ERROR)
> return DMA_ERROR_CODE;
>
> + xen_dma_map_page(dev, pfn_to_page(map >> PAGE_SHIFT),
> + map & ~PAGE_MASK, size, dir, attrs);
> dev_addr = xen_phys_to_bus(map);
>
> /*
> @@ -410,12 +417,15 @@ EXPORT_SYMBOL_GPL(xen_swiotlb_map_page);
> * whatever the device wrote there.
> */
> static void xen_unmap_single(struct device *hwdev, dma_addr_t dev_addr,
> - size_t size, enum dma_data_direction dir)
> + size_t size, enum dma_data_direction dir,
> + struct dma_attrs *attrs)
> {
> phys_addr_t paddr = xen_bus_to_phys(dev_addr);
>
> BUG_ON(dir == DMA_NONE);
>
> + xen_dma_unmap_page(hwdev, paddr, size, dir, attrs);
> +
> /* NOTE: We use dev_addr here, not paddr! */
> if (is_xen_swiotlb_buffer(dev_addr)) {
> swiotlb_tbl_unmap_single(hwdev, paddr, size, dir);
> @@ -438,7 +448,7 @@ void xen_swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
> size_t size, enum dma_data_direction dir,
> struct dma_attrs *attrs)
> {
> - xen_unmap_single(hwdev, dev_addr, size, dir);
> + xen_unmap_single(hwdev, dev_addr, size, dir, attrs);
> }
> EXPORT_SYMBOL_GPL(xen_swiotlb_unmap_page);
>
> @@ -461,11 +471,15 @@ xen_swiotlb_sync_single(struct device *hwdev, dma_addr_t dev_addr,
>
> BUG_ON(dir == DMA_NONE);
>
> + if (target == SYNC_FOR_CPU)
> + xen_dma_sync_single_for_cpu(hwdev, paddr, size, dir);
> +
> /* NOTE: We use dev_addr here, not paddr! */
> - if (is_xen_swiotlb_buffer(dev_addr)) {
> + if (is_xen_swiotlb_buffer(dev_addr))
> swiotlb_tbl_sync_single(hwdev, paddr, size, dir, target);
> - return;
> - }
> +
> + if (target == SYNC_FOR_DEVICE)
> + xen_dma_sync_single_for_cpu(hwdev, paddr, size, dir);
>
> if (dir != DMA_FROM_DEVICE)
> return;
> @@ -536,8 +550,17 @@ xen_swiotlb_map_sg_attrs(struct device *hwdev, struct scatterlist *sgl,
> return DMA_ERROR_CODE;
> }
> sg->dma_address = xen_phys_to_bus(map);
> - } else
> + } else {
> + /* we are not interested in the dma_addr returned by
> + * xen_dma_map_page, only in the potential cache flushes executed
> + * by the function. */
> + xen_dma_map_page(hwdev, pfn_to_page(paddr >> PAGE_SHIFT),
> + paddr & ~PAGE_MASK,
> + sg->length,
> + dir,
> + attrs);
> sg->dma_address = dev_addr;
> + }
> sg_dma_len(sg) = sg->length;
> }
> return nelems;
> @@ -559,7 +582,7 @@ xen_swiotlb_unmap_sg_attrs(struct device *hwdev, struct scatterlist *sgl,
> BUG_ON(dir == DMA_NONE);
>
> for_each_sg(sgl, sg, nelems, i)
> - xen_unmap_single(hwdev, sg->dma_address, sg_dma_len(sg), dir);
> + xen_unmap_single(hwdev, sg->dma_address, sg_dma_len(sg), dir, attrs);
>
> }
> EXPORT_SYMBOL_GPL(xen_swiotlb_unmap_sg_attrs);
> --
> 1.7.2.5
>
On Thu, Oct 17, 2013 at 06:42:18PM +0100, Stefano Stabellini wrote:
> Hi all,
> this patch series enables xen-swiotlb on arm and arm64.
>
> It has been heavily reworked compared to the previous versions in order
> to achieve better performances and to address review comments.
>
> We are not using dma_mark_clean to ensure coherency anymore. We call the
> platform implementation of map_page and unmap_page.
>
> We assume that dom0 has been mapped 1:1 (physical address ==
> machine address), that is what Xen on ARM currently does.
> As a consequence we only use the swiotlb to handle dma
> requests involving pages corresponding to grant refs. Obviously these
> pages cannot be part of the 1:1 because they belong to another domain.
>
> To improve code readability this version of the series introduces a
> "p2m" for arm and arm64, implemented using two red-black trees. The p2m
> tracks physical to machine and machine to physical mappings. Given that
> we assume a 1:1 mapping in dom0, the only entries in both trees are
> grant refs that have been mapped in dom0.
>
> Another consequence of the new approach is that the hypercalls we were
> previously introducing (XENMEM_exchange_and_pin, XENMEM_pin,
> XENMEM_unpin) are not needed anymore.
> This latest version doesn't need any hypervisor changes anymore.
>From the Xen and SWIOTLB I am ok with the patch series, except
for these two patches:
swiotlb-xen: no need to do execute two bus_to_phys translations in a ..
swiotlb-xen: use xen_dma_map/unmap_page, xen_dma_sync_single_for_cpu/device
which I think need a bit more of explanation. In worst case they can be
#ifdef CONFIG_ARM
..
#endif
albeit that will need to be fixed in the future to remove the #ifdef I think.
>
> Cheers,
>
> Stefano
>
>
> Changes in v8:
> - use __phys_to_pfn and __pfn_to_phys in phys_to_dma and dma_to_phys;
> - cast 0 to dma_addr_t in the definition of DMA_ERROR_CODE;
> - move pfn_to_mfn and mfn_to_pfn to page.h as static inline functions;
> - no need to walk the two p2m trees if phys_to_mach.rb_node is NULL;
> - correctly handle multipage p2m entries;
> - substitute the p2m spin_lock with a rwlock;
> - assume dom0 is mapped 1:1, no need to call XENMEM_exchange in
> xen_create_contiguous_region;
> - add two simple performance improvements for swiotlb-xen.
>
> Changes in v7:
> - dma_mark_clean: switch to an empty implementation (we are going to use
> the platform map_page and unmap_page to ensure coherency);
> - dma_capable: use coherent_dma_mask if dma_mask hasn't been
> allocated;
> - introduce a p2m for xen on arm and arm64;
> - call __set_phys_to_machine_multi from xen_create_contiguous_region to
> update the p2m;
> - remove XENMEM_unpin and XENMEM_exchange_and_pin;
> - set nr_exchanged to 0 before calling the hypercall;
> - return xen_dma_ops only if we are the initial domain;
> - rename __get_dma_ops to __generic_dma_ops;
> - call __generic_dma_ops(hwdev)->alloc/free on arm64 too;
> - introduce xen_dma_map/unmap_page and xen_dma_sync_single_for_cpu/device;
> - use xen_dma_map/unmap_page, xen_dma_sync_single_for_cpu/device in
> swiotlb-xen;
> - use dev_warn instead of pr_warn;
> - remove the extra autotranslate check in biomerge.c;
> - call set_phys_to_machine after mapping grant refs in gnttab_map_refs;
> - call set_phys_to_machine after unmapping grant refs in gnttab_unmap_refs;
> - allow __set_phys_to_machine to be called for autotranslate guests on
> x86.
>
> Changes in v6:
> - check for dev->dma_mask being NULL in dma_capable;
> - update the comments and the hypercalls structures;
> - add a xen_dma_info entry to the rbtree in xen_swiotlb_alloc_coherent
> to keep track of the new mapping. Free the entry in xen_swiotlb_free_coherent;
> - rename xen_dma_seg to dma_info in xen_swiotlb_alloc/free_coherent to
> avoid confusions;
> - introduce and export xen_dma_ops;
> - call xen_mm_init from as arch_initcall;
> - call __get_dma_ops to get the native dma_ops pointer on arm;
> - do not merge biovecs;
> - add single page optimization: pin the page rather than bouncing.
>
> Changes in v5:
> - dropped the first two patches, already in the Xen tree;
> - implement dma_mark_clean using dmac_flush_range on arm;
> - add "arm64: define DMA_ERROR_CODE"
> - better comment for XENMEM_exchange_and_pin return codes;
> - fix xen_dma_add_entry error path;
> - remove the spin_lock: the red-black tree is not modified at run time;
> - add "swiotlb-xen: introduce xen_swiotlb_set_dma_mask";
> - add "xen: introduce xen_alloc/free_coherent_pages";
> - add "swiotlb-xen: use xen_alloc/free_coherent_pages";
> - add "swiotlb: don't assume that io_tlb_start-io_tlb_end is coherent".
>
> Changes in v4:
> - rename XENMEM_get_dma_buf to XENMEM_exchange_and_pin;
> - rename XENMEM_put_dma_buf to XENMEM_unpin;
> - improve the documentation of the new hypercalls;
> - add a note about out.address_bits for XENMEM_exchange;
> - code style fixes;
> - add err_out label in xen_dma_add_entry;
> - remove INVALID_ADDRESS, use DMA_ERROR_CODE instead;
> - add in-code comments regarding the usage of xen_dma_seg[0].dma_addr.
>
> Changes in v3:
> - add a patch to compile SWIOTLB without CONFIG_NEED_SG_DMA_LENGTH;
> - add a patch to compile SWIOTLB_XEN without CONFIG_NEED_SG_DMA_LENGTH;
> - arm/dma_capable: do not treat dma_mask as a limit;
> - arm/dmabounce: keep using arm_dma_ops;
> - add missing __init in xen_early_init declaration;
> - many code style and name changes in swiotlb-xen.c;
> - improve error checks in xen_dma_add_entry;
> - warn on XENMEM_put_dma_buf failures.
>
> Changes in v2:
> - fixed a couple of errors in xen_bus_to_phys, xen_phys_to_bus and
> xen_swiotlb_fixup.
>
>
>
>
> Julien Grall (1):
> ASoC: Samsung: Rename dma_ops by samsung_dma_ops
>
> Stefano Stabellini (18):
> arm: make SWIOTLB available
> arm64: define DMA_ERROR_CODE
> arm/xen,arm64/xen: introduce p2m
> xen/x86: allow __set_phys_to_machine for autotranslate guests
> xen: make xen_create_contiguous_region return the dma address
> xen/arm,arm64: enable SWIOTLB_XEN
> swiotlb-xen: introduce xen_swiotlb_set_dma_mask
> arm/xen: get_dma_ops: return xen_dma_ops if we are running as xen_initial_domain
> arm64/xen: get_dma_ops: return xen_dma_ops if we are running as xen_initial_domain
> xen: introduce xen_alloc/free_coherent_pages
> swiotlb-xen: use xen_alloc/free_coherent_pages
> xen: introduce xen_dma_map/unmap_page and xen_dma_sync_single_for_cpu/device
> swiotlb-xen: use xen_dma_map/unmap_page, xen_dma_sync_single_for_cpu/device
> swiotlb: print a warning when the swiotlb is full
> arm,arm64: do not always merge biovec if we are running on Xen
> grant-table: call set_phys_to_machine after mapping grant refs
> swiotlb-xen: no need to do execute two bus_to_phys translations in a row
> swiotlb-xen: static inline xen_phys_to_bus, xen_bus_to_phys, xen_virt_to_bus and range_straddles_page_boundary
>
> arch/arm/Kconfig | 7 +
> arch/arm/include/asm/dma-mapping.h | 50 +++++++-
> arch/arm/include/asm/io.h | 8 +
> arch/arm/include/asm/xen/hypervisor.h | 2 +
> arch/arm/include/asm/xen/page-coherent.h | 50 +++++++
> arch/arm/include/asm/xen/page.h | 50 ++++++-
> arch/arm/xen/Makefile | 2 +-
> arch/arm/xen/mm.c | 65 +++++++++
> arch/arm/xen/p2m.c | 208 ++++++++++++++++++++++++++++
> arch/arm64/Kconfig | 1 +
> arch/arm64/include/asm/dma-mapping.h | 14 ++-
> arch/arm64/include/asm/io.h | 9 ++
> arch/arm64/include/asm/xen/page-coherent.h | 47 +++++++
> arch/arm64/xen/Makefile | 2 +-
> arch/ia64/include/asm/xen/page-coherent.h | 38 +++++
> arch/x86/include/asm/xen/page-coherent.h | 38 +++++
> arch/x86/xen/mmu.c | 11 +-
> arch/x86/xen/p2m.c | 6 +-
> drivers/xen/Kconfig | 1 -
> drivers/xen/grant-table.c | 17 ++-
> drivers/xen/swiotlb-xen.c | 116 ++++++++++++----
> include/xen/swiotlb-xen.h | 2 +
> include/xen/xen-ops.h | 7 +-
> lib/swiotlb.c | 1 +
> sound/soc/samsung/dma.c | 4 +-
> 25 files changed, 704 insertions(+), 52 deletions(-)
> create mode 100644 arch/arm/include/asm/xen/page-coherent.h
> create mode 100644 arch/arm/xen/mm.c
> create mode 100644 arch/arm/xen/p2m.c
> create mode 100644 arch/arm64/include/asm/xen/page-coherent.h
> create mode 100644 arch/ia64/include/asm/xen/page-coherent.h
> create mode 100644 arch/x86/include/asm/xen/page-coherent.h
>
>
> git://git.kernel.org/pub/scm/linux/kernel/git/sstabellini/xen.git swiotlb-xen-8
(Add samsung sounds maintainers)
On 10/17/2013 06:43 PM, Stefano Stabellini wrote:
> From: Julien Grall <[email protected]>
>
> The commit "arm: introduce a global dma_ops pointer" introduce compilation issue
> when CONFIG_SND_SOC_SAMSUNG is enabled.
>
> sound/soc/samsung/dma.c:345:27: error: conflicting types for 'dma_ops'
> /local/home/julien/works/arndale/linux/arch/arm/include/asm/dma-mapping.h:16:28:
> note: previous declaration of 'dma_ops' was here
>
> Signed-off-by: Julien Grall <[email protected]>
> Signed-off-by: Stefano Stabellini <[email protected]>
> ---
> sound/soc/samsung/dma.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/samsung/dma.c b/sound/soc/samsung/dma.c
> index 21b7926..827c0d8 100644
> --- a/sound/soc/samsung/dma.c
> +++ b/sound/soc/samsung/dma.c
> @@ -341,7 +341,7 @@ static int dma_mmap(struct snd_pcm_substream *substream,
> runtime->dma_bytes);
> }
>
> -static struct snd_pcm_ops dma_ops = {
> +static struct snd_pcm_ops samsung_dma_ops = {
> .open = dma_open,
> .close = dma_close,
> .ioctl = snd_pcm_lib_ioctl,
> @@ -428,7 +428,7 @@ out:
> }
>
> static struct snd_soc_platform_driver samsung_asoc_platform = {
> - .ops = &dma_ops,
> + .ops = &samsung_dma_ops,
> .pcm_new = dma_new,
> .pcm_free = dma_free_dma_buffers,
> };
>
--
Julien Grall
On Wed, 23 Oct 2013, Konrad Rzeszutek Wilk wrote:
> On Thu, Oct 17, 2013 at 06:43:32PM +0100, Stefano Stabellini wrote:
> > When mapping/unmapping grant refs, call
> > set_phys_to_machine to update the P2M with the new mappings for
> > autotranslate guests.
>
> Heh. That sounds so odd (p2m + xlat).
> Can you add a sentence saying that this is a mostly a nop on x86 please?
>
> (Well, the call to set_phys_to_machine does end up with this check:
>
> if (unlikely(xen_feature(XENFEAT_auto_translated_physmap))) {
> BUG_ON(pfn != mfn && mfn != INVALID_P2M_ENTRY);
> return true;
> }
> But that should be a fairly small amount of code.
>
Sure, I'll add a comment.
> >
> > Signed-off-by: Stefano Stabellini <[email protected]>
> > ---
> > drivers/xen/grant-table.c | 17 +++++++++++++++--
> > 1 files changed, 15 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
> > index 04cdeb8..a36cb03 100644
> > --- a/drivers/xen/grant-table.c
> > +++ b/drivers/xen/grant-table.c
> > @@ -49,6 +49,7 @@
> > #include <xen/grant_table.h>
> > #include <xen/interface/memory.h>
> > #include <xen/hvc-console.h>
> > +#include <xen/swiotlb-xen.h>
> > #include <asm/xen/hypercall.h>
> > #include <asm/xen/interface.h>
> >
> > @@ -889,8 +890,15 @@ int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops,
> > gnttab_retry_eagain_gop(GNTTABOP_map_grant_ref, map_ops + i,
> > &map_ops[i].status, __func__);
> >
> > - if (xen_feature(XENFEAT_auto_translated_physmap))
> > + if (xen_feature(XENFEAT_auto_translated_physmap)) {
> > + for (i = 0; i < count; i++) {
> > + if (map_ops[i].status)
> > + continue;
> > + set_phys_to_machine(map_ops[i].host_addr >> PAGE_SHIFT,
> > + map_ops[i].dev_bus_addr >> PAGE_SHIFT);
> > + }
> > return ret;
> > + }
> >
> > if (!in_interrupt() && paravirt_get_lazy_mode() == PARAVIRT_LAZY_NONE) {
> > arch_enter_lazy_mmu_mode();
> > @@ -933,8 +941,13 @@ int gnttab_unmap_refs(struct gnttab_unmap_grant_ref *unmap_ops,
> > if (ret)
> > return ret;
> >
> > - if (xen_feature(XENFEAT_auto_translated_physmap))
> > + if (xen_feature(XENFEAT_auto_translated_physmap)) {
> > + for (i = 0; i < count; i++) {
> > + set_phys_to_machine(unmap_ops[i].host_addr >> PAGE_SHIFT,
> > + INVALID_P2M_ENTRY);
> > + }
> > return ret;
> > + }
> >
> > if (!in_interrupt() && paravirt_get_lazy_mode() == PARAVIRT_LAZY_NONE) {
> > arch_enter_lazy_mmu_mode();
> > --
> > 1.7.2.5
> >
>
On Wed, 23 Oct 2013, Konrad Rzeszutek Wilk wrote:
> On Thu, Oct 17, 2013 at 06:43:28PM +0100, Stefano Stabellini wrote:
> > Call xen_dma_map_page, xen_dma_unmap_page, xen_dma_sync_single_for_cpu,
> > xen_dma_sync_single_for_device from swiotlb-xen to ensure cpu/device
> > coherency of the pages used for DMA, including the ones belonging to the
> > swiotlb buffer.
>
> You lost me.
>
> Isn't it the driver's responsibility to do this?
>
> Looking at what 'xen_dma_map_page()' does for x86 it looks to add an extra
> call - page_to_phys - and we ignore it here.
map_page on arm calls the right cache flushes needed to communicate with
the device. Same with unmap_page.
On x86 they are basically nop.
Only map_page and unmap_page know exactly what needs to be done to
map a page for dma, this is why we need to call them here.
> > Signed-off-by: Stefano Stabellini <[email protected]>
> > ---
> > drivers/xen/swiotlb-xen.c | 39 +++++++++++++++++++++++++++++++--------
> > 1 files changed, 31 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> > index 189b8db..4221cb5 100644
> > --- a/drivers/xen/swiotlb-xen.c
> > +++ b/drivers/xen/swiotlb-xen.c
> > @@ -378,8 +378,13 @@ dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
> > * buffering it.
> > */
> > if (dma_capable(dev, dev_addr, size) &&
> > - !range_straddles_page_boundary(phys, size) && !swiotlb_force)
> > + !range_straddles_page_boundary(phys, size) && !swiotlb_force) {
> > + /* we are not interested in the dma_addr returned by
> > + * xen_dma_map_page, only in the potential cache flushes executed
> > + * by the function. */
> > + xen_dma_map_page(dev, page, offset, size, dir, attrs);
> > return dev_addr;
> > + }
> >
> > /*
> > * Oh well, have to allocate and map a bounce buffer.
> > @@ -388,6 +393,8 @@ dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
> > if (map == SWIOTLB_MAP_ERROR)
> > return DMA_ERROR_CODE;
> >
> > + xen_dma_map_page(dev, pfn_to_page(map >> PAGE_SHIFT),
> > + map & ~PAGE_MASK, size, dir, attrs);
> > dev_addr = xen_phys_to_bus(map);
> >
> > /*
> > @@ -410,12 +417,15 @@ EXPORT_SYMBOL_GPL(xen_swiotlb_map_page);
> > * whatever the device wrote there.
> > */
> > static void xen_unmap_single(struct device *hwdev, dma_addr_t dev_addr,
> > - size_t size, enum dma_data_direction dir)
> > + size_t size, enum dma_data_direction dir,
> > + struct dma_attrs *attrs)
> > {
> > phys_addr_t paddr = xen_bus_to_phys(dev_addr);
> >
> > BUG_ON(dir == DMA_NONE);
> >
> > + xen_dma_unmap_page(hwdev, paddr, size, dir, attrs);
> > +
> > /* NOTE: We use dev_addr here, not paddr! */
> > if (is_xen_swiotlb_buffer(dev_addr)) {
> > swiotlb_tbl_unmap_single(hwdev, paddr, size, dir);
> > @@ -438,7 +448,7 @@ void xen_swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
> > size_t size, enum dma_data_direction dir,
> > struct dma_attrs *attrs)
> > {
> > - xen_unmap_single(hwdev, dev_addr, size, dir);
> > + xen_unmap_single(hwdev, dev_addr, size, dir, attrs);
> > }
> > EXPORT_SYMBOL_GPL(xen_swiotlb_unmap_page);
> >
> > @@ -461,11 +471,15 @@ xen_swiotlb_sync_single(struct device *hwdev, dma_addr_t dev_addr,
> >
> > BUG_ON(dir == DMA_NONE);
> >
> > + if (target == SYNC_FOR_CPU)
> > + xen_dma_sync_single_for_cpu(hwdev, paddr, size, dir);
> > +
> > /* NOTE: We use dev_addr here, not paddr! */
> > - if (is_xen_swiotlb_buffer(dev_addr)) {
> > + if (is_xen_swiotlb_buffer(dev_addr))
> > swiotlb_tbl_sync_single(hwdev, paddr, size, dir, target);
> > - return;
> > - }
> > +
> > + if (target == SYNC_FOR_DEVICE)
> > + xen_dma_sync_single_for_cpu(hwdev, paddr, size, dir);
> >
> > if (dir != DMA_FROM_DEVICE)
> > return;
> > @@ -536,8 +550,17 @@ xen_swiotlb_map_sg_attrs(struct device *hwdev, struct scatterlist *sgl,
> > return DMA_ERROR_CODE;
> > }
> > sg->dma_address = xen_phys_to_bus(map);
> > - } else
> > + } else {
> > + /* we are not interested in the dma_addr returned by
> > + * xen_dma_map_page, only in the potential cache flushes executed
> > + * by the function. */
> > + xen_dma_map_page(hwdev, pfn_to_page(paddr >> PAGE_SHIFT),
> > + paddr & ~PAGE_MASK,
> > + sg->length,
> > + dir,
> > + attrs);
> > sg->dma_address = dev_addr;
> > + }
> > sg_dma_len(sg) = sg->length;
> > }
> > return nelems;
> > @@ -559,7 +582,7 @@ xen_swiotlb_unmap_sg_attrs(struct device *hwdev, struct scatterlist *sgl,
> > BUG_ON(dir == DMA_NONE);
> >
> > for_each_sg(sgl, sg, nelems, i)
> > - xen_unmap_single(hwdev, sg->dma_address, sg_dma_len(sg), dir);
> > + xen_unmap_single(hwdev, sg->dma_address, sg_dma_len(sg), dir, attrs);
> >
> > }
> > EXPORT_SYMBOL_GPL(xen_swiotlb_unmap_sg_attrs);
> > --
> > 1.7.2.5
> >
>
> On Thu, Oct 17, 2013 at 06:43:33PM +0100, Stefano Stabellini wrote:
> > xen_unmap_single calls xen_bus_to_phys, then passes dev_addr to
> > is_xen_swiotlb_buffer that calls xen_bus_to_phys again.
>
> It does?
>
> I see it call mfn_to_local_pfn which calls pfn_to_mfn and
> get_phys_to_machine (which I presume is going to be a slow
> operation on ARM).
Yeah, I am trying to reduce the number of pfn_to_mfn and mfn_to_pfn
translations.
That said, this patch is not needed to have a fully functional
swiotlb-xen on arm and I would OK with postponing it.
> > xen_bus_to_phys is potentially a slow operation that might involve
> > walking a tree.
> > We can avoid calling xen_bus_to_phys twice by removing the
> > is_xen_swiotlb_buffer call and doing the check directly in
> > xen_unmap_single using the physical address.
> >
> > Signed-off-by: Stefano Stabellini <[email protected]>
>
> Have you tested this with a PFN that is in a foreign domain?
That would be blkback. I only tested blkback on ARM, I'll test on x86
too.
> I am bit uncomfortable with this - I recall when developing this
> that I had hit some odd cases of doing DMA of a page to a guest
> and it not always working. It was the case of not detecting whether
> the pfn was foreign or not. Sadly I can't recall the details.
>
> I would think that this problem would also be present with ARM?
> But perhaps all the new changes in the pfn_to_mfn that had lately
> been done have fixed this?
No, because on ARM the guest is autotranslate so the pfn returned by
mfn_to_pfn is always "local".
However thanks to the m2p_override now we should get the "local" pfn
from mfn_to_pfn on x86 too.
I'll test this case though.
> > ---
> > drivers/xen/swiotlb-xen.c | 4 ++--
> > 1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> > index 4d50058..6d9ca61 100644
> > --- a/drivers/xen/swiotlb-xen.c
> > +++ b/drivers/xen/swiotlb-xen.c
> > @@ -426,8 +426,8 @@ static void xen_unmap_single(struct device *hwdev, dma_addr_t dev_addr,
> >
> > xen_dma_unmap_page(hwdev, paddr, size, dir, attrs);
> >
> > - /* NOTE: We use dev_addr here, not paddr! */
> > - if (is_xen_swiotlb_buffer(dev_addr)) {
> > + if (paddr >= virt_to_phys(xen_io_tlb_start) &&
> > + paddr < virt_to_phys(xen_io_tlb_end)) {
> > swiotlb_tbl_unmap_single(hwdev, paddr, size, dir);
> > return;
> > }
> > --
> > 1.7.2.5
> >
On Wed, Oct 23, 2013 at 06:20:25PM +0100, Stefano Stabellini wrote:
> On Wed, 23 Oct 2013, Konrad Rzeszutek Wilk wrote:
> > On Thu, Oct 17, 2013 at 06:43:28PM +0100, Stefano Stabellini wrote:
> > > Call xen_dma_map_page, xen_dma_unmap_page, xen_dma_sync_single_for_cpu,
> > > xen_dma_sync_single_for_device from swiotlb-xen to ensure cpu/device
> > > coherency of the pages used for DMA, including the ones belonging to the
> > > swiotlb buffer.
> >
> > You lost me.
> >
> > Isn't it the driver's responsibility to do this?
> >
> > Looking at what 'xen_dma_map_page()' does for x86 it looks to add an extra
> > call - page_to_phys - and we ignore it here.
>
> map_page on arm calls the right cache flushes needed to communicate with
> the device. Same with unmap_page.
If this is flushing the cache then I think it makes more sense to do
that without this fancy 'dma_map_page'.
Just call it 'xen_flush_dma_page' and make it a nop on all platforms
except ARM.
> On x86 they are basically nop.
It calls page_to_phys in your patch. That is hardly nop.
> Only map_page and unmap_page know exactly what needs to be done to
> map a page for dma, this is why we need to call them here.
>
>
> > > Signed-off-by: Stefano Stabellini <[email protected]>
> > > ---
> > > drivers/xen/swiotlb-xen.c | 39 +++++++++++++++++++++++++++++++--------
> > > 1 files changed, 31 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> > > index 189b8db..4221cb5 100644
> > > --- a/drivers/xen/swiotlb-xen.c
> > > +++ b/drivers/xen/swiotlb-xen.c
> > > @@ -378,8 +378,13 @@ dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
> > > * buffering it.
> > > */
> > > if (dma_capable(dev, dev_addr, size) &&
> > > - !range_straddles_page_boundary(phys, size) && !swiotlb_force)
> > > + !range_straddles_page_boundary(phys, size) && !swiotlb_force) {
> > > + /* we are not interested in the dma_addr returned by
> > > + * xen_dma_map_page, only in the potential cache flushes executed
> > > + * by the function. */
> > > + xen_dma_map_page(dev, page, offset, size, dir, attrs);
> > > return dev_addr;
> > > + }
> > >
> > > /*
> > > * Oh well, have to allocate and map a bounce buffer.
> > > @@ -388,6 +393,8 @@ dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
> > > if (map == SWIOTLB_MAP_ERROR)
> > > return DMA_ERROR_CODE;
> > >
> > > + xen_dma_map_page(dev, pfn_to_page(map >> PAGE_SHIFT),
> > > + map & ~PAGE_MASK, size, dir, attrs);
> > > dev_addr = xen_phys_to_bus(map);
> > >
> > > /*
> > > @@ -410,12 +417,15 @@ EXPORT_SYMBOL_GPL(xen_swiotlb_map_page);
> > > * whatever the device wrote there.
> > > */
> > > static void xen_unmap_single(struct device *hwdev, dma_addr_t dev_addr,
> > > - size_t size, enum dma_data_direction dir)
> > > + size_t size, enum dma_data_direction dir,
> > > + struct dma_attrs *attrs)
> > > {
> > > phys_addr_t paddr = xen_bus_to_phys(dev_addr);
> > >
> > > BUG_ON(dir == DMA_NONE);
> > >
> > > + xen_dma_unmap_page(hwdev, paddr, size, dir, attrs);
> > > +
> > > /* NOTE: We use dev_addr here, not paddr! */
> > > if (is_xen_swiotlb_buffer(dev_addr)) {
> > > swiotlb_tbl_unmap_single(hwdev, paddr, size, dir);
> > > @@ -438,7 +448,7 @@ void xen_swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
> > > size_t size, enum dma_data_direction dir,
> > > struct dma_attrs *attrs)
> > > {
> > > - xen_unmap_single(hwdev, dev_addr, size, dir);
> > > + xen_unmap_single(hwdev, dev_addr, size, dir, attrs);
> > > }
> > > EXPORT_SYMBOL_GPL(xen_swiotlb_unmap_page);
> > >
> > > @@ -461,11 +471,15 @@ xen_swiotlb_sync_single(struct device *hwdev, dma_addr_t dev_addr,
> > >
> > > BUG_ON(dir == DMA_NONE);
> > >
> > > + if (target == SYNC_FOR_CPU)
> > > + xen_dma_sync_single_for_cpu(hwdev, paddr, size, dir);
> > > +
> > > /* NOTE: We use dev_addr here, not paddr! */
> > > - if (is_xen_swiotlb_buffer(dev_addr)) {
> > > + if (is_xen_swiotlb_buffer(dev_addr))
> > > swiotlb_tbl_sync_single(hwdev, paddr, size, dir, target);
> > > - return;
> > > - }
> > > +
> > > + if (target == SYNC_FOR_DEVICE)
> > > + xen_dma_sync_single_for_cpu(hwdev, paddr, size, dir);
> > >
> > > if (dir != DMA_FROM_DEVICE)
> > > return;
> > > @@ -536,8 +550,17 @@ xen_swiotlb_map_sg_attrs(struct device *hwdev, struct scatterlist *sgl,
> > > return DMA_ERROR_CODE;
> > > }
> > > sg->dma_address = xen_phys_to_bus(map);
> > > - } else
> > > + } else {
> > > + /* we are not interested in the dma_addr returned by
> > > + * xen_dma_map_page, only in the potential cache flushes executed
> > > + * by the function. */
> > > + xen_dma_map_page(hwdev, pfn_to_page(paddr >> PAGE_SHIFT),
> > > + paddr & ~PAGE_MASK,
> > > + sg->length,
> > > + dir,
> > > + attrs);
> > > sg->dma_address = dev_addr;
> > > + }
> > > sg_dma_len(sg) = sg->length;
> > > }
> > > return nelems;
> > > @@ -559,7 +582,7 @@ xen_swiotlb_unmap_sg_attrs(struct device *hwdev, struct scatterlist *sgl,
> > > BUG_ON(dir == DMA_NONE);
> > >
> > > for_each_sg(sgl, sg, nelems, i)
> > > - xen_unmap_single(hwdev, sg->dma_address, sg_dma_len(sg), dir);
> > > + xen_unmap_single(hwdev, sg->dma_address, sg_dma_len(sg), dir, attrs);
> > >
> > > }
> > > EXPORT_SYMBOL_GPL(xen_swiotlb_unmap_sg_attrs);
> > > --
> > > 1.7.2.5
> > >
> >
On Wed, Oct 23, 2013 at 06:34:13PM +0100, Stefano Stabellini wrote:
> > On Thu, Oct 17, 2013 at 06:43:33PM +0100, Stefano Stabellini wrote:
> > > xen_unmap_single calls xen_bus_to_phys, then passes dev_addr to
> > > is_xen_swiotlb_buffer that calls xen_bus_to_phys again.
> >
> > It does?
> >
> > I see it call mfn_to_local_pfn which calls pfn_to_mfn and
> > get_phys_to_machine (which I presume is going to be a slow
> > operation on ARM).
>
> Yeah, I am trying to reduce the number of pfn_to_mfn and mfn_to_pfn
> translations.
> That said, this patch is not needed to have a fully functional
> swiotlb-xen on arm and I would OK with postponing it.
OK, lets do that as it requires some hard thinking.
>
>
> > > xen_bus_to_phys is potentially a slow operation that might involve
> > > walking a tree.
> > > We can avoid calling xen_bus_to_phys twice by removing the
> > > is_xen_swiotlb_buffer call and doing the check directly in
> > > xen_unmap_single using the physical address.
> > >
> > > Signed-off-by: Stefano Stabellini <[email protected]>
> >
> > Have you tested this with a PFN that is in a foreign domain?
>
> That would be blkback. I only tested blkback on ARM, I'll test on x86
> too.
>
>
> > I am bit uncomfortable with this - I recall when developing this
> > that I had hit some odd cases of doing DMA of a page to a guest
> > and it not always working. It was the case of not detecting whether
> > the pfn was foreign or not. Sadly I can't recall the details.
> >
> > I would think that this problem would also be present with ARM?
> > But perhaps all the new changes in the pfn_to_mfn that had lately
> > been done have fixed this?
>
> No, because on ARM the guest is autotranslate so the pfn returned by
> mfn_to_pfn is always "local".
> However thanks to the m2p_override now we should get the "local" pfn
> from mfn_to_pfn on x86 too.
> I'll test this case though.
Right, so maybe the pfn_to_mfn_local test is useless nowadays?
>
>
> > > ---
> > > drivers/xen/swiotlb-xen.c | 4 ++--
> > > 1 files changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> > > index 4d50058..6d9ca61 100644
> > > --- a/drivers/xen/swiotlb-xen.c
> > > +++ b/drivers/xen/swiotlb-xen.c
> > > @@ -426,8 +426,8 @@ static void xen_unmap_single(struct device *hwdev, dma_addr_t dev_addr,
> > >
> > > xen_dma_unmap_page(hwdev, paddr, size, dir, attrs);
> > >
> > > - /* NOTE: We use dev_addr here, not paddr! */
> > > - if (is_xen_swiotlb_buffer(dev_addr)) {
> > > + if (paddr >= virt_to_phys(xen_io_tlb_start) &&
> > > + paddr < virt_to_phys(xen_io_tlb_end)) {
> > > swiotlb_tbl_unmap_single(hwdev, paddr, size, dir);
> > > return;
> > > }
> > > --
> > > 1.7.2.5
> > >
On Wed, 23 Oct 2013, Konrad Rzeszutek Wilk wrote:
> On Wed, Oct 23, 2013 at 06:34:13PM +0100, Stefano Stabellini wrote:
> > > On Thu, Oct 17, 2013 at 06:43:33PM +0100, Stefano Stabellini wrote:
> > > > xen_unmap_single calls xen_bus_to_phys, then passes dev_addr to
> > > > is_xen_swiotlb_buffer that calls xen_bus_to_phys again.
> > >
> > > It does?
> > >
> > > I see it call mfn_to_local_pfn which calls pfn_to_mfn and
> > > get_phys_to_machine (which I presume is going to be a slow
> > > operation on ARM).
> >
> > Yeah, I am trying to reduce the number of pfn_to_mfn and mfn_to_pfn
> > translations.
> > That said, this patch is not needed to have a fully functional
> > swiotlb-xen on arm and I would OK with postponing it.
>
> OK, lets do that as it requires some hard thinking.
OK.
> > > I am bit uncomfortable with this - I recall when developing this
> > > that I had hit some odd cases of doing DMA of a page to a guest
> > > and it not always working. It was the case of not detecting whether
> > > the pfn was foreign or not. Sadly I can't recall the details.
> > >
> > > I would think that this problem would also be present with ARM?
> > > But perhaps all the new changes in the pfn_to_mfn that had lately
> > > been done have fixed this?
> >
> > No, because on ARM the guest is autotranslate so the pfn returned by
> > mfn_to_pfn is always "local".
> > However thanks to the m2p_override now we should get the "local" pfn
> > from mfn_to_pfn on x86 too.
> > I'll test this case though.
>
> Right, so maybe the pfn_to_mfn_local test is useless nowadays?
Yes, I think so. This might be better handled by removing
mfn_to_local_pfn altogether.
On Wed, 23 Oct 2013, Konrad Rzeszutek Wilk wrote:
> On Wed, Oct 23, 2013 at 06:20:25PM +0100, Stefano Stabellini wrote:
> > On Wed, 23 Oct 2013, Konrad Rzeszutek Wilk wrote:
> > > On Thu, Oct 17, 2013 at 06:43:28PM +0100, Stefano Stabellini wrote:
> > > > Call xen_dma_map_page, xen_dma_unmap_page, xen_dma_sync_single_for_cpu,
> > > > xen_dma_sync_single_for_device from swiotlb-xen to ensure cpu/device
> > > > coherency of the pages used for DMA, including the ones belonging to the
> > > > swiotlb buffer.
> > >
> > > You lost me.
> > >
> > > Isn't it the driver's responsibility to do this?
> > >
> > > Looking at what 'xen_dma_map_page()' does for x86 it looks to add an extra
> > > call - page_to_phys - and we ignore it here.
> >
> > map_page on arm calls the right cache flushes needed to communicate with
> > the device. Same with unmap_page.
>
> If this is flushing the cache then I think it makes more sense to do
> that without this fancy 'dma_map_page'.
>
> Just call it 'xen_flush_dma_page' and make it a nop on all platforms
> except ARM.
I am OK with making it a nop on x86, it makes sense.
However I would like to keep it called xen_dma_map_page: after all it
corresponds exactly to the native map_page dma_op. It is part of the same
"contract".
> > On x86 they are basically nop.
>
> It calls page_to_phys in your patch. That is hardly nop.
I see. It is certainly worth optimizing it out on x86.
Of course if one day the x86 map_page dma_op starts doing something
useful, we can go back to call it from xen_dma_map_page.
On Thu, 24 Oct 2013, Stefano Stabellini wrote:
> On Wed, 23 Oct 2013, Konrad Rzeszutek Wilk wrote:
> > On Wed, Oct 23, 2013 at 06:20:25PM +0100, Stefano Stabellini wrote:
> > > On Wed, 23 Oct 2013, Konrad Rzeszutek Wilk wrote:
> > > > On Thu, Oct 17, 2013 at 06:43:28PM +0100, Stefano Stabellini wrote:
> > > > > Call xen_dma_map_page, xen_dma_unmap_page, xen_dma_sync_single_for_cpu,
> > > > > xen_dma_sync_single_for_device from swiotlb-xen to ensure cpu/device
> > > > > coherency of the pages used for DMA, including the ones belonging to the
> > > > > swiotlb buffer.
> > > >
> > > > You lost me.
> > > >
> > > > Isn't it the driver's responsibility to do this?
> > > >
> > > > Looking at what 'xen_dma_map_page()' does for x86 it looks to add an extra
> > > > call - page_to_phys - and we ignore it here.
> > >
> > > map_page on arm calls the right cache flushes needed to communicate with
> > > the device. Same with unmap_page.
> >
> > If this is flushing the cache then I think it makes more sense to do
> > that without this fancy 'dma_map_page'.
> >
> > Just call it 'xen_flush_dma_page' and make it a nop on all platforms
> > except ARM.
>
> I am OK with making it a nop on x86, it makes sense.
> However I would like to keep it called xen_dma_map_page: after all it
> corresponds exactly to the native map_page dma_op. It is part of the same
> "contract".
for example we can turn xen_dma_map_page into a function that returns
void, this way we can remove the page_to_phys on x86
On Thu, Oct 24, 2013 at 11:44:50AM +0100, Stefano Stabellini wrote:
> On Wed, 23 Oct 2013, Konrad Rzeszutek Wilk wrote:
> > On Wed, Oct 23, 2013 at 06:20:25PM +0100, Stefano Stabellini wrote:
> > > On Wed, 23 Oct 2013, Konrad Rzeszutek Wilk wrote:
> > > > On Thu, Oct 17, 2013 at 06:43:28PM +0100, Stefano Stabellini wrote:
> > > > > Call xen_dma_map_page, xen_dma_unmap_page, xen_dma_sync_single_for_cpu,
> > > > > xen_dma_sync_single_for_device from swiotlb-xen to ensure cpu/device
> > > > > coherency of the pages used for DMA, including the ones belonging to the
> > > > > swiotlb buffer.
> > > >
> > > > You lost me.
> > > >
> > > > Isn't it the driver's responsibility to do this?
> > > >
> > > > Looking at what 'xen_dma_map_page()' does for x86 it looks to add an extra
> > > > call - page_to_phys - and we ignore it here.
> > >
> > > map_page on arm calls the right cache flushes needed to communicate with
> > > the device. Same with unmap_page.
> >
> > If this is flushing the cache then I think it makes more sense to do
> > that without this fancy 'dma_map_page'.
> >
> > Just call it 'xen_flush_dma_page' and make it a nop on all platforms
> > except ARM.
>
> I am OK with making it a nop on x86, it makes sense.
> However I would like to keep it called xen_dma_map_page: after all it
> corresponds exactly to the native map_page dma_op. It is part of the same
> "contract".
OK.
>
>
> > > On x86 they are basically nop.
> >
> > It calls page_to_phys in your patch. That is hardly nop.
>
> I see. It is certainly worth optimizing it out on x86.
> Of course if one day the x86 map_page dma_op starts doing something
> useful, we can go back to call it from xen_dma_map_page.
Sure.
On Thu, Oct 24, 2013 at 02:11:48PM +0100, Stefano Stabellini wrote:
> On Thu, 24 Oct 2013, Stefano Stabellini wrote:
> > On Wed, 23 Oct 2013, Konrad Rzeszutek Wilk wrote:
> > > On Wed, Oct 23, 2013 at 06:20:25PM +0100, Stefano Stabellini wrote:
> > > > On Wed, 23 Oct 2013, Konrad Rzeszutek Wilk wrote:
> > > > > On Thu, Oct 17, 2013 at 06:43:28PM +0100, Stefano Stabellini wrote:
> > > > > > Call xen_dma_map_page, xen_dma_unmap_page, xen_dma_sync_single_for_cpu,
> > > > > > xen_dma_sync_single_for_device from swiotlb-xen to ensure cpu/device
> > > > > > coherency of the pages used for DMA, including the ones belonging to the
> > > > > > swiotlb buffer.
> > > > >
> > > > > You lost me.
> > > > >
> > > > > Isn't it the driver's responsibility to do this?
> > > > >
> > > > > Looking at what 'xen_dma_map_page()' does for x86 it looks to add an extra
> > > > > call - page_to_phys - and we ignore it here.
> > > >
> > > > map_page on arm calls the right cache flushes needed to communicate with
> > > > the device. Same with unmap_page.
> > >
> > > If this is flushing the cache then I think it makes more sense to do
> > > that without this fancy 'dma_map_page'.
> > >
> > > Just call it 'xen_flush_dma_page' and make it a nop on all platforms
> > > except ARM.
> >
> > I am OK with making it a nop on x86, it makes sense.
> > However I would like to keep it called xen_dma_map_page: after all it
> > corresponds exactly to the native map_page dma_op. It is part of the same
> > "contract".
>
> for example we can turn xen_dma_map_page into a function that returns
> void, this way we can remove the page_to_phys on x86
Right, the v9 patchset you posted fixes my concern.