[PATCH v6 0/7] MR432: Add support for SPDRP_BASE_CONTAINERID by querying drivers with BusQueryContainerID
This is the hidclass/ntoskrnl/setupapi counterpart to !359. As with that merge request, implementing `BusQueryContainerID` in the bus driver is left out and will be addressed in a different MR.
I believe this is useful to expose the Container ID no matter how those end up being assigned by the bus driver.
-- v6: ntoskrnl: Set device ContainerID from driver
From: Claire Girka claire@sitedethib.com
--- dlls/setupapi/devinst.c | 2 ++ include/setupapi.h | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/dlls/setupapi/devinst.c b/dlls/setupapi/devinst.c index 73e721753aa..901af4650c8 100644 --- a/dlls/setupapi/devinst.c +++ b/dlls/setupapi/devinst.c @@ -96,6 +96,7 @@ static const WCHAR Capabilities[] = {'C','a','p','a','b','i','l','i','t','i','e' static const WCHAR UINumber[] = {'U','I','N','u','m','b','e','r',0}; static const WCHAR UpperFilters[] = {'U','p','p','e','r','F','i','l','t','e','r','s',0}; static const WCHAR LowerFilters[] = {'L','o','w','e','r','F','i','l','t','e','r','s',0}; +static const WCHAR ContainerId[] = {'C','o','n','t','a','i','n','e','r','I','d',0}; static const WCHAR Phantom[] = {'P','h','a','n','t','o','m',0}; static const WCHAR SymbolicLink[] = {'S','y','m','b','o','l','i','c','L','i','n','k',0}; static const WCHAR Control[] = {'C','o','n','t','r','o','l',0}; @@ -654,6 +655,7 @@ static const struct PropertyMapEntry PropertyMap[] = { { REG_DWORD, "UINumber", UINumber }, { REG_MULTI_SZ, "UpperFilters", UpperFilters }, { REG_MULTI_SZ, "LowerFilters", LowerFilters }, + [SPDRP_BASE_CONTAINERID] = { REG_SZ, "ContainerId", ContainerId }, };
static BOOL SETUPDI_SetDeviceRegistryPropertyW(struct device *device, diff --git a/include/setupapi.h b/include/setupapi.h index 14e3774fc42..806df7baaf5 100644 --- a/include/setupapi.h +++ b/include/setupapi.h @@ -1315,7 +1315,8 @@ DECL_WINELIB_SETUPAPI_TYPE_AW(PSP_INF_SIGNER_INFO) #define SPDRP_REMOVAL_POLICY_HW_DEFAULT 0x00000020 #define SPDRP_REMOVAL_POLICY_OVERRIDE 0x00000021 #define SPDRP_INSTALL_STATE 0x00000022 -#define SPDRP_MAXIMUM_PROPERTY 0x00000023 +#define SPDRP_BASE_CONTAINERID 0x00000024 +#define SPDRP_MAXIMUM_PROPERTY 0x00000025
#define DPROMPT_SUCCESS 0 #define DPROMPT_CANCEL 1
From: Claire Girka claire@sitedethib.com
--- dlls/ntoskrnl.exe/tests/driver_pnp.c | 5 ++++- dlls/ntoskrnl.exe/tests/ntoskrnl.c | 4 ++++ 2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/dlls/ntoskrnl.exe/tests/driver_pnp.c b/dlls/ntoskrnl.exe/tests/driver_pnp.c index 9727292d65d..824c6554693 100644 --- a/dlls/ntoskrnl.exe/tests/driver_pnp.c +++ b/dlls/ntoskrnl.exe/tests/driver_pnp.c @@ -228,7 +228,10 @@ static NTSTATUS query_id(struct device *device, IRP *irp, BUS_QUERY_ID_TYPE type }
case BusQueryContainerID: - return STATUS_NOT_SUPPORTED; + if (!(id = ExAllocatePool(PagedPool, 39 * sizeof(WCHAR)))) + return STATUS_NO_MEMORY; + wcscpy(id, L"{12345678-1234-1234-1234-123456789123}"); + break;
default: ok(0, "Unexpected ID query type %#x.\n", type); diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c index 64debcbebbf..bd361ce2fa3 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -1646,6 +1646,10 @@ static void test_pnp_devices(void) ok(size == sizeof(expect_hardware_id), "got size %lu\n", size); ok(!memcmp(buffer, expect_hardware_id, size), "got hardware IDs %s\n", debugstr_an(buffer, size));
+ ret = SetupDiGetDeviceRegistryPropertyA(set, &device, SPDRP_BASE_CONTAINERID, + &type, (BYTE *)buffer, sizeof(buffer), &size); + todo_wine ok(ret, "got error %#lx\n", GetLastError()); + ret = SetupDiGetDeviceRegistryPropertyA(set, &device, SPDRP_COMPATIBLEIDS, &type, (BYTE *)buffer, sizeof(buffer), &size); ok(ret, "got error %#lx\n", GetLastError());
From: Claire Girka claire@sitedethib.com
Do not assume the underlying driver will return meaningful data, as it may not support BusQueryContainerID which will be queried in a next commit. --- dlls/hidclass.sys/pnp.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/dlls/hidclass.sys/pnp.c b/dlls/hidclass.sys/pnp.c index 873c55c4a67..91d80f74403 100644 --- a/dlls/hidclass.sys/pnp.c +++ b/dlls/hidclass.sys/pnp.c @@ -84,11 +84,15 @@ static NTSTATUS get_device_id(DEVICE_OBJECT *device, BUS_QUERY_ID_TYPE type, WCH irpsp->MinorFunction = IRP_MN_QUERY_ID; irpsp->Parameters.QueryId.IdType = type;
+ irp->IoStatus.Status = STATUS_NOT_SUPPORTED; if (IoCallDriver(device, irp) == STATUS_PENDING) KeWaitForSingleObject(&event, Executive, KernelMode, FALSE, NULL);
- wcscpy(id, (WCHAR *)irp_status.Information); - ExFreePool((WCHAR *)irp_status.Information); + if (!irp_status.Status) { + wcscpy(id, (WCHAR *)irp_status.Information); + ExFreePool((WCHAR *)irp_status.Information); + } + return irp_status.Status; }
From: Claire Girka claire@sitedethib.com
--- dlls/hidclass.sys/hid.h | 1 + dlls/hidclass.sys/pnp.c | 3 +++ 2 files changed, 4 insertions(+)
diff --git a/dlls/hidclass.sys/hid.h b/dlls/hidclass.sys/hid.h index e20f12809eb..b8cec55fb7c 100644 --- a/dlls/hidclass.sys/hid.h +++ b/dlls/hidclass.sys/hid.h @@ -81,6 +81,7 @@ typedef struct _BASE_DEVICE_EXTENSION * for convenience. */ WCHAR device_id[MAX_DEVICE_ID_LEN]; WCHAR instance_id[MAX_DEVICE_ID_LEN]; + WCHAR container_id[MAX_GUID_STRING_LEN]; const GUID *class_guid;
BOOL is_fdo; diff --git a/dlls/hidclass.sys/pnp.c b/dlls/hidclass.sys/pnp.c index 91d80f74403..a2315b686ca 100644 --- a/dlls/hidclass.sys/pnp.c +++ b/dlls/hidclass.sys/pnp.c @@ -172,6 +172,9 @@ static NTSTATUS WINAPI driver_add_device(DRIVER_OBJECT *driver, DEVICE_OBJECT *b swprintf(ext->device_id, ARRAY_SIZE(ext->device_id), L"HID\%s", wcsrchr(device_id, '\') + 1); wcscpy(ext->instance_id, instance_id);
+ if (get_device_id(bus_pdo, BusQueryContainerID, ext->container_id)) + ext->container_id[0] = 0; + is_xinput_class = !wcsncmp(device_id, L"WINEXINPUT\", 7) && wcsstr(device_id, L"&XI_") != NULL; if (is_xinput_class) ext->class_guid = &GUID_DEVINTERFACE_WINEXINPUT; else ext->class_guid = &GUID_DEVINTERFACE_HID;
From: Claire Girka claire@sitedethib.com
--- dlls/hidclass.sys/pnp.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/dlls/hidclass.sys/pnp.c b/dlls/hidclass.sys/pnp.c index a2315b686ca..271c777129d 100644 --- a/dlls/hidclass.sys/pnp.c +++ b/dlls/hidclass.sys/pnp.c @@ -231,6 +231,7 @@ static void create_child(minidriver *minidriver, DEVICE_OBJECT *fdo) KeInitializeSpinLock( &pdo_ext->u.pdo.queues_lock ); wcscpy(pdo_ext->device_id, fdo_ext->device_id); wcscpy(pdo_ext->instance_id, fdo_ext->instance_id); + wcscpy(pdo_ext->container_id, fdo_ext->container_id); pdo_ext->class_guid = fdo_ext->class_guid;
pdo_ext->u.pdo.information.VendorID = attr.VendorID;
From: Claire Girka claire@sitedethib.com
--- dlls/hidclass.sys/pnp.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/dlls/hidclass.sys/pnp.c b/dlls/hidclass.sys/pnp.c index 271c777129d..023a4d9fa44 100644 --- a/dlls/hidclass.sys/pnp.c +++ b/dlls/hidclass.sys/pnp.c @@ -419,8 +419,16 @@ static NTSTATUS pdo_pnp(DEVICE_OBJECT *device, IRP *irp) irp->IoStatus.Information = (ULONG_PTR)id; status = STATUS_SUCCESS; break; - case BusQueryContainerID: + if (ext->container_id[0]) { + lstrcpyW(id, ext->container_id); + irp->IoStatus.Information = (ULONG_PTR)id; + status = STATUS_SUCCESS; + } else { + ExFreePool(id); + } + break; + case BusQueryDeviceSerialNumber: FIXME("unimplemented id type %#x\n", irpsp->Parameters.QueryId.IdType); ExFreePool(id);
From: Claire Girka claire@sitedethib.com
--- dlls/ntoskrnl.exe/pnp.c | 8 ++++++++ dlls/ntoskrnl.exe/tests/ntoskrnl.c | 6 +++++- 2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/dlls/ntoskrnl.exe/pnp.c b/dlls/ntoskrnl.exe/pnp.c index 71c03586897..e5c9b441b87 100644 --- a/dlls/ntoskrnl.exe/pnp.c +++ b/dlls/ntoskrnl.exe/pnp.c @@ -318,6 +318,7 @@ static void enumerate_new_device( DEVICE_OBJECT *device, HDEVINFO set ) BOOL need_driver = TRUE; NTSTATUS status; HKEY key; + WCHAR *id;
if (get_device_instance_id( device, device_instance_id )) return; @@ -353,6 +354,13 @@ static void enumerate_new_device( DEVICE_OBJECT *device, HDEVINFO set ) return; }
+ if (!get_device_id(device, BusQueryContainerID, &id) && id) + { + SetupDiSetDeviceRegistryPropertyW( set, &sp_device, SPDRP_BASE_CONTAINERID, (BYTE *)id, + (lstrlenW( id ) + 1) * sizeof(WCHAR) ); + ExFreePool( id ); + } + start_device( device, set, &sp_device ); }
diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c index bd361ce2fa3..28c65cccf2b 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -1460,6 +1460,7 @@ static void test_pnp_devices(void) { static const char expect_hardware_id[] = "winetest_hardware\0winetest_hardware_1\0"; static const char expect_compat_id[] = "winetest_compat\0winetest_compat_1\0"; + static const char expect_container_id[] = "{12345678-1234-1234-1234-123456789123}";
char buffer[200]; SP_DEVICE_INTERFACE_DETAIL_DATA_A *iface_detail = (void *)buffer; @@ -1648,7 +1649,10 @@ static void test_pnp_devices(void)
ret = SetupDiGetDeviceRegistryPropertyA(set, &device, SPDRP_BASE_CONTAINERID, &type, (BYTE *)buffer, sizeof(buffer), &size); - todo_wine ok(ret, "got error %#lx\n", GetLastError()); + ok(ret, "got error %#lx\n", GetLastError()); + ok(type == REG_SZ, "got type %lu\n", type); + ok(size == sizeof(expect_container_id), "got size %lu\n", size); + ok(!memcmp(buffer, expect_container_id, size), "got container ID %s\n", debugstr_a(buffer));
ret = SetupDiGetDeviceRegistryPropertyA(set, &device, SPDRP_COMPATIBLEIDS, &type, (BYTE *)buffer, sizeof(buffer), &size);
On Wed Jul 13 06:29:40 2022 +0000, Claire wrote:
I investigated, and my understanding is that `install_device_driver` in `dlls/ntoskrnl.exe/pnp.c` is only called the first time the HID device is registered. This means that changing `install_device_driver` alone is not sufficient to update the `ContainerID` if it changes, nor to register a `ContainerID` at all if the device was ever used before the change is implemented. I am not sure where the best place to update the `ContainerID` should be.
I decided to move it to `enumerate_new_device`, so that it is done even if the driver does not need to be installed, and avoid doing it once per interface.
On Wed Jul 13 17:06:25 2022 +0000, **** wrote:
Zebediah Figura replied on the mailing list:
On 7/13/22 01:29, Claire (@ClearlyClaire) wrote: > On Wed Jul 13 03:00:55 2022 +0000, **** wrote: >> Zebediah Figura replied on the mailing list: >> \`\`\` >> On 7/12/22 16:12, Claire (@ClearlyClaire) wrote: >>> On Tue Jul 12 18:48:28 2022 +0000, **** wrote: >>>> Zebediah Figura replied on the mailing list: >>>> \`\`\` >>>> On 7/12/22 13:01, Claire Girka wrote: >>>>> From: Claire Girka <claire@sitedethib.com> >>>>> >>>>> --- >>>>> dlls/ntoskrnl.exe/pnp.c | 8 ++++++++ >>>>> 1 file changed, 8 insertions(+) >>>>> >>>>> diff --git a/dlls/ntoskrnl.exe/pnp.c b/dlls/ntoskrnl.exe/pnp.c >>>>> index e12e46f94f1..77d879a07a9 100644 >>>>> --- a/dlls/ntoskrnl.exe/pnp.c >>>>> +++ b/dlls/ntoskrnl.exe/pnp.c >>>>> @@ -880,6 +880,7 @@ NTSTATUS WINAPI >>>> IoRegisterDeviceInterface(DEVICE_OBJECT *device, const GUID *cla >>>>> struct wine_rb_entry *entry; >>>>> DWORD required; >>>>> HDEVINFO set; >>>>> + WCHAR *id; >>>>> >>>>> TRACE("device %p, class_guid %s, refstr %s, symbolic_link %p.\n", >>>>> device, debugstr_guid(class_guid), >> debugstr_us(refstr), symbolic_link); >>>>> @@ -913,6 +914,13 @@ NTSTATUS WINAPI >>>> IoRegisterDeviceInterface(DEVICE_OBJECT *device, const GUID *cla >>>>> return STATUS_UNSUCCESSFUL; >>>>> } >>>>> >>>>> + if (!get_device_id(device, BusQueryContainerID, &id) && id) >>>>> + { >>>>> + SetupDiSetDeviceRegistryPropertyW( set, &sp_device, >>>> SPDRP_BASE_CONTAINERID, (BYTE *)id, >>>>> + (lstrlenW( id ) + 1) * sizeof(WCHAR) ); >>>>> + ExFreePool( id ); >>>>> + } >>>>> + >>>>> data->DevicePath[1] = '?'; >>>>> TRACE("Returning path %s.\n", debugstr_w(data->DevicePath)); >>>>> RtlCreateUnicodeString( &device_path, data->DevicePath); >>>> Why do we need this? >>>> _______________________________________________ >>>> wine-gitlab mailing list -- wine-gitlab@winehq.org >>>> To unsubscribe send an email to wine-gitlab-leave@winehq.org >>>> \`\`\` >>> I would need to investigate more, but the property isn't properly >> exposed without that change. >>> >> Please do; I don't think it should be necessary, and we'd rather fix it >> the right way upstream if not. >> _______________________________________________ >> wine-gitlab mailing list -- wine-gitlab@winehq.org >> To unsubscribe send an email to wine-gitlab-leave@winehq.org >> \`\`\` > I investigated, and my understanding is that `install_device_driver` in `dlls/ntoskrnl.exe/pnp.c` is only called the first time the HID device is registered. This means that changing `install_device_driver` alone is not sufficient to update the `ContainerID` if it changes, nor to register a `ContainerID` at all if the device was ever used before the change is implemented. I am not sure where the best place to update the `ContainerID` should be. > Why is it possible for the container ID to change? _______________________________________________ wine-gitlab mailing list -- wine-gitlab@winehq.org To unsubscribe send an email to wine-gitlab-leave@winehq.org
That is a good question. Maybe it shouldn't. But one reason I can think of is that we may change how we assign ContainerIDs (for now we don't, I currently have patches that work reasonably well, but if we change approaches, we would change how we assign them).
Another reason is that the container device (the USB hub itself) does not expose a serial ID, but its HID child device seems to. Therefore, Wine will reuse the same devnode across sessions and across ports for the HID device, while the other child devices may not be matched the same way (other than querying the sibling HID device, which seems unreasonable to do in general).
(For reference, Windows does not seem to see or use that serial number in any way, and creates a different device node depending on the USB port the gamepad is plugged into.)
On 7/13/22 12:47, Claire (@ClearlyClaire) wrote:
On Wed Jul 13 17:06:25 2022 +0000, **** wrote:
Zebediah Figura replied on the mailing list:
On 7/13/22 01:29, Claire (@ClearlyClaire) wrote: > On Wed Jul 13 03:00:55 2022 +0000, **** wrote: >> Zebediah Figura replied on the mailing list: >> \`\`\` >> On 7/12/22 16:12, Claire (@ClearlyClaire) wrote: >>> On Tue Jul 12 18:48:28 2022 +0000, **** wrote: >>>> Zebediah Figura replied on the mailing list: >>>> \`\`\` >>>> On 7/12/22 13:01, Claire Girka wrote: >>>>> From: Claire Girka <claire@sitedethib.com> >>>>> >>>>> --- >>>>> dlls/ntoskrnl.exe/pnp.c | 8 ++++++++ >>>>> 1 file changed, 8 insertions(+) >>>>> >>>>> diff --git a/dlls/ntoskrnl.exe/pnp.c b/dlls/ntoskrnl.exe/pnp.c >>>>> index e12e46f94f1..77d879a07a9 100644 >>>>> --- a/dlls/ntoskrnl.exe/pnp.c >>>>> +++ b/dlls/ntoskrnl.exe/pnp.c >>>>> @@ -880,6 +880,7 @@ NTSTATUS WINAPI >>>> IoRegisterDeviceInterface(DEVICE_OBJECT *device, const GUID *cla >>>>> struct wine_rb_entry *entry; >>>>> DWORD required; >>>>> HDEVINFO set; >>>>> + WCHAR *id; >>>>> >>>>> TRACE("device %p, class_guid %s, refstr %s, symbolic_link %p.\n", >>>>> device, debugstr_guid(class_guid), >> debugstr_us(refstr), symbolic_link); >>>>> @@ -913,6 +914,13 @@ NTSTATUS WINAPI >>>> IoRegisterDeviceInterface(DEVICE_OBJECT *device, const GUID *cla >>>>> return STATUS_UNSUCCESSFUL; >>>>> } >>>>> >>>>> + if (!get_device_id(device, BusQueryContainerID, &id) && id) >>>>> + { >>>>> + SetupDiSetDeviceRegistryPropertyW( set, &sp_device, >>>> SPDRP_BASE_CONTAINERID, (BYTE *)id, >>>>> + (lstrlenW( id ) + 1) * sizeof(WCHAR) ); >>>>> + ExFreePool( id ); >>>>> + } >>>>> + >>>>> data->DevicePath[1] = '?'; >>>>> TRACE("Returning path %s.\n", debugstr_w(data->DevicePath)); >>>>> RtlCreateUnicodeString( &device_path, data->DevicePath); >>>> Why do we need this? >>>> _______________________________________________ >>>> wine-gitlab mailing list -- wine-gitlab@winehq.org >>>> To unsubscribe send an email to wine-gitlab-leave@winehq.org >>>> \`\`\` >>> I would need to investigate more, but the property isn't properly >> exposed without that change. >>> >> Please do; I don't think it should be necessary, and we'd rather fix it >> the right way upstream if not. >> _______________________________________________ >> wine-gitlab mailing list -- wine-gitlab@winehq.org >> To unsubscribe send an email to wine-gitlab-leave@winehq.org >> \`\`\` > I investigated, and my understanding is that `install_device_driver` in `dlls/ntoskrnl.exe/pnp.c` is only called the first time the HID device is registered. This means that changing `install_device_driver` alone is not sufficient to update the `ContainerID` if it changes, nor to register a `ContainerID` at all if the device was ever used before the change is implemented. I am not sure where the best place to update the `ContainerID` should be. > Why is it possible for the container ID to change? _______________________________________________ wine-gitlab mailing list -- wine-gitlab@winehq.org To unsubscribe send an email to wine-gitlab-leave@winehq.orgThat is a good question. Maybe it shouldn't. But one reason I can think of is that we may change how we assign ContainerIDs (for now we don't, I currently have patches that work reasonably well, but if we change approaches, we would change how we assign them).
Another reason is that the container device (the USB hub itself) does not expose a serial ID, but its HID child device seems to. Therefore, Wine will reuse the same devnode across sessions and across ports for the HID device, while the other child devices may not be matched the same way (other than querying the sibling HID device, which seems unreasonable to do in general).
(For reference, Windows does not seem to see or use that serial number in any way, and creates a different device node depending on the USB port the gamepad is plugged into.)
Well, sure, the container ID for the "same" device might change across boots, for one reason or another. But once the device is plugged in and enumerated I wouldn't expect the container ID to change, should it? In which case we should only need to query it once, and I wouldn't expect it to matter when.
On Wed Jul 13 18:15:56 2022 +0000, **** wrote:
Zebediah Figura replied on the mailing list:
On 7/13/22 12:47, Claire (@ClearlyClaire) wrote: > On Wed Jul 13 17:06:25 2022 +0000, **** wrote: >> Zebediah Figura replied on the mailing list: >> \`\`\` >> On 7/13/22 01:29, Claire (@ClearlyClaire) wrote: >>> On Wed Jul 13 03:00:55 2022 +0000, **** wrote: >>>> Zebediah Figura replied on the mailing list: >>>> \`\`\` >>>> On 7/12/22 16:12, Claire (@ClearlyClaire) wrote: >>>>> On Tue Jul 12 18:48:28 2022 +0000, **** wrote: >>>>>> Zebediah Figura replied on the mailing list: >>>>>> \`\`\` >>>>>> On 7/12/22 13:01, Claire Girka wrote: >>>>>>> From: Claire Girka <claire@sitedethib.com> >>>>>>> >>>>>>> --- >>>>>>> dlls/ntoskrnl.exe/pnp.c | 8 ++++++++ >>>>>>> 1 file changed, 8 insertions(+) >>>>>>> >>>>>>> diff --git a/dlls/ntoskrnl.exe/pnp.c b/dlls/ntoskrnl.exe/pnp.c >>>>>>> index e12e46f94f1..77d879a07a9 100644 >>>>>>> --- a/dlls/ntoskrnl.exe/pnp.c >>>>>>> +++ b/dlls/ntoskrnl.exe/pnp.c >>>>>>> @@ -880,6 +880,7 @@ NTSTATUS WINAPI >>>>>> IoRegisterDeviceInterface(DEVICE_OBJECT *device, const GUID *cla >>>>>>> struct wine_rb_entry *entry; >>>>>>> DWORD required; >>>>>>> HDEVINFO set; >>>>>>> + WCHAR *id; >>>>>>> >>>>>>> TRACE("device %p, class_guid %s, refstr %s, symbolic_link %p.\n", >>>>>>> device, debugstr_guid(class_guid), >>>> debugstr_us(refstr), symbolic_link); >>>>>>> @@ -913,6 +914,13 @@ NTSTATUS WINAPI >>>>>> IoRegisterDeviceInterface(DEVICE_OBJECT *device, const GUID *cla >>>>>>> return STATUS_UNSUCCESSFUL; >>>>>>> } >>>>>>> >>>>>>> + if (!get_device_id(device, BusQueryContainerID, &id) && id) >>>>>>> + { >>>>>>> + SetupDiSetDeviceRegistryPropertyW( set, &sp_device, >>>>>> SPDRP_BASE_CONTAINERID, (BYTE *)id, >>>>>>> + (lstrlenW( id ) + 1) * sizeof(WCHAR) ); >>>>>>> + ExFreePool( id ); >>>>>>> + } >>>>>>> + >>>>>>> data->DevicePath[1] = '?'; >>>>>>> TRACE("Returning path %s.\n", debugstr_w(data->DevicePath)); >>>>>>> RtlCreateUnicodeString( &device_path, data->DevicePath); >>>>>> Why do we need this? >>>>>> _______________________________________________ >>>>>> wine-gitlab mailing list -- wine-gitlab@winehq.org >>>>>> To unsubscribe send an email to wine-gitlab-leave@winehq.org >>>>>> \`\`\` >>>>> I would need to investigate more, but the property isn't properly >>>> exposed without that change. >>>>> >>>> Please do; I don't think it should be necessary, and we'd rather fix it >>>> the right way upstream if not. >>>> _______________________________________________ >>>> wine-gitlab mailing list -- wine-gitlab@winehq.org >>>> To unsubscribe send an email to wine-gitlab-leave@winehq.org >>>> \`\`\` >>> I investigated, and my understanding is that `install_device_driver` >> in `dlls/ntoskrnl.exe/pnp.c` is only called the first time the HID >> device is registered. This means that changing `install_device_driver` >> alone is not sufficient to update the `ContainerID` if it changes, nor >> to register a `ContainerID` at all if the device was ever used before >> the change is implemented. I am not sure where the best place to update >> the `ContainerID` should be. >>> >> Why is it possible for the container ID to change? >> _______________________________________________ >> wine-gitlab mailing list -- wine-gitlab@winehq.org >> To unsubscribe send an email to wine-gitlab-leave@winehq.org >> \`\`\` > That is a good question. Maybe it shouldn't. But one reason I can think of is that we may change how we assign ContainerIDs (for now we don't, I currently have patches that work reasonably well, but if we change approaches, we would change how we assign them). > > Another reason is that the container device (the USB hub itself) does not expose a serial ID, but its HID child device seems to. Therefore, Wine will reuse the same devnode across sessions and across ports for the HID device, while the other child devices may not be matched the same way (other than querying the sibling HID device, which seems unreasonable to do in general). > > (For reference, Windows does not seem to see or use that serial number in any way, and creates a different device node depending on the USB port the gamepad is plugged into.) > Well, sure, the container ID for the "same" device might change across boots, for one reason or another. But once the device is plugged in and enumerated I wouldn't expect the container ID to change, should it? In which case we should only need to query it once, and I wouldn't expect it to matter when. _______________________________________________ wine-gitlab mailing list -- wine-gitlab@winehq.org To unsubscribe send an email to wine-gitlab-leave@winehq.org
Yes, once plugged in, the Container ID does not change. In earlier versions of the patchset, the Container ID was queried at driver installation (a step that is not re-executed on reboots and re-plugs if the same identifiers get reused by Wine) and, because that wasn't enough, at each interface registration (which was needlessly often). I believe the current version of the patch, which does it when enumerating a new device, fits your expectations.
-
Claire (＠ClearlyClaire) -
Claire Girka
-
Zebediah Figura