On Fri, May 02, 2014 at 12:18:07PM +0100, Liviu Dudau wrote: > On Thu, May 01, 2014 at 08:05:04PM +0100, Bjorn Helgaas wrote: > > On Thu, May 1, 2014 at 8:08 AM, James Bottomley > > <jbottomley@xxxxxxxxxxxxx> wrote: > > > On Wed, 2014-04-30 at 14:33 -0600, Bjorn Helgaas wrote: > > >> dma_declare_coherent_memory() takes two addresses for a region of memory: a > > >> "bus_addr" and a "device_addr". I think the intent is that "bus_addr" is > > >> the physical address a *CPU* would use to access the region, and > > >> "device_addr" is the bus address the *device* would use to address the > > >> region. > > >> > > >> Rename "bus_addr" to "phys_addr" and change its type to phys_addr_t. > > > > > > Remind me what the difference between phys_addr_t and dma_addr_t are. > > > > > > I thought phys_addr_t was the maximum address the CPU could reach after > > > address translation and dma_addr_t was the maximum physical address any > > > bus attached memory mapped devices could appear at. (of course, mostly > > > they're the same). > > > > I assumed phys_addr_t was for physical addresses generated by a CPU > > and dma_addr_t was for addresses generated by a device, e.g., > > addresses you would see with a PCI bus analyzer or in a PCI BAR. But > > ARCH_DMA_ADDR_T_64BIT does seem more connected to processor-specific > > things, e.g., ARM_LPAE, than to device bus properties like "support > > 64-bit PCI, not just 32-bit PCI." > > > > DMA-API-HOWTO.txt contains this: > > > > ... the definition of the dma_addr_t (which can hold any valid DMA > > address for the platform) type which should be used everywhere you > > hold a DMA (bus) address returned from the DMA mapping functions. > > > > and clearly you use a dma_addr_t from dma_alloc_coherent(), etc., to > > program the device. It seems silly to have phys_addr_t and dma_addr_t > > for two (possibly slightly different) flavors of CPU physical > > addresses, and then sort of overload dma_addr_t by also using it to > > hold the addresses devices use for DMA. > > dma_addr_t is a phys_addr_t masked according to the capabilities of the > *DMA engine* (not device). I don't quite agree with this. phys_addr_t is used for physical addresses generated by a CPU, e.g., ioremap() takes a phys_addr_t and returns a void * (a virtual address). A dma_addr_t is an address you get from dma_map_page() or a similar interface, and you program into a device (or DMA engine if you prefer). You can't generate a dma_addr_t simply by masking a phys_addr_t because an IOMMU can make arbitrary mappings of bus addresses to physical memory addresses. It is meaningless for a CPU to generate a reference to a dma_addr_t. It will work in some simple cases where there's no IOMMU, but it won't work in general. > Quite a lot of PCI devices (all?) now contain > a DMA engine, so the distinction is somewhat blurred nowadays. I'll take your word for the DMA engine/device distinction, but I don't see how it's relevant here. Linux doesn't have a generic idea of a DMA engine; we just treat it as part of the device capabilities. > > I'm not a CPU > > person, so I don't understand the usefulness of dma_addr_t as a > > separate type to hold CPU addresses at which memory-mapped devices can > > appear. If that's all it is, why not just use phys_addr_t everywhere > > and get rid of dma_addr_t altogether? > > Because dma_addr_t imposes further restrictions based on what the DMA engine > can access. You could have a DMA engine that can only access 32bit addresses > in a 40+ bit CPU addresses system. Yes; this is an argument for having separate types for bus addresses and CPU addresses. I think dma_addr_t *is* for bus addresses. I thought James was suggesting that it was a subset of CPU physical address space, and that any valid dma_addr_t is also a valid phys_addr_t. That doesn't seem right to me. > > dma_declare_coherent_memory() calls ioremap() on bus_addr, which seems > > a clear indication that bus_addr is really a physical address usable > > by the CPU. But I do see your point that bus_addr is a CPU address > > for a memory-mapped device, and would thus fit into your idea of a > > dma_addr_t. > > > > I think this is the only part of the DMA API that deals with CPU > > physical addresses. I suppose we could side-step this question by > > having the caller do the ioremap; then dma_declare_coherent_memory() > > could just take a CPU virtual address (a void *) and a device address > > (a dma_addr_t). > > And how would that change the meaning of the function? I think the interesting thing here is that we ioremap() a bus address. That doesn't work in general because ioremap() is expecting a CPU physical address, not a bus address. We don't have a generic way for dma_declare_coherent_memory() to convert a bus address to a CPU physical address. The caller knows a little more and probably *could* do that, e.g., a PCI driver could use pcibios_bus_to_resource(). If we made a generic way to do the conversion, it would definitely be nicer to leave the ioremap() inside dma_declare_coherent_memory(). Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
