On Mon, Mar 30, 2015 at 01:33:28PM +0200, Michael S. Tsirkin wrote:
> The basic class ID macros in pci_ids.h are pretty useful for userspace
> using the pci sysfs interface, and they aren't fundamentally different
> from the constants in pci_regs.h - both are defined in the
> pci spec.
>
> At the moment userspace is forced to duplicate these macros
> (e.g. QEMU does this, so do seabios, gpxe, and others), it is better to
> expose them in /usr/include/linux/pci_ids.h so everyone can just include
> this header.
I agree that it would be nice for applications to get these definitions
from a single place, but I'm not sure that include/uapi/linux/pci_ids.h
needs to be that place.
These constants are just copies of what's in the spec, and I don't think
you're suggesting that the constants are necessary to use a kernel API.
I know the kernel does provide access to values via sysfs "class" files,
but the kernel is just passing the values through from the hardware.
That's analogous to reading the class with setpci, and I don't think it
leads to a requirement that the kernel export all the information about how
to interpret the class values.
I haven't looked at libpci or libudev, but it sounds like you think those
are not good solutions. Is that because they don't currently have this
information? People don't want to add dependencies on them?
Bjorn
On Thu, Apr 02, 2015 at 03:53:47PM -0500, Bjorn Helgaas wrote:
> On Mon, Mar 30, 2015 at 01:33:28PM +0200, Michael S. Tsirkin wrote:
> > The basic class ID macros in pci_ids.h are pretty useful for userspace
> > using the pci sysfs interface, and they aren't fundamentally different
> > from the constants in pci_regs.h - both are defined in the
> > pci spec.
> >
> > At the moment userspace is forced to duplicate these macros
> > (e.g. QEMU does this, so do seabios, gpxe, and others), it is better to
> > expose them in /usr/include/linux/pci_ids.h so everyone can just include
> > this header.
>
> I agree that it would be nice for applications to get these definitions
> from a single place, but I'm not sure that include/uapi/linux/pci_ids.h
> needs to be that place.
>
> These constants are just copies of what's in the spec, and I don't think
> you're suggesting that the constants are necessary to use a kernel API.
>
> I know the kernel does provide access to values via sysfs "class" files,
> but the kernel is just passing the values through from the hardware.
> That's analogous to reading the class with setpci, and I don't think it
> leads to a requirement that the kernel export all the information about how
> to interpret the class values.
>
> I haven't looked at libpci or libudev, but it sounds like you think those
> are not good solutions. Is that because they don't currently have this
> information? People don't want to add dependencies on them?
>
> Bjorn
People don't want to add dependencies on them.
--
MST
On Thu, Apr 02, 2015 at 11:28:19PM +0200, Michael S. Tsirkin wrote:
> On Thu, Apr 02, 2015 at 03:53:47PM -0500, Bjorn Helgaas wrote:
> > On Mon, Mar 30, 2015 at 01:33:28PM +0200, Michael S. Tsirkin wrote:
> > > The basic class ID macros in pci_ids.h are pretty useful for userspace
> > > using the pci sysfs interface, and they aren't fundamentally different
> > > from the constants in pci_regs.h - both are defined in the
> > > pci spec.
> > >
> > > At the moment userspace is forced to duplicate these macros
> > > (e.g. QEMU does this, so do seabios, gpxe, and others), it is better to
> > > expose them in /usr/include/linux/pci_ids.h so everyone can just include
> > > this header.
> >
> > I agree that it would be nice for applications to get these definitions
> > from a single place, but I'm not sure that include/uapi/linux/pci_ids.h
> > needs to be that place.
> >
> > These constants are just copies of what's in the spec, and I don't think
> > you're suggesting that the constants are necessary to use a kernel API.
> >
> > I know the kernel does provide access to values via sysfs "class" files,
> > but the kernel is just passing the values through from the hardware.
> > That's analogous to reading the class with setpci, and I don't think it
> > leads to a requirement that the kernel export all the information about how
> > to interpret the class values.
> >
> > I haven't looked at libpci or libudev, but it sounds like you think those
> > are not good solutions. Is that because they don't currently have this
> > information? People don't want to add dependencies on them?
> >
> > Bjorn
>
> People don't want to add dependencies on them.
So you want us to create yet-a-third-location for these values? Both
libpci and libudev are on all systems. And it's not a run-time
dependancy you are talking about here, but a build-time one, which is
quite different. Having a build-time dependancy on packages that ship
with all Linux distributions seems acceptable to me.
thanks,
greg k-h
[+cc Martin]
On Thu, Apr 2, 2015 at 4:28 PM, Michael S. Tsirkin <[email protected]> wrote:
> On Thu, Apr 02, 2015 at 03:53:47PM -0500, Bjorn Helgaas wrote:
>> On Mon, Mar 30, 2015 at 01:33:28PM +0200, Michael S. Tsirkin wrote:
>> > The basic class ID macros in pci_ids.h are pretty useful for userspace
>> > using the pci sysfs interface, and they aren't fundamentally different
>> > from the constants in pci_regs.h - both are defined in the
>> > pci spec.
>> >
>> > At the moment userspace is forced to duplicate these macros
>> > (e.g. QEMU does this, so do seabios, gpxe, and others), it is better to
>> > expose them in /usr/include/linux/pci_ids.h so everyone can just include
>> > this header.
>>
>> I agree that it would be nice for applications to get these definitions
>> from a single place, but I'm not sure that include/uapi/linux/pci_ids.h
>> needs to be that place.
>>
>> These constants are just copies of what's in the spec, and I don't think
>> you're suggesting that the constants are necessary to use a kernel API.
>>
>> I know the kernel does provide access to values via sysfs "class" files,
>> but the kernel is just passing the values through from the hardware.
>> That's analogous to reading the class with setpci, and I don't think it
>> leads to a requirement that the kernel export all the information about how
>> to interpret the class values.
>>
>> I haven't looked at libpci or libudev, but it sounds like you think those
>> are not good solutions. Is that because they don't currently have this
>> information? People don't want to add dependencies on them?
>
> People don't want to add dependencies on them.
Why not? I'm not a user-space programmer, so it's not obvious to me
what the problems with adding a dependency are. If a package provides
functionality you want, it seems like a *good* thing to use it and
depend on it rather than reimplementing the functionality yourself.
The /usr/include/pci/header.h supplied by libpci-dev (on Ubuntu) looks
like it has most or all of the constants you want.
Bjorn
On Thu, Apr 02, 2015 at 11:59:21PM +0200, Greg KH wrote:
> On Thu, Apr 02, 2015 at 11:28:19PM +0200, Michael S. Tsirkin wrote:
> > On Thu, Apr 02, 2015 at 03:53:47PM -0500, Bjorn Helgaas wrote:
> > > On Mon, Mar 30, 2015 at 01:33:28PM +0200, Michael S. Tsirkin wrote:
> > > > The basic class ID macros in pci_ids.h are pretty useful for userspace
> > > > using the pci sysfs interface, and they aren't fundamentally different
> > > > from the constants in pci_regs.h - both are defined in the
> > > > pci spec.
> > > >
> > > > At the moment userspace is forced to duplicate these macros
> > > > (e.g. QEMU does this, so do seabios, gpxe, and others), it is better to
> > > > expose them in /usr/include/linux/pci_ids.h so everyone can just include
> > > > this header.
> > >
> > > I agree that it would be nice for applications to get these definitions
> > > from a single place, but I'm not sure that include/uapi/linux/pci_ids.h
> > > needs to be that place.
> > >
> > > These constants are just copies of what's in the spec, and I don't think
> > > you're suggesting that the constants are necessary to use a kernel API.
> > >
> > > I know the kernel does provide access to values via sysfs "class" files,
> > > but the kernel is just passing the values through from the hardware.
> > > That's analogous to reading the class with setpci, and I don't think it
> > > leads to a requirement that the kernel export all the information about how
> > > to interpret the class values.
> > >
> > > I haven't looked at libpci or libudev, but it sounds like you think those
> > > are not good solutions. Is that because they don't currently have this
> > > information? People don't want to add dependencies on them?
> > >
> > > Bjorn
> >
> > People don't want to add dependencies on them.
>
> So you want us to create yet-a-third-location for these values?
I propose making this part of base Linux system headers, yes.
BTW it's not a third location actually. It would be a third location in a
library, but there are many other packages that duplicate this code.
Since we have this code around anyway, if we just export it, then
there's hope that with time all this duplicate code will go away, then
when e.g. a new value is added to spec, people will add it to linux and
everyone benefits. As I work on QEMU, I know QEMU will do it,
likely bios and gpxe too, I expect libpci/libudev to be no different.
> Both
> libpci and libudev are on all systems. And it's not a run-time
> dependancy you are talking about here, but a build-time one, which is
> quite different.
Yes: build-time dependencies annoy developers, run-time ones annoy users :)
In a sense build-time dependencies are more annoying as it's harder to
resolve them.
> Having a build-time dependancy on packages that ship
> with all Linux distributions seems acceptable to me.
>
> thanks,
>
> greg k-h
On Thu, Apr 02, 2015 at 05:32:14PM -0500, Bjorn Helgaas wrote:
> [+cc Martin]
That's a good idea. Martin, could you please answer the following:
assuming that linux exported linux/pci_ids.h providing class
IDs that are currently in /usr/include/pci/header.h
in a header /usr/include/linux/pci_ids.h,
would libpci be open to replacing part of
/usr/include/pci/header.h with #include <linux/pci_ids.h>,
assuming that a solution for old systems that lack this
header is also provided?
> On Thu, Apr 2, 2015 at 4:28 PM, Michael S. Tsirkin <[email protected]> wrote:
> > On Thu, Apr 02, 2015 at 03:53:47PM -0500, Bjorn Helgaas wrote:
> >> On Mon, Mar 30, 2015 at 01:33:28PM +0200, Michael S. Tsirkin wrote:
> >> > The basic class ID macros in pci_ids.h are pretty useful for userspace
> >> > using the pci sysfs interface, and they aren't fundamentally different
> >> > from the constants in pci_regs.h - both are defined in the
> >> > pci spec.
> >> >
> >> > At the moment userspace is forced to duplicate these macros
> >> > (e.g. QEMU does this, so do seabios, gpxe, and others), it is better to
> >> > expose them in /usr/include/linux/pci_ids.h so everyone can just include
> >> > this header.
> >>
> >> I agree that it would be nice for applications to get these definitions
> >> from a single place, but I'm not sure that include/uapi/linux/pci_ids.h
> >> needs to be that place.
> >>
> >> These constants are just copies of what's in the spec, and I don't think
> >> you're suggesting that the constants are necessary to use a kernel API.
> >>
> >> I know the kernel does provide access to values via sysfs "class" files,
> >> but the kernel is just passing the values through from the hardware.
> >> That's analogous to reading the class with setpci, and I don't think it
> >> leads to a requirement that the kernel export all the information about how
> >> to interpret the class values.
> >>
> >> I haven't looked at libpci or libudev, but it sounds like you think those
> >> are not good solutions. Is that because they don't currently have this
> >> information? People don't want to add dependencies on them?
> >
> > People don't want to add dependencies on them.
>
> Why not? I'm not a user-space programmer, so it's not obvious to me
> what the problems with adding a dependency are. If a package provides
> functionality you want, it seems like a *good* thing to use it and
> depend on it rather than reimplementing the functionality yourself.
> The /usr/include/pci/header.h supplied by libpci-dev (on Ubuntu) looks
> like it has most or all of the constants you want.
>
> Bjorn
Well, so does libudev-devel according to Greg. Do you know why doesn't
libudev-devel include /usr/include/pci/header.h?
xorg has a copy too: /usr/include/xorg/xf86Pci.h
Why does not xorg include /usr/include/pci/header.h?
I think I know - I think that's because each extra dependency does
introduce overhead: it's another community to interact with to fix bugs
and add features, it's more portability and version skew headaches.
Besides, I'm guessing that the linux header is less likely to have bugs.
Consider PCI_CLASS_STORAGE_IDE:
pciutils]$ git grep PCI_CLASS_STORAGE_IDE
lib/header.h:#define PCI_CLASS_STORAGE_IDE 0x0101
$ git grep PCI_CLASS_STORAGE_IDE
arch/alpha/kernel/pci.c: if (dev->class >> 8 == PCI_CLASS_STORAGE_IDE) {
....
many more hits.
pciutils does nothing with this value itself, it's possible for a distro
to ship a wrong header, and no one will notice. OTOH Linux will break if
it's wrong. In fact there are 3 values libpci does appear to use
internally:
PCI_CLASS_BRIDGE_HOST
PCI_CLASS_DISPLAY_VGA
PCI_CLASS_BRIDGE_PCI
I'm guessing others are re-exported for the benefit of
applications using libpci.
--
MST
Hello!
> That's a good idea. Martin, could you please answer the following:
> assuming that linux exported linux/pci_ids.h providing class
> IDs that are currently in /usr/include/pci/header.h
> in a header /usr/include/linux/pci_ids.h,
> would libpci be open to replacing part of
> /usr/include/pci/header.h with #include <linux/pci_ids.h>,
> assuming that a solution for old systems that lack this
> header is also provided?
Please remember that libpci is cross-platform. Your proposal would make it
use <linux/pci_ids.h> on Linux and provide its own definitions on all other
systems, which is likely to bring less consistency, not more.
I do not see any point in using kernel headers for things, which are unrelated
to the kernel.
> pciutils does nothing with this value itself, it's possible for a distro
> to ship a wrong header, and no one will notice. OTOH Linux will break if
> it's wrong. In fact there are 3 values libpci does appear to use
> internally:
> PCI_CLASS_BRIDGE_HOST
> PCI_CLASS_DISPLAY_VGA
> PCI_CLASS_BRIDGE_PCI
>
> I'm guessing others are re-exported for the benefit of
> applications using libpci.
Exactly. And I expect that it will be quite similar with the kernel --
most classes defined in <linux/pci_ids.h> are not likely to be used anywhere
in the kernel.
Martin