[PATCH kernel RFC 0/2] powerpc/ioda2: An attempt to allow DMA masks between 32 and 59

[PATCH kernel RFC 0/2] powerpc/ioda2: An attempt to allow DMA masks between 32 and 59

[Alexey Kardashevskiy]

This is an attempt to allow DMA mask 40 or similar which are not large
enough to use either a PHB3 bypass mode or a sketchy bypass.

This is based on sha1
582549e3fbe1 Linus Torvalds Merge tag 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma 

Please comment. Thanks.



Alexey Kardashevskiy (2):
  powerpc/powernv/ioda: Allocate TCE table levels on demand for default
    DMA window
  powerpc/powernv/ioda2: Create bigger default window with 64k IOMMU
    pages

 arch/powerpc/include/asm/iommu.h              |  8 ++-
 arch/powerpc/platforms/powernv/pci.h          |  2 +-
 arch/powerpc/kernel/iommu.c                   | 58 +++++++++++++------
 arch/powerpc/platforms/powernv/pci-ioda-tce.c | 19 +++---
 arch/powerpc/platforms/powernv/pci-ioda.c     | 14 ++++-
 5 files changed, 66 insertions(+), 35 deletions(-)

-- 
2.17.1

[PATCH kernel RFC 1/2] powerpc/powernv/ioda: Allocate TCE table levels on demand for default DMA window

[Alexey Kardashevskiy]

We allocate only the first level of multilevel TCE tables for KVM
already (alloc_userspace_copy==true), and the rest is allocated on demand.
This is not enabled though for baremetal.

This removes the KVM limitation (implicit, via the alloc_userspace_copy
parameter) and always allocates just the first level. The on-demand
allocation of missing levels is already implemented.

As from now on DMA map might happen with disabled interrupts, this
allocates TCEs with GFP_ATOMIC.

To save time when creating a new clean table, this skips non-allocated
indirect TCE entries in pnv_tce_free just like we already do in
the VFIO IOMMU TCE driver.

This changes the default level number from 1 to 2 to reduce the amount
of memory required for the default 32bit DMA window at the boot time.
The default window size is up to 2GB which requires 4MB of TCEs which is
unlikely to be used entirely or at all as most devices these days are
64bit capable so by switching to 2 levels by default we save 4032KB of
RAM per a device.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 arch/powerpc/platforms/powernv/pci.h          |  2 +-
 arch/powerpc/platforms/powernv/pci-ioda-tce.c | 19 +++++++++----------
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
index 07464d3bad4a..1b8aba83d1b2 100644
--- a/arch/powerpc/platforms/powernv/pci.h
+++ b/arch/powerpc/platforms/powernv/pci.h
@@ -224,7 +224,7 @@ extern struct iommu_table_group *pnv_npu_compound_attach(
 		struct pnv_ioda_pe *pe);
 
 /* pci-ioda-tce.c */
-#define POWERNV_IOMMU_DEFAULT_LEVELS	1
+#define POWERNV_IOMMU_DEFAULT_LEVELS	2
 #define POWERNV_IOMMU_MAX_LEVELS	5
 
 extern int pnv_tce_build(struct iommu_table *tbl, long index, long npages,
diff --git a/arch/powerpc/platforms/powernv/pci-ioda-tce.c b/arch/powerpc/platforms/powernv/pci-ioda-tce.c
index 7fc3d951f421..2e827546b983 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda-tce.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda-tce.c
@@ -36,7 +36,7 @@ static __be64 *pnv_alloc_tce_level(int nid, unsigned int shift)
 	struct page *tce_mem = NULL;
 	__be64 *addr;
 
-	tce_mem = alloc_pages_node(nid, GFP_KERNEL | __GFP_NOWARN,
+	tce_mem = alloc_pages_node(nid, GFP_ATOMIC | __GFP_NOWARN,
 			shift - PAGE_SHIFT);
 	if (!tce_mem) {
 		pr_err("Failed to allocate a TCE memory, level shift=%d\n",
@@ -162,6 +162,9 @@ void pnv_tce_free(struct iommu_table *tbl, long index, long npages)
 
 		if (ptce)
 			*ptce = cpu_to_be64(0);
+		else
+			/* align to level_size which is power of two */
+			i |= tbl->it_level_size - 1;
 	}
 }
 
@@ -261,7 +264,6 @@ long pnv_pci_ioda2_table_alloc_pages(int nid, __u64 bus_offset,
 	unsigned int table_shift = max_t(unsigned int, entries_shift + 3,
 			PAGE_SHIFT);
 	const unsigned long tce_table_size = 1UL << table_shift;
-	unsigned int tmplevels = levels;
 
 	if (!levels || (levels > POWERNV_IOMMU_MAX_LEVELS))
 		return -EINVAL;
@@ -269,9 +271,6 @@ long pnv_pci_ioda2_table_alloc_pages(int nid, __u64 bus_offset,
 	if (!is_power_of_2(window_size))
 		return -EINVAL;
 
-	if (alloc_userspace_copy && (window_size > (1ULL << 32)))
-		tmplevels = 1;
-
 	/* Adjust direct table size from window_size and levels */
 	entries_shift = (entries_shift + levels - 1) / levels;
 	level_shift = entries_shift + 3;
@@ -282,7 +281,7 @@ long pnv_pci_ioda2_table_alloc_pages(int nid, __u64 bus_offset,
 
 	/* Allocate TCE table */
 	addr = pnv_pci_ioda2_table_do_alloc_pages(nid, level_shift,
-			tmplevels, tce_table_size, &offset, &total_allocated);
+			1, tce_table_size, &offset, &total_allocated);
 
 	/* addr==NULL means that the first level allocation failed */
 	if (!addr)
@@ -293,18 +292,18 @@ long pnv_pci_ioda2_table_alloc_pages(int nid, __u64 bus_offset,
 	 * we did not allocate as much as we wanted,
 	 * release partially allocated table.
 	 */
-	if (tmplevels == levels && offset < tce_table_size)
+	if (levels == 1 && offset < tce_table_size)
 		goto free_tces_exit;
 
 	/* Allocate userspace view of the TCE table */
 	if (alloc_userspace_copy) {
 		offset = 0;
 		uas = pnv_pci_ioda2_table_do_alloc_pages(nid, level_shift,
-				tmplevels, tce_table_size, &offset,
+				1, tce_table_size, &offset,
 				&total_allocated_uas);
 		if (!uas)
 			goto free_tces_exit;
-		if (tmplevels == levels && (offset < tce_table_size ||
+		if (levels == 1 && (offset < tce_table_size ||
 				total_allocated_uas != total_allocated))
 			goto free_uas_exit;
 	}
@@ -319,7 +318,7 @@ long pnv_pci_ioda2_table_alloc_pages(int nid, __u64 bus_offset,
 
 	pr_debug("Created TCE table: ws=%08llx ts=%lx @%08llx base=%lx uas=%p levels=%d/%d\n",
 			window_size, tce_table_size, bus_offset, tbl->it_base,
-			tbl->it_userspace, tmplevels, levels);
+			tbl->it_userspace, 1, levels);
 
 	return 0;
 
-- 
2.17.1

[PATCH kernel RFC 2/2] powerpc/powernv/ioda2: Create bigger default window with 64k IOMMU pages

[Alexey Kardashevskiy]

At the moment we create a small window only for 32bit devices, the window
maps 0..2GB of the PCI space only. For other devices we either use
a sketchy bypass or hardware bypass but the former can only work if
the amount of RAM is no bigger than the device's DMA mask and the latter
requires devices to support at least 59bit DMA.

This extends the default DMA window to the maximum size possible to allow
more flexibility. The default window size has a limitation as
the allocation bitmap in iommu_table is a contiguous array with a bit per
an IOMMU page. Also, this increases the default IOMMU page size to
the system page size to allow higher supported DMA masks.

As the extended window now overlaps the 32bit MMIO region, this adds
reservation capability to iommu_init_table().

After this change the default window size is 0x80000000000==1<<43 so
devices limited to DMA mask smaller than the amount of system RAM can
still use more than just 2GB of memory for DMA.

With the on-demand allocation of indirect TCE table levels enabled and
2 levels, the first TCE level size is just
1<<ceil((log2(0x7ffffffffff+1)-16)/2)=16384 TCEs or 2 system pages.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 arch/powerpc/include/asm/iommu.h          |  8 +++-
 arch/powerpc/kernel/iommu.c               | 58 +++++++++++++++--------
 arch/powerpc/platforms/powernv/pci-ioda.c | 14 ++++--
 3 files changed, 56 insertions(+), 24 deletions(-)

diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
index 0ac52392ed99..5ea782e04803 100644
--- a/arch/powerpc/include/asm/iommu.h
+++ b/arch/powerpc/include/asm/iommu.h
@@ -124,6 +124,8 @@ struct iommu_table {
 	struct iommu_table_ops *it_ops;
 	struct kref    it_kref;
 	int it_nid;
+	unsigned long it_reserved_start; /* Start of not-DMA-able (MMIO) area */
+	unsigned long it_reserved_end;
 };
 
 #define IOMMU_TABLE_USERSPACE_ENTRY_RO(tbl, entry) \
@@ -162,8 +164,10 @@ extern int iommu_tce_table_put(struct iommu_table *tbl);
 /* Initializes an iommu_table based in values set in the passed-in
  * structure
  */
-extern struct iommu_table *iommu_init_table(struct iommu_table * tbl,
-					    int nid);
+extern struct iommu_table *iommu_init_table_res(struct iommu_table *tbl,
+		int nid, unsigned long res_start, unsigned long res_end);
+#define iommu_init_table(tbl, nid) iommu_init_table_res((tbl), (nid), 0, 0)
+
 #define IOMMU_TABLE_GROUP_MAX_TABLES	2
 
 struct iommu_table_group;
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index 33bbd59cff79..209306ce7f4b 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -646,11 +646,43 @@ static void iommu_table_clear(struct iommu_table *tbl)
 #endif
 }
 
+static void iommu_table_reserve_pages(struct iommu_table *tbl)
+{
+	int i;
+
+	/*
+	 * Reserve page 0 so it will not be used for any mappings.
+	 * This avoids buggy drivers that consider page 0 to be invalid
+	 * to crash the machine or even lose data.
+	 */
+	if (tbl->it_offset == 0)
+		set_bit(0, tbl->it_map);
+
+	for (i = tbl->it_reserved_start; i < tbl->it_reserved_end; ++i)
+		set_bit(i, tbl->it_map);
+}
+
+static void iommu_table_release_pages(struct iommu_table *tbl)
+{
+	int i;
+
+	/*
+	 * In case we have reserved the first bit, we should not emit
+	 * the warning below.
+	 */
+	if (tbl->it_offset == 0)
+		clear_bit(0, tbl->it_map);
+
+	for (i = tbl->it_reserved_start; i < tbl->it_reserved_end; ++i)
+		clear_bit(i, tbl->it_map);
+}
+
 /*
  * Build a iommu_table structure.  This contains a bit map which
  * is used to manage allocation of the tce space.
  */
-struct iommu_table *iommu_init_table(struct iommu_table *tbl, int nid)
+struct iommu_table *iommu_init_table_res(struct iommu_table *tbl, int nid,
+		unsigned long res_start, unsigned long res_end)
 {
 	unsigned long sz;
 	static int welcomed = 0;
@@ -669,13 +701,9 @@ struct iommu_table *iommu_init_table(struct iommu_table *tbl, int nid)
 	tbl->it_map = page_address(page);
 	memset(tbl->it_map, 0, sz);
 
-	/*
-	 * Reserve page 0 so it will not be used for any mappings.
-	 * This avoids buggy drivers that consider page 0 to be invalid
-	 * to crash the machine or even lose data.
-	 */
-	if (tbl->it_offset == 0)
-		set_bit(0, tbl->it_map);
+	tbl->it_reserved_start = res_start;
+	tbl->it_reserved_end = res_end;
+	iommu_table_reserve_pages(tbl);
 
 	/* We only split the IOMMU table if we have 1GB or more of space */
 	if ((tbl->it_size << tbl->it_page_shift) >= (1UL * 1024 * 1024 * 1024))
@@ -727,12 +755,7 @@ static void iommu_table_free(struct kref *kref)
 		return;
 	}
 
-	/*
-	 * In case we have reserved the first bit, we should not emit
-	 * the warning below.
-	 */
-	if (tbl->it_offset == 0)
-		clear_bit(0, tbl->it_map);
+	iommu_table_release_pages(tbl);
 
 	/* verify that table contains no entries */
 	if (!bitmap_empty(tbl->it_map, tbl->it_size))
@@ -1037,8 +1060,7 @@ int iommu_take_ownership(struct iommu_table *tbl)
 	for (i = 0; i < tbl->nr_pools; i++)
 		spin_lock(&tbl->pools[i].lock);
 
-	if (tbl->it_offset == 0)
-		clear_bit(0, tbl->it_map);
+	iommu_table_reserve_pages(tbl);
 
 	if (!bitmap_empty(tbl->it_map, tbl->it_size)) {
 		pr_err("iommu_tce: it_map is not empty");
@@ -1068,9 +1090,7 @@ void iommu_release_ownership(struct iommu_table *tbl)
 
 	memset(tbl->it_map, 0, sz);
 
-	/* Restore bit#0 set by iommu_init_table() */
-	if (tbl->it_offset == 0)
-		set_bit(0, tbl->it_map);
+	iommu_table_release_pages(tbl);
 
 	for (i = 0; i < tbl->nr_pools; i++)
 		spin_unlock(&tbl->pools[i].lock);
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index dcfb5469678e..3eb755cf13d7 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -2434,10 +2434,13 @@ static long pnv_pci_ioda2_setup_default_config(struct pnv_ioda_pe *pe)
 	 * DMA window can be larger than available memory, which will
 	 * cause errors later.
 	 */
-	const u64 window_size = min((u64)pe->table_group.tce32_size, max_memory);
+	const u64 maxblock = 1UL << (PAGE_SHIFT + MAX_ORDER - 1);
+	/* iommu_table::it_map uses 1 bit per IOMMU page, hence 8 */
+	const u64 window_size = min((maxblock * 8) << PAGE_SHIFT, max_memory);
+	unsigned long res_start = 0, res_end = 0;
 
 	rc = pnv_pci_ioda2_create_table(&pe->table_group, 0,
-			IOMMU_PAGE_SHIFT_4K,
+			PAGE_SHIFT,
 			window_size,
 			POWERNV_IOMMU_DEFAULT_LEVELS, false, &tbl);
 	if (rc) {
@@ -2446,7 +2449,12 @@ static long pnv_pci_ioda2_setup_default_config(struct pnv_ioda_pe *pe)
 		return rc;
 	}
 
-	iommu_init_table(tbl, pe->phb->hose->node);
+	/* We use top part of 32bit space for MMIO so exclude it from DMA */
+	if (window_size > pe->phb->ioda.m32_pci_base) {
+		res_start = pe->phb->ioda.m32_pci_base >> tbl->it_page_shift;
+		res_end = min(window_size, SZ_4G) >> tbl->it_page_shift;
+	}
+	iommu_init_table_res(tbl, pe->phb->hose->node, res_start, res_end);
 
 	rc = pnv_pci_ioda2_set_window(&pe->table_group, 0, tbl);
 	if (rc) {
-- 
2.17.1

Re: [PATCH kernel RFC 0/2] powerpc/ioda2: An attempt to allow DMA masks between 32 and 59

[Russell Currey]

I'm gonna try and benchmark this on a few different devices for performance, with 64k TCEs (as is), with larger TCE sizes, and against sketchy bypass.  Hopefully if performance isn't too far off, we can get rid of sketchy bypass entirely and have a more robust solution.

-- 
  Russell Currey
  ruscur@russell.cc

On Fri, Apr 12, 2019, at 4:44 PM, Alexey Kardashevskiy wrote:
> 
> This is an attempt to allow DMA mask 40 or similar which are not large
> enough to use either a PHB3 bypass mode or a sketchy bypass.
> 
> This is based on sha1
> 582549e3fbe1 Linus Torvalds Merge tag 'for-linus' of 
> git://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma 
> 
> Please comment. Thanks.
> 
> 
> 
> Alexey Kardashevskiy (2):
>   powerpc/powernv/ioda: Allocate TCE table levels on demand for default
>     DMA window
>   powerpc/powernv/ioda2: Create bigger default window with 64k IOMMU
>     pages
> 
>  arch/powerpc/include/asm/iommu.h              |  8 ++-
>  arch/powerpc/platforms/powernv/pci.h          |  2 +-
>  arch/powerpc/kernel/iommu.c                   | 58 +++++++++++++------
>  arch/powerpc/platforms/powernv/pci-ioda-tce.c | 19 +++---
>  arch/powerpc/platforms/powernv/pci-ioda.c     | 14 ++++-
>  5 files changed, 66 insertions(+), 35 deletions(-)
> 
> -- 
> 2.17.1
> 
>

Re: [PATCH kernel RFC 0/2] powerpc/ioda2: An attempt to allow DMA masks between 32 and 59

[Russell Currey]

On Fri, 2019-04-12 at 16:44 +1000, Alexey Kardashevskiy wrote:
> This is an attempt to allow DMA mask 40 or similar which are not
> large
> enough to use either a PHB3 bypass mode or a sketchy bypass.
> 
> This is based on sha1
> 582549e3fbe1 Linus Torvalds Merge tag 'for-linus' of
> git://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma 
> 
> Please comment. Thanks.

Seems to uniformly fail on POWER8 and sometimes on P9 too.

On P8:

[    2.423206] Failed to allocate a TCE memory, level shift=26
[    2.423351] pci 0001:03     : [PE# fd] Failed to create 32-bit TCE
table, err -12

On P9:

[    0.303055] pci 0003:01     : [PE# 1fd] Setting up 32-bit TCE table
at 0..80000000
[    0.303119] Failed to allocate a TCE memory, level shift=30
[    0.303147] pci 0003:01     : [PE# 1fd] Failed to create 32-bit TCE
table, err -12

Is it selecting the wrong TCE size?

> Alexey Kardashevskiy (2):
  powerpc/powernv/ioda: Allocate TCE table
> levels on demand for default
    DMA window
  powerpc/powernv/ioda2:
> Create bigger default window with 64k IOMMU
    pages

 arch/powerpc/include/asm/iommu.h              |  8 ++-
 arch/powerpc/platforms/powernv/pci.h          |  2 +-
 arch/powerpc/kernel/iommu.c                   | 58 +++++++++++++--
----
 arch/powerpc/platforms/powernv/pci-ioda-tce.c | 19 +++---
 arch/powerpc/platforms/powernv/pci-ioda.c     | 14 ++++-
 5 files changed, 66 insertions(+), 35 deletions(-)

Re: [PATCH kernel RFC 0/2] powerpc/ioda2: An attempt to allow DMA masks between 32 and 59

[Alexey Kardashevskiy]

On 23/04/2019 10:58, Russell Currey wrote:
> On Fri, 2019-04-12 at 16:44 +1000, Alexey Kardashevskiy wrote:
>> This is an attempt to allow DMA mask 40 or similar which are not
>> large
>> enough to use either a PHB3 bypass mode or a sketchy bypass.
>>
>> This is based on sha1
>> 582549e3fbe1 Linus Torvalds Merge tag 'for-linus' of
>> git://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma 
>>
>> Please comment. Thanks.
> 
> Seems to uniformly fail on POWER8 and sometimes on P9 too.
> 
> On P8:
> 
> [    2.423206] Failed to allocate a TCE memory, level shift=26
> [    2.423351] pci 0001:03     : [PE# fd] Failed to create 32-bit TCE
> table, err -12
> 
> On P9:
> 
> [    0.303055] pci 0003:01     : [PE# 1fd] Setting up 32-bit TCE table
> at 0..80000000
> [    0.303119] Failed to allocate a TCE memory, level shift=30
> [    0.303147] pci 0003:01     : [PE# 1fd] Failed to create 32-bit TCE
> table, err -12
> 
> Is it selecting the wrong TCE size?

Nah, it is too big level size, i.e. it is bigger than max allowed order.

Try:

#define POWERNV_IOMMU_DEFAULT_LEVELS 3



>> Alexey Kardashevskiy (2):
>   powerpc/powernv/ioda: Allocate TCE table
>> levels on demand for default
>     DMA window
>   powerpc/powernv/ioda2:
>> Create bigger default window with 64k IOMMU
>     pages
> 
>  arch/powerpc/include/asm/iommu.h              |  8 ++-
>  arch/powerpc/platforms/powernv/pci.h          |  2 +-
>  arch/powerpc/kernel/iommu.c                   | 58 +++++++++++++--
> ----
>  arch/powerpc/platforms/powernv/pci-ioda-tce.c | 19 +++---
>  arch/powerpc/platforms/powernv/pci-ioda.c     | 14 ++++-
>  5 files changed, 66 insertions(+), 35 deletions(-)
> 
> 

-- 
Alexey