| Message ID | 4281f8e01b9fc5628cbf4a5c77abd642801e23c7.1490188942.git.dwmw2@infradead.org |
|---|---|
| State | Changes Requested |
| Headers | show |
On 3/22/2017 9:25 AM, David Woodhouse wrote: > > +#ifdef __aarch64__ > +/* ARM64 wants to be special and not expose this through /proc like everyone else */ > +#undef HAVE_PCI_MMAP > +#endif > + Where is this ARM64 special requirement coming from?
On Wed, 2017-03-22 at 09:54 -0400, Sinan Kaya wrote: > On 3/22/2017 9:25 AM, David Woodhouse wrote: > > > > > > +#ifdef __aarch64__ > > +/* ARM64 wants to be special and not expose this through /proc > > like everyone else */ > > +#undef HAVE_PCI_MMAP > > +#endif > > + > Where is this ARM64 special requirement coming from? The idea is that as a new platform, ARM64 shouldn't need to implement legacy userspace interfaces. http://lists.infradead.org/pipermail/linux-arm-kernel/2016-April/422571.html
On 3/22/2017 10:04 AM, David Woodhouse wrote: > On Wed, 2017-03-22 at 09:54 -0400, Sinan Kaya wrote: >> On 3/22/2017 9:25 AM, David Woodhouse wrote: >>> >>> >>> +#ifdef __aarch64__ >>> +/* ARM64 wants to be special and not expose this through /proc >>> like everyone else */ >>> +#undef HAVE_PCI_MMAP >>> +#endif >>> + >> Where is this ARM64 special requirement coming from? > > The idea is that as a new platform, ARM64 shouldn't need to implement > legacy userspace interfaces. > > http://lists.infradead.org/pipermail/linux-arm-kernel/2016-April/422571.html > Aren't we breaking an ABI for userspace? I know DPDK relies on this feature.
On Wed, Mar 22, 2017 at 10:15:04AM -0400, Sinan Kaya wrote: > On 3/22/2017 10:04 AM, David Woodhouse wrote: > > On Wed, 2017-03-22 at 09:54 -0400, Sinan Kaya wrote: > >> On 3/22/2017 9:25 AM, David Woodhouse wrote: > >>> > >>> > >>> +#ifdef __aarch64__ > >>> +/* ARM64 wants to be special and not expose this through /proc > >>> like everyone else */ > >>> +#undef HAVE_PCI_MMAP > >>> +#endif > >>> + > >> Where is this ARM64 special requirement coming from? > > > > The idea is that as a new platform, ARM64 shouldn't need to implement > > legacy userspace interfaces. > > > > http://lists.infradead.org/pipermail/linux-arm-kernel/2016-April/422571.html > > > > Aren't we breaking an ABI for userspace? I know DPDK relies on this feature. It relies on the /proc interface? That's the first I've ever heard of that -- everybody so far has only been interested in the sysfs stuff. Nothing's more broken than before, because we've never supported the /proc interface, but if existing arm64 code out there is failing because of that then I'm of course open to supporting it. I'm just surprised that nobody else has come up with that before, since DPDK is in common use. Can you point me at the specific code, please? Will
On 3/22/2017 10:18 AM, Will Deacon wrote: > On Wed, Mar 22, 2017 at 10:15:04AM -0400, Sinan Kaya wrote: >> On 3/22/2017 10:04 AM, David Woodhouse wrote: >>> On Wed, 2017-03-22 at 09:54 -0400, Sinan Kaya wrote: >>>> On 3/22/2017 9:25 AM, David Woodhouse wrote: >>>>> >>>>> >>>>> +#ifdef __aarch64__ >>>>> +/* ARM64 wants to be special and not expose this through /proc >>>>> like everyone else */ >>>>> +#undef HAVE_PCI_MMAP >>>>> +#endif >>>>> + >>>> Where is this ARM64 special requirement coming from? >>> >>> The idea is that as a new platform, ARM64 shouldn't need to implement >>> legacy userspace interfaces. >>> >>> http://lists.infradead.org/pipermail/linux-arm-kernel/2016-April/422571.html >>> >> >> Aren't we breaking an ABI for userspace? I know DPDK relies on this feature. > > It relies on the /proc interface? That's the first I've ever heard of that > -- everybody so far has only been interested in the sysfs stuff. > > Nothing's more broken than before, because we've never supported the /proc > interface, but if existing arm64 code out there is failing because of that > then I'm of course open to supporting it. I'm just surprised that nobody > else has come up with that before, since DPDK is in common use. > > Can you point me at the specific code, please? I'm correcting myself. I had to go back my memory from last year. DPDK requires HAVE_PCI_MMAP to be set. We have been carrying some old maillist patch around for DPDK customers internally. When HAVE_PCI_MMAP is set, resource files are created in sysfs and procfs. DPDK is using the files in sysfs directory not procfs directory. Having HAVE_PCI_MMAP defined is the DPDK requirement. > > Will >
On Wed, Mar 22, 2017 at 2:25 PM, David Woodhouse <dwmw2@infradead.org> wrote: > From: David Woodhouse <dwmw@amazon.co.uk> > > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> > --- > drivers/pci/proc.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c > index 2d9cfa4..a940f4b 100644 > --- a/drivers/pci/proc.c > +++ b/drivers/pci/proc.c > @@ -17,6 +17,11 @@ > > static int proc_initialized; /* = 0 */ > > +#ifdef __aarch64__ > +/* ARM64 wants to be special and not expose this through /proc like everyone else */ > +#undef HAVE_PCI_MMAP > +#endif I'd still prefer this to be a whitelist of the existing architectures using PCI MMAP in procfs, there is really no reason for arm64 to be special, the one thing we want to control here is whether new architectures (including arm64) that have never had either the sysfs or the procfs interface should get one or both of them. As it seems that there are important use cases for the sysfs interface and your patch series will just make that work everywhere, I'd argue that we should just always provide the sysfs interface now, and use HAVE_PCI_MMAP only control the procfs interface. That way, we turn on the sysfs interface on arc, arm64, frv and tile as well as any future architecture with PCI support, but leave the procfs support as opt-in. Arnd
On Fri, 2017-03-24 at 17:13 +0100, Arnd Bergmann wrote: > > I'd still prefer this to be a whitelist of the existing architectures using PCI > MMAP in procfs, there is really no reason for arm64 to be special, the > one thing we want to control here is whether new architectures (including > arm64) that have never had either the sysfs or the procfs interface > should get one or both of them. > > As it seems that there are important use cases for the sysfs interface > and your patch series will just make that work everywhere, I'd argue > that we should just always provide the sysfs interface now, and use > HAVE_PCI_MMAP only control the procfs interface. I still have no sympathy for the "we don't want <this> tiny part of the legacy generic non-architecture-specific procfs ABI to exist on ARM64". Why not just kill /proc/bus/pci *entirely* there? But sure, I suppose we could refactor things so that the sysfs mmap bits depend on (HAVE_PCI_MMAP || ARCH_GENERIC_MMAP_RESOURCE_RANGE) rather than having architectures define the latter in *addition* to the former. > That way, we turn on the sysfs interface on arc, arm64, frv and tile > as well as any future architecture with PCI support, but leave > the procfs support as opt-in. OK. My plan was for ARCH_GENERIC_MMAP_RESOURCE_RANGE to go away once all architectures were converted, and HAVE_PCI_MMAP to be all that's left. It does make send to do arc, fr-v and tile too though. So we can do it that way.
diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c index 2d9cfa4..a940f4b 100644 --- a/drivers/pci/proc.c +++ b/drivers/pci/proc.c @@ -17,6 +17,11 @@ static int proc_initialized; /* = 0 */ +#ifdef __aarch64__ +/* ARM64 wants to be special and not expose this through /proc like everyone else */ +#undef HAVE_PCI_MMAP +#endif + static loff_t proc_bus_pci_lseek(struct file *file, loff_t off, int whence) { struct pci_dev *dev = PDE_DATA(file_inode(file));