| Message ID | 1249069310.20192.220.camel@macbook.infradead.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Fri, 2009-07-31 at 20:41 +0100, David Woodhouse wrote: > On an iMac G5, the b43 driver is failing to initialise because trying to > set the dma mask to 30-bit fails. Even though there's only 512MiB of RAM > in the machine anyway: > https://bugzilla.redhat.com/show_bug.cgi?id=514787 > > We should probably let it succeed if the available RAM in the system > doesn't exceed the requested limit. > > Signed-off-by: David Woodhouse <David.Woodhouse@intel.com> Also, isn't our iommu code smart enough to clamp allocations to the DMA mask nowadays ? In that case, we could probably just force iommu on always... Cheers, Ben. > diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c > index 20a60d6..1769a8e 100644 > --- a/arch/powerpc/kernel/dma.c > +++ b/arch/powerpc/kernel/dma.c > @@ -90,11 +90,11 @@ static void dma_direct_unmap_sg(struct device *dev, > struct scatterlist *sg, > static int dma_direct_dma_supported(struct device *dev, u64 mask) > { > #ifdef CONFIG_PPC64 > - /* Could be improved to check for memory though it better be > - * done via some global so platforms can set the limit in case > + extern unsigned long highest_memmap_pfn; > + /* Could be improved so platforms can set the limit in case > * they have limited DMA windows > */ > - return mask >= DMA_BIT_MASK(32); > + return (mask >> PAGE_SHIFT) >= highest_memmap_pfn; > #else > return 1; > #endif >
On Sat, 2009-08-01 at 08:25 +1000, Benjamin Herrenschmidt wrote: > On Fri, 2009-07-31 at 20:41 +0100, David Woodhouse wrote: > > On an iMac G5, the b43 driver is failing to initialise because trying to > > set the dma mask to 30-bit fails. Even though there's only 512MiB of RAM > > in the machine anyway: > > https://bugzilla.redhat.com/show_bug.cgi?id=514787 > > > > We should probably let it succeed if the available RAM in the system > > doesn't exceed the requested limit. > > > > Signed-off-by: David Woodhouse <David.Woodhouse@intel.com> > > Also, isn't our iommu code smart enough to clamp allocations to the DMA > mask nowadays ? In that case, we could probably just force iommu on > always... We're not using the IOMMU on this box: PowerMac motherboard: iMac G5 DART: table not allocated, using direct DMA
On Sat, 2009-08-01 at 08:54 +0100, David Woodhouse wrote: > On Sat, 2009-08-01 at 08:25 +1000, Benjamin Herrenschmidt wrote: > > On Fri, 2009-07-31 at 20:41 +0100, David Woodhouse wrote: > > > On an iMac G5, the b43 driver is failing to initialise because trying to > > > set the dma mask to 30-bit fails. Even though there's only 512MiB of RAM > > > in the machine anyway: > > > https://bugzilla.redhat.com/show_bug.cgi?id=514787 > > > > > > We should probably let it succeed if the available RAM in the system > > > doesn't exceed the requested limit. > > > > > > Signed-off-by: David Woodhouse <David.Woodhouse@intel.com> > > > > Also, isn't our iommu code smart enough to clamp allocations to the DMA > > mask nowadays ? In that case, we could probably just force iommu on > > always... > > We're not using the IOMMU on this box: > > PowerMac motherboard: iMac G5 > DART: table not allocated, using direct DMA I know, I was suggesting we do :-) Cheers, Ben.
On Sat, 2009-08-01 at 18:00 +1000, Benjamin Herrenschmidt wrote: > On Sat, 2009-08-01 at 08:54 +0100, David Woodhouse wrote: > > On Sat, 2009-08-01 at 08:25 +1000, Benjamin Herrenschmidt wrote: > > > On Fri, 2009-07-31 at 20:41 +0100, David Woodhouse wrote: > > > > On an iMac G5, the b43 driver is failing to initialise because trying to > > > > set the dma mask to 30-bit fails. Even though there's only 512MiB of RAM > > > > in the machine anyway: > > > > https://bugzilla.redhat.com/show_bug.cgi?id=514787 > > > > > > > > We should probably let it succeed if the available RAM in the system > > > > doesn't exceed the requested limit. > > > > > > > > Signed-off-by: David Woodhouse <David.Woodhouse@intel.com> > > > > > > Also, isn't our iommu code smart enough to clamp allocations to the DMA > > > mask nowadays ? In that case, we could probably just force iommu on > > > always... > > > > We're not using the IOMMU on this box: > > > > PowerMac motherboard: iMac G5 > > DART: table not allocated, using direct DMA > > I know, I was suggesting we do :-) I'm not sure. Losing 16MiB on a machine which only has 512MiB anyway doesn't seem ideal, and we'll want to make the no-iommu code DTRT _anyway_, surely? So we might as well let the DART keep its existing logic (which is only to bother if we have more than 1GiB of RAM; a limit chosen specifically because of the Broadcom brokenness).
On Sat, 2009-08-01 at 10:00 +0100, David Woodhouse wrote: > I'm not sure. Losing 16MiB on a machine which only has 512MiB anyway > doesn't seem ideal, and we'll want to make the no-iommu code DTRT > _anyway_, surely? > > So we might as well let the DART keep its existing logic (which is > only > to bother if we have more than 1GiB of RAM; Ah right, so when do we enable the DART ? Above 1G ? I though it was above 2G but we may well have moved that down to 1G just for b43 indeed. I definitely agree on the fix to the mask so it only compares to the available RAM. I'll check that in when I'm back from the snow fields on tuesday :-) Cheers, Ben,
On Sun, 2009-08-02 at 17:50 +1000, Benjamin Herrenschmidt wrote: > On Sat, 2009-08-01 at 10:00 +0100, David Woodhouse wrote: > > I'm not sure. Losing 16MiB on a machine which only has 512MiB anyway > > doesn't seem ideal, and we'll want to make the no-iommu code DTRT > > _anyway_, surely? > > > > So we might as well let the DART keep its existing logic (which is > > only > > to bother if we have more than 1GiB of RAM; > > Ah right, so when do we enable the DART ? Above 1G ? I though it was > above 2G but we may well have moved that down to 1G just for b43 indeed. void __init alloc_dart_table(void) { /* Only reserve DART space if machine has more than 1GB of RAM * or if requested with iommu=on on cmdline. * * 1GB of RAM is picked as limit because some default devices * (i.e. Airport Extreme) have 30 bit address range limits. */ if (iommu_is_off) return; if (!iommu_force_on && lmb_end_of_DRAM() <= 0x40000000ull) return; > I definitely agree on the fix to the mask so it only compares to the > available RAM. I'll check that in when I'm back from the snow fields > on tuesday :-) I see one potential failure mode with this. You need: - No IOMMU - Crappy devices - Hotpluggable memory - Boot with only "low" memory, and allow a pci_set_dma_mask() to succeed because you don't have that much memory anyway. - Hotplug some "high" memory that the crappy device can't reach. Do we care about that scenario? I think we might be able to "fix" it by setting the memory_limit when we allow pci_set_dma_mask() to succeed? That will effectively prevent the addition of memory that our crappy device can't reach, won't it?
On Mon, 2009-08-03 at 14:14 +0100, David Woodhouse wrote: > Do we care about that scenario? I think we might be able to "fix" it by > setting the memory_limit when we allow pci_set_dma_mask() to succeed? > That will effectively prevent the addition of memory that our crappy > device can't reach, won't it? We don't support hotplug memory on those machines anyway, do we ? Cheers, Ben.
diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c index 20a60d6..1769a8e 100644 --- a/arch/powerpc/kernel/dma.c +++ b/arch/powerpc/kernel/dma.c @@ -90,11 +90,11 @@ static void dma_direct_unmap_sg(struct device *dev, struct scatterlist *sg, static int dma_direct_dma_supported(struct device *dev, u64 mask) { #ifdef CONFIG_PPC64 - /* Could be improved to check for memory though it better be - * done via some global so platforms can set the limit in case + extern unsigned long highest_memmap_pfn; + /* Could be improved so platforms can set the limit in case * they have limited DMA windows */ - return mask >= DMA_BIT_MASK(32); + return (mask >> PAGE_SHIFT) >= highest_memmap_pfn; #else return 1;
On an iMac G5, the b43 driver is failing to initialise because trying to set the dma mask to 30-bit fails. Even though there's only 512MiB of RAM in the machine anyway: https://bugzilla.redhat.com/show_bug.cgi?id=514787 We should probably let it succeed if the available RAM in the system doesn't exceed the requested limit. Signed-off-by: David Woodhouse <David.Woodhouse@intel.com> #endif