[+ryder] On Fri, Nov 10, 2017 at 02:07:05PM +0100, Thierry Reding wrote: [...] > > The really neat version is to take a known non-memory physical address like > > the host controller's own MMIO region, which has no legitimate reason to > > ever be used as a DMA address. pcie-mediatek almost gets this right, but by > > using virt_to_phys() on an ioremapped address they end up with nonsense > > rather than the correct address (although realistically you would have to be > > extremely unlucky for said nonsense to collide with a real DMA address given > > to a PCI endpoint later). Following on from above, dma_map_resource() would > > be the foolproof way to get that right. > > Yes, that was our intention as well. Our initial plan was to use an > address from the PCI aperture within Tegra that wasn't being used for > any other purpose. Hi Thierry, to wrap up this thread, why an address from the PCI aperture and not a host bridge register ? I CC'ed Ryder so that he can explain to me please what: PCIE_MSI_VECTOR and PCIE_IMSI_ADDR offsets in: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/host/pcie-mediatek.c?h=v4.14 register space represent (IIUC the driver uses: virt_to_phys(port->base + PCIE_MSI_VECTOR); as MSI doorbell address, which is wrong anyway as Robin explained, just want to understand how that register was chosen - it is never written or read in the driver so it is hard to figure that out) to understand whether the approach can be replicated instead of relying on GFP_DMA for pages allocation. Thanks, Lorenzo > However, we ran into some odd corner cases where this wasn't working > as expected. As a temporary solution we wanted to move to GFP_DMA32 > (or GFP_DMA) in order to support 32-bit only MSI endpoints. > > Eventually we'll want to get rid of the allocation altogether, we just > need to find a set of values that work reliably. In the meantime, it > looks as though GFP_DMA would be the right solution as long as we have > to stick with __get_free_pages(). > > Alternatively, since we've already verified that the MSI writes are > never committed to memory, we could choose some random address pointing > to system memory as well, but I'm reluctant to do that because it could > end up being confusing for users (and developers) to see some random > address showing up somewhere. A physical address such as the beginning > of system memory should always work and might be unique enough to > indicate that it is special. > > Thierry
