From 873be895ed1fe65010fb84c6b71e2ec0a6b6fc91 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 2 May 2023 02:28:35 +0900 Subject: [PATCH] udev: add USB revision in ID_PATH xHCI host controller may register two (or more?) USB root hubs for USB 2.0 and USB 3.0, and devices under the hubs may have same ID_PATH. So, to avoid the conflict, let's introduce ID_PATH_WITH_USB_REVISION that includes the USB revision. Closes #19406. --- rules.d/60-drm.rules | 9 ++- rules.d/60-persistent-alsa.rules | 3 +- rules.d/60-persistent-input.rules | 10 ++- rules.d/60-persistent-storage-tape.rules | 9 ++- rules.d/60-persistent-storage.rules.in | 20 +++-- rules.d/60-persistent-v4l.rules | 6 +- rules.d/60-serial.rules | 6 +- src/udev/udev-builtin-path_id.c | 98 ++++++++++++++++++++++-- 8 files changed, 134 insertions(+), 27 deletions(-) diff --git a/rules.d/60-drm.rules b/rules.d/60-drm.rules index f7f3435d508..061b2a2a74f 100644 --- a/rules.d/60-drm.rules +++ b/rules.d/60-drm.rules @@ -3,6 +3,9 @@ ACTION!="remove", SUBSYSTEM=="drm", SUBSYSTEMS=="pci|usb|platform", IMPORT{builtin}="path_id" # by-path -ENV{ID_PATH}=="?*", KERNEL=="card*", SYMLINK+="dri/by-path/$env{ID_PATH}-card" -ENV{ID_PATH}=="?*", KERNEL=="controlD*", SYMLINK+="dri/by-path/$env{ID_PATH}-control" -ENV{ID_PATH}=="?*", KERNEL=="renderD*", SYMLINK+="dri/by-path/$env{ID_PATH}-render" +KERNEL=="card*", ENV{ID_PATH}=="?*", SYMLINK+="dri/by-path/$env{ID_PATH}-card" +KERNEL=="card*", ENV{ID_PATH_WITH_USB_REVISION}=="?*", SYMLINK+="dri/by-path/$env{ID_PATH_WITH_USB_REVISION}-card" +KERNEL=="controlD*", ENV{ID_PATH}=="?*", SYMLINK+="dri/by-path/$env{ID_PATH}-control" +KERNEL=="controlD*", ENV{ID_PATH_WITH_USB_REVISION}=="?*", SYMLINK+="dri/by-path/$env{ID_PATH_WITH_USB_REVISION}-control" +KERNEL=="renderD*", ENV{ID_PATH}=="?*", SYMLINK+="dri/by-path/$env{ID_PATH}-render" +KERNEL=="renderD*", ENV{ID_PATH_WITH_USB_REVISION}=="?*", SYMLINK+="dri/by-path/$env{ID_PATH_WITH_USB_REVISION}-render" diff --git a/rules.d/60-persistent-alsa.rules b/rules.d/60-persistent-alsa.rules index 8154e2dbb5e..466ab1c1512 100644 --- a/rules.d/60-persistent-alsa.rules +++ b/rules.d/60-persistent-alsa.rules @@ -9,6 +9,7 @@ ENV{ID_SERIAL}=="?*", ENV{ID_USB_INTERFACE_NUM}=="?*", SYMLINK+="snd/by-id/$env{ ENV{ID_SERIAL}=="?*", ENV{ID_USB_INTERFACE_NUM}=="", SYMLINK+="snd/by-id/$env{ID_BUS}-$env{ID_SERIAL}" IMPORT{builtin}="path_id" -ENV{ID_PATH}=="?*", SYMLINK+="snd/by-path/$env{ID_PATH}" +ENV{ID_PATH}=="?*", SYMLINK+="snd/by-path/$env{ID_PATH}" +ENV{ID_PATH_WITH_USB_REVISION}=="?*", SYMLINK+="snd/by-path/$env{ID_PATH_WITH_USB_REVISION}" LABEL="persistent_alsa_end" diff --git a/rules.d/60-persistent-input.rules b/rules.d/60-persistent-input.rules index 52f4ddb7e63..d02b46caf07 100644 --- a/rules.d/60-persistent-input.rules +++ b/rules.d/60-persistent-input.rules @@ -33,10 +33,14 @@ SUBSYSTEMS=="usb", ENV{ID_BUS}=="?*", KERNEL=="event*", ENV{.INPUT_CLASS}=="", A # by-path SUBSYSTEMS=="pci|usb|platform|acpi", IMPORT{builtin}="path_id" -ENV{ID_PATH}=="?*", KERNEL=="mouse*|js*", ENV{.INPUT_CLASS}=="?*", SYMLINK+="input/by-path/$env{ID_PATH}-$env{.INPUT_CLASS}" -ENV{ID_PATH}=="?*", KERNEL=="event*", ENV{.INPUT_CLASS}=="?*", SYMLINK+="input/by-path/$env{ID_PATH}-event-$env{.INPUT_CLASS}" +ENV{.INPUT_CLASS}=="?*", KERNEL=="mouse*|js*", ENV{ID_PATH}=="?*", SYMLINK+="input/by-path/$env{ID_PATH}-$env{.INPUT_CLASS}" +ENV{.INPUT_CLASS}=="?*", KERNEL=="mouse*|js*", ENV{ID_PATH_WITH_USB_REVISION}=="?*", SYMLINK+="input/by-path/$env{ID_PATH_WITH_USB_REVISION}-$env{.INPUT_CLASS}" +ENV{.INPUT_CLASS}=="?*", KERNEL=="event*", ENV{ID_PATH}=="?*", SYMLINK+="input/by-path/$env{ID_PATH}-event-$env{.INPUT_CLASS}" +ENV{.INPUT_CLASS}=="?*", KERNEL=="event*", ENV{ID_PATH_WITH_USB_REVISION}=="?*", SYMLINK+="input/by-path/$env{ID_PATH_WITH_USB_REVISION}-event-$env{.INPUT_CLASS}" # allow empty class for platform, usb and i2c devices; platform supports only a single interface that way -SUBSYSTEMS=="usb|platform|i2c", ENV{ID_PATH}=="?*", KERNEL=="event*", ENV{.INPUT_CLASS}=="", \ +SUBSYSTEMS=="usb|platform|i2c", KERNEL=="event*", ENV{.INPUT_CLASS}=="", ENV{ID_PATH}=="?*", \ SYMLINK+="input/by-path/$env{ID_PATH}-event" +SUBSYSTEMS=="usb|platform|i2c", KERNEL=="event*", ENV{.INPUT_CLASS}=="", ENV{ID_PATH_WITH_USB_REVISION}=="?*", \ + SYMLINK+="input/by-path/$env{ID_PATH_WITH_USB_REVISION}-event" LABEL="persistent_input_end" diff --git a/rules.d/60-persistent-storage-tape.rules b/rules.d/60-persistent-storage-tape.rules index 803f82c583e..0678d71e6d5 100644 --- a/rules.d/60-persistent-storage-tape.rules +++ b/rules.d/60-persistent-storage-tape.rules @@ -19,7 +19,8 @@ ENV{ID_SERIAL}=="?*", SYMLINK+="tape/by-id/scsi-$env{ID_SERIAL} tape/by-id/scsi- ENV{ID_SCSI_SERIAL}=="?*", SYMLINK+="tape/by-id/scsi-$env{ID_SCSI_SERIAL}" IMPORT{builtin}="path_id" -ENV{ID_PATH}=="?*", SYMLINK+="tape/by-path/$env{ID_PATH}-changer" +ENV{ID_PATH}=="?*", SYMLINK+="tape/by-path/$env{ID_PATH}-changer" +ENV{ID_PATH_WITH_USB_REVISION}=="?*", SYMLINK+="tape/by-path/$env{ID_PATH_WITH_USB_REVISION}-changer" LABEL="medium_changer_end" @@ -36,7 +37,9 @@ KERNEL=="nst*[0-9]", ENV{ID_SCSI_SERIAL}=="?*", SYMLINK+="tape/by-id/$env{ID_BUS # by-path (parent device path) KERNEL=="st*[0-9]|nst*[0-9]", IMPORT{builtin}="path_id" -KERNEL=="st*[0-9]", ENV{ID_PATH}=="?*", SYMLINK+="tape/by-path/$env{ID_PATH}" -KERNEL=="nst*[0-9]", ENV{ID_PATH}=="?*", SYMLINK+="tape/by-path/$env{ID_PATH}-nst" +KERNEL=="st*[0-9]", ENV{ID_PATH}=="?*", SYMLINK+="tape/by-path/$env{ID_PATH}" +KERNEL=="st*[0-9]", ENV{ID_PATH_WITH_USB_REVISION}=="?*", SYMLINK+="tape/by-path/$env{ID_PATH_WITH_USB_REVISION}" +KERNEL=="nst*[0-9]", ENV{ID_PATH}=="?*", SYMLINK+="tape/by-path/$env{ID_PATH}-nst" +KERNEL=="nst*[0-9]", ENV{ID_PATH_WITH_USB_REVISION}=="?*", SYMLINK+="tape/by-path/$env{ID_PATH_WITH_USB_REVISION}-nst" LABEL="persistent_storage_tape_end" diff --git a/rules.d/60-persistent-storage.rules.in b/rules.d/60-persistent-storage.rules.in index 88f88a83edb..4b921bfe85c 100644 --- a/rules.d/60-persistent-storage.rules.in +++ b/rules.d/60-persistent-storage.rules.in @@ -111,16 +111,20 @@ KERNEL=="msblk[0-9]p[0-9]|mspblk[0-9]p[0-9]", ENV{ID_NAME}=="?*", ENV{ID_SERIAL} # by-path ENV{DEVTYPE}=="disk", DEVPATH!="*/virtual/*", IMPORT{builtin}="path_id" ENV{DEVTYPE}=="disk", SUBSYSTEMS=="nvme-subsystem", IMPORT{builtin}="path_id" -KERNEL=="mmcblk[0-9]boot[0-9]", ENV{DEVTYPE}=="disk", ENV{ID_PATH}=="?*", SYMLINK+="disk/by-path/$env{ID_PATH}-boot%n" -KERNEL!="mmcblk[0-9]boot[0-9]", ENV{DEVTYPE}=="disk", ENV{ID_PATH}=="?*", SYMLINK+="disk/by-path/$env{ID_PATH}" -ENV{DEVTYPE}=="partition", ENV{ID_PATH}=="?*", SYMLINK+="disk/by-path/$env{ID_PATH}-part%n" -# compatible links for ATA devices -KERNEL!="mmcblk[0-9]boot[0-9]", ENV{DEVTYPE}=="disk", ENV{ID_PATH_ATA_COMPAT}=="?*", SYMLINK+="disk/by-path/$env{ID_PATH_ATA_COMPAT}" -ENV{DEVTYPE}=="partition", ENV{ID_PATH_ATA_COMPAT}=="?*", SYMLINK+="disk/by-path/$env{ID_PATH_ATA_COMPAT}-part%n" +KERNEL=="mmcblk[0-9]boot[0-9]", ENV{DEVTYPE}=="disk", ENV{ID_PATH}=="?*", SYMLINK+="disk/by-path/$env{ID_PATH}-boot%n" +KERNEL=="mmcblk[0-9]boot[0-9]", ENV{DEVTYPE}=="disk", ENV{ID_PATH_WITH_USB_REVISION}=="?*", SYMLINK+="disk/by-path/$env{ID_PATH_WITH_USB_REVISION}-boot%n" +KERNEL!="mmcblk[0-9]boot[0-9]", ENV{DEVTYPE}=="disk", ENV{ID_PATH}=="?*", SYMLINK+="disk/by-path/$env{ID_PATH}" +KERNEL!="mmcblk[0-9]boot[0-9]", ENV{DEVTYPE}=="disk", ENV{ID_PATH_ATA_COMPAT}=="?*", SYMLINK+="disk/by-path/$env{ID_PATH_ATA_COMPAT}" +KERNEL!="mmcblk[0-9]boot[0-9]", ENV{DEVTYPE}=="disk", ENV{ID_PATH_WITH_USB_REVISION}=="?*", SYMLINK+="disk/by-path/$env{ID_PATH_WITH_USB_REVISION}" +ENV{DEVTYPE}=="partition", ENV{ID_PATH}=="?*", SYMLINK+="disk/by-path/$env{ID_PATH}-part%n" +ENV{DEVTYPE}=="partition", ENV{ID_PATH_ATA_COMPAT}=="?*", SYMLINK+="disk/by-path/$env{ID_PATH_ATA_COMPAT}-part%n" +ENV{DEVTYPE}=="partition", ENV{ID_PATH_WITH_USB_REVISION}=="?*", SYMLINK+="disk/by-path/$env{ID_PATH_WITH_USB_REVISION}-part%n" # legacy virtio-pci by-path links (deprecated) -KERNEL=="vd*[!0-9]", ENV{ID_PATH}=="pci-*", SYMLINK+="disk/by-path/virtio-$env{ID_PATH}" -KERNEL=="vd*[0-9]", ENV{ID_PATH}=="pci-*", SYMLINK+="disk/by-path/virtio-$env{ID_PATH}-part%n" +KERNEL=="vd*[!0-9]", ENV{ID_PATH}=="pci-*", SYMLINK+="disk/by-path/virtio-$env{ID_PATH}" +KERNEL=="vd*[!0-9]", ENV{ID_PATH_WITH_USB_REVISION}=="pci-*", SYMLINK+="disk/by-path/virtio-$env{ID_PATH_WITH_USB_REVISION}" +KERNEL=="vd*[0-9]", ENV{ID_PATH}=="pci-*", SYMLINK+="disk/by-path/virtio-$env{ID_PATH}-part%n" +KERNEL=="vd*[0-9]", ENV{ID_PATH_WITH_USB_REVISION}=="pci-*", SYMLINK+="disk/by-path/virtio-$env{ID_PATH_WITH_USB_REVISION}-part%n" {% if HAVE_BLKID %} # allow admin to disable probing the filesystem for slow devices like floppy disk drives diff --git a/rules.d/60-persistent-v4l.rules b/rules.d/60-persistent-v4l.rules index 93c5ee8c276..071650a45b0 100644 --- a/rules.d/60-persistent-v4l.rules +++ b/rules.d/60-persistent-v4l.rules @@ -14,7 +14,9 @@ TEST!="index", GOTO="persistent_v4l_end" ATTR{index}!="?*", GOTO="persistent_v4l_end" IMPORT{builtin}="path_id" -ENV{ID_PATH}=="?*", KERNEL=="video*|vbi*", SYMLINK+="v4l/by-path/$env{ID_PATH}-video-index$attr{index}" -ENV{ID_PATH}=="?*", KERNEL=="audio*", SYMLINK+="v4l/by-path/$env{ID_PATH}-audio-index$attr{index}" +KERNEL=="video*|vbi*", ENV{ID_PATH}=="?*", SYMLINK+="v4l/by-path/$env{ID_PATH}-video-index$attr{index}" +KERNEL=="video*|vbi*", ENV{ID_PATH_WITH_USB_REVISION}=="?*", SYMLINK+="v4l/by-path/$env{ID_PATH_WITH_USB_REVISION}-video-index$attr{index}" +KERNEL=="audio*", ENV{ID_PATH}=="?*", SYMLINK+="v4l/by-path/$env{ID_PATH}-audio-index$attr{index}" +KERNEL=="audio*", ENV{ID_PATH_WITH_USB_REVISION}=="?*", SYMLINK+="v4l/by-path/$env{ID_PATH_WITH_USB_REVISION}-audio-index$attr{index}" LABEL="persistent_v4l_end" diff --git a/rules.d/60-serial.rules b/rules.d/60-serial.rules index a0e66323a9b..04321227342 100644 --- a/rules.d/60-serial.rules +++ b/rules.d/60-serial.rules @@ -14,8 +14,10 @@ KERNEL!="ttyUSB[0-9]*|ttyACM[0-9]*", GOTO="serial_end" SUBSYSTEMS=="usb-serial", ENV{.ID_PORT}="$attr{port_number}" IMPORT{builtin}="path_id" -ENV{ID_PATH}=="?*", ENV{.ID_PORT}=="", SYMLINK+="serial/by-path/$env{ID_PATH}" -ENV{ID_PATH}=="?*", ENV{.ID_PORT}=="?*", SYMLINK+="serial/by-path/$env{ID_PATH}-port$env{.ID_PORT}" +ENV{ID_PATH}=="?*", ENV{.ID_PORT}=="", SYMLINK+="serial/by-path/$env{ID_PATH}" +ENV{ID_PATH_WITH_USB_REVISION}=="?*", ENV{.ID_PORT}=="", SYMLINK+="serial/by-path/$env{ID_PATH_WITH_USB_REVISION}" +ENV{ID_PATH}=="?*", ENV{.ID_PORT}=="?*", SYMLINK+="serial/by-path/$env{ID_PATH}-port$env{.ID_PORT}" +ENV{ID_PATH_WITH_USB_REVISION}=="?*", ENV{.ID_PORT}=="?*", SYMLINK+="serial/by-path/$env{ID_PATH_WITH_USB_REVISION}-port$env{.ID_PORT}" ENV{ID_BUS}=="", GOTO="serial_end" ENV{ID_SERIAL}=="", GOTO="serial_end" diff --git a/src/udev/udev-builtin-path_id.c b/src/udev/udev-builtin-path_id.c index 2bdeb40d50d..467c9a6ad39 100644 --- a/src/udev/udev-builtin-path_id.c +++ b/src/udev/udev-builtin-path_id.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -488,8 +489,51 @@ static void handle_scsi_tape(sd_device *dev, char **path) { path_prepend(path, "st%c", name[2]); } +static int get_usb_revision(sd_device *dev) { + uint8_t protocol; + const char *s; + int r; + + assert(dev); + + /* Returns usb revision 1, 2, or 3. */ + + r = sd_device_get_sysattr_value(dev, "bDeviceProtocol", &s); + if (r < 0) + return r; + + r = safe_atou8_full(s, 16, &protocol); + if (r < 0) + return r; + + switch (protocol) { + case USB_HUB_PR_HS_NO_TT: /* Full speed hub (USB1) or Hi-speed hub without TT (USB2) */ + + /* See speed_show() in drivers/usb/core/sysfs.c of the kernel. */ + r = sd_device_get_sysattr_value(dev, "speed", &s); + if (r < 0) + return r; + + if (streq(s, "480")) + return 2; + + return 1; + + case USB_HUB_PR_HS_SINGLE_TT: /* Hi-speed hub with single TT */ + case USB_HUB_PR_HS_MULTI_TT: /* Hi-speed hub with multiple TT */ + return 2; + + case USB_HUB_PR_SS: /* Super speed hub */ + return 3; + + default: + return -EPROTONOSUPPORT; + } +} + static sd_device *handle_usb(sd_device *parent, char **path) { const char *devtype, *str, *port; + int r; if (sd_device_get_devtype(parent, &devtype) < 0) return parent; @@ -503,12 +547,28 @@ static sd_device *handle_usb(sd_device *parent, char **path) { return parent; port++; - /* USB host number may change across reboots (and probably even without reboot). The part after - * USB host number is determined by device topology and so does not change. Hence, drop the - * host number and always use '0' instead. */ + parent = skip_subsystem(parent, "usb"); + if (!parent) + return NULL; - path_prepend(path, "usb-0:%s", port); - return skip_subsystem(parent, "usb"); + /* USB host number may change across reboots (and probably even without reboot). The part after USB + * host number is determined by device topology and so does not change. Hence, drop the host number + * and always use '0' instead. + * + * xHCI host controllers may register two (or more?) USB root hubs for USB 2.0 and USB 3.0, and the + * sysname, whose host number replaced with 0, of a device under the hubs may conflict with others. + * To avoid the conflict, let's include the USB revision of the root hub to the PATH_ID. + * See issue https://github.com/systemd/systemd/issues/19406 for more details. */ + r = get_usb_revision(parent); + if (r < 0) { + log_device_debug_errno(parent, r, "Failed to get the USB revision number, ignoring: %m"); + path_prepend(path, "usb-0:%s", port); + } else { + assert(r > 0); + path_prepend(path, "usbv%i-0:%s", r, port); + } + + return parent; } static sd_device *handle_bcma(sd_device *parent, char **path) { @@ -590,6 +650,32 @@ static int find_real_nvme_parent(sd_device *dev, sd_device **ret) { return 0; } +static void add_id_with_usb_revision(sd_device *dev, bool test, char *path) { + char *p; + int r; + + assert(dev); + assert(path); + + /* When the path contains the USB revision, let's adds ID_PATH_WITH_USB_REVISION property and + * drop the version specifier for later use. */ + + p = strstrafter(path, "-usbv"); + if (!p) + return; + if (!ascii_isdigit(p[0])) + return; + if (p[1] != '-') + return; + + r = udev_builtin_add_property(dev, test, "ID_PATH_WITH_USB_REVISION", path); + if (r < 0) + log_device_debug_errno(dev, r, "Failed to add ID_PATH_WITH_USB_REVISION property, ignoring: %m"); + + /* Drop the USB revision specifier for backward compatibility. */ + memmove(p - 1, p + 1, strlen(p + 1) + 1); +} + static void add_id_tag(sd_device *dev, bool test, const char *path) { char tag[UDEV_NAME_SIZE]; size_t i = 0; @@ -783,6 +869,8 @@ static int builtin_path_id(UdevEvent *event, int argc, char *argv[], bool test) !supported_transport) return -ENOENT; + add_id_with_usb_revision(dev, test, path); + r = udev_builtin_add_property(dev, test, "ID_PATH", path); if (r < 0) log_device_debug_errno(dev, r, "Failed to add ID_PATH property, ignoring: %m");