[PATCH v8 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.
-- v8: 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 | 9 ++++++++- 2 files changed, 16 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..d2a96d17264 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -1460,6 +1460,8 @@ 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}"; + static const WCHAR expect_container_id_w[] = L"{12345678-1234-1234-1234-123456789123}";
char buffer[200]; SP_DEVICE_INTERFACE_DETAIL_DATA_A *iface_detail = (void *)buffer; @@ -1648,7 +1650,12 @@ 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(broken(size == sizeof(expect_container_id_w)) /* Some Windows environment return an UTF-16 string for some reason */ || + size == sizeof(expect_container_id), "got size %lu\n", size); + ok(broken(!memcmp(buffer, expect_container_id_w, size)) /* Some Windows environment return an UTF-16 string for some reason */ || + !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 18:53:36 2022 +0000, **** wrote:
Zebediah Figura replied on the mailing list:
On 7/13/22 13:29, Claire (@ClearlyClaire) wrote: > On Wed Jul 13 17:25:36 2022 +0000, **** wrote: >> Zebediah Figura replied on the mailing list: >> \`\`\` >> On 7/13/22 04:28, Claire (@ClearlyClaire) wrote: >>> On Tue Jul 12 23:34:52 2022 +0000, **** wrote: >>>> Marvin replied on the mailing list: >>>> \`\`\` >>>> Hi, >>>> It looks like your patch introduced the new failures shown below. >>>> Please investigate and fix them before resubmitting your patch. >>>> If they are not new, fixing them anyway would help a lot. Otherwise >>>> please ask for the known failures list to be updated. >>>> The tests also ran into some preexisting test failures. If you know how >>>> to fix them that would be helpful. See the TestBot job for the details: >>>> The full results can be found at: >>>> https://testbot.winehq.org/JobDetails.pl?Key=118863 >>>> Your paranoid android. >>>> === w7u_2qxl (32 bit report) === >>>> ntoskrnl.exe: >>>> ntoskrnl.c:1654: Test failed: got size 78 >>>> ntoskrnl.c:1655: Test failed: got container ID "{" >>>> === w7u_el (32 bit report) === >>>> ntoskrnl.exe: >>>> ntoskrnl.c:1654: Test failed: got size 78 >>>> ntoskrnl.c:1655: Test failed: got container ID "{" >>>> === w8 (32 bit report) === >>>> ntoskrnl.exe: >>>> ntoskrnl.c:1654: Test failed: got size 78 >>>> ntoskrnl.c:1655: Test failed: got container ID "{" >>>> === w1064_tsign (64 bit report) === >>>> ntoskrnl.exe: >>>> ntoskrnl.c:1654: Test failed: got size 78 >>>> ntoskrnl.c:1655: Test failed: got container ID "{" >>>> === debian11 (32 bit Hebrew:Israel report) === >>>> ntoskrnl.exe: >>>> driver_pnp.c:706: Test failed: Got 1 remove events. >>>> === debian11 (32 bit Hindi:India report) === >>>> ntoskrnl.exe: >>>> driver_pnp.c:706: Test failed: Got 1 remove events. >>>> \`\`\` >>> I do not understand those test failures. From what I understand, they >> are from running the test on certain Windows environments, but even >> then, I do not understand why `SetupDiGetDeviceInstanceIdA` would return >> a UTF-16 string as opposed to the ANSI string it is supposed to return. >> The >> [documentation](https://docs.microsoft.com/en-us/windows-hardware/drivers/kernel/irp-mn-query-id) >> says the response to `BusQueryContainerID` is a `REG_SZ` value to be >> stored to a `WCHAR` pointer, so I'm not sure what the issue would be. >>> >> Probably Windows is just broken in this respect. I'd either adjust the >> test (and implementation) to match, or use broken(). >> _______________________________________________ >> wine-gitlab mailing list -- wine-gitlab@winehq.org >> To unsubscribe send an email to wine-gitlab-leave@winehq.org >> \`\`\` > I wouldn't know how to match this behavior as it seems fairly inconsistent (it seems like 4 out of 23 windows test envs returned a UTF-16 string while all the others returned the expected ANSI string). Using `broken()`. > I think that's just because those machines are the only ones that are capable of running the ntoskrnl tests in the first place; the tests are skipped everywhere else. _______________________________________________ wine-gitlab mailing list -- wine-gitlab@winehq.org To unsubscribe send an email to wine-gitlab-leave@winehq.org
Oh, makes sense, I hadn't noticed the other envs skipped it. I added `broken` calls, but I can probably simplify the tests by just calling the widechar variant of the function if you prefer. I'm not sure how much sense replicating Windows' broken behavior would make, and the games I've seen actually use it use the widechar variant.
-
Claire (＠ClearlyClaire) -
Claire Girka