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

[Lorenzo Pieralisi]

On Thu, Oct 09, 2014 at 11:51:43AM +0100, Arnd Bergmann wrote:

[...]

> > 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.
> 
> My best guess is that this is a typo and that Darrick meant PCI_MMAP_SYSFS
> in the changelog, which is the same thing that the code does. It's also
> the sensible thing to do.
> 
> This probably means that the procfs interface is now also broken on
> sparc.
> 
> > I will try to grab an integrator board to give it a go.
> 
> Ok, good idea.

Grabbed, tested it, my theory was correct, I can't map PCI resources
to userspace. Actually, if I pass resource offset as a fixed-up address, mmap
succeeds through proc, but it does not mmap the resource, it maps
the resource + mem_offset that happens to be RAM :D for the PCI slot I am
using.

I am testing on an oldish (3.16) kernel since I am having trouble with
mainline PCI and my network adapter on integrator, but I do not see why this
is a problem, this bug has been there forever.

By removing mem_offset from pci_mmap_page_range() everything works fine,
both proc and sys mappings are ok.

> > > > 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.
> 
> The PCI core tries to be ready for PCI host bridges that have multiple
> discontiguous memory spaces with different offsets, although I don't know
> of anybody has that. However if we decide to implement a generic 
> pci_mmap_page_range that tries to take the offset into account, we should
> use the resource list in the host bridge because it can tell us the correct
> offsets.
> 
> However, given what you found, the procfs interface being broken since
> 2010 on both architectures (arm32 and sparc) that try to honor the offset,
> we should probably go back to your previous suggestion of removing
> the offset handling, which would make it possible to use the procfs
> interface and the sysfs interface on all architectures.
> 
> Would you be able to prepare a patch that does this and circulate that
> with the sparc, powerpc and microblaze maintainers as well as Darrick
> and Martin who were involved with the pci_mmap_fits change?

Yes, but let's step back a second. I think that the proc interface
should expect an offset as passed to the user in /proc/bus/pci/devices,
and there the resource is exposed through pci_resource_to_user().

Hence, the pci_mmap_fits() should check the offset against the
resource filtered through pci_resource_to_user(), job done, patch
is trivial, and does what pci_resource_to_user() was meant for IMHO.

Then we have to decide what to do with arm32 code:

1) we remove mem_offset from pci_mmap_page_range() (and rely on default
   pci_resource_to_user())

or

2) we provide pci_resource_to_user() for arm32 which does the CPU->bus
   conversion for us (and leave mem_offset as-is in pci_mmap_range())

Thoughts ?

Thanks,
Lorenzo

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/