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 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
