[PATCH v7 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.
-- v7: 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..f5e3695ac8e 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}\0"; + static const WCHAR expect_container_id_w[] = L"{12345678-1234-1234-1234-123456789123}\0";
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 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()`.
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.orgI 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.
-
Claire (＠ClearlyClaire) -
Claire Girka
-
Zebediah Figura