[PATCH libpciaccess] linux: support 32 bit PCI domains
Mark Kettenis
mark.kettenis at xs4all.nl
Wed Jul 12 17:30:18 UTC 2017
> Date: Tue, 11 Jul 2017 10:32:19 -0700
> From: Stephen Hemminger <stephen at networkplumber.org>
>
> On Tue, 11 Jul 2017 10:33:56 +0100
> Emil Velikov <emil.l.velikov at gmail.com> wrote:
>
> > On 11 July 2017 at 05:39, Alan Coopersmith <alan.coopersmith at oracle.com> wrote:
> > > On 07/10/17 11:35 AM, Stephen Hemminger wrote:
> > >>
> > >> The PCI domain maybe larger than 16 bits on Microsoft Azure and other
> > >> virtual environments. PCI busses reported by ACPI are limited to
> > >> 16 bits, but in Azure the domain value for pass through devices is
> > >> intentionally larger than 16 bits to avoid clashing with local
> > >> devices. This is needed to support pass through of GPU devices.
> > >>
> > >> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=101744
> > >> Signed-off-by: Stephen Hemminger <sthemmin at microsoft.com>
> > >> ---
> > >> include/pciaccess.h | 2 +-
> > >> src/linux_sysfs.c | 16 +++-------------
> > >> 2 files changed, 4 insertions(+), 14 deletions(-)
> > >>
> > >> diff --git a/include/pciaccess.h b/include/pciaccess.h
> > >> index 1d7aa4beabfd..93ed76f3cd25 100644
> > >> --- a/include/pciaccess.h
> > >> +++ b/include/pciaccess.h
> > >> @@ -321,7 +321,7 @@ struct pci_device {
> > >> * the domain will always be zero.
> > >> */
> > >> /*@{*/
> > >> - uint16_t domain;
> > >> + uint32_t domain;
> > >> uint8_t bus;
> > >> uint8_t dev;
> > >> uint8_t func;
> > >
> > >
> > > Isn't that going to break the ABI?
> > >
> > Yes it will. There was a similar patch a year ago[1], which ultimately
> > led to trimming the upper bits of the domain.
> >
> > Now that there's an actual need for said data, it might be worth
> > looking into the suggestions put in the earlier thread.
> >
> > Stephen, can you give that a try?
> >
> > -Emil
> >
> > [1] https://lists.freedesktop.org/archives/xorg-devel/2016-August/050586.html
>
> Yes, binary compatibility is a problem. Putting additional data at end
> of structure is possible, but as was raised in the thread, this won't work
> if some code puts the structure on the stack.
>
> An alternative (like pci-utils) would be to have a sacrificial element.
> BSD PCI code would also have to change to do this.
As explained in the thread, this is still an ABI break. There is not
much point in making these changes if you have to bump the shared
library major anyway.
> >From 5d8b06d59561d834ca09fdaf1d5bba38adfa86ea Mon Sep 17 00:00:00 2001
> From: Stephen Hemminger <sthemmin at microsoft.com>
> Date: Mon, 10 Jul 2017 11:18:35 -0700
> Subject: [PATCH libpciaccess] linux: support 32 bit PCI domains
>
> The PCI domain maybe larger than 16 bits on Microsoft Azure and other
> virtual environments. PCI busses reported by ACPI are limited to
> 16 bits, but in Azure the domain value for pass through devices is
> intentionally larger than 16 bits to avoid clashing with local
> devices. This is needed to support pass through of GPU devices.
>
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=101744
> Signed-off-by: Stephen Hemminger <sthemmin at microsoft.com>
> ---
> include/pciaccess.h | 13 +++++++++++--
> src/linux_sysfs.c | 25 ++++++++++++-------------
> 2 files changed, 23 insertions(+), 15 deletions(-)
>
> diff --git a/include/pciaccess.h b/include/pciaccess.h
> index 1d7aa4beabfd..4e6b8b4a389b 100644
> --- a/include/pciaccess.h
> +++ b/include/pciaccess.h
> @@ -319,15 +319,18 @@ struct pci_device {
> * Complete bus identification, including domain, of the device. On
> * platforms that do not support PCI domains (e.g., 32-bit x86 hardware),
> * the domain will always be zero.
> + *
> + * Domain is really 32 bits, but only expose 16-bit version
> + * for backward compatibility. 0xffff if the real domain doesn't
> + * fint in 16 bits.
> */
> /*@{*/
> - uint16_t domain;
> + uint16_t domain_16;
> uint8_t bus;
> uint8_t dev;
> uint8_t func;
> /*@}*/
>
> -
> /**
> * \name Vendor / device ID
> *
> @@ -385,6 +388,12 @@ struct pci_device {
> * Used by the VGA arbiter. Type of resource decoded by the device and
> * the file descriptor (/dev/vga_arbiter). */
> int vgaarb_rsrc;
> +
> +
> + /**
> + * PCI domain value (full 32 bits)
> + */
> + uint32_t domain;
> };
>
>
> diff --git a/src/linux_sysfs.c b/src/linux_sysfs.c
> index dd8ef3e489b1..a8bc2e1977b0 100644
> --- a/src/linux_sysfs.c
> +++ b/src/linux_sysfs.c
> @@ -118,28 +118,18 @@ pci_system_linux_sysfs_create( void )
>
>
> /**
> - * Filter out the names "." and ".." from the scanned sysfs entries, and
> - * domains requiring 32-bits.
> + * Filter out the names "." and ".." from the scanned sysfs entries.
> *
> * \param d Directory entry being processed by \c scandir.
> *
> * \return
> - * Zero if the entry name matches either "." or "..", or the domain requires
> - * 32 bits, non-zero otherwise.
> + * Zero if the entry name matches either "." or ".."
> *
> * \sa scandir, populate_entries
> */
> static int
> scan_sys_pci_filter( const struct dirent * d )
> {
> - if (d->d_name[0] != '.') {
> - unsigned dom = 0;
> -
> - sscanf(d->d_name, "%x:", &dom);
> - if (dom > USHRT_MAX)
> - return 0;
> - }
> -
> return !((strcmp( d->d_name, "." ) == 0)
> || (strcmp( d->d_name, ".." ) == 0));
> }
> @@ -218,10 +208,19 @@ populate_entries( struct pci_system * p )
> (struct pci_device_private *) &p->devices[i];
>
>
> - sscanf(devices[i]->d_name, "%04x:%02x:%02x.%1u",
> + sscanf(devices[i]->d_name, "%x:%02x:%02x.%1u",
> & dom, & bus, & dev, & func);
>
> device->base.domain = dom;
> + /*
> + * Applications compiled with older versions do not expect
> + * 32-bit domain numbers. To keep them working, we keep a 16-bit
> + * version of the domain number at the previous location.
> + */
> + if (dom > 0xffff)
> + device->base.domain_16 = 0xffff;
> + else
> + device->base.domain_16 = dom;
> device->base.bus = bus;
> device->base.dev = dev;
> device->base.func = func;
> --
> 2.11.0
>
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
>
>
More information about the xorg-devel
mailing list