x86: Fix 64-bit DMA masks on VIA
Subsystems (3)
RFC PATCH 1/1 x86: Fix 64-bit DMA masks on VIA
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.
---
This is not tested, due to the lack of hardware.
arch/x86/kernel/pci-dma_64.c +5 -4
@@ -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 ; }
include/asm-x86/dma-mapping_32.h +4 -4
@@ -137,10 +137,6 @@ dma_supported(struct device *dev, u64 maif (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 masif (!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 ;
Signature · 1 line
Greetings Michael.
Quoted 4 lines · 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.
The driver is broken then. It is supposed to retry with a small mask on an error. Please fix the driver.
-Andi
Quoted 9 lines · On Thursday 24 April 2008 15:43:50 Andi Kleen wrote:
Quoted 4 lines · 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.
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?
Signature · 1 line
Greetings Michael.
Quoted 9 lines · Michael Buesch
Quoted 8 lines · On Thursday 24 April 2008 15:43:50 Andi Kleen wrote:
Quoted 4 lines · 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.
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.
Quoted 3 lines
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
-Andi
From: Michael Buesch <[email protected]>
Date: Thu, 24 Apr 2008 16:06:00 +0200
Quoted 2 lines
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.
Quoted 8 lines · On Thu, 24 Apr 2008 16:06:00 +0200 Michael Buesch <[email protected]> wrote:
Quoted 7 lines · On Thursday 24 April 2008 15:43:50 Andi Kleen wrote:
Quoted 4 lines · 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.
Quoted 6 lines
Quoted 2 lines
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
Quoted 14 lines · On Thursday 24 April 2008 16:32:40 Alan Cox wrote:
Quoted 11 lines · On Thu, 24 Apr 2008 16:06:00 +0200 Michael Buesch <[email protected]> wrote:
Quoted 9 lines · On Thursday 24 April 2008 15:43:50 Andi Kleen wrote:
Quoted 4 lines · 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.
The driver is broken then. It is supposed to retry with a small mask on an error. Please fix the driver.
Quoted 7 lines
Quoted 3 lines
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.
Signature · 1 line
Greetings Michael.
Quoted 8 lines · Alan Cox
Quoted 8 lines
Quoted 7 lines
Quoted 4 lines
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.
Quoted 8 lines
Quoted 4 lines
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
Quoted 14 lines · Ingo Molnar
Quoted 13 lines · Alan Cox
Quoted 12 lines
Quoted 7 lines
Quoted 4 lines
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?
Quoted 7 lines
Quoted 3 lines
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
Quoted 23 lines · On Monday 28 April 2008 19:04:07 Jesse Barnes wrote:
Quoted 14 lines · Ingo Molnar
Quoted 13 lines · Alan Cox
Quoted 12 lines
Quoted 7 lines
Quoted 4 lines
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?
Quoted 7 lines
Quoted 3 lines
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)
Quoted 6 lines
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)
drivers/base/dma-mapping.c +35 -0
@@ -216,3 +216,38 @@ void dmam_release_declared_memory(structEXPORT_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);
include/linux/dma-mapping.h +5 -0
@@ -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 ;
Signature · 1 line
Greetings Michael.