All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH V2 0/2] swiotlb: Add child io tlb mem support
@ 2022-05-02 12:54 ` Tianyu Lan
  0 siblings, 0 replies; 20+ messages in thread
From: Tianyu Lan @ 2022-05-02 12:54 UTC (permalink / raw)
  To: hch, m.szyprowski, robin.murphy, michael.h.kelley, kys
  Cc: Tianyu Lan, iommu, linux-kernel, vkuznets, brijesh.singh,
	konrad.wilk, hch, wei.liu, parri.andrea, thomas.lendacky,
	linux-hyperv, andi.kleen, kirill.shutemov

From: Tianyu Lan <Tianyu.Lan@microsoft.com>

Traditionally swiotlb was not performance critical because it was only
used for slow devices. But in some setups, like TDX/SEV confidential
guests, all IO has to go through swiotlb. Currently swiotlb only has a
single lock. Under high IO load with multiple CPUs this can lead to
significant lock contention on the swiotlb lock.

This patch adds child IO TLB mem support to resolve spinlock overhead
among device's queues. Each device may allocate IO tlb mem and setup
child IO TLB mem according to queue number. The number child IO tlb
mem maybe set up equal with device queue number and this helps to resolve
swiotlb spinlock overhead among devices and queues.

Patch 2 introduces IO TLB Block concepts and swiotlb_device_allocate()
API to allocate per-device swiotlb bounce buffer. The new API Accepts
queue number as the number of child IO TLB mem to set up device's IO
TLB mem.

Tianyu Lan (2):
  swiotlb: Add Child IO TLB mem support
  Swiotlb: Add device bounce buffer allocation interface

 include/linux/swiotlb.h |  40 ++++++
 kernel/dma/swiotlb.c    | 290 ++++++++++++++++++++++++++++++++++++++--
 2 files changed, 317 insertions(+), 13 deletions(-)

-- 
2.25.1


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

* [RFC PATCH V2 0/2] swiotlb: Add child io tlb mem support
@ 2022-05-02 12:54 ` Tianyu Lan
  0 siblings, 0 replies; 20+ messages in thread
From: Tianyu Lan @ 2022-05-02 12:54 UTC (permalink / raw)
  To: hch, m.szyprowski, robin.murphy, michael.h.kelley, kys
  Cc: parri.andrea, thomas.lendacky, wei.liu, Tianyu Lan, linux-hyperv,
	konrad.wilk, linux-kernel, kirill.shutemov, iommu, andi.kleen,
	brijesh.singh, vkuznets, hch

From: Tianyu Lan <Tianyu.Lan@microsoft.com>

Traditionally swiotlb was not performance critical because it was only
used for slow devices. But in some setups, like TDX/SEV confidential
guests, all IO has to go through swiotlb. Currently swiotlb only has a
single lock. Under high IO load with multiple CPUs this can lead to
significant lock contention on the swiotlb lock.

This patch adds child IO TLB mem support to resolve spinlock overhead
among device's queues. Each device may allocate IO tlb mem and setup
child IO TLB mem according to queue number. The number child IO tlb
mem maybe set up equal with device queue number and this helps to resolve
swiotlb spinlock overhead among devices and queues.

Patch 2 introduces IO TLB Block concepts and swiotlb_device_allocate()
API to allocate per-device swiotlb bounce buffer. The new API Accepts
queue number as the number of child IO TLB mem to set up device's IO
TLB mem.

Tianyu Lan (2):
  swiotlb: Add Child IO TLB mem support
  Swiotlb: Add device bounce buffer allocation interface

 include/linux/swiotlb.h |  40 ++++++
 kernel/dma/swiotlb.c    | 290 ++++++++++++++++++++++++++++++++++++++--
 2 files changed, 317 insertions(+), 13 deletions(-)

-- 
2.25.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* [RFC PATCH V2 1/2] swiotlb: Add Child IO TLB mem support
  2022-05-02 12:54 ` Tianyu Lan
@ 2022-05-02 12:54   ` Tianyu Lan
  -1 siblings, 0 replies; 20+ messages in thread
From: Tianyu Lan @ 2022-05-02 12:54 UTC (permalink / raw)
  To: hch, m.szyprowski, robin.murphy, michael.h.kelley, kys
  Cc: Tianyu Lan, iommu, linux-kernel, vkuznets, brijesh.singh,
	konrad.wilk, hch, wei.liu, parri.andrea, thomas.lendacky,
	linux-hyperv, andi.kleen, kirill.shutemov

From: Tianyu Lan <Tianyu.Lan@microsoft.com>

Traditionally swiotlb was not performance critical because it was only
used for slow devices. But in some setups, like TDX/SEV confidential
guests, all IO has to go through swiotlb. Currently swiotlb only has a
single lock. Under high IO load with multiple CPUs this can lead to
significant lock contention on the swiotlb lock.

This patch adds child IO TLB mem support to resolve spinlock overhead
among device's queues. Each device may allocate IO tlb mem and setup
child IO TLB mem according to queue number. Swiotlb code allocates
bounce buffer among child IO tlb mem iterately.

Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
 include/linux/swiotlb.h |  7 +++
 kernel/dma/swiotlb.c    | 97 ++++++++++++++++++++++++++++++++++++-----
 2 files changed, 94 insertions(+), 10 deletions(-)

diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 7ed35dd3de6e..4a3f6a7b4b7e 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -89,6 +89,9 @@ extern enum swiotlb_force swiotlb_force;
  * @late_alloc:	%true if allocated using the page allocator
  * @force_bounce: %true if swiotlb bouncing is forced
  * @for_alloc:  %true if the pool is used for memory allocation
+ * @child_nslot:The number of IO TLB slot in the child IO TLB mem.
+ * @num_child:  The child io tlb mem number in the pool.
+ * @child_start:The child index to start searching in the next round.
  */
 struct io_tlb_mem {
 	phys_addr_t start;
@@ -102,6 +105,10 @@ struct io_tlb_mem {
 	bool late_alloc;
 	bool force_bounce;
 	bool for_alloc;
+	unsigned int num_child;
+	unsigned int child_nslot;
+	unsigned int child_start;
+	struct io_tlb_mem *child;
 	struct io_tlb_slot {
 		phys_addr_t orig_addr;
 		size_t alloc_size;
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index e2ef0864eb1e..32e8f42530b6 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -207,6 +207,26 @@ static void swiotlb_init_io_tlb_mem(struct io_tlb_mem *mem, phys_addr_t start,
 		mem->force_bounce = true;
 
 	spin_lock_init(&mem->lock);
+
+	if (mem->num_child) {
+		mem->child_nslot = nslabs / mem->num_child;
+		mem->child_start = 0;
+
+		/*
+		 * Initialize child IO TLB mem, divide IO TLB pool
+		 * into child number. Reuse parent mem->slot in the
+		 * child mem->slot.
+		 */
+		for (i = 0; i < mem->num_child; i++) {
+			mem->child[i].slots = mem->slots + i * mem->child_nslot;
+			mem->child[i].num_child = 0;
+
+			swiotlb_init_io_tlb_mem(&mem->child[i],
+				start + ((i * mem->child_nslot) << IO_TLB_SHIFT),
+				mem->child_nslot, late_alloc);
+		}
+	}
+
 	for (i = 0; i < mem->nslabs; i++) {
 		mem->slots[i].list = IO_TLB_SEGSIZE - io_tlb_offset(i);
 		mem->slots[i].orig_addr = INVALID_PHYS_ADDR;
@@ -336,16 +356,18 @@ int swiotlb_init_late(size_t size, gfp_t gfp_mask,
 
 	mem->slots = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
 		get_order(array_size(sizeof(*mem->slots), nslabs)));
-	if (!mem->slots) {
-		free_pages((unsigned long)vstart, order);
-		return -ENOMEM;
-	}
+	if (!mem->slots)
+		goto error_slots;
 
 	set_memory_decrypted((unsigned long)vstart, bytes >> PAGE_SHIFT);
 	swiotlb_init_io_tlb_mem(mem, virt_to_phys(vstart), nslabs, true);
 
 	swiotlb_print_info();
 	return 0;
+
+error_slots:
+	free_pages((unsigned long)vstart, order);
+	return -ENOMEM;
 }
 
 void __init swiotlb_exit(void)
@@ -483,10 +505,11 @@ static unsigned int wrap_index(struct io_tlb_mem *mem, unsigned int index)
  * Find a suitable number of IO TLB entries size that will fit this request and
  * allocate a buffer from that IO TLB pool.
  */
-static int swiotlb_find_slots(struct device *dev, phys_addr_t orig_addr,
-			      size_t alloc_size, unsigned int alloc_align_mask)
+static int swiotlb_do_find_slots(struct io_tlb_mem *mem,
+				 struct device *dev, phys_addr_t orig_addr,
+				 size_t alloc_size,
+				 unsigned int alloc_align_mask)
 {
-	struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
 	unsigned long boundary_mask = dma_get_seg_boundary(dev);
 	dma_addr_t tbl_dma_addr =
 		phys_to_dma_unencrypted(dev, mem->start) & boundary_mask;
@@ -565,6 +588,46 @@ static int swiotlb_find_slots(struct device *dev, phys_addr_t orig_addr,
 	return index;
 }
 
+static int swiotlb_find_slots(struct device *dev, phys_addr_t orig_addr,
+			      size_t alloc_size, unsigned int alloc_align_mask)
+{
+	struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
+	struct io_tlb_mem *child_mem = mem;
+	int start = 0, i = 0, index;
+
+	if (mem->num_child) {
+		i = start = mem->child_start;
+		mem->child_start = (mem->child_start + 1) % mem->num_child;
+		child_mem = mem->child;
+	}
+
+	do {
+		index = swiotlb_do_find_slots(child_mem + i, dev, orig_addr,
+					      alloc_size, alloc_align_mask);
+		if (index >= 0)
+			return i * mem->child_nslot + index;
+		if (++i >= mem->num_child)
+			i = 0;
+	} while (i != start);
+
+	return -1;
+}
+
+static unsigned long mem_used(struct io_tlb_mem *mem)
+{
+	int i;
+	unsigned long used = 0;
+
+	if (mem->num_child) {
+		for (i = 0; i < mem->num_child; i++)
+			used += mem->child[i].used;
+	} else {
+		used = mem->used;
+	}
+
+	return used;
+}
+
 phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
 		size_t mapping_size, size_t alloc_size,
 		unsigned int alloc_align_mask, enum dma_data_direction dir,
@@ -594,7 +657,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
 		if (!(attrs & DMA_ATTR_NO_WARN))
 			dev_warn_ratelimited(dev,
 	"swiotlb buffer is full (sz: %zd bytes), total %lu (slots), used %lu (slots)\n",
-				 alloc_size, mem->nslabs, mem->used);
+				     alloc_size, mem->nslabs, mem_used(mem));
 		return (phys_addr_t)DMA_MAPPING_ERROR;
 	}
 
@@ -617,9 +680,9 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
 	return tlb_addr;
 }
 
-static void swiotlb_release_slots(struct device *dev, phys_addr_t tlb_addr)
+static void swiotlb_do_release_slots(struct io_tlb_mem *mem,
+				     struct device *dev, phys_addr_t tlb_addr)
 {
-	struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
 	unsigned long flags;
 	unsigned int offset = swiotlb_align_offset(dev, tlb_addr);
 	int index = (tlb_addr - offset - mem->start) >> IO_TLB_SHIFT;
@@ -660,6 +723,20 @@ static void swiotlb_release_slots(struct device *dev, phys_addr_t tlb_addr)
 	spin_unlock_irqrestore(&mem->lock, flags);
 }
 
+static void swiotlb_release_slots(struct device *dev, phys_addr_t tlb_addr)
+{
+	struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
+	int index, offset;
+
+	if (mem->num_child) {
+		offset = swiotlb_align_offset(dev, tlb_addr);	
+		index = (tlb_addr - offset - mem->start) >> IO_TLB_SHIFT;
+		mem = &mem->child[index / mem->child_nslot];
+	}
+
+	swiotlb_do_release_slots(mem, dev, tlb_addr);
+}
+
 /*
  * tlb_addr is the physical address of the bounce buffer to unmap.
  */
-- 
2.25.1


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

* [RFC PATCH V2 1/2] swiotlb: Add Child IO TLB mem support
@ 2022-05-02 12:54   ` Tianyu Lan
  0 siblings, 0 replies; 20+ messages in thread
From: Tianyu Lan @ 2022-05-02 12:54 UTC (permalink / raw)
  To: hch, m.szyprowski, robin.murphy, michael.h.kelley, kys
  Cc: parri.andrea, thomas.lendacky, wei.liu, Tianyu Lan, linux-hyperv,
	konrad.wilk, linux-kernel, kirill.shutemov, iommu, andi.kleen,
	brijesh.singh, vkuznets, hch

From: Tianyu Lan <Tianyu.Lan@microsoft.com>

Traditionally swiotlb was not performance critical because it was only
used for slow devices. But in some setups, like TDX/SEV confidential
guests, all IO has to go through swiotlb. Currently swiotlb only has a
single lock. Under high IO load with multiple CPUs this can lead to
significant lock contention on the swiotlb lock.

This patch adds child IO TLB mem support to resolve spinlock overhead
among device's queues. Each device may allocate IO tlb mem and setup
child IO TLB mem according to queue number. Swiotlb code allocates
bounce buffer among child IO tlb mem iterately.

Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
 include/linux/swiotlb.h |  7 +++
 kernel/dma/swiotlb.c    | 97 ++++++++++++++++++++++++++++++++++++-----
 2 files changed, 94 insertions(+), 10 deletions(-)

diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 7ed35dd3de6e..4a3f6a7b4b7e 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -89,6 +89,9 @@ extern enum swiotlb_force swiotlb_force;
  * @late_alloc:	%true if allocated using the page allocator
  * @force_bounce: %true if swiotlb bouncing is forced
  * @for_alloc:  %true if the pool is used for memory allocation
+ * @child_nslot:The number of IO TLB slot in the child IO TLB mem.
+ * @num_child:  The child io tlb mem number in the pool.
+ * @child_start:The child index to start searching in the next round.
  */
 struct io_tlb_mem {
 	phys_addr_t start;
@@ -102,6 +105,10 @@ struct io_tlb_mem {
 	bool late_alloc;
 	bool force_bounce;
 	bool for_alloc;
+	unsigned int num_child;
+	unsigned int child_nslot;
+	unsigned int child_start;
+	struct io_tlb_mem *child;
 	struct io_tlb_slot {
 		phys_addr_t orig_addr;
 		size_t alloc_size;
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index e2ef0864eb1e..32e8f42530b6 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -207,6 +207,26 @@ static void swiotlb_init_io_tlb_mem(struct io_tlb_mem *mem, phys_addr_t start,
 		mem->force_bounce = true;
 
 	spin_lock_init(&mem->lock);
+
+	if (mem->num_child) {
+		mem->child_nslot = nslabs / mem->num_child;
+		mem->child_start = 0;
+
+		/*
+		 * Initialize child IO TLB mem, divide IO TLB pool
+		 * into child number. Reuse parent mem->slot in the
+		 * child mem->slot.
+		 */
+		for (i = 0; i < mem->num_child; i++) {
+			mem->child[i].slots = mem->slots + i * mem->child_nslot;
+			mem->child[i].num_child = 0;
+
+			swiotlb_init_io_tlb_mem(&mem->child[i],
+				start + ((i * mem->child_nslot) << IO_TLB_SHIFT),
+				mem->child_nslot, late_alloc);
+		}
+	}
+
 	for (i = 0; i < mem->nslabs; i++) {
 		mem->slots[i].list = IO_TLB_SEGSIZE - io_tlb_offset(i);
 		mem->slots[i].orig_addr = INVALID_PHYS_ADDR;
@@ -336,16 +356,18 @@ int swiotlb_init_late(size_t size, gfp_t gfp_mask,
 
 	mem->slots = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
 		get_order(array_size(sizeof(*mem->slots), nslabs)));
-	if (!mem->slots) {
-		free_pages((unsigned long)vstart, order);
-		return -ENOMEM;
-	}
+	if (!mem->slots)
+		goto error_slots;
 
 	set_memory_decrypted((unsigned long)vstart, bytes >> PAGE_SHIFT);
 	swiotlb_init_io_tlb_mem(mem, virt_to_phys(vstart), nslabs, true);
 
 	swiotlb_print_info();
 	return 0;
+
+error_slots:
+	free_pages((unsigned long)vstart, order);
+	return -ENOMEM;
 }
 
 void __init swiotlb_exit(void)
@@ -483,10 +505,11 @@ static unsigned int wrap_index(struct io_tlb_mem *mem, unsigned int index)
  * Find a suitable number of IO TLB entries size that will fit this request and
  * allocate a buffer from that IO TLB pool.
  */
-static int swiotlb_find_slots(struct device *dev, phys_addr_t orig_addr,
-			      size_t alloc_size, unsigned int alloc_align_mask)
+static int swiotlb_do_find_slots(struct io_tlb_mem *mem,
+				 struct device *dev, phys_addr_t orig_addr,
+				 size_t alloc_size,
+				 unsigned int alloc_align_mask)
 {
-	struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
 	unsigned long boundary_mask = dma_get_seg_boundary(dev);
 	dma_addr_t tbl_dma_addr =
 		phys_to_dma_unencrypted(dev, mem->start) & boundary_mask;
@@ -565,6 +588,46 @@ static int swiotlb_find_slots(struct device *dev, phys_addr_t orig_addr,
 	return index;
 }
 
+static int swiotlb_find_slots(struct device *dev, phys_addr_t orig_addr,
+			      size_t alloc_size, unsigned int alloc_align_mask)
+{
+	struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
+	struct io_tlb_mem *child_mem = mem;
+	int start = 0, i = 0, index;
+
+	if (mem->num_child) {
+		i = start = mem->child_start;
+		mem->child_start = (mem->child_start + 1) % mem->num_child;
+		child_mem = mem->child;
+	}
+
+	do {
+		index = swiotlb_do_find_slots(child_mem + i, dev, orig_addr,
+					      alloc_size, alloc_align_mask);
+		if (index >= 0)
+			return i * mem->child_nslot + index;
+		if (++i >= mem->num_child)
+			i = 0;
+	} while (i != start);
+
+	return -1;
+}
+
+static unsigned long mem_used(struct io_tlb_mem *mem)
+{
+	int i;
+	unsigned long used = 0;
+
+	if (mem->num_child) {
+		for (i = 0; i < mem->num_child; i++)
+			used += mem->child[i].used;
+	} else {
+		used = mem->used;
+	}
+
+	return used;
+}
+
 phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
 		size_t mapping_size, size_t alloc_size,
 		unsigned int alloc_align_mask, enum dma_data_direction dir,
@@ -594,7 +657,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
 		if (!(attrs & DMA_ATTR_NO_WARN))
 			dev_warn_ratelimited(dev,
 	"swiotlb buffer is full (sz: %zd bytes), total %lu (slots), used %lu (slots)\n",
-				 alloc_size, mem->nslabs, mem->used);
+				     alloc_size, mem->nslabs, mem_used(mem));
 		return (phys_addr_t)DMA_MAPPING_ERROR;
 	}
 
@@ -617,9 +680,9 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
 	return tlb_addr;
 }
 
-static void swiotlb_release_slots(struct device *dev, phys_addr_t tlb_addr)
+static void swiotlb_do_release_slots(struct io_tlb_mem *mem,
+				     struct device *dev, phys_addr_t tlb_addr)
 {
-	struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
 	unsigned long flags;
 	unsigned int offset = swiotlb_align_offset(dev, tlb_addr);
 	int index = (tlb_addr - offset - mem->start) >> IO_TLB_SHIFT;
@@ -660,6 +723,20 @@ static void swiotlb_release_slots(struct device *dev, phys_addr_t tlb_addr)
 	spin_unlock_irqrestore(&mem->lock, flags);
 }
 
+static void swiotlb_release_slots(struct device *dev, phys_addr_t tlb_addr)
+{
+	struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
+	int index, offset;
+
+	if (mem->num_child) {
+		offset = swiotlb_align_offset(dev, tlb_addr);	
+		index = (tlb_addr - offset - mem->start) >> IO_TLB_SHIFT;
+		mem = &mem->child[index / mem->child_nslot];
+	}
+
+	swiotlb_do_release_slots(mem, dev, tlb_addr);
+}
+
 /*
  * tlb_addr is the physical address of the bounce buffer to unmap.
  */
-- 
2.25.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* [RFC PATCH V2 2/2] Swiotlb: Add device bounce buffer allocation interface
  2022-05-02 12:54 ` Tianyu Lan
@ 2022-05-02 12:54   ` Tianyu Lan
  -1 siblings, 0 replies; 20+ messages in thread
From: Tianyu Lan @ 2022-05-02 12:54 UTC (permalink / raw)
  To: hch, m.szyprowski, robin.murphy, michael.h.kelley, kys
  Cc: Tianyu Lan, iommu, linux-kernel, vkuznets, brijesh.singh,
	konrad.wilk, hch, wei.liu, parri.andrea, thomas.lendacky,
	linux-hyperv, andi.kleen, kirill.shutemov

From: Tianyu Lan <Tianyu.Lan@microsoft.com>

In SEV/TDX Confidential VM, device DMA transaction needs use swiotlb
bounce buffer to share data with host/hypervisor. The swiotlb spinlock
introduces overhead among devices if they share io tlb mem. Avoid such
issue, introduce swiotlb_device_allocate() to allocate device bounce
buffer from default io tlb pool and set up child IO tlb mem for queue
bounce buffer allocaton according input queue number. Device may have
multi io queues and setting up the same number of child io tlb mem may
help to resolve spinlock overhead among queues.

Introduce IO TLB Block unit(2MB) concepts to allocate big bounce buffer
from default pool for devices. IO TLB segment(256k) is too small.

Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
 include/linux/swiotlb.h |  35 +++++++-
 kernel/dma/swiotlb.c    | 195 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 225 insertions(+), 5 deletions(-)

diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 4a3f6a7b4b7e..efd29e884fd7 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -31,6 +31,14 @@ struct scatterlist;
 #define IO_TLB_SHIFT 11
 #define IO_TLB_SIZE (1 << IO_TLB_SHIFT)
 
+/*
+ * IO TLB BLOCK UNIT as device bounce buffer allocation unit.
+ * This allows device allocates bounce buffer from default io
+ * tlb pool.
+ */
+#define IO_TLB_BLOCKSIZE   (8 * IO_TLB_SEGSIZE)
+#define IO_TLB_BLOCK_UNIT  (IO_TLB_BLOCKSIZE << IO_TLB_SHIFT)
+
 /* default to 64MB */
 #define IO_TLB_DEFAULT_SIZE (64UL<<20)
 
@@ -89,9 +97,11 @@ extern enum swiotlb_force swiotlb_force;
  * @late_alloc:	%true if allocated using the page allocator
  * @force_bounce: %true if swiotlb bouncing is forced
  * @for_alloc:  %true if the pool is used for memory allocation
- * @child_nslot:The number of IO TLB slot in the child IO TLB mem.
  * @num_child:  The child io tlb mem number in the pool.
+ * @child_nslot:The number of IO TLB slot in the child IO TLB mem.
+ * @child_nblock:The number of IO TLB block in the child IO TLB mem.
  * @child_start:The child index to start searching in the next round.
+ * @block_start:The block index to start searching in the next round.
  */
 struct io_tlb_mem {
 	phys_addr_t start;
@@ -107,8 +117,16 @@ struct io_tlb_mem {
 	bool for_alloc;
 	unsigned int num_child;
 	unsigned int child_nslot;
+	unsigned int child_nblock;
 	unsigned int child_start;
+	unsigned int block_index;
 	struct io_tlb_mem *child;
+	struct io_tlb_mem *parent;
+	struct io_tlb_block {
+		size_t alloc_size;
+		unsigned long start_slot;
+		unsigned int list;
+	} *block;
 	struct io_tlb_slot {
 		phys_addr_t orig_addr;
 		size_t alloc_size;
@@ -137,6 +155,10 @@ unsigned int swiotlb_max_segment(void);
 size_t swiotlb_max_mapping_size(struct device *dev);
 bool is_swiotlb_active(struct device *dev);
 void __init swiotlb_adjust_size(unsigned long size);
+int swiotlb_device_allocate(struct device *dev,
+			    unsigned int area_num,
+			    unsigned long size);
+void swiotlb_device_free(struct device *dev);
 #else
 static inline void swiotlb_init(bool addressing_limited, unsigned int flags)
 {
@@ -169,6 +191,17 @@ static inline bool is_swiotlb_active(struct device *dev)
 static inline void swiotlb_adjust_size(unsigned long size)
 {
 }
+
+void swiotlb_device_free(struct device *dev)
+{
+}
+
+int swiotlb_device_allocate(struct device *dev,
+			    unsigned int area_num,
+			    unsigned long size)
+{
+	return -ENOMEM;
+}
 #endif /* CONFIG_SWIOTLB */
 
 extern void swiotlb_print_info(void);
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 32e8f42530b6..f8a0711cd9de 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -195,7 +195,8 @@ static void swiotlb_init_io_tlb_mem(struct io_tlb_mem *mem, phys_addr_t start,
 				    unsigned long nslabs, bool late_alloc)
 {
 	void *vaddr = phys_to_virt(start);
-	unsigned long bytes = nslabs << IO_TLB_SHIFT, i;
+	unsigned long bytes = nslabs << IO_TLB_SHIFT, i, j;
+	unsigned int block_num = nslabs / IO_TLB_BLOCKSIZE;
 
 	mem->nslabs = nslabs;
 	mem->start = start;
@@ -210,6 +211,7 @@ static void swiotlb_init_io_tlb_mem(struct io_tlb_mem *mem, phys_addr_t start,
 
 	if (mem->num_child) {
 		mem->child_nslot = nslabs / mem->num_child;
+		mem->child_nblock = block_num / mem->num_child;
 		mem->child_start = 0;
 
 		/*
@@ -219,15 +221,24 @@ static void swiotlb_init_io_tlb_mem(struct io_tlb_mem *mem, phys_addr_t start,
 		 */
 		for (i = 0; i < mem->num_child; i++) {
 			mem->child[i].slots = mem->slots + i * mem->child_nslot;
-			mem->child[i].num_child = 0;
+			mem->child[i].block = mem->block + i * mem->child_nblock;
+			mem->child[i].num_child = 0;			
 
 			swiotlb_init_io_tlb_mem(&mem->child[i],
 				start + ((i * mem->child_nslot) << IO_TLB_SHIFT),
 				mem->child_nslot, late_alloc);
 		}
+
+		return;
 	}
 
-	for (i = 0; i < mem->nslabs; i++) {
+	for (i = 0, j = 0; i < mem->nslabs; i++) {
+		if (!(i % IO_TLB_BLOCKSIZE)) {
+			mem->block[j].alloc_size = 0;
+			mem->block[j].list = block_num--;
+			j++;
+		}
+
 		mem->slots[i].list = IO_TLB_SEGSIZE - io_tlb_offset(i);
 		mem->slots[i].orig_addr = INVALID_PHYS_ADDR;
 		mem->slots[i].alloc_size = 0;
@@ -292,6 +303,13 @@ void __init swiotlb_init_remap(bool addressing_limit, unsigned int flags,
 		panic("%s: Failed to allocate %zu bytes align=0x%lx\n",
 		      __func__, alloc_size, PAGE_SIZE);
 
+	mem->num_child = 0;
+	mem->block = memblock_alloc(sizeof(struct io_tlb_block) *
+				    (default_nslabs / IO_TLB_BLOCKSIZE),
+				     SMP_CACHE_BYTES);
+	if (!mem->block)
+		panic("%s: Failed to allocate mem->block.\n", __func__);
+
 	swiotlb_init_io_tlb_mem(mem, __pa(tlb), default_nslabs, false);
 	mem->force_bounce = flags & SWIOTLB_FORCE;
 
@@ -316,7 +334,7 @@ int swiotlb_init_late(size_t size, gfp_t gfp_mask,
 	unsigned long nslabs = ALIGN(size >> IO_TLB_SHIFT, IO_TLB_SEGSIZE);
 	unsigned long bytes;
 	unsigned char *vstart = NULL;
-	unsigned int order;
+	unsigned int order, block_order;
 	int rc = 0;
 
 	if (swiotlb_force_disable)
@@ -354,6 +372,13 @@ int swiotlb_init_late(size_t size, gfp_t gfp_mask,
 		goto retry;
 	}
 
+	block_order = get_order(array_size(sizeof(*mem->block),
+		nslabs / IO_TLB_BLOCKSIZE));
+	mem->block = (struct io_tlb_block *)
+		__get_free_pages(GFP_KERNEL | __GFP_ZERO, block_order);
+	if (!mem->block)
+		goto error_block;
+
 	mem->slots = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
 		get_order(array_size(sizeof(*mem->slots), nslabs)));
 	if (!mem->slots)
@@ -366,6 +391,8 @@ int swiotlb_init_late(size_t size, gfp_t gfp_mask,
 	return 0;
 
 error_slots:
+	free_pages((unsigned long)mem->block, block_order);
+error_block:
 	free_pages((unsigned long)vstart, order);
 	return -ENOMEM;
 }
@@ -375,6 +402,7 @@ void __init swiotlb_exit(void)
 	struct io_tlb_mem *mem = &io_tlb_default_mem;
 	unsigned long tbl_vaddr;
 	size_t tbl_size, slots_size;
+	unsigned int block_array_size, block_order;
 
 	if (swiotlb_force_bounce)
 		return;
@@ -386,12 +414,16 @@ void __init swiotlb_exit(void)
 	tbl_vaddr = (unsigned long)phys_to_virt(mem->start);
 	tbl_size = PAGE_ALIGN(mem->end - mem->start);
 	slots_size = PAGE_ALIGN(array_size(sizeof(*mem->slots), mem->nslabs));
+	block_array_size = array_size(sizeof(*mem->block), mem->nslabs / IO_TLB_BLOCKSIZE);
 
 	set_memory_encrypted(tbl_vaddr, tbl_size >> PAGE_SHIFT);
 	if (mem->late_alloc) {
+		block_order = get_order(block_array_size);
+		free_pages((unsigned long)mem->block, block_order);
 		free_pages(tbl_vaddr, get_order(tbl_size));
 		free_pages((unsigned long)mem->slots, get_order(slots_size));
 	} else {
+		memblock_free_late(__pa(mem->block), block_array_size);
 		memblock_free_late(mem->start, tbl_size);
 		memblock_free_late(__pa(mem->slots), slots_size);
 	}
@@ -839,6 +871,161 @@ static int __init __maybe_unused swiotlb_create_default_debugfs(void)
 late_initcall(swiotlb_create_default_debugfs);
 #endif
 
+static void swiotlb_do_free_block(struct io_tlb_mem *mem,
+		phys_addr_t start, unsigned int block_num)
+{
+
+	unsigned int start_slot = (start - mem->start) >> IO_TLB_SHIFT;
+	unsigned int block_index = start_slot / IO_TLB_BLOCKSIZE;
+	unsigned int mem_block_num = mem->nslabs / IO_TLB_BLOCKSIZE;
+	unsigned long flags;
+	int count, i, num;
+
+	spin_lock_irqsave(&mem->lock, flags);
+	if (block_index + block_num < mem_block_num)
+		count = mem->block[block_index + mem_block_num].list;
+	else
+		count = 0;
+
+
+	for (i = block_index + block_num; i >= block_index; i--) {
+		mem->block[i].list = ++count;
+		/* Todo: recover slot->list and alloc_size here. */
+	}
+
+	for (i = block_index - 1, num = block_index % mem_block_num;
+	    i < num && mem->block[i].list; i--)
+		mem->block[i].list = ++count;
+
+	spin_unlock_irqrestore(&mem->lock, flags);
+}
+
+static void swiotlb_free_block(struct io_tlb_mem *mem,
+			       phys_addr_t start, unsigned int block_num)
+{
+	unsigned int slot_index, child_index;
+
+	if (mem->num_child) {
+		slot_index = (start - mem->start) >> IO_TLB_SHIFT;
+		child_index = slot_index / mem->child_nslot;
+
+		swiotlb_do_free_block(&mem->child[child_index],
+				      start, block_num);
+	} else {
+		swiotlb_do_free_block(mem, start, block_num);
+	}
+}
+
+void swiotlb_device_free(struct device *dev)
+{
+	struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
+	struct io_tlb_mem *parent_mem = dev->dma_io_tlb_mem->parent;
+
+	swiotlb_free_block(parent_mem, mem->start, mem->nslabs / IO_TLB_BLOCKSIZE);
+}
+
+
+static struct page *swiotlb_alloc_block(struct io_tlb_mem *mem, unsigned int block_num)
+{
+	unsigned int block_index, nslot;
+	phys_addr_t tlb_addr;
+	unsigned long flags;
+	int i, j;
+
+	if (!mem || !mem->block)
+		return NULL;
+
+	spin_lock_irqsave(&mem->lock, flags);
+	block_index = mem->block_index;
+
+	/* Todo: Search more blocks. */
+	if (mem->block[block_index].list < block_num) {
+		spin_unlock_irqrestore(&mem->lock, flags);
+		return NULL;
+	}
+
+	/* Update block and slot list. */
+	for (i = block_index; i < block_index + block_num; i++) {
+		mem->block[i].list = 0;
+		mem->block[i].alloc_size = IO_TLB_BLOCKSIZE;
+		for (j = 0; j < IO_TLB_BLOCKSIZE; j++) {
+			nslot = i * IO_TLB_BLOCKSIZE + j;
+			mem->slots[nslot].list = 0;
+			mem->slots[nslot].alloc_size = IO_TLB_SIZE;
+		}
+	}
+
+	mem->index = nslot + 1;
+	mem->block_index += block_num;
+	mem->used += block_num * IO_TLB_BLOCKSIZE;
+	spin_unlock_irqrestore(&mem->lock, flags);
+
+	tlb_addr = slot_addr(mem->start, block_index * IO_TLB_BLOCKSIZE);
+	return pfn_to_page(PFN_DOWN(tlb_addr));
+}
+
+/*
+ * swiotlb_device_allocate - Allocate bounce buffer fo device from
+ * default io tlb pool. The allocation size should be aligned with
+ * IO_TLB_BLOCK_UNIT.
+ */
+int swiotlb_device_allocate(struct device *dev,
+			    unsigned int queue_num,
+			    unsigned long size)
+{
+	struct io_tlb_mem *mem, *parent_mem = dev->dma_io_tlb_mem;
+	unsigned long nslabs = ALIGN(size >> IO_TLB_SHIFT, IO_TLB_BLOCKSIZE);
+	struct page *page;
+	int ret = -ENOMEM;
+
+	page = swiotlb_alloc_block(parent_mem, nslabs / IO_TLB_BLOCKSIZE);
+	if (!page)
+		return -ENOMEM;
+
+	mem = kzalloc(sizeof(*mem), GFP_KERNEL);
+	if (!mem)
+		goto error_mem;
+
+	mem->slots = kzalloc(array_size(sizeof(*mem->slots), nslabs),
+			     GFP_KERNEL);
+	if (!mem->slots)
+		goto error_slots;
+
+	mem->block = kcalloc(nslabs / IO_TLB_BLOCKSIZE,
+				sizeof(struct io_tlb_block),
+				GFP_KERNEL);
+	if (!mem->block)
+		goto error_block;
+
+	mem->num_child = queue_num;
+	mem->child = kcalloc(queue_num,
+				sizeof(struct io_tlb_mem),
+				GFP_KERNEL);
+	if (!mem->child)
+		goto error_child;
+
+
+	swiotlb_init_io_tlb_mem(mem, page_to_phys(page), nslabs, true);
+	mem->force_bounce = true;
+	mem->for_alloc = true;
+
+	mem->vaddr = parent_mem->vaddr + page_to_phys(page) -  parent_mem->start;
+	dev->dma_io_tlb_mem->parent = parent_mem;
+	dev->dma_io_tlb_mem = mem;
+	return 0;
+
+error_child:
+	kfree(mem->block);
+error_block:
+	kfree(mem->slots);
+error_slots:
+	kfree(mem);
+error_mem:
+	swiotlb_free_block(mem, page_to_phys(page), nslabs / IO_TLB_BLOCKSIZE);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(swiotlb_device_allocate);
+
 #ifdef CONFIG_DMA_RESTRICTED_POOL
 
 struct page *swiotlb_alloc(struct device *dev, size_t size)
-- 
2.25.1


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

* [RFC PATCH V2 2/2] Swiotlb: Add device bounce buffer allocation interface
@ 2022-05-02 12:54   ` Tianyu Lan
  0 siblings, 0 replies; 20+ messages in thread
From: Tianyu Lan @ 2022-05-02 12:54 UTC (permalink / raw)
  To: hch, m.szyprowski, robin.murphy, michael.h.kelley, kys
  Cc: parri.andrea, thomas.lendacky, wei.liu, Tianyu Lan, linux-hyperv,
	konrad.wilk, linux-kernel, kirill.shutemov, iommu, andi.kleen,
	brijesh.singh, vkuznets, hch

From: Tianyu Lan <Tianyu.Lan@microsoft.com>

In SEV/TDX Confidential VM, device DMA transaction needs use swiotlb
bounce buffer to share data with host/hypervisor. The swiotlb spinlock
introduces overhead among devices if they share io tlb mem. Avoid such
issue, introduce swiotlb_device_allocate() to allocate device bounce
buffer from default io tlb pool and set up child IO tlb mem for queue
bounce buffer allocaton according input queue number. Device may have
multi io queues and setting up the same number of child io tlb mem may
help to resolve spinlock overhead among queues.

Introduce IO TLB Block unit(2MB) concepts to allocate big bounce buffer
from default pool for devices. IO TLB segment(256k) is too small.

Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
 include/linux/swiotlb.h |  35 +++++++-
 kernel/dma/swiotlb.c    | 195 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 225 insertions(+), 5 deletions(-)

diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 4a3f6a7b4b7e..efd29e884fd7 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -31,6 +31,14 @@ struct scatterlist;
 #define IO_TLB_SHIFT 11
 #define IO_TLB_SIZE (1 << IO_TLB_SHIFT)
 
+/*
+ * IO TLB BLOCK UNIT as device bounce buffer allocation unit.
+ * This allows device allocates bounce buffer from default io
+ * tlb pool.
+ */
+#define IO_TLB_BLOCKSIZE   (8 * IO_TLB_SEGSIZE)
+#define IO_TLB_BLOCK_UNIT  (IO_TLB_BLOCKSIZE << IO_TLB_SHIFT)
+
 /* default to 64MB */
 #define IO_TLB_DEFAULT_SIZE (64UL<<20)
 
@@ -89,9 +97,11 @@ extern enum swiotlb_force swiotlb_force;
  * @late_alloc:	%true if allocated using the page allocator
  * @force_bounce: %true if swiotlb bouncing is forced
  * @for_alloc:  %true if the pool is used for memory allocation
- * @child_nslot:The number of IO TLB slot in the child IO TLB mem.
  * @num_child:  The child io tlb mem number in the pool.
+ * @child_nslot:The number of IO TLB slot in the child IO TLB mem.
+ * @child_nblock:The number of IO TLB block in the child IO TLB mem.
  * @child_start:The child index to start searching in the next round.
+ * @block_start:The block index to start searching in the next round.
  */
 struct io_tlb_mem {
 	phys_addr_t start;
@@ -107,8 +117,16 @@ struct io_tlb_mem {
 	bool for_alloc;
 	unsigned int num_child;
 	unsigned int child_nslot;
+	unsigned int child_nblock;
 	unsigned int child_start;
+	unsigned int block_index;
 	struct io_tlb_mem *child;
+	struct io_tlb_mem *parent;
+	struct io_tlb_block {
+		size_t alloc_size;
+		unsigned long start_slot;
+		unsigned int list;
+	} *block;
 	struct io_tlb_slot {
 		phys_addr_t orig_addr;
 		size_t alloc_size;
@@ -137,6 +155,10 @@ unsigned int swiotlb_max_segment(void);
 size_t swiotlb_max_mapping_size(struct device *dev);
 bool is_swiotlb_active(struct device *dev);
 void __init swiotlb_adjust_size(unsigned long size);
+int swiotlb_device_allocate(struct device *dev,
+			    unsigned int area_num,
+			    unsigned long size);
+void swiotlb_device_free(struct device *dev);
 #else
 static inline void swiotlb_init(bool addressing_limited, unsigned int flags)
 {
@@ -169,6 +191,17 @@ static inline bool is_swiotlb_active(struct device *dev)
 static inline void swiotlb_adjust_size(unsigned long size)
 {
 }
+
+void swiotlb_device_free(struct device *dev)
+{
+}
+
+int swiotlb_device_allocate(struct device *dev,
+			    unsigned int area_num,
+			    unsigned long size)
+{
+	return -ENOMEM;
+}
 #endif /* CONFIG_SWIOTLB */
 
 extern void swiotlb_print_info(void);
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 32e8f42530b6..f8a0711cd9de 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -195,7 +195,8 @@ static void swiotlb_init_io_tlb_mem(struct io_tlb_mem *mem, phys_addr_t start,
 				    unsigned long nslabs, bool late_alloc)
 {
 	void *vaddr = phys_to_virt(start);
-	unsigned long bytes = nslabs << IO_TLB_SHIFT, i;
+	unsigned long bytes = nslabs << IO_TLB_SHIFT, i, j;
+	unsigned int block_num = nslabs / IO_TLB_BLOCKSIZE;
 
 	mem->nslabs = nslabs;
 	mem->start = start;
@@ -210,6 +211,7 @@ static void swiotlb_init_io_tlb_mem(struct io_tlb_mem *mem, phys_addr_t start,
 
 	if (mem->num_child) {
 		mem->child_nslot = nslabs / mem->num_child;
+		mem->child_nblock = block_num / mem->num_child;
 		mem->child_start = 0;
 
 		/*
@@ -219,15 +221,24 @@ static void swiotlb_init_io_tlb_mem(struct io_tlb_mem *mem, phys_addr_t start,
 		 */
 		for (i = 0; i < mem->num_child; i++) {
 			mem->child[i].slots = mem->slots + i * mem->child_nslot;
-			mem->child[i].num_child = 0;
+			mem->child[i].block = mem->block + i * mem->child_nblock;
+			mem->child[i].num_child = 0;			
 
 			swiotlb_init_io_tlb_mem(&mem->child[i],
 				start + ((i * mem->child_nslot) << IO_TLB_SHIFT),
 				mem->child_nslot, late_alloc);
 		}
+
+		return;
 	}
 
-	for (i = 0; i < mem->nslabs; i++) {
+	for (i = 0, j = 0; i < mem->nslabs; i++) {
+		if (!(i % IO_TLB_BLOCKSIZE)) {
+			mem->block[j].alloc_size = 0;
+			mem->block[j].list = block_num--;
+			j++;
+		}
+
 		mem->slots[i].list = IO_TLB_SEGSIZE - io_tlb_offset(i);
 		mem->slots[i].orig_addr = INVALID_PHYS_ADDR;
 		mem->slots[i].alloc_size = 0;
@@ -292,6 +303,13 @@ void __init swiotlb_init_remap(bool addressing_limit, unsigned int flags,
 		panic("%s: Failed to allocate %zu bytes align=0x%lx\n",
 		      __func__, alloc_size, PAGE_SIZE);
 
+	mem->num_child = 0;
+	mem->block = memblock_alloc(sizeof(struct io_tlb_block) *
+				    (default_nslabs / IO_TLB_BLOCKSIZE),
+				     SMP_CACHE_BYTES);
+	if (!mem->block)
+		panic("%s: Failed to allocate mem->block.\n", __func__);
+
 	swiotlb_init_io_tlb_mem(mem, __pa(tlb), default_nslabs, false);
 	mem->force_bounce = flags & SWIOTLB_FORCE;
 
@@ -316,7 +334,7 @@ int swiotlb_init_late(size_t size, gfp_t gfp_mask,
 	unsigned long nslabs = ALIGN(size >> IO_TLB_SHIFT, IO_TLB_SEGSIZE);
 	unsigned long bytes;
 	unsigned char *vstart = NULL;
-	unsigned int order;
+	unsigned int order, block_order;
 	int rc = 0;
 
 	if (swiotlb_force_disable)
@@ -354,6 +372,13 @@ int swiotlb_init_late(size_t size, gfp_t gfp_mask,
 		goto retry;
 	}
 
+	block_order = get_order(array_size(sizeof(*mem->block),
+		nslabs / IO_TLB_BLOCKSIZE));
+	mem->block = (struct io_tlb_block *)
+		__get_free_pages(GFP_KERNEL | __GFP_ZERO, block_order);
+	if (!mem->block)
+		goto error_block;
+
 	mem->slots = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
 		get_order(array_size(sizeof(*mem->slots), nslabs)));
 	if (!mem->slots)
@@ -366,6 +391,8 @@ int swiotlb_init_late(size_t size, gfp_t gfp_mask,
 	return 0;
 
 error_slots:
+	free_pages((unsigned long)mem->block, block_order);
+error_block:
 	free_pages((unsigned long)vstart, order);
 	return -ENOMEM;
 }
@@ -375,6 +402,7 @@ void __init swiotlb_exit(void)
 	struct io_tlb_mem *mem = &io_tlb_default_mem;
 	unsigned long tbl_vaddr;
 	size_t tbl_size, slots_size;
+	unsigned int block_array_size, block_order;
 
 	if (swiotlb_force_bounce)
 		return;
@@ -386,12 +414,16 @@ void __init swiotlb_exit(void)
 	tbl_vaddr = (unsigned long)phys_to_virt(mem->start);
 	tbl_size = PAGE_ALIGN(mem->end - mem->start);
 	slots_size = PAGE_ALIGN(array_size(sizeof(*mem->slots), mem->nslabs));
+	block_array_size = array_size(sizeof(*mem->block), mem->nslabs / IO_TLB_BLOCKSIZE);
 
 	set_memory_encrypted(tbl_vaddr, tbl_size >> PAGE_SHIFT);
 	if (mem->late_alloc) {
+		block_order = get_order(block_array_size);
+		free_pages((unsigned long)mem->block, block_order);
 		free_pages(tbl_vaddr, get_order(tbl_size));
 		free_pages((unsigned long)mem->slots, get_order(slots_size));
 	} else {
+		memblock_free_late(__pa(mem->block), block_array_size);
 		memblock_free_late(mem->start, tbl_size);
 		memblock_free_late(__pa(mem->slots), slots_size);
 	}
@@ -839,6 +871,161 @@ static int __init __maybe_unused swiotlb_create_default_debugfs(void)
 late_initcall(swiotlb_create_default_debugfs);
 #endif
 
+static void swiotlb_do_free_block(struct io_tlb_mem *mem,
+		phys_addr_t start, unsigned int block_num)
+{
+
+	unsigned int start_slot = (start - mem->start) >> IO_TLB_SHIFT;
+	unsigned int block_index = start_slot / IO_TLB_BLOCKSIZE;
+	unsigned int mem_block_num = mem->nslabs / IO_TLB_BLOCKSIZE;
+	unsigned long flags;
+	int count, i, num;
+
+	spin_lock_irqsave(&mem->lock, flags);
+	if (block_index + block_num < mem_block_num)
+		count = mem->block[block_index + mem_block_num].list;
+	else
+		count = 0;
+
+
+	for (i = block_index + block_num; i >= block_index; i--) {
+		mem->block[i].list = ++count;
+		/* Todo: recover slot->list and alloc_size here. */
+	}
+
+	for (i = block_index - 1, num = block_index % mem_block_num;
+	    i < num && mem->block[i].list; i--)
+		mem->block[i].list = ++count;
+
+	spin_unlock_irqrestore(&mem->lock, flags);
+}
+
+static void swiotlb_free_block(struct io_tlb_mem *mem,
+			       phys_addr_t start, unsigned int block_num)
+{
+	unsigned int slot_index, child_index;
+
+	if (mem->num_child) {
+		slot_index = (start - mem->start) >> IO_TLB_SHIFT;
+		child_index = slot_index / mem->child_nslot;
+
+		swiotlb_do_free_block(&mem->child[child_index],
+				      start, block_num);
+	} else {
+		swiotlb_do_free_block(mem, start, block_num);
+	}
+}
+
+void swiotlb_device_free(struct device *dev)
+{
+	struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
+	struct io_tlb_mem *parent_mem = dev->dma_io_tlb_mem->parent;
+
+	swiotlb_free_block(parent_mem, mem->start, mem->nslabs / IO_TLB_BLOCKSIZE);
+}
+
+
+static struct page *swiotlb_alloc_block(struct io_tlb_mem *mem, unsigned int block_num)
+{
+	unsigned int block_index, nslot;
+	phys_addr_t tlb_addr;
+	unsigned long flags;
+	int i, j;
+
+	if (!mem || !mem->block)
+		return NULL;
+
+	spin_lock_irqsave(&mem->lock, flags);
+	block_index = mem->block_index;
+
+	/* Todo: Search more blocks. */
+	if (mem->block[block_index].list < block_num) {
+		spin_unlock_irqrestore(&mem->lock, flags);
+		return NULL;
+	}
+
+	/* Update block and slot list. */
+	for (i = block_index; i < block_index + block_num; i++) {
+		mem->block[i].list = 0;
+		mem->block[i].alloc_size = IO_TLB_BLOCKSIZE;
+		for (j = 0; j < IO_TLB_BLOCKSIZE; j++) {
+			nslot = i * IO_TLB_BLOCKSIZE + j;
+			mem->slots[nslot].list = 0;
+			mem->slots[nslot].alloc_size = IO_TLB_SIZE;
+		}
+	}
+
+	mem->index = nslot + 1;
+	mem->block_index += block_num;
+	mem->used += block_num * IO_TLB_BLOCKSIZE;
+	spin_unlock_irqrestore(&mem->lock, flags);
+
+	tlb_addr = slot_addr(mem->start, block_index * IO_TLB_BLOCKSIZE);
+	return pfn_to_page(PFN_DOWN(tlb_addr));
+}
+
+/*
+ * swiotlb_device_allocate - Allocate bounce buffer fo device from
+ * default io tlb pool. The allocation size should be aligned with
+ * IO_TLB_BLOCK_UNIT.
+ */
+int swiotlb_device_allocate(struct device *dev,
+			    unsigned int queue_num,
+			    unsigned long size)
+{
+	struct io_tlb_mem *mem, *parent_mem = dev->dma_io_tlb_mem;
+	unsigned long nslabs = ALIGN(size >> IO_TLB_SHIFT, IO_TLB_BLOCKSIZE);
+	struct page *page;
+	int ret = -ENOMEM;
+
+	page = swiotlb_alloc_block(parent_mem, nslabs / IO_TLB_BLOCKSIZE);
+	if (!page)
+		return -ENOMEM;
+
+	mem = kzalloc(sizeof(*mem), GFP_KERNEL);
+	if (!mem)
+		goto error_mem;
+
+	mem->slots = kzalloc(array_size(sizeof(*mem->slots), nslabs),
+			     GFP_KERNEL);
+	if (!mem->slots)
+		goto error_slots;
+
+	mem->block = kcalloc(nslabs / IO_TLB_BLOCKSIZE,
+				sizeof(struct io_tlb_block),
+				GFP_KERNEL);
+	if (!mem->block)
+		goto error_block;
+
+	mem->num_child = queue_num;
+	mem->child = kcalloc(queue_num,
+				sizeof(struct io_tlb_mem),
+				GFP_KERNEL);
+	if (!mem->child)
+		goto error_child;
+
+
+	swiotlb_init_io_tlb_mem(mem, page_to_phys(page), nslabs, true);
+	mem->force_bounce = true;
+	mem->for_alloc = true;
+
+	mem->vaddr = parent_mem->vaddr + page_to_phys(page) -  parent_mem->start;
+	dev->dma_io_tlb_mem->parent = parent_mem;
+	dev->dma_io_tlb_mem = mem;
+	return 0;
+
+error_child:
+	kfree(mem->block);
+error_block:
+	kfree(mem->slots);
+error_slots:
+	kfree(mem);
+error_mem:
+	swiotlb_free_block(mem, page_to_phys(page), nslabs / IO_TLB_BLOCKSIZE);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(swiotlb_device_allocate);
+
 #ifdef CONFIG_DMA_RESTRICTED_POOL
 
 struct page *swiotlb_alloc(struct device *dev, size_t size)
-- 
2.25.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [RFC PATCH V2 2/2] Swiotlb: Add device bounce buffer allocation interface
  2022-05-02 12:54   ` Tianyu Lan
@ 2022-05-04  8:11 ` Dan Carpenter
  -1 siblings, 0 replies; 20+ messages in thread
From: kernel test robot @ 2022-05-02 20:36 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 8652 bytes --]

CC: kbuild-all(a)lists.01.org
BCC: lkp(a)intel.com
In-Reply-To: <20220502125436.23607-3-ltykernel@gmail.com>
References: <20220502125436.23607-3-ltykernel@gmail.com>
TO: Tianyu Lan <ltykernel@gmail.com>

Hi Tianyu,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on next-20220429]
[cannot apply to linus/master v5.18-rc5 v5.18-rc4 v5.18-rc3 v5.18-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Tianyu-Lan/swiotlb-Add-child-io-tlb-mem-support/20220502-205700
base:    5469f0c06732a077c70a759a81f2a1f00b277694
:::::: branch date: 8 hours ago
:::::: commit date: 8 hours ago
config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20220503/202205030442.Iugj4ezG-lkp(a)intel.com/config)
compiler: gcc-11 (Debian 11.2.0-20) 11.2.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
kernel/dma/swiotlb.c:958 swiotlb_alloc_block() error: uninitialized symbol 'nslot'.
kernel/dma/swiotlb.c:1024 swiotlb_device_allocate() error: double free of 'mem'

vim +/nslot +958 kernel/dma/swiotlb.c

3349f5b007cd7e Tianyu Lan 2022-05-02   926  
3349f5b007cd7e Tianyu Lan 2022-05-02   927  
3349f5b007cd7e Tianyu Lan 2022-05-02   928  static struct page *swiotlb_alloc_block(struct io_tlb_mem *mem, unsigned int block_num)
3349f5b007cd7e Tianyu Lan 2022-05-02   929  {
3349f5b007cd7e Tianyu Lan 2022-05-02   930  	unsigned int block_index, nslot;
3349f5b007cd7e Tianyu Lan 2022-05-02   931  	phys_addr_t tlb_addr;
3349f5b007cd7e Tianyu Lan 2022-05-02   932  	unsigned long flags;
3349f5b007cd7e Tianyu Lan 2022-05-02   933  	int i, j;
3349f5b007cd7e Tianyu Lan 2022-05-02   934  
3349f5b007cd7e Tianyu Lan 2022-05-02   935  	if (!mem || !mem->block)
3349f5b007cd7e Tianyu Lan 2022-05-02   936  		return NULL;
3349f5b007cd7e Tianyu Lan 2022-05-02   937  
3349f5b007cd7e Tianyu Lan 2022-05-02   938  	spin_lock_irqsave(&mem->lock, flags);
3349f5b007cd7e Tianyu Lan 2022-05-02   939  	block_index = mem->block_index;
3349f5b007cd7e Tianyu Lan 2022-05-02   940  
3349f5b007cd7e Tianyu Lan 2022-05-02   941  	/* Todo: Search more blocks. */
3349f5b007cd7e Tianyu Lan 2022-05-02   942  	if (mem->block[block_index].list < block_num) {
3349f5b007cd7e Tianyu Lan 2022-05-02   943  		spin_unlock_irqrestore(&mem->lock, flags);
3349f5b007cd7e Tianyu Lan 2022-05-02   944  		return NULL;
3349f5b007cd7e Tianyu Lan 2022-05-02   945  	}
3349f5b007cd7e Tianyu Lan 2022-05-02   946  
3349f5b007cd7e Tianyu Lan 2022-05-02   947  	/* Update block and slot list. */
3349f5b007cd7e Tianyu Lan 2022-05-02   948  	for (i = block_index; i < block_index + block_num; i++) {
3349f5b007cd7e Tianyu Lan 2022-05-02   949  		mem->block[i].list = 0;
3349f5b007cd7e Tianyu Lan 2022-05-02   950  		mem->block[i].alloc_size = IO_TLB_BLOCKSIZE;
3349f5b007cd7e Tianyu Lan 2022-05-02   951  		for (j = 0; j < IO_TLB_BLOCKSIZE; j++) {
3349f5b007cd7e Tianyu Lan 2022-05-02   952  			nslot = i * IO_TLB_BLOCKSIZE + j;
3349f5b007cd7e Tianyu Lan 2022-05-02   953  			mem->slots[nslot].list = 0;
3349f5b007cd7e Tianyu Lan 2022-05-02   954  			mem->slots[nslot].alloc_size = IO_TLB_SIZE;
3349f5b007cd7e Tianyu Lan 2022-05-02   955  		}
3349f5b007cd7e Tianyu Lan 2022-05-02   956  	}
3349f5b007cd7e Tianyu Lan 2022-05-02   957  
3349f5b007cd7e Tianyu Lan 2022-05-02  @958  	mem->index = nslot + 1;
3349f5b007cd7e Tianyu Lan 2022-05-02   959  	mem->block_index += block_num;
3349f5b007cd7e Tianyu Lan 2022-05-02   960  	mem->used += block_num * IO_TLB_BLOCKSIZE;
3349f5b007cd7e Tianyu Lan 2022-05-02   961  	spin_unlock_irqrestore(&mem->lock, flags);
3349f5b007cd7e Tianyu Lan 2022-05-02   962  
3349f5b007cd7e Tianyu Lan 2022-05-02   963  	tlb_addr = slot_addr(mem->start, block_index * IO_TLB_BLOCKSIZE);
3349f5b007cd7e Tianyu Lan 2022-05-02   964  	return pfn_to_page(PFN_DOWN(tlb_addr));
3349f5b007cd7e Tianyu Lan 2022-05-02   965  }
3349f5b007cd7e Tianyu Lan 2022-05-02   966  
3349f5b007cd7e Tianyu Lan 2022-05-02   967  /*
3349f5b007cd7e Tianyu Lan 2022-05-02   968   * swiotlb_device_allocate - Allocate bounce buffer fo device from
3349f5b007cd7e Tianyu Lan 2022-05-02   969   * default io tlb pool. The allocation size should be aligned with
3349f5b007cd7e Tianyu Lan 2022-05-02   970   * IO_TLB_BLOCK_UNIT.
3349f5b007cd7e Tianyu Lan 2022-05-02   971   */
3349f5b007cd7e Tianyu Lan 2022-05-02   972  int swiotlb_device_allocate(struct device *dev,
3349f5b007cd7e Tianyu Lan 2022-05-02   973  			    unsigned int queue_num,
3349f5b007cd7e Tianyu Lan 2022-05-02   974  			    unsigned long size)
3349f5b007cd7e Tianyu Lan 2022-05-02   975  {
3349f5b007cd7e Tianyu Lan 2022-05-02   976  	struct io_tlb_mem *mem, *parent_mem = dev->dma_io_tlb_mem;
3349f5b007cd7e Tianyu Lan 2022-05-02   977  	unsigned long nslabs = ALIGN(size >> IO_TLB_SHIFT, IO_TLB_BLOCKSIZE);
3349f5b007cd7e Tianyu Lan 2022-05-02   978  	struct page *page;
3349f5b007cd7e Tianyu Lan 2022-05-02   979  	int ret = -ENOMEM;
3349f5b007cd7e Tianyu Lan 2022-05-02   980  
3349f5b007cd7e Tianyu Lan 2022-05-02   981  	page = swiotlb_alloc_block(parent_mem, nslabs / IO_TLB_BLOCKSIZE);
3349f5b007cd7e Tianyu Lan 2022-05-02   982  	if (!page)
3349f5b007cd7e Tianyu Lan 2022-05-02   983  		return -ENOMEM;
3349f5b007cd7e Tianyu Lan 2022-05-02   984  
3349f5b007cd7e Tianyu Lan 2022-05-02   985  	mem = kzalloc(sizeof(*mem), GFP_KERNEL);
3349f5b007cd7e Tianyu Lan 2022-05-02   986  	if (!mem)
3349f5b007cd7e Tianyu Lan 2022-05-02   987  		goto error_mem;
3349f5b007cd7e Tianyu Lan 2022-05-02   988  
3349f5b007cd7e Tianyu Lan 2022-05-02   989  	mem->slots = kzalloc(array_size(sizeof(*mem->slots), nslabs),
3349f5b007cd7e Tianyu Lan 2022-05-02   990  			     GFP_KERNEL);
3349f5b007cd7e Tianyu Lan 2022-05-02   991  	if (!mem->slots)
3349f5b007cd7e Tianyu Lan 2022-05-02   992  		goto error_slots;
3349f5b007cd7e Tianyu Lan 2022-05-02   993  
3349f5b007cd7e Tianyu Lan 2022-05-02   994  	mem->block = kcalloc(nslabs / IO_TLB_BLOCKSIZE,
3349f5b007cd7e Tianyu Lan 2022-05-02   995  				sizeof(struct io_tlb_block),
3349f5b007cd7e Tianyu Lan 2022-05-02   996  				GFP_KERNEL);
3349f5b007cd7e Tianyu Lan 2022-05-02   997  	if (!mem->block)
3349f5b007cd7e Tianyu Lan 2022-05-02   998  		goto error_block;
3349f5b007cd7e Tianyu Lan 2022-05-02   999  
3349f5b007cd7e Tianyu Lan 2022-05-02  1000  	mem->num_child = queue_num;
3349f5b007cd7e Tianyu Lan 2022-05-02  1001  	mem->child = kcalloc(queue_num,
3349f5b007cd7e Tianyu Lan 2022-05-02  1002  				sizeof(struct io_tlb_mem),
3349f5b007cd7e Tianyu Lan 2022-05-02  1003  				GFP_KERNEL);
3349f5b007cd7e Tianyu Lan 2022-05-02  1004  	if (!mem->child)
3349f5b007cd7e Tianyu Lan 2022-05-02  1005  		goto error_child;
3349f5b007cd7e Tianyu Lan 2022-05-02  1006  
3349f5b007cd7e Tianyu Lan 2022-05-02  1007  
3349f5b007cd7e Tianyu Lan 2022-05-02  1008  	swiotlb_init_io_tlb_mem(mem, page_to_phys(page), nslabs, true);
3349f5b007cd7e Tianyu Lan 2022-05-02  1009  	mem->force_bounce = true;
3349f5b007cd7e Tianyu Lan 2022-05-02  1010  	mem->for_alloc = true;
3349f5b007cd7e Tianyu Lan 2022-05-02  1011  
3349f5b007cd7e Tianyu Lan 2022-05-02  1012  	mem->vaddr = parent_mem->vaddr + page_to_phys(page) -  parent_mem->start;
3349f5b007cd7e Tianyu Lan 2022-05-02  1013  	dev->dma_io_tlb_mem->parent = parent_mem;
3349f5b007cd7e Tianyu Lan 2022-05-02  1014  	dev->dma_io_tlb_mem = mem;
3349f5b007cd7e Tianyu Lan 2022-05-02  1015  	return 0;
3349f5b007cd7e Tianyu Lan 2022-05-02  1016  
3349f5b007cd7e Tianyu Lan 2022-05-02  1017  error_child:
3349f5b007cd7e Tianyu Lan 2022-05-02  1018  	kfree(mem->block);
3349f5b007cd7e Tianyu Lan 2022-05-02  1019  error_block:
3349f5b007cd7e Tianyu Lan 2022-05-02  1020  	kfree(mem->slots);
3349f5b007cd7e Tianyu Lan 2022-05-02  1021  error_slots:
3349f5b007cd7e Tianyu Lan 2022-05-02  1022  	kfree(mem);
3349f5b007cd7e Tianyu Lan 2022-05-02  1023  error_mem:
3349f5b007cd7e Tianyu Lan 2022-05-02 @1024  	swiotlb_free_block(mem, page_to_phys(page), nslabs / IO_TLB_BLOCKSIZE);
3349f5b007cd7e Tianyu Lan 2022-05-02  1025  	return ret;
3349f5b007cd7e Tianyu Lan 2022-05-02  1026  }
3349f5b007cd7e Tianyu Lan 2022-05-02  1027  EXPORT_SYMBOL_GPL(swiotlb_device_allocate);
3349f5b007cd7e Tianyu Lan 2022-05-02  1028  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

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

* Re: [RFC PATCH V2 2/2] Swiotlb: Add device bounce buffer allocation interface
@ 2022-05-04  8:11 ` Dan Carpenter
  0 siblings, 0 replies; 20+ messages in thread
From: Dan Carpenter @ 2022-05-04  8:11 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 4926 bytes --]

Hi Tianyu,

url:    https://github.com/intel-lab-lkp/linux/commits/Tianyu-Lan/swiotlb-Add-child-io-tlb-mem-support/20220502-205700
base:    5469f0c06732a077c70a759a81f2a1f00b277694
config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20220503/202205030442.Iugj4ezG-lkp(a)intel.com/config)
compiler: gcc-11 (Debian 11.2.0-20) 11.2.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
kernel/dma/swiotlb.c:1024 swiotlb_device_allocate() error: double free of 'mem'

vim +/nslot +958 kernel/dma/swiotlb.c

3349f5b007cd7e Tianyu Lan 2022-05-02   972  int swiotlb_device_allocate(struct device *dev,
3349f5b007cd7e Tianyu Lan 2022-05-02   973  			    unsigned int queue_num,
3349f5b007cd7e Tianyu Lan 2022-05-02   974  			    unsigned long size)
3349f5b007cd7e Tianyu Lan 2022-05-02   975  {
3349f5b007cd7e Tianyu Lan 2022-05-02   976  	struct io_tlb_mem *mem, *parent_mem = dev->dma_io_tlb_mem;
3349f5b007cd7e Tianyu Lan 2022-05-02   977  	unsigned long nslabs = ALIGN(size >> IO_TLB_SHIFT, IO_TLB_BLOCKSIZE);
3349f5b007cd7e Tianyu Lan 2022-05-02   978  	struct page *page;
3349f5b007cd7e Tianyu Lan 2022-05-02   979  	int ret = -ENOMEM;
3349f5b007cd7e Tianyu Lan 2022-05-02   980  
3349f5b007cd7e Tianyu Lan 2022-05-02   981  	page = swiotlb_alloc_block(parent_mem, nslabs / IO_TLB_BLOCKSIZE);
3349f5b007cd7e Tianyu Lan 2022-05-02   982  	if (!page)
3349f5b007cd7e Tianyu Lan 2022-05-02   983  		return -ENOMEM;
3349f5b007cd7e Tianyu Lan 2022-05-02   984  
3349f5b007cd7e Tianyu Lan 2022-05-02   985  	mem = kzalloc(sizeof(*mem), GFP_KERNEL);
3349f5b007cd7e Tianyu Lan 2022-05-02   986  	if (!mem)
3349f5b007cd7e Tianyu Lan 2022-05-02   987  		goto error_mem;
3349f5b007cd7e Tianyu Lan 2022-05-02   988  
3349f5b007cd7e Tianyu Lan 2022-05-02   989  	mem->slots = kzalloc(array_size(sizeof(*mem->slots), nslabs),
3349f5b007cd7e Tianyu Lan 2022-05-02   990  			     GFP_KERNEL);
3349f5b007cd7e Tianyu Lan 2022-05-02   991  	if (!mem->slots)
3349f5b007cd7e Tianyu Lan 2022-05-02   992  		goto error_slots;
3349f5b007cd7e Tianyu Lan 2022-05-02   993  
3349f5b007cd7e Tianyu Lan 2022-05-02   994  	mem->block = kcalloc(nslabs / IO_TLB_BLOCKSIZE,
3349f5b007cd7e Tianyu Lan 2022-05-02   995  				sizeof(struct io_tlb_block),
3349f5b007cd7e Tianyu Lan 2022-05-02   996  				GFP_KERNEL);
3349f5b007cd7e Tianyu Lan 2022-05-02   997  	if (!mem->block)
3349f5b007cd7e Tianyu Lan 2022-05-02   998  		goto error_block;
3349f5b007cd7e Tianyu Lan 2022-05-02   999  
3349f5b007cd7e Tianyu Lan 2022-05-02  1000  	mem->num_child = queue_num;
3349f5b007cd7e Tianyu Lan 2022-05-02  1001  	mem->child = kcalloc(queue_num,
3349f5b007cd7e Tianyu Lan 2022-05-02  1002  				sizeof(struct io_tlb_mem),
3349f5b007cd7e Tianyu Lan 2022-05-02  1003  				GFP_KERNEL);
3349f5b007cd7e Tianyu Lan 2022-05-02  1004  	if (!mem->child)
3349f5b007cd7e Tianyu Lan 2022-05-02  1005  		goto error_child;
3349f5b007cd7e Tianyu Lan 2022-05-02  1006  
3349f5b007cd7e Tianyu Lan 2022-05-02  1007  
3349f5b007cd7e Tianyu Lan 2022-05-02  1008  	swiotlb_init_io_tlb_mem(mem, page_to_phys(page), nslabs, true);
3349f5b007cd7e Tianyu Lan 2022-05-02  1009  	mem->force_bounce = true;
3349f5b007cd7e Tianyu Lan 2022-05-02  1010  	mem->for_alloc = true;
3349f5b007cd7e Tianyu Lan 2022-05-02  1011  
3349f5b007cd7e Tianyu Lan 2022-05-02  1012  	mem->vaddr = parent_mem->vaddr + page_to_phys(page) -  parent_mem->start;
3349f5b007cd7e Tianyu Lan 2022-05-02  1013  	dev->dma_io_tlb_mem->parent = parent_mem;
3349f5b007cd7e Tianyu Lan 2022-05-02  1014  	dev->dma_io_tlb_mem = mem;
3349f5b007cd7e Tianyu Lan 2022-05-02  1015  	return 0;
3349f5b007cd7e Tianyu Lan 2022-05-02  1016  
3349f5b007cd7e Tianyu Lan 2022-05-02  1017  error_child:
3349f5b007cd7e Tianyu Lan 2022-05-02  1018  	kfree(mem->block);
3349f5b007cd7e Tianyu Lan 2022-05-02  1019  error_block:
3349f5b007cd7e Tianyu Lan 2022-05-02  1020  	kfree(mem->slots);
3349f5b007cd7e Tianyu Lan 2022-05-02  1021  error_slots:
3349f5b007cd7e Tianyu Lan 2022-05-02  1022  	kfree(mem);
                                                ^^^^^^^^^^
Free

3349f5b007cd7e Tianyu Lan 2022-05-02  1023  error_mem:
3349f5b007cd7e Tianyu Lan 2022-05-02 @1024  	swiotlb_free_block(mem, page_to_phys(page), nslabs / IO_TLB_BLOCKSIZE);
                                                                   ^^^
This was probably intended to be parent_mem.  My git tree does not
have swiotlb_free_block() so I have not looked at why Smatch says
that the first parameter is freed.  The kbuild-bot does not do cross
function analysis so this must be parsed inline.

3349f5b007cd7e Tianyu Lan 2022-05-02  1025  	return ret;
3349f5b007cd7e Tianyu Lan 2022-05-02  1026  }

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

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

* Re: [RFC PATCH V2 0/2] swiotlb: Add child io tlb mem support
  2022-05-02 12:54 ` Tianyu Lan
@ 2022-05-09 11:49   ` Tianyu Lan
  -1 siblings, 0 replies; 20+ messages in thread
From: Tianyu Lan @ 2022-05-09 11:49 UTC (permalink / raw)
  To: hch, robin.murphy
  Cc: Tianyu Lan, iommu, linux-kernel, vkuznets, brijesh.singh,
	konrad.wilk, hch, wei.liu, parri.andrea, thomas.lendacky,
	linux-hyperv, andi.kleen, kirill.shutemov, m.szyprowski,
	michael.h.kelley, kys

On 5/2/2022 8:54 PM, Tianyu Lan wrote:
> From: Tianyu Lan <Tianyu.Lan@microsoft.com>
> 
> Traditionally swiotlb was not performance critical because it was only
> used for slow devices. But in some setups, like TDX/SEV confidential
> guests, all IO has to go through swiotlb. Currently swiotlb only has a
> single lock. Under high IO load with multiple CPUs this can lead to
> significant lock contention on the swiotlb lock.
> 
> This patch adds child IO TLB mem support to resolve spinlock overhead
> among device's queues. Each device may allocate IO tlb mem and setup
> child IO TLB mem according to queue number. The number child IO tlb
> mem maybe set up equal with device queue number and this helps to resolve
> swiotlb spinlock overhead among devices and queues.
> 
> Patch 2 introduces IO TLB Block concepts and swiotlb_device_allocate()
> API to allocate per-device swiotlb bounce buffer. The new API Accepts
> queue number as the number of child IO TLB mem to set up device's IO
> TLB mem.

Gentile ping...

Thanks.
> 
> Tianyu Lan (2):
>    swiotlb: Add Child IO TLB mem support
>    Swiotlb: Add device bounce buffer allocation interface
> 
>   include/linux/swiotlb.h |  40 ++++++
>   kernel/dma/swiotlb.c    | 290 ++++++++++++++++++++++++++++++++++++++--
>   2 files changed, 317 insertions(+), 13 deletions(-)
> 

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

* Re: [RFC PATCH V2 0/2] swiotlb: Add child io tlb mem support
@ 2022-05-09 11:49   ` Tianyu Lan
  0 siblings, 0 replies; 20+ messages in thread
From: Tianyu Lan @ 2022-05-09 11:49 UTC (permalink / raw)
  To: hch, robin.murphy
  Cc: parri.andrea, thomas.lendacky, wei.liu, Tianyu Lan, linux-hyperv,
	konrad.wilk, linux-kernel, kirill.shutemov, iommu,
	michael.h.kelley, andi.kleen, brijesh.singh, vkuznets, kys, hch

On 5/2/2022 8:54 PM, Tianyu Lan wrote:
> From: Tianyu Lan <Tianyu.Lan@microsoft.com>
> 
> Traditionally swiotlb was not performance critical because it was only
> used for slow devices. But in some setups, like TDX/SEV confidential
> guests, all IO has to go through swiotlb. Currently swiotlb only has a
> single lock. Under high IO load with multiple CPUs this can lead to
> significant lock contention on the swiotlb lock.
> 
> This patch adds child IO TLB mem support to resolve spinlock overhead
> among device's queues. Each device may allocate IO tlb mem and setup
> child IO TLB mem according to queue number. The number child IO tlb
> mem maybe set up equal with device queue number and this helps to resolve
> swiotlb spinlock overhead among devices and queues.
> 
> Patch 2 introduces IO TLB Block concepts and swiotlb_device_allocate()
> API to allocate per-device swiotlb bounce buffer. The new API Accepts
> queue number as the number of child IO TLB mem to set up device's IO
> TLB mem.

Gentile ping...

Thanks.
> 
> Tianyu Lan (2):
>    swiotlb: Add Child IO TLB mem support
>    Swiotlb: Add device bounce buffer allocation interface
> 
>   include/linux/swiotlb.h |  40 ++++++
>   kernel/dma/swiotlb.c    | 290 ++++++++++++++++++++++++++++++++++++++--
>   2 files changed, 317 insertions(+), 13 deletions(-)
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [RFC PATCH V2 1/2] swiotlb: Add Child IO TLB mem support
  2022-05-02 12:54   ` Tianyu Lan
@ 2022-05-16  7:34     ` Christoph Hellwig
  -1 siblings, 0 replies; 20+ messages in thread
From: Christoph Hellwig @ 2022-05-16  7:34 UTC (permalink / raw)
  To: Tianyu Lan
  Cc: hch, m.szyprowski, robin.murphy, michael.h.kelley, kys,
	Tianyu Lan, iommu, linux-kernel, vkuznets, brijesh.singh,
	konrad.wilk, hch, wei.liu, parri.andrea, thomas.lendacky,
	linux-hyperv, andi.kleen, kirill.shutemov

I don't really understand how 'childs' fit in here.  The code also
doesn't seem to be usable without patch 2 and a caller of the
new functions added in patch 2, so it is rather impossible to review.

Also:

 1) why is SEV/TDX so different from other cases that need bounce
    buffering to treat it different and we can't work on a general
    scalability improvement
 2) per previous discussions at how swiotlb itself works, it is
    clear that another option is to just make pages we DMA to
    shared with the hypervisor.  Why don't we try that at least
    for larger I/O?

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

* Re: [RFC PATCH V2 1/2] swiotlb: Add Child IO TLB mem support
@ 2022-05-16  7:34     ` Christoph Hellwig
  0 siblings, 0 replies; 20+ messages in thread
From: Christoph Hellwig @ 2022-05-16  7:34 UTC (permalink / raw)
  To: Tianyu Lan
  Cc: parri.andrea, michael.h.kelley, wei.liu, Tianyu Lan,
	linux-hyperv, konrad.wilk, linux-kernel, hch, iommu,
	kirill.shutemov, thomas.lendacky, andi.kleen, brijesh.singh,
	vkuznets, kys, robin.murphy, hch

I don't really understand how 'childs' fit in here.  The code also
doesn't seem to be usable without patch 2 and a caller of the
new functions added in patch 2, so it is rather impossible to review.

Also:

 1) why is SEV/TDX so different from other cases that need bounce
    buffering to treat it different and we can't work on a general
    scalability improvement
 2) per previous discussions at how swiotlb itself works, it is
    clear that another option is to just make pages we DMA to
    shared with the hypervisor.  Why don't we try that at least
    for larger I/O?
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [RFC PATCH V2 1/2] swiotlb: Add Child IO TLB mem support
  2022-05-16  7:34     ` Christoph Hellwig
@ 2022-05-16 13:08       ` Tianyu Lan
  -1 siblings, 0 replies; 20+ messages in thread
From: Tianyu Lan @ 2022-05-16 13:08 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: m.szyprowski, robin.murphy, michael.h.kelley, kys, Tianyu Lan,
	iommu, linux-kernel, vkuznets, brijesh.singh, konrad.wilk, hch,
	wei.liu, parri.andrea, thomas.lendacky, linux-hyperv, andi.kleen,
	kirill.shutemov

On 5/16/2022 3:34 PM, Christoph Hellwig wrote:
> I don't really understand how 'childs' fit in here.  The code also
> doesn't seem to be usable without patch 2 and a caller of the
> new functions added in patch 2, so it is rather impossible to review.

Hi Christoph:
      OK. I will merge two patches and add a caller patch. The motivation
is to avoid global spin lock when devices use swiotlb bounce buffer and
this introduces overhead during high throughput cases. In my test
environment, current code can achieve about 24Gb/s network throughput
with SWIOTLB force enabled and it can achieve about 40Gb/s without
SWIOTLB force. Storage also has the same issue.
      Per-device IO TLB mem may resolve global spin lock issue among
devices but device still may have multi queues. Multi queues still need
to share one spin lock. This is why introduce child or IO tlb areas in
the previous patches. Each device queues will have separate child IO TLB
mem and single spin lock to manage their IO TLB buffers.
      Otherwise, global spin lock still cost cpu usage during high 
throughput even when there is performance regression. Each device queues 
needs to spin on the different cpus to acquire the global lock. Child IO
TLB mem also may resolve the cpu issue.

> 
> Also:
> 
>   1) why is SEV/TDX so different from other cases that need bounce
>      buffering to treat it different and we can't work on a general
>      scalability improvement

	Other cases also have global spin lock issue but it depends on
         whether hits the bottleneck. The cpu usage issue may be ignored.

>   2) per previous discussions at how swiotlb itself works, it is
>      clear that another option is to just make pages we DMA to
>      shared with the hypervisor.  Why don't we try that at least
>      for larger I/O?

	For confidential VM(Both TDX and SEV), we need to use bounce
	buffer to copy between private memory that hypervisor can't
	access directly and shared memory. For security consideration,
	confidential VM	should not share IO stack DMA pages with
        	hypervisor directly to avoid attack from hypervisor when IO
	stack handles the DMA data.
	

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

* Re: [RFC PATCH V2 1/2] swiotlb: Add Child IO TLB mem support
@ 2022-05-16 13:08       ` Tianyu Lan
  0 siblings, 0 replies; 20+ messages in thread
From: Tianyu Lan @ 2022-05-16 13:08 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: parri.andrea, thomas.lendacky, wei.liu, Tianyu Lan, linux-hyperv,
	konrad.wilk, linux-kernel, michael.h.kelley, iommu,
	kirill.shutemov, andi.kleen, brijesh.singh, vkuznets, kys,
	robin.murphy, hch

On 5/16/2022 3:34 PM, Christoph Hellwig wrote:
> I don't really understand how 'childs' fit in here.  The code also
> doesn't seem to be usable without patch 2 and a caller of the
> new functions added in patch 2, so it is rather impossible to review.

Hi Christoph:
      OK. I will merge two patches and add a caller patch. The motivation
is to avoid global spin lock when devices use swiotlb bounce buffer and
this introduces overhead during high throughput cases. In my test
environment, current code can achieve about 24Gb/s network throughput
with SWIOTLB force enabled and it can achieve about 40Gb/s without
SWIOTLB force. Storage also has the same issue.
      Per-device IO TLB mem may resolve global spin lock issue among
devices but device still may have multi queues. Multi queues still need
to share one spin lock. This is why introduce child or IO tlb areas in
the previous patches. Each device queues will have separate child IO TLB
mem and single spin lock to manage their IO TLB buffers.
      Otherwise, global spin lock still cost cpu usage during high 
throughput even when there is performance regression. Each device queues 
needs to spin on the different cpus to acquire the global lock. Child IO
TLB mem also may resolve the cpu issue.

> 
> Also:
> 
>   1) why is SEV/TDX so different from other cases that need bounce
>      buffering to treat it different and we can't work on a general
>      scalability improvement

	Other cases also have global spin lock issue but it depends on
         whether hits the bottleneck. The cpu usage issue may be ignored.

>   2) per previous discussions at how swiotlb itself works, it is
>      clear that another option is to just make pages we DMA to
>      shared with the hypervisor.  Why don't we try that at least
>      for larger I/O?

	For confidential VM(Both TDX and SEV), we need to use bounce
	buffer to copy between private memory that hypervisor can't
	access directly and shared memory. For security consideration,
	confidential VM	should not share IO stack DMA pages with
        	hypervisor directly to avoid attack from hypervisor when IO
	stack handles the DMA data.
	
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* RE: [RFC PATCH V2 1/2] swiotlb: Add Child IO TLB mem support
  2022-05-16  7:34     ` Christoph Hellwig
  (?)
  (?)
@ 2022-05-30  1:52     ` Michael Kelley (LINUX) via iommu
  2022-05-31  7:16         ` hch
  -1 siblings, 1 reply; 20+ messages in thread
From: Michael Kelley (LINUX) via iommu @ 2022-05-30  1:52 UTC (permalink / raw)
  To: Christoph Hellwig, Tianyu Lan, robin.murphy, andi.kleen
  Cc: parri.andrea, thomas.lendacky, wei.liu, Tianyu Lan, linux-hyperv,
	konrad.wilk, linux-kernel, kirill.shutemov, iommu, brijesh.singh,
	vkuznets, KY Srinivasan, hch

From: Christoph Hellwig <hch@infradead.org> Sent: Monday, May 16, 2022 12:35 AM
> 
> I don't really understand how 'childs' fit in here.  The code also
> doesn't seem to be usable without patch 2 and a caller of the
> new functions added in patch 2, so it is rather impossible to review.
> 
> Also:
> 
>  1) why is SEV/TDX so different from other cases that need bounce
>     buffering to treat it different and we can't work on a general
>     scalability improvement
>  2) per previous discussions at how swiotlb itself works, it is
>     clear that another option is to just make pages we DMA to
>     shared with the hypervisor.  Why don't we try that at least
>     for larger I/O?

Tianyu already responded, but let me offer an expanded view.
I have better knowledge of AMD's SEV-SNP than of Intel's TDX,
so my details might be off for TDX.

Taking your question (2) first, two things must be done when guest
memory pages transition between the "shared with the hypervisor"
and the "private to the guest" states:

A) Some bookkeeping between the guest and host, which requires
a hypercall.  Doing a hypercall isn't super-fast, but for large I/Os,
it could be a reasonable tradeoff if we could avoid bounce buffer
copying.

B) The contents of the memory buffer must transition between
encrypted and not encrypted.  The hardware doesn't provide
any mechanism to do such a transition "in place".  The only
way to transition is for the CPU to copy the contents between
an encrypted area and an unencrypted area of memory.

Because of (B), we're stuck needing bounce buffers.  There's no
way to avoid them with the current hardware.  Tianyu also pointed
out not wanting to expose uninitialized guest memory to the host,
so, for example, sharing a read buffer with the host requires that
it first be initialized to zero.

For your question (1), I think we all would agree that SEV-SNP and
TDX usage of bounce buffers isn't fundamentally different from other
uses -- they just put a lot more load on the bounce buffering
mechanism. If done well, general swiotlb scalability improvements
should be sufficient and are much preferred.

You made a recent comment about almost being done removing
all knowledge of swiotlb from drivers [1].  I agree with that goal.
However, Tianyu's recent patches for improving swiotlb scalability
don't align with that goal.  A while back, you suggested using
per-device swiotlb regions [2], and I think Tianyu's patch sets have
taken that approach.  But that approach requires going beyond the
existing per-device swiotlb regions to get scalability with multi-channel
devices, and that's leading us in the wrong direction.

We should reset and make sure we agree on the top-level approach.
1) We want general scalability improvements to swiotlb.  These
    improvements should scale to high CPUs counts (> 100) and for
    multiple NUMA nodes.
2) Drivers should not require any special knowledge of swiotlb to
    benefit from the improvements.  No new swiotlb APIs should be
    need to be used by drivers -- the swiotlb scalability improvements
    should be transparent.
3) The scalability improvements should not be based on device
    boundaries since a single device may have multiple channels
    doing bounce buffering on multiple CPUs in parallel.

Anything else?

The patch from Andi Kleen [3] (not submitted upstream, but referenced
by Tianyu as the basis for his patches) seems like a good starting point
for meeting the top-level approach.  Andi and Robin had some
back-and-forth about Andi's patch that I haven't delved into yet, but
getting that worked out seems like a better overall approach.  I had
an offline chat with Tianyu, and he would agree as well.

Agree?  Disagree?

Michael

[1]  https://lore.kernel.org/lkml/YmqonHKBT8ftYHgY@infradead.org/
[2] https://lore.kernel.org/lkml/20220222080543.GA5412@lst.de/
[3] https://github.com/intel/tdx/commit/4529b578
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [RFC PATCH V2 1/2] swiotlb: Add Child IO TLB mem support
  2022-05-30  1:52     ` Michael Kelley (LINUX) via iommu
@ 2022-05-31  7:16         ` hch
  0 siblings, 0 replies; 20+ messages in thread
From: hch @ 2022-05-31  7:16 UTC (permalink / raw)
  To: Michael Kelley (LINUX)
  Cc: Christoph Hellwig, Tianyu Lan, robin.murphy, andi.kleen,
	m.szyprowski, KY Srinivasan, Tianyu Lan, iommu, linux-kernel,
	vkuznets, brijesh.singh, konrad.wilk, hch, wei.liu, parri.andrea,
	thomas.lendacky, linux-hyperv, kirill.shutemov

On Mon, May 30, 2022 at 01:52:37AM +0000, Michael Kelley (LINUX) wrote:
> B) The contents of the memory buffer must transition between
> encrypted and not encrypted.  The hardware doesn't provide
> any mechanism to do such a transition "in place".  The only
> way to transition is for the CPU to copy the contents between
> an encrypted area and an unencrypted area of memory.
> 
> Because of (B), we're stuck needing bounce buffers.  There's no
> way to avoid them with the current hardware.  Tianyu also pointed
> out not wanting to expose uninitialized guest memory to the host,
> so, for example, sharing a read buffer with the host requires that
> it first be initialized to zero.

Ok, B is a deal breaker.  I just brought this in because I've received
review comments that state bouncing is just the easiest option for
now and we could map it into the hypervisor in the future.  But at
least for SEV that does not seem like an option without hardware
changes.

> We should reset and make sure we agree on the top-level approach.
> 1) We want general scalability improvements to swiotlb.  These
>     improvements should scale to high CPUs counts (> 100) and for
>     multiple NUMA nodes.
> 2) Drivers should not require any special knowledge of swiotlb to
>     benefit from the improvements.  No new swiotlb APIs should be
>     need to be used by drivers -- the swiotlb scalability improvements
>     should be transparent.
> 3) The scalability improvements should not be based on device
>     boundaries since a single device may have multiple channels
>     doing bounce buffering on multiple CPUs in parallel.

Agreed to all counts.

> The patch from Andi Kleen [3] (not submitted upstream, but referenced
> by Tianyu as the basis for his patches) seems like a good starting point
> for meeting the top-level approach.

Yes, I think doing per-cpu and/or per-node scaling sounds like the
right general approach. Why was this never sent out?

> Andi and Robin had some
> back-and-forth about Andi's patch that I haven't delved into yet, but
> getting that worked out seems like a better overall approach.  I had
> an offline chat with Tianyu, and he would agree as well.

Where was this discussion?

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

* Re: [RFC PATCH V2 1/2] swiotlb: Add Child IO TLB mem support
@ 2022-05-31  7:16         ` hch
  0 siblings, 0 replies; 20+ messages in thread
From: hch @ 2022-05-31  7:16 UTC (permalink / raw)
  To: Michael Kelley (LINUX)
  Cc: parri.andrea, thomas.lendacky, wei.liu, Tianyu Lan, linux-hyperv,
	konrad.wilk, linux-kernel, Tianyu Lan, Christoph Hellwig, iommu,
	kirill.shutemov, andi.kleen, brijesh.singh, vkuznets,
	KY Srinivasan, robin.murphy, hch

On Mon, May 30, 2022 at 01:52:37AM +0000, Michael Kelley (LINUX) wrote:
> B) The contents of the memory buffer must transition between
> encrypted and not encrypted.  The hardware doesn't provide
> any mechanism to do such a transition "in place".  The only
> way to transition is for the CPU to copy the contents between
> an encrypted area and an unencrypted area of memory.
> 
> Because of (B), we're stuck needing bounce buffers.  There's no
> way to avoid them with the current hardware.  Tianyu also pointed
> out not wanting to expose uninitialized guest memory to the host,
> so, for example, sharing a read buffer with the host requires that
> it first be initialized to zero.

Ok, B is a deal breaker.  I just brought this in because I've received
review comments that state bouncing is just the easiest option for
now and we could map it into the hypervisor in the future.  But at
least for SEV that does not seem like an option without hardware
changes.

> We should reset and make sure we agree on the top-level approach.
> 1) We want general scalability improvements to swiotlb.  These
>     improvements should scale to high CPUs counts (> 100) and for
>     multiple NUMA nodes.
> 2) Drivers should not require any special knowledge of swiotlb to
>     benefit from the improvements.  No new swiotlb APIs should be
>     need to be used by drivers -- the swiotlb scalability improvements
>     should be transparent.
> 3) The scalability improvements should not be based on device
>     boundaries since a single device may have multiple channels
>     doing bounce buffering on multiple CPUs in parallel.

Agreed to all counts.

> The patch from Andi Kleen [3] (not submitted upstream, but referenced
> by Tianyu as the basis for his patches) seems like a good starting point
> for meeting the top-level approach.

Yes, I think doing per-cpu and/or per-node scaling sounds like the
right general approach. Why was this never sent out?

> Andi and Robin had some
> back-and-forth about Andi's patch that I haven't delved into yet, but
> getting that worked out seems like a better overall approach.  I had
> an offline chat with Tianyu, and he would agree as well.

Where was this discussion?
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* RE: [RFC PATCH V2 1/2] swiotlb: Add Child IO TLB mem support
  2022-05-31  7:16         ` hch
  (?)
@ 2022-05-31 14:13         ` Michael Kelley (LINUX) via iommu
  -1 siblings, 0 replies; 20+ messages in thread
From: Michael Kelley (LINUX) via iommu @ 2022-05-31 14:13 UTC (permalink / raw)
  To: hch
  Cc: parri.andrea, thomas.lendacky, wei.liu, Tianyu Lan, linux-hyperv,
	konrad.wilk, linux-kernel, Tianyu Lan, Christoph Hellwig, iommu,
	kirill.shutemov, andi.kleen, brijesh.singh, vkuznets,
	KY Srinivasan, robin.murphy

From: hch@lst.de <hch@lst.de> Sent: Tuesday, May 31, 2022 12:17 AM
> 
> On Mon, May 30, 2022 at 01:52:37AM +0000, Michael Kelley (LINUX) wrote:
> > B) The contents of the memory buffer must transition between
> > encrypted and not encrypted.  The hardware doesn't provide
> > any mechanism to do such a transition "in place".  The only
> > way to transition is for the CPU to copy the contents between
> > an encrypted area and an unencrypted area of memory.
> >
> > Because of (B), we're stuck needing bounce buffers.  There's no
> > way to avoid them with the current hardware.  Tianyu also pointed
> > out not wanting to expose uninitialized guest memory to the host,
> > so, for example, sharing a read buffer with the host requires that
> > it first be initialized to zero.
> 
> Ok, B is a deal breaker.  I just brought this in because I've received
> review comments that state bouncing is just the easiest option for
> now and we could map it into the hypervisor in the future.  But at
> least for SEV that does not seem like an option without hardware
> changes.
> 
> > We should reset and make sure we agree on the top-level approach.
> > 1) We want general scalability improvements to swiotlb.  These
> >     improvements should scale to high CPUs counts (> 100) and for
> >     multiple NUMA nodes.
> > 2) Drivers should not require any special knowledge of swiotlb to
> >     benefit from the improvements.  No new swiotlb APIs should be
> >     need to be used by drivers -- the swiotlb scalability improvements
> >     should be transparent.
> > 3) The scalability improvements should not be based on device
> >     boundaries since a single device may have multiple channels
> >     doing bounce buffering on multiple CPUs in parallel.
> 
> Agreed to all counts.
> 
> > The patch from Andi Kleen [3] (not submitted upstream, but referenced
> > by Tianyu as the basis for his patches) seems like a good starting point
> > for meeting the top-level approach.
> 
> Yes, I think doing per-cpu and/or per-node scaling sounds like the
> right general approach. Why was this never sent out?

I'll defer to Andi on what his thinking/plan is for this patch.

> 
> > Andi and Robin had some
> > back-and-forth about Andi's patch that I haven't delved into yet, but
> > getting that worked out seems like a better overall approach.  I had
> > an offline chat with Tianyu, and he would agree as well.
> 
> Where was this discussion?

I was just referring to this thread that you are already on:
https://lore.kernel.org/lkml/e7b644f0-6c90-fe99-792d-75c38505dc54@arm.com/

Michael

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [RFC PATCH V2 2/2] Swiotlb: Add device bounce buffer allocation interface
@ 2022-05-03 11:49 kernel test robot
  0 siblings, 0 replies; 20+ messages in thread
From: kernel test robot @ 2022-05-03 11:49 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 20795 bytes --]

CC: llvm(a)lists.linux.dev
CC: kbuild-all(a)lists.01.org
BCC: lkp(a)intel.com
In-Reply-To: <20220502125436.23607-3-ltykernel@gmail.com>
References: <20220502125436.23607-3-ltykernel@gmail.com>
TO: Tianyu Lan <ltykernel@gmail.com>

Hi Tianyu,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on next-20220429]
[cannot apply to linus/master v5.18-rc5 v5.18-rc4 v5.18-rc3 v5.18-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Tianyu-Lan/swiotlb-Add-child-io-tlb-mem-support/20220502-205700
base:    5469f0c06732a077c70a759a81f2a1f00b277694
:::::: branch date: 23 hours ago
:::::: commit date: 23 hours ago
config: x86_64-randconfig-c007 (https://download.01.org/0day-ci/archive/20220503/202205031911.3lYKS95Q-lkp(a)intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 363b3a645a1e30011cc8da624f13dac5fd915628)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/3349f5b007cd7eca2f82daa9dcaf4d234716c069
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Tianyu-Lan/swiotlb-Add-child-io-tlb-mem-support/20220502-205700
        git checkout 3349f5b007cd7eca2f82daa9dcaf4d234716c069
        # save the config file
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 clang-analyzer 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


clang-analyzer warnings: (new ones prefixed by >>)
   kernel/dma/swiotlb.c:982:2: note: Taking false branch
           if (!page)
           ^
   kernel/dma/swiotlb.c:985:8: note: Calling 'kzalloc'
           mem = kzalloc(sizeof(*mem), GFP_KERNEL);
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/slab.h:733:9: note: Calling 'kmalloc'
           return kmalloc(size, flags | __GFP_ZERO);
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/slab.h:588:2: note: Taking false branch
           if (__builtin_constant_p(size)) {
           ^
   include/linux/slab.h:605:2: note: Returning pointer, which participates in a condition later
           return __kmalloc(size, flags);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/slab.h:605:2: note: Returning pointer
           return __kmalloc(size, flags);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/slab.h:733:9: note: Returning from 'kmalloc'
           return kmalloc(size, flags | __GFP_ZERO);
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/slab.h:733:2: note: Returning pointer, which participates in a condition later
           return kmalloc(size, flags | __GFP_ZERO);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/slab.h:733:2: note: Returning pointer
           return kmalloc(size, flags | __GFP_ZERO);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/dma/swiotlb.c:985:8: note: Returning from 'kzalloc'
           mem = kzalloc(sizeof(*mem), GFP_KERNEL);
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/dma/swiotlb.c:985:2: note: Value assigned to 'mem'
           mem = kzalloc(sizeof(*mem), GFP_KERNEL);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/dma/swiotlb.c:986:6: note: Assuming 'mem' is null
           if (!mem)
               ^~~~
   kernel/dma/swiotlb.c:986:2: note: Taking true branch
           if (!mem)
           ^
   kernel/dma/swiotlb.c:987:3: note: Control jumps to line 1024
                   goto error_mem;
                   ^
   kernel/dma/swiotlb.c:1024:21: note: Passing null pointer value via 1st parameter 'mem'
           swiotlb_free_block(mem, page_to_phys(page), nslabs / IO_TLB_BLOCKSIZE);
                              ^~~
   kernel/dma/swiotlb.c:1024:2: note: Calling 'swiotlb_free_block'
           swiotlb_free_block(mem, page_to_phys(page), nslabs / IO_TLB_BLOCKSIZE);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/dma/swiotlb.c:908:6: note: Access to field 'num_child' results in a dereference of a null pointer (loaded from variable 'mem')
           if (mem->num_child) {
               ^~~
   kernel/dma/swiotlb.c:958:21: warning: The left operand of '+' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult]
           mem->index = nslot + 1;
                              ^
   kernel/dma/swiotlb.c:981:9: note: Calling 'swiotlb_alloc_block'
           page = swiotlb_alloc_block(parent_mem, nslabs / IO_TLB_BLOCKSIZE);
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/dma/swiotlb.c:930:28: note: 'nslot' declared without an initial value
           unsigned int block_index, nslot;
                                     ^~~~~
   kernel/dma/swiotlb.c:935:6: note: Assuming 'mem' is non-null
           if (!mem || !mem->block)
               ^~~~
   kernel/dma/swiotlb.c:935:6: note: Left side of '||' is false
   kernel/dma/swiotlb.c:935:14: note: Assuming field 'block' is non-null
           if (!mem || !mem->block)
                       ^~~~~~~~~~~
   kernel/dma/swiotlb.c:935:2: note: Taking false branch
           if (!mem || !mem->block)
           ^
   kernel/dma/swiotlb.c:938:2: note: Loop condition is false.  Exiting loop
           spin_lock_irqsave(&mem->lock, flags);
           ^
   include/linux/spinlock.h:384:2: note: expanded from macro 'spin_lock_irqsave'
           raw_spin_lock_irqsave(spinlock_check(lock), flags);     \
           ^
   include/linux/spinlock.h:240:2: note: expanded from macro 'raw_spin_lock_irqsave'
           do {                                            \
           ^
   kernel/dma/swiotlb.c:938:2: note: Loop condition is false.  Exiting loop
           spin_lock_irqsave(&mem->lock, flags);
           ^
   include/linux/spinlock.h:382:43: note: expanded from macro 'spin_lock_irqsave'
   #define spin_lock_irqsave(lock, flags)                          \
                                                                   ^
   kernel/dma/swiotlb.c:942:6: note: Assuming 'block_num' is <= field 'list'
           if (mem->block[block_index].list < block_num) {
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/dma/swiotlb.c:942:2: note: Taking false branch
           if (mem->block[block_index].list < block_num) {
           ^
   kernel/dma/swiotlb.c:948:24: note: Assuming the condition is false
           for (i = block_index; i < block_index + block_num; i++) {
                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/dma/swiotlb.c:948:2: note: Loop condition is false. Execution continues on line 958
           for (i = block_index; i < block_index + block_num; i++) {
           ^
   kernel/dma/swiotlb.c:958:21: note: The left operand of '+' is a garbage value
           mem->index = nslot + 1;
                        ~~~~~ ^
>> kernel/dma/swiotlb.c:1024:2: warning: Use of memory after it is freed [clang-analyzer-unix.Malloc]
           swiotlb_free_block(mem, page_to_phys(page), nslabs / IO_TLB_BLOCKSIZE);
           ^                  ~~~
   kernel/dma/swiotlb.c:982:6: note: Assuming 'page' is non-null
           if (!page)
               ^~~~~
   kernel/dma/swiotlb.c:982:2: note: Taking false branch
           if (!page)
           ^
   kernel/dma/swiotlb.c:986:6: note: Assuming 'mem' is non-null
           if (!mem)
               ^~~~
   kernel/dma/swiotlb.c:986:2: note: Taking false branch
           if (!mem)
           ^
   kernel/dma/swiotlb.c:991:6: note: Assuming field 'slots' is null
           if (!mem->slots)
               ^~~~~~~~~~~
   kernel/dma/swiotlb.c:991:2: note: Taking true branch
           if (!mem->slots)
           ^
   kernel/dma/swiotlb.c:992:3: note: Control jumps to line 1022
                   goto error_slots;
                   ^
   kernel/dma/swiotlb.c:1022:2: note: Memory is released
           kfree(mem);
           ^~~~~~~~~~
   kernel/dma/swiotlb.c:1024:2: note: Use of memory after it is freed
           swiotlb_free_block(mem, page_to_phys(page), nslabs / IO_TLB_BLOCKSIZE);
           ^                  ~~~
   Suppressed 48 warnings (48 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   53 warnings generated.
   mm/gup.c:711:2: warning: Value stored to 'page' is never read [clang-analyzer-deadcode.DeadStores]
           page = follow_trans_huge_pmd(vma, address, pmd, flags);
           ^      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   mm/gup.c:711:2: note: Value stored to 'page' is never read
           page = follow_trans_huge_pmd(vma, address, pmd, flags);
           ^      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   Suppressed 52 warnings (52 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   50 warnings generated.
   Suppressed 50 warnings (50 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   46 warnings generated.
   Suppressed 46 warnings (46 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   57 warnings generated.
   mm/memory.c:335:2: warning: Value stored to 'p4d' is never read [clang-analyzer-deadcode.DeadStores]
           p4d = p4d_offset(pgd, start);
           ^     ~~~~~~~~~~~~~~~~~~~~~~
   mm/memory.c:335:2: note: Value stored to 'p4d' is never read
           p4d = p4d_offset(pgd, start);
           ^     ~~~~~~~~~~~~~~~~~~~~~~
   mm/memory.c:498:2: warning: Call to function 'memset' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memset_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
           memset(rss, 0, sizeof(int) * NR_MM_COUNTERS);
           ^
   include/linux/fortify-string.h:272:25: note: expanded from macro 'memset'
   #define memset(p, c, s) __fortify_memset_chk(p, c, s,                   \
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/fortify-string.h:265:2: note: expanded from macro '__fortify_memset_chk'
           __underlying_memset(p, c, __fortify_size);                      \
           ^~~~~~~~~~~~~~~~~~~
   include/linux/fortify-string.h:47:29: note: expanded from macro '__underlying_memset'
   #define __underlying_memset     __builtin_memset
                                   ^~~~~~~~~~~~~~~~
   mm/memory.c:498:2: note: Call to function 'memset' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memset_s' in case of C11
           memset(rss, 0, sizeof(int) * NR_MM_COUNTERS);
           ^
   include/linux/fortify-string.h:272:25: note: expanded from macro 'memset'
   #define memset(p, c, s) __fortify_memset_chk(p, c, s,                   \
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/fortify-string.h:265:2: note: expanded from macro '__fortify_memset_chk'
           __underlying_memset(p, c, __fortify_size);                      \
           ^~~~~~~~~~~~~~~~~~~
   include/linux/fortify-string.h:47:29: note: expanded from macro '__underlying_memset'
   #define __underlying_memset     __builtin_memset
                                   ^~~~~~~~~~~~~~~~
   mm/memory.c:2636:3: warning: 1st function call argument is an uninitialized value [clang-analyzer-core.CallAndMessage]
                   pte_unmap_unlock(mapped_pte, ptl);
                   ^
   include/linux/mm.h:2350:2: note: expanded from macro 'pte_unmap_unlock'
           spin_unlock(ptl);                               \
           ^
   mm/memory.c:2808:9: note: Calling '__apply_to_page_range'
           return __apply_to_page_range(mm, addr, size, fn, data, false);
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   mm/memory.c:2760:14: note: Assuming 'addr' is < 'end'
           if (WARN_ON(addr >= end))
                       ^
   include/asm-generic/bug.h:122:25: note: expanded from macro 'WARN_ON'
           int __ret_warn_on = !!(condition);                              \
                                  ^~~~~~~~~
   mm/memory.c:2760:6: note: Taking false branch
           if (WARN_ON(addr >= end))
               ^
   include/asm-generic/bug.h:123:2: note: expanded from macro 'WARN_ON'
           if (unlikely(__ret_warn_on))                                    \
           ^
   mm/memory.c:2760:2: note: Taking false branch
           if (WARN_ON(addr >= end))

vim +1024 kernel/dma/swiotlb.c

3349f5b007cd7e Tianyu Lan 2022-05-02   926  
3349f5b007cd7e Tianyu Lan 2022-05-02   927  
3349f5b007cd7e Tianyu Lan 2022-05-02   928  static struct page *swiotlb_alloc_block(struct io_tlb_mem *mem, unsigned int block_num)
3349f5b007cd7e Tianyu Lan 2022-05-02   929  {
3349f5b007cd7e Tianyu Lan 2022-05-02   930  	unsigned int block_index, nslot;
3349f5b007cd7e Tianyu Lan 2022-05-02   931  	phys_addr_t tlb_addr;
3349f5b007cd7e Tianyu Lan 2022-05-02   932  	unsigned long flags;
3349f5b007cd7e Tianyu Lan 2022-05-02   933  	int i, j;
3349f5b007cd7e Tianyu Lan 2022-05-02   934  
3349f5b007cd7e Tianyu Lan 2022-05-02   935  	if (!mem || !mem->block)
3349f5b007cd7e Tianyu Lan 2022-05-02   936  		return NULL;
3349f5b007cd7e Tianyu Lan 2022-05-02   937  
3349f5b007cd7e Tianyu Lan 2022-05-02   938  	spin_lock_irqsave(&mem->lock, flags);
3349f5b007cd7e Tianyu Lan 2022-05-02   939  	block_index = mem->block_index;
3349f5b007cd7e Tianyu Lan 2022-05-02   940  
3349f5b007cd7e Tianyu Lan 2022-05-02   941  	/* Todo: Search more blocks. */
3349f5b007cd7e Tianyu Lan 2022-05-02   942  	if (mem->block[block_index].list < block_num) {
3349f5b007cd7e Tianyu Lan 2022-05-02   943  		spin_unlock_irqrestore(&mem->lock, flags);
3349f5b007cd7e Tianyu Lan 2022-05-02   944  		return NULL;
3349f5b007cd7e Tianyu Lan 2022-05-02   945  	}
3349f5b007cd7e Tianyu Lan 2022-05-02   946  
3349f5b007cd7e Tianyu Lan 2022-05-02   947  	/* Update block and slot list. */
3349f5b007cd7e Tianyu Lan 2022-05-02  @948  	for (i = block_index; i < block_index + block_num; i++) {
3349f5b007cd7e Tianyu Lan 2022-05-02   949  		mem->block[i].list = 0;
3349f5b007cd7e Tianyu Lan 2022-05-02   950  		mem->block[i].alloc_size = IO_TLB_BLOCKSIZE;
3349f5b007cd7e Tianyu Lan 2022-05-02   951  		for (j = 0; j < IO_TLB_BLOCKSIZE; j++) {
3349f5b007cd7e Tianyu Lan 2022-05-02   952  			nslot = i * IO_TLB_BLOCKSIZE + j;
3349f5b007cd7e Tianyu Lan 2022-05-02   953  			mem->slots[nslot].list = 0;
3349f5b007cd7e Tianyu Lan 2022-05-02   954  			mem->slots[nslot].alloc_size = IO_TLB_SIZE;
3349f5b007cd7e Tianyu Lan 2022-05-02   955  		}
3349f5b007cd7e Tianyu Lan 2022-05-02   956  	}
3349f5b007cd7e Tianyu Lan 2022-05-02   957  
3349f5b007cd7e Tianyu Lan 2022-05-02   958  	mem->index = nslot + 1;
3349f5b007cd7e Tianyu Lan 2022-05-02   959  	mem->block_index += block_num;
3349f5b007cd7e Tianyu Lan 2022-05-02   960  	mem->used += block_num * IO_TLB_BLOCKSIZE;
3349f5b007cd7e Tianyu Lan 2022-05-02   961  	spin_unlock_irqrestore(&mem->lock, flags);
3349f5b007cd7e Tianyu Lan 2022-05-02   962  
3349f5b007cd7e Tianyu Lan 2022-05-02   963  	tlb_addr = slot_addr(mem->start, block_index * IO_TLB_BLOCKSIZE);
3349f5b007cd7e Tianyu Lan 2022-05-02   964  	return pfn_to_page(PFN_DOWN(tlb_addr));
3349f5b007cd7e Tianyu Lan 2022-05-02   965  }
3349f5b007cd7e Tianyu Lan 2022-05-02   966  
3349f5b007cd7e Tianyu Lan 2022-05-02   967  /*
3349f5b007cd7e Tianyu Lan 2022-05-02   968   * swiotlb_device_allocate - Allocate bounce buffer fo device from
3349f5b007cd7e Tianyu Lan 2022-05-02   969   * default io tlb pool. The allocation size should be aligned with
3349f5b007cd7e Tianyu Lan 2022-05-02   970   * IO_TLB_BLOCK_UNIT.
3349f5b007cd7e Tianyu Lan 2022-05-02   971   */
3349f5b007cd7e Tianyu Lan 2022-05-02   972  int swiotlb_device_allocate(struct device *dev,
3349f5b007cd7e Tianyu Lan 2022-05-02   973  			    unsigned int queue_num,
3349f5b007cd7e Tianyu Lan 2022-05-02   974  			    unsigned long size)
3349f5b007cd7e Tianyu Lan 2022-05-02   975  {
3349f5b007cd7e Tianyu Lan 2022-05-02   976  	struct io_tlb_mem *mem, *parent_mem = dev->dma_io_tlb_mem;
3349f5b007cd7e Tianyu Lan 2022-05-02   977  	unsigned long nslabs = ALIGN(size >> IO_TLB_SHIFT, IO_TLB_BLOCKSIZE);
3349f5b007cd7e Tianyu Lan 2022-05-02   978  	struct page *page;
3349f5b007cd7e Tianyu Lan 2022-05-02   979  	int ret = -ENOMEM;
3349f5b007cd7e Tianyu Lan 2022-05-02   980  
3349f5b007cd7e Tianyu Lan 2022-05-02   981  	page = swiotlb_alloc_block(parent_mem, nslabs / IO_TLB_BLOCKSIZE);
3349f5b007cd7e Tianyu Lan 2022-05-02   982  	if (!page)
3349f5b007cd7e Tianyu Lan 2022-05-02   983  		return -ENOMEM;
3349f5b007cd7e Tianyu Lan 2022-05-02   984  
3349f5b007cd7e Tianyu Lan 2022-05-02   985  	mem = kzalloc(sizeof(*mem), GFP_KERNEL);
3349f5b007cd7e Tianyu Lan 2022-05-02   986  	if (!mem)
3349f5b007cd7e Tianyu Lan 2022-05-02   987  		goto error_mem;
3349f5b007cd7e Tianyu Lan 2022-05-02   988  
3349f5b007cd7e Tianyu Lan 2022-05-02   989  	mem->slots = kzalloc(array_size(sizeof(*mem->slots), nslabs),
3349f5b007cd7e Tianyu Lan 2022-05-02   990  			     GFP_KERNEL);
3349f5b007cd7e Tianyu Lan 2022-05-02   991  	if (!mem->slots)
3349f5b007cd7e Tianyu Lan 2022-05-02   992  		goto error_slots;
3349f5b007cd7e Tianyu Lan 2022-05-02   993  
3349f5b007cd7e Tianyu Lan 2022-05-02   994  	mem->block = kcalloc(nslabs / IO_TLB_BLOCKSIZE,
3349f5b007cd7e Tianyu Lan 2022-05-02   995  				sizeof(struct io_tlb_block),
3349f5b007cd7e Tianyu Lan 2022-05-02   996  				GFP_KERNEL);
3349f5b007cd7e Tianyu Lan 2022-05-02   997  	if (!mem->block)
3349f5b007cd7e Tianyu Lan 2022-05-02   998  		goto error_block;
3349f5b007cd7e Tianyu Lan 2022-05-02   999  
3349f5b007cd7e Tianyu Lan 2022-05-02  1000  	mem->num_child = queue_num;
3349f5b007cd7e Tianyu Lan 2022-05-02  1001  	mem->child = kcalloc(queue_num,
3349f5b007cd7e Tianyu Lan 2022-05-02  1002  				sizeof(struct io_tlb_mem),
3349f5b007cd7e Tianyu Lan 2022-05-02  1003  				GFP_KERNEL);
3349f5b007cd7e Tianyu Lan 2022-05-02  1004  	if (!mem->child)
3349f5b007cd7e Tianyu Lan 2022-05-02  1005  		goto error_child;
3349f5b007cd7e Tianyu Lan 2022-05-02  1006  
3349f5b007cd7e Tianyu Lan 2022-05-02  1007  
3349f5b007cd7e Tianyu Lan 2022-05-02  1008  	swiotlb_init_io_tlb_mem(mem, page_to_phys(page), nslabs, true);
3349f5b007cd7e Tianyu Lan 2022-05-02  1009  	mem->force_bounce = true;
3349f5b007cd7e Tianyu Lan 2022-05-02  1010  	mem->for_alloc = true;
3349f5b007cd7e Tianyu Lan 2022-05-02  1011  
3349f5b007cd7e Tianyu Lan 2022-05-02  1012  	mem->vaddr = parent_mem->vaddr + page_to_phys(page) -  parent_mem->start;
3349f5b007cd7e Tianyu Lan 2022-05-02  1013  	dev->dma_io_tlb_mem->parent = parent_mem;
3349f5b007cd7e Tianyu Lan 2022-05-02  1014  	dev->dma_io_tlb_mem = mem;
3349f5b007cd7e Tianyu Lan 2022-05-02  1015  	return 0;
3349f5b007cd7e Tianyu Lan 2022-05-02  1016  
3349f5b007cd7e Tianyu Lan 2022-05-02  1017  error_child:
3349f5b007cd7e Tianyu Lan 2022-05-02  1018  	kfree(mem->block);
3349f5b007cd7e Tianyu Lan 2022-05-02  1019  error_block:
3349f5b007cd7e Tianyu Lan 2022-05-02  1020  	kfree(mem->slots);
3349f5b007cd7e Tianyu Lan 2022-05-02  1021  error_slots:
3349f5b007cd7e Tianyu Lan 2022-05-02  1022  	kfree(mem);
3349f5b007cd7e Tianyu Lan 2022-05-02  1023  error_mem:
3349f5b007cd7e Tianyu Lan 2022-05-02 @1024  	swiotlb_free_block(mem, page_to_phys(page), nslabs / IO_TLB_BLOCKSIZE);
3349f5b007cd7e Tianyu Lan 2022-05-02  1025  	return ret;
3349f5b007cd7e Tianyu Lan 2022-05-02  1026  }
3349f5b007cd7e Tianyu Lan 2022-05-02  1027  EXPORT_SYMBOL_GPL(swiotlb_device_allocate);
3349f5b007cd7e Tianyu Lan 2022-05-02  1028  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

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

* Re: [RFC PATCH V2 2/2] Swiotlb: Add device bounce buffer allocation interface
@ 2022-05-02 19:56 kernel test robot
  0 siblings, 0 replies; 20+ messages in thread
From: kernel test robot @ 2022-05-02 19:56 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 5813 bytes --]

CC: kbuild-all(a)lists.01.org
BCC: lkp(a)intel.com
In-Reply-To: <20220502125436.23607-3-ltykernel@gmail.com>
References: <20220502125436.23607-3-ltykernel@gmail.com>
TO: Tianyu Lan <ltykernel@gmail.com>

Hi Tianyu,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on next-20220429]
[cannot apply to linus/master v5.18-rc5 v5.18-rc4 v5.18-rc3 v5.18-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Tianyu-Lan/swiotlb-Add-child-io-tlb-mem-support/20220502-205700
base:    5469f0c06732a077c70a759a81f2a1f00b277694
:::::: branch date: 7 hours ago
:::::: commit date: 7 hours ago
config: i386-randconfig-c001 (https://download.01.org/0day-ci/archive/20220503/202205030322.WAeHGAi5-lkp(a)intel.com/config)
compiler: gcc-11 (Debian 11.2.0-20) 11.2.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Julia Lawall <julia.lawall@lip6.fr>


cocci warnings: (new ones prefixed by >>)
>> kernel/dma/swiotlb.c:1024:20-23: ERROR: reference preceded by free on line 1022

vim +1024 kernel/dma/swiotlb.c

3349f5b007cd7ec Tianyu Lan 2022-05-02   966  
3349f5b007cd7ec Tianyu Lan 2022-05-02   967  /*
3349f5b007cd7ec Tianyu Lan 2022-05-02   968   * swiotlb_device_allocate - Allocate bounce buffer fo device from
3349f5b007cd7ec Tianyu Lan 2022-05-02   969   * default io tlb pool. The allocation size should be aligned with
3349f5b007cd7ec Tianyu Lan 2022-05-02   970   * IO_TLB_BLOCK_UNIT.
3349f5b007cd7ec Tianyu Lan 2022-05-02   971   */
3349f5b007cd7ec Tianyu Lan 2022-05-02   972  int swiotlb_device_allocate(struct device *dev,
3349f5b007cd7ec Tianyu Lan 2022-05-02   973  			    unsigned int queue_num,
3349f5b007cd7ec Tianyu Lan 2022-05-02   974  			    unsigned long size)
3349f5b007cd7ec Tianyu Lan 2022-05-02   975  {
3349f5b007cd7ec Tianyu Lan 2022-05-02   976  	struct io_tlb_mem *mem, *parent_mem = dev->dma_io_tlb_mem;
3349f5b007cd7ec Tianyu Lan 2022-05-02   977  	unsigned long nslabs = ALIGN(size >> IO_TLB_SHIFT, IO_TLB_BLOCKSIZE);
3349f5b007cd7ec Tianyu Lan 2022-05-02   978  	struct page *page;
3349f5b007cd7ec Tianyu Lan 2022-05-02   979  	int ret = -ENOMEM;
3349f5b007cd7ec Tianyu Lan 2022-05-02   980  
3349f5b007cd7ec Tianyu Lan 2022-05-02   981  	page = swiotlb_alloc_block(parent_mem, nslabs / IO_TLB_BLOCKSIZE);
3349f5b007cd7ec Tianyu Lan 2022-05-02   982  	if (!page)
3349f5b007cd7ec Tianyu Lan 2022-05-02   983  		return -ENOMEM;
3349f5b007cd7ec Tianyu Lan 2022-05-02   984  
3349f5b007cd7ec Tianyu Lan 2022-05-02   985  	mem = kzalloc(sizeof(*mem), GFP_KERNEL);
3349f5b007cd7ec Tianyu Lan 2022-05-02   986  	if (!mem)
3349f5b007cd7ec Tianyu Lan 2022-05-02   987  		goto error_mem;
3349f5b007cd7ec Tianyu Lan 2022-05-02   988  
3349f5b007cd7ec Tianyu Lan 2022-05-02   989  	mem->slots = kzalloc(array_size(sizeof(*mem->slots), nslabs),
3349f5b007cd7ec Tianyu Lan 2022-05-02   990  			     GFP_KERNEL);
3349f5b007cd7ec Tianyu Lan 2022-05-02   991  	if (!mem->slots)
3349f5b007cd7ec Tianyu Lan 2022-05-02   992  		goto error_slots;
3349f5b007cd7ec Tianyu Lan 2022-05-02   993  
3349f5b007cd7ec Tianyu Lan 2022-05-02   994  	mem->block = kcalloc(nslabs / IO_TLB_BLOCKSIZE,
3349f5b007cd7ec Tianyu Lan 2022-05-02   995  				sizeof(struct io_tlb_block),
3349f5b007cd7ec Tianyu Lan 2022-05-02   996  				GFP_KERNEL);
3349f5b007cd7ec Tianyu Lan 2022-05-02   997  	if (!mem->block)
3349f5b007cd7ec Tianyu Lan 2022-05-02   998  		goto error_block;
3349f5b007cd7ec Tianyu Lan 2022-05-02   999  
3349f5b007cd7ec Tianyu Lan 2022-05-02  1000  	mem->num_child = queue_num;
3349f5b007cd7ec Tianyu Lan 2022-05-02  1001  	mem->child = kcalloc(queue_num,
3349f5b007cd7ec Tianyu Lan 2022-05-02  1002  				sizeof(struct io_tlb_mem),
3349f5b007cd7ec Tianyu Lan 2022-05-02  1003  				GFP_KERNEL);
3349f5b007cd7ec Tianyu Lan 2022-05-02  1004  	if (!mem->child)
3349f5b007cd7ec Tianyu Lan 2022-05-02  1005  		goto error_child;
3349f5b007cd7ec Tianyu Lan 2022-05-02  1006  
3349f5b007cd7ec Tianyu Lan 2022-05-02  1007  
3349f5b007cd7ec Tianyu Lan 2022-05-02  1008  	swiotlb_init_io_tlb_mem(mem, page_to_phys(page), nslabs, true);
3349f5b007cd7ec Tianyu Lan 2022-05-02  1009  	mem->force_bounce = true;
3349f5b007cd7ec Tianyu Lan 2022-05-02  1010  	mem->for_alloc = true;
3349f5b007cd7ec Tianyu Lan 2022-05-02  1011  
3349f5b007cd7ec Tianyu Lan 2022-05-02  1012  	mem->vaddr = parent_mem->vaddr + page_to_phys(page) -  parent_mem->start;
3349f5b007cd7ec Tianyu Lan 2022-05-02  1013  	dev->dma_io_tlb_mem->parent = parent_mem;
3349f5b007cd7ec Tianyu Lan 2022-05-02  1014  	dev->dma_io_tlb_mem = mem;
3349f5b007cd7ec Tianyu Lan 2022-05-02  1015  	return 0;
3349f5b007cd7ec Tianyu Lan 2022-05-02  1016  
3349f5b007cd7ec Tianyu Lan 2022-05-02  1017  error_child:
3349f5b007cd7ec Tianyu Lan 2022-05-02  1018  	kfree(mem->block);
3349f5b007cd7ec Tianyu Lan 2022-05-02  1019  error_block:
3349f5b007cd7ec Tianyu Lan 2022-05-02  1020  	kfree(mem->slots);
3349f5b007cd7ec Tianyu Lan 2022-05-02  1021  error_slots:
3349f5b007cd7ec Tianyu Lan 2022-05-02 @1022  	kfree(mem);
3349f5b007cd7ec Tianyu Lan 2022-05-02  1023  error_mem:
3349f5b007cd7ec Tianyu Lan 2022-05-02 @1024  	swiotlb_free_block(mem, page_to_phys(page), nslabs / IO_TLB_BLOCKSIZE);
3349f5b007cd7ec Tianyu Lan 2022-05-02  1025  	return ret;
3349f5b007cd7ec Tianyu Lan 2022-05-02  1026  }
3349f5b007cd7ec Tianyu Lan 2022-05-02  1027  EXPORT_SYMBOL_GPL(swiotlb_device_allocate);
3349f5b007cd7ec Tianyu Lan 2022-05-02  1028  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

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

end of thread, other threads:[~2022-05-31 14:13 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-02 20:36 [RFC PATCH V2 2/2] Swiotlb: Add device bounce buffer allocation interface kernel test robot
2022-05-04  8:11 ` Dan Carpenter
  -- strict thread matches above, loose matches on Subject: below --
2022-05-03 11:49 kernel test robot
2022-05-02 19:56 kernel test robot
2022-05-02 12:54 [RFC PATCH V2 0/2] swiotlb: Add child io tlb mem support Tianyu Lan
2022-05-02 12:54 ` Tianyu Lan
2022-05-02 12:54 ` [RFC PATCH V2 1/2] swiotlb: Add Child IO TLB " Tianyu Lan
2022-05-02 12:54   ` Tianyu Lan
2022-05-16  7:34   ` Christoph Hellwig
2022-05-16  7:34     ` Christoph Hellwig
2022-05-16 13:08     ` Tianyu Lan
2022-05-16 13:08       ` Tianyu Lan
2022-05-30  1:52     ` Michael Kelley (LINUX) via iommu
2022-05-31  7:16       ` hch
2022-05-31  7:16         ` hch
2022-05-31 14:13         ` Michael Kelley (LINUX) via iommu
2022-05-02 12:54 ` [RFC PATCH V2 2/2] Swiotlb: Add device bounce buffer allocation interface Tianyu Lan
2022-05-02 12:54   ` Tianyu Lan
2022-05-09 11:49 ` [RFC PATCH V2 0/2] swiotlb: Add child io tlb mem support Tianyu Lan
2022-05-09 11:49   ` Tianyu Lan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.