2009-03-31 22:53:55

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: [PATCH] swiotlb updates for Xen dom0


Hi Fujita,

This series adds Xen support to x86 swiotlb. This is mostly a matter of
adding some Xen code into the existing hooks in pci-swiotlb_64.c:
- swiotlb_alloc_boot
- swiotlb_arch_range_needs_mapping
- swiotlb_phys_to_bus/bus_to_phys

These hooks are conditional on xen_pv_domain(), which compiles to a
constant 0 when CONFIG_XEN is not enabled (and is a simple variable
read when it is).

All the Xen-specific code is in xen-iommu.c.

This series is just the patches which touch lib/swiotlb.c or
pci-swiotlb_64.c. You can see them with more context in:
git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git for-linus/xen/dom0/swiotlb

Thanks,
J

arch/x86/kernel/pci-swiotlb_64.c | 31 ++++++++++++++++-
arch/x86/xen/Kconfig | 1
drivers/pci/xen-iommu.c | 68 ++++++++++++++++++++++++++++++++++++---
include/xen/swiotlb.h | 38 +++++++++++++++++++++
lib/swiotlb.c | 3 +
5 files changed, 133 insertions(+), 8 deletions(-)


2009-03-31 22:52:31

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: [PATCH 2/9] xen swiotlb: fixup swiotlb is chunks smaller than MAX_CONTIG_ORDER

From: Ian Campbell <[email protected]>

Impact: bugfix

Don't attempt to make larger memory ranges than Xen can cope with
contiguous.

Signed-off-by: Ian Campbell <[email protected]>
Signed-off-by: Jeremy Fitzhardinge <[email protected]>
---
drivers/pci/xen-iommu.c | 27 ++++++++++++++++++---------
1 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/xen-iommu.c b/drivers/pci/xen-iommu.c
index c593058..b9b4620 100644
--- a/drivers/pci/xen-iommu.c
+++ b/drivers/pci/xen-iommu.c
@@ -5,6 +5,7 @@
#include <linux/module.h>
#include <linux/version.h>
#include <linux/scatterlist.h>
+#include <linux/swiotlb.h>
#include <linux/io.h>
#include <linux/bug.h>

@@ -36,19 +37,27 @@ do { \
} while (0)


+static int max_dma_bits = 32;
+
void xen_swiotlb_fixup(void *buf, size_t size, unsigned long nslabs)
{
- unsigned order = get_order(size);
-
- printk(KERN_DEBUG "xen_swiotlb_fixup: buf=%p size=%zu order=%u\n",
- buf, size, order);
-
- if (WARN_ON(size != (PAGE_SIZE << order)))
- return;
-
- if (xen_create_contiguous_region((unsigned long)buf,
- order, 0xffffffff))
- printk(KERN_ERR "xen_create_contiguous_region failed\n");
+ int i, rc;
+ int dma_bits;
+
+ printk(KERN_DEBUG "xen_swiotlb_fixup: buf=%p size=%zu\n",
+ buf, size);
+
+ dma_bits = get_order(IO_TLB_SEGSIZE << IO_TLB_SHIFT) + PAGE_SHIFT;
+ for (i = 0; i < nslabs; i += IO_TLB_SEGSIZE) {
+ do {
+ rc = xen_create_contiguous_region(
+ (unsigned long)buf + (i << IO_TLB_SHIFT),
+ get_order(IO_TLB_SEGSIZE << IO_TLB_SHIFT),
+ dma_bits);
+ } while (rc && dma_bits++ < max_dma_bits);
+ if (rc)
+ panic(KERN_ERR "xen_create_contiguous_region failed\n");
+ }
}
static inline int address_needs_mapping(struct device *hwdev,
dma_addr_t addr)
--
1.6.0.6

2009-03-31 22:52:47

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: [PATCH 3/9] xen: add hooks for mapping phys<->bus addresses in swiotlb

From: Ian Campbell <[email protected]>

Impact: Xen support for DMA

Add hooks to allow Xen to do translation between pfn and mfns for the swiotlb
layer, so that dma actually ends up going to the proper machine pages.

Signed-off-by: Ian Campbell <[email protected]>
Signed-off-by: Jeremy Fitzhardinge <[email protected]>
Reviewed-by: "H. Peter Anvin" <[email protected]>
---
arch/x86/kernel/pci-swiotlb_64.c | 6 ++++++
drivers/pci/xen-iommu.c | 10 ++++++++++
include/xen/swiotlb.h | 12 ++++++++++++
3 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/pci-swiotlb_64.c b/arch/x86/kernel/pci-swiotlb_64.c
index be379af..7f87ce2 100644
--- a/arch/x86/kernel/pci-swiotlb_64.c
+++ b/arch/x86/kernel/pci-swiotlb_64.c
@@ -38,11 +38,17 @@ void *swiotlb_alloc(unsigned order, unsigned long nslabs)

dma_addr_t swiotlb_phys_to_bus(struct device *hwdev, phys_addr_t paddr)
{
+ if (xen_pv_domain())
+ return xen_phys_to_bus(paddr);
+
return paddr;
}

phys_addr_t swiotlb_bus_to_phys(dma_addr_t baddr)
{
+ if (xen_pv_domain())
+ return xen_bus_to_phys(baddr);
+
return baddr;
}

diff --git a/drivers/pci/xen-iommu.c b/drivers/pci/xen-iommu.c
index b9b4620..47d87a7 100644
--- a/drivers/pci/xen-iommu.c
+++ b/drivers/pci/xen-iommu.c
@@ -59,6 +59,16 @@ void xen_swiotlb_fixup(void *buf, size_t size, unsigned long nslabs)
panic(KERN_ERR "xen_create_contiguous_region failed\n");
}
}
+dma_addr_t xen_phys_to_bus(phys_addr_t paddr)
+{
+ return phys_to_machine(XPADDR(paddr)).maddr;
+}
+
+phys_addr_t xen_bus_to_phys(dma_addr_t daddr)
+{
+ return machine_to_phys(XMADDR(daddr)).paddr;
+}
+
static inline int address_needs_mapping(struct device *hwdev,
dma_addr_t addr)
{
diff --git a/include/xen/swiotlb.h b/include/xen/swiotlb.h
index 8d59439..3d96b07 100644
--- a/include/xen/swiotlb.h
+++ b/include/xen/swiotlb.h
@@ -3,10 +3,22 @@

#ifdef CONFIG_PCI_XEN
extern void xen_swiotlb_fixup(void *buf, size_t size, unsigned long nslabs);
+extern phys_addr_t xen_bus_to_phys(dma_addr_t daddr);
+extern dma_addr_t xen_phys_to_bus(phys_addr_t paddr);
#else
static inline void xen_swiotlb_fixup(void *buf, size_t size, unsigned long nslabs)
{
}
+
+static inline phys_addr_t xen_bus_to_phys(dma_addr_t daddr)
+{
+ return daddr;
+}
+
+static inline dma_addr_t xen_phys_to_bus(phys_addr_t paddr)
+{
+ return paddr;
+}
#endif

#endif /* _XEN_SWIOTLB_H */
--
1.6.0.6

2009-03-31 22:53:12

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: [PATCH 4/9] xen/swiotlb: add hook for swiotlb_arch_range_needs_mapping

From: Ian Campbell <[email protected]>

Impact: Xen support for DMA

Add hook so that Xen can determine whether a particular address range needs
pfn<->mfn mapping.

Signed-off-by: Ian Campbell <[email protected]>
Signed-off-by: Jeremy Fitzhardinge <[email protected]>
Reviewed-by: "H. Peter Anvin" <[email protected]>
---
arch/x86/kernel/pci-swiotlb_64.c | 3 +++
drivers/pci/xen-iommu.c | 5 +++++
include/xen/swiotlb.h | 8 ++++++++
3 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/pci-swiotlb_64.c b/arch/x86/kernel/pci-swiotlb_64.c
index 7f87ce2..7aa8c18 100644
--- a/arch/x86/kernel/pci-swiotlb_64.c
+++ b/arch/x86/kernel/pci-swiotlb_64.c
@@ -54,6 +54,9 @@ phys_addr_t swiotlb_bus_to_phys(dma_addr_t baddr)

int __weak swiotlb_arch_range_needs_mapping(phys_addr_t paddr, size_t size)
{
+ if (xen_pv_domain())
+ return xen_range_needs_mapping(paddr, size);
+
return 0;
}

diff --git a/drivers/pci/xen-iommu.c b/drivers/pci/xen-iommu.c
index 47d87a7..fcef078 100644
--- a/drivers/pci/xen-iommu.c
+++ b/drivers/pci/xen-iommu.c
@@ -118,6 +118,11 @@ static int range_straddles_page_boundary(phys_addr_t p, size_t size)
return 1;
}

+int xen_range_needs_mapping(phys_addr_t paddr, size_t size)
+{
+ return range_straddles_page_boundary(paddr, size);
+}
+
static inline void xen_dma_unmap_page(struct page *page)
{
/* Xen TODO: 2.6.18 xen calls __gnttab_dma_unmap_page here
diff --git a/include/xen/swiotlb.h b/include/xen/swiotlb.h
index 3d96b07..e975aa0 100644
--- a/include/xen/swiotlb.h
+++ b/include/xen/swiotlb.h
@@ -5,6 +5,7 @@
extern void xen_swiotlb_fixup(void *buf, size_t size, unsigned long nslabs);
extern phys_addr_t xen_bus_to_phys(dma_addr_t daddr);
extern dma_addr_t xen_phys_to_bus(phys_addr_t paddr);
+extern int xen_range_needs_mapping(phys_addr_t phys, size_t size);
#else
static inline void xen_swiotlb_fixup(void *buf, size_t size, unsigned long nslabs)
{
@@ -19,6 +20,13 @@ static inline dma_addr_t xen_phys_to_bus(phys_addr_t paddr)
{
return paddr;
}
+
+static inline int xen_range_needs_mapping(phys_addr_t phys, size_t size)
+{
+ return 0;
+}
#endif

+
+
#endif /* _XEN_SWIOTLB_H */
--
1.6.0.6

2009-03-31 22:53:38

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: [PATCH 1/9] xen: make sure swiotlb allocation is physically contigious

Impact: make swiotlb allocation suitable for Xen

When allocating the swiotlb buffer under Xen, make sure the memory is
physically contiguous so that its really suitable for DMA.

Do this by allocating the memory as usual, but then call a Xen
function to rearrange the underlying pages to be physically
contiguous.

Signed-off-by: Jeremy Fitzhardinge <[email protected]>
Reviewed-by: "H. Peter Anvin" <[email protected]>
---
arch/x86/kernel/pci-swiotlb_64.c | 17 +++++++++++++++--
drivers/pci/xen-iommu.c | 16 ++++++++++++++++
include/xen/swiotlb.h | 12 ++++++++++++
3 files changed, 43 insertions(+), 2 deletions(-)
create mode 100644 include/xen/swiotlb.h

diff --git a/arch/x86/kernel/pci-swiotlb_64.c b/arch/x86/kernel/pci-swiotlb_64.c
index 0f03b06..be379af 100644
--- a/arch/x86/kernel/pci-swiotlb_64.c
+++ b/arch/x86/kernel/pci-swiotlb_64.c
@@ -11,16 +11,29 @@
#include <asm/swiotlb.h>
#include <asm/dma.h>

+#include <xen/swiotlb.h>
+#include <asm/xen/hypervisor.h>
+
int swiotlb __read_mostly;

void * __init swiotlb_alloc_boot(size_t size, unsigned long nslabs)
{
- return alloc_bootmem_low_pages(size);
+ void *ret = alloc_bootmem_low_pages(size);
+
+ if (ret && xen_pv_domain())
+ xen_swiotlb_fixup(ret, size, nslabs);
+
+ return ret;
}

void *swiotlb_alloc(unsigned order, unsigned long nslabs)
{
- return (void *)__get_free_pages(GFP_DMA | __GFP_NOWARN, order);
+ void *ret = (void *)__get_free_pages(GFP_DMA | __GFP_NOWARN, order);
+
+ if (ret && xen_pv_domain())
+ xen_swiotlb_fixup(ret, 1u << order, nslabs);
+
+ return ret;
}

dma_addr_t swiotlb_phys_to_bus(struct device *hwdev, phys_addr_t paddr)
diff --git a/drivers/pci/xen-iommu.c b/drivers/pci/xen-iommu.c
index 32a8b49..c593058 100644
--- a/drivers/pci/xen-iommu.c
+++ b/drivers/pci/xen-iommu.c
@@ -12,6 +12,7 @@
#include <xen/grant_table.h>
#include <xen/page.h>
#include <xen/xen-ops.h>
+#include <xen/swiotlb.h>

#include <asm/iommu.h>
#include <asm/swiotlb.h>
@@ -34,6 +35,21 @@ do { \
(unsigned long long)addr + size); \
} while (0)

+
+void xen_swiotlb_fixup(void *buf, size_t size, unsigned long nslabs)
+{
+ unsigned order = get_order(size);
+
+ printk(KERN_DEBUG "xen_swiotlb_fixup: buf=%p size=%zu order=%u\n",
+ buf, size, order);
+
+ if (WARN_ON(size != (PAGE_SIZE << order)))
+ return;
+
+ if (xen_create_contiguous_region((unsigned long)buf,
+ order, 0xffffffff))
+ printk(KERN_ERR "xen_create_contiguous_region failed\n");
+}
static inline int address_needs_mapping(struct device *hwdev,
dma_addr_t addr)
{
diff --git a/include/xen/swiotlb.h b/include/xen/swiotlb.h
new file mode 100644
index 0000000..8d59439
--- /dev/null
+++ b/include/xen/swiotlb.h
@@ -0,0 +1,12 @@
+#ifndef _XEN_SWIOTLB_H
+#define _XEN_SWIOTLB_H
+
+#ifdef CONFIG_PCI_XEN
+extern void xen_swiotlb_fixup(void *buf, size_t size, unsigned long nslabs);
+#else
+static inline void xen_swiotlb_fixup(void *buf, size_t size, unsigned long nslabs)
+{
+}
+#endif
+
+#endif /* _XEN_SWIOTLB_H */
--
1.6.0.6

2009-03-31 22:54:29

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: [PATCH 5/9] xen: enable swiotlb for xen domain 0.

From: Ian Campbell <[email protected]>

Impact: Xen DMA support

Enable swiotlb when running as a Xen dom0 domain.

Signed-off-by: Ian Campbell <[email protected]>
Signed-off-by: Jeremy Fitzhardinge <[email protected]>
---
arch/x86/kernel/pci-swiotlb_64.c | 3 +++
arch/x86/xen/Kconfig | 1 +
drivers/pci/xen-iommu.c | 5 +++++
include/xen/swiotlb.h | 6 ++++++
4 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/pci-swiotlb_64.c b/arch/x86/kernel/pci-swiotlb_64.c
index 7aa8c18..3b9f817 100644
--- a/arch/x86/kernel/pci-swiotlb_64.c
+++ b/arch/x86/kernel/pci-swiotlb_64.c
@@ -103,6 +103,9 @@ void __init pci_swiotlb_init(void)
if (!iommu_detected && !no_iommu && max_pfn > MAX_DMA32_PFN)
swiotlb = 1;
#endif
+ if (xen_wants_swiotlb())
+ swiotlb = 1;
+
if (swiotlb_force)
swiotlb = 1;
if (swiotlb) {
diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig
index 87c13db..2c85967 100644
--- a/arch/x86/xen/Kconfig
+++ b/arch/x86/xen/Kconfig
@@ -6,6 +6,7 @@ config XEN
bool "Xen guest support"
select PARAVIRT
select PARAVIRT_CLOCK
+ select SWIOTLB
depends on X86_64 || (X86_32 && X86_PAE && !X86_VISWS)
depends on X86_CMPXCHG && X86_TSC
help
diff --git a/drivers/pci/xen-iommu.c b/drivers/pci/xen-iommu.c
index fcef078..95d0753 100644
--- a/drivers/pci/xen-iommu.c
+++ b/drivers/pci/xen-iommu.c
@@ -59,6 +59,11 @@ void xen_swiotlb_fixup(void *buf, size_t size, unsigned long nslabs)
panic(KERN_ERR "xen_create_contiguous_region failed\n");
}
}
+int xen_wants_swiotlb(void)
+{
+ return xen_initial_domain();
+}
+
dma_addr_t xen_phys_to_bus(phys_addr_t paddr)
{
return phys_to_machine(XPADDR(paddr)).maddr;
diff --git a/include/xen/swiotlb.h b/include/xen/swiotlb.h
index e975aa0..9134516 100644
--- a/include/xen/swiotlb.h
+++ b/include/xen/swiotlb.h
@@ -6,6 +6,7 @@ extern void xen_swiotlb_fixup(void *buf, size_t size, unsigned long nslabs);
extern phys_addr_t xen_bus_to_phys(dma_addr_t daddr);
extern dma_addr_t xen_phys_to_bus(phys_addr_t paddr);
extern int xen_range_needs_mapping(phys_addr_t phys, size_t size);
+extern int xen_wants_swiotlb(void);
#else
static inline void xen_swiotlb_fixup(void *buf, size_t size, unsigned long nslabs)
{
@@ -25,6 +26,11 @@ static inline int xen_range_needs_mapping(phys_addr_t phys, size_t size)
{
return 0;
}
+
+static inline int xen_wants_swiotlb(void)
+{
+ return 0;
+}
#endif


--
1.6.0.6

2009-03-31 22:54:53

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: [PATCH 7/9] xen/swiotlb: improve comment on gfp flags in xen_alloc_coherent()

From: Jeremy Fitzhardinge <[email protected]>

Impact: cleanup

Clarify why we don't care about the kernel's pseudo-phys restrictions,
so long as the underlying pages are in the right place.

Signed-off-by: Jeremy Fitzhardinge <[email protected]>
---
drivers/pci/xen-iommu.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/xen-iommu.c b/drivers/pci/xen-iommu.c
index 2c90642..080aec1 100644
--- a/drivers/pci/xen-iommu.c
+++ b/drivers/pci/xen-iommu.c
@@ -198,15 +198,17 @@ static void *xen_alloc_coherent(struct device *dev, size_t size,
unsigned long vstart;
u64 mask;

- /* ignore region specifiers */
+ /*
+ * Ignore region specifiers - the kernel's ideas of
+ * pseudo-phys memory layout has nothing to do with the
+ * machine physical layout. We can't allocate highmem
+ * because we can't return a pointer to it.
+ */
gfp &= ~(__GFP_DMA | __GFP_HIGHMEM);

if (dma_alloc_from_coherent(dev, size, dma_handle, &ret))
return ret;

- if (dev == NULL || (dev->coherent_dma_mask < 0xffffffff))
- gfp |= GFP_DMA;
-
vstart = __get_free_pages(gfp, order);
ret = (void *)vstart;

--
1.6.0.6

2009-03-31 22:55:35

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: [PATCH 9/9] pci-swiotlb: remove __weak vs __weak binding

From: Jeremy Fitzhardinge <[email protected]>

swiotlb_arch_range_needs_mapping() is weak in lib/swiotlb.c so that
architectures can override it. The override versions shouldn't be
weak too.

Signed-off-by: Jeremy Fitzhardinge <[email protected]>
Reviewed-by: "H. Peter Anvin" <[email protected]>
---
arch/x86/kernel/pci-swiotlb_64.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/pci-swiotlb_64.c b/arch/x86/kernel/pci-swiotlb_64.c
index 3b9f817..7894315 100644
--- a/arch/x86/kernel/pci-swiotlb_64.c
+++ b/arch/x86/kernel/pci-swiotlb_64.c
@@ -52,7 +52,7 @@ phys_addr_t swiotlb_bus_to_phys(dma_addr_t baddr)
return baddr;
}

-int __weak swiotlb_arch_range_needs_mapping(phys_addr_t paddr, size_t size)
+int swiotlb_arch_range_needs_mapping(phys_addr_t paddr, size_t size)
{
if (xen_pv_domain())
return xen_range_needs_mapping(paddr, size);
--
1.6.0.6

2009-03-31 22:55:58

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: [PATCH 8/9] xen/swiotlb: add sync functions

From: Jeremy Fitzhardinge <[email protected]>

Impact: bugfix

Add all the missing sync functions. This fixes iwlagn.
(Need to think about what to do with non-swiotlb mode.)

Signed-off-by: Jeremy Fitzhardinge <[email protected]>
---
drivers/pci/xen-iommu.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/xen-iommu.c b/drivers/pci/xen-iommu.c
index 080aec1..e85c1cf 100644
--- a/drivers/pci/xen-iommu.c
+++ b/drivers/pci/xen-iommu.c
@@ -311,6 +311,13 @@ static struct dma_mapping_ops xen_swiotlb_dma_ops = {

.mapping_error = swiotlb_dma_mapping_error,

+ .sync_single_for_cpu = swiotlb_sync_single_for_cpu,
+ .sync_single_for_device = swiotlb_sync_single_for_device,
+ .sync_single_range_for_cpu = swiotlb_sync_single_range_for_cpu,
+ .sync_single_range_for_device = swiotlb_sync_single_range_for_device,
+ .sync_sg_for_cpu = swiotlb_sync_sg_for_cpu,
+ .sync_sg_for_device = swiotlb_sync_sg_for_device,
+
.is_phys = 0,
};

--
1.6.0.6

2009-03-31 22:56:25

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: [PATCH 6/9] swiotlb: use swiotlb_alloc_boot to allocate emergency pool

Impact: bugfix

Also fix xen_swiotlb_fixup() to deal with sub-slab-sized allocations.

Signed-off-by: Jeremy Fitzhardinge <[email protected]>
Acked-by: FUJITA Tomonori <[email protected]>
---
drivers/pci/xen-iommu.c | 12 +++++++++---
lib/swiotlb.c | 3 ++-
2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/xen-iommu.c b/drivers/pci/xen-iommu.c
index 95d0753..2c90642 100644
--- a/drivers/pci/xen-iommu.c
+++ b/drivers/pci/xen-iommu.c
@@ -48,16 +48,22 @@ void xen_swiotlb_fixup(void *buf, size_t size, unsigned long nslabs)
buf, size);

dma_bits = get_order(IO_TLB_SEGSIZE << IO_TLB_SHIFT) + PAGE_SHIFT;
- for (i = 0; i < nslabs; i += IO_TLB_SEGSIZE) {
+
+ i = 0;
+ do {
+ int slabs = min(nslabs - i, (unsigned long)IO_TLB_SEGSIZE);
+
do {
rc = xen_create_contiguous_region(
(unsigned long)buf + (i << IO_TLB_SHIFT),
- get_order(IO_TLB_SEGSIZE << IO_TLB_SHIFT),
+ get_order(slabs << IO_TLB_SHIFT),
dma_bits);
} while (rc && dma_bits++ < max_dma_bits);
if (rc)
panic(KERN_ERR "xen_create_contiguous_region failed\n");
- }
+
+ i += slabs;
+ } while(i < nslabs);
}
int xen_wants_swiotlb(void)
{
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 0de836d..e8df499 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -202,7 +202,8 @@ swiotlb_init_with_default_size(size_t default_size)
/*
* Get the overflow emergency buffer
*/
- io_tlb_overflow_buffer = alloc_bootmem_low(io_tlb_overflow);
+ io_tlb_overflow_buffer = swiotlb_alloc_boot(io_tlb_overflow,
+ io_tlb_overflow >> IO_TLB_SHIFT);
if (!io_tlb_overflow_buffer)
panic("Cannot allocate SWIOTLB overflow buffer!\n");

--
1.6.0.6

2009-04-02 06:31:01

by FUJITA Tomonori

[permalink] [raw]
Subject: Re: [PATCH] swiotlb updates for Xen dom0

On Tue, 31 Mar 2009 15:52:06 -0700
Jeremy Fitzhardinge <[email protected]> wrote:

> This series adds Xen support to x86 swiotlb. This is mostly a matter of
> adding some Xen code into the existing hooks in pci-swiotlb_64.c:
> - swiotlb_alloc_boot
> - swiotlb_arch_range_needs_mapping
> - swiotlb_phys_to_bus/bus_to_phys
>
> These hooks are conditional on xen_pv_domain(), which compiles to a
> constant 0 when CONFIG_XEN is not enabled (and is a simple variable
> read when it is).
>
> All the Xen-specific code is in xen-iommu.c.
>
> This series is just the patches which touch lib/swiotlb.c or
> pci-swiotlb_64.c. You can see them with more context in:
> git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git for-linus/xen/dom0/swiotlb
>
> Thanks,
> J
>
> arch/x86/kernel/pci-swiotlb_64.c | 31 ++++++++++++++++-
> arch/x86/xen/Kconfig | 1
> drivers/pci/xen-iommu.c | 68 ++++++++++++++++++++++++++++++++++++---
> include/xen/swiotlb.h | 38 +++++++++++++++++++++
> lib/swiotlb.c | 3 +
> 5 files changed, 133 insertions(+), 8 deletions(-)

This patchset looks ugly.

You add 'if we are Xen, we do A. We do B if not' code into 6 functions
though pic-swiotlb_64.c has only 8 functions. In addition, 5 of 8
functions were added for Xen.

Why can't you have something like arch/x86/xen/pci-swiotlb.c, which
works as arch/x86/kernel/pci-swiotlb_64.c? That should be much
cleaner.

2009-04-02 07:13:20

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: Re: [PATCH] swiotlb updates for Xen dom0

FUJITA Tomonori wrote:
> On Tue, 31 Mar 2009 15:52:06 -0700
> Jeremy Fitzhardinge <[email protected]> wrote:
>
>
>> This series adds Xen support to x86 swiotlb. This is mostly a matter of
>> adding some Xen code into the existing hooks in pci-swiotlb_64.c:
>> - swiotlb_alloc_boot
>> - swiotlb_arch_range_needs_mapping
>> - swiotlb_phys_to_bus/bus_to_phys
>>
>> These hooks are conditional on xen_pv_domain(), which compiles to a
>> constant 0 when CONFIG_XEN is not enabled (and is a simple variable
>> read when it is).
>>
>> All the Xen-specific code is in xen-iommu.c.
>>
>> This series is just the patches which touch lib/swiotlb.c or
>> pci-swiotlb_64.c. You can see them with more context in:
>> git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git for-linus/xen/dom0/swiotlb
>>
>> Thanks,
>> J
>>
>> arch/x86/kernel/pci-swiotlb_64.c | 31 ++++++++++++++++-
>> arch/x86/xen/Kconfig | 1
>> drivers/pci/xen-iommu.c | 68 ++++++++++++++++++++++++++++++++++++---
>> include/xen/swiotlb.h | 38 +++++++++++++++++++++
>> lib/swiotlb.c | 3 +
>> 5 files changed, 133 insertions(+), 8 deletions(-)
>>
>
> This patchset looks ugly.
>
> You add 'if we are Xen, we do A. We do B if not' code into 6 functions
> though pic-swiotlb_64.c has only 8 functions. In addition, 5 of 8
> functions were added for Xen.
>
> Why can't you have something like arch/x86/xen/pci-swiotlb.c, which
> works as arch/x86/kernel/pci-swiotlb_64.c? That should be much
> cleaner.
>

Sure. Something like this? (Applied on top of the rest of the series,
after kernel/pci-swiotlb_64.c => pci_swiotlb.c) It leaves one if(xen)
in the init function, but its consistent with the other clauses setting
swiotlb=1.

My only concern with this scheme is that if there's ever a non-Xen x86
user who wants to override these functions, we'll need to move them back
out into a common file because its not possible to have two overriding
definitions for a weak function (one hopes that the linker will warn if
that arises).

J

Subject: [PATCH] xen/swiotlb: move all Xen-specific swiotlb operations to separate file

Most of these functions are no-ops on non-Xen systems, so we can fall
back to the default functions if CONFIG_XEN is not defined.

Signed-off-by: Jeremy Fitzhardinge <[email protected]>

diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c
index ead556f..2d8dd35 100644
--- a/arch/x86/kernel/pci-swiotlb.c
+++ b/arch/x86/kernel/pci-swiotlb.c
@@ -12,54 +12,9 @@
#include <asm/dma.h>

#include <xen/swiotlb.h>
-#include <asm/xen/hypervisor.h>

int swiotlb __read_mostly;

-void * __init swiotlb_alloc_boot(size_t size, unsigned long nslabs)
-{
- void *ret = alloc_bootmem_low_pages(size);
-
- if (ret && xen_pv_domain())
- xen_swiotlb_fixup(ret, size, nslabs);
-
- return ret;
-}
-
-void *swiotlb_alloc(unsigned order, unsigned long nslabs)
-{
- void *ret = (void *)__get_free_pages(GFP_DMA | __GFP_NOWARN, order);
-
- if (ret && xen_pv_domain())
- xen_swiotlb_fixup(ret, 1u << order, nslabs);
-
- return ret;
-}
-
-dma_addr_t swiotlb_phys_to_bus(struct device *hwdev, phys_addr_t paddr)
-{
- if (xen_pv_domain())
- return xen_phys_to_bus(paddr);
-
- return paddr;
-}
-
-phys_addr_t swiotlb_bus_to_phys(dma_addr_t baddr)
-{
- if (xen_pv_domain())
- return xen_bus_to_phys(baddr);
-
- return baddr;
-}
-
-int swiotlb_arch_range_needs_mapping(phys_addr_t paddr, size_t size)
-{
- if (xen_pv_domain())
- return xen_range_needs_mapping(paddr, size);
-
- return 0;
-}
-
static void *x86_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
dma_addr_t *dma_handle, gfp_t flags)
{
diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile
index 639965a..b021e72 100644
--- a/arch/x86/xen/Makefile
+++ b/arch/x86/xen/Makefile
@@ -12,4 +12,4 @@ obj-y := enlighten.o setup.o multicalls.o mmu.o irq.o \
obj-$(CONFIG_SMP) += smp.o spinlock.o
obj-$(CONFIG_XEN_DEBUG_FS) += debugfs.o
obj-$(CONFIG_XEN_DOM0) += vga.o apic.o
-obj-$(CONFIG_XEN_DOM0_PCI) += pci.o
\ No newline at end of file
+obj-$(CONFIG_XEN_DOM0_PCI) += pci.o pci-swiotlb.o
\ No newline at end of file
diff --git a/arch/x86/xen/pci-swiotlb.c b/arch/x86/xen/pci-swiotlb.c
new file mode 100644
index 0000000..82b2291
--- /dev/null
+++ b/arch/x86/xen/pci-swiotlb.c
@@ -0,0 +1,51 @@
+#include <linux/bootmem.h>
+#include <linux/gfp.h>
+
+#include <xen/swiotlb.h>
+#include <asm/xen/hypervisor.h>
+
+/*
+ * This file defines overrides for weak functions with default
+ * implementations in lib/swiotlb.c.
+ */
+
+void * __init swiotlb_alloc_boot(size_t size, unsigned long nslabs)
+{
+ void *ret = alloc_bootmem_low_pages(size);
+
+ if (ret && xen_pv_domain())
+ xen_swiotlb_fixup(ret, size, nslabs);
+
+ return ret;
+}
+
+void *swiotlb_alloc(unsigned order, unsigned long nslabs)
+{
+ /* Never called on x86. Warn, just in case. */
+ WARN_ON(1);
+ return NULL;
+}
+
+dma_addr_t swiotlb_phys_to_bus(struct device *hwdev, phys_addr_t paddr)
+{
+ if (xen_pv_domain())
+ return xen_phys_to_bus(paddr);
+
+ return paddr;
+}
+
+phys_addr_t swiotlb_bus_to_phys(dma_addr_t baddr)
+{
+ if (xen_pv_domain())
+ return xen_bus_to_phys(baddr);
+
+ return baddr;
+}
+
+int swiotlb_arch_range_needs_mapping(phys_addr_t paddr, size_t size)
+{
+ if (xen_pv_domain())
+ return xen_range_needs_mapping(paddr, size);
+
+ return 0;
+}