From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933019AbaEEXBD (ORCPT ); Mon, 5 May 2014 19:01:03 -0400 Received: from mail-ie0-f175.google.com ([209.85.223.175]:52530 "EHLO mail-ie0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932864AbaEEXBB (ORCPT ); Mon, 5 May 2014 19:01:01 -0400 Date: Mon, 5 May 2014 17:01:04 -0600 From: Bjorn Helgaas To: Arnd Bergmann Cc: James Bottomley , "rdunlap@infradead.org" , "linux-pci@vger.kernel.org" , "gregkh@linuxfoundation.org" , "linux-doc@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH] DMA-API: Change dma_declare_coherent_memory() CPU address to phys_addr_t Message-ID: <20140505230104.GB8883@google.com> References: <20140430203321.30056.14833.stgit@bhelgaas-glaptop.roam.corp.google.com> <1399011100.2174.45.camel@dabdike> <11081303.OBX0h8mIfH@wuerfel> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <11081303.OBX0h8mIfH@wuerfel> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, May 02, 2014 at 10:42:18AM +0200, Arnd Bergmann wrote: > On Friday 02 May 2014 06:11:42 James Bottomley wrote: > > On Thu, 2014-05-01 at 13:05 -0600, Bjorn Helgaas wrote: > > > On Thu, May 1, 2014 at 8:08 AM, James Bottomley > > > 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." > > > > OK, but even in your definition dma_declare_coherent_memory() should > > operate on dma_addr_t because the input is a region of memory you got > > from the device. Somehow you generate and idea from the device > > configuration of where this piece of memory sits on the device and > > that's what you feed into dma_declare_coherent_memory(). It maps that > > region and makes it available to the CPU allocators. > > I think it's ambiguous at best at the moment, there is no clear > answer what it should be until we define the semantics more clearly. > > At the moment, we have these callers > > arch/arm/mach-imx/mach-imx27_visstrim_m10.c: dma = dma_declare_coherent_memory(&pdev->dev, > arch/arm/mach-imx/mach-imx27_visstrim_m10.c: dma = dma_declare_coherent_memory(&pdev->dev, > arch/arm/mach-imx/mach-imx27_visstrim_m10.c: dma = dma_declare_coherent_memory(&pdev->dev, > arch/arm/mach-imx/mach-imx27_visstrim_m10.c: dma = dma_declare_coherent_memory(&pdev->dev, > arch/arm/mach-imx/mach-mx31_3ds.c: dma = dma_declare_coherent_memory(&pdev->dev, > arch/arm/mach-imx/mach-mx31moboard.c: dma = dma_declare_coherent_memory(&pdev->dev, > arch/arm/mach-imx/mach-mx35_3ds.c: dma = dma_declare_coherent_memory(&pdev->dev, > arch/arm/mach-imx/mach-pcm037.c: dma = dma_declare_coherent_memory(&pdev->dev, > arch/arm/plat-samsung/s5p-dev-mfc.c: if (dma_declare_coherent_memory(area->dev, area->base, > arch/sh/drivers/pci/fixups-dreamcast.c: BUG_ON(!dma_declare_coherent_memory(&dev->dev, > drivers/media/platform/s5p-mfc/s5p_mfc.c: if (dma_declare_coherent_memory(dev->mem_dev_l, mem_info[ > drivers/media/platform/s5p-mfc/s5p_mfc.c: if (dma_declare_coherent_memory(dev->mem_dev_r, mem_info[ > drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c: err = dma_declare_coherent_memory > drivers/scsi/NCR_Q720.c: if (dma_declare_coherent_memory(dev, base_addr, base_addr, > drivers/usb/host/ohci-sm501.c: if (!dma_declare_coherent_memory(dev, mem->start, > drivers/usb/host/ohci-tmio.c: if (!dma_declare_coherent_memory(&dev->dev, sram->start, > > I don't know about NCR_Q720, but all others are only used on machines > where physical addresses and bus addresses are in the same space. In general, the driver doesn't know whether physical and bus addresses are in the same space. At least, I *hope* it doesn't have to know, because it can't be very generic if it does. > There are other machines that may always have to go through an IOMMU, > or that have a fixed offset between dma addresses and physical addresses > and that need to call a phys_to_dma() to convert between the two, but > none of those call dma_declare_coherent_memory. > ... > We also have machines on ARM can never do DMA to addresses above the > 32-bit boundary but can have more than 4GB RAM (using swiotlb or iommu). > Building a kernel for these machines uses a 32-bit dma_addr_t and > a 64-bit phys_addr_t. The PCI core currently prevents those machines from having MMIO space above 4GB on the bus, which doesn't sound strictly necessary. > > Well, first we need to answer whether we need separate phys_addr_t and > > dma_addr_t ... we know the former represents physical memory addressed > > by the CPU and the latter bus physical addresses from devices ... but > > they're essentially the same. > > I think BARs should be phys_addr_t: in the example with 32-bit dma_addr_t > and 64-bit phys_addr_t, you would be able to put the MMIO registers > into a high address if the PCI host supports that, the only limitation > would be that no device could DMA into a high address for doing inter-device > DMA. I could imagine using u64 for BARs, but I don't see why we would use phys_addr_t. I think it's simpler to describe dma_addr_t as "sufficient to hold any address on the bus, whether for DMA or MMIO accesses." There isn't really a connection between the CPU address width and the bus address width. A 64-bit CPU could have a 32-bit conventional PCI bus, and a 32-bit CPU could use a bus with all MMIO space above 4GB given a host bridge that did appropriate translation. (I don't know why one would do the latter, but I don't know why it wouldn't work.) Bjorn