allow larger than require DMA masks

allow larger than require DMA masks

[Christoph Hellwig]

Hi all,

this series finishes off converting our dma mask model to split between
device capabilities (dev->dma_mask and dev->coherent_dma_mask) and system
limitations (dev->bus_dma_mask).  We already accept larger than required
masks in most dma_map_ops implementation, in case of x86 and
implementations based on it since the dawn of time.  Only one parisc
and two sparc64 instances failed larger than required DMA masks, and
this series fixes that up and updates the documentation that devices
don't need to handle DMA mask fallbacks.

[PATCH 1/5] ccio: allow large DMA masks

[Christoph Hellwig]

There is no harm in setting a 64-bit mask even if we don't need it,
and the current ccio code is one of the few place in the kernel
still rejecting larger than required DMA masks.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/parisc/ccio-dma.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/parisc/ccio-dma.c b/drivers/parisc/ccio-dma.c
index 8d2fc84119c6..24a68fb7b2f9 100644
--- a/drivers/parisc/ccio-dma.c
+++ b/drivers/parisc/ccio-dma.c
@@ -710,8 +710,8 @@ ccio_dma_supported(struct device *dev, u64 mask)
 		return 0;
 	}
 
-	/* only support 32-bit devices (ie PCI/GSC) */
-	return (int)(mask == 0xffffffffUL);
+	/* only support 32-bit or better devices (ie PCI/GSC) */
+	return (int)(mask >= 0xffffffffUL);
 }
 
 /**
-- 
2.20.1

[PATCH 2/5] sparc64: refactor the ali DMA quirk

[Christoph Hellwig]

Do the quirk first in the dma_supported routines, as we don't need
any of the other checks for it, and remove the duplicate mask checking
that is already done by the callers.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/sparc/kernel/iommu.c     |  7 +++---
 arch/sparc/kernel/kernel.h    |  6 ++++-
 arch/sparc/kernel/pci.c       | 46 ++++++++++++-----------------------
 arch/sparc/kernel/pci_sun4v.c |  5 +++-
 4 files changed, 27 insertions(+), 37 deletions(-)

diff --git a/arch/sparc/kernel/iommu.c b/arch/sparc/kernel/iommu.c
index b1a09080e8da..0c253f1c852e 100644
--- a/arch/sparc/kernel/iommu.c
+++ b/arch/sparc/kernel/iommu.c
@@ -745,14 +745,13 @@ static int dma_4u_supported(struct device *dev, u64 device_mask)
 {
 	struct iommu *iommu = dev->archdata.iommu;
 
+	if (ali_sound_dma_hack(dev, device_mask))
+		return 1;
+
 	if (device_mask > DMA_BIT_MASK(32))
 		return 0;
 	if ((device_mask & iommu->dma_addr_mask) == iommu->dma_addr_mask)
 		return 1;
-#ifdef CONFIG_PCI
-	if (dev_is_pci(dev))
-		return pci64_dma_supported(to_pci_dev(dev), device_mask);
-#endif
 	return 0;
 }
 
diff --git a/arch/sparc/kernel/kernel.h b/arch/sparc/kernel/kernel.h
index ddffd368e057..f6f498ba3198 100644
--- a/arch/sparc/kernel/kernel.h
+++ b/arch/sparc/kernel/kernel.h
@@ -45,7 +45,11 @@ void __irq_entry smp_receive_signal_client(int irq, struct pt_regs *regs);
 void __irq_entry smp_kgdb_capture_client(int irq, struct pt_regs *regs);
 
 /* pci.c */
-int pci64_dma_supported(struct pci_dev *pdev, u64 device_mask);
+#ifdef CONFIG_PCI
+int ali_sound_dma_hack(struct device *dev, u64 device_mask);
+#else
+#define ali_sound_dma_hack(dev, mask)	(0)
+#endif
 
 /* signal32.c */
 void do_sigreturn32(struct pt_regs *regs);
diff --git a/arch/sparc/kernel/pci.c b/arch/sparc/kernel/pci.c
index bcfec6a85d23..5ed43828e078 100644
--- a/arch/sparc/kernel/pci.c
+++ b/arch/sparc/kernel/pci.c
@@ -956,51 +956,35 @@ void arch_teardown_msi_irq(unsigned int irq)
 }
 #endif /* !(CONFIG_PCI_MSI) */
 
-static void ali_sound_dma_hack(struct pci_dev *pdev, int set_bit)
+/* ALI sound chips generate 31-bits of DMA, a special register
+ * determines what bit 31 is emitted as.
+ */
+int ali_sound_dma_hack(struct device *dev, u64 device_mask)
 {
+	struct iommu *iommu = dev->archdata.iommu;
 	struct pci_dev *ali_isa_bridge;
 	u8 val;
 
-	/* ALI sound chips generate 31-bits of DMA, a special register
-	 * determines what bit 31 is emitted as.
-	 */
+	if (!dev_is_pci(dev))
+		return 0;
+
+	if (to_pci_dev(dev)->vendor != PCI_VENDOR_ID_AL ||
+	    to_pci_dev(dev)->device != PCI_DEVICE_ID_AL_M5451 ||
+	    device_mask != 0x7fffffff)
+		return 0;
+
 	ali_isa_bridge = pci_get_device(PCI_VENDOR_ID_AL,
 					 PCI_DEVICE_ID_AL_M1533,
 					 NULL);
 
 	pci_read_config_byte(ali_isa_bridge, 0x7e, &val);
-	if (set_bit)
+	if (iommu->dma_addr_mask & 0x80000000)
 		val |= 0x01;
 	else
 		val &= ~0x01;
 	pci_write_config_byte(ali_isa_bridge, 0x7e, val);
 	pci_dev_put(ali_isa_bridge);
-}
-
-int pci64_dma_supported(struct pci_dev *pdev, u64 device_mask)
-{
-	u64 dma_addr_mask;
-
-	if (pdev == NULL) {
-		dma_addr_mask = 0xffffffff;
-	} else {
-		struct iommu *iommu = pdev->dev.archdata.iommu;
-
-		dma_addr_mask = iommu->dma_addr_mask;
-
-		if (pdev->vendor == PCI_VENDOR_ID_AL &&
-		    pdev->device == PCI_DEVICE_ID_AL_M5451 &&
-		    device_mask == 0x7fffffff) {
-			ali_sound_dma_hack(pdev,
-					   (dma_addr_mask & 0x80000000) != 0);
-			return 1;
-		}
-	}
-
-	if (device_mask >= (1UL << 32UL))
-		return 0;
-
-	return (device_mask & dma_addr_mask) == dma_addr_mask;
+	return 1;
 }
 
 void pci_resource_to_user(const struct pci_dev *pdev, int bar,
diff --git a/arch/sparc/kernel/pci_sun4v.c b/arch/sparc/kernel/pci_sun4v.c
index fa0e42b4cbfb..d30eb22b6e11 100644
--- a/arch/sparc/kernel/pci_sun4v.c
+++ b/arch/sparc/kernel/pci_sun4v.c
@@ -676,6 +676,9 @@ static int dma_4v_supported(struct device *dev, u64 device_mask)
 	struct iommu *iommu = dev->archdata.iommu;
 	u64 dma_addr_mask = iommu->dma_addr_mask;
 
+	if (ali_sound_dma_hack(dev, device_mask))
+		return 1;
+
 	if (device_mask > DMA_BIT_MASK(32)) {
 		if (iommu->atu)
 			dma_addr_mask = iommu->atu->dma_addr_mask;
@@ -685,7 +688,7 @@ static int dma_4v_supported(struct device *dev, u64 device_mask)
 
 	if ((device_mask & dma_addr_mask) == dma_addr_mask)
 		return 1;
-	return pci64_dma_supported(to_pci_dev(dev), device_mask);
+	return 0;
 }
 
 static const struct dma_map_ops sun4v_dma_ops = {
-- 
2.20.1

Re: [PATCH 2/5] sparc64: refactor the ali DMA quirk

[David Miller]

From: Christoph Hellwig <hch@lst.de>
Date: Fri, 15 Feb 2019 15:45:56 +0100

> Do the quirk first in the dma_supported routines, as we don't need
> any of the other checks for it, and remove the duplicate mask checking
> that is already done by the callers.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Acked-by: David S. Miller <davem@davemloft.net>

[PATCH 3/5] sparc64/iommu: allow large DMA masks

[Christoph Hellwig]

We've been moving to a model where the device just sets the DMA mask
supported by it, instead of having to fallback to something it thinks
the platform might support.  Sparc64 is the remaining holdout forcing
drivers to supply a matching mask.  Change dma_4u_supported to just
check if the supplied dma mask is large enough as nothing in the
iommu.c code (or the core DMA code) actually looks at the DMA mask
later on.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/sparc/kernel/iommu.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/sparc/kernel/iommu.c b/arch/sparc/kernel/iommu.c
index 0c253f1c852e..4ae7388b1bff 100644
--- a/arch/sparc/kernel/iommu.c
+++ b/arch/sparc/kernel/iommu.c
@@ -748,11 +748,9 @@ static int dma_4u_supported(struct device *dev, u64 device_mask)
 	if (ali_sound_dma_hack(dev, device_mask))
 		return 1;
 
-	if (device_mask > DMA_BIT_MASK(32))
+	if (device_mask < iommu->dma_addr_mask)
 		return 0;
-	if ((device_mask & iommu->dma_addr_mask) == iommu->dma_addr_mask)
-		return 1;
-	return 0;
+	return 1;
 }
 
 static const struct dma_map_ops sun4u_dma_ops = {
-- 
2.20.1

Re: [PATCH 3/5] sparc64/iommu: allow large DMA masks

[David Miller]

From: Christoph Hellwig <hch@lst.de>
Date: Fri, 15 Feb 2019 15:45:57 +0100

> We've been moving to a model where the device just sets the DMA mask
> supported by it, instead of having to fallback to something it thinks
> the platform might support.  Sparc64 is the remaining holdout forcing
> drivers to supply a matching mask.  Change dma_4u_supported to just
> check if the supplied dma mask is large enough as nothing in the
> iommu.c code (or the core DMA code) actually looks at the DMA mask
> later on.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Acked-by: David S. Miller <davem@davemloft.net>

[PATCH 4/5] sparc64/pci_sun4v: allow large DMA masks

[Christoph Hellwig]

We've been moving to a model where the device just sets the DMA mask
supported by it, instead of having to fallback to something it thinks
the platform might support.  Sparc64 is the remaining holdout forcing
drivers to supply a matching mask.  Change dma_4v_supported to just
check if the supplied dma mask is large enough. and adjust the mapping
code to check ATU presence in addition to the DMA mask to decice on
the mapping method.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/sparc/kernel/pci_sun4v.c | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/arch/sparc/kernel/pci_sun4v.c b/arch/sparc/kernel/pci_sun4v.c
index d30eb22b6e11..a8af6023c126 100644
--- a/arch/sparc/kernel/pci_sun4v.c
+++ b/arch/sparc/kernel/pci_sun4v.c
@@ -92,7 +92,7 @@ static long iommu_batch_flush(struct iommu_batch *p, u64 mask)
 		prot &= (HV_PCI_MAP_ATTR_READ | HV_PCI_MAP_ATTR_WRITE);
 
 	while (npages != 0) {
-		if (mask <= DMA_BIT_MASK(32)) {
+		if (mask <= DMA_BIT_MASK(32) || !pbm->iommu->atu) {
 			num = pci_sun4v_iommu_map(devhandle,
 						  HV_PCI_TSBID(0, entry),
 						  npages,
@@ -208,7 +208,7 @@ static void *dma_4v_alloc_coherent(struct device *dev, size_t size,
 	atu = iommu->atu;
 
 	mask = dev->coherent_dma_mask;
-	if (mask <= DMA_BIT_MASK(32))
+	if (mask <= DMA_BIT_MASK(32) || !atu)
 		tbl = &iommu->tbl;
 	else
 		tbl = &atu->tbl;
@@ -674,21 +674,12 @@ static void dma_4v_unmap_sg(struct device *dev, struct scatterlist *sglist,
 static int dma_4v_supported(struct device *dev, u64 device_mask)
 {
 	struct iommu *iommu = dev->archdata.iommu;
-	u64 dma_addr_mask = iommu->dma_addr_mask;
 
 	if (ali_sound_dma_hack(dev, device_mask))
 		return 1;
-
-	if (device_mask > DMA_BIT_MASK(32)) {
-		if (iommu->atu)
-			dma_addr_mask = iommu->atu->dma_addr_mask;
-		else
-			return 0;
-	}
-
-	if ((device_mask & dma_addr_mask) == dma_addr_mask)
-		return 1;
-	return 0;
+	if (device_mask < iommu->dma_addr_mask)
+		return 0;
+	return 1;
 }
 
 static const struct dma_map_ops sun4v_dma_ops = {
-- 
2.20.1

Re: [PATCH 4/5] sparc64/pci_sun4v: allow large DMA masks

[David Miller]

From: Christoph Hellwig <hch@lst.de>
Date: Fri, 15 Feb 2019 15:45:58 +0100

> We've been moving to a model where the device just sets the DMA mask
> supported by it, instead of having to fallback to something it thinks
> the platform might support.  Sparc64 is the remaining holdout forcing
> drivers to supply a matching mask.  Change dma_4v_supported to just
> check if the supplied dma mask is large enough. and adjust the mapping
> code to check ATU presence in addition to the DMA mask to decice on
                                                            ^^^^^^

"decide"

> the mapping method.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Acked-by: David S. Miller <davem@davemloft.net>

[PATCH 5/5] Documentation/DMA-API-HOWTO: update dma_mask sections

[Christoph Hellwig]

We don't require drivers to guess a DMA mask that might actually
match the system capabilities any more, so fix up the documentation
to clear this up.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 Documentation/DMA-API-HOWTO.txt | 121 +++++++++++---------------------
 1 file changed, 41 insertions(+), 80 deletions(-)

diff --git a/Documentation/DMA-API-HOWTO.txt b/Documentation/DMA-API-HOWTO.txt
index f0cc3f772265..8e948fae34af 100644
--- a/Documentation/DMA-API-HOWTO.txt
+++ b/Documentation/DMA-API-HOWTO.txt
@@ -146,114 +146,75 @@ What about block I/O and networking buffers?  The block I/O and
 networking subsystems make sure that the buffers they use are valid
 for you to DMA from/to.
 
-DMA addressing limitations
+DMA addressing capabilities
 ==========================
 
-Does your device have any DMA addressing limitations?  For example, is
-your device only capable of driving the low order 24-bits of address?
-If so, you need to inform the kernel of this fact.
+By default, the kernel assumes that your device can address 32-bits of DMA
+addressing.  For a 64-bit capable device, this needs to be increased, and for
+a device with limitations, it needs to be decreased.
 
-By default, the kernel assumes that your device can address the full
-32-bits.  For a 64-bit capable device, this needs to be increased.
-And for a device with limitations, as discussed in the previous
-paragraph, it needs to be decreased.
+Special note about PCI: PCI-X specification requires PCI-X devices to support
+64-bit addressing (DAC) for all transactions.  And at least one platform (SGI
+SN2) requires 64-bit consistent allocations to operate correctly when the IO
+bus is in PCI-X mode.
 
-Special note about PCI: PCI-X specification requires PCI-X devices to
-support 64-bit addressing (DAC) for all transactions.  And at least
-one platform (SGI SN2) requires 64-bit consistent allocations to
-operate correctly when the IO bus is in PCI-X mode.
+For correct operation, you must set the DMA mask to inform the kernel about
+your devices DMA addressing capabilities.
 
-For correct operation, you must interrogate the kernel in your device
-probe routine to see if the DMA controller on the machine can properly
-support the DMA addressing limitation your device has.  It is good
-style to do this even if your device holds the default setting,
-because this shows that you did think about these issues wrt. your
-device.
-
-The query is performed via a call to dma_set_mask_and_coherent()::
+This is performed via a call to dma_set_mask_and_coherent()::
 
 	int dma_set_mask_and_coherent(struct device *dev, u64 mask);
 
-which will query the mask for both streaming and coherent APIs together.
-If you have some special requirements, then the following two separate
-queries can be used instead:
+which will set the mask for both streaming and coherent APIs together.  If you
+have some special requirements, then the following two separate calls can be
+used instead:
 
-	The query for streaming mappings is performed via a call to
+	The setup for streaming mappings is performed via a call to
 	dma_set_mask()::
 
 		int dma_set_mask(struct device *dev, u64 mask);
 
-	The query for consistent allocations is performed via a call
+	The setup for consistent allocations is performed via a call
 	to dma_set_coherent_mask()::
 
 		int dma_set_coherent_mask(struct device *dev, u64 mask);
 
-Here, dev is a pointer to the device struct of your device, and mask
-is a bit mask describing which bits of an address your device
-supports.  It returns zero if your card can perform DMA properly on
-the machine given the address mask you provided.  In general, the
-device struct of your device is embedded in the bus-specific device
-struct of your device.  For example, &pdev->dev is a pointer to the
-device struct of a PCI device (pdev is a pointer to the PCI device
-struct of your device).
+Here, dev is a pointer to the device struct of your device, and mask is a bit
+mask describing which bits of an address your device supports.  Often the
+device struct of your device is embedded in the bus-specific device struct of
+your device.  For example, &pdev->dev is a pointer to the device struct of a
+PCI device (pdev is a pointer to the PCI device struct of your device).
 
-If it returns non-zero, your device cannot perform DMA properly on
-this platform, and attempting to do so will result in undefined
-behavior.  You must either use a different mask, or not use DMA.
+These calls usually return zero to indicated your device can perform DMA
+properly on the machine given the address mask you provided, but they might
+return an error if the mask is too small to be supportable on the given
+system.  If it returns non-zero, your device cannot perform DMA properly on
+this platform, and attempting to do so will result in undefined behavior.
+You must not use DMA on this device unless the dma_set_mask family of
+functions has returned success.
 
-This means that in the failure case, you have three options:
+This means that in the failure case, you have two options:
 
-1) Use another DMA mask, if possible (see below).
-2) Use some non-DMA mode for data transfer, if possible.
-3) Ignore this device and do not initialize it.
+1) Use some non-DMA mode for data transfer, if possible.
+2) Ignore this device and do not initialize it.
 
-It is recommended that your driver print a kernel KERN_WARNING message
-when you end up performing either #2 or #3.  In this manner, if a user
-of your driver reports that performance is bad or that the device is not
-even detected, you can ask them for the kernel messages to find out
-exactly why.
+It is recommended that your driver print a kernel KERN_WARNING message when
+setting the DMA mask fails.  In this manner, if a user of your driver reports
+that performance is bad or that the device is not even detected, you can ask
+them for the kernel messages to find out exactly why.
 
-The standard 32-bit addressing device would do something like this::
+The standard 64-bit addressing device would do something like this::
 
-	if (dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32))) {
+	if (dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64))) {
 		dev_warn(dev, "mydev: No suitable DMA available\n");
 		goto ignore_this_device;
 	}
 
-Another common scenario is a 64-bit capable device.  The approach here
-is to try for 64-bit addressing, but back down to a 32-bit mask that
-should not fail.  The kernel may fail the 64-bit mask not because the
-platform is not capable of 64-bit addressing.  Rather, it may fail in
-this case simply because 32-bit addressing is done more efficiently
-than 64-bit addressing.  For example, Sparc64 PCI SAC addressing is
-more efficient than DAC addressing.
-
-Here is how you would handle a 64-bit capable device which can drive
-all 64-bits when accessing streaming DMA::
-
-	int using_dac;
+If the device only supports 32-bit addressing for descriptors in the
+coherent allocations, but supports full 64-bits fro streaming mappings
+it would look like this:
 
-	if (!dma_set_mask(dev, DMA_BIT_MASK(64))) {
-		using_dac = 1;
-	} else if (!dma_set_mask(dev, DMA_BIT_MASK(32))) {
-		using_dac = 0;
-	} else {
-		dev_warn(dev, "mydev: No suitable DMA available\n");
-		goto ignore_this_device;
-	}
-
-If a card is capable of using 64-bit consistent allocations as well,
-the case would look like this::
-
-	int using_dac, consistent_using_dac;
-
-	if (!dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64))) {
-		using_dac = 1;
-		consistent_using_dac = 1;
-	} else if (!dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32))) {
-		using_dac = 0;
-		consistent_using_dac = 0;
-	} else {
+	if (dma_set_mask(dev, DMA_BIT_MASK(64))) {
 		dev_warn(dev, "mydev: No suitable DMA available\n");
 		goto ignore_this_device;
 	}
-- 
2.20.1

Re: [PATCH 5/5] Documentation/DMA-API-HOWTO: update dma_mask sections

[Yang, Shunyong]

Hi, Christoph,

On 2019/2/15 22:46, Christoph Hellwig wrote:
> We don't require drivers to guess a DMA mask that might actually
> match the system capabilities any more, so fix up the documentation
> to clear this up.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  Documentation/DMA-API-HOWTO.txt | 121 +++++++++++---------------------
>  1 file changed, 41 insertions(+), 80 deletions(-)
> 
> diff --git a/Documentation/DMA-API-HOWTO.txt b/Documentation/DMA-API-HOWTO.txt
> index f0cc3f772265..8e948fae34af 100644
> --- a/Documentation/DMA-API-HOWTO.txt
> +++ b/Documentation/DMA-API-HOWTO.txt
> @@ -146,114 +146,75 @@ What about block I/O and networking buffers?  The block I/O and
>  networking subsystems make sure that the buffers they use are valid
>  for you to DMA from/to.
>  
> -DMA addressing limitations
> +DMA addressing capabilities
>  ==========================
>  
> -Does your device have any DMA addressing limitations?  For example, is
> -your device only capable of driving the low order 24-bits of address?
> -If so, you need to inform the kernel of this fact.
> +By default, the kernel assumes that your device can address 32-bits of DMA
> +addressing.  For a 64-bit capable device, this needs to be increased, and for
> +a device with limitations, it needs to be decreased.
>  
> -By default, the kernel assumes that your device can address the full
> -32-bits.  For a 64-bit capable device, this needs to be increased.
> -And for a device with limitations, as discussed in the previous
> -paragraph, it needs to be decreased.
> +Special note about PCI: PCI-X specification requires PCI-X devices to support
> +64-bit addressing (DAC) for all transactions.  And at least one platform (SGI
> +SN2) requires 64-bit consistent allocations to operate correctly when the IO
> +bus is in PCI-X mode.
>  
> -Special note about PCI: PCI-X specification requires PCI-X devices to
> -support 64-bit addressing (DAC) for all transactions.  And at least
> -one platform (SGI SN2) requires 64-bit consistent allocations to
> -operate correctly when the IO bus is in PCI-X mode.
> +For correct operation, you must set the DMA mask to inform the kernel about
> +your devices DMA addressing capabilities.
>  
> -For correct operation, you must interrogate the kernel in your device
> -probe routine to see if the DMA controller on the machine can properly
> -support the DMA addressing limitation your device has.  It is good
> -style to do this even if your device holds the default setting,
> -because this shows that you did think about these issues wrt. your
> -device.
> -
> -The query is performed via a call to dma_set_mask_and_coherent()::
> +This is performed via a call to dma_set_mask_and_coherent()::
>  
>  	int dma_set_mask_and_coherent(struct device *dev, u64 mask);
>  
> -which will query the mask for both streaming and coherent APIs together.
> -If you have some special requirements, then the following two separate
> -queries can be used instead:
> +which will set the mask for both streaming and coherent APIs together.  If you
> +have some special requirements, then the following two separate calls can be
> +used instead:
>  
> -	The query for streaming mappings is performed via a call to
> +	The setup for streaming mappings is performed via a call to
>  	dma_set_mask()::
>  
>  		int dma_set_mask(struct device *dev, u64 mask);
>  
> -	The query for consistent allocations is performed via a call
> +	The setup for consistent allocations is performed via a call
>  	to dma_set_coherent_mask()::
>  
>  		int dma_set_coherent_mask(struct device *dev, u64 mask);
>  
> -Here, dev is a pointer to the device struct of your device, and mask
> -is a bit mask describing which bits of an address your device
> -supports.  It returns zero if your card can perform DMA properly on
> -the machine given the address mask you provided.  In general, the
> -device struct of your device is embedded in the bus-specific device
> -struct of your device.  For example, &pdev->dev is a pointer to the
> -device struct of a PCI device (pdev is a pointer to the PCI device
> -struct of your device).
> +Here, dev is a pointer to the device struct of your device, and mask is a bit
> +mask describing which bits of an address your device supports.  Often the
> +device struct of your device is embedded in the bus-specific device struct of
> +your device.  For example, &pdev->dev is a pointer to the device struct of a
> +PCI device (pdev is a pointer to the PCI device struct of your device).
>  
> -If it returns non-zero, your device cannot perform DMA properly on
> -this platform, and attempting to do so will result in undefined
> -behavior.  You must either use a different mask, or not use DMA.
> +These calls usually return zero to indicated your device can perform DMA
> +properly on the machine given the address mask you provided, but they might
> +return an error if the mask is too small to be supportable on the given
> +system.  If it returns non-zero, your device cannot perform DMA properly on
> +this platform, and attempting to do so will result in undefined behavior.
> +You must not use DMA on this device unless the dma_set_mask family of
> +functions has returned success.
>  
> -This means that in the failure case, you have three options:
> +This means that in the failure case, you have two options:
>  
> -1) Use another DMA mask, if possible (see below).
> -2) Use some non-DMA mode for data transfer, if possible.
> -3) Ignore this device and do not initialize it.
> +1) Use some non-DMA mode for data transfer, if possible.
> +2) Ignore this device and do not initialize it.
>  
> -It is recommended that your driver print a kernel KERN_WARNING message
> -when you end up performing either #2 or #3.  In this manner, if a user
> -of your driver reports that performance is bad or that the device is not
> -even detected, you can ask them for the kernel messages to find out
> -exactly why.
> +It is recommended that your driver print a kernel KERN_WARNING message when
> +setting the DMA mask fails.  In this manner, if a user of your driver reports
> +that performance is bad or that the device is not even detected, you can ask
> +them for the kernel messages to find out exactly why.
>  
> -The standard 32-bit addressing device would do something like this::
> +The standard 64-bit addressing device would do something like this::
>  
> -	if (dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32))) {
> +	if (dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64))) {
>  		dev_warn(dev, "mydev: No suitable DMA available\n");
>  		goto ignore_this_device;
>  	}
>  
> -Another common scenario is a 64-bit capable device.  The approach here
> -is to try for 64-bit addressing, but back down to a 32-bit mask that
> -should not fail.  The kernel may fail the 64-bit mask not because the
> -platform is not capable of 64-bit addressing.  Rather, it may fail in
> -this case simply because 32-bit addressing is done more efficiently
> -than 64-bit addressing.  For example, Sparc64 PCI SAC addressing is
> -more efficient than DAC addressing.
> -
> -Here is how you would handle a 64-bit capable device which can drive
> -all 64-bits when accessing streaming DMA::
> -
> -	int using_dac;
> +If the device only supports 32-bit addressing for descriptors in the
> +coherent allocations, but supports full 64-bits fro streaming mappings
                                                   ^^^
"for"

Thanks.
Shunyong.

> +it would look like this:
>  
> -	if (!dma_set_mask(dev, DMA_BIT_MASK(64))) {
> -		using_dac = 1;
> -	} else if (!dma_set_mask(dev, DMA_BIT_MASK(32))) {
> -		using_dac = 0;
> -	} else {
> -		dev_warn(dev, "mydev: No suitable DMA available\n");
> -		goto ignore_this_device;
> -	}
> -
> -If a card is capable of using 64-bit consistent allocations as well,
> -the case would look like this::
> -
> -	int using_dac, consistent_using_dac;
> -
> -	if (!dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64))) {
> -		using_dac = 1;
> -		consistent_using_dac = 1;
> -	} else if (!dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32))) {
> -		using_dac = 0;
> -		consistent_using_dac = 0;
> -	} else {
> +	if (dma_set_mask(dev, DMA_BIT_MASK(64))) {
>  		dev_warn(dev, "mydev: No suitable DMA available\n");
>  		goto ignore_this_device;
>  	}
> 

Re: allow larger than require DMA masks

[Sven Schnelle]

Hi,

On Fri, Feb 15, 2019 at 03:45:54PM +0100, Christoph Hellwig wrote:
> Hi all,
> 
> this series finishes off converting our dma mask model to split between
> device capabilities (dev->dma_mask and dev->coherent_dma_mask) and system
> limitations (dev->bus_dma_mask).  We already accept larger than required
> masks in most dma_map_ops implementation, in case of x86 and
> implementations based on it since the dawn of time.  Only one parisc
> and two sparc64 instances failed larger than required DMA masks, and
> this series fixes that up and updates the documentation that devices
> don't need to handle DMA mask fallbacks.
>

I just tried latest linux-5.4 git on my hp c8000 (parisc), and got the following
error:

[   27.246866] sata_sil24 0000:00:01.0: Applying completion IRQ loss on PCI-X errata fix
[   27.336968] sata_sil24 0000:00:01.0: DMA enable failed
[   27.476922] sata_sil24: probe of 0000:00:01.0 failed with error -5

This is caused by commit dcc02c19cc06bd7bc1b6db0aa0087a2b6eb05b94:
Author: Christoph Hellwig <hch@lst.de>
Date:   Mon Aug 26 12:57:24 2019 +0200

    sata_sil24: use dma_set_mask_and_coherent

    Use the dma_set_mask_and_coherent helper to set the DMA mask. Rely
    on the relatively recent change that setting a larger than required
    mask will never fail to avoid the need for the boilerplate 32-bit
    fallback code.

    Signed-off-by: Christoph Hellwig <hch@lst.de>
    Signed-off-by: Jens Axboe <axboe@kernel.dk>

However, the real problem seems to be in sba_dma_supported():

»       /* Documentation/DMA-API-HOWTO.txt tells drivers to try 64-bit
»        * first, then fall back to 32-bit if that fails.
»        * We are just "encouraging" 32-bit DMA masks here since we can
»        * never allow IOMMU bypass unless we add special support for ZX1.
»        */
	if (mask > ~0U)
»     »       return 0;

Removing the if() makes the DMA mapping work. It's almost midnight here,
so i won't look into that any further today. Does anyone have an opinion
on this behaviour? Otherwise i will look a bit more into this in the
next days.

Regards
Sven

Re: allow larger than require DMA masks

[James Bottomley]

On Mon, 2019-09-23 at 23:14 +0200, Sven Schnelle wrote:
> Hi,
> 
> On Fri, Feb 15, 2019 at 03:45:54PM +0100, Christoph Hellwig wrote:
> > Hi all,
> > 
> > this series finishes off converting our dma mask model to split
> > between device capabilities (dev->dma_mask and dev-
> > >coherent_dma_mask) and system limitations (dev->bus_dma_mask).  We 
> > already accept larger than required masks in most dma_map_ops
> > implementation, in case of x86 and implementations based on it
> > since the dawn of time.  Only one parisc and two sparc64 instances
> > failed larger than required DMA masks, and this series fixes that
> > up and updates the documentation that devices don't need to handle
> > DMA mask fallbacks.
> > 
> 
> I just tried latest linux-5.4 git on my hp c8000 (parisc), and got
> the following
> error:
> 
> [   27.246866] sata_sil24 0000:00:01.0: Applying completion IRQ loss
> on PCI-X errata fix
> [   27.336968] sata_sil24 0000:00:01.0: DMA enable failed
> [   27.476922] sata_sil24: probe of 0000:00:01.0 failed with error -5
> 
> This is caused by commit dcc02c19cc06bd7bc1b6db0aa0087a2b6eb05b94:
> Author: Christoph Hellwig <hch@lst.de>
> Date:   Mon Aug 26 12:57:24 2019 +0200
> 
>     sata_sil24: use dma_set_mask_and_coherent
> 
>     Use the dma_set_mask_and_coherent helper to set the DMA mask.
> Rely
>     on the relatively recent change that setting a larger than
> required
>     mask will never fail to avoid the need for the boilerplate 32-bit
>     fallback code.
> 
>     Signed-off-by: Christoph Hellwig <hch@lst.de>
>     Signed-off-by: Jens Axboe <axboe@kernel.dk>
> 
> However, the real problem seems to be in sba_dma_supported():
> 
> »       /* Documentation/DMA-API-HOWTO.txt tells drivers to try 
> »        * first, then fall back to 32-bit if that fails.
> »        * We are just "encouraging" 32-bit DMA masks here since we
> can
> »        * never allow IOMMU bypass unless we add special support for
> ZX1.
> »        */
> 	if (mask > ~0U)
> »     »       return 0;
> 
> Removing the if() makes the DMA mapping work. It's almost midnight
> here, so i won't look into that any further today. Does anyone have
> an opinion on this behaviour? Otherwise i will look a bit more into
> this in the next days.

The reason for the if was to kick the device into 32 bit descriptors,
which are usually more efficient, especially with older dual descriptor
format cards like we have on parisc systems.

Nothing should go wrong if we don't fail the larger mask request except
that the card uses the inefficient descriptors whereas we'll only
supply it with 32 bit addresses.

James

Re: allow larger than require DMA masks

[Christoph Hellwig]

On Mon, Sep 23, 2019 at 08:59:42PM -0400, James Bottomley wrote:
> > 	if (mask > ~0U)
> > »     »       return 0;
> > 
> > Removing the if() makes the DMA mapping work. It's almost midnight
> > here, so i won't look into that any further today. Does anyone have
> > an opinion on this behaviour? Otherwise i will look a bit more into
> > this in the next days.
> 
> The reason for the if was to kick the device into 32 bit descriptors,
> which are usually more efficient, especially with older dual descriptor
> format cards like we have on parisc systems.

These days we use the dma_get_required_mask API to query for that.

Svens patch looks right for how we are now using the DMA mask setting
API.

Re: allow larger than require DMA masks

[James Bottomley]

On Tue, 2019-09-24 at 23:25 +0200, Christoph Hellwig wrote:
> On Mon, Sep 23, 2019 at 08:59:42PM -0400, James Bottomley wrote:
> > > 	if (mask > ~0U)
> > > »     »       return 0;
> > > 
> > > Removing the if() makes the DMA mapping work. It's almost
> > > midnight here, so i won't look into that any further today. Does
> > > anyone have an opinion on this behaviour? Otherwise i will look a
> > > bit more into this in the next days.
> > 
> > The reason for the if was to kick the device into 32 bit
> > descriptors, which are usually more efficient, especially with
> > older dual descriptor format cards like we have on parisc systems.
> 
> These days we use the dma_get_required_mask API to query for that.

It looks like sym53c8xx didn't get the memo.  On the other hand, I'm
fairly certain it should be compiled in addressing mode zero on all
parisc systems (since all the 64 bit ones have iommus), so I think we
can take care of this ourselves.

> Svens patch looks right for how we are now using the DMA mask setting
> API.

Agreed.

James