On Mon, May 19, 2014 at 10:56 AM, Catalin Marinas <catalin.marinas@arm.com> wrote:
On Mon, May 19, 2014 at 10:44:51AM +0100, Arnd Bergmann wrote:
> On Monday 19 May 2014 10:03:40 Catalin Marinas wrote:
> > On Mon, May 19, 2014 at 09:32:43AM +0100, Arnd Bergmann wrote:
> > > The more important question is what happens to high buffers allocated elsewhere
> > > that get passed into dma_map_sg by a device driver. Depending on the DT properties
> > > of the device and its parents, this needs to do one of three things:
> > >
> > > a) translate the 64-bit virtual address into a 64-bit bus address
> > > b) create an IOMMU entry for the 64-bit address and pass the 32-bit IOMMU
> > >    address to the driver
> > > c) use the swiotlb code to create a bounce buffer at a 32-bit DMA address
> > >    and copy the data around
> > >
> > > It's definitely wrong to just hardcode a DMA mask in the driver because that
> > > code doesn't know which of the three cases is being used. Moreover, you can't
> > > do it using an #ifdef CONFIG_ARM64, because it's completely independent of
> > > the architecture, and we need to do the exact same logic on ARM32 and any
> > > other architecture.
> >
> > I agree.
> >
> > The problem we currently have is system topology description to pass the
> > DMA mask and in a hierarchical way. I can see Santosh's patches
> > introducing dma-ranges but the coherent dma mask still set as 32-bit. We
> > can use the dma-ranges to infer a mask but that's only specific to the
> > device and the driver doesn't know whether it goes through an iommu or
> > not.
>
> We definitely have to fix this very quickly, before people start building
> real arm64 systems and shipping them.
>
> We should not merge any hacks that attempt to work around the problem,
> but try to come to a conclusion how to handle them properly.
> My hope was that we could just always set the dma mask to whatever
> the DT says it should be to keep the burden from device drivers,
> unless they want to restrict it further (e.g. when the specific
> peripheral hardware has a bug that prevents us from using high addresses,
> even though the SoC in theory supports it everywhere).

I agree.

> Rob Herring argued that we should always mimic PCI and call dma_set_mask()
> in drivers but default to a 32-bit mask otherwise, independent of whether
> the hardware can do more or less than that, IIRC.

Can we not default to something built up from dma-ranges? Or 32-bit if
dma-ranges property is missing?

My reasoning was the information may not come from DT. For AHCI, the 32 vs. 64 bit capability is in a capability register and the DMA mask is set based on that. This information only exists with-in the driver.

I think some sort of capability merging is needed here to merge a bus mask with a device mask. The current api doesn't seem to support this as adjusting the set mask is expected to fail if the requested mask is not supported. Here is how I see it working. Devices can have a default mask (32-bit typically). The default is simply the common case. Really the mask should only be set for DMA capable devices, but many drivers fail to set the mask. Devices can then enlarge or shrink what they support based on knowing the IP block features or getting capabilities from the IP block such as the AHCI case. This mask then needs to be adjusted (only shrinking) if the parent bus or platform has restrictions which is the part that should come from dma-ranges.

Keep in mind that dma-ranges is defined as a bus property, not a property of a device. I don't have any issue with allowing it in a device, but I don't think it should be required and am not yet convinced it is needed. 

Rob