[PATCH 1/5] ntoskrnl: Implement IoSetDevicePropertyData().
From: Arkadiusz Hiler <ahiler(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- In PATCH 1, the return GetLastError() look wrong to me but the same is done in IoGetDeviceProperty so I kept them. I guess we would need to convert the setupapi error back to NTSTATUS, or implement the higher level setupapi on top of these lower level primitives intead. In PATCH 2, I moved the error case before IoInvalidateDeviceRelations, because I don't know what the call is actually expecting. Maybe it was unnecessary. dlls/ntoskrnl.exe/ntoskrnl.exe.spec | 1 + dlls/ntoskrnl.exe/pnp.c | 51 +++++++++++++++++++++++++++++ include/ddk/wdm.h | 4 +++ 3 files changed, 56 insertions(+) diff --git a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec index 2e5e2e6e11b..4eb08faec2e 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec +++ b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec @@ -465,6 +465,7 @@ @ stdcall IoReuseIrp(ptr long) @ stub IoSetCompletionRoutineEx @ stdcall IoSetDeviceInterfaceState(ptr long) +@ stdcall IoSetDevicePropertyData(ptr ptr long long long long ptr) @ stub IoSetDeviceToVerify @ stub IoSetFileOrigin @ stub IoSetHardErrorOrVerifyDevice diff --git a/dlls/ntoskrnl.exe/pnp.c b/dlls/ntoskrnl.exe/pnp.c index 095344b1073..e425712d738 100644 --- a/dlls/ntoskrnl.exe/pnp.c +++ b/dlls/ntoskrnl.exe/pnp.c @@ -38,6 +38,12 @@ DEFINE_GUID(GUID_NULL,0,0,0,0,0,0,0,0,0,0,0); WINE_DEFAULT_DEBUG_CHANNEL(plugplay); +static inline const char *debugstr_propkey( const DEVPROPKEY *id ) +{ + if (!id) return "(null)"; + return wine_dbg_sprintf( "{%s,%04x}", wine_dbgstr_guid( &id->fmtid ), id->pid ); +} + #define MAX_SERVICE_NAME 260 struct device_interface @@ -783,6 +789,51 @@ NTSTATUS WINAPI IoSetDeviceInterfaceState( UNICODE_STRING *name, BOOLEAN enable return ret; } +/*********************************************************************** + * IoSetDevicePropertyData (NTOSKRNL.EXE.@) + */ +NTSTATUS WINAPI IoSetDevicePropertyData( DEVICE_OBJECT *device, const DEVPROPKEY *property_key, LCID lcid, + ULONG flags, DEVPROPTYPE type, ULONG size, void *data ) +{ + SP_DEVINFO_DATA sp_device = {sizeof(sp_device)}; + WCHAR device_instance_id[MAX_DEVICE_ID_LEN]; + NTSTATUS status; + HDEVINFO set; + + TRACE( "device %p, property_key %s, lcid %#x, flags %#x, type %#x, size %u, data %p.\n", + device, debugstr_propkey(property_key), lcid, flags, type, size, data ); + + /* flags is always treated as PLUGPLAY_PROPERTY_PERSISTENT starting with Win 8 / 2012 */ + + if (lcid != LOCALE_NEUTRAL) FIXME( "only LOCALE_NEUTRAL is supported\n" ); + + if ((status = get_device_instance_id( device, device_instance_id ))) return status; + + if ((set = SetupDiCreateDeviceInfoList( &GUID_NULL, NULL )) == INVALID_HANDLE_VALUE) + { + ERR( "Failed to create device list, error %#x.\n", GetLastError() ); + return GetLastError(); + } + + if (!SetupDiOpenDeviceInfoW( set, device_instance_id, NULL, 0, &sp_device )) + { + ERR( "Failed to open device, error %#x.\n", GetLastError() ); + SetupDiDestroyDeviceInfoList( set ); + return GetLastError(); + } + + if (!SetupDiSetDevicePropertyW( set, &sp_device, property_key, type, data, size, 0 )) + { + ERR( "Failed to set property, error %#x.\n", GetLastError() ); + SetupDiDestroyDeviceInfoList( set ); + return GetLastError(); + } + + SetupDiDestroyDeviceInfoList( set ); + + return STATUS_SUCCESS; +} + /*********************************************************************** * IoRegisterDeviceInterface (NTOSKRNL.EXE.@) */ diff --git a/include/ddk/wdm.h b/include/ddk/wdm.h index 51097bfe3ab..447833c4bc9 100644 --- a/include/ddk/wdm.h +++ b/include/ddk/wdm.h @@ -21,6 +21,7 @@ #define _NTDDK_ #include <ntstatus.h> +#include <devpropdef.h> #ifdef _WIN64 #define POINTER_ALIGNMENT DECLSPEC_ALIGN(8) @@ -1681,6 +1682,8 @@ void WINAPI ExReleaseResourceForThreadLite(ERESOURCE*,ERESOURCE_THREAD); ULONG WINAPI ExSetTimerResolution(ULONG,BOOLEAN); void WINAPI ExUnregisterCallback(void*); +#define PLUGPLAY_PROPERTY_PERSISTENT 0x0001 + void WINAPI IoAcquireCancelSpinLock(KIRQL*); NTSTATUS WINAPI IoAcquireRemoveLockEx(IO_REMOVE_LOCK*,void*,const char*,ULONG, ULONG); NTSTATUS WINAPI IoAllocateDriverObjectExtension(PDRIVER_OBJECT,PVOID,ULONG,PVOID*); @@ -1728,6 +1731,7 @@ void WINAPI IoReleaseRemoveLockAndWaitEx(IO_REMOVE_LOCK*,void*,ULONG); void WINAPI IoReleaseRemoveLockEx(IO_REMOVE_LOCK*,void*,ULONG); void WINAPI IoReuseIrp(IRP*,NTSTATUS); NTSTATUS WINAPI IoSetDeviceInterfaceState(UNICODE_STRING*,BOOLEAN); +NTSTATUS WINAPI IoSetDevicePropertyData(DEVICE_OBJECT*,const DEVPROPKEY*,LCID,ULONG,DEVPROPTYPE,ULONG,void*); NTSTATUS WINAPI IoWMIRegistrationControl(PDEVICE_OBJECT,ULONG); void FASTCALL KeAcquireInStackQueuedSpinLockAtDpcLevel(KSPIN_LOCK*,KLOCK_QUEUE_HANDLE*); -- 2.31.0
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/hidclass.sys/device.c | 4 ---- dlls/hidclass.sys/pnp.c | 18 ++++++++++++++++-- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/dlls/hidclass.sys/device.c b/dlls/hidclass.sys/device.c index a7825080deb..7dc97781b6b 100644 --- a/dlls/hidclass.sys/device.c +++ b/dlls/hidclass.sys/device.c @@ -21,7 +21,6 @@ #include <stdarg.h> #define NONAMELESSUNION #define NONAMELESSSTRUCT -#include "initguid.h" #include "hid.h" #include "winreg.h" #include "winuser.h" @@ -30,9 +29,6 @@ #include "ddk/hidsdi.h" #include "ddk/hidtypes.h" #include "ddk/wdm.h" -#include "devguid.h" -#include "ntddmou.h" -#include "ntddkbd.h" WINE_DEFAULT_DEBUG_CHANNEL(hid); WINE_DECLARE_DEBUG_CHANNEL(hid_report); diff --git a/dlls/hidclass.sys/pnp.c b/dlls/hidclass.sys/pnp.c index 3d81c16356e..2769b78c13b 100644 --- a/dlls/hidclass.sys/pnp.c +++ b/dlls/hidclass.sys/pnp.c @@ -21,7 +21,10 @@ #define NONAMELESSUNION #include <unistd.h> #include <stdarg.h> +#include "initguid.h" #include "hid.h" +#include "devguid.h" +#include "devpkey.h" #include "ntddmou.h" #include "ntddkbd.h" #include "ddk/hidtypes.h" @@ -33,6 +36,8 @@ WINE_DEFAULT_DEBUG_CHANNEL(hid); +DEFINE_DEVPROPKEY(DEVPROPKEY_HID_HANDLE, 0xbc62e415, 0xf4fe, 0x405c, 0x8e, 0xda, 0x63, 0x6f, 0xb5, 0x9f, 0x08, 0x98, 2); + #if defined(__i386__) && !defined(_WIN32) extern void * WINAPI wrap_fastcall_func1( void *func, const void *a ); @@ -231,8 +236,6 @@ static void create_child(minidriver *minidriver, DEVICE_OBJECT *fdo) pdo_ext->u.pdo.information.DescriptorSize = pdo_ext->u.pdo.preparsed_data->dwSize; - IoInvalidateDeviceRelations(fdo_ext->u.fdo.hid_ext.PhysicalDeviceObject, BusRelations); - page = pdo_ext->u.pdo.preparsed_data->caps.UsagePage; usage = pdo_ext->u.pdo.preparsed_data->caps.Usage; if (page == HID_USAGE_PAGE_GENERIC && usage == HID_USAGE_GENERIC_MOUSE) @@ -242,6 +245,17 @@ static void create_child(minidriver *minidriver, DEVICE_OBJECT *fdo) else pdo_ext->u.pdo.rawinput_handle = alloc_rawinput_handle(); + if ((status = IoSetDevicePropertyData(child_pdo, &DEVPROPKEY_HID_HANDLE, LOCALE_NEUTRAL, + PLUGPLAY_PROPERTY_PERSISTENT, DEVPROP_TYPE_UINT32, + sizeof(pdo_ext->u.pdo.rawinput_handle), &pdo_ext->u.pdo.rawinput_handle))) + { + ERR("Failed to set device handle property, status %x\n", status); + IoDeleteDevice(child_pdo); + return; + } + + IoInvalidateDeviceRelations(fdo_ext->u.fdo.hid_ext.PhysicalDeviceObject, BusRelations); + pdo_ext->u.pdo.poll_interval = DEFAULT_POLL_INTERVAL; pdo_ext->u.pdo.ring_buffer = RingBuffer_Create( -- 2.31.0
On Mon, Apr 26, 2021 at 12:38:00PM +0200, Rémi Bernon wrote:
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
As I've said on the previous revision - this seems to be in line what Windows is doing. The rawinput handle device handles are non-pointer integer values that are constant between processes. I like how simple this got with the recent changes in the area :-) Signed-off-by: Arkadiusz Hiler <ahiler(a)codeweavers.com>
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/user32/rawinput.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/dlls/user32/rawinput.c b/dlls/user32/rawinput.c index 129c1933c36..ad93517ae8a 100644 --- a/dlls/user32/rawinput.c +++ b/dlls/user32/rawinput.c @@ -38,11 +38,14 @@ #include "user_private.h" #include "initguid.h" +#include "devpkey.h" #include "ntddmou.h" #include "ntddkbd.h" WINE_DEFAULT_DEBUG_CHANNEL(rawinput); +DEFINE_DEVPROPKEY(DEVPROPKEY_HID_HANDLE, 0xbc62e415, 0xf4fe, 0x405c, 0x8e, 0xda, 0x63, 0x6f, 0xb5, 0x9f, 0x08, 0x98, 2); + struct device { SP_DEVICE_INTERFACE_DETAIL_DATA_W *detail; @@ -92,17 +95,27 @@ static BOOL array_reserve(void **elements, unsigned int *capacity, unsigned int static struct device *add_device(HDEVINFO set, SP_DEVICE_INTERFACE_DATA *iface) { + SP_DEVINFO_DATA device_data = {sizeof(device_data)}; SP_DEVICE_INTERFACE_DETAIL_DATA_W *detail; struct device *device; + UINT32 handle; HANDLE file; - DWORD size; + DWORD size, type; - SetupDiGetDeviceInterfaceDetailW(set, iface, NULL, 0, &size, NULL); + SetupDiGetDeviceInterfaceDetailW(set, iface, NULL, 0, &size, &device_data); if (GetLastError() != ERROR_INSUFFICIENT_BUFFER) { ERR("Failed to get device path, error %#x.\n", GetLastError()); return FALSE; } + + if (!SetupDiGetDevicePropertyW(set, &device_data, &DEVPROPKEY_HID_HANDLE, &type, (BYTE *)&handle, sizeof(handle), NULL, 0) || + type != DEVPROP_TYPE_UINT32) + { + ERR("Failed to get device handle, error %#x.\n", GetLastError()); + return NULL; + } + if (!(detail = malloc(size))) { ERR("Failed to allocate memory.\n"); @@ -111,7 +124,7 @@ static struct device *add_device(HDEVINFO set, SP_DEVICE_INTERFACE_DATA *iface) detail->cbSize = sizeof(SP_DEVICE_INTERFACE_DETAIL_DATA_W); SetupDiGetDeviceInterfaceDetailW(set, iface, detail, size, NULL, NULL); - TRACE("Found HID device %s.\n", debugstr_w(detail->DevicePath)); + TRACE("Found device %x / %s.\n", handle, debugstr_w(detail->DevicePath)); file = CreateFileW(detail->DevicePath, GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, 0, 0); -- 2.31.0
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=89334 Your paranoid android. === debiant2 (32 bit Chinese:China report) === user32: win.c:10766: Test failed: 000D0110: expected prev 00430124, got 00000000 win.c:10780: Test failed: hwnd should NOT be topmost win.c:10782: Test failed: 000D0110: expected NOT topmost win.c:10726: Test failed: 1: hwnd 000D0110 is still topmost
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/user32/rawinput.c | 32 +++++++++++++++++++++++++++++--- dlls/user32/tests/input.c | 2 +- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/dlls/user32/rawinput.c b/dlls/user32/rawinput.c index ad93517ae8a..a2cf5769fcc 100644 --- a/dlls/user32/rawinput.c +++ b/dlls/user32/rawinput.c @@ -50,6 +50,7 @@ struct device { SP_DEVICE_INTERFACE_DETAIL_DATA_W *detail; HANDLE file; + HANDLE handle; RID_DEVICE_INFO info; PHIDP_PREPARSED_DATA data; }; @@ -100,7 +101,7 @@ static struct device *add_device(HDEVINFO set, SP_DEVICE_INTERFACE_DATA *iface) struct device *device; UINT32 handle; HANDLE file; - DWORD size, type; + DWORD i, size, type; SetupDiGetDeviceInterfaceDetailW(set, iface, NULL, 0, &size, &device_data); if (GetLastError() != ERROR_INSUFFICIENT_BUFFER) @@ -116,6 +117,15 @@ static struct device *add_device(HDEVINFO set, SP_DEVICE_INTERFACE_DATA *iface) return NULL; } + for (i = 0; i < rawinput_devices_count; ++i) + { + if (rawinput_devices[i].handle == UlongToHandle(handle)) + { + TRACE("Updating device %x / %s\n", handle, debugstr_w(rawinput_devices[i].detail->DevicePath)); + return rawinput_devices + i; + } + } + if (!(detail = malloc(size))) { ERR("Failed to allocate memory.\n"); @@ -147,6 +157,7 @@ static struct device *add_device(HDEVINFO set, SP_DEVICE_INTERFACE_DATA *iface) device = &rawinput_devices[rawinput_devices_count++]; device->detail = detail; device->file = file; + device->handle = ULongToHandle(handle); device->info.cbSize = sizeof(RID_DEVICE_INFO); device->data = NULL; @@ -244,6 +255,20 @@ static void find_devices(void) } +static struct device *find_device_from_handle(HANDLE handle) +{ + UINT i; + for (i = 0; i < rawinput_devices_count; ++i) + if (rawinput_devices[i].handle == handle) + return rawinput_devices + i; + find_devices(); + for (i = 0; i < rawinput_devices_count; ++i) + if (rawinput_devices[i].handle == handle) + return rawinput_devices + i; + return NULL; +} + + struct rawinput_thread_data *rawinput_thread_data(void) { struct user_thread_info *thread_info = get_user_thread_info(); @@ -401,7 +426,7 @@ UINT WINAPI GetRawInputDeviceList(RAWINPUTDEVICELIST *devices, UINT *device_coun for (i = 0; i < rawinput_devices_count; ++i) { - devices[2 + i].hDevice = &rawinput_devices[i]; + devices[2 + i].hDevice = rawinput_devices[i].handle; devices[2 + i].dwType = rawinput_devices[i].info.dwType; } @@ -671,7 +696,7 @@ UINT WINAPI GetRawInputDeviceInfoW(HANDLE handle, UINT command, void *data, UINT static const RID_DEVICE_INFO_MOUSE mouse_info = {1, 5, 0, FALSE}; RID_DEVICE_INFO info; - struct device *device = handle; + struct device *device; const void *to_copy; UINT to_copy_bytes, avail_bytes; @@ -679,6 +704,7 @@ UINT WINAPI GetRawInputDeviceInfoW(HANDLE handle, UINT command, void *data, UINT handle, command, data, data_size); if (!data_size) return ~0U; + if (!(device = find_device_from_handle(handle))) return ~0U; /* each case below must set: * *data_size: length (meaning defined by command) of data we want to copy diff --git a/dlls/user32/tests/input.c b/dlls/user32/tests/input.c index 2397b4104e4..b86194e6d0d 100644 --- a/dlls/user32/tests/input.c +++ b/dlls/user32/tests/input.c @@ -1885,7 +1885,7 @@ static void test_GetRawInputDeviceList(void) * understand that; so use the \\?\ prefix instead */ name[1] = '\\'; file = CreateFileW(name, 0, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, 0, NULL); - todo_wine_if(i == 0 || i == 1) + todo_wine_if(info.dwType == RIM_TYPEMOUSE || info.dwType == RIM_TYPEKEYBOARD) ok(file != INVALID_HANDLE_VALUE, "Failed to open %s, error %u\n", wine_dbgstr_w(name), GetLastError()); sz = 0; -- 2.31.0
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=89335 Your paranoid android. === w7u_2qxl (32 bit report) === user32: input.c:756: Test failed: 0 (a4/0): 00 from 00 -> 80 unexpected input.c:756: Test failed: 0 (a4/0): 41 from 01 -> 00 unexpected === w7u_el (32 bit report) === user32: input.c:4292: Test failed: SendInput triggered unexpected message 0xc042 === wvistau64 (64 bit report) === user32: input.c:756: Test failed: 0 (a4/0): 01 from 01 -> 00 unexpected input.c:756: Test failed: 0 (a4/0): 11 from 01 -> 00 unexpected input.c:756: Test failed: 0 (a4/0): a2 from 01 -> 00 unexpected === w1064 (64 bit report) === user32: input.c:3258: Test failed: expected WM_LBUTTONDOWN message input.c:3259: Test failed: expected WM_LBUTTONUP message input.c:3286: Test failed: expected WM_NCHITTEST message input.c:3287: Test failed: expected WM_RBUTTONDOWN message input.c:3288: Test failed: expected WM_RBUTTONUP message input.c:3317: Test failed: expected WM_LBUTTONDOWN message input.c:3318: Test failed: expected WM_LBUTTONUP message input.c:3371: Test failed: expected loop with WM_NCHITTEST messages input.c:3424: Test failed: expected WM_LBUTTONDOWN message input.c:3425: Test failed: expected WM_LBUTTONUP message input.c:1322: Test failed: GetCursorPos: (99,100)
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/user32/rawinput.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/dlls/user32/rawinput.c b/dlls/user32/rawinput.c index a2cf5769fcc..5c009624fe6 100644 --- a/dlls/user32/rawinput.c +++ b/dlls/user32/rawinput.c @@ -408,29 +408,24 @@ UINT WINAPI GetRawInputDeviceList(RAWINPUTDEVICELIST *devices, UINT *device_coun if (!devices) { - *device_count = 2 + rawinput_devices_count; + *device_count = rawinput_devices_count; return 0; } - if (*device_count < 2 + rawinput_devices_count) + if (*device_count < rawinput_devices_count) { SetLastError(ERROR_INSUFFICIENT_BUFFER); - *device_count = 2 + rawinput_devices_count; + *device_count = rawinput_devices_count; return ~0U; } - devices[0].hDevice = WINE_MOUSE_HANDLE; - devices[0].dwType = RIM_TYPEMOUSE; - devices[1].hDevice = WINE_KEYBOARD_HANDLE; - devices[1].dwType = RIM_TYPEKEYBOARD; - for (i = 0; i < rawinput_devices_count; ++i) { - devices[2 + i].hDevice = rawinput_devices[i].handle; - devices[2 + i].dwType = rawinput_devices[i].info.dwType; + devices[i].hDevice = rawinput_devices[i].handle; + devices[i].dwType = rawinput_devices[i].info.dwType; } - return 2 + rawinput_devices_count; + return rawinput_devices_count; } /*********************************************************************** -- 2.31.0
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=89336 Your paranoid android. === debiant2 (32 bit Chinese:China report) === user32: win.c:11487: Test failed: got normal pos (200,88)-(400,288) win.c:11490: Test failed: got window rect (200,88)-(400,288) win.c:11503: Test failed: got normal pos (200,88)-(400,288) === debiant2 (64 bit WoW report) === user32: win.c:10102: Test failed: Expected foreground window 00000000000E0120, got 0000000000EE0050
On Mon, Apr 26, 2021 at 12:38:03PM +0200, Rémi Bernon wrote:
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
I think it may be better to move this patch one up or squash it into the 4th one, because of the duplicated IDs. Anyhow, Signed-off-by: Arkadiusz Hiler <ahiler(a)codeweavers.com>
On 4/26/21 5:37 AM, Rémi Bernon wrote:
From: Arkadiusz Hiler <ahiler(a)codeweavers.com>
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> ---
In PATCH 1, the return GetLastError() look wrong to me but the same is done in IoGetDeviceProperty so I kept them. I guess we would need to convert the setupapi error back to NTSTATUS, or implement the higher level setupapi on top of these lower level primitives intead.
It's maybe not exactly ideal, but it's also not clear how to map setupapi errors to NTSTATUS, and moreover it probably doesn't matter anyway (we don't expect those functions to fail). I don't think we can implement setupapi functions on top of ntoskrnl ones; that crosses the kernel boundary (and there's no clear syscall interface for it). Probably on Windows the code is just duplicated. FWIW, we have a PnP test driver now, so it's actually possible to write tests for this function.
In PATCH 2, I moved the error case before IoInvalidateDeviceRelations, because I don't know what the call is actually expecting. Maybe it was unnecessary.
I think it's correct as-is to move it—as soon as we call IoInvalidateDeviceRelations() the device can be exposed. Actually, to be completely correct (i.e. if we were running this driver in Windows), I think we should treat it as possible to receive IRP_MN_QUERY_DEVICE_RELATIONS at any time. What we probably should be doing is to initialize everything necessary *before* the statement "fdo_ext->u.fdo.child_pdo = child_pdo", and then everything else at IRP_MN_START_DEVICE (on the child PDO).
dlls/ntoskrnl.exe/ntoskrnl.exe.spec | 1 + dlls/ntoskrnl.exe/pnp.c | 51 +++++++++++++++++++++++++++++ include/ddk/wdm.h | 4 +++ 3 files changed, 56 insertions(+)
diff --git a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec index 2e5e2e6e11b..4eb08faec2e 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec +++ b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec @@ -465,6 +465,7 @@ @ stdcall IoReuseIrp(ptr long) @ stub IoSetCompletionRoutineEx @ stdcall IoSetDeviceInterfaceState(ptr long) +@ stdcall IoSetDevicePropertyData(ptr ptr long long long long ptr) @ stub IoSetDeviceToVerify @ stub IoSetFileOrigin @ stub IoSetHardErrorOrVerifyDevice diff --git a/dlls/ntoskrnl.exe/pnp.c b/dlls/ntoskrnl.exe/pnp.c index 095344b1073..e425712d738 100644 --- a/dlls/ntoskrnl.exe/pnp.c +++ b/dlls/ntoskrnl.exe/pnp.c @@ -38,6 +38,12 @@ DEFINE_GUID(GUID_NULL,0,0,0,0,0,0,0,0,0,0,0);
WINE_DEFAULT_DEBUG_CHANNEL(plugplay);
+static inline const char *debugstr_propkey( const DEVPROPKEY *id ) +{ + if (!id) return "(null)"; + return wine_dbg_sprintf( "{%s,%04x}", wine_dbgstr_guid( &id->fmtid ), id->pid ); +} + #define MAX_SERVICE_NAME 260
struct device_interface @@ -783,6 +789,51 @@ NTSTATUS WINAPI IoSetDeviceInterfaceState( UNICODE_STRING *name, BOOLEAN enable return ret; }
+/*********************************************************************** + * IoSetDevicePropertyData (NTOSKRNL.EXE.@) + */ +NTSTATUS WINAPI IoSetDevicePropertyData( DEVICE_OBJECT *device, const DEVPROPKEY *property_key, LCID lcid, + ULONG flags, DEVPROPTYPE type, ULONG size, void *data ) +{ + SP_DEVINFO_DATA sp_device = {sizeof(sp_device)}; + WCHAR device_instance_id[MAX_DEVICE_ID_LEN]; + NTSTATUS status; + HDEVINFO set; + + TRACE( "device %p, property_key %s, lcid %#x, flags %#x, type %#x, size %u, data %p.\n", + device, debugstr_propkey(property_key), lcid, flags, type, size, data ); + + /* flags is always treated as PLUGPLAY_PROPERTY_PERSISTENT starting with Win 8 / 2012 */ + + if (lcid != LOCALE_NEUTRAL) FIXME( "only LOCALE_NEUTRAL is supported\n" ); + + if ((status = get_device_instance_id( device, device_instance_id ))) return status; + + if ((set = SetupDiCreateDeviceInfoList( &GUID_NULL, NULL )) == INVALID_HANDLE_VALUE) + { + ERR( "Failed to create device list, error %#x.\n", GetLastError() ); + return GetLastError(); + } + + if (!SetupDiOpenDeviceInfoW( set, device_instance_id, NULL, 0, &sp_device )) + { + ERR( "Failed to open device, error %#x.\n", GetLastError() ); + SetupDiDestroyDeviceInfoList( set ); + return GetLastError(); + } + + if (!SetupDiSetDevicePropertyW( set, &sp_device, property_key, type, data, size, 0 )) + { + ERR( "Failed to set property, error %#x.\n", GetLastError() ); + SetupDiDestroyDeviceInfoList( set ); + return GetLastError(); + } + + SetupDiDestroyDeviceInfoList( set ); + + return STATUS_SUCCESS; +} + /*********************************************************************** * IoRegisterDeviceInterface (NTOSKRNL.EXE.@) */ diff --git a/include/ddk/wdm.h b/include/ddk/wdm.h index 51097bfe3ab..447833c4bc9 100644 --- a/include/ddk/wdm.h +++ b/include/ddk/wdm.h @@ -21,6 +21,7 @@ #define _NTDDK_
#include <ntstatus.h> +#include <devpropdef.h>
#ifdef _WIN64 #define POINTER_ALIGNMENT DECLSPEC_ALIGN(8) @@ -1681,6 +1682,8 @@ void WINAPI ExReleaseResourceForThreadLite(ERESOURCE*,ERESOURCE_THREAD); ULONG WINAPI ExSetTimerResolution(ULONG,BOOLEAN); void WINAPI ExUnregisterCallback(void*);
+#define PLUGPLAY_PROPERTY_PERSISTENT 0x0001 + void WINAPI IoAcquireCancelSpinLock(KIRQL*); NTSTATUS WINAPI IoAcquireRemoveLockEx(IO_REMOVE_LOCK*,void*,const char*,ULONG, ULONG); NTSTATUS WINAPI IoAllocateDriverObjectExtension(PDRIVER_OBJECT,PVOID,ULONG,PVOID*); @@ -1728,6 +1731,7 @@ void WINAPI IoReleaseRemoveLockAndWaitEx(IO_REMOVE_LOCK*,void*,ULONG); void WINAPI IoReleaseRemoveLockEx(IO_REMOVE_LOCK*,void*,ULONG); void WINAPI IoReuseIrp(IRP*,NTSTATUS); NTSTATUS WINAPI IoSetDeviceInterfaceState(UNICODE_STRING*,BOOLEAN); +NTSTATUS WINAPI IoSetDevicePropertyData(DEVICE_OBJECT*,const DEVPROPKEY*,LCID,ULONG,DEVPROPTYPE,ULONG,void*); NTSTATUS WINAPI IoWMIRegistrationControl(PDEVICE_OBJECT,ULONG);
void FASTCALL KeAcquireInStackQueuedSpinLockAtDpcLevel(KSPIN_LOCK*,KLOCK_QUEUE_HANDLE*);
On 4/26/21 6:28 PM, Zebediah Figura (she/her) wrote:
On 4/26/21 5:37 AM, Rémi Bernon wrote:
From: Arkadiusz Hiler <ahiler(a)codeweavers.com>
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> ---
In PATCH 1, the return GetLastError() look wrong to me but the same is done in IoGetDeviceProperty so I kept them. I guess we would need to convert the setupapi error back to NTSTATUS, or implement the higher level setupapi on top of these lower level primitives intead.
It's maybe not exactly ideal, but it's also not clear how to map setupapi errors to NTSTATUS, and moreover it probably doesn't matter anyway (we don't expect those functions to fail).
I don't think we can implement setupapi functions on top of ntoskrnl ones; that crosses the kernel boundary (and there's no clear syscall interface for it). Probably on Windows the code is just duplicated.
FWIW, we have a PnP test driver now, so it's actually possible to write tests for this function.
In PATCH 2, I moved the error case before IoInvalidateDeviceRelations, because I don't know what the call is actually expecting. Maybe it was unnecessary.
I think it's correct as-is to move it—as soon as we call IoInvalidateDeviceRelations() the device can be exposed.
Actually, to be completely correct (i.e. if we were running this driver in Windows), I think we should treat it as possible to receive IRP_MN_QUERY_DEVICE_RELATIONS at any time. What we probably should be doing is to initialize everything necessary *before* the statement "fdo_ext->u.fdo.child_pdo = child_pdo", and then everything else at IRP_MN_START_DEVICE (on the child PDO).
It's actually causing IoSetDevicePropertyData to fail the first time a device is detected (so on prefix creation for both mouse and keyboard devices), as setupapi doesn't know about it until we call IoInvalidateDeviceRelations. Note however, although it fails to set the property, as SetupDiDestroyDeviceInfoList clears the last error, it still returns success, and the device initialization continues. I think it may be better to move it after, as it's less likely that anything happens in between, while missing the property is going to make rawinput device enumeration skip the device. But it's just a workaround. -- Rémi Bernon <rbernon(a)codeweavers.com>
On 5/4/21 12:37 PM, Rémi Bernon wrote:
On 4/26/21 6:28 PM, Zebediah Figura (she/her) wrote:
On 4/26/21 5:37 AM, Rémi Bernon wrote:
From: Arkadiusz Hiler <ahiler(a)codeweavers.com>
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> ---
In PATCH 1, the return GetLastError() look wrong to me but the same is done in IoGetDeviceProperty so I kept them. I guess we would need to convert the setupapi error back to NTSTATUS, or implement the higher level setupapi on top of these lower level primitives intead.
It's maybe not exactly ideal, but it's also not clear how to map setupapi errors to NTSTATUS, and moreover it probably doesn't matter anyway (we don't expect those functions to fail).
I don't think we can implement setupapi functions on top of ntoskrnl ones; that crosses the kernel boundary (and there's no clear syscall interface for it). Probably on Windows the code is just duplicated.
FWIW, we have a PnP test driver now, so it's actually possible to write tests for this function.
In PATCH 2, I moved the error case before IoInvalidateDeviceRelations, because I don't know what the call is actually expecting. Maybe it was unnecessary.
I think it's correct as-is to move it—as soon as we call IoInvalidateDeviceRelations() the device can be exposed.
Actually, to be completely correct (i.e. if we were running this driver in Windows), I think we should treat it as possible to receive IRP_MN_QUERY_DEVICE_RELATIONS at any time. What we probably should be doing is to initialize everything necessary *before* the statement "fdo_ext->u.fdo.child_pdo = child_pdo", and then everything else at IRP_MN_START_DEVICE (on the child PDO).
It's actually causing IoSetDevicePropertyData to fail the first time a device is detected (so on prefix creation for both mouse and keyboard devices), as setupapi doesn't know about it until we call IoInvalidateDeviceRelations.
Note however, although it fails to set the property, as SetupDiDestroyDeviceInfoList clears the last error, it still returns success, and the device initialization continues.
I think it may be better to move it after, as it's less likely that anything happens in between, while missing the property is going to make rawinput device enumeration skip the device. But it's just a workaround.
Sorry, can you please clarify—what's causing the initial call to fail?
On 5/4/21 9:11 PM, Zebediah Figura (she/her) wrote:
On 5/4/21 12:37 PM, Rémi Bernon wrote:
On 4/26/21 6:28 PM, Zebediah Figura (she/her) wrote:
On 4/26/21 5:37 AM, Rémi Bernon wrote:
From: Arkadiusz Hiler <ahiler(a)codeweavers.com>
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> ---
In PATCH 1, the return GetLastError() look wrong to me but the same is done in IoGetDeviceProperty so I kept them. I guess we would need to convert the setupapi error back to NTSTATUS, or implement the higher level setupapi on top of these lower level primitives intead.
It's maybe not exactly ideal, but it's also not clear how to map setupapi errors to NTSTATUS, and moreover it probably doesn't matter anyway (we don't expect those functions to fail).
I don't think we can implement setupapi functions on top of ntoskrnl ones; that crosses the kernel boundary (and there's no clear syscall interface for it). Probably on Windows the code is just duplicated.
FWIW, we have a PnP test driver now, so it's actually possible to write tests for this function.
In PATCH 2, I moved the error case before IoInvalidateDeviceRelations, because I don't know what the call is actually expecting. Maybe it was unnecessary.
I think it's correct as-is to move it—as soon as we call IoInvalidateDeviceRelations() the device can be exposed.
Actually, to be completely correct (i.e. if we were running this driver in Windows), I think we should treat it as possible to receive IRP_MN_QUERY_DEVICE_RELATIONS at any time. What we probably should be doing is to initialize everything necessary *before* the statement "fdo_ext->u.fdo.child_pdo = child_pdo", and then everything else at IRP_MN_START_DEVICE (on the child PDO).
It's actually causing IoSetDevicePropertyData to fail the first time a device is detected (so on prefix creation for both mouse and keyboard devices), as setupapi doesn't know about it until we call IoInvalidateDeviceRelations.
Note however, although it fails to set the property, as SetupDiDestroyDeviceInfoList clears the last error, it still returns success, and the device initialization continues.
I think it may be better to move it after, as it's less likely that anything happens in between, while missing the property is going to make rawinput device enumeration skip the device. But it's just a workaround.
Sorry, can you please clarify—what's causing the initial call to fail?
IoSetDevicePropertyData fails, because SetupDiOpenDeviceInfoW fails, because, I think, the registry keys haven't been created yet (it returns ERROR_NO_SUCH_DEVINST). My understanding (although I didn't investigate much) is that IoInvalidateDeviceRelations creates the registry keys if the device has not been seen before. The symptoms are these messages being printed when a device is first seen (so on initial prefix creation for mouse and keyboard devices): err:plugplay:IoSetDevicePropertyData Failed to open device, error 0xe000020b. -- Rémi Bernon <rbernon(a)codeweavers.com>
participants (4)
-
Arkadiusz Hiler -
Marvin -
Rémi Bernon -
Zebediah Figura (she/her)