[PATCH libdrm 0/2] Add libdrm support for auxbus devices

[PATCH libdrm 0/2] Add libdrm support for auxbus devices

[Alistair Popple]

The new Nova[1] NVIDIA GPU driver exposes GPUs as logical devices on an
auxbus. Currently libdrm doesn't support auxbus, so it ignores Nova GPUs
during enumeration. This adds support for devices on an auxbus.

Rather than follow the lead of virtio and resolve the logical auxbus device
back to the physical PCI device, we add a new bus type and associated parser.
This is because in the future a single physical GPU may present multiple auxbus
devices, so it needs to be possible to resolve individual devices even if they
share the same parent GPU.

[1] - https://rust-for-linux.com/nova-gpu-driver

Alistair Popple (2):
  xf86drm: Pass PCI path rather than maj/min when parsing sysfs files
  xf86drm: Expose auxiliary bus GPUs as DRM_BUS_AUXILIARY

 xf86drm.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++-------
 xf86drm.h |  12 +++++
 2 files changed, 130 insertions(+), 17 deletions(-)

-- 
2.54.0

[PATCH libdrm 1/2] xf86drm: Pass PCI path rather than maj/min when parsing sysfs files

[Alistair Popple]

Both parse_separate_sysfs_files() and parse_config_sysfs_file()
take a maj/min device numbers and resolve the actual PCI path using
get_pci_path(). This was to allow virtio to resolve the correct path.

However there is no reason to call get_pci_path() in these functions -
in all cases the caller can call get_pci_path() prior to calling these
functions. This results in a minor code simplification for a future
change which allows callers to provide the PCI device path instead of
using get_pci_path().

Signed-off-by: Alistair Popple <apopple@nvidia.com>
---
 xf86drm.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/xf86drm.c b/xf86drm.c
index a8b39a31..25f2a3ff 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -3833,7 +3833,7 @@ static int drmGetMaxNodeName(void)
 }
 
 #ifdef __linux__
-static int parse_separate_sysfs_files(int maj, int min,
+static int parse_separate_sysfs_files(const char *pci_path,
                                       drmPciDeviceInfoPtr device,
                                       bool ignore_revision)
 {
@@ -3844,13 +3844,11 @@ static int parse_separate_sysfs_files(int maj, int min,
       "subsystem_vendor",
       "subsystem_device",
     };
-    char path[PATH_MAX + 1], pci_path[PATH_MAX + 1];
+    char path[PATH_MAX + 1];
     unsigned int data[ARRAY_SIZE(attrs)];
     FILE *fp;
     int ret;
 
-    get_pci_path(maj, min, pci_path);
-
     for (unsigned i = ignore_revision ? 1 : 0; i < ARRAY_SIZE(attrs); i++) {
         if (snprintf(path, PATH_MAX, "%s/%s", pci_path, attrs[i]) < 0)
             return -errno;
@@ -3875,15 +3873,13 @@ static int parse_separate_sysfs_files(int maj, int min,
     return 0;
 }
 
-static int parse_config_sysfs_file(int maj, int min,
+static int parse_config_sysfs_file(const char *pci_path,
                                    drmPciDeviceInfoPtr device)
 {
-    char path[PATH_MAX + 1], pci_path[PATH_MAX + 1];
+    char path[PATH_MAX + 1];
     unsigned char config[64];
     int fd, ret;
 
-    get_pci_path(maj, min, pci_path);
-
     if (snprintf(path, PATH_MAX, "%s/config", pci_path) < 0)
         return -errno;
 
@@ -3911,11 +3907,15 @@ static int drmParsePciDeviceInfo(int maj, int min,
                                  uint32_t flags)
 {
 #ifdef __linux__
+    char pci_path[PATH_MAX + 1];
+
+    get_pci_path(maj, min, pci_path);
+
     if (!(flags & DRM_DEVICE_GET_PCI_REVISION))
-        return parse_separate_sysfs_files(maj, min, device, true);
+        return parse_separate_sysfs_files(pci_path, device, true);
 
-    if (parse_separate_sysfs_files(maj, min, device, false))
-        return parse_config_sysfs_file(maj, min, device);
+    if (parse_separate_sysfs_files(pci_path, device, false))
+        return parse_config_sysfs_file(pci_path, device);
 
     return 0;
 #elif defined(__OpenBSD__) || defined(__DragonFly__)
-- 
2.54.0

[PATCH libdrm 2/2] xf86drm: Expose auxiliary bus GPUs as DRM_BUS_AUXILIARY

[Alistair Popple]

NVIDIA GPUs supported by the nova-drm driver appear on an auxiliary
bus created by nova-core which binds to the physical PCIe GPU
device. To allow these to be enumerated and supported by libdrm add a
DRM_BUS_AUXILIARY bus type.

As the auxbus may in future contain multiple sibling GPUs for a single
parent we can't take the virtio approach of just reusing the PCI
subsystem parsers and searching up the bus for the PCI device.

Instead we add a proper auxbus parser and device. The bus info carries
the backing PCI device plus the auxiliary device name which uniquely
identifies each GPU even when siblings share a PCI parent.

As the auxbus parser already knows the parent PCI device there is no
need to scan up the bus like virtio does. Instead we modify the existing
drmParsePci functions to allow the device path to be passed in directly.

This relies on the auxiliary device being a direct child of the backing
PCI device in sysfs. The auxbus parser resolves the DRM device's sysfs
path, records the leaf as the auxiliary device name and strips it to
obtain the backing PCI device path. If the parent is not a PCI device
the PCI parsers fail and the device is skipped.

Signed-off-by: Alistair Popple <apopple@nvidia.com>
---
 xf86drm.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++----
 xf86drm.h |  12 ++++++
 2 files changed, 121 insertions(+), 8 deletions(-)

diff --git a/xf86drm.c b/xf86drm.c
index 25f2a3ff..bb22fb11 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -3597,6 +3597,7 @@ static int get_subsystem_type(const char *device_path)
         { "/host1x", DRM_BUS_HOST1X },
         { "/virtio", DRM_BUS_VIRTIO },
         { "/faux", DRM_BUS_FAUX },
+        { "/auxiliary", DRM_BUS_AUXILIARY },
     };
 
     strncpy(path, device_path, PATH_MAX);
@@ -3722,14 +3723,23 @@ static int get_sysctl_pci_bus_info(int maj, int min, drmPciBusInfoPtr info)
 }
 #endif
 
-static int drmParsePciBusInfo(int maj, int min, drmPciBusInfoPtr info)
+static int drmParsePciBusInfo(int maj, int min, const char *pci_path,
+                              drmPciBusInfoPtr info)
 {
 #ifdef __linux__
     unsigned int domain, bus, dev, func;
-    char pci_path[PATH_MAX + 1], *value;
+    char resolved_path[PATH_MAX + 1], *value;
     int num;
 
-    get_pci_path(maj, min, pci_path);
+    /*
+     * Callers that know the backing PCI device (e.g. auxiliary bus devices)
+     * pass its sysfs path in directly. Otherwise resolve it here, possibly by
+     * searching up the bus hierarchy (e.g. in the case of virtio).
+     */
+    if (!pci_path) {
+        get_pci_path(maj, min, resolved_path);
+        pci_path = resolved_path;
+    }
 
     value = sysfs_uevent_get(pci_path, "PCI_SLOT_NAME");
     if (!value)
@@ -3803,6 +3813,9 @@ drm_public int drmDevicesEqual(drmDevicePtr a, drmDevicePtr b)
     case DRM_BUS_FAUX:
         return memcmp(a->businfo.faux, b->businfo.faux, sizeof(drmFauxBusInfo)) == 0;
 
+    case DRM_BUS_AUXILIARY:
+        return memcmp(a->businfo.aux, b->businfo.aux, sizeof(drmAuxBusInfo)) == 0;
+
     default:
         break;
     }
@@ -3902,14 +3915,21 @@ static int parse_config_sysfs_file(const char *pci_path,
 }
 #endif
 
-static int drmParsePciDeviceInfo(int maj, int min,
+static int drmParsePciDeviceInfo(int maj, int min, const char *pci_path,
                                  drmPciDeviceInfoPtr device,
                                  uint32_t flags)
 {
 #ifdef __linux__
-    char pci_path[PATH_MAX + 1];
+    char resolved_path[PATH_MAX + 1];
 
-    get_pci_path(maj, min, pci_path);
+    /*
+     * Callers that already know the backing PCI device (e.g. auxiliary bus
+     * devices) pass its sysfs path in directly; otherwise resolve it here.
+     */
+    if (!pci_path) {
+        get_pci_path(maj, min, resolved_path);
+        pci_path = resolved_path;
+    }
 
     if (!(flags & DRM_DEVICE_GET_PCI_REVISION))
         return parse_separate_sysfs_files(pci_path, device, true);
@@ -4111,7 +4131,7 @@ static int drmProcessPciDevice(drmDevicePtr *device,
 
     dev->businfo.pci = (drmPciBusInfoPtr)addr;
 
-    ret = drmParsePciBusInfo(maj, min, dev->businfo.pci);
+    ret = drmParsePciBusInfo(maj, min, NULL, dev->businfo.pci);
     if (ret)
         goto free_device;
 
@@ -4120,7 +4140,7 @@ static int drmProcessPciDevice(drmDevicePtr *device,
         addr += sizeof(drmPciBusInfo);
         dev->deviceinfo.pci = (drmPciDeviceInfoPtr)addr;
 
-        ret = drmParsePciDeviceInfo(maj, min, dev->deviceinfo.pci, flags);
+        ret = drmParsePciDeviceInfo(maj, min, NULL, dev->deviceinfo.pci, flags);
         if (ret)
             goto free_device;
     }
@@ -4521,6 +4541,84 @@ free_device:
     return ret;
 }
 
+static int drmProcessAuxDevice(drmDevicePtr *device,
+                               const char *node, int node_type,
+                               int maj, int min, bool fetch_deviceinfo,
+                               uint32_t flags)
+{
+#ifdef __linux__
+    char path[PATH_MAX + 1] = "";
+    char real_path[PATH_MAX + 1] = "";
+    drmAuxBusInfoPtr aux;
+    drmDevicePtr dev;
+    char *ptr, *term;
+    int ret;
+
+    /*
+     * This assumes the auxiliary device is a direct child of the backing PCI
+     * device in sysfs, i.e. the DRM device's sysfs parent is the auxiliary
+     * device whose own parent is the PCI device, e.g.
+     *
+     *   /sys/devices/.../0000:01:00.0/nova-core.nova-drm.0
+     *
+     * Resolving the DRM device's sysfs path therefore yields the auxiliary
+     * device path; its leaf is the auxiliary device name and stripping the
+     * leaf leaves the sysfs path of the backing PCI device.
+     */
+    snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
+    if (!realpath(path, real_path))
+        return -errno;
+
+    term = strrchr(real_path, '/');
+    if (!term)
+        return -EINVAL;
+
+    dev = drmDeviceAlloc(node_type, node, sizeof(drmAuxBusInfo),
+                         sizeof(drmPciDeviceInfo), &ptr);
+    if (!dev)
+        return -ENOMEM;
+
+    dev->bustype = DRM_BUS_AUXILIARY;
+    dev->businfo.aux = aux = (drmAuxBusInfoPtr)ptr;
+
+    strncpy(aux->name, term + 1, DRM_AUX_DEVICE_NAME_LEN - 1);
+    aux->name[DRM_AUX_DEVICE_NAME_LEN - 1] = '\0';
+
+    /*
+     * Strip the leaf to get the sysfs path of the physical PCI device to pass
+     * to the PCI parsers. If the parent is not a PCI device the parsers will
+     * fail and the device is skipped.
+     */
+    *term = '\0';
+
+    ret = drmParsePciBusInfo(maj, min, real_path, &aux->parent);
+    if (ret)
+        goto free_device;
+
+    /* Fetch the device info if the user has requested it */
+    if (fetch_deviceinfo) {
+        ptr += sizeof(drmAuxBusInfo);
+        dev->deviceinfo.aux = (drmPciDeviceInfoPtr)ptr;
+
+        ret = drmParsePciDeviceInfo(maj, min, real_path, dev->deviceinfo.aux,
+                                    flags);
+        if (ret)
+            goto free_device;
+    }
+
+    *device = dev;
+
+    return 0;
+
+free_device:
+    free(dev);
+    return ret;
+#else
+#warning "Missing implementation of drmProcessAuxDevice"
+    return -EINVAL;
+#endif
+}
+
 static int
 process_device(drmDevicePtr *device, const char *d_name,
                int req_subsystem_type,
@@ -4564,6 +4662,9 @@ process_device(drmDevicePtr *device, const char *d_name,
     case DRM_BUS_VIRTIO:
         return drmProcessPciDevice(device, node, node_type, maj, min,
                                    fetch_deviceinfo, flags);
+    case DRM_BUS_AUXILIARY:
+        return drmProcessAuxDevice(device, node, node_type, maj, min,
+                                   fetch_deviceinfo, flags);
     case DRM_BUS_USB:
         return drmProcessUsbDevice(device, node, node_type, maj, min,
                                    fetch_deviceinfo, flags);
diff --git a/xf86drm.h b/xf86drm.h
index b45337a4..654e4caf 100644
--- a/xf86drm.h
+++ b/xf86drm.h
@@ -844,6 +844,7 @@ extern char *drmGetRenderDeviceNameFromFd(int fd);
 #define DRM_BUS_PLATFORM  2
 #define DRM_BUS_HOST1X    3
 #define DRM_BUS_FAUX      4
+#define DRM_BUS_AUXILIARY 5
 
 typedef struct _drmPciBusInfo {
     uint16_t domain;
@@ -896,6 +897,15 @@ typedef struct _drmFauxBusInfo {
     char name[DRM_FAUX_DEVICE_NAME_LEN];
 } drmFauxBusInfo, *drmFauxBusInfoPtr;
 
+#define DRM_AUX_DEVICE_NAME_LEN 512
+
+typedef struct _drmAuxBusInfo {
+    drmPciBusInfo parent;               /* Underlying (physical) PCI device */
+    char name[DRM_AUX_DEVICE_NAME_LEN]; /* Uniquely identifies the device on the
+                                           auxiliary bus, e.g.
+                                           "nova-core.nova-drm.0" */
+} drmAuxBusInfo, *drmAuxBusInfoPtr;
+
 typedef struct _drmDevice {
     char **nodes; /* DRM_NODE_MAX sized array */
     int available_nodes; /* DRM_NODE_* bitmask */
@@ -906,12 +916,14 @@ typedef struct _drmDevice {
         drmPlatformBusInfoPtr platform;
         drmHost1xBusInfoPtr host1x;
         drmFauxBusInfoPtr faux;
+        drmAuxBusInfoPtr aux;
     } businfo;
     union {
         drmPciDeviceInfoPtr pci;
         drmUsbDeviceInfoPtr usb;
         drmPlatformDeviceInfoPtr platform;
         drmHost1xDeviceInfoPtr host1x;
+        drmPciDeviceInfoPtr aux; /* Device info of the underlying PCI device */
     } deviceinfo;
 } drmDevice, *drmDevicePtr;
 
-- 
2.54.0

[PATCH 0/2] Add libdrm support for auxbus devices

[Alistair Popple]

The new Nova[1] NVIDIA GPU driver exposes GPUs as logical devices on an
auxbus. Currently libdrm doesn't support auxbus, so it ignores Nova GPUs
during enumeration. This adds support for devices on an auxbus.

Rather than follow the lead of virtio and resolve the logical auxbus device
back to the physical PCI device, we add a new bus type and associated parser.
This is because in future it's a single physical GPU may present multiple auxbus
devices, so it needs to be possible to resolve individual devices even if they
share the same parent GPU.

[1] - https://rust-for-linux.com/nova-gpu-driver

Alistair Popple (2):
  xf86drm: Pass PCI path rather than maj/min when parsing sysfs files
  xf86drm: Expose auxiliary bus GPUs as DRM_BUS_AUXILIARY

 xf86drm.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++-------
 xf86drm.h |  12 +++++
 2 files changed, 130 insertions(+), 17 deletions(-)

-- 
2.54.0