thr3ads.net - Xen devel - [PATCH] Xen-SWIOTLB fixes (v4) for v3.7 [Sep 2012]

If this information is useful, please help other people find it:
Share via:

Konrad Rzeszutek Wilk

2012-Sep-10 19:45 UTC

[PATCH] Xen-SWIOTLB fixes (v4) for v3.7

The original problem was:
<begin>
"if one boots a PV 64-bit guests with more than 4GB, the SWIOTLB [Xen]
gets turned on - and 64MB of precious low-memory gets used." was totally
bogus. The SWIOTLB that gets turned on is the *native* one - which does
not exhaust any low-memory of the host. But it does eat up perfectly
fine 64MB of the guest and never gets used.

So this patchset has some things I wanted to do for some time:
 [PATCH 01/10] xen/swiotlb: Simplify the logic.

Just so that next time I am not confused.

 [PATCH 02/10] xen/swiotlb: With more than 4GB on 64-bit, disable the

and don''t turn the *native* SWIOTLB on PV guests and waste those 64MB.
<end>


The rest are  exciting new patches - basically I want to emulate what
IA64 does which is to turn on the SWIOTLB late in the bootup cycle.
This means not using the alloc_bootmem and having a "late" variant
to initialize SWIOTLB. There is some surgery in the SWIOTLB library:
 [PATCH 03/10] swiotlb: add the late swiotlb initialization function

to allow it to use an io_tlb passed in. Note: I hadn''t tested this
on IA64 and that is something I need to do.

And then the implementation in the Xen-SWIOTLB to use it:
 [PATCH 06/10] xen/swiotlb: Use the swiotlb_late_init_with_tbl to
along with Xen PCI frontend to utilize it.
 [PATCH 07/10] xen/pcifront: Use Xen-SWIOTLB when initting if

The end result is that a PV guest can now dynamically(*) deal with
PCI passthrough cards. I say "dynamically" b/c if one boots a PV guest
with more than 3GB without using ''e820_hole'' (or is it called
''e820_host''
now?) the PCI subsystem won''t be able to squeeze the BARs as they
are RAM occupied. The workaround is to boot with ''e820_hole''
or some
new work where we manipulate at boot time the E820 to leave a nice
big 1GB hole under 4G - and with all the work on the P2M tree that
should be fairly easy actually.

Note: If one uses ''iommu=soft'' on the Linux command line, the
Xen-SWIOTLB
still gets turned on.

Konrad Rzeszutek Wilk

2012-Sep-10 19:45 UTC

head link

[PATCH 01/10] xen/swiotlb: Simplify the logic.

Its pretty easy:
 1). We only check to see if we need Xen SWIOTLB for PV guests.
 2). If swiotlb=force or iommu=soft is set, then Xen SWIOTLB will
     be enabled.
 3). If it is an initial domain, then Xen SWIOTLB will be enabled.
 4). Native SWIOTLB must be disabled for PV guests.

Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/xen/pci-swiotlb-xen.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/x86/xen/pci-swiotlb-xen.c b/arch/x86/xen/pci-swiotlb-xen.c
index 967633a..b6a5340 100644
--- a/arch/x86/xen/pci-swiotlb-xen.c
+++ b/arch/x86/xen/pci-swiotlb-xen.c
@@ -34,19 +34,20 @@ static struct dma_map_ops xen_swiotlb_dma_ops = {
 int __init pci_xen_swiotlb_detect(void)
 {
 
+	if (!xen_pv_domain())
+		return 0;
+
 	/* If running as PV guest, either iommu=soft, or swiotlb=force will
 	 * activate this IOMMU. If running as PV privileged, activate it
 	 * irregardless.
 	 */
-	if ((xen_initial_domain() || swiotlb || swiotlb_force) &&
-	    (xen_pv_domain()))
+	if ((xen_initial_domain() || swiotlb || swiotlb_force))
 		xen_swiotlb = 1;
 
 	/* If we are running under Xen, we MUST disable the native SWIOTLB.
 	 * Don''t worry about swiotlb_force flag activating the native, as
 	 * the ''swiotlb'' flag is the only one turning it on. */
-	if (xen_pv_domain())
-		swiotlb = 0;
+	swiotlb = 0;
 
 	return xen_swiotlb;
 }
-- 
1.7.7.6

Konrad Rzeszutek Wilk

2012-Sep-10 19:45 UTC

head link

[PATCH 02/10] xen/swiotlb: With more than 4GB on 64-bit, disable the native SWIOTLB.

If a PV guest is booted the native SWIOTLB should not be
turned on. It does not help us (we don''t have any PCI devices)
and it eats 64MB of good memory. In the case of PV guests
with PCI devices we need the Xen-SWIOTLB one.

[v1: Rewrite comment per Stefano''s suggestion]
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/xen/pci-swiotlb-xen.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/arch/x86/xen/pci-swiotlb-xen.c b/arch/x86/xen/pci-swiotlb-xen.c
index b6a5340..1c17227 100644
--- a/arch/x86/xen/pci-swiotlb-xen.c
+++ b/arch/x86/xen/pci-swiotlb-xen.c
@@ -8,6 +8,11 @@
 #include <xen/xen.h>
 #include <asm/iommu_table.h>
 
+#ifdef CONFIG_X86_64
+#include <asm/iommu.h>
+#include <asm/dma.h>
+#endif
+
 int xen_swiotlb __read_mostly;
 
 static struct dma_map_ops xen_swiotlb_dma_ops = {
@@ -49,6 +54,15 @@ int __init pci_xen_swiotlb_detect(void)
 	 * the ''swiotlb'' flag is the only one turning it on. */
 	swiotlb = 0;
 
+#ifdef CONFIG_X86_64
+	/* pci_swiotlb_detect_4gb turns on native SWIOTLB if no_iommu == 0
+	 * (so no iommu=X command line over-writes).
+	 * Considering that PV guests do not want the *native SWIOTLB* but
+	 * only Xen SWIOTLB it is not useful to us so set no_iommu=1 here.
+	 */
+	if (max_pfn > MAX_DMA32_PFN)
+		no_iommu = 1;
+#endif
 	return xen_swiotlb;
 }
 
-- 
1.7.7.6

Konrad Rzeszutek Wilk

2012-Sep-10 19:46 UTC

head link

[PATCH 03/10] swiotlb: add the late swiotlb initialization function with iotlb memory

This enables the caller to initialize swiotlb with its own iotlb
memory late in the bootup.

See git commit eb605a5754d050a25a9f00d718fb173f24c486ef
"swiotlb: add swiotlb_tbl_map_single library function" which will
explain the full details of what it can be used for.

CC: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
[v1: Fold in smatch warning]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 include/linux/swiotlb.h |    1 +
 lib/swiotlb.c           |   33 ++++++++++++++++++++++++---------
 2 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index e872526..8d08b3e 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -25,6 +25,7 @@ extern int swiotlb_force;
 extern void swiotlb_init(int verbose);
 extern void swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int
verbose);
 extern unsigned long swiotlb_nr_tbl(void);
+extern int swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs);
 
 /*
  * Enumeration for sync targets
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 45bc1f8..f114bf6 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -170,7 +170,7 @@ void __init swiotlb_init_with_tbl(char *tlb, unsigned long
nslabs, int verbose)
  * Statically reserve bounce buffer space and initialize bounce buffer data
  * structures for the software IO TLB used to implement the DMA API.
  */
-void __init
+static void __init
 swiotlb_init_with_default_size(size_t default_size, int verbose)
 {
 	unsigned long bytes;
@@ -206,8 +206,9 @@ swiotlb_init(int verbose)
 int
 swiotlb_late_init_with_default_size(size_t default_size)
 {
-	unsigned long i, bytes, req_nslabs = io_tlb_nslabs;
+	unsigned long bytes, req_nslabs = io_tlb_nslabs;
 	unsigned int order;
+	int rc = 0;
 
 	if (!io_tlb_nslabs) {
 		io_tlb_nslabs = (default_size >> IO_TLB_SHIFT);
@@ -229,16 +230,32 @@ swiotlb_late_init_with_default_size(size_t default_size)
 		order--;
 	}
 
-	if (!io_tlb_start)
-		goto cleanup1;
-
+	if (!io_tlb_start) {
+		io_tlb_nslabs = req_nslabs;
+		return -ENOMEM;
+	}
 	if (order != get_order(bytes)) {
 		printk(KERN_WARNING "Warning: only able to allocate %ld MB "
 		       "for software IO TLB\n", (PAGE_SIZE << order) >>
20);
 		io_tlb_nslabs = SLABS_PER_PAGE << order;
-		bytes = io_tlb_nslabs << IO_TLB_SHIFT;
 	}
+	rc = swiotlb_late_init_with_tbl(io_tlb_start, io_tlb_nslabs);
+	if (rc)
+		free_pages((unsigned long)io_tlb_start, order);
+	return rc;
+}
+
+int
+swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
+{
+	unsigned long i, bytes;
+
+	bytes = nslabs << IO_TLB_SHIFT;
+
+	io_tlb_nslabs = nslabs;
+	io_tlb_start = tlb;
 	io_tlb_end = io_tlb_start + bytes;
+
 	memset(io_tlb_start, 0, bytes);
 
 	/*
@@ -288,10 +305,8 @@ cleanup3:
 	io_tlb_list = NULL;
 cleanup2:
 	io_tlb_end = NULL;
-	free_pages((unsigned long)io_tlb_start, order);
 	io_tlb_start = NULL;
-cleanup1:
-	io_tlb_nslabs = req_nslabs;
+	io_tlb_nslabs = 0;
 	return -ENOMEM;
 }
 
-- 
1.7.7.6

Konrad Rzeszutek Wilk

2012-Sep-10 19:46 UTC

head link

[PATCH 04/10] xen/swiotlb: Move the nr_tbl determination in its own function.

Moving the function out of the way to prepare for the late
SWIOTLB init.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 drivers/xen/swiotlb-xen.c |   21 +++++++++++----------
 1 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 1afb4fb..a2aad6e 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -144,25 +144,26 @@ xen_swiotlb_fixup(void *buf, size_t size, unsigned long
nslabs)
 	} while (i < nslabs);
 	return 0;
 }
+static unsigned long xen_set_nslabs(unsigned long nr_tbl)
+{
+	if (!nr_tbl) {
+		xen_io_tlb_nslabs = (64 * 1024 * 1024 >> IO_TLB_SHIFT);
+		xen_io_tlb_nslabs = ALIGN(xen_io_tlb_nslabs, IO_TLB_SEGSIZE);
+	} else
+		xen_io_tlb_nslabs = nr_tbl;
 
+	return xen_io_tlb_nslabs << IO_TLB_SHIFT;
+}
 void __init xen_swiotlb_init(int verbose)
 {
 	unsigned long bytes;
 	int rc = -ENOMEM;
-	unsigned long nr_tbl;
 	char *m = NULL;
 	unsigned int repeat = 3;
 
-	nr_tbl = swiotlb_nr_tbl();
-	if (nr_tbl)
-		xen_io_tlb_nslabs = nr_tbl;
-	else {
-		xen_io_tlb_nslabs = (64 * 1024 * 1024 >> IO_TLB_SHIFT);
-		xen_io_tlb_nslabs = ALIGN(xen_io_tlb_nslabs, IO_TLB_SEGSIZE);
-	}
+	xen_io_tlb_nslabs = swiotlb_nr_tbl();
 retry:
-	bytes = xen_io_tlb_nslabs << IO_TLB_SHIFT;
-
+	bytes = xen_set_nslabs(xen_io_tlb_nslabs);
 	/*
 	 * Get IO TLB memory from any location.
 	 */
-- 
1.7.7.6

Konrad Rzeszutek Wilk

2012-Sep-10 19:46 UTC

head link

[PATCH 05/10] xen/swiotlb: Move the error strings to its own function.

That way we can more easily reuse those errors when using the
late SWIOTLB init.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 drivers/xen/swiotlb-xen.c |   35 +++++++++++++++++++++++++++--------
 1 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index a2aad6e..701b103 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -154,11 +154,33 @@ static unsigned long xen_set_nslabs(unsigned long nr_tbl)
 
 	return xen_io_tlb_nslabs << IO_TLB_SHIFT;
 }
+
+enum xen_swiotlb_err {
+	XEN_SWIOTLB_UNKNOWN = 0,
+	XEN_SWIOTLB_ENOMEM,
+	XEN_SWIOTLB_EFIXUP
+};
+
+static const char *xen_swiotlb_error(enum xen_swiotlb_err err)
+{
+	switch (err) {
+	case XEN_SWIOTLB_ENOMEM:
+		return "Cannot allocate Xen-SWIOTLB buffer\n";
+	case XEN_SWIOTLB_EFIXUP:
+		return "Failed to get contiguous memory for DMA from Xen!\n"\
+		    "You either: don''t have the permissions, do not have"\
+		    " enough free memory under 4GB, or the hypervisor memory"\
+		    " is too fragmented!";
+	default:
+		break;
+	}
+	return "";
+}
 void __init xen_swiotlb_init(int verbose)
 {
 	unsigned long bytes;
 	int rc = -ENOMEM;
-	char *m = NULL;
+	enum xen_swiotlb_err m_ret = XEN_SWIOTLB_UNKNOWN;
 	unsigned int repeat = 3;
 
 	xen_io_tlb_nslabs = swiotlb_nr_tbl();
@@ -169,7 +191,7 @@ retry:
 	 */
 	xen_io_tlb_start = alloc_bootmem_pages(PAGE_ALIGN(bytes));
 	if (!xen_io_tlb_start) {
-		m = "Cannot allocate Xen-SWIOTLB buffer!\n";
+		m_ret = XEN_SWIOTLB_ENOMEM;
 		goto error;
 	}
 	xen_io_tlb_end = xen_io_tlb_start + bytes;
@@ -181,10 +203,7 @@ retry:
 			       xen_io_tlb_nslabs);
 	if (rc) {
 		free_bootmem(__pa(xen_io_tlb_start), PAGE_ALIGN(bytes));
-		m = "Failed to get contiguous memory for DMA from Xen!\n"\
-		    "You either: don''t have the permissions, do not have"\
-		    " enough free memory under 4GB, or the hypervisor memory"\
-		    "is too fragmented!";
+		m_ret = XEN_SWIOTLB_EFIXUP;
 		goto error;
 	}
 	start_dma_addr = xen_virt_to_bus(xen_io_tlb_start);
@@ -199,8 +218,8 @@ error:
 		      (xen_io_tlb_nslabs << IO_TLB_SHIFT) >> 20);
 		goto retry;
 	}
-	xen_raw_printk("%s (rc:%d)", m, rc);
-	panic("%s (rc:%d)", m, rc);
+	xen_raw_printk("%s (rc:%d)", xen_swiotlb_error(m_ret), rc);
+	panic("%s (rc:%d)", xen_swiotlb_error(m_ret), rc);
 }
 
 void *
-- 
1.7.7.6

Konrad Rzeszutek Wilk

2012-Sep-10 19:46 UTC

head link

[PATCH 06/10] xen/swiotlb: Use the swiotlb_late_init_with_tbl to init Xen-SWIOTLB late when PV PCI is used.

With this patch we provide the functionality to initialize the
Xen-SWIOTLB late in the bootup cycle - specifically for
Xen PCI-frontend. We still will work if the user had
supplied ''iommu=soft'' on the Linux command line.

CC: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
[v1: Fix smatch warnings]
[v2: Added check for xen_swiotlb]
[v3: Rebased with new xen-swiotlb changes]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/include/asm/xen/swiotlb-xen.h |    2 +
 arch/x86/xen/pci-swiotlb-xen.c         |   22 ++++++++++++++-
 drivers/xen/swiotlb-xen.c              |   48 +++++++++++++++++++++++++------
 include/xen/swiotlb-xen.h              |    2 +-
 4 files changed, 62 insertions(+), 12 deletions(-)

diff --git a/arch/x86/include/asm/xen/swiotlb-xen.h
b/arch/x86/include/asm/xen/swiotlb-xen.h
index 1be1ab7..ee52fca 100644
--- a/arch/x86/include/asm/xen/swiotlb-xen.h
+++ b/arch/x86/include/asm/xen/swiotlb-xen.h
@@ -5,10 +5,12 @@
 extern int xen_swiotlb;
 extern int __init pci_xen_swiotlb_detect(void);
 extern void __init pci_xen_swiotlb_init(void);
+extern int pci_xen_swiotlb_init_late(void);
 #else
 #define xen_swiotlb (0)
 static inline int __init pci_xen_swiotlb_detect(void) { return 0; }
 static inline void __init pci_xen_swiotlb_init(void) { }
+static inline int pci_xen_swiotlb_init_late(void) { return -ENXIO; }
 #endif
 
 #endif /* _ASM_X86_SWIOTLB_XEN_H */
diff --git a/arch/x86/xen/pci-swiotlb-xen.c b/arch/x86/xen/pci-swiotlb-xen.c
index 1c17227..406f9c4 100644
--- a/arch/x86/xen/pci-swiotlb-xen.c
+++ b/arch/x86/xen/pci-swiotlb-xen.c
@@ -12,7 +12,7 @@
 #include <asm/iommu.h>
 #include <asm/dma.h>
 #endif
-
+#include <linux/export.h>
 int xen_swiotlb __read_mostly;
 
 static struct dma_map_ops xen_swiotlb_dma_ops = {
@@ -76,6 +76,26 @@ void __init pci_xen_swiotlb_init(void)
 		pci_request_acs();
 	}
 }
+
+int pci_xen_swiotlb_init_late(void)
+{
+	int rc;
+
+	if (xen_swiotlb)
+		return 0;
+
+	rc = xen_swiotlb_init(1);
+	if (rc)
+		return rc;
+
+	dma_ops = &xen_swiotlb_dma_ops;
+	/* Make sure ACS will be enabled */
+	pci_request_acs();
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pci_xen_swiotlb_init_late);
+
 IOMMU_INIT_FINISH(pci_xen_swiotlb_detect,
 		  0,
 		  pci_xen_swiotlb_init,
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 701b103..f0825cb 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -176,9 +176,9 @@ static const char *xen_swiotlb_error(enum xen_swiotlb_err
err)
 	}
 	return "";
 }
-void __init xen_swiotlb_init(int verbose)
+int __ref xen_swiotlb_init(int verbose)
 {
-	unsigned long bytes;
+	unsigned long bytes, order;
 	int rc = -ENOMEM;
 	enum xen_swiotlb_err m_ret = XEN_SWIOTLB_UNKNOWN;
 	unsigned int repeat = 3;
@@ -186,10 +186,28 @@ void __init xen_swiotlb_init(int verbose)
 	xen_io_tlb_nslabs = swiotlb_nr_tbl();
 retry:
 	bytes = xen_set_nslabs(xen_io_tlb_nslabs);
+	order = get_order(xen_io_tlb_nslabs << IO_TLB_SHIFT);
 	/*
 	 * Get IO TLB memory from any location.
 	 */
-	xen_io_tlb_start = alloc_bootmem_pages(PAGE_ALIGN(bytes));
+	if (!after_bootmem)
+		xen_io_tlb_start = alloc_bootmem_pages(PAGE_ALIGN(bytes));
+	else {
+#define SLABS_PER_PAGE (1 << (PAGE_SHIFT - IO_TLB_SHIFT))
+#define IO_TLB_MIN_SLABS ((1<<20) >> IO_TLB_SHIFT)
+		while ((SLABS_PER_PAGE << order) > IO_TLB_MIN_SLABS) {
+			xen_io_tlb_start = (void *)__get_free_pages(__GFP_NOWARN, order);
+			if (xen_io_tlb_start)
+				break;
+			order--;
+		}
+		if (order != get_order(bytes)) {
+			pr_warn("Warning: only able to allocate %ld MB "
+				"for software IO TLB\n", (PAGE_SIZE << order) >> 20);
+			xen_io_tlb_nslabs = SLABS_PER_PAGE << order;
+			bytes = xen_io_tlb_nslabs << IO_TLB_SHIFT;
+		}
+	}
 	if (!xen_io_tlb_start) {
 		m_ret = XEN_SWIOTLB_ENOMEM;
 		goto error;
@@ -202,14 +220,21 @@ retry:
 			       bytes,
 			       xen_io_tlb_nslabs);
 	if (rc) {
-		free_bootmem(__pa(xen_io_tlb_start), PAGE_ALIGN(bytes));
+		if (!after_bootmem)
+			free_bootmem(__pa(xen_io_tlb_start), PAGE_ALIGN(bytes));
+		else {
+			free_pages((unsigned long)xen_io_tlb_start, order);
+			xen_io_tlb_start = NULL;
+		}
 		m_ret = XEN_SWIOTLB_EFIXUP;
 		goto error;
 	}
 	start_dma_addr = xen_virt_to_bus(xen_io_tlb_start);
-	swiotlb_init_with_tbl(xen_io_tlb_start, xen_io_tlb_nslabs, verbose);
-
-	return;
+	if (!after_bootmem)
+		swiotlb_init_with_tbl(xen_io_tlb_start, xen_io_tlb_nslabs, verbose);
+	else
+		rc = swiotlb_late_init_with_tbl(xen_io_tlb_start, xen_io_tlb_nslabs);
+	return rc;
 error:
 	if (repeat--) {
 		xen_io_tlb_nslabs = max(1024UL, /* Min is 2MB */
@@ -218,10 +243,13 @@ error:
 		      (xen_io_tlb_nslabs << IO_TLB_SHIFT) >> 20);
 		goto retry;
 	}
-	xen_raw_printk("%s (rc:%d)", xen_swiotlb_error(m_ret), rc);
-	panic("%s (rc:%d)", xen_swiotlb_error(m_ret), rc);
+	pr_err("%s (rc:%d)", xen_swiotlb_error(m_ret), rc);
+	if (!after_bootmem)
+		panic("%s (rc:%d)", xen_swiotlb_error(m_ret), rc);
+	else
+		free_pages((unsigned long)xen_io_tlb_start, order);
+	return rc;
 }
-
 void *
 xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
 			   dma_addr_t *dma_handle, gfp_t flags,
diff --git a/include/xen/swiotlb-xen.h b/include/xen/swiotlb-xen.h
index 4f4d449..f26f9f3 100644
--- a/include/xen/swiotlb-xen.h
+++ b/include/xen/swiotlb-xen.h
@@ -3,7 +3,7 @@
 
 #include <linux/swiotlb.h>
 
-extern void xen_swiotlb_init(int verbose);
+extern int xen_swiotlb_init(int verbose);
 
 extern void
 *xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
-- 
1.7.7.6

Konrad Rzeszutek Wilk

2012-Sep-10 19:46 UTC

head link

[PATCH 07/10] xen/pcifront: Use Xen-SWIOTLB when initting if required.

We piggyback on "xen/swiotlb: Use the swiotlb_late_init_with_tbl to init
Xen-SWIOTLB late when PV PCI is used." functionality to start up
the Xen-SWIOTLB if we are hot-plugged. This allows us to bypass
the need to supply ''iommu=soft'' on the Linux command line
(mostly).
With this patch, if a user forgot ''iommu=soft'' on the command
line,
and hotplug a PCI device they will get:

pcifront pci-0: Installing PCI frontend
Warning: only able to allocate 4 MB for software IO TLB
software IO TLB [mem 0x2a000000-0x2a3fffff] (4MB) mapped at
[ffff88002a000000-ffff88002a3fffff]
pcifront pci-0: Creating PCI Frontend Bus 0000:00
pcifront pci-0: PCI host bridge to bus 0000:00
pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
pci_bus 0000:00: root bus resource [mem 0x00000000-0xfffffffff]
pci 0000:00:00.0: [8086:10d3] type 00 class 0x020000
pci 0000:00:00.0: reg 10: [mem 0xfe5c0000-0xfe5dffff]
pci 0000:00:00.0: reg 14: [mem 0xfe500000-0xfe57ffff]
pci 0000:00:00.0: reg 18: [io  0xe000-0xe01f]
pci 0000:00:00.0: reg 1c: [mem 0xfe5e0000-0xfe5e3fff]
pcifront pci-0: claiming resource 0000:00:00.0/0
pcifront pci-0: claiming resource 0000:00:00.0/1
pcifront pci-0: claiming resource 0000:00:00.0/2
pcifront pci-0: claiming resource 0000:00:00.0/3
e1000e: Intel(R) PRO/1000 Network Driver - 2.0.0-k
e1000e: Copyright(c) 1999 - 2012 Intel Corporation.
e1000e 0000:00:00.0: Disabling ASPM L0s L1
e1000e 0000:00:00.0: enabling device (0000 -> 0002)
e1000e 0000:00:00.0: Xen PCI mapped GSI16 to IRQ34
e1000e 0000:00:00.0: (unregistered net_device): Interrupt Throttling Rate
(ints/sec) set to dynamic conservative mode
e1000e 0000:00:00.0: eth0: (PCI Express:2.5GT/s:Width x1) 00:1b:21:ab:c6:13
e1000e 0000:00:00.0: eth0: Intel(R) PRO/1000 Network Connection
e1000e 0000:00:00.0: eth0: MAC: 3, PHY: 8, PBA No: E46981-005

The "Warning only" will go away if one supplies
''iommu=soft'' instead
as we have a higher chance of being able to allocate large swaths of
memory.

Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 drivers/pci/xen-pcifront.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index d6cc62c..0ad8ca3 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -21,6 +21,7 @@
 #include <linux/bitops.h>
 #include <linux/time.h>
 
+#include <asm/xen/swiotlb-xen.h>
 #define INVALID_GRANT_REF (0)
 #define INVALID_EVTCHN    (-1)
 
@@ -668,7 +669,7 @@ static irqreturn_t pcifront_handler_aer(int irq, void *dev)
 	schedule_pcifront_aer_op(pdev);
 	return IRQ_HANDLED;
 }
-static int pcifront_connect(struct pcifront_device *pdev)
+static int pcifront_connect_and_init_dma(struct pcifront_device *pdev)
 {
 	int err = 0;
 
@@ -681,9 +682,13 @@ static int pcifront_connect(struct pcifront_device *pdev)
 		dev_err(&pdev->xdev->dev, "PCI frontend already
installed!\n");
 		err = -EEXIST;
 	}
-
 	spin_unlock(&pcifront_dev_lock);
 
+	if (!err && !swiotlb_nr_tbl()) {
+		err = pci_xen_swiotlb_init_late();
+		if (err)
+			dev_err(&pdev->xdev->dev, "Could not setup SWIOTLB!\n");
+	}
 	return err;
 }
 
@@ -842,10 +847,10 @@ static int __devinit pcifront_try_connect(struct
pcifront_device *pdev)
 	    XenbusStateInitialised)
 		goto out;
 
-	err = pcifront_connect(pdev);
+	err = pcifront_connect_and_init_dma(pdev);
 	if (err) {
 		xenbus_dev_fatal(pdev->xdev, err,
-				 "Error connecting PCI Frontend");
+				 "Error setting up PCI Frontend");
 		goto out;
 	}
 
-- 
1.7.7.6

Konrad Rzeszutek Wilk

2012-Sep-10 19:46 UTC

head link

[PATCH 08/10] xen/swiotlb: Remove functions not needed anymore.

Sparse warns us off:
drivers/xen/swiotlb-xen.c:506:1: warning: symbol
''xen_swiotlb_map_sg'' was not declared. Should it be static?
drivers/xen/swiotlb-xen.c:534:1: warning: symbol
''xen_swiotlb_unmap_sg'' was not declared. Should it be static?

and it looks like we do not need this function at all.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 drivers/xen/swiotlb-xen.c |   16 ----------------
 include/xen/swiotlb-xen.h |    9 ---------
 2 files changed, 0 insertions(+), 25 deletions(-)

diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index f0825cb..6f81994 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -514,14 +514,6 @@ xen_swiotlb_map_sg_attrs(struct device *hwdev, struct
scatterlist *sgl,
 }
 EXPORT_SYMBOL_GPL(xen_swiotlb_map_sg_attrs);
 
-int
-xen_swiotlb_map_sg(struct device *hwdev, struct scatterlist *sgl, int nelems,
-		   enum dma_data_direction dir)
-{
-	return xen_swiotlb_map_sg_attrs(hwdev, sgl, nelems, dir, NULL);
-}
-EXPORT_SYMBOL_GPL(xen_swiotlb_map_sg);
-
 /*
  * Unmap a set of streaming mode DMA translations.  Again, cpu read rules
  * concerning calls here are the same as for swiotlb_unmap_page() above.
@@ -542,14 +534,6 @@ xen_swiotlb_unmap_sg_attrs(struct device *hwdev, struct
scatterlist *sgl,
 }
 EXPORT_SYMBOL_GPL(xen_swiotlb_unmap_sg_attrs);
 
-void
-xen_swiotlb_unmap_sg(struct device *hwdev, struct scatterlist *sgl, int nelems,
-		     enum dma_data_direction dir)
-{
-	return xen_swiotlb_unmap_sg_attrs(hwdev, sgl, nelems, dir, NULL);
-}
-EXPORT_SYMBOL_GPL(xen_swiotlb_unmap_sg);
-
 /*
  * Make physical memory consistent for a set of streaming mode DMA translations
  * after a transfer.
diff --git a/include/xen/swiotlb-xen.h b/include/xen/swiotlb-xen.h
index f26f9f3..a0db2b7 100644
--- a/include/xen/swiotlb-xen.h
+++ b/include/xen/swiotlb-xen.h
@@ -23,15 +23,6 @@ extern dma_addr_t xen_swiotlb_map_page(struct device *dev,
struct page *page,
 extern 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);
-/*
-extern int
-xen_swiotlb_map_sg(struct device *hwdev, struct scatterlist *sg, int nents,
-		   enum dma_data_direction dir);
-
-extern void
-xen_swiotlb_unmap_sg(struct device *hwdev, struct scatterlist *sg, int nents,
-		     enum dma_data_direction dir);
-*/
 extern int
 xen_swiotlb_map_sg_attrs(struct device *hwdev, struct scatterlist *sgl,
 			 int nelems, enum dma_data_direction dir,
-- 
1.7.7.6

Konrad Rzeszutek Wilk

2012-Sep-10 19:46 UTC

head link

[PATCH 09/10] xen/swiotlb: Fix compile warnings when using plain integer instead of NULL pointer.

arch/x86/xen/pci-swiotlb-xen.c:96:1: warning: Using plain integer as NULL
pointer
arch/x86/xen/pci-swiotlb-xen.c:96:1: warning: Using plain integer as NULL
pointer

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/xen/pci-swiotlb-xen.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/xen/pci-swiotlb-xen.c b/arch/x86/xen/pci-swiotlb-xen.c
index 406f9c4..fc0c78f 100644
--- a/arch/x86/xen/pci-swiotlb-xen.c
+++ b/arch/x86/xen/pci-swiotlb-xen.c
@@ -97,6 +97,6 @@ int pci_xen_swiotlb_init_late(void)
 EXPORT_SYMBOL_GPL(pci_xen_swiotlb_init_late);
 
 IOMMU_INIT_FINISH(pci_xen_swiotlb_detect,
-		  0,
+		  NULL,
 		  pci_xen_swiotlb_init,
-		  0);
+		  NULL);
-- 
1.7.7.6

Konrad Rzeszutek Wilk

2012-Sep-10 19:46 UTC

head link

[PATCH 10/10] xen/swiotlb: Depending on after_bootmem is not correct.

When PCI IOMMUs are initialized it is after after_bootmem but
before a lot of "other" subsystems are initialized. As such
the check for after_bootmem is incorrect and we should
just use a parameter to define whether we are early or late.

This solves this bootup problem:

__ex_table already sorted, skipping sortM
Initializing CPU#0
Warning: only able to allocate 1 MB for software IO TLB
Xen-SWIOTLB: Lowering to 2MB
Warning: only able to allocate 1 MB for software IO TLB
Xen-SWIOTLB: Lowering to 2MB
Warning: only able to allocate 1 MB for software IO TLB
Xen-SWIOTLB: Lowering to 2MB
Warning: only able to allocate 1 MB for software IO TLB
Cannot allocate Xen-SWIOTLB buffer (rc:-12)

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/xen/pci-swiotlb-xen.c |    4 ++--
 drivers/xen/swiotlb-xen.c      |   11 ++++++-----
 include/xen/swiotlb-xen.h      |    2 +-
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/arch/x86/xen/pci-swiotlb-xen.c b/arch/x86/xen/pci-swiotlb-xen.c
index fc0c78f..1608244 100644
--- a/arch/x86/xen/pci-swiotlb-xen.c
+++ b/arch/x86/xen/pci-swiotlb-xen.c
@@ -69,7 +69,7 @@ int __init pci_xen_swiotlb_detect(void)
 void __init pci_xen_swiotlb_init(void)
 {
 	if (xen_swiotlb) {
-		xen_swiotlb_init(1);
+		xen_swiotlb_init(1, true /* early */);
 		dma_ops = &xen_swiotlb_dma_ops;
 
 		/* Make sure ACS will be enabled */
@@ -84,7 +84,7 @@ int pci_xen_swiotlb_init_late(void)
 	if (xen_swiotlb)
 		return 0;
 
-	rc = xen_swiotlb_init(1);
+	rc = xen_swiotlb_init(1, false /* late */);
 	if (rc)
 		return rc;
 
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 6f81994..7443e19 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -176,7 +176,7 @@ static const char *xen_swiotlb_error(enum xen_swiotlb_err
err)
 	}
 	return "";
 }
-int __ref xen_swiotlb_init(int verbose)
+int __ref xen_swiotlb_init(int verbose, bool early)
 {
 	unsigned long bytes, order;
 	int rc = -ENOMEM;
@@ -190,7 +190,7 @@ retry:
 	/*
 	 * Get IO TLB memory from any location.
 	 */
-	if (!after_bootmem)
+	if (early)
 		xen_io_tlb_start = alloc_bootmem_pages(PAGE_ALIGN(bytes));
 	else {
 #define SLABS_PER_PAGE (1 << (PAGE_SHIFT - IO_TLB_SHIFT))
@@ -220,7 +220,7 @@ retry:
 			       bytes,
 			       xen_io_tlb_nslabs);
 	if (rc) {
-		if (!after_bootmem)
+		if (early)
 			free_bootmem(__pa(xen_io_tlb_start), PAGE_ALIGN(bytes));
 		else {
 			free_pages((unsigned long)xen_io_tlb_start, order);
@@ -230,7 +230,8 @@ retry:
 		goto error;
 	}
 	start_dma_addr = xen_virt_to_bus(xen_io_tlb_start);
-	if (!after_bootmem)
+	rc = 0;
+	if (early)
 		swiotlb_init_with_tbl(xen_io_tlb_start, xen_io_tlb_nslabs, verbose);
 	else
 		rc = swiotlb_late_init_with_tbl(xen_io_tlb_start, xen_io_tlb_nslabs);
@@ -244,7 +245,7 @@ error:
 		goto retry;
 	}
 	pr_err("%s (rc:%d)", xen_swiotlb_error(m_ret), rc);
-	if (!after_bootmem)
+	if (early)
 		panic("%s (rc:%d)", xen_swiotlb_error(m_ret), rc);
 	else
 		free_pages((unsigned long)xen_io_tlb_start, order);
diff --git a/include/xen/swiotlb-xen.h b/include/xen/swiotlb-xen.h
index a0db2b7..de8bcc6 100644
--- a/include/xen/swiotlb-xen.h
+++ b/include/xen/swiotlb-xen.h
@@ -3,7 +3,7 @@
 
 #include <linux/swiotlb.h>
 
-extern int xen_swiotlb_init(int verbose);
+extern int xen_swiotlb_init(int verbose, bool early);
 
 extern void
 *xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
-- 
1.7.7.6

Ian Jackson

2012-Sep-13 15:53 UTC

head link

Re: [Xen-devel] [PATCH 09/10] xen/swiotlb: Fix compile warnings when using plain integer instead of NULL pointer.

Konrad Rzeszutek Wilk writes ("[Xen-devel] [PATCH 09/10] xen/swiotlb: Fix
compile warnings when using plain integer instead of NULL
pointer."):> arch/x86/xen/pci-swiotlb-xen.c:96:1: warning: Using plain integer as NULL
pointer
> arch/x86/xen/pci-swiotlb-xen.c:96:1: warning: Using plain integer as NULL
pointer
This warning is simply wrong for this code:
>  IOMMU_INIT_FINISH(pci_xen_swiotlb_detect,
> -		  0,
> +		  NULL,
There is (according to the C language specifications) nothing wrong
with writing 0 for a null pointer.

Nor does CODING_STYLE say that it is forbidden to just write 0.

Ian.

Ian Jackson

2012-Sep-13 15:56 UTC

head link

Re: [Xen-devel] [PATCH 09/10] xen/swiotlb: Fix compile warnings when using plain integer instead of NULL pointer.

Ian Jackson writes ("Re: [Xen-devel] [PATCH 09/10] xen/swiotlb: Fix compile
warnings when using plain integer instead of NULL
pointer."):> Konrad Rzeszutek Wilk writes ("[Xen-devel] [PATCH 09/10] xen/swiotlb:
Fix compile warnings when using plain integer instead of NULL pointer."):
> > arch/x86/xen/pci-swiotlb-xen.c:96:1: warning: Using plain integer as
NULL pointer
> > arch/x86/xen/pci-swiotlb-xen.c:96:1: warning: Using plain integer as
NULL pointer
> 
> This warning is simply wrong for this code:
> 
> >  IOMMU_INIT_FINISH(pci_xen_swiotlb_detect,
> > -		  0,
> > +		  NULL,
> 
> There is (according to the C language specifications) nothing wrong
> with writing 0 for a null pointer.
> 
> Nor does CODING_STYLE say that it is forbidden to just write 0.
Oh wait this is Linux kernel code, not in Xen?  Still,
Documentation/CodingStyle doesn''t say that NULL is required.

Ian.

Ian Jackson

2012-Sep-13 16:00 UTC

head link

Re: [Xen-devel] [PATCH 09/10] xen/swiotlb: Fix compile warnings when using plain integer instead of NULL pointer.

Ian Jackson writes ("Re: [Xen-devel] [PATCH 09/10] xen/swiotlb: Fix compile
warnings when using plain integer instead of NULL
pointer."):> Oh wait this is Linux kernel code, not in Xen?  Still,
> Documentation/CodingStyle doesn''t say that NULL is required.
Someone on irc found me this rant from Linus:
  https://lkml.org/lkml/2004/7/8/7
That suggests that perhaps someone should patch CodingStyle to
document this style requirement.

That someone shouldn''t be me because I don''t agree with the
requirement and a rationale I''d write for it would come out sounding
sarcastic.

Ian.

Stefano Stabellini

2012-Sep-14 16:00 UTC

head link

Re: [PATCH 05/10] xen/swiotlb: Move the error strings to its own function.

On Mon, 10 Sep 2012, Konrad Rzeszutek Wilk wrote:> That way we can more easily reuse those errors when using the
> late SWIOTLB init.
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

>  drivers/xen/swiotlb-xen.c |   35 +++++++++++++++++++++++++++--------
>  1 files changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index a2aad6e..701b103 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -154,11 +154,33 @@ static unsigned long xen_set_nslabs(unsigned long
nr_tbl)
>  
>  	return xen_io_tlb_nslabs << IO_TLB_SHIFT;
>  }
> +
> +enum xen_swiotlb_err {
> +	XEN_SWIOTLB_UNKNOWN = 0,
> +	XEN_SWIOTLB_ENOMEM,
> +	XEN_SWIOTLB_EFIXUP
> +};
> +
> +static const char *xen_swiotlb_error(enum xen_swiotlb_err err)
> +{
> +	switch (err) {
> +	case XEN_SWIOTLB_ENOMEM:
> +		return "Cannot allocate Xen-SWIOTLB buffer\n";
> +	case XEN_SWIOTLB_EFIXUP:
> +		return "Failed to get contiguous memory for DMA from Xen!\n"\
> +		    "You either: don''t have the permissions, do not
have"\
> +		    " enough free memory under 4GB, or the hypervisor memory"\
> +		    " is too fragmented!";
> +	default:
> +		break;
> +	}
> +	return "";
> +}
>  void __init xen_swiotlb_init(int verbose)
>  {
>  	unsigned long bytes;
>  	int rc = -ENOMEM;
> -	char *m = NULL;
> +	enum xen_swiotlb_err m_ret = XEN_SWIOTLB_UNKNOWN;
>  	unsigned int repeat = 3;
>  
>  	xen_io_tlb_nslabs = swiotlb_nr_tbl();
> @@ -169,7 +191,7 @@ retry:
>  	 */
>  	xen_io_tlb_start = alloc_bootmem_pages(PAGE_ALIGN(bytes));
>  	if (!xen_io_tlb_start) {
> -		m = "Cannot allocate Xen-SWIOTLB buffer!\n";
> +		m_ret = XEN_SWIOTLB_ENOMEM;
>  		goto error;
>  	}
>  	xen_io_tlb_end = xen_io_tlb_start + bytes;
> @@ -181,10 +203,7 @@ retry:
>  			       xen_io_tlb_nslabs);
>  	if (rc) {
>  		free_bootmem(__pa(xen_io_tlb_start), PAGE_ALIGN(bytes));
> -		m = "Failed to get contiguous memory for DMA from Xen!\n"\
> -		    "You either: don''t have the permissions, do not
have"\
> -		    " enough free memory under 4GB, or the hypervisor memory"\
> -		    "is too fragmented!";
> +		m_ret = XEN_SWIOTLB_EFIXUP;
>  		goto error;
>  	}
>  	start_dma_addr = xen_virt_to_bus(xen_io_tlb_start);
> @@ -199,8 +218,8 @@ error:
>  		      (xen_io_tlb_nslabs << IO_TLB_SHIFT) >> 20);
>  		goto retry;
>  	}
> -	xen_raw_printk("%s (rc:%d)", m, rc);
> -	panic("%s (rc:%d)", m, rc);
> +	xen_raw_printk("%s (rc:%d)", xen_swiotlb_error(m_ret), rc);
> +	panic("%s (rc:%d)", xen_swiotlb_error(m_ret), rc);
>  }
>  
>  void *
> -- 
> 1.7.7.6
>

Stefano Stabellini

2012-Sep-14 16:06 UTC

head link

Re: [PATCH 08/10] xen/swiotlb: Remove functions not needed anymore.

On Mon, 10 Sep 2012, Konrad Rzeszutek Wilk wrote:> Sparse warns us off:
> drivers/xen/swiotlb-xen.c:506:1: warning: symbol
''xen_swiotlb_map_sg'' was not declared. Should it be static?
> drivers/xen/swiotlb-xen.c:534:1: warning: symbol
''xen_swiotlb_unmap_sg'' was not declared. Should it be static?
> 
> and it looks like we do not need this function at all.
A grep seems to find them used in arch/x86/xen/pci-swiotlb-xen.c.
I fail to see where you removed them from pci-swiotlb-xen.c. Is it in
this patch series or another one?

> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  drivers/xen/swiotlb-xen.c |   16 ----------------
>  include/xen/swiotlb-xen.h |    9 ---------
>  2 files changed, 0 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index f0825cb..6f81994 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -514,14 +514,6 @@ xen_swiotlb_map_sg_attrs(struct device *hwdev, struct
scatterlist *sgl,
>  }
>  EXPORT_SYMBOL_GPL(xen_swiotlb_map_sg_attrs);
>  
> -int
> -xen_swiotlb_map_sg(struct device *hwdev, struct scatterlist *sgl, int
nelems,
> -		   enum dma_data_direction dir)
> -{
> -	return xen_swiotlb_map_sg_attrs(hwdev, sgl, nelems, dir, NULL);
> -}
> -EXPORT_SYMBOL_GPL(xen_swiotlb_map_sg);
> -
>  /*
>   * Unmap a set of streaming mode DMA translations.  Again, cpu read rules
>   * concerning calls here are the same as for swiotlb_unmap_page() above.
> @@ -542,14 +534,6 @@ xen_swiotlb_unmap_sg_attrs(struct device *hwdev,
struct scatterlist *sgl,
>  }
>  EXPORT_SYMBOL_GPL(xen_swiotlb_unmap_sg_attrs);
>  
> -void
> -xen_swiotlb_unmap_sg(struct device *hwdev, struct scatterlist *sgl, int
nelems,
> -		     enum dma_data_direction dir)
> -{
> -	return xen_swiotlb_unmap_sg_attrs(hwdev, sgl, nelems, dir, NULL);
> -}
> -EXPORT_SYMBOL_GPL(xen_swiotlb_unmap_sg);
> -
>  /*
>   * Make physical memory consistent for a set of streaming mode DMA
translations
>   * after a transfer.
> diff --git a/include/xen/swiotlb-xen.h b/include/xen/swiotlb-xen.h
> index f26f9f3..a0db2b7 100644
> --- a/include/xen/swiotlb-xen.h
> +++ b/include/xen/swiotlb-xen.h
> @@ -23,15 +23,6 @@ extern dma_addr_t xen_swiotlb_map_page(struct device
*dev, struct page *page,
>  extern 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);
> -/*
> -extern int
> -xen_swiotlb_map_sg(struct device *hwdev, struct scatterlist *sg, int
nents,
> -		   enum dma_data_direction dir);
> -
> -extern void
> -xen_swiotlb_unmap_sg(struct device *hwdev, struct scatterlist *sg, int
nents,
> -		     enum dma_data_direction dir);
> -*/
>  extern int
>  xen_swiotlb_map_sg_attrs(struct device *hwdev, struct scatterlist *sgl,
>  			 int nelems, enum dma_data_direction dir,
> -- 
> 1.7.7.6
>

Stefano Stabellini

2012-Sep-14 16:08 UTC

head link

Re: [PATCH 04/10] xen/swiotlb: Move the nr_tbl determination in its own function.

On Mon, 10 Sep 2012, Konrad Rzeszutek Wilk wrote:> Moving the function out of the way to prepare for the late
> SWIOTLB init.
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>  drivers/xen/swiotlb-xen.c |   21 +++++++++++----------
>  1 files changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index 1afb4fb..a2aad6e 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -144,25 +144,26 @@ xen_swiotlb_fixup(void *buf, size_t size, unsigned
long nslabs)
>  	} while (i < nslabs);
>  	return 0;
>  }
> +static unsigned long xen_set_nslabs(unsigned long nr_tbl)
> +{
> +	if (!nr_tbl) {
> +		xen_io_tlb_nslabs = (64 * 1024 * 1024 >> IO_TLB_SHIFT);
> +		xen_io_tlb_nslabs = ALIGN(xen_io_tlb_nslabs, IO_TLB_SEGSIZE);
> +	} else
> +		xen_io_tlb_nslabs = nr_tbl;
>  
> +	return xen_io_tlb_nslabs << IO_TLB_SHIFT;
> +}
>  void __init xen_swiotlb_init(int verbose)
>  {
>  	unsigned long bytes;
>  	int rc = -ENOMEM;
> -	unsigned long nr_tbl;
>  	char *m = NULL;
>  	unsigned int repeat = 3;
>  
> -	nr_tbl = swiotlb_nr_tbl();
> -	if (nr_tbl)
> -		xen_io_tlb_nslabs = nr_tbl;
> -	else {
> -		xen_io_tlb_nslabs = (64 * 1024 * 1024 >> IO_TLB_SHIFT);
> -		xen_io_tlb_nslabs = ALIGN(xen_io_tlb_nslabs, IO_TLB_SEGSIZE);
> -	}
> +	xen_io_tlb_nslabs = swiotlb_nr_tbl();
>  retry:
> -	bytes = xen_io_tlb_nslabs << IO_TLB_SHIFT;
> -
> +	bytes = xen_set_nslabs(xen_io_tlb_nslabs);
>  	/*
>  	 * Get IO TLB memory from any location.
>  	 */
> -- 
> 1.7.7.6
>

Stefano Stabellini

2012-Sep-14 16:10 UTC

head link

Re: [PATCH 10/10] xen/swiotlb: Depending on after_bootmem is not correct.

On Mon, 10 Sep 2012, Konrad Rzeszutek Wilk wrote:> When PCI IOMMUs are initialized it is after after_bootmem but
> before a lot of "other" subsystems are initialized. As such
> the check for after_bootmem is incorrect and we should
> just use a parameter to define whether we are early or late.
> 
> This solves this bootup problem:
> 
> __ex_table already sorted, skipping sortM
> Initializing CPU#0
> Warning: only able to allocate 1 MB for software IO TLB
> Xen-SWIOTLB: Lowering to 2MB
> Warning: only able to allocate 1 MB for software IO TLB
> Xen-SWIOTLB: Lowering to 2MB
> Warning: only able to allocate 1 MB for software IO TLB
> Xen-SWIOTLB: Lowering to 2MB
> Warning: only able to allocate 1 MB for software IO TLB
> Cannot allocate Xen-SWIOTLB buffer (rc:-12)
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  arch/x86/xen/pci-swiotlb-xen.c |    4 ++--
>  drivers/xen/swiotlb-xen.c      |   11 ++++++-----
>  include/xen/swiotlb-xen.h      |    2 +-
>  3 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/xen/pci-swiotlb-xen.c
b/arch/x86/xen/pci-swiotlb-xen.c
> index fc0c78f..1608244 100644
> --- a/arch/x86/xen/pci-swiotlb-xen.c
> +++ b/arch/x86/xen/pci-swiotlb-xen.c
> @@ -69,7 +69,7 @@ int __init pci_xen_swiotlb_detect(void)
>  void __init pci_xen_swiotlb_init(void)
>  {
>  	if (xen_swiotlb) {
> -		xen_swiotlb_init(1);
> +		xen_swiotlb_init(1, true /* early */);
>  		dma_ops = &xen_swiotlb_dma_ops;
>  
>  		/* Make sure ACS will be enabled */
> @@ -84,7 +84,7 @@ int pci_xen_swiotlb_init_late(void)
>  	if (xen_swiotlb)
>  		return 0;
>  
> -	rc = xen_swiotlb_init(1);
> +	rc = xen_swiotlb_init(1, false /* late */);
>  	if (rc)
>  		return rc;
>  
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index 6f81994..7443e19 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -176,7 +176,7 @@ static const char *xen_swiotlb_error(enum
xen_swiotlb_err err)
>  	}
>  	return "";
>  }
> -int __ref xen_swiotlb_init(int verbose)
> +int __ref xen_swiotlb_init(int verbose, bool early)
>  {
>  	unsigned long bytes, order;
>  	int rc = -ENOMEM;
> @@ -190,7 +190,7 @@ retry:
>  	/*
>  	 * Get IO TLB memory from any location.
>  	 */
> -	if (!after_bootmem)
> +	if (early)
>  		xen_io_tlb_start = alloc_bootmem_pages(PAGE_ALIGN(bytes));
>  	else {
>  #define SLABS_PER_PAGE (1 << (PAGE_SHIFT - IO_TLB_SHIFT))
> @@ -220,7 +220,7 @@ retry:
>  			       bytes,
>  			       xen_io_tlb_nslabs);
>  	if (rc) {
> -		if (!after_bootmem)
> +		if (early)
>  			free_bootmem(__pa(xen_io_tlb_start), PAGE_ALIGN(bytes));
>  		else {
>  			free_pages((unsigned long)xen_io_tlb_start, order);
> @@ -230,7 +230,8 @@ retry:
>  		goto error;
>  	}
>  	start_dma_addr = xen_virt_to_bus(xen_io_tlb_start);
> -	if (!after_bootmem)
> +	rc = 0;     ^
why does this change belong to this patch?

> +	if (early)
>  		swiotlb_init_with_tbl(xen_io_tlb_start, xen_io_tlb_nslabs, verbose);
>  	else
>  		rc = swiotlb_late_init_with_tbl(xen_io_tlb_start, xen_io_tlb_nslabs);
> @@ -244,7 +245,7 @@ error:
>  		goto retry;
>  	}
>  	pr_err("%s (rc:%d)", xen_swiotlb_error(m_ret), rc);
> -	if (!after_bootmem)
> +	if (early)
>  		panic("%s (rc:%d)", xen_swiotlb_error(m_ret), rc);
>  	else
>  		free_pages((unsigned long)xen_io_tlb_start, order);
> diff --git a/include/xen/swiotlb-xen.h b/include/xen/swiotlb-xen.h
> index a0db2b7..de8bcc6 100644
> --- a/include/xen/swiotlb-xen.h
> +++ b/include/xen/swiotlb-xen.h
> @@ -3,7 +3,7 @@
>  
>  #include <linux/swiotlb.h>
>  
> -extern int xen_swiotlb_init(int verbose);
> +extern int xen_swiotlb_init(int verbose, bool early);
>  
>  extern void
>  *xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
> -- 
> 1.7.7.6
>

Stefano Stabellini

2012-Sep-14 16:11 UTC

head link

Re: [PATCH 09/10] xen/swiotlb: Fix compile warnings when using plain integer instead of NULL pointer.

On Mon, 10 Sep 2012, Konrad Rzeszutek Wilk wrote:> arch/x86/xen/pci-swiotlb-xen.c:96:1: warning: Using plain integer as NULL
pointer
> arch/x86/xen/pci-swiotlb-xen.c:96:1: warning: Using plain integer as NULL
pointer
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

>  arch/x86/xen/pci-swiotlb-xen.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/xen/pci-swiotlb-xen.c
b/arch/x86/xen/pci-swiotlb-xen.c
> index 406f9c4..fc0c78f 100644
> --- a/arch/x86/xen/pci-swiotlb-xen.c
> +++ b/arch/x86/xen/pci-swiotlb-xen.c
> @@ -97,6 +97,6 @@ int pci_xen_swiotlb_init_late(void)
>  EXPORT_SYMBOL_GPL(pci_xen_swiotlb_init_late);
>  
>  IOMMU_INIT_FINISH(pci_xen_swiotlb_detect,
> -		  0,
> +		  NULL,
>  		  pci_xen_swiotlb_init,
> -		  0);
> +		  NULL);
> -- 
> 1.7.7.6
>

Stefano Stabellini

2012-Sep-14 16:26 UTC

head link

Re: [PATCH 06/10] xen/swiotlb: Use the swiotlb_late_init_with_tbl to init Xen-SWIOTLB late when PV PCI is used.

On Mon, 10 Sep 2012, Konrad Rzeszutek Wilk wrote:> With this patch we provide the functionality to initialize the
> Xen-SWIOTLB late in the bootup cycle - specifically for
> Xen PCI-frontend. We still will work if the user had
> supplied ''iommu=soft'' on the Linux command line.
> 
> CC: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> [v1: Fix smatch warnings]
> [v2: Added check for xen_swiotlb]
> [v3: Rebased with new xen-swiotlb changes]
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

>  arch/x86/include/asm/xen/swiotlb-xen.h |    2 +
>  arch/x86/xen/pci-swiotlb-xen.c         |   22 ++++++++++++++-
>  drivers/xen/swiotlb-xen.c              |   48
+++++++++++++++++++++++++------
>  include/xen/swiotlb-xen.h              |    2 +-
>  4 files changed, 62 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86/include/asm/xen/swiotlb-xen.h
b/arch/x86/include/asm/xen/swiotlb-xen.h
> index 1be1ab7..ee52fca 100644
> --- a/arch/x86/include/asm/xen/swiotlb-xen.h
> +++ b/arch/x86/include/asm/xen/swiotlb-xen.h
> @@ -5,10 +5,12 @@
>  extern int xen_swiotlb;
>  extern int __init pci_xen_swiotlb_detect(void);
>  extern void __init pci_xen_swiotlb_init(void);
> +extern int pci_xen_swiotlb_init_late(void);
>  #else
>  #define xen_swiotlb (0)
>  static inline int __init pci_xen_swiotlb_detect(void) { return 0; }
>  static inline void __init pci_xen_swiotlb_init(void) { }
> +static inline int pci_xen_swiotlb_init_late(void) { return -ENXIO; }
>  #endif
>  
>  #endif /* _ASM_X86_SWIOTLB_XEN_H */
> diff --git a/arch/x86/xen/pci-swiotlb-xen.c
b/arch/x86/xen/pci-swiotlb-xen.c
> index 1c17227..406f9c4 100644
> --- a/arch/x86/xen/pci-swiotlb-xen.c
> +++ b/arch/x86/xen/pci-swiotlb-xen.c
> @@ -12,7 +12,7 @@
>  #include <asm/iommu.h>
>  #include <asm/dma.h>
>  #endif
> -
> +#include <linux/export.h>
>  int xen_swiotlb __read_mostly;
>  
>  static struct dma_map_ops xen_swiotlb_dma_ops = {
> @@ -76,6 +76,26 @@ void __init pci_xen_swiotlb_init(void)
>  		pci_request_acs();
>  	}
>  }
> +
> +int pci_xen_swiotlb_init_late(void)
> +{
> +	int rc;
> +
> +	if (xen_swiotlb)
> +		return 0;
> +
> +	rc = xen_swiotlb_init(1);
> +	if (rc)
> +		return rc;
> +
> +	dma_ops = &xen_swiotlb_dma_ops;
> +	/* Make sure ACS will be enabled */
> +	pci_request_acs();
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(pci_xen_swiotlb_init_late);
> +
>  IOMMU_INIT_FINISH(pci_xen_swiotlb_detect,
>  		  0,
>  		  pci_xen_swiotlb_init,
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index 701b103..f0825cb 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -176,9 +176,9 @@ static const char *xen_swiotlb_error(enum
xen_swiotlb_err err)
>  	}
>  	return "";
>  }
> -void __init xen_swiotlb_init(int verbose)
> +int __ref xen_swiotlb_init(int verbose)
>  {
> -	unsigned long bytes;
> +	unsigned long bytes, order;
>  	int rc = -ENOMEM;
>  	enum xen_swiotlb_err m_ret = XEN_SWIOTLB_UNKNOWN;
>  	unsigned int repeat = 3;
> @@ -186,10 +186,28 @@ void __init xen_swiotlb_init(int verbose)
>  	xen_io_tlb_nslabs = swiotlb_nr_tbl();
>  retry:
>  	bytes = xen_set_nslabs(xen_io_tlb_nslabs);
> +	order = get_order(xen_io_tlb_nslabs << IO_TLB_SHIFT);
>  	/*
>  	 * Get IO TLB memory from any location.
>  	 */
> -	xen_io_tlb_start = alloc_bootmem_pages(PAGE_ALIGN(bytes));
> +	if (!after_bootmem)
> +		xen_io_tlb_start = alloc_bootmem_pages(PAGE_ALIGN(bytes));
> +	else {
> +#define SLABS_PER_PAGE (1 << (PAGE_SHIFT - IO_TLB_SHIFT))
> +#define IO_TLB_MIN_SLABS ((1<<20) >> IO_TLB_SHIFT)
> +		while ((SLABS_PER_PAGE << order) > IO_TLB_MIN_SLABS) {
> +			xen_io_tlb_start = (void *)__get_free_pages(__GFP_NOWARN, order);
> +			if (xen_io_tlb_start)
> +				break;
> +			order--;
> +		}
> +		if (order != get_order(bytes)) {
> +			pr_warn("Warning: only able to allocate %ld MB "
> +				"for software IO TLB\n", (PAGE_SIZE << order) >>
20);
> +			xen_io_tlb_nslabs = SLABS_PER_PAGE << order;
> +			bytes = xen_io_tlb_nslabs << IO_TLB_SHIFT;
> +		}
> +	}
>  	if (!xen_io_tlb_start) {
>  		m_ret = XEN_SWIOTLB_ENOMEM;
>  		goto error;
> @@ -202,14 +220,21 @@ retry:
>  			       bytes,
>  			       xen_io_tlb_nslabs);
>  	if (rc) {
> -		free_bootmem(__pa(xen_io_tlb_start), PAGE_ALIGN(bytes));
> +		if (!after_bootmem)
> +			free_bootmem(__pa(xen_io_tlb_start), PAGE_ALIGN(bytes));
> +		else {
> +			free_pages((unsigned long)xen_io_tlb_start, order);
> +			xen_io_tlb_start = NULL;
> +		}
>  		m_ret = XEN_SWIOTLB_EFIXUP;
>  		goto error;
>  	}
>  	start_dma_addr = xen_virt_to_bus(xen_io_tlb_start);
> -	swiotlb_init_with_tbl(xen_io_tlb_start, xen_io_tlb_nslabs, verbose);
> -
> -	return;
> +	if (!after_bootmem)
> +		swiotlb_init_with_tbl(xen_io_tlb_start, xen_io_tlb_nslabs, verbose);
> +	else
> +		rc = swiotlb_late_init_with_tbl(xen_io_tlb_start, xen_io_tlb_nslabs);
> +	return rc;
>  error:
>  	if (repeat--) {
>  		xen_io_tlb_nslabs = max(1024UL, /* Min is 2MB */
> @@ -218,10 +243,13 @@ error:
>  		      (xen_io_tlb_nslabs << IO_TLB_SHIFT) >> 20);
>  		goto retry;
>  	}
> -	xen_raw_printk("%s (rc:%d)", xen_swiotlb_error(m_ret), rc);
> -	panic("%s (rc:%d)", xen_swiotlb_error(m_ret), rc);
> +	pr_err("%s (rc:%d)", xen_swiotlb_error(m_ret), rc);
> +	if (!after_bootmem)
> +		panic("%s (rc:%d)", xen_swiotlb_error(m_ret), rc);
> +	else
> +		free_pages((unsigned long)xen_io_tlb_start, order);
> +	return rc;
>  }
> -
>  void *
>  xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
>  			   dma_addr_t *dma_handle, gfp_t flags,
> diff --git a/include/xen/swiotlb-xen.h b/include/xen/swiotlb-xen.h
> index 4f4d449..f26f9f3 100644
> --- a/include/xen/swiotlb-xen.h
> +++ b/include/xen/swiotlb-xen.h
> @@ -3,7 +3,7 @@
>  
>  #include <linux/swiotlb.h>
>  
> -extern void xen_swiotlb_init(int verbose);
> +extern int xen_swiotlb_init(int verbose);
>  
>  extern void
>  *xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
> -- 
> 1.7.7.6
>

Konrad Rzeszutek Wilk

2012-Sep-14 17:06 UTC

head link

Re: [PATCH 08/10] xen/swiotlb: Remove functions not needed anymore.

On Fri, Sep 14, 2012 at 05:06:16PM +0100, Stefano Stabellini
wrote:> On Mon, 10 Sep 2012, Konrad Rzeszutek Wilk wrote:
> > Sparse warns us off:
> > drivers/xen/swiotlb-xen.c:506:1: warning: symbol
''xen_swiotlb_map_sg'' was not declared. Should it be static?
> > drivers/xen/swiotlb-xen.c:534:1: warning: symbol
''xen_swiotlb_unmap_sg'' was not declared. Should it be static?
> > 
> > and it looks like we do not need this function at all.
> 
> A grep seems to find them used in arch/x86/xen/pci-swiotlb-xen.c.
> I fail to see where you removed them from pci-swiotlb-xen.c. Is it in
> this patch series or another one?
I am not seeing them in that file. I think you found the
other variant of it:

xen_swiotlb_map_sg_attrs
> 
> 
> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > ---
> >  drivers/xen/swiotlb-xen.c |   16 ----------------
> >  include/xen/swiotlb-xen.h |    9 ---------
> >  2 files changed, 0 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> > index f0825cb..6f81994 100644
> > --- a/drivers/xen/swiotlb-xen.c
> > +++ b/drivers/xen/swiotlb-xen.c
> > @@ -514,14 +514,6 @@ xen_swiotlb_map_sg_attrs(struct device *hwdev,
struct scatterlist *sgl,
> >  }
> >  EXPORT_SYMBOL_GPL(xen_swiotlb_map_sg_attrs);
> >  
> > -int
> > -xen_swiotlb_map_sg(struct device *hwdev, struct scatterlist *sgl, int
nelems,
> > -		   enum dma_data_direction dir)
> > -{
> > -	return xen_swiotlb_map_sg_attrs(hwdev, sgl, nelems, dir, NULL);
> > -}
> > -EXPORT_SYMBOL_GPL(xen_swiotlb_map_sg);
> > -
> >  /*
> >   * Unmap a set of streaming mode DMA translations.  Again, cpu read
rules
> >   * concerning calls here are the same as for swiotlb_unmap_page()
above.
> > @@ -542,14 +534,6 @@ xen_swiotlb_unmap_sg_attrs(struct device *hwdev,
struct scatterlist *sgl,
> >  }
> >  EXPORT_SYMBOL_GPL(xen_swiotlb_unmap_sg_attrs);
> >  
> > -void
> > -xen_swiotlb_unmap_sg(struct device *hwdev, struct scatterlist *sgl,
int nelems,
> > -		     enum dma_data_direction dir)
> > -{
> > -	return xen_swiotlb_unmap_sg_attrs(hwdev, sgl, nelems, dir, NULL);
> > -}
> > -EXPORT_SYMBOL_GPL(xen_swiotlb_unmap_sg);
> > -
> >  /*
> >   * Make physical memory consistent for a set of streaming mode DMA
translations
> >   * after a transfer.
> > diff --git a/include/xen/swiotlb-xen.h b/include/xen/swiotlb-xen.h
> > index f26f9f3..a0db2b7 100644
> > --- a/include/xen/swiotlb-xen.h
> > +++ b/include/xen/swiotlb-xen.h
> > @@ -23,15 +23,6 @@ extern dma_addr_t xen_swiotlb_map_page(struct
device *dev, struct page *page,
> >  extern 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);
> > -/*
> > -extern int
> > -xen_swiotlb_map_sg(struct device *hwdev, struct scatterlist *sg, int
nents,
> > -		   enum dma_data_direction dir);
> > -
> > -extern void
> > -xen_swiotlb_unmap_sg(struct device *hwdev, struct scatterlist *sg,
int nents,
> > -		     enum dma_data_direction dir);
> > -*/
> >  extern int
> >  xen_swiotlb_map_sg_attrs(struct device *hwdev, struct scatterlist
*sgl,
> >  			 int nelems, enum dma_data_direction dir,
> > -- 
> > 1.7.7.6
> >

Konrad Rzeszutek Wilk

2012-Sep-14 17:09 UTC

head link

Re: [PATCH 10/10] xen/swiotlb: Depending on after_bootmem is not correct.

On Fri, Sep 14, 2012 at 05:10:48PM +0100, Stefano Stabellini
wrote:> On Mon, 10 Sep 2012, Konrad Rzeszutek Wilk wrote:
> > When PCI IOMMUs are initialized it is after after_bootmem but
> > before a lot of "other" subsystems are initialized. As such
> > the check for after_bootmem is incorrect and we should
> > just use a parameter to define whether we are early or late.
> > 
> > This solves this bootup problem:
> > 
> > __ex_table already sorted, skipping sortM
> > Initializing CPU#0
> > Warning: only able to allocate 1 MB for software IO TLB
> > Xen-SWIOTLB: Lowering to 2MB
> > Warning: only able to allocate 1 MB for software IO TLB
> > Xen-SWIOTLB: Lowering to 2MB
> > Warning: only able to allocate 1 MB for software IO TLB
> > Xen-SWIOTLB: Lowering to 2MB
> > Warning: only able to allocate 1 MB for software IO TLB
> > Cannot allocate Xen-SWIOTLB buffer (rc:-12)
> > 
> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > ---
> >  arch/x86/xen/pci-swiotlb-xen.c |    4 ++--
> >  drivers/xen/swiotlb-xen.c      |   11 ++++++-----
> >  include/xen/swiotlb-xen.h      |    2 +-
> >  3 files changed, 9 insertions(+), 8 deletions(-)
> > 
> > diff --git a/arch/x86/xen/pci-swiotlb-xen.c
b/arch/x86/xen/pci-swiotlb-xen.c
> > index fc0c78f..1608244 100644
> > --- a/arch/x86/xen/pci-swiotlb-xen.c
> > +++ b/arch/x86/xen/pci-swiotlb-xen.c
> > @@ -69,7 +69,7 @@ int __init pci_xen_swiotlb_detect(void)
> >  void __init pci_xen_swiotlb_init(void)
> >  {
> >  	if (xen_swiotlb) {
> > -		xen_swiotlb_init(1);
> > +		xen_swiotlb_init(1, true /* early */);
> >  		dma_ops = &xen_swiotlb_dma_ops;
> >  
> >  		/* Make sure ACS will be enabled */
> > @@ -84,7 +84,7 @@ int pci_xen_swiotlb_init_late(void)
> >  	if (xen_swiotlb)
> >  		return 0;
> >  
> > -	rc = xen_swiotlb_init(1);
> > +	rc = xen_swiotlb_init(1, false /* late */);
> >  	if (rc)
> >  		return rc;
> >  
> > diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> > index 6f81994..7443e19 100644
> > --- a/drivers/xen/swiotlb-xen.c
> > +++ b/drivers/xen/swiotlb-xen.c
> > @@ -176,7 +176,7 @@ static const char *xen_swiotlb_error(enum
xen_swiotlb_err err)
> >  	}
> >  	return "";
> >  }
> > -int __ref xen_swiotlb_init(int verbose)
> > +int __ref xen_swiotlb_init(int verbose, bool early)
> >  {
> >  	unsigned long bytes, order;
> >  	int rc = -ENOMEM;
> > @@ -190,7 +190,7 @@ retry:
> >  	/*
> >  	 * Get IO TLB memory from any location.
> >  	 */
> > -	if (!after_bootmem)
> > +	if (early)
> >  		xen_io_tlb_start = alloc_bootmem_pages(PAGE_ALIGN(bytes));
> >  	else {
> >  #define SLABS_PER_PAGE (1 << (PAGE_SHIFT - IO_TLB_SHIFT))
> > @@ -220,7 +220,7 @@ retry:
> >  			       bytes,
> >  			       xen_io_tlb_nslabs);
> >  	if (rc) {
> > -		if (!after_bootmem)
> > +		if (early)
> >  			free_bootmem(__pa(xen_io_tlb_start), PAGE_ALIGN(bytes));
> >  		else {
> >  			free_pages((unsigned long)xen_io_tlb_start, order);
> > @@ -230,7 +230,8 @@ retry:
> >  		goto error;
> >  	}
> >  	start_dma_addr = xen_virt_to_bus(xen_io_tlb_start);
> > -	if (!after_bootmem)
> > +	rc = 0;
>      ^
> why does this change belong to this patch?
Good eyes. I was being sneaky and rolled it in. But I can''t recall why
I needed
this. <sigh>
> 
> 
> > +	if (early)
> >  		swiotlb_init_with_tbl(xen_io_tlb_start, xen_io_tlb_nslabs,
verbose);
> >  	else
> >  		rc = swiotlb_late_init_with_tbl(xen_io_tlb_start,
xen_io_tlb_nslabs);
> > @@ -244,7 +245,7 @@ error:
> >  		goto retry;
> >  	}
> >  	pr_err("%s (rc:%d)", xen_swiotlb_error(m_ret), rc);
> > -	if (!after_bootmem)
> > +	if (early)
> >  		panic("%s (rc:%d)", xen_swiotlb_error(m_ret), rc);
> >  	else
> >  		free_pages((unsigned long)xen_io_tlb_start, order);
> > diff --git a/include/xen/swiotlb-xen.h b/include/xen/swiotlb-xen.h
> > index a0db2b7..de8bcc6 100644
> > --- a/include/xen/swiotlb-xen.h
> > +++ b/include/xen/swiotlb-xen.h
> > @@ -3,7 +3,7 @@
> >  
> >  #include <linux/swiotlb.h>
> >  
> > -extern int xen_swiotlb_init(int verbose);
> > +extern int xen_swiotlb_init(int verbose, bool early);
> >  
> >  extern void
> >  *xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
> > -- 
> > 1.7.7.6
> >

Stefano Stabellini

2012-Sep-17 10:52 UTC

head link

Re: [PATCH 08/10] xen/swiotlb: Remove functions not needed anymore.

On Fri, 14 Sep 2012, Konrad Rzeszutek Wilk wrote:> On Fri, Sep 14, 2012 at 05:06:16PM +0100, Stefano Stabellini wrote:
> > On Mon, 10 Sep 2012, Konrad Rzeszutek Wilk wrote:
> > > Sparse warns us off:
> > > drivers/xen/swiotlb-xen.c:506:1: warning: symbol
''xen_swiotlb_map_sg'' was not declared. Should it be static?
> > > drivers/xen/swiotlb-xen.c:534:1: warning: symbol
''xen_swiotlb_unmap_sg'' was not declared. Should it be static?
> > > 
> > > and it looks like we do not need this function at all.
> > 
> > A grep seems to find them used in arch/x86/xen/pci-swiotlb-xen.c.
> > I fail to see where you removed them from pci-swiotlb-xen.c. Is it in
> > this patch series or another one?
> 
> I am not seeing them in that file. I think you found the
> other variant of it:
> 
> xen_swiotlb_map_sg_attrs
Yes, you are right.

Konrad Rzeszutek Wilk

2012-Sep-17 14:23 UTC

head link

Re: [PATCH 10/10] xen/swiotlb: Depending on after_bootmem is not correct.

> >  	start_dma_addr = xen_virt_to_bus(xen_io_tlb_start);
> > -	if (!after_bootmem)
> > +	rc = 0;
>      ^
> why does this change belong to this patch?
> 
> 
I took that out of the this patch, so it is now:


From c5bc5502abc0f70b682c0f2a70d08e6319825163 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Wed, 5 Sep 2012 13:35:47 -0400
Subject: [PATCH 1/2] xen/swiotlb: Depending on after_bootmem is not correct.

When PCI IOMMUs are initialized it is after after_bootmem but
before a lot of "other" subsystems are initialized. As such
the check for after_bootmem is incorrect and we should
just use a parameter to define whether we are early or late.

This solves this bootup problem:

__ex_table already sorted, skipping sortM
Initializing CPU#0
Warning: only able to allocate 1 MB for software IO TLB
Xen-SWIOTLB: Lowering to 2MB
Warning: only able to allocate 1 MB for software IO TLB
Xen-SWIOTLB: Lowering to 2MB
Warning: only able to allocate 1 MB for software IO TLB
Xen-SWIOTLB: Lowering to 2MB
Warning: only able to allocate 1 MB for software IO TLB
Cannot allocate Xen-SWIOTLB buffer (rc:-12)

[v1: Had rc=0 in it by mistake]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/xen/pci-swiotlb-xen.c |    4 ++--
 drivers/xen/swiotlb-xen.c      |   10 +++++-----
 include/xen/swiotlb-xen.h      |    2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/x86/xen/pci-swiotlb-xen.c b/arch/x86/xen/pci-swiotlb-xen.c
index fc0c78f..1608244 100644
--- a/arch/x86/xen/pci-swiotlb-xen.c
+++ b/arch/x86/xen/pci-swiotlb-xen.c
@@ -69,7 +69,7 @@ int __init pci_xen_swiotlb_detect(void)
 void __init pci_xen_swiotlb_init(void)
 {
 	if (xen_swiotlb) {
-		xen_swiotlb_init(1);
+		xen_swiotlb_init(1, true /* early */);
 		dma_ops = &xen_swiotlb_dma_ops;
 
 		/* Make sure ACS will be enabled */
@@ -84,7 +84,7 @@ int pci_xen_swiotlb_init_late(void)
 	if (xen_swiotlb)
 		return 0;
 
-	rc = xen_swiotlb_init(1);
+	rc = xen_swiotlb_init(1, false /* late */);
 	if (rc)
 		return rc;
 
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 6f81994..02a52f3 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -176,7 +176,7 @@ static const char *xen_swiotlb_error(enum xen_swiotlb_err
err)
 	}
 	return "";
 }
-int __ref xen_swiotlb_init(int verbose)
+int __ref xen_swiotlb_init(int verbose, bool early)
 {
 	unsigned long bytes, order;
 	int rc = -ENOMEM;
@@ -190,7 +190,7 @@ retry:
 	/*
 	 * Get IO TLB memory from any location.
 	 */
-	if (!after_bootmem)
+	if (early)
 		xen_io_tlb_start = alloc_bootmem_pages(PAGE_ALIGN(bytes));
 	else {
 #define SLABS_PER_PAGE (1 << (PAGE_SHIFT - IO_TLB_SHIFT))
@@ -220,7 +220,7 @@ retry:
 			       bytes,
 			       xen_io_tlb_nslabs);
 	if (rc) {
-		if (!after_bootmem)
+		if (early)
 			free_bootmem(__pa(xen_io_tlb_start), PAGE_ALIGN(bytes));
 		else {
 			free_pages((unsigned long)xen_io_tlb_start, order);
@@ -230,7 +230,7 @@ retry:
 		goto error;
 	}
 	start_dma_addr = xen_virt_to_bus(xen_io_tlb_start);
-	if (!after_bootmem)
+	if (early)
 		swiotlb_init_with_tbl(xen_io_tlb_start, xen_io_tlb_nslabs, verbose);
 	else
 		rc = swiotlb_late_init_with_tbl(xen_io_tlb_start, xen_io_tlb_nslabs);
@@ -244,7 +244,7 @@ error:
 		goto retry;
 	}
 	pr_err("%s (rc:%d)", xen_swiotlb_error(m_ret), rc);
-	if (!after_bootmem)
+	if (early)
 		panic("%s (rc:%d)", xen_swiotlb_error(m_ret), rc);
 	else
 		free_pages((unsigned long)xen_io_tlb_start, order);
diff --git a/include/xen/swiotlb-xen.h b/include/xen/swiotlb-xen.h
index a0db2b7..de8bcc6 100644
--- a/include/xen/swiotlb-xen.h
+++ b/include/xen/swiotlb-xen.h
@@ -3,7 +3,7 @@
 
 #include <linux/swiotlb.h>
 
-extern int xen_swiotlb_init(int verbose);
+extern int xen_swiotlb_init(int verbose, bool early);
 
 extern void
 *xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
-- 
1.7.7.6

Konrad Rzeszutek Wilk

2012-Sep-17 14:25 UTC

head link

Is: [PATCH 11/10] xen/swiotlb: For early initialization, return zero on success. Was: Re: [PATCH 10/10] xen/swiotlb: Depending on after_bootmem is not correct.

On Mon, Sep 17, 2012 at 10:23:15AM -0400, Konrad Rzeszutek Wilk
wrote:> > >  	start_dma_addr = xen_virt_to_bus(xen_io_tlb_start);
> > > -	if (!after_bootmem)
> > > +	rc = 0;
> >      ^
> > why does this change belong to this patch?
> > 
> > 
> 
> I took that out of the this patch, so it is now:
> 
..and spun out another patch to address the rc=0:

From f85175ce01ba722cd4612230e7331dc0d4c8666f Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Mon, 17 Sep 2012 10:20:09 -0400
Subject: [PATCH 2/2] xen/swiotlb: For early initialization, return zero on
 success.

If everything is setup properly we would return -ENOMEM since
rc by default is set to that value. Lets not do that and return
a proper return code.

Note: The reason the early code needs this special treatment
is that it SWIOTLB library call does not return anything (and
had it failed it would call panic()) - but our function does.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 drivers/xen/swiotlb-xen.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 02a52f3..ab4c66c 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -230,9 +230,10 @@ retry:
 		goto error;
 	}
 	start_dma_addr = xen_virt_to_bus(xen_io_tlb_start);
-	if (early)
+	if (early) {
 		swiotlb_init_with_tbl(xen_io_tlb_start, xen_io_tlb_nslabs, verbose);
-	else
+		rc = 0;
+	} else
 		rc = swiotlb_late_init_with_tbl(xen_io_tlb_start, xen_io_tlb_nslabs);
 	return rc;
 error:
-- 
1.7.7.6

Stefano Stabellini

2012-Sep-17 14:52 UTC

head link

Re: [PATCH 10/10] xen/swiotlb: Depending on after_bootmem is not correct.

On Mon, 17 Sep 2012, Konrad Rzeszutek Wilk wrote:> > >  	start_dma_addr = xen_virt_to_bus(xen_io_tlb_start);
> > > -	if (!after_bootmem)
> > > +	rc = 0;
> >      ^
> > why does this change belong to this patch?
> > 
> > 
> 
> I took that out of the this patch, so it is now:
> 
> 
> >From c5bc5502abc0f70b682c0f2a70d08e6319825163 Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Date: Wed, 5 Sep 2012 13:35:47 -0400
> Subject: [PATCH 1/2] xen/swiotlb: Depending on after_bootmem is not
correct.
> 
> When PCI IOMMUs are initialized it is after after_bootmem but
> before a lot of "other" subsystems are initialized. As such
> the check for after_bootmem is incorrect and we should
> just use a parameter to define whether we are early or late.
Given that the after_bootmem checks are introduced by patch 6/10, I
would just merge the two.
In any case, it looks OK now.


> This solves this bootup problem:
> 
> __ex_table already sorted, skipping sortM
> Initializing CPU#0
> Warning: only able to allocate 1 MB for software IO TLB
> Xen-SWIOTLB: Lowering to 2MB
> Warning: only able to allocate 1 MB for software IO TLB
> Xen-SWIOTLB: Lowering to 2MB
> Warning: only able to allocate 1 MB for software IO TLB
> Xen-SWIOTLB: Lowering to 2MB
> Warning: only able to allocate 1 MB for software IO TLB
> Cannot allocate Xen-SWIOTLB buffer (rc:-12)
> 
> [v1: Had rc=0 in it by mistake]
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  arch/x86/xen/pci-swiotlb-xen.c |    4 ++--
>  drivers/xen/swiotlb-xen.c      |   10 +++++-----
>  include/xen/swiotlb-xen.h      |    2 +-
>  3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/xen/pci-swiotlb-xen.c
b/arch/x86/xen/pci-swiotlb-xen.c
> index fc0c78f..1608244 100644
> --- a/arch/x86/xen/pci-swiotlb-xen.c
> +++ b/arch/x86/xen/pci-swiotlb-xen.c
> @@ -69,7 +69,7 @@ int __init pci_xen_swiotlb_detect(void)
>  void __init pci_xen_swiotlb_init(void)
>  {
>  	if (xen_swiotlb) {
> -		xen_swiotlb_init(1);
> +		xen_swiotlb_init(1, true /* early */);
>  		dma_ops = &xen_swiotlb_dma_ops;
>  
>  		/* Make sure ACS will be enabled */
> @@ -84,7 +84,7 @@ int pci_xen_swiotlb_init_late(void)
>  	if (xen_swiotlb)
>  		return 0;
>  
> -	rc = xen_swiotlb_init(1);
> +	rc = xen_swiotlb_init(1, false /* late */);
>  	if (rc)
>  		return rc;
>  
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index 6f81994..02a52f3 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -176,7 +176,7 @@ static const char *xen_swiotlb_error(enum
xen_swiotlb_err err)
>  	}
>  	return "";
>  }
> -int __ref xen_swiotlb_init(int verbose)
> +int __ref xen_swiotlb_init(int verbose, bool early)
>  {
>  	unsigned long bytes, order;
>  	int rc = -ENOMEM;
> @@ -190,7 +190,7 @@ retry:
>  	/*
>  	 * Get IO TLB memory from any location.
>  	 */
> -	if (!after_bootmem)
> +	if (early)
>  		xen_io_tlb_start = alloc_bootmem_pages(PAGE_ALIGN(bytes));
>  	else {
>  #define SLABS_PER_PAGE (1 << (PAGE_SHIFT - IO_TLB_SHIFT))
> @@ -220,7 +220,7 @@ retry:
>  			       bytes,
>  			       xen_io_tlb_nslabs);
>  	if (rc) {
> -		if (!after_bootmem)
> +		if (early)
>  			free_bootmem(__pa(xen_io_tlb_start), PAGE_ALIGN(bytes));
>  		else {
>  			free_pages((unsigned long)xen_io_tlb_start, order);
> @@ -230,7 +230,7 @@ retry:
>  		goto error;
>  	}
>  	start_dma_addr = xen_virt_to_bus(xen_io_tlb_start);
> -	if (!after_bootmem)
> +	if (early)
>  		swiotlb_init_with_tbl(xen_io_tlb_start, xen_io_tlb_nslabs, verbose);
>  	else
>  		rc = swiotlb_late_init_with_tbl(xen_io_tlb_start, xen_io_tlb_nslabs);
> @@ -244,7 +244,7 @@ error:
>  		goto retry;
>  	}
>  	pr_err("%s (rc:%d)", xen_swiotlb_error(m_ret), rc);
> -	if (!after_bootmem)
> +	if (early)
>  		panic("%s (rc:%d)", xen_swiotlb_error(m_ret), rc);
>  	else
>  		free_pages((unsigned long)xen_io_tlb_start, order);
> diff --git a/include/xen/swiotlb-xen.h b/include/xen/swiotlb-xen.h
> index a0db2b7..de8bcc6 100644
> --- a/include/xen/swiotlb-xen.h
> +++ b/include/xen/swiotlb-xen.h
> @@ -3,7 +3,7 @@
>  
>  #include <linux/swiotlb.h>
>  
> -extern int xen_swiotlb_init(int verbose);
> +extern int xen_swiotlb_init(int verbose, bool early);
>  
>  extern void
>  *xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
> -- 
> 1.7.7.6
>

Stefano Stabellini

2012-Sep-17 14:53 UTC

head link

Re: Is: [PATCH 11/10] xen/swiotlb: For early initialization, return zero on success. Was: Re: [PATCH 10/10] xen/swiotlb: Depending on after_bootmem is not correct.

On Mon, 17 Sep 2012, Konrad Rzeszutek Wilk wrote:> On Mon, Sep 17, 2012 at 10:23:15AM -0400, Konrad Rzeszutek Wilk wrote:
> > > >  	start_dma_addr = xen_virt_to_bus(xen_io_tlb_start);
> > > > -	if (!after_bootmem)
> > > > +	rc = 0;
> > >      ^
> > > why does this change belong to this patch?
> > > 
> > > 
> > 
> > I took that out of the this patch, so it is now:
> > 
> 
> ..and spun out another patch to address the rc=0:
> 
> >From f85175ce01ba722cd4612230e7331dc0d4c8666f Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Date: Mon, 17 Sep 2012 10:20:09 -0400
> Subject: [PATCH 2/2] xen/swiotlb: For early initialization, return zero on
>  success.
> 
> If everything is setup properly we would return -ENOMEM since
> rc by default is set to that value. Lets not do that and return
> a proper return code.
> 
> Note: The reason the early code needs this special treatment
> is that it SWIOTLB library call does not return anything (and
> had it failed it would call panic()) - but our function does.
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>  drivers/xen/swiotlb-xen.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index 02a52f3..ab4c66c 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -230,9 +230,10 @@ retry:
>  		goto error;
>  	}
>  	start_dma_addr = xen_virt_to_bus(xen_io_tlb_start);
> -	if (early)
> +	if (early) {
>  		swiotlb_init_with_tbl(xen_io_tlb_start, xen_io_tlb_nslabs, verbose);
> -	else
> +		rc = 0;
> +	} else
>  		rc = swiotlb_late_init_with_tbl(xen_io_tlb_start, xen_io_tlb_nslabs);
>  	return rc;
>  error:
> -- 
> 1.7.7.6
>

Konrad Rzeszutek Wilk

2012-Sep-17 17:02 UTC

head link

Re: [PATCH 10/10] xen/swiotlb: Depending on after_bootmem is not correct.

On Mon, Sep 17, 2012 at 03:52:20PM +0100, Stefano Stabellini
wrote:> On Mon, 17 Sep 2012, Konrad Rzeszutek Wilk wrote:
> > > >  	start_dma_addr = xen_virt_to_bus(xen_io_tlb_start);
> > > > -	if (!after_bootmem)
> > > > +	rc = 0;
> > >      ^
> > > why does this change belong to this patch?
> > > 
> > > 
> > 
> > I took that out of the this patch, so it is now:
> > 
> > 
> > >From c5bc5502abc0f70b682c0f2a70d08e6319825163 Mon Sep 17 00:00:00
2001
> > From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > Date: Wed, 5 Sep 2012 13:35:47 -0400
> > Subject: [PATCH 1/2] xen/swiotlb: Depending on after_bootmem is not
correct.
> > 
> > When PCI IOMMUs are initialized it is after after_bootmem but
> > before a lot of "other" subsystems are initialized. As such
> > the check for after_bootmem is incorrect and we should
> > just use a parameter to define whether we are early or late.
> 
> Given that the after_bootmem checks are introduced by patch 6/10, I
> would just merge the two.
> In any case, it looks OK now.
Squashed it in:

From b82776005369899c1c7ca2e4b2414bb64b538d2c Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Thu, 23 Aug 2012 14:36:15 -0400
Subject: [PATCH] xen/swiotlb: Use the swiotlb_late_init_with_tbl to init
 Xen-SWIOTLB late when PV PCI is used.

With this patch we provide the functionality to initialize the
Xen-SWIOTLB late in the bootup cycle - specifically for
Xen PCI-frontend. We still will work if the user had
supplied ''iommu=soft'' on the Linux command line.

Note: We cannot depend on after_bootmem to automatically
determine whether this is early or not. This is because
when PCI IOMMUs are initialized it is after after_bootmem but
before a lot of "other" subsystems are initialized.

CC: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
[v1: Fix smatch warnings]
[v2: Added check for xen_swiotlb]
[v3: Rebased with new xen-swiotlb changes]
[v4: squashed xen/swiotlb: Depending on after_bootmem is not correct in]
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/include/asm/xen/swiotlb-xen.h |    2 +
 arch/x86/xen/pci-swiotlb-xen.c         |   24 ++++++++++++++-
 drivers/xen/swiotlb-xen.c              |   48 +++++++++++++++++++++++++------
 include/xen/swiotlb-xen.h              |    2 +-
 4 files changed, 63 insertions(+), 13 deletions(-)

diff --git a/arch/x86/include/asm/xen/swiotlb-xen.h
b/arch/x86/include/asm/xen/swiotlb-xen.h
index 1be1ab7..ee52fca 100644
--- a/arch/x86/include/asm/xen/swiotlb-xen.h
+++ b/arch/x86/include/asm/xen/swiotlb-xen.h
@@ -5,10 +5,12 @@
 extern int xen_swiotlb;
 extern int __init pci_xen_swiotlb_detect(void);
 extern void __init pci_xen_swiotlb_init(void);
+extern int pci_xen_swiotlb_init_late(void);
 #else
 #define xen_swiotlb (0)
 static inline int __init pci_xen_swiotlb_detect(void) { return 0; }
 static inline void __init pci_xen_swiotlb_init(void) { }
+static inline int pci_xen_swiotlb_init_late(void) { return -ENXIO; }
 #endif
 
 #endif /* _ASM_X86_SWIOTLB_XEN_H */
diff --git a/arch/x86/xen/pci-swiotlb-xen.c b/arch/x86/xen/pci-swiotlb-xen.c
index 1c17227..b152640 100644
--- a/arch/x86/xen/pci-swiotlb-xen.c
+++ b/arch/x86/xen/pci-swiotlb-xen.c
@@ -12,7 +12,7 @@
 #include <asm/iommu.h>
 #include <asm/dma.h>
 #endif
-
+#include <linux/export.h>
 int xen_swiotlb __read_mostly;
 
 static struct dma_map_ops xen_swiotlb_dma_ops = {
@@ -69,13 +69,33 @@ int __init pci_xen_swiotlb_detect(void)
 void __init pci_xen_swiotlb_init(void)
 {
 	if (xen_swiotlb) {
-		xen_swiotlb_init(1);
+		xen_swiotlb_init(1, true /* early */);
 		dma_ops = &xen_swiotlb_dma_ops;
 
 		/* Make sure ACS will be enabled */
 		pci_request_acs();
 	}
 }
+
+int pci_xen_swiotlb_init_late(void)
+{
+	int rc;
+
+	if (xen_swiotlb)
+		return 0;
+
+	rc = xen_swiotlb_init(1, false /* late */);
+	if (rc)
+		return rc;
+
+	dma_ops = &xen_swiotlb_dma_ops;
+	/* Make sure ACS will be enabled */
+	pci_request_acs();
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pci_xen_swiotlb_init_late);
+
 IOMMU_INIT_FINISH(pci_xen_swiotlb_detect,
 		  0,
 		  pci_xen_swiotlb_init,
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 701b103..7461edb 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -176,9 +176,9 @@ static const char *xen_swiotlb_error(enum xen_swiotlb_err
err)
 	}
 	return "";
 }
-void __init xen_swiotlb_init(int verbose)
+int __ref xen_swiotlb_init(int verbose, bool early)
 {
-	unsigned long bytes;
+	unsigned long bytes, order;
 	int rc = -ENOMEM;
 	enum xen_swiotlb_err m_ret = XEN_SWIOTLB_UNKNOWN;
 	unsigned int repeat = 3;
@@ -186,10 +186,28 @@ void __init xen_swiotlb_init(int verbose)
 	xen_io_tlb_nslabs = swiotlb_nr_tbl();
 retry:
 	bytes = xen_set_nslabs(xen_io_tlb_nslabs);
+	order = get_order(xen_io_tlb_nslabs << IO_TLB_SHIFT);
 	/*
 	 * Get IO TLB memory from any location.
 	 */
-	xen_io_tlb_start = alloc_bootmem_pages(PAGE_ALIGN(bytes));
+	if (early)
+		xen_io_tlb_start = alloc_bootmem_pages(PAGE_ALIGN(bytes));
+	else {
+#define SLABS_PER_PAGE (1 << (PAGE_SHIFT - IO_TLB_SHIFT))
+#define IO_TLB_MIN_SLABS ((1<<20) >> IO_TLB_SHIFT)
+		while ((SLABS_PER_PAGE << order) > IO_TLB_MIN_SLABS) {
+			xen_io_tlb_start = (void *)__get_free_pages(__GFP_NOWARN, order);
+			if (xen_io_tlb_start)
+				break;
+			order--;
+		}
+		if (order != get_order(bytes)) {
+			pr_warn("Warning: only able to allocate %ld MB "
+				"for software IO TLB\n", (PAGE_SIZE << order) >> 20);
+			xen_io_tlb_nslabs = SLABS_PER_PAGE << order;
+			bytes = xen_io_tlb_nslabs << IO_TLB_SHIFT;
+		}
+	}
 	if (!xen_io_tlb_start) {
 		m_ret = XEN_SWIOTLB_ENOMEM;
 		goto error;
@@ -202,14 +220,21 @@ retry:
 			       bytes,
 			       xen_io_tlb_nslabs);
 	if (rc) {
-		free_bootmem(__pa(xen_io_tlb_start), PAGE_ALIGN(bytes));
+		if (early)
+			free_bootmem(__pa(xen_io_tlb_start), PAGE_ALIGN(bytes));
+		else {
+			free_pages((unsigned long)xen_io_tlb_start, order);
+			xen_io_tlb_start = NULL;
+		}
 		m_ret = XEN_SWIOTLB_EFIXUP;
 		goto error;
 	}
 	start_dma_addr = xen_virt_to_bus(xen_io_tlb_start);
-	swiotlb_init_with_tbl(xen_io_tlb_start, xen_io_tlb_nslabs, verbose);
-
-	return;
+	if (early)
+		swiotlb_init_with_tbl(xen_io_tlb_start, xen_io_tlb_nslabs, verbose);
+	else
+		rc = swiotlb_late_init_with_tbl(xen_io_tlb_start, xen_io_tlb_nslabs);
+	return rc;
 error:
 	if (repeat--) {
 		xen_io_tlb_nslabs = max(1024UL, /* Min is 2MB */
@@ -218,10 +243,13 @@ error:
 		      (xen_io_tlb_nslabs << IO_TLB_SHIFT) >> 20);
 		goto retry;
 	}
-	xen_raw_printk("%s (rc:%d)", xen_swiotlb_error(m_ret), rc);
-	panic("%s (rc:%d)", xen_swiotlb_error(m_ret), rc);
+	pr_err("%s (rc:%d)", xen_swiotlb_error(m_ret), rc);
+	if (early)
+		panic("%s (rc:%d)", xen_swiotlb_error(m_ret), rc);
+	else
+		free_pages((unsigned long)xen_io_tlb_start, order);
+	return rc;
 }
-
 void *
 xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
 			   dma_addr_t *dma_handle, gfp_t flags,
diff --git a/include/xen/swiotlb-xen.h b/include/xen/swiotlb-xen.h
index 4f4d449..289ee50 100644
--- a/include/xen/swiotlb-xen.h
+++ b/include/xen/swiotlb-xen.h
@@ -3,7 +3,7 @@
 
 #include <linux/swiotlb.h>
 
-extern void xen_swiotlb_init(int verbose);
+extern int xen_swiotlb_init(int verbose, bool early);
 
 extern void
 *xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
-- 
1.7.7.6

Konrad Rzeszutek Wilk

2012-Sep-22 13:28 UTC

head link

Re: [Xen-devel] [PATCH] Xen-SWIOTLB fixes (v4) for v3.7

>
> to allow it to use an io_tlb passed in. Note: I hadn''t tested this
> on IA64 and that is something I need to do.
Done. I got my hands on a HP zx6000 and the patch series did not show
any regressions.

Xen devel - Sep 2012 - [PATCH] Xen-SWIOTLB fixes (v4) for v3.7

[PATCH] Xen-SWIOTLB fixes (v4) for v3.7

[PATCH 01/10] xen/swiotlb: Simplify the logic.

[PATCH 02/10] xen/swiotlb: With more than 4GB on 64-bit, disable the native SWIOTLB.

[PATCH 03/10] swiotlb: add the late swiotlb initialization function with iotlb memory

[PATCH 04/10] xen/swiotlb: Move the nr_tbl determination in its own function.

[PATCH 05/10] xen/swiotlb: Move the error strings to its own function.

[PATCH 06/10] xen/swiotlb: Use the swiotlb_late_init_with_tbl to init Xen-SWIOTLB late when PV PCI is used.

[PATCH 07/10] xen/pcifront: Use Xen-SWIOTLB when initting if required.

[PATCH 08/10] xen/swiotlb: Remove functions not needed anymore.

[PATCH 09/10] xen/swiotlb: Fix compile warnings when using plain integer instead of NULL pointer.

[PATCH 10/10] xen/swiotlb: Depending on after_bootmem is not correct.

Re: [Xen-devel] [PATCH 09/10] xen/swiotlb: Fix compile warnings when using plain integer instead of NULL pointer.

Re: [Xen-devel] [PATCH 09/10] xen/swiotlb: Fix compile warnings when using plain integer instead of NULL pointer.

Re: [Xen-devel] [PATCH 09/10] xen/swiotlb: Fix compile warnings when using plain integer instead of NULL pointer.

Re: [PATCH 05/10] xen/swiotlb: Move the error strings to its own function.

Re: [PATCH 08/10] xen/swiotlb: Remove functions not needed anymore.

Re: [PATCH 04/10] xen/swiotlb: Move the nr_tbl determination in its own function.

Re: [PATCH 10/10] xen/swiotlb: Depending on after_bootmem is not correct.

Re: [PATCH 09/10] xen/swiotlb: Fix compile warnings when using plain integer instead of NULL pointer.

Re: [PATCH 06/10] xen/swiotlb: Use the swiotlb_late_init_with_tbl to init Xen-SWIOTLB late when PV PCI is used.

Re: [PATCH 08/10] xen/swiotlb: Remove functions not needed anymore.

Re: [PATCH 10/10] xen/swiotlb: Depending on after_bootmem is not correct.

Re: [PATCH 08/10] xen/swiotlb: Remove functions not needed anymore.

Re: [PATCH 10/10] xen/swiotlb: Depending on after_bootmem is not correct.

Is: [PATCH 11/10] xen/swiotlb: For early initialization, return zero on success. Was: Re: [PATCH 10/10] xen/swiotlb: Depending on after_bootmem is not correct.

Re: [PATCH 10/10] xen/swiotlb: Depending on after_bootmem is not correct.

Re: Is: [PATCH 11/10] xen/swiotlb: For early initialization, return zero on success. Was: Re: [PATCH 10/10] xen/swiotlb: Depending on after_bootmem is not correct.

Re: [PATCH 10/10] xen/swiotlb: Depending on after_bootmem is not correct.

Re: [Xen-devel] [PATCH] Xen-SWIOTLB fixes (v4) for v3.7