[PATCH RFC] x86: Fix 64-bit DMA masks on VIA

[PATCH RFC] x86: Fix 64-bit DMA masks on VIA

[Michael Buesch]

This untested patch is supposed to fix DMAing on some VIA boards.
Currently the DMA subsystem returns an error, if the driver does
tell that it supports a 64bit DMA mask. So the driver probing
would fail in that case. This happens for some b43 cards with
64bit DMA engines on VIA boards.
Instead of failing the dma_supported() check, simply strip off the
high 32bits.

Signed-off-by: Michael Buesch <mb@bu3sch.de>

---

This is not tested, due to the lack of hardware.


Index: wireless-testing/arch/x86/kernel/pci-dma_64.c
===================================================================
--- wireless-testing.orig/arch/x86/kernel/pci-dma_64.c	2008-04-01 15:53:17.000000000 +0200
+++ wireless-testing/arch/x86/kernel/pci-dma_64.c	2008-04-23 20:49:42.000000000 +0200
@@ -173,11 +173,8 @@ int dma_supported(struct device *dev, u6
 {
 #ifdef CONFIG_PCI
 	if (mask > 0xffffffff && forbid_dac > 0) {
-
-
-
 		printk(KERN_INFO "PCI: Disallowing DAC for device %s\n", dev->bus_id);
-		return 0;
+		/* We strip off the high 32 bits in dma_set_mask() */
 	}
 #endif
 
@@ -215,6 +212,10 @@ int dma_set_mask(struct device *dev, u64
 {
 	if (!dev->dma_mask || !dma_supported(dev, mask))
 		return -EIO;
+
+	if (forbid_dac > 0)
+		mask &= 0xffffffff;
+
 	*dev->dma_mask = mask;
 	return 0;
 }
Index: wireless-testing/include/asm-x86/dma-mapping_32.h
===================================================================
--- wireless-testing.orig/include/asm-x86/dma-mapping_32.h	2008-02-16 19:08:15.000000000 +0100
+++ wireless-testing/include/asm-x86/dma-mapping_32.h	2008-04-23 20:46:12.000000000 +0200
@@ -137,10 +137,6 @@ dma_supported(struct device *dev, u64 ma
         if(mask < 0x00ffffff)
                 return 0;
 
-	/* Work around chipset bugs */
-	if (forbid_dac > 0 && mask > 0xffffffffULL)
-		return 0;
-
 	return 1;
 }
 
@@ -150,6 +146,10 @@ dma_set_mask(struct device *dev, u64 mas
 	if(!dev->dma_mask || !dma_supported(dev, mask))
 		return -EIO;
 
+	/* Work around chipset bugs */
+	if (forbid_dac > 0)
+		mask &= 0xffffffffULL;
+
 	*dev->dma_mask = mask;
 
 	return 0;

-- 
Greetings Michael.

Re: [PATCH RFC] x86: Fix 64-bit DMA masks on VIA

[Andi Kleen]

Michael Buesch <mb@bu3sch.de> writes:

> This untested patch is supposed to fix DMAing on some VIA boards.
> Currently the DMA subsystem returns an error, if the driver does
> tell that it supports a 64bit DMA mask. So the driver probing
> would fail in that case. 

The driver is broken then. It is supposed to retry with a small
mask on an error. Please fix the driver.

-Andi

Re: [PATCH RFC] x86: Fix 64-bit DMA masks on VIA

[Michael Buesch]

On Thursday 24 April 2008 15:43:50 Andi Kleen wrote:
> Michael Buesch <mb@bu3sch.de> writes:
> 
> > This untested patch is supposed to fix DMAing on some VIA boards.
> > Currently the DMA subsystem returns an error, if the driver does
> > tell that it supports a 64bit DMA mask. So the driver probing
> > would fail in that case. 
> 
> The driver is broken then. It is supposed to retry with a small
> mask on an error. Please fix the driver.

I already added a workaround to the driver.
Why do we need to workaround this in _every_ driver? (Note that _every_
driver supporting a 64bit mask is affected). Why not fix it in the DMA layer?

-- 
Greetings Michael.

Re: [PATCH RFC] x86: Fix 64-bit DMA masks on VIA

[Andi Kleen]

Michael Buesch wrote:
> On Thursday 24 April 2008 15:43:50 Andi Kleen wrote:
>> Michael Buesch <mb@bu3sch.de> writes:
>>
>>> This untested patch is supposed to fix DMAing on some VIA boards.
>>> Currently the DMA subsystem returns an error, if the driver does
>>> tell that it supports a 64bit DMA mask. So the driver probing
>>> would fail in that case. 
>> The driver is broken then. It is supposed to retry with a small
>> mask on an error. Please fix the driver.

I must admit my comment was slightly wrong. In some cases
it can make sense to start with a small mask and retry bigger.

> 
> I already added a workaround to the driver.
> Why do we need to workaround this in _every_ driver? 

The API was designed this way because many devices support different
hardware interfaces for different address sizes. So for example with a
32bit mask you might be able to transfer less data than with a 64bit
mask. And with the retry steps you should be able to figure out the
most efficient format for the current system.

See the discussion in Documentation/DMA-mapping.txt

cc: DaveM; I think the concept was from him originally.

-Andi

Re: [PATCH RFC] x86: Fix 64-bit DMA masks on VIA

[David Miller]

From: Michael Buesch <mb@bu3sch.de>
Date: Thu, 24 Apr 2008 16:06:00 +0200

> Why do we need to workaround this in _every_ driver? (Note that _every_
> driver supporting a 64bit mask is affected). Why not fix it in the DMA layer?

Because the driver has to make a conscious choice of what
DMA mask it wants to use.  Datastructures can change based
upon whether 32-bit or 64-bit DMA is available, etc.

Re: [PATCH RFC] x86: Fix 64-bit DMA masks on VIA

[Alan Cox]

On Thu, 24 Apr 2008 16:06:00 +0200
Michael Buesch <mb@bu3sch.de> wrote:

> On Thursday 24 April 2008 15:43:50 Andi Kleen wrote:
> > Michael Buesch <mb@bu3sch.de> writes:
> > 
> > > This untested patch is supposed to fix DMAing on some VIA boards.
> > > Currently the DMA subsystem returns an error, if the driver does
> > > tell that it supports a 64bit DMA mask. So the driver probing
> > > would fail in that case. 
> > 
> > The driver is broken then. It is supposed to retry with a small
> > mask on an error. Please fix the driver.
> 
> I already added a workaround to the driver.
> Why do we need to workaround this in _every_ driver? (Note that _every_
> driver supporting a 64bit mask is affected). Why not fix it in the DMA layer?

Some hardware wants to know it can get a given DMA mask or failure. I
agree however that a "pci_prefer_64bit_dma(pdev)" function would be a
good patch for someone to submit tot he PCI layer code.

Alan

Re: [PATCH RFC] x86: Fix 64-bit DMA masks on VIA

[Michael Buesch]

On Thursday 24 April 2008 16:32:40 Alan Cox wrote:
> On Thu, 24 Apr 2008 16:06:00 +0200
> Michael Buesch <mb@bu3sch.de> wrote:
> 
> > On Thursday 24 April 2008 15:43:50 Andi Kleen wrote:
> > > Michael Buesch <mb@bu3sch.de> writes:
> > > 
> > > > This untested patch is supposed to fix DMAing on some VIA boards.
> > > > Currently the DMA subsystem returns an error, if the driver does
> > > > tell that it supports a 64bit DMA mask. So the driver probing
> > > > would fail in that case. 
> > > 
> > > The driver is broken then. It is supposed to retry with a small
> > > mask on an error. Please fix the driver.
> > 
> > I already added a workaround to the driver.
> > Why do we need to workaround this in _every_ driver? (Note that _every_
> > driver supporting a 64bit mask is affected). Why not fix it in the DMA layer?
> 
> Some hardware wants to know it can get a given DMA mask or failure. I
> agree however that a "pci_prefer_64bit_dma(pdev)" function would be a
> good patch for someone to submit tot he PCI layer code.

Yeah well. I see the issue. However, I think the actual probing should
be done in the DMA layer. We could pass dma_set_mask() the mask as a pointer
and modify the mask value to what is actually used. So the driver would
know what mask we felt back to. That was actually my first idea,
but I preferred to submit a more simple solution without an API change
to the list.

However, in the end I don't care too much, as our driver is fixed.
I just think that we will have more of these bugs in the future. Especially,
as this bug won't hit on the majority of platforms.

-- 
Greetings Michael.

Re: [PATCH RFC] x86: Fix 64-bit DMA masks on VIA

[Ingo Molnar]

* Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> > > > This untested patch is supposed to fix DMAing on some VIA 
> > > > boards. Currently the DMA subsystem returns an error, if the 
> > > > driver does tell that it supports a 64bit DMA mask. So the 
> > > > driver probing would fail in that case.
> > > 
> > > The driver is broken then. It is supposed to retry with a small 
> > > mask on an error. Please fix the driver.
> > 
> > I already added a workaround to the driver. Why do we need to 
> > workaround this in _every_ driver? (Note that _every_ driver 
> > supporting a 64bit mask is affected). Why not fix it in the DMA 
> > layer?
> 
> Some hardware wants to know it can get a given DMA mask or failure. I 
> agree however that a "pci_prefer_64bit_dma(pdev)" function would be a 
> good patch for someone to submit tot he PCI layer code.

yes, and i suspect Michael is correct in suggesting that the majority of 
drivers would use that interface and would let the PCI layer handle the 
probing/fallback details. (Jesse Cc:-ed)

	Ingo

Re: [PATCH RFC] x86: Fix 64-bit DMA masks on VIA

[Jesse Barnes]

On Monday, April 28, 2008 9:53 am Ingo Molnar wrote:
> * Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> > > > > This untested patch is supposed to fix DMAing on some VIA
> > > > > boards. Currently the DMA subsystem returns an error, if the
> > > > > driver does tell that it supports a 64bit DMA mask. So the
> > > > > driver probing would fail in that case.
> > > >
> > > > The driver is broken then. It is supposed to retry with a small
> > > > mask on an error. Please fix the driver.
> > >
> > > I already added a workaround to the driver. Why do we need to
> > > workaround this in _every_ driver? (Note that _every_ driver
> > > supporting a 64bit mask is affected). Why not fix it in the DMA
> > > layer?
> >
> > Some hardware wants to know it can get a given DMA mask or failure. I
> > agree however that a "pci_prefer_64bit_dma(pdev)" function would be a
> > good patch for someone to submit tot he PCI layer code.
>
> yes, and i suspect Michael is correct in suggesting that the majority of
> drivers would use that interface and would let the PCI layer handle the
> probing/fallback details. (Jesse Cc:-ed)

With an implied fallback to 32 bits?  Michael's right (at least I think 
Michael's the one being quoted there) that "try 64 then fallback to 32 on 
error" is a pretty common sight, so having a hint that says you'd like 64 but 
don't really care would be a win for drivers.

Michael, want to hack something up?

Thanks,
Jesse

Re: [PATCH RFC] x86: Fix 64-bit DMA masks on VIA

[Michael Buesch]

On Monday 28 April 2008 19:04:07 Jesse Barnes wrote:
> On Monday, April 28, 2008 9:53 am Ingo Molnar wrote:
> > * Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> > > > > > This untested patch is supposed to fix DMAing on some VIA
> > > > > > boards. Currently the DMA subsystem returns an error, if the
> > > > > > driver does tell that it supports a 64bit DMA mask. So the
> > > > > > driver probing would fail in that case.
> > > > >
> > > > > The driver is broken then. It is supposed to retry with a small
> > > > > mask on an error. Please fix the driver.
> > > >
> > > > I already added a workaround to the driver. Why do we need to
> > > > workaround this in _every_ driver? (Note that _every_ driver
> > > > supporting a 64bit mask is affected). Why not fix it in the DMA
> > > > layer?
> > >
> > > Some hardware wants to know it can get a given DMA mask or failure. I
> > > agree however that a "pci_prefer_64bit_dma(pdev)" function would be a
> > > good patch for someone to submit tot he PCI layer code.
> >
> > yes, and i suspect Michael is correct in suggesting that the majority of
> > drivers would use that interface and would let the PCI layer handle the
> > probing/fallback details. (Jesse Cc:-ed)
> 
> With an implied fallback to 32 bits?  Michael's right (at least I think 
> Michael's the one being quoted there) that "try 64 then fallback to 32 on 
> error" is a pretty common sight, so having a hint that says you'd like 64 but 
> don't really care would be a win for drivers.
> 
> Michael, want to hack something up?

Something like this? (untested)


Index: wireless-testing/drivers/base/dma-mapping.c
===================================================================
--- wireless-testing.orig/drivers/base/dma-mapping.c	2008-04-28 23:34:19.000000000 +0200
+++ wireless-testing/drivers/base/dma-mapping.c	2008-04-28 23:46:25.000000000 +0200
@@ -216,3 +216,38 @@ void dmam_release_declared_memory(struct
 EXPORT_SYMBOL(dmam_release_declared_memory);
 
 #endif
+
+/**
+ * dma_set_mask_weak - Set the DMA mask. Retry with smaller masks.
+ * @dev: Device to set the mask on.
+ * @mask: Pointer to the mask that you want to set. The function will
+ * 	modify the mask and set it to the actually used mask, in case
+ * 	it had to fall back to a smaller mask.
+ *
+ * Set the DMA mask and allow falling back to smaller masks in
+ * case of an error.
+ *
+ * RETURNS:
+ * 0 on success, -errno on failure.
+ */
+int dma_set_mask_weak(struct device *dev, u64 *mask)
+{
+	u64 m = *mask;
+	int err;
+
+	if (m < DMA_MIN_FALLBACK_MASK)
+		return -EINVAL;
+	while (1) {
+		err = dma_set_mask(dev, m);
+		if (!err)
+			break;
+		/* Did not like this one. Try a smaller one. */
+		m >>= 1;
+		if (m < DMA_MIN_FALLBACK_MASK)
+			return err;
+	}
+	*mask = m;
+
+	return 0;
+}
+EXPORT_SYMBOL(dma_set_mask_weak);
Index: wireless-testing/include/linux/dma-mapping.h
===================================================================
--- wireless-testing.orig/include/linux/dma-mapping.h	2008-04-28 23:34:19.000000000 +0200
+++ wireless-testing/include/linux/dma-mapping.h	2008-04-28 23:35:35.000000000 +0200
@@ -60,6 +60,11 @@ static inline int is_device_dma_capable(
 
 extern u64 dma_get_required_mask(struct device *dev);
 
+extern int dma_set_mask_weak(struct device *dev, u64 *mask);
+/* Smallest mask fallback used by dma_set_mask_weak(). */
+#define DMA_MIN_FALLBACK_MASK	DMA_BIT_MASK(24) /* 16 MB */
+
+
 static inline unsigned int dma_get_max_seg_size(struct device *dev)
 {
 	return dev->dma_parms ? dev->dma_parms->max_segment_size : 65536;


-- 
Greetings Michael.