From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from service87.mimecast.com ([91.220.42.44]:45133 "EHLO service87.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755836AbaJIJE3 convert rfc822-to-8bit (ORCPT ); Thu, 9 Oct 2014 05:04:29 -0400 Date: Thu, 9 Oct 2014 10:04:20 +0100 From: Lorenzo Pieralisi To: Arnd Bergmann Cc: "linux-arm-kernel@lists.infradead.org" , Mark Rutland , "devicetree@vger.kernel.org" , "jason@lakedaemon.net" , "linux-doc@vger.kernel.org" , Marc Zyngier , "linux-pci@vger.kernel.org" , Liviu Dudau , "linux-kernel@vger.kernel.org" , Will Deacon , "robh+dt@kernel.org" , "suravee.suthikulpanit@amd.com" , Catalin Marinas , "bhelgaas@google.com" , "rmk+kernel@arm.linux.org.uk" , "tglx@linutronix.de" Subject: Re: [RFC 2/4] PCI: generic: Add support for ARM64 and MSI(x) Message-ID: <20141009090420.GA16293@e102568-lin.cambridge.arm.com> References: <1411937610-22125-1-git-send-email-suravee.suthikulpanit@amd.com> <3978481.XNAIA6gzAG@wuerfel> <20141008101942.GA7179@e102568-lin.cambridge.arm.com> <2323270.n17xqW47JB@wuerfel> MIME-Version: 1.0 In-Reply-To: <2323270.n17xqW47JB@wuerfel> Content-Type: text/plain; charset=WINDOWS-1252 Sender: linux-pci-owner@vger.kernel.org List-ID: On Wed, Oct 08, 2014 at 03:47:43PM +0100, Arnd Bergmann wrote: > On Wednesday 08 October 2014 11:19:43 Lorenzo Pieralisi wrote: > > > > Ok. So, unless I am missing something, on platform with mem_offset != 0 > > /proc and /sys interfaces for remapping PCI resources can't work (IIUC > > the proc interface expects the user to pass in the resource address as > > seen from /proc/bus/pci/devices - which are not BAR values. Even if the > > user passed the BAR value to mmap, pci_mmap_fits() in proc_bus_pci_mmap() > > would fail since it compares the pgoff to resource values, which are not > > BAR values). > > I think you are right for the sysfs interface, that one can't possibly > work because of the incorrect address computation. > > For the /procfs interface, I think it can work as long as the offsets > used there are coming from the config space dump in /proc/bus/pci/* > rather than from the /sys/bus/pci/devices/*/resource file. > > > As things stand I think we can safely remove the mem_offset (and > > pci_sys_data dependency) from pci_mmap_page_range(). I do not think > > we can break userspace in any way, basically because it can't work at > > the moment, again, happy to be corrected if I am wrong, please shout. > > Please look at the procfs interface again. That one can be defined > in two ways (either like sparc and arm, or like powerpc and microblaze) > but either one should be able to work with user space that expects > that interface and break with user space that expects the other one. I agree as long as pci_mmap_page_range() is concerned, but I am referring to the pci_mmap_fits() implementation here: start = vma->vm_pgoff; size = ((pci_resource_len(pdev, resno) - 1) >> PAGE_SHIFT) + 1; pci_start = (mmap_api == PCI_MMAP_PROCFS) ? pci_resource_start(pdev, resno) >> PAGE_SHIFT : 0; if (start >= pci_start && start < pci_start + size && start + nr <= pci_start + size) return 1; return 0; pci_mmap_fits(), when mapping from procfs always check the offset against resources, which are fixed-up addresses. If we passed the values dumped from the device config space (as pci_mmap_page_range() expects on arm) IMHO the check above would fail (always referring to platforms where mem_offset != 0). Last changes where introduced by commit 8c05cd08a, whose commit log adds to my confusion: "[...] I think what we want here is for pci_start to be 0 when mmap_api == PCI_MMAP_PROCFS.[...]" But that's not what the code does. I will try to grab an integrator board to give it a go. > > Or we can add mem_offset to the host bridge (after all architectures like > > PowerPC and Microblaze have a pci_mem_offset variable in their host > > controllers), but still, this removes pci_sys_data dependency but does > > not solve the pci_mmap_page_range() issue. > > The host bridge already stores the mem_offset in terms of the resource > list, so we could readily use that, except that it might break the > powerpc hack if that is still in use. Well, yes, I am not saying it can't be done by using the resources list, I am just trying to understand if that's really useful. Thank you ! Lorenzo