IOMMU Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/2] swiotlb: Cleanup and consistency fix
@ 2019-06-11 17:58 Florian Fainelli
  2019-06-11 17:58 ` [PATCH 1/2] swiotlb: Group identical cleanup in swiotlb_cleanup() Florian Fainelli
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Florian Fainelli @ 2019-06-11 17:58 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andrew Lunn, Florian Fainelli, Konrad Rzeszutek Wilk,
	open list:NETWORKING \[GENERAL\],
	Vivien Didelot, open list:DMA MAPPING HELPERS, David S. Miller,
	bcm-kernel-feedback-list, Robin Murphy, Christoph Hellwig

Hi Christoph,

Still with my contrived memory layout where there is no physical memory
the kernel can use below 4GB, it was possible to fail swiotlb_init(),
but still not hit swiotlb_map_single() since all peripherals have a
DMA_BIT_MASK() that is within the remaining addressable physical memory.

The second path could be backported to stable, but for the same reasons
as the one we had just discussed before, this requires a very contrived
test case that is not necessarily realistic or would warrant a stable
backport IMHO.

Thanks!

Florian Fainelli (2):
  swiotlb: Group identical cleanup in swiotlb_cleanup()
  swiotlb: Return consistent SWIOTLB segments/nr_tbl

 kernel/dma/swiotlb.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

-- 
2.17.1

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

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

* [PATCH 1/2] swiotlb: Group identical cleanup in swiotlb_cleanup()
  2019-06-11 17:58 [PATCH 0/2] swiotlb: Cleanup and consistency fix Florian Fainelli
@ 2019-06-11 17:58 ` Florian Fainelli
  2019-06-14  9:46   ` Christoph Hellwig
  2019-06-11 17:58 ` [PATCH 2/2] swiotlb: Return consistent SWIOTLB segments/nr_tbl Florian Fainelli
  2019-06-11 18:48 ` [PATCH 0/2] swiotlb: Cleanup and consistency fix Konrad Rzeszutek Wilk
  2 siblings, 1 reply; 6+ messages in thread
From: Florian Fainelli @ 2019-06-11 17:58 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andrew Lunn, Florian Fainelli, Konrad Rzeszutek Wilk,
	open list:NETWORKING \[GENERAL\],
	Vivien Didelot, open list:DMA MAPPING HELPERS, David S. Miller,
	bcm-kernel-feedback-list, Robin Murphy, Christoph Hellwig

Avoid repeating the zeroing of global swiotlb variables in two locations
and introduce swiotlb_cleanup() to do that.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 kernel/dma/swiotlb.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 13f0cb080a4d..b2b5c5df273c 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -317,6 +317,14 @@ swiotlb_late_init_with_default_size(size_t default_size)
 	return rc;
 }
 
+static void swiotlb_cleanup(void)
+{
+	io_tlb_end = 0;
+	io_tlb_start = 0;
+	io_tlb_nslabs = 0;
+	max_segment = 0;
+}
+
 int
 swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
 {
@@ -367,10 +375,7 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
 	                                                 sizeof(int)));
 	io_tlb_list = NULL;
 cleanup3:
-	io_tlb_end = 0;
-	io_tlb_start = 0;
-	io_tlb_nslabs = 0;
-	max_segment = 0;
+	swiotlb_cleanup();
 	return -ENOMEM;
 }
 
@@ -394,10 +399,7 @@ void __init swiotlb_exit(void)
 		memblock_free_late(io_tlb_start,
 				   PAGE_ALIGN(io_tlb_nslabs << IO_TLB_SHIFT));
 	}
-	io_tlb_start = 0;
-	io_tlb_end = 0;
-	io_tlb_nslabs = 0;
-	max_segment = 0;
+	swiotlb_cleanup();
 }
 
 /*
-- 
2.17.1

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

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

* [PATCH 2/2] swiotlb: Return consistent SWIOTLB segments/nr_tbl
  2019-06-11 17:58 [PATCH 0/2] swiotlb: Cleanup and consistency fix Florian Fainelli
  2019-06-11 17:58 ` [PATCH 1/2] swiotlb: Group identical cleanup in swiotlb_cleanup() Florian Fainelli
@ 2019-06-11 17:58 ` Florian Fainelli
  2019-06-14  9:47   ` Christoph Hellwig
  2019-06-11 18:48 ` [PATCH 0/2] swiotlb: Cleanup and consistency fix Konrad Rzeszutek Wilk
  2 siblings, 1 reply; 6+ messages in thread
From: Florian Fainelli @ 2019-06-11 17:58 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andrew Lunn, Florian Fainelli, Konrad Rzeszutek Wilk,
	open list:NETWORKING \[GENERAL\],
	Vivien Didelot, open list:DMA MAPPING HELPERS, David S. Miller,
	bcm-kernel-feedback-list, Robin Murphy, Christoph Hellwig

With a specifically contrived memory layout where there is no physical
memory available to the kernel below the 4GB boundary, we will fail to
perform the initial swiotlb_init() call and set no_iotlb_memory to true.

There are drivers out there that call into swiotlb_nr_tbl() to determine
whether they can use the SWIOTLB. With the right DMA_BIT_MASK() value
for these drivers (say 64-bit), they won't ever need to hit
swiotlb_tbl_map_single() so this can go unoticed and we would be
possibly lying about those drivers.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 kernel/dma/swiotlb.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index b2b5c5df273c..e906ef2e6315 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -129,15 +129,17 @@ setup_io_tlb_npages(char *str)
 }
 early_param("swiotlb", setup_io_tlb_npages);
 
+static bool no_iotlb_memory;
+
 unsigned long swiotlb_nr_tbl(void)
 {
-	return io_tlb_nslabs;
+	return unlikely(no_iotlb_memory) ? 0 : io_tlb_nslabs;
 }
 EXPORT_SYMBOL_GPL(swiotlb_nr_tbl);
 
 unsigned int swiotlb_max_segment(void)
 {
-	return max_segment;
+	return unlikely(no_iotlb_memory) ? 0 : max_segment;
 }
 EXPORT_SYMBOL_GPL(swiotlb_max_segment);
 
@@ -160,8 +162,6 @@ unsigned long swiotlb_size_or_default(void)
 	return size ? size : (IO_TLB_DEFAULT_SIZE);
 }
 
-static bool no_iotlb_memory;
-
 void swiotlb_print_info(void)
 {
 	unsigned long bytes = io_tlb_nslabs << IO_TLB_SHIFT;
-- 
2.17.1

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

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

* Re: [PATCH 0/2] swiotlb: Cleanup and consistency fix
  2019-06-11 17:58 [PATCH 0/2] swiotlb: Cleanup and consistency fix Florian Fainelli
  2019-06-11 17:58 ` [PATCH 1/2] swiotlb: Group identical cleanup in swiotlb_cleanup() Florian Fainelli
  2019-06-11 17:58 ` [PATCH 2/2] swiotlb: Return consistent SWIOTLB segments/nr_tbl Florian Fainelli
@ 2019-06-11 18:48 ` Konrad Rzeszutek Wilk
  2 siblings, 0 replies; 6+ messages in thread
From: Konrad Rzeszutek Wilk @ 2019-06-11 18:48 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Andrew Lunn, David S. Miller, open list:NETWORKING \[GENERAL\],
	linux-kernel, Vivien Didelot, open list:DMA MAPPING HELPERS,
	bcm-kernel-feedback-list, Robin Murphy, Christoph Hellwig

On Tue, Jun 11, 2019 at 10:58:23AM -0700, Florian Fainelli wrote:
> Hi Christoph,

I pulled the patches in my tree. 
> 
> Still with my contrived memory layout where there is no physical memory
> the kernel can use below 4GB, it was possible to fail swiotlb_init(),
> but still not hit swiotlb_map_single() since all peripherals have a
> DMA_BIT_MASK() that is within the remaining addressable physical memory.
> 
> The second path could be backported to stable, but for the same reasons
> as the one we had just discussed before, this requires a very contrived
> test case that is not necessarily realistic or would warrant a stable
> backport IMHO.
> 
> Thanks!
> 
> Florian Fainelli (2):
>   swiotlb: Group identical cleanup in swiotlb_cleanup()
>   swiotlb: Return consistent SWIOTLB segments/nr_tbl
> 
>  kernel/dma/swiotlb.c | 26 ++++++++++++++------------
>  1 file changed, 14 insertions(+), 12 deletions(-)
> 
> -- 
> 2.17.1
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [PATCH 1/2] swiotlb: Group identical cleanup in swiotlb_cleanup()
  2019-06-11 17:58 ` [PATCH 1/2] swiotlb: Group identical cleanup in swiotlb_cleanup() Florian Fainelli
@ 2019-06-14  9:46   ` Christoph Hellwig
  0 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2019-06-14  9:46 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Andrew Lunn, Konrad Rzeszutek Wilk,
	open list:NETWORKING \[GENERAL\],
	linux-kernel, Vivien Didelot, open list:DMA MAPPING HELPERS,
	David S. Miller, bcm-kernel-feedback-list, Robin Murphy,
	Christoph Hellwig

On Tue, Jun 11, 2019 at 10:58:24AM -0700, Florian Fainelli wrote:
> Avoid repeating the zeroing of global swiotlb variables in two locations
> and introduce swiotlb_cleanup() to do that.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [PATCH 2/2] swiotlb: Return consistent SWIOTLB segments/nr_tbl
  2019-06-11 17:58 ` [PATCH 2/2] swiotlb: Return consistent SWIOTLB segments/nr_tbl Florian Fainelli
@ 2019-06-14  9:47   ` Christoph Hellwig
  0 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2019-06-14  9:47 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Andrew Lunn, Konrad Rzeszutek Wilk,
	open list:NETWORKING \[GENERAL\],
	linux-kernel, Vivien Didelot, open list:DMA MAPPING HELPERS,
	David S. Miller, bcm-kernel-feedback-list, Robin Murphy,
	Christoph Hellwig

On Tue, Jun 11, 2019 at 10:58:25AM -0700, Florian Fainelli wrote:
> With a specifically contrived memory layout where there is no physical
> memory available to the kernel below the 4GB boundary, we will fail to
> perform the initial swiotlb_init() call and set no_iotlb_memory to true.
> 
> There are drivers out there that call into swiotlb_nr_tbl() to determine
> whether they can use the SWIOTLB. With the right DMA_BIT_MASK() value
> for these drivers (say 64-bit), they won't ever need to hit
> swiotlb_tbl_map_single() so this can go unoticed and we would be
> possibly lying about those drivers.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  kernel/dma/swiotlb.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index b2b5c5df273c..e906ef2e6315 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -129,15 +129,17 @@ setup_io_tlb_npages(char *str)
>  }
>  early_param("swiotlb", setup_io_tlb_npages);
>  
> +static bool no_iotlb_memory;
> +
>  unsigned long swiotlb_nr_tbl(void)
>  {
> -	return io_tlb_nslabs;
> +	return unlikely(no_iotlb_memory) ? 0 : io_tlb_nslabs;
>  }
>  EXPORT_SYMBOL_GPL(swiotlb_nr_tbl);
>  
>  unsigned int swiotlb_max_segment(void)
>  {
> -	return max_segment;
> +	return unlikely(no_iotlb_memory) ? 0 : max_segment;

I wouldn't bother with the unlikely here as anythign querying
swiotlb details should pretty much be a slow path already.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-11 17:58 [PATCH 0/2] swiotlb: Cleanup and consistency fix Florian Fainelli
2019-06-11 17:58 ` [PATCH 1/2] swiotlb: Group identical cleanup in swiotlb_cleanup() Florian Fainelli
2019-06-14  9:46   ` Christoph Hellwig
2019-06-11 17:58 ` [PATCH 2/2] swiotlb: Return consistent SWIOTLB segments/nr_tbl Florian Fainelli
2019-06-14  9:47   ` Christoph Hellwig
2019-06-11 18:48 ` [PATCH 0/2] swiotlb: Cleanup and consistency fix Konrad Rzeszutek Wilk

IOMMU Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-iommu/0 linux-iommu/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-iommu linux-iommu/ https://lore.kernel.org/linux-iommu \
		iommu@lists.linux-foundation.org iommu@archiver.kernel.org
	public-inbox-index linux-iommu


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.linux-foundation.lists.iommu


AGPL code for this site: git clone https://public-inbox.org/ public-inbox