r
Because of several reports that it doesn't work
TBD needs a real confirmation this fixes the problem
TBD needs more testing
Signed-off-by: Andi Kleen <[email protected]>
---
Documentation/x86_64/boot-options.txt | 4 +++
arch/x86_64/kernel/pci-dma.c | 42 ++++++++++++++++++++++++++++++++++
2 files changed, 46 insertions(+)
Index: linux/Documentation/x86_64/boot-options.txt
===================================================================
--- linux.orig/Documentation/x86_64/boot-options.txt
+++ linux/Documentation/x86_64/boot-options.txt
@@ -199,6 +199,10 @@ IOMMU
allowed overwrite iommu off workarounds for specific chipsets.
soft Use software bounce buffering (default for Intel machines)
noaperture Don't touch the aperture for AGP.
+ allowdac Allow DMA >4GB - default selected based on chipset bugs
+ When off all DMA over >4GB is forced through an IOMMU or bounce
+ buffering.
+ nodac Forbid DMA >4GB
swiotlb=pages[,force]
Index: linux/arch/x86_64/kernel/pci-dma.c
===================================================================
--- linux.orig/arch/x86_64/kernel/pci-dma.c
+++ linux/arch/x86_64/kernel/pci-dma.c
@@ -170,11 +170,47 @@ void dma_free_coherent(struct device *de
}
EXPORT_SYMBOL(dma_free_coherent);
+static int allow_dac;
+
+static int bridge_from_vendor(struct device *dev, u16 vendor)
+{
+#ifdef CONFIG_PCI
+ struct pci_bus *bus;
+ if (dev->bus != &pci_bus_type)
+ return 0;
+ bus = to_pci_dev(dev)->bus;
+ /* RED-PEN
+ Assumes no locking is needed on these lists because someone
+ should hold a reference count on the target device.
+ Correct assumption? */
+ while (bus != NULL) {
+ if (bus->self && bus->self->vendor == vendor)
+ return 1;
+ bus = bus->parent;
+ }
+#endif
+ return 0;
+}
+
int dma_supported(struct device *dev, u64 mask)
{
if (dma_ops->dma_supported)
return dma_ops->dma_supported(dev, mask);
+ if (mask > DMA_32BIT_MASK) {
+ /* Some VIA bridges seem to have trouble with Double Address
+ Cycle. Disable it behind them all for now. The driver
+ should fall back to non DAC. */
+ if (bridge_from_vendor(dev, PCI_VENDOR_ID_VIA) && !allow_dac) {
+ printk(KERN_INFO
+ "PCI: %s disallowing DAC because of VIA bridge.\n",
+ dev->bus_id);
+ return 0;
+ }
+ if (allow_dac < 0)
+ return 0;
+ }
+
/* Copied from i386. Doesn't make much sense, because it will
only work for pci_alloc_coherent.
The caller just has to use GFP_DMA in this case. */
@@ -231,6 +267,8 @@ EXPORT_SYMBOL(dma_set_mask);
allowed overwrite iommu off workarounds for specific chipsets.
soft Use software bounce buffering (default for Intel machines)
noaperture Don't touch the aperture for AGP.
+ allowdac Allow DMA >4GB - default selected based on chipset bugs
+ nodac Forbid DMA >4GB
*/
__init int iommu_setup(char *p)
{
@@ -264,6 +302,10 @@ __init int iommu_setup(char *p)
iommu_merge = 0;
if (!strncmp(p, "forcesac",8))
iommu_sac_force = 1;
+ if (!strncmp(p, "allowdac", 8))
+ allow_dac = 1;
+ if (!strncmp(p, "nodac", 5))
+ allow_dac = -1;
#ifdef CONFIG_SWIOTLB
if (!strncmp(p, "soft",4))
On Thursday 10 August 2006 22:55, Muli Ben-Yehuda wrote:
>
> > Because of several reports that it doesn't work
> >
> > TBD needs a real confirmation this fixes the problem
> > TBD needs more testing
>
> Should this go to mainline then?
I've haven't decided yet. I put it out for review for now at least.
I got various reports of the VIA bridges having trouble with DAC over the
years, but usually when I asked for confirmation the reporters disappeared.
I finally did the patch now because with cheap 2GB DIMMs VIA systems
with 4GB (which gives some memory over 4GB) are becomming more common.
But again the last reporters disappeared this time.
I will probably not sent it off before final confirmation again.
> > +static int allow_dac;
>
> __read_mostly, like the rest?
Ok. Changed
>
> > +static int bridge_from_vendor(struct device *dev, u16 vendor)
> > +{
> > +#ifdef CONFIG_PCI
>
> My preference would be to provide two versions of this function, one
> empty (!CONFIG_PCI) and one with the code you have here.
I prefer it this way.
> > int dma_supported(struct device *dev, u64 mask)
> > {
> > if (dma_ops->dma_supported)
> > return dma_ops->dma_supported(dev, mask);
>
> I just checked, no ops has a dma_supported method... should we remove
> it?
The if()? Possible.
> > __init int iommu_setup(char *p)
> > {
> > @@ -264,6 +302,10 @@ __init int iommu_setup(char *p)
> > iommu_merge = 0;
> > if (!strncmp(p, "forcesac",8))
> > iommu_sac_force = 1;
> > + if (!strncmp(p, "allowdac", 8))
> > + allow_dac = 1;
> > + if (!strncmp(p, "nodac", 5))
> > + allow_dac = -1;
>
> Why <0? we usually set 1 for enabled and 0 for disabled.
For hardware workarounds it is usually best to have three values:
- Force disabled
- Default based on black/white list
The black/white list is not active when != 0
- Force enabled
I tend to use -1/0/1 for this.
-Andi
On Fri, Aug 11, 2006 at 08:51:53AM +0200, Andi Kleen wrote:
> I've haven't decided yet. I put it out for review for now at least.
>
> I got various reports of the VIA bridges having trouble with DAC over the
> years, but usually when I asked for confirmation the reporters disappeared.
> I finally did the patch now because with cheap 2GB DIMMs VIA systems
> with 4GB (which gives some memory over 4GB) are becomming more common.
>
> But again the last reporters disappeared this time.
>
> I will probably not sent it off before final confirmation again.
Ok. I'll give it and the rest of the patches a spin on my systems with
and without Calgary on Sunday.
> > > int dma_supported(struct device *dev, u64 mask)
> > > {
> > > if (dma_ops->dma_supported)
> > > return dma_ops->dma_supported(dev, mask);
> >
> > I just checked, no ops has a dma_supported method... should we remove
> > it?
>
> The if()? Possible.
the .dma_supported member of dma_ops. If no one is using it, I don't
see a point in keeping it there - we can always reintroduce it when an
IOMMU implementation that needs it comes along.
> > > + if (!strncmp(p, "nodac", 5))
> > > + allow_dac = -1;
> >
> > Why <0? we usually set 1 for enabled and 0 for disabled.
>
> For hardware workarounds it is usually best to have three values:
>
> - Force disabled
> - Default based on black/white list
> The black/white list is not active when != 0
> - Force enabled
>
> I tend to use -1/0/1 for this.
I understand, makes sense. Thanks.
Cheers,
Muli
On Friday 11 August 2006 21:13, Muli Ben-Yehuda wrote:
> On Fri, Aug 11, 2006 at 08:51:53AM +0200, Andi Kleen wrote:
>
> > I've haven't decided yet. I put it out for review for now at least.
> >
> > I got various reports of the VIA bridges having trouble with DAC over the
> > years, but usually when I asked for confirmation the reporters disappeared.
> > I finally did the patch now because with cheap 2GB DIMMs VIA systems
> > with 4GB (which gives some memory over 4GB) are becomming more common.
> >
> > But again the last reporters disappeared this time.
> >
> > I will probably not sent it off before final confirmation again.
>
> Ok. I'll give it and the rest of the patches a spin on my systems with
> and without Calgary on Sunday.
At least for this patch it would only make sense if you had a VIA box
with memory >4GB (and most likely it wouldn't have booted before)
-Andi
On Fri, Aug 11, 2006 at 09:36:12PM +0200, Andi Kleen wrote:
> At least for this patch it would only make sense if you had a VIA box
> with memory >4GB (and most likely it wouldn't have booted before)
I don't think I have a VIA box with >4GB memory, sorry. I'm also
interested however in making sure it doesn't introduce any
regressions on my other boxes...
Cheers,
Muli