[PATCH] swiotlb: swiotlb_{alloc, free}_buffer should depend on CONFIG_DMA_DIRECT_OPS

[PATCH] swiotlb: swiotlb_{alloc, free}_buffer should depend on CONFIG_DMA_DIRECT_OPS

[Christoph Hellwig]

Otherwise we might get unused symbol warnings for configs that built
swiotlb.c only for use by xen-swiotlb.c and that don't otherwise select
CONFIG_DMA_DIRECT_OPS, which is possible on arm.

Fixes: 16e73adbca76 ("dma/swiotlb: Remove swiotlb_{alloc,free}_coherent()")
Reported-by: Stephen Rothwell <sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org>
Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
---
 lib/swiotlb.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 15954b86f09e..47aeb04c1997 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -692,6 +692,7 @@ void swiotlb_tbl_sync_single(struct device *hwdev, phys_addr_t tlb_addr,
 	}
 }
 
+#ifdef CONFIG_DMA_DIRECT_OPS
 static inline bool dma_coherent_ok(struct device *dev, dma_addr_t addr,
 		size_t size)
 {
@@ -763,6 +764,7 @@ static bool swiotlb_free_buffer(struct device *dev, size_t size,
 				 DMA_ATTR_SKIP_CPU_SYNC);
 	return true;
 }
+#endif
 
 static void
 swiotlb_full(struct device *dev, size_t size, enum dma_data_direction dir,
-- 
2.14.2

Re: [PATCH] swiotlb: swiotlb_{alloc,free}_buffer should depend on CONFIG_DMA_DIRECT_OPS

[Konrad Rzeszutek Wilk]

On Fri, Mar 23, 2018 at 06:49:30PM +0100, Christoph Hellwig wrote:
> Otherwise we might get unused symbol warnings for configs that built
> swiotlb.c only for use by xen-swiotlb.c and that don't otherwise select
> CONFIG_DMA_DIRECT_OPS, which is possible on arm.
> 
> Fixes: 16e73adbca76 ("dma/swiotlb: Remove swiotlb_{alloc,free}_coherent()")
> Reported-by: Stephen Rothwell <sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org>
> Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>


Alternatively could we set the Kconfig to slect DMA_DIRECT_OPS?

> ---
>  lib/swiotlb.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/swiotlb.c b/lib/swiotlb.c
> index 15954b86f09e..47aeb04c1997 100644
> --- a/lib/swiotlb.c
> +++ b/lib/swiotlb.c
> @@ -692,6 +692,7 @@ void swiotlb_tbl_sync_single(struct device *hwdev, phys_addr_t tlb_addr,
>  	}
>  }
>  
> +#ifdef CONFIG_DMA_DIRECT_OPS
>  static inline bool dma_coherent_ok(struct device *dev, dma_addr_t addr,
>  		size_t size)
>  {
> @@ -763,6 +764,7 @@ static bool swiotlb_free_buffer(struct device *dev, size_t size,
>  				 DMA_ATTR_SKIP_CPU_SYNC);
>  	return true;
>  }
> +#endif
>  
>  static void
>  swiotlb_full(struct device *dev, size_t size, enum dma_data_direction dir,
> -- 
> 2.14.2
> 

Re: [PATCH] swiotlb: swiotlb_{alloc,free}_buffer should depend on CONFIG_DMA_DIRECT_OPS

[Thomas Gleixner]

On Fri, 23 Mar 2018, Konrad Rzeszutek Wilk wrote:

> On Fri, Mar 23, 2018 at 06:49:30PM +0100, Christoph Hellwig wrote:
> > Otherwise we might get unused symbol warnings for configs that built
> > swiotlb.c only for use by xen-swiotlb.c and that don't otherwise select
> > CONFIG_DMA_DIRECT_OPS, which is possible on arm.
> > 
> > Fixes: 16e73adbca76 ("dma/swiotlb: Remove swiotlb_{alloc,free}_coherent()")
> > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> 
> Alternatively could we set the Kconfig to slect DMA_DIRECT_OPS?

You only want to do that when you actually need the code. If not it's a
pointless exercise.

But Christoph change makes sense independent of that because the next
oddball Kconfig will come along sooner than later and run into the very
same problem.

Thanks,

	tglx

Re: [PATCH] swiotlb: swiotlb_{alloc,free}_buffer should depend on CONFIG_DMA_DIRECT_OPS

[Christoph Hellwig]

On Fri, Mar 23, 2018 at 02:57:07PM -0400, Konrad Rzeszutek Wilk wrote:
> On Fri, Mar 23, 2018 at 06:49:30PM +0100, Christoph Hellwig wrote:
> > Otherwise we might get unused symbol warnings for configs that built
> > swiotlb.c only for use by xen-swiotlb.c and that don't otherwise select
> > CONFIG_DMA_DIRECT_OPS, which is possible on arm.
> > 
> > Fixes: 16e73adbca76 ("dma/swiotlb: Remove swiotlb_{alloc,free}_coherent()")
> > Reported-by: Stephen Rothwell <sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org>
> > Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
> 
> 
> Alternatively could we set the Kconfig to slect DMA_DIRECT_OPS?


IFF we build swiotlb.c only for xen-swiotlb we don't need DMA_DIRECT_OPS.
IF you are fine with just requring it as well that would be doable nicely
only if we consolidate to a single defintion of CONFIG_SWIOTLB instead
of one per architecture.  Which is something we should do anyway,
so I'll look into it once I get some time.

Re: [PATCH] swiotlb: swiotlb_{alloc,free}_buffer should depend on CONFIG_DMA_DIRECT_OPS

[Konrad Rzeszutek Wilk]

On Sat, Mar 24, 2018 at 06:03:51PM +0100, Christoph Hellwig wrote:
> On Fri, Mar 23, 2018 at 02:57:07PM -0400, Konrad Rzeszutek Wilk wrote:
> > On Fri, Mar 23, 2018 at 06:49:30PM +0100, Christoph Hellwig wrote:
> > > Otherwise we might get unused symbol warnings for configs that built
> > > swiotlb.c only for use by xen-swiotlb.c and that don't otherwise select
> > > CONFIG_DMA_DIRECT_OPS, which is possible on arm.
> > > 
> > > Fixes: 16e73adbca76 ("dma/swiotlb: Remove swiotlb_{alloc,free}_coherent()")
> > > Reported-by: Stephen Rothwell <sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org>
> > > Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
> > 
> > 
> > Alternatively could we set the Kconfig to slect DMA_DIRECT_OPS?
> 
> 
> IFF we build swiotlb.c only for xen-swiotlb we don't need DMA_DIRECT_OPS.

I don't think there is ever an case where you want a Xen specific build.

> IF you are fine with just requring it as well that would be doable nicely
> only if we consolidate to a single defintion of CONFIG_SWIOTLB instead
> of one per architecture.  Which is something we should do anyway,
> so I'll look into it once I get some time.

Sounds good. Thank you.

Re: [PATCH] swiotlb: swiotlb_{alloc,free}_buffer should depend on CONFIG_DMA_DIRECT_OPS

[Christoph Hellwig]

On Sat, Mar 24, 2018 at 04:05:45PM -0400, Konrad Rzeszutek Wilk wrote:
> > > > Otherwise we might get unused symbol warnings for configs that built
> > > > swiotlb.c only for use by xen-swiotlb.c and that don't otherwise select
> > > > CONFIG_DMA_DIRECT_OPS, which is possible on arm.
> > > > 
> > > > Fixes: 16e73adbca76 ("dma/swiotlb: Remove swiotlb_{alloc,free}_coherent()")
> > > > Reported-by: Stephen Rothwell <sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org>
> > > > Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
> > > 
> > > 
> > > Alternatively could we set the Kconfig to slect DMA_DIRECT_OPS?
> > 
> > 
> > IFF we build swiotlb.c only for xen-swiotlb we don't need DMA_DIRECT_OPS.
> 
> I don't think there is ever an case where you want a Xen specific build.

arm never uses swiotlb directly, so any swiotlb.c build on arm is purely
for xen-swiotlb.c