Re: [PATCH 31/33] dma-direct: reject too small dma masks

[Robin Murphy]

On 10/01/18 15:32, Christoph Hellwig wrote:

On Wed, Jan 10, 2018 at 11:49:34AM +, Robin Murphy wrote:

+#ifdef CONFIG_ZONE_DMA
+   if (mask < DMA_BIT_MASK(ARCH_ZONE_DMA_BITS))
+   return 0;
+#else
+   /*
+* Because 32-bit DMA masks are so common we expect every architecture
+* to be able to satisfy them - either by not supporting more physical
+* memory, or by providing a ZONE_DMA32.  If neither is the case, the
+* architecture needs to use an IOMMU instead of the direct mapping.
+*/
+   if (mask < DMA_BIT_MASK(32))
+   return 0;


Do you think it's worth the effort to be a little more accommodating here?
i.e.:

return dma_max_pfn(dev) >= max_pfn;

We seem to have a fair few 28-31 bit masks for older hardware which
probably associates with host systems packing equivalently small amounts of
RAM.


And those devices don't have a ZONE_DMA?  I think we could do something
like that, but I'd rather have it as a separate commit with a good
explanation.  Maybe you can just send on on top of the series?


Good point - other than the IXP4xx platform and possibly the Broadcom 
network drivers, it's probably only x86-relevant stuff where the concern 
is moot. Let's just keep the simple assumption then, until actually 
proven otherwise.


Robin.

Re: [PATCH 31/33] dma-direct: reject too small dma masks

[Robin Murphy]

On 10/01/18 15:32, Christoph Hellwig wrote:

On Wed, Jan 10, 2018 at 11:49:34AM +, Robin Murphy wrote:

+#ifdef CONFIG_ZONE_DMA
+   if (mask < DMA_BIT_MASK(ARCH_ZONE_DMA_BITS))
+   return 0;
+#else
+   /*
+* Because 32-bit DMA masks are so common we expect every architecture
+* to be able to satisfy them - either by not supporting more physical
+* memory, or by providing a ZONE_DMA32.  If neither is the case, the
+* architecture needs to use an IOMMU instead of the direct mapping.
+*/
+   if (mask < DMA_BIT_MASK(32))
+   return 0;


Do you think it's worth the effort to be a little more accommodating here?
i.e.:

return dma_max_pfn(dev) >= max_pfn;

We seem to have a fair few 28-31 bit masks for older hardware which
probably associates with host systems packing equivalently small amounts of
RAM.


And those devices don't have a ZONE_DMA?  I think we could do something
like that, but I'd rather have it as a separate commit with a good
explanation.  Maybe you can just send on on top of the series?


Good point - other than the IXP4xx platform and possibly the Broadcom 
network drivers, it's probably only x86-relevant stuff where the concern 
is moot. Let's just keep the simple assumption then, until actually 
proven otherwise.


Robin.

Re: [PATCH 31/33] dma-direct: reject too small dma masks

[Christoph Hellwig]

On Wed, Jan 10, 2018 at 11:49:34AM +, Robin Murphy wrote:
>> +#ifdef CONFIG_ZONE_DMA
>> +if (mask < DMA_BIT_MASK(ARCH_ZONE_DMA_BITS))
>> +return 0;
>> +#else
>> +/*
>> + * Because 32-bit DMA masks are so common we expect every architecture
>> + * to be able to satisfy them - either by not supporting more physical
>> + * memory, or by providing a ZONE_DMA32.  If neither is the case, the
>> + * architecture needs to use an IOMMU instead of the direct mapping.
>> + */
>> +if (mask < DMA_BIT_MASK(32))
>> +return 0;
>
> Do you think it's worth the effort to be a little more accommodating here? 
> i.e.:
>
>   return dma_max_pfn(dev) >= max_pfn;
>
> We seem to have a fair few 28-31 bit masks for older hardware which 
> probably associates with host systems packing equivalently small amounts of 
> RAM.

And those devices don't have a ZONE_DMA?  I think we could do something
like that, but I'd rather have it as a separate commit with a good
explanation.  Maybe you can just send on on top of the series?

Re: [PATCH 31/33] dma-direct: reject too small dma masks

[Christoph Hellwig]

On Wed, Jan 10, 2018 at 11:49:34AM +, Robin Murphy wrote:
>> +#ifdef CONFIG_ZONE_DMA
>> +if (mask < DMA_BIT_MASK(ARCH_ZONE_DMA_BITS))
>> +return 0;
>> +#else
>> +/*
>> + * Because 32-bit DMA masks are so common we expect every architecture
>> + * to be able to satisfy them - either by not supporting more physical
>> + * memory, or by providing a ZONE_DMA32.  If neither is the case, the
>> + * architecture needs to use an IOMMU instead of the direct mapping.
>> + */
>> +if (mask < DMA_BIT_MASK(32))
>> +return 0;
>
> Do you think it's worth the effort to be a little more accommodating here? 
> i.e.:
>
>   return dma_max_pfn(dev) >= max_pfn;
>
> We seem to have a fair few 28-31 bit masks for older hardware which 
> probably associates with host systems packing equivalently small amounts of 
> RAM.

And those devices don't have a ZONE_DMA?  I think we could do something
like that, but I'd rather have it as a separate commit with a good
explanation.  Maybe you can just send on on top of the series?

Re: [PATCH 31/33] dma-direct: reject too small dma masks

[Robin Murphy]

On 10/01/18 08:00, Christoph Hellwig wrote:

Signed-off-by: Christoph Hellwig 
---
  include/linux/dma-direct.h |  1 +
  lib/dma-direct.c   | 19 +++
  2 files changed, 20 insertions(+)

diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
index 4788bf0bf683..bcdb1a3e4b1f 100644
--- a/include/linux/dma-direct.h
+++ b/include/linux/dma-direct.h
@@ -42,5 +42,6 @@ void *dma_direct_alloc(struct device *dev, size_t size, 
dma_addr_t *dma_handle,
gfp_t gfp, unsigned long attrs);
  void dma_direct_free(struct device *dev, size_t size, void *cpu_addr,
dma_addr_t dma_addr, unsigned long attrs);
+int dma_direct_supported(struct device *dev, u64 mask);
  
  #endif /* _LINUX_DMA_DIRECT_H */

diff --git a/lib/dma-direct.c b/lib/dma-direct.c
index 784a68dfdbe3..40b1f92f2214 100644
--- a/lib/dma-direct.c
+++ b/lib/dma-direct.c
@@ -122,6 +122,24 @@ static int dma_direct_map_sg(struct device *dev, struct 
scatterlist *sgl,
return nents;
  }
  
+int dma_direct_supported(struct device *dev, u64 mask)

+{
+#ifdef CONFIG_ZONE_DMA
+   if (mask < DMA_BIT_MASK(ARCH_ZONE_DMA_BITS))
+   return 0;
+#else
+   /*
+* Because 32-bit DMA masks are so common we expect every architecture
+* to be able to satisfy them - either by not supporting more physical
+* memory, or by providing a ZONE_DMA32.  If neither is the case, the
+* architecture needs to use an IOMMU instead of the direct mapping.
+*/
+   if (mask < DMA_BIT_MASK(32))
+   return 0;


Do you think it's worth the effort to be a little more accommodating 
here? i.e.:


return dma_max_pfn(dev) >= max_pfn;

We seem to have a fair few 28-31 bit masks for older hardware which 
probably associates with host systems packing equivalently small amounts 
of RAM.


Otherwise though,

Reviewed-by: Robin Murphy 

Robin.


+#endif
+   return 1;
+}
+
  static int dma_direct_mapping_error(struct device *dev, dma_addr_t dma_addr)
  {
return dma_addr == DIRECT_MAPPING_ERROR;
@@ -132,6 +150,7 @@ const struct dma_map_ops dma_direct_ops = {
.free   = dma_direct_free,
.map_page   = dma_direct_map_page,
.map_sg = dma_direct_map_sg,
+   .dma_supported  = dma_direct_supported,
.mapping_error  = dma_direct_mapping_error,
  };
  EXPORT_SYMBOL(dma_direct_ops);

Re: [PATCH 31/33] dma-direct: reject too small dma masks

[Robin Murphy]

On 10/01/18 08:00, Christoph Hellwig wrote:

Signed-off-by: Christoph Hellwig 
---
  include/linux/dma-direct.h |  1 +
  lib/dma-direct.c   | 19 +++
  2 files changed, 20 insertions(+)

diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
index 4788bf0bf683..bcdb1a3e4b1f 100644
--- a/include/linux/dma-direct.h
+++ b/include/linux/dma-direct.h
@@ -42,5 +42,6 @@ void *dma_direct_alloc(struct device *dev, size_t size, 
dma_addr_t *dma_handle,
gfp_t gfp, unsigned long attrs);
  void dma_direct_free(struct device *dev, size_t size, void *cpu_addr,
dma_addr_t dma_addr, unsigned long attrs);
+int dma_direct_supported(struct device *dev, u64 mask);
  
  #endif /* _LINUX_DMA_DIRECT_H */

diff --git a/lib/dma-direct.c b/lib/dma-direct.c
index 784a68dfdbe3..40b1f92f2214 100644
--- a/lib/dma-direct.c
+++ b/lib/dma-direct.c
@@ -122,6 +122,24 @@ static int dma_direct_map_sg(struct device *dev, struct 
scatterlist *sgl,
return nents;
  }
  
+int dma_direct_supported(struct device *dev, u64 mask)

+{
+#ifdef CONFIG_ZONE_DMA
+   if (mask < DMA_BIT_MASK(ARCH_ZONE_DMA_BITS))
+   return 0;
+#else
+   /*
+* Because 32-bit DMA masks are so common we expect every architecture
+* to be able to satisfy them - either by not supporting more physical
+* memory, or by providing a ZONE_DMA32.  If neither is the case, the
+* architecture needs to use an IOMMU instead of the direct mapping.
+*/
+   if (mask < DMA_BIT_MASK(32))
+   return 0;


Do you think it's worth the effort to be a little more accommodating 
here? i.e.:


return dma_max_pfn(dev) >= max_pfn;

We seem to have a fair few 28-31 bit masks for older hardware which 
probably associates with host systems packing equivalently small amounts 
of RAM.


Otherwise though,

Reviewed-by: Robin Murphy 

Robin.


+#endif
+   return 1;
+}
+
  static int dma_direct_mapping_error(struct device *dev, dma_addr_t dma_addr)
  {
return dma_addr == DIRECT_MAPPING_ERROR;
@@ -132,6 +150,7 @@ const struct dma_map_ops dma_direct_ops = {
.free   = dma_direct_free,
.map_page   = dma_direct_map_page,
.map_sg = dma_direct_map_sg,
+   .dma_supported  = dma_direct_supported,
.mapping_error  = dma_direct_mapping_error,
  };
  EXPORT_SYMBOL(dma_direct_ops);