[RFC 2/4] PCI: generic: Add support for ARM64 and MSI(x)

On Wed, Oct 08, 2014 at 03:47:43PM +0100, Arnd Bergmann wrote:

On Wednesday 08 October 2014 11:19:43 Lorenzo Pieralisi wrote:

quoted

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.
 

quoted

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.

quoted

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