[PATCH 0/3] MR8769: winebus: Miscellaneous changes relating to device identification.
These patches are prerequisites for getting DualShock 4 controllers to work via USB in Tekken 8. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8769
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winebus.sys/main.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c index ba8603204d4..0a871269fa9 100644 --- a/dlls/winebus.sys/main.c +++ b/dlls/winebus.sys/main.c @@ -289,6 +289,18 @@ static void remove_pending_irps(DEVICE_OBJECT *device) } } +static void make_unique_serial(struct device_extension *device) +{ + struct device_extension *ext; + + LIST_FOR_EACH_ENTRY(ext, &device_list, struct device_extension, entry) + if (!wcscmp(device->desc.serialnumber, ext->desc.serialnumber)) break; + if (&ext->entry == &device_list && *device->desc.serialnumber) return; + + swprintf(device->desc.serialnumber, ARRAY_SIZE(device->desc.serialnumber), L"%04x%08x%04x%04x", + device->index, device->desc.input, device->desc.pid, device->desc.vid); +} + static DEVICE_OBJECT *bus_create_hid_device(struct device_desc *desc, UINT64 unix_device) { struct device_extension *ext; @@ -333,6 +345,10 @@ static DEVICE_OBJECT *bus_create_hid_device(struct device_desc *desc, UINT64 uni InitializeCriticalSectionEx(&ext->cs, 0, RTL_CRITICAL_SECTION_FLAG_FORCE_DEBUG_INFO); ext->cs.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": cs"); + /* Overcooked! All You Can Eat only adds controllers with unique serial numbers + * Prefer keeping serial numbers unique over keeping them consistent across runs */ + make_unique_serial(ext); + /* add to list of pnp devices */ if (before) list_add_before(before, &ext->entry); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8769
From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/winebus.sys/main.c | 47 ++++++++++++++++++++++++++++++++++++++ dlls/winebus.sys/unixlib.h | 2 ++ 2 files changed, 49 insertions(+) diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c index 0a871269fa9..49b805b713e 100644 --- a/dlls/winebus.sys/main.c +++ b/dlls/winebus.sys/main.c @@ -41,6 +41,9 @@ #include "unixlib.h" +#include "initguid.h" +DEFINE_GUID(GUID_NULL,0,0,0,0,0,0,0,0,0,0,0); + WINE_DEFAULT_DEBUG_CHANNEL(hid); static DRIVER_OBJECT *driver_obj; @@ -264,6 +267,24 @@ static WCHAR *get_compatible_ids(DEVICE_OBJECT *device) return dst; } +#define GUID_STRING_LENGTH 39 +static WCHAR *get_container_id(DEVICE_OBJECT *device) +{ + struct device_extension *ext = (struct device_extension *)device->DeviceExtension; + GUID *guid = &ext->desc.container_id; + WCHAR *dst; + + if ((dst = ExAllocatePool(PagedPool, GUID_STRING_LENGTH * sizeof(WCHAR)))) + { + swprintf(dst, GUID_STRING_LENGTH, L"{%08lX-%04X-%04X-%02X%02X-%02X%02X%02X%02X%02X%02X}", + guid->Data1, guid->Data2, guid->Data3, guid->Data4[0], guid->Data4[1], guid->Data4[2], guid->Data4[3], + guid->Data4[4], guid->Data4[5], guid->Data4[6], guid->Data4[7]); + } + + TRACE("Returning container ID %s.\n", debugstr_w(dst)); + return dst; +} + static IRP *pop_pending_read(struct device_extension *ext) { IRP *pending; @@ -301,6 +322,21 @@ static void make_unique_serial(struct device_extension *device) device->index, device->desc.input, device->desc.pid, device->desc.vid); } +static void make_unique_container_id(struct device_extension *device) +{ + struct device_extension *ext; + + LIST_FOR_EACH_ENTRY(ext, &device_list, struct device_extension, entry) + if (IsEqualGUID(&device->desc.container_id, &ext->desc.container_id)) break; + if (&ext->entry == &device_list && !IsEqualGUID(&device->desc.container_id, &GUID_NULL)) return; + + device->desc.container_id.Data1 = device->desc.input; + device->desc.container_id.Data2 = device->index; + device->desc.container_id.Data3 = device->desc.pid; + device->desc.container_id.Data4[0] = (device->desc.vid >> 8) & 0xff; + device->desc.container_id.Data4[1] = (device->desc.vid) & 0xff; +} + static DEVICE_OBJECT *bus_create_hid_device(struct device_desc *desc, UINT64 unix_device) { struct device_extension *ext; @@ -349,6 +385,13 @@ static DEVICE_OBJECT *bus_create_hid_device(struct device_desc *desc, UINT64 uni * Prefer keeping serial numbers unique over keeping them consistent across runs */ make_unique_serial(ext); + /* + * Some games use container ID to match the bus device to the HID + * device in order to get things like DEVPKEY_Device_BusReportedDeviceDesc. + * Create a unique container ID to facilitate this. + */ + make_unique_container_id(ext); + /* add to list of pnp devices */ if (before) list_add_before(before, &ext->entry); @@ -707,6 +750,10 @@ static NTSTATUS handle_IRP_MN_QUERY_ID(DEVICE_OBJECT *device, IRP *irp) TRACE("BusQueryInstanceID\n"); irp->IoStatus.Information = (ULONG_PTR)get_instance_id(device); break; + case BusQueryContainerID: + TRACE("BusQueryContainerID\n"); + irp->IoStatus.Information = (ULONG_PTR)get_container_id(device); + break; default: WARN("Unhandled type %08x\n", type); return status; diff --git a/dlls/winebus.sys/unixlib.h b/dlls/winebus.sys/unixlib.h index 8dab8af0d57..f8626438c92 100644 --- a/dlls/winebus.sys/unixlib.h +++ b/dlls/winebus.sys/unixlib.h @@ -45,6 +45,8 @@ struct device_desc WCHAR manufacturer[MAX_PATH]; WCHAR product[MAX_PATH]; WCHAR serialnumber[MAX_PATH]; + + GUID container_id; }; struct device_options -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8769
From: Connor McAdams <cmcadams(a)codeweavers.com> Some games use the bus device instance to query properties of a HID device. Tekken 8 does this to get a string for the device to display in the UI. It queries all present devices for their container IDs, and when it finds one with the same container ID as the HID device, it checks the device instance string for a "BTHENUM" or "USB" enumerator. Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/winebus.sys/bus_iohid.c | 9 +++++++-- dlls/winebus.sys/bus_udev.c | 3 ++- dlls/winebus.sys/main.c | 22 ++++++++++++++++------ dlls/winebus.sys/unixlib.h | 14 +++++++++++--- 4 files changed, 36 insertions(+), 12 deletions(-) diff --git a/dlls/winebus.sys/bus_iohid.c b/dlls/winebus.sys/bus_iohid.c index 648d55dadb8..0b9a0c52c46 100644 --- a/dlls/winebus.sys/bus_iohid.c +++ b/dlls/winebus.sys/bus_iohid.c @@ -287,8 +287,13 @@ static void handle_DeviceMatchingCallback(void *context, IOReturn result, void * desc.uid = CFNumberToDWORD(IOHIDDeviceGetProperty(IOHIDDevice, CFSTR(kIOHIDLocationIDKey))); if ((str = IOHIDDeviceGetProperty(IOHIDDevice, CFSTR(kIOHIDTransportKey)))) - desc.is_bluetooth = !CFStringCompare(str, CFSTR(kIOHIDTransportBluetoothValue), 0) || - !CFStringCompare(str, CFSTR(kIOHIDTransportBluetoothLowEnergyValue), 0); + { + if (!CFStringCompare(str, CFSTR(kIOHIDTransportBluetoothValue), 0) || + !CFStringCompare(str, CFSTR(kIOHIDTransportBluetoothLowEnergyValue), 0)) + desc.bus_type = BUS_TYPE_BLUETOOTH; + else if (!CFStringCompare(str, CFSTR(kIOHIDTransportUSBValue), 0)) + desc.bus_type = BUS_TYPE_USB; + } if (usages.UsagePage != HID_USAGE_PAGE_GENERIC || !(usages.Usage == HID_USAGE_GENERIC_JOYSTICK || usages.Usage == HID_USAGE_GENERIC_GAMEPAD)) diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c index 4f929d0cb45..fec7a74f989 100644 --- a/dlls/winebus.sys/bus_udev.c +++ b/dlls/winebus.sys/bus_udev.c @@ -1362,7 +1362,8 @@ static void udev_add_device(struct udev_device *dev, int fd) get_device_subsystem_info(dev, "hid", NULL, &desc, &bus); get_device_subsystem_info(dev, "input", NULL, &desc, &bus); get_device_subsystem_info(dev, "usb", "usb_device", &desc, &bus); - if (bus == BUS_BLUETOOTH) desc.is_bluetooth = TRUE; + if (bus == BUS_BLUETOOTH) desc.bus_type = BUS_TYPE_BLUETOOTH; + else if (bus == BUS_USB) desc.bus_type = BUS_TYPE_USB; if (!(subsystem = udev_device_get_subsystem(dev))) { diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c index 49b805b713e..d839f451809 100644 --- a/dlls/winebus.sys/main.c +++ b/dlls/winebus.sys/main.c @@ -203,21 +203,31 @@ static WCHAR *get_instance_id(DEVICE_OBJECT *device) return dst; } +static const WCHAR *bus_type_str[] = +{ + L"WINEBUS", /* BUS_TYPE_UNKNOWN */ + L"USB", /* BUS_TYPE_USB */ + L"BTHENUM", /* BUS_TYPE_BLUETOOTH */ +}; + static WCHAR *get_device_id(DEVICE_OBJECT *device) { static const WCHAR input_format[] = L"&MI_%02u"; - static const WCHAR winebus_format[] = L"WINEBUS\\VID_%04X&PID_%04X"; + static const WCHAR winebus_format[] = L"%s\\VID_%04X&PID_%04X"; struct device_extension *ext = (struct device_extension *)device->DeviceExtension; - DWORD pos = 0, len = 0, input_len = 0, winebus_len = 25; + DWORD pos = 0, len = 0, input_len = 0, winebus_len = 18; + const WCHAR *bus_str; WCHAR *dst; + assert(ext->desc.bus_type < BUS_TYPE_COUNT); + bus_str = bus_type_str[ext->desc.bus_type]; if (ext->desc.input != -1) input_len = 14; - len += winebus_len + input_len + 1; + len += winebus_len + input_len + wcslen(bus_str) + 1; if ((dst = ExAllocatePool(PagedPool, len * sizeof(WCHAR)))) { - pos += swprintf(dst + pos, len - pos, winebus_format, ext->desc.vid, ext->desc.pid); + pos += swprintf(dst + pos, len - pos, winebus_format, bus_str, ext->desc.vid, ext->desc.pid); if (input_len) pos += swprintf(dst + pos, len - pos, input_format, ext->desc.input); } @@ -367,12 +377,12 @@ static DEVICE_OBJECT *bus_create_hid_device(struct device_desc *desc, UINT64 uni ext->unix_device = unix_device; list_init(&ext->reports); - if (desc->is_hidraw && desc->is_bluetooth && is_dualshock4_gamepad(desc->vid, desc->pid)) + if (desc->is_hidraw && (desc->bus_type == BUS_TYPE_BLUETOOTH) && is_dualshock4_gamepad(desc->vid, desc->pid)) { TRACE("Enabling report fixup for Bluetooth DualShock4 device %p\n", device); ext->report_fixups |= HIDRAW_FIXUP_DUALSHOCK_BT; } - if (desc->is_hidraw && desc->is_bluetooth && is_dualsense_gamepad(desc->vid, desc->pid)) + if (desc->is_hidraw && (desc->bus_type == BUS_TYPE_BLUETOOTH) && is_dualsense_gamepad(desc->vid, desc->pid)) { TRACE("Enabling report fixup for Bluetooth DualSense device %p\n", device); ext->report_fixups |= HIDRAW_FIXUP_DUALSENSE_BT; diff --git a/dlls/winebus.sys/unixlib.h b/dlls/winebus.sys/unixlib.h index f8626438c92..b296f20178d 100644 --- a/dlls/winebus.sys/unixlib.h +++ b/dlls/winebus.sys/unixlib.h @@ -38,9 +38,9 @@ struct device_desc UINT version; UINT input; UINT uid; + UINT bus_type; BOOL is_gamepad; BOOL is_hidraw; - BOOL is_bluetooth; WCHAR manufacturer[MAX_PATH]; WCHAR product[MAX_PATH]; @@ -78,6 +78,14 @@ enum bus_event_type BUS_EVENT_TYPE_INPUT_REPORT, }; +enum bus_type +{ + BUS_TYPE_UNKNOWN, + BUS_TYPE_USB, + BUS_TYPE_BLUETOOTH, + BUS_TYPE_COUNT, +}; + struct bus_event { UINT type; @@ -153,9 +161,9 @@ enum unix_funcs static inline const char *debugstr_device_desc(struct device_desc *desc) { if (!desc) return "(null)"; - return wine_dbg_sprintf("{vid %04x, pid %04x, version %04x, input %d, uid %08x, is_gamepad %u, is_hidraw %u, is_bluetooth %u}", + return wine_dbg_sprintf("{vid %04x, pid %04x, version %04x, input %d, uid %08x, is_gamepad %u, is_hidraw %u, bus_type %u}", desc->vid, desc->pid, desc->version, desc->input, desc->uid, - desc->is_gamepad, desc->is_hidraw, desc->is_bluetooth); + desc->is_gamepad, desc->is_hidraw, desc->bus_type); } static inline BOOL is_xbox_gamepad(WORD vid, WORD pid) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8769
Rémi Bernon (@rbernon) commented about dlls/winebus.sys/unixlib.h:
WCHAR manufacturer[MAX_PATH]; WCHAR product[MAX_PATH]; WCHAR serialnumber[MAX_PATH]; + + GUID container_id;
Do we need to store it here? It's not accessed from the unix side. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8769#note_113039
Rémi Bernon (@rbernon) commented about dlls/winebus.sys/main.c:
device->index, device->desc.input, device->desc.pid, device->desc.vid); }
+static void make_unique_container_id(struct device_extension *device) +{ + struct device_extension *ext; + + LIST_FOR_EACH_ENTRY(ext, &device_list, struct device_extension, entry) + if (IsEqualGUID(&device->desc.container_id, &ext->desc.container_id)) break; + if (&ext->entry == &device_list && !IsEqualGUID(&device->desc.container_id, &GUID_NULL)) return; + + device->desc.container_id.Data1 = device->desc.input; + device->desc.container_id.Data2 = device->index; + device->desc.container_id.Data3 = device->desc.pid; + device->desc.container_id.Data4[0] = (device->desc.vid >> 8) & 0xff; + device->desc.container_id.Data4[1] = (device->desc.vid) & 0xff;
I don't expect input/index to be usually large, what about putting MAKELONG(vid,pid) in Data1, index in Data2 and input in Data3. This leaves Data4 free for now, which could be initialized to something unique? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8769#note_113040
Rémi Bernon (@rbernon) commented about dlls/winebus.sys/main.c:
InitializeCriticalSectionEx(&ext->cs, 0, RTL_CRITICAL_SECTION_FLAG_FORCE_DEBUG_INFO); ext->cs.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": cs");
+ /* Overcooked! All You Can Eat only adds controllers with unique serial numbers + * Prefer keeping serial numbers unique over keeping them consistent across runs */ + make_unique_serial(ext);
Let's remove the code that does the same in bus_sdl.c only while at it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8769#note_113041
On Mon Aug 18 09:21:59 2025 +0000, Rémi Bernon wrote:
Do we need to store it here? It's not accessed from the unix side. Not immediately, but in a later patch I use udev to create a container ID that is able to be reproduced in `winepulse.drv` for DualSense support. I can keep it out of unix code until that gets upstream, if that makes more sense here.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8769#note_113055
On Mon Aug 18 09:21:59 2025 +0000, Rémi Bernon wrote:
Let's remove the code that does the same in bus_sdl.c only while at it. Would it be better to do this in a separate patch, or in the first patch? The reason I ask is because the first patch is authored by you, so I think I'd need permission to change it there. :)
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8769#note_113056
On Mon Aug 18 09:21:59 2025 +0000, Rémi Bernon wrote:
I don't expect input/index to be usually large, what about putting MAKELONG(vid,pid) in Data1, index in Data2 and input in Data3. This leaves Data4 free for now, which could be initialized to something unique? In the patch I have for creating container ID from udev information, I use the `USEC_INITIALIZED` timestamp value in Data4. We could maybe generate a timestamp here to add extra uniqueness, if that makes sense.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8769#note_113057
On Mon Aug 18 12:09:33 2025 +0000, Connor McAdams wrote:
Would it be better to do this in a separate patch, or in the first patch? The reason I ask is because the first patch is authored by you, so I think I'd need permission to change it there. :) Sure, feel free to squash the change with the first patch.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8769#note_113059
On Mon Aug 18 12:08:16 2025 +0000, Connor McAdams wrote:
Not immediately, but in a later patch I use udev to create a container ID that is able to be reproduced in `winepulse.drv` for DualSense support. I can keep it out of unix code until that gets upstream, if that makes more sense here. Still feels a bit weird if we're filling it on the PE side after all. If we're going to pass only the usec from the unix side (optionally iiuc), maybe it'd be better to only have a timestamp field there.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8769#note_113060
On Mon Aug 18 12:11:23 2025 +0000, Connor McAdams wrote:
In the patch I have for creating container ID from udev information, I use the `USEC_INITIALIZED` timestamp value in Data4. We could maybe generate a timestamp here to add extra uniqueness, if that makes sense. Yeah I think it could be good to avoid differences between all of the backends. The backend can provide a specific device timestamp, or the code here could make up one unique.
To be clear, are we confident that USEC_INITIALIZED is identical across all of the Linux devices of the same physical device and hence usable as container ID, right? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8769#note_113061
On Mon Aug 18 13:40:06 2025 +0000, Rémi Bernon wrote:
Still feels a bit weird if we're filling it on the PE side after all. If we're going to pass only the usec from the unix side (optionally iiuc), maybe it'd be better to only have a timestamp field there. So, my initial idea was to have the code here be a kind of fallback similar to how `make_unique_serial()` only creates a string for `serialnumber` if the bus backend doesn't provide one. We always need a unique container ID in order to match the `{WINEBUS,BTHENUM,USB}\\` bus device instance with the `HID\\` device instance, regardless of the bus back end being used.
Eventually winepulse.drv needs a way to match a USB audio device's container ID to the bus/hid device instance. To do that, I've used udev, the patches for it are the final two in my branch [here.](https://gitlab.winehq.org/cmcadams/wine/-/commits/WIP/udev-container-id-v1) Each HID bus back end provides different information that can be used to populate a container ID. As an example, udev provides `BUSNUM` and `DEVNUM`, IOHID provides `kIOHIDLocationIDKey`, which is similar but the value ranges aren't the same. It seems like it'd be difficult to create a set of variables to populate in each back end that ultimately end up creating a container ID, so I chose instead to create one in each back end, with a fallback if that isn't possible. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8769#note_113083
On Mon Aug 18 13:41:57 2025 +0000, Rémi Bernon wrote:
Yeah I think it could be good to avoid differences between all of the backends. The backend can provide a specific device timestamp, or the code here could make up one unique. To be clear, are we confident that USEC_INITIALIZED is identical across all of the Linux devices of the same physical device and hence usable as container ID, right? `USEC_INITIALIZED` is different across the various udev devices created for the controller, but if we use the root USB udev device we can get a consistent `USEC_INITIALIZED` value. See [here](https://gitlab.winehq.org/cmcadams/wine/-/commit/426547f138195a57ab7b88ffb8c... winepulse.drv, and [here](https://gitlab.winehq.org/cmcadams/wine/-/commit/2bf60d670d2eb76b74b578995e8...) in winebus.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8769#note_113086
participants (3)
-
Connor McAdams -
Connor McAdams (＠cmcadams)
-
Rémi Bernon