linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Xen-SWIOTLB fies (v3) for v3.7.
@ 2012-08-16 15:54 Konrad Rzeszutek Wilk
  2012-08-16 15:54 ` [PATCH 1/5] xen/swiotlb: Simplify the logic Konrad Rzeszutek Wilk
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-08-16 15:54 UTC (permalink / raw)
  To: linux-kernel, xen-devel

Since changeset: v2: https://lkml.org/lkml/2012/7/31/337
 - fixed smatch warnings (add __ref and appropiate header files.

Besides that it is exactly like the one I posted in July except
it has been tested more extensively.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/5] xen/swiotlb: Simplify the logic.
  2012-08-16 15:54 [PATCH] Xen-SWIOTLB fies (v3) for v3.7 Konrad Rzeszutek Wilk
@ 2012-08-16 15:54 ` Konrad Rzeszutek Wilk
  2012-08-17 17:08   ` [Xen-devel] " Stefano Stabellini
  2012-08-16 15:54 ` [PATCH 2/5] xen/swiotlb: With more than 4GB on 64-bit, disable the native SWIOTLB Konrad Rzeszutek Wilk
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-08-16 15:54 UTC (permalink / raw)
  To: linux-kernel, xen-devel; +Cc: Konrad Rzeszutek Wilk

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.

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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/5] xen/swiotlb: With more than 4GB on 64-bit, disable the native SWIOTLB.
  2012-08-16 15:54 [PATCH] Xen-SWIOTLB fies (v3) for v3.7 Konrad Rzeszutek Wilk
  2012-08-16 15:54 ` [PATCH 1/5] xen/swiotlb: Simplify the logic Konrad Rzeszutek Wilk
@ 2012-08-16 15:54 ` Konrad Rzeszutek Wilk
  2012-08-16 15:54 ` [PATCH 3/5] swiotlb: add the late swiotlb initialization function with iotlb memory Konrad Rzeszutek Wilk
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-08-16 15:54 UTC (permalink / raw)
  To: linux-kernel, xen-devel; +Cc: Konrad Rzeszutek Wilk

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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/5] swiotlb: add the late swiotlb initialization function with iotlb memory
  2012-08-16 15:54 [PATCH] Xen-SWIOTLB fies (v3) for v3.7 Konrad Rzeszutek Wilk
  2012-08-16 15:54 ` [PATCH 1/5] xen/swiotlb: Simplify the logic Konrad Rzeszutek Wilk
  2012-08-16 15:54 ` [PATCH 2/5] xen/swiotlb: With more than 4GB on 64-bit, disable the native SWIOTLB Konrad Rzeszutek Wilk
@ 2012-08-16 15:54 ` Konrad Rzeszutek Wilk
  2012-08-16 15:54 ` [PATCH 4/5] xen/swiotlb: Use the swiotlb_late_init_with_tbl to init Xen-SWIOTLB late when PV PCI is used Konrad Rzeszutek Wilk
  2012-08-16 15:54 ` [PATCH 5/5] xen/pcifront: Use Xen-SWIOTLB when initting if required Konrad Rzeszutek Wilk
  4 siblings, 0 replies; 10+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-08-16 15:54 UTC (permalink / raw)
  To: linux-kernel, xen-devel; +Cc: Konrad Rzeszutek Wilk, FUJITA Tomonori

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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/5] xen/swiotlb: Use the swiotlb_late_init_with_tbl to init Xen-SWIOTLB late when PV PCI is used.
  2012-08-16 15:54 [PATCH] Xen-SWIOTLB fies (v3) for v3.7 Konrad Rzeszutek Wilk
                   ` (2 preceding siblings ...)
  2012-08-16 15:54 ` [PATCH 3/5] swiotlb: add the late swiotlb initialization function with iotlb memory Konrad Rzeszutek Wilk
@ 2012-08-16 15:54 ` Konrad Rzeszutek Wilk
  2012-08-17 17:25   ` [Xen-devel] " Stefano Stabellini
  2012-08-16 15:54 ` [PATCH 5/5] xen/pcifront: Use Xen-SWIOTLB when initting if required Konrad Rzeszutek Wilk
  4 siblings, 1 reply; 10+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-08-16 15:54 UTC (permalink / raw)
  To: linux-kernel, xen-devel; +Cc: Konrad Rzeszutek Wilk, FUJITA Tomonori

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]
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         |   17 +++++++++-
 drivers/xen/swiotlb-xen.c              |   54 ++++++++++++++++++++++++++-----
 include/xen/swiotlb-xen.h              |    1 +
 4 files changed, 64 insertions(+), 10 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..031d8bc 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,21 @@ void __init pci_xen_swiotlb_init(void)
 		pci_request_acs();
 	}
 }
+
+int pci_xen_swiotlb_init_late(void)
+{
+	int rc = xen_swiotlb_late_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 1afb4fb..1942a3e 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -145,13 +145,14 @@ xen_swiotlb_fixup(void *buf, size_t size, unsigned long nslabs)
 	return 0;
 }
 
-void __init xen_swiotlb_init(int verbose)
+static int __ref __xen_swiotlb_init(int verbose, bool early)
 {
 	unsigned long bytes;
 	int rc = -ENOMEM;
 	unsigned long nr_tbl;
 	char *m = NULL;
 	unsigned int repeat = 3;
+	unsigned long order;
 
 	nr_tbl = swiotlb_nr_tbl();
 	if (nr_tbl)
@@ -161,12 +162,31 @@ void __init xen_swiotlb_init(int verbose)
 		xen_io_tlb_nslabs = ALIGN(xen_io_tlb_nslabs, IO_TLB_SEGSIZE);
 	}
 retry:
+	order = get_order(xen_io_tlb_nslabs << IO_TLB_SHIFT);
 	bytes = 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 = "Cannot allocate Xen-SWIOTLB buffer!\n";
 		goto error;
@@ -179,17 +199,22 @@ 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));
 		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!";
+		    " is too fragmented!";
 		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;
+	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);
+	return rc;
 error:
 	if (repeat--) {
 		xen_io_tlb_nslabs = max(1024UL, /* Min is 2MB */
@@ -198,10 +223,21 @@ 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);
+	pr_err("%s (rc:%d)", m, rc);
+	if (early)
+		panic("%s (rc:%d)", m, rc);
+	else
+		free_pages((unsigned long)xen_io_tlb_start, order);
+	return rc;
+}
+void __init xen_swiotlb_init(int verbose)
+{
+	__xen_swiotlb_init(verbose, true /* early */);
+}
+int xen_swiotlb_late_init(int verbose)
+{
+	return __xen_swiotlb_init(verbose, false /* late */);
 }
-
 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..d38d984 100644
--- a/include/xen/swiotlb-xen.h
+++ b/include/xen/swiotlb-xen.h
@@ -4,6 +4,7 @@
 #include <linux/swiotlb.h>
 
 extern void xen_swiotlb_init(int verbose);
+extern int  xen_swiotlb_late_init(int verbose);
 
 extern void
 *xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/5] xen/pcifront: Use Xen-SWIOTLB when initting if required.
  2012-08-16 15:54 [PATCH] Xen-SWIOTLB fies (v3) for v3.7 Konrad Rzeszutek Wilk
                   ` (3 preceding siblings ...)
  2012-08-16 15:54 ` [PATCH 4/5] xen/swiotlb: Use the swiotlb_late_init_with_tbl to init Xen-SWIOTLB late when PV PCI is used Konrad Rzeszutek Wilk
@ 2012-08-16 15:54 ` Konrad Rzeszutek Wilk
  2012-08-17 17:14   ` [Xen-devel] " Stefano Stabellini
  4 siblings, 1 reply; 10+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-08-16 15:54 UTC (permalink / raw)
  To: linux-kernel, xen-devel; +Cc: Konrad Rzeszutek Wilk

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.

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

diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index d6cc62c..ca92801 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;
 }
 
@@ -699,6 +704,7 @@ static void pcifront_disconnect(struct pcifront_device *pdev)
 
 	spin_unlock(&pcifront_dev_lock);
 }
+
 static struct pcifront_device *alloc_pdev(struct xenbus_device *xdev)
 {
 	struct pcifront_device *pdev;
@@ -842,10 +848,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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [Xen-devel] [PATCH 1/5] xen/swiotlb: Simplify the logic.
  2012-08-16 15:54 ` [PATCH 1/5] xen/swiotlb: Simplify the logic Konrad Rzeszutek Wilk
@ 2012-08-17 17:08   ` Stefano Stabellini
  0 siblings, 0 replies; 10+ messages in thread
From: Stefano Stabellini @ 2012-08-17 17:08 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: linux-kernel, xen-devel

On Thu, 16 Aug 2012, Konrad Rzeszutek Wilk wrote:
> 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.
> 
> 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 |    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
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Xen-devel] [PATCH 5/5] xen/pcifront: Use Xen-SWIOTLB when initting if required.
  2012-08-16 15:54 ` [PATCH 5/5] xen/pcifront: Use Xen-SWIOTLB when initting if required Konrad Rzeszutek Wilk
@ 2012-08-17 17:14   ` Stefano Stabellini
  0 siblings, 0 replies; 10+ messages in thread
From: Stefano Stabellini @ 2012-08-17 17:14 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: linux-kernel, xen-devel

On Thu, 16 Aug 2012, Konrad Rzeszutek Wilk wrote:
> 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.
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>


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

>  drivers/pci/xen-pcifront.c |   14 ++++++++++----
>  1 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
> index d6cc62c..ca92801 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;
>  }
>  
> @@ -699,6 +704,7 @@ static void pcifront_disconnect(struct pcifront_device *pdev)
>  
>  	spin_unlock(&pcifront_dev_lock);
>  }
> +
>  static struct pcifront_device *alloc_pdev(struct xenbus_device *xdev)
>  {
>  	struct pcifront_device *pdev;
> @@ -842,10 +848,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
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Xen-devel] [PATCH 4/5] xen/swiotlb: Use the swiotlb_late_init_with_tbl to init Xen-SWIOTLB late when PV PCI is used.
  2012-08-16 15:54 ` [PATCH 4/5] xen/swiotlb: Use the swiotlb_late_init_with_tbl to init Xen-SWIOTLB late when PV PCI is used Konrad Rzeszutek Wilk
@ 2012-08-17 17:25   ` Stefano Stabellini
  2012-08-23 19:30     ` Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 10+ messages in thread
From: Stefano Stabellini @ 2012-08-17 17:25 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: linux-kernel, xen-devel, FUJITA Tomonori

On Thu, 16 Aug 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]
> 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         |   17 +++++++++-
>  drivers/xen/swiotlb-xen.c              |   54 ++++++++++++++++++++++++++-----
>  include/xen/swiotlb-xen.h              |    1 +
>  4 files changed, 64 insertions(+), 10 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..031d8bc 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,21 @@ void __init pci_xen_swiotlb_init(void)
>  		pci_request_acs();
>  	}
>  }
> +
> +int pci_xen_swiotlb_init_late(void)
> +{
> +	int rc = xen_swiotlb_late_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);

shouldn't we be checking whether the xen_swiotlb has already been
initialized?


>  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 1afb4fb..1942a3e 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -145,13 +145,14 @@ xen_swiotlb_fixup(void *buf, size_t size, unsigned long nslabs)
>  	return 0;
>  }
>  
> -void __init xen_swiotlb_init(int verbose)
> +static int __ref __xen_swiotlb_init(int verbose, bool early)
>  {
>  	unsigned long bytes;
>  	int rc = -ENOMEM;
>  	unsigned long nr_tbl;
>  	char *m = NULL;
>  	unsigned int repeat = 3;
> +	unsigned long order;
>
>  	nr_tbl = swiotlb_nr_tbl();
>  	if (nr_tbl)
> @@ -161,12 +162,31 @@ void __init xen_swiotlb_init(int verbose)
>  		xen_io_tlb_nslabs = ALIGN(xen_io_tlb_nslabs, IO_TLB_SEGSIZE);
>  	}
>  retry:
> +	order = get_order(xen_io_tlb_nslabs << IO_TLB_SHIFT);
>  	bytes = 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 = "Cannot allocate Xen-SWIOTLB buffer!\n";
>  		goto error;
> @@ -179,17 +199,22 @@ 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));
>  		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!";
> +		    " is too fragmented!";
>  		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;
> +	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);
> +	return rc;
>  error:
>  	if (repeat--) {
>  		xen_io_tlb_nslabs = max(1024UL, /* Min is 2MB */
> @@ -198,10 +223,21 @@ 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);
> +	pr_err("%s (rc:%d)", m, rc);
> +	if (early)
> +		panic("%s (rc:%d)", m, rc);
> +	else
> +		free_pages((unsigned long)xen_io_tlb_start, order);
> +	return rc;
> +}

All these "if" make the code a harder to read. Would it be possible at
least to unify the error paths and just check on after_bootmem whether
we need to call free_pages or free_bootmem?
In fact using after_bootmem you might get away without an early
parameter.


> +void __init xen_swiotlb_init(int verbose)
> +{
> +	__xen_swiotlb_init(verbose, true /* early */);
> +}
> +int xen_swiotlb_late_init(int verbose)
> +{
> +	return __xen_swiotlb_init(verbose, false /* late */);
>  }
>  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..d38d984 100644
> --- a/include/xen/swiotlb-xen.h
> +++ b/include/xen/swiotlb-xen.h
> @@ -4,6 +4,7 @@
>  #include <linux/swiotlb.h>
>  
>  extern void xen_swiotlb_init(int verbose);
> +extern int  xen_swiotlb_late_init(int verbose);
>  
>  extern void
>  *xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
> -- 
> 1.7.7.6
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Xen-devel] [PATCH 4/5] xen/swiotlb: Use the swiotlb_late_init_with_tbl to init Xen-SWIOTLB late when PV PCI is used.
  2012-08-17 17:25   ` [Xen-devel] " Stefano Stabellini
@ 2012-08-23 19:30     ` Konrad Rzeszutek Wilk
  0 siblings, 0 replies; 10+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-08-23 19:30 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: linux-kernel, xen-devel, FUJITA Tomonori

On Fri, Aug 17, 2012 at 06:25:56PM +0100, Stefano Stabellini wrote:
> On Thu, 16 Aug 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]
> > 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         |   17 +++++++++-
> >  drivers/xen/swiotlb-xen.c              |   54 ++++++++++++++++++++++++++-----
> >  include/xen/swiotlb-xen.h              |    1 +
> >  4 files changed, 64 insertions(+), 10 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..031d8bc 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,21 @@ void __init pci_xen_swiotlb_init(void)
> >  		pci_request_acs();
> >  	}
> >  }
> > +
> > +int pci_xen_swiotlb_init_late(void)
> > +{
> > +	int rc = xen_swiotlb_late_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);
> 
> shouldn't we be checking whether the xen_swiotlb has already been
> initialized?

Yes.
> 
> 
> >  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 1afb4fb..1942a3e 100644
> > --- a/drivers/xen/swiotlb-xen.c
> > +++ b/drivers/xen/swiotlb-xen.c
> > @@ -145,13 +145,14 @@ xen_swiotlb_fixup(void *buf, size_t size, unsigned long nslabs)
> >  	return 0;
> >  }
> >  
> > -void __init xen_swiotlb_init(int verbose)
> > +static int __ref __xen_swiotlb_init(int verbose, bool early)
> >  {
> >  	unsigned long bytes;
> >  	int rc = -ENOMEM;
> >  	unsigned long nr_tbl;
> >  	char *m = NULL;
> >  	unsigned int repeat = 3;
> > +	unsigned long order;
> >
> >  	nr_tbl = swiotlb_nr_tbl();
> >  	if (nr_tbl)
> > @@ -161,12 +162,31 @@ void __init xen_swiotlb_init(int verbose)
> >  		xen_io_tlb_nslabs = ALIGN(xen_io_tlb_nslabs, IO_TLB_SEGSIZE);
> >  	}
> >  retry:
> > +	order = get_order(xen_io_tlb_nslabs << IO_TLB_SHIFT);
> >  	bytes = 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 = "Cannot allocate Xen-SWIOTLB buffer!\n";
> >  		goto error;
> > @@ -179,17 +199,22 @@ 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));
> >  		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!";
> > +		    " is too fragmented!";
> >  		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;
> > +	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);
> > +	return rc;
> >  error:
> >  	if (repeat--) {
> >  		xen_io_tlb_nslabs = max(1024UL, /* Min is 2MB */
> > @@ -198,10 +223,21 @@ 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);
> > +	pr_err("%s (rc:%d)", m, rc);
> > +	if (early)
> > +		panic("%s (rc:%d)", m, rc);
> > +	else
> > +		free_pages((unsigned long)xen_io_tlb_start, order);
> > +	return rc;
> > +}
> 
> All these "if" make the code a harder to read. Would it be possible at
> least to unify the error paths and just check on after_bootmem whether
> we need to call free_pages or free_bootmem?
> In fact using after_bootmem you might get away without an early
> parameter.


Sure thing. Here is a stab at it.


>From 573db1dbc6adf3f5efc8c699714329caffa43daf Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Thu, 23 Aug 2012 13:55:26 -0400
Subject: [PATCH 1/3] 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



>From 1bee8a244d1fed47e307c5951a98f7922cab7993 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Thu, 23 Aug 2012 14:03:55 -0400
Subject: [PATCH 2/3] 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


and the last one:


>From d815253cbf26a5273e77f829cc414e0808500263 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 3/3] 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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2012-08-23 19:41 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-16 15:54 [PATCH] Xen-SWIOTLB fies (v3) for v3.7 Konrad Rzeszutek Wilk
2012-08-16 15:54 ` [PATCH 1/5] xen/swiotlb: Simplify the logic Konrad Rzeszutek Wilk
2012-08-17 17:08   ` [Xen-devel] " Stefano Stabellini
2012-08-16 15:54 ` [PATCH 2/5] xen/swiotlb: With more than 4GB on 64-bit, disable the native SWIOTLB Konrad Rzeszutek Wilk
2012-08-16 15:54 ` [PATCH 3/5] swiotlb: add the late swiotlb initialization function with iotlb memory Konrad Rzeszutek Wilk
2012-08-16 15:54 ` [PATCH 4/5] xen/swiotlb: Use the swiotlb_late_init_with_tbl to init Xen-SWIOTLB late when PV PCI is used Konrad Rzeszutek Wilk
2012-08-17 17:25   ` [Xen-devel] " Stefano Stabellini
2012-08-23 19:30     ` Konrad Rzeszutek Wilk
2012-08-16 15:54 ` [PATCH 5/5] xen/pcifront: Use Xen-SWIOTLB when initting if required Konrad Rzeszutek Wilk
2012-08-17 17:14   ` [Xen-devel] " Stefano Stabellini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).