All of lore.kernel.org
 help / color / mirror / Atom feed
From: vinod.koul@intel.com (Vinod Koul)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] dma-mapping: tidy up dma_parms default handling
Date: Mon, 22 Dec 2014 20:55:17 +0530	[thread overview]
Message-ID: <20141222152517.GM16827@intel.com> (raw)
In-Reply-To: <ffdc5455d698d2525f2d126d0adabf9418fd76b1.1418990609.git.robin.murphy@arm.com>

On Fri, Dec 19, 2014 at 05:39:09PM +0000, Robin Murphy wrote:
> Many DMA controllers and other devices set max_segment_size to
> indicate their scatter-gather capability, but have no interest in
> segment_boundary_mask. However, the existence of a dma_parms structure
> precludes the use of any default value, leaving them as zeros (assuming
> a properly kzalloc'ed structure). If a well-behaved IOMMU (or SWIOTLB)
> then tries to respect this by ensuring a mapped segment does not cross
> a zero-byte boundary, hilarity ensues.
> 
> Since zero is a nonsensical value for either parameter, treat it as an
> indicator for "default", as might be expected. In the process, clean up
> a bit by replacing the bare constants with slightly more meaningful
> macros and removing the superfluous "else" statements.
> 
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
> 
> Hi Vinod, dmaengine folks,
> 
> This isn't strictly a dmaengine patch, but get_maintainer.pl pointed at you,
> and there do happen to be more dmaengine drivers potentially affected by this
> than anything else - the current PL330 driver blows up arm64's SWIOTLB when
> running dmatest on the Juno platform, which is what brought the underlying
> problem to light.
That's due to include/linux/dma*, I will fix that up.

I think arm folks should be able to help review this... Last few commit
point to Will Decon carrying these

-- 
~Vinod

> 
>  include/linux/dma-mapping.h | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index c3007cb..99ba736 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -141,7 +141,9 @@ static inline void arch_teardown_dma_ops(struct device *dev) { }
>  
>  static inline unsigned int dma_get_max_seg_size(struct device *dev)
>  {
> -	return dev->dma_parms ? dev->dma_parms->max_segment_size : 65536;
> +	if (dev->dma_parms && dev->dma_parms->max_segment_size)
> +		return dev->dma_parms->max_segment_size;
> +	return SZ_64K;
>  }
>  
>  static inline unsigned int dma_set_max_seg_size(struct device *dev,
> @@ -150,14 +152,15 @@ static inline unsigned int dma_set_max_seg_size(struct device *dev,
>  	if (dev->dma_parms) {
>  		dev->dma_parms->max_segment_size = size;
>  		return 0;
> -	} else
> -		return -EIO;
> +	}
> +	return -EIO;
>  }
>  
>  static inline unsigned long dma_get_seg_boundary(struct device *dev)
>  {
> -	return dev->dma_parms ?
> -		dev->dma_parms->segment_boundary_mask : 0xffffffff;
> +	if (dev->dma_parms && dev->dma_parms->segment_boundary_mask)
> +		return dev->dma_parms->segment_boundary_mask;
> +	return DMA_BIT_MASK(32);
>  }
>  
>  static inline int dma_set_seg_boundary(struct device *dev, unsigned long mask)
> @@ -165,8 +168,8 @@ static inline int dma_set_seg_boundary(struct device *dev, unsigned long mask)
>  	if (dev->dma_parms) {
>  		dev->dma_parms->segment_boundary_mask = mask;
>  		return 0;
> -	} else
> -		return -EIO;
> +	}
> +	return -EIO;
>  }
>  
>  #ifndef dma_max_pfn
> -- 
> 1.9.1
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 

WARNING: multiple messages have this Message-ID (diff)
From: Vinod Koul <vinod.koul@intel.com>
To: Robin Murphy <robin.murphy@arm.com>, Will Deacon <will.deacon@arm.com>
Cc: dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, dan.j.williams@intel.com,
	lars@metafoo.de, liviu.dudau@arm.com, andrew.jackson@arm.com
Subject: Re: [PATCH] dma-mapping: tidy up dma_parms default handling
Date: Mon, 22 Dec 2014 20:55:17 +0530	[thread overview]
Message-ID: <20141222152517.GM16827@intel.com> (raw)
In-Reply-To: <ffdc5455d698d2525f2d126d0adabf9418fd76b1.1418990609.git.robin.murphy@arm.com>

On Fri, Dec 19, 2014 at 05:39:09PM +0000, Robin Murphy wrote:
> Many DMA controllers and other devices set max_segment_size to
> indicate their scatter-gather capability, but have no interest in
> segment_boundary_mask. However, the existence of a dma_parms structure
> precludes the use of any default value, leaving them as zeros (assuming
> a properly kzalloc'ed structure). If a well-behaved IOMMU (or SWIOTLB)
> then tries to respect this by ensuring a mapped segment does not cross
> a zero-byte boundary, hilarity ensues.
> 
> Since zero is a nonsensical value for either parameter, treat it as an
> indicator for "default", as might be expected. In the process, clean up
> a bit by replacing the bare constants with slightly more meaningful
> macros and removing the superfluous "else" statements.
> 
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
> 
> Hi Vinod, dmaengine folks,
> 
> This isn't strictly a dmaengine patch, but get_maintainer.pl pointed at you,
> and there do happen to be more dmaengine drivers potentially affected by this
> than anything else - the current PL330 driver blows up arm64's SWIOTLB when
> running dmatest on the Juno platform, which is what brought the underlying
> problem to light.
That's due to include/linux/dma*, I will fix that up.

I think arm folks should be able to help review this... Last few commit
point to Will Decon carrying these

-- 
~Vinod

> 
>  include/linux/dma-mapping.h | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index c3007cb..99ba736 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -141,7 +141,9 @@ static inline void arch_teardown_dma_ops(struct device *dev) { }
>  
>  static inline unsigned int dma_get_max_seg_size(struct device *dev)
>  {
> -	return dev->dma_parms ? dev->dma_parms->max_segment_size : 65536;
> +	if (dev->dma_parms && dev->dma_parms->max_segment_size)
> +		return dev->dma_parms->max_segment_size;
> +	return SZ_64K;
>  }
>  
>  static inline unsigned int dma_set_max_seg_size(struct device *dev,
> @@ -150,14 +152,15 @@ static inline unsigned int dma_set_max_seg_size(struct device *dev,
>  	if (dev->dma_parms) {
>  		dev->dma_parms->max_segment_size = size;
>  		return 0;
> -	} else
> -		return -EIO;
> +	}
> +	return -EIO;
>  }
>  
>  static inline unsigned long dma_get_seg_boundary(struct device *dev)
>  {
> -	return dev->dma_parms ?
> -		dev->dma_parms->segment_boundary_mask : 0xffffffff;
> +	if (dev->dma_parms && dev->dma_parms->segment_boundary_mask)
> +		return dev->dma_parms->segment_boundary_mask;
> +	return DMA_BIT_MASK(32);
>  }
>  
>  static inline int dma_set_seg_boundary(struct device *dev, unsigned long mask)
> @@ -165,8 +168,8 @@ static inline int dma_set_seg_boundary(struct device *dev, unsigned long mask)
>  	if (dev->dma_parms) {
>  		dev->dma_parms->segment_boundary_mask = mask;
>  		return 0;
> -	} else
> -		return -EIO;
> +	}
> +	return -EIO;
>  }
>  
>  #ifndef dma_max_pfn
> -- 
> 1.9.1
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 

  reply	other threads:[~2014-12-22 15:25 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-19 17:39 [PATCH] dma-mapping: tidy up dma_parms default handling Robin Murphy
2015-01-09 16:56 ` [PATCH RESEND] " Robin Murphy
2014-12-22 15:25 ` Vinod Koul [this message]
2014-12-22 15:25   ` [PATCH] " Vinod Koul
2015-01-09 19:45 ` [PATCH RESEND] " Arnd Bergmann
2015-01-09 19:45   ` Arnd Bergmann
2015-01-12 13:00   ` Will Deacon
2015-01-12 13:00     ` Will Deacon
2015-01-12 13:00     ` Will Deacon
2015-01-12 13:07   ` Robin Murphy
2015-01-12 13:07     ` Robin Murphy
2015-01-12 13:07     ` Robin Murphy
     [not found] ` <ffdc5455d698d2525f2d126d0adabf9418fd76b1.1418990609.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>
2015-01-13 11:19   ` Marek Szyprowski
2015-01-13 11:19     ` Marek Szyprowski
2015-01-13 11:19     ` Marek Szyprowski
2015-01-22  3:45     ` Sumit Semwal
2015-01-22  3:45       ` Sumit Semwal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141222152517.GM16827@intel.com \
    --to=vinod.koul@intel.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.