October 2016 - Wine-Devel - list.winehq.org

Running drivers in userspace/VM

by KOLANICH

Hello everybody. ReactOS project is devoted to making an OS kernel binary compatible with NT kernel. In their official FAQ it is written that they cannot be replaced with GNU/Linux + Wine because of drivers which cannot be run in Wine. I'm a bit sceptical about their OS, so I wonder why cannot drivers be run in wine? Drivers are run in ring 0 or 1, right? Why it is impossible/too complicated to a) expose some privileged instructions into userland through API; b) expose IO space using IOPL; c) make a mock of NT kernel to allow the drivers use its ABI; d) patch the binaries automatically to use API instead of privileged instructions; e) if a driver doesn't works well enough in userland, create a micro-VM using VT-x and VT-d and put there NT kernel mock and the driver?

Oct. 7, 2016

Re: wined3d: Add missing '\n' to FIXME() messages.

by Henri Verbeet

On 6 October 2016 at 15:31, Andrey Gusev <andrey.goosev(a)gmail.com> wrote: > + FIXME("unknown_semantics(0x%08x).\n", semantic->usage); Could you change that to something like "Unrecognised semantic usage %#x.\n"? > + if (mmask) FIXME("_unrecognized_modifier(%#x).\n", mmask); Similarly, something like "Unrecognised modifier %#x.\n" would probably be more appropriate here.

Oct. 7, 2016

Re: [PATCH 02/16] rpcrt4: Restore original error code when ReadFile fails with ERROR_MORE_DATA.

by Jacek Caban

On 06.10.2016 14:27, Alexandre Julliard wrote: > Jacek Caban <jacek(a)codeweavers.com> writes: > >> @@ -448,6 +448,7 @@ static int rpcrt4_conn_np_read(RpcConnection *Connection, >> char *buf = buffer; >> BOOL ret = TRUE; >> unsigned int bytes_left = count; >> + DWORD err = GetLastError(); >> >> while (bytes_left) >> { >> @@ -460,6 +461,7 @@ static int rpcrt4_conn_np_read(RpcConnection *Connection, >> bytes_left -= bytes_read; >> buf += bytes_read; >> } >> + if (ret) SetLastError(err); > I'd suggest using NtReadFile instead. Good point, thanks. I sent an updated patch. It seems to me that we could even get rid the loop here once message mode support is in (but it would need closer look at rpcrt4 first). Thanks, Jacek

Oct. 6, 2016

Re: [PATCH 02/16] rpcrt4: Restore original error code when ReadFile fails with ERROR_MORE_DATA.

by Alexandre Julliard

Jacek Caban <jacek(a)codeweavers.com> writes: > @@ -448,6 +448,7 @@ static int rpcrt4_conn_np_read(RpcConnection *Connection, > char *buf = buffer; > BOOL ret = TRUE; > unsigned int bytes_left = count; > + DWORD err = GetLastError(); > > while (bytes_left) > { > @@ -460,6 +461,7 @@ static int rpcrt4_conn_np_read(RpcConnection *Connection, > bytes_left -= bytes_read; > buf += bytes_read; > } > + if (ret) SetLastError(err); I'd suggest using NtReadFile instead. -- Alexandre Julliard julliard(a)winehq.org

Oct. 6, 2016

Re: [PATCH] winebus.sys: Implement IOCTL_HID_GET_DEVICE_ATTRIBUTES for hid devices

by Aric Stewart

On 10/6/16 7:00 AM, Sebastian Lackner wrote: > On 06.10.2016 04:51, Aric Stewart wrote: >> Signed-off-by: Aric Stewart <aric(a)codeweavers.com> >> --- >> dlls/winebus.sys/bus.h | 1 + >> dlls/winebus.sys/bus_udev.c | 1 + >> dlls/winebus.sys/main.c | 41 +++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 43 insertions(+) >> >> >> >> 0001-winebus.sys-Implement-IOCTL_HID_GET_DEVICE_ATTRIBUTES-.txt >> >> >> diff --git a/dlls/winebus.sys/bus.h b/dlls/winebus.sys/bus.h >> index 099558b..342a66b 100644 >> --- a/dlls/winebus.sys/bus.h >> +++ b/dlls/winebus.sys/bus.h >> @@ -34,3 +34,4 @@ DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW, >> const GUID *class, const platform_vtbl *vtbl, DWORD platform_data_size) DECLSPEC_HIDDEN; >> DEVICE_OBJECT *bus_find_hid_device(const platform_vtbl *vtbl, void *platform_dev) DECLSPEC_HIDDEN; >> void bus_remove_hid_device(DEVICE_OBJECT *device) DECLSPEC_HIDDEN; >> +NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp) DECLSPEC_HIDDEN; >> diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c >> index 93e56f3..a01d88e 100644 >> --- a/dlls/winebus.sys/bus_udev.c >> +++ b/dlls/winebus.sys/bus_udev.c >> @@ -299,6 +299,7 @@ NTSTATUS WINAPI udev_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registry >> >> udev_driver_obj = driver; >> driver->MajorFunction[IRP_MJ_PNP] = common_pnp_dispatch; >> + driver->MajorFunction[IRP_MJ_INTERNAL_DEVICE_CONTROL] = hid_internal_dispatch; > > I guess you intentionally did not name it "common_*" because non-HID devices might > need a different handler, is that correct? > That is correct. -aric >> >> if (!(events[0] = CreateEventW(NULL, TRUE, FALSE, NULL))) >> goto error; >> diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c >> index 09a59d6..ef19322 100644 >> --- a/dlls/winebus.sys/main.c >> +++ b/dlls/winebus.sys/main.c >> @@ -30,7 +30,9 @@ >> #include "winternl.h" >> #include "winreg.h" >> #include "setupapi.h" >> +#include "winioctl.h" >> #include "ddk/wdm.h" >> +#include "ddk/hidport.h" >> #include "wine/debug.h" >> #include "wine/unicode.h" >> #include "wine/list.h" >> @@ -346,6 +348,45 @@ NTSTATUS WINAPI common_pnp_dispatch(DEVICE_OBJECT *device, IRP *irp) >> return status; >> } >> >> +NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp) >> +{ >> + NTSTATUS rc = irp->IoStatus.u.Status; > > I would suggest to use rename the variable to "status". > >> + IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation(irp); >> + struct device_extension *extension = (struct device_extension *)device->DeviceExtension; >> + >> + TRACE("(%p, %p)\n", device, irp); >> + >> + switch(irpsp->Parameters.DeviceIoControl.IoControlCode) > > There is a space missing after "switch". > >> + { >> + case IOCTL_HID_GET_DEVICE_ATTRIBUTES: >> + { >> + PHID_DEVICE_ATTRIBUTES attr = (PHID_DEVICE_ATTRIBUTES)irp->UserBuffer; > > It would be better to use HID_DEVICE_ATTRIBUTES * instead of the version with the "P" prefix. > Also, please add a check for Parameters.DeviceIoControl.OutputBufferLength to make sure > the buffer is big enough. > > >> + TRACE("IOCTL_HID_GET_DEVICE_ATTRIBUTES\n"); >> + >> + RtlZeroMemory(attr, sizeof(HID_DEVICE_ATTRIBUTES)); > > Is there any specific reason to use RtlZeroMemory instead of memset? Something like > memset(attr, 0, sizeof(*attr)) would be easier. ;) > >> + attr->Size = sizeof(HID_DEVICE_ATTRIBUTES); >> + attr->VendorID = extension->vid; >> + attr->ProductID = extension->pid; >> + attr->VersionNumber = extension->version; >> + irp->IoStatus.u.Status = rc = STATUS_SUCCESS; >> + irp->IoStatus.Information = sizeof(HID_DEVICE_ATTRIBUTES); >> + break; >> + } >> + default: >> + { >> + ULONG code = irpsp->Parameters.DeviceIoControl.IoControlCode; >> + FIXME("Unsupported ioctl %x (device=%x access=%x func=%x method=%x)\n", >> + code, code >> 16, (code >> 14) & 3, (code >> 2) & 0xfff, code & 3); >> + break; >> + } >> + } >> + >> + if (rc != STATUS_PENDING) >> + IoCompleteRequest( irp, IO_NO_INCREMENT ); > > Do you need to handle asynchronous requests in this function? If not you can remove the > if check. For consistency reasons, you should probably remove the space after "(" and > before ")" in the IoCompleteRequest call. > >> + >> + return rc; >> +} >> + >> NTSTATUS WINAPI DriverEntry( DRIVER_OBJECT *driver, UNICODE_STRING *path ) >> { >> static const WCHAR udevW[] = {'\\','D','r','i','v','e','r','\\','U','D','E','V',0}; >> >> >> >

Oct. 6, 2016

Re: [PATCH] winebus.sys: Implement IOCTL_HID_GET_DEVICE_ATTRIBUTES for hid devices

by Sebastian Lackner

On 06.10.2016 04:51, Aric Stewart wrote: > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/winebus.sys/bus.h | 1 + > dlls/winebus.sys/bus_udev.c | 1 + > dlls/winebus.sys/main.c | 41 +++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 43 insertions(+) > > > > 0001-winebus.sys-Implement-IOCTL_HID_GET_DEVICE_ATTRIBUTES-.txt > > > diff --git a/dlls/winebus.sys/bus.h b/dlls/winebus.sys/bus.h > index 099558b..342a66b 100644 > --- a/dlls/winebus.sys/bus.h > +++ b/dlls/winebus.sys/bus.h > @@ -34,3 +34,4 @@ DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW, > const GUID *class, const platform_vtbl *vtbl, DWORD platform_data_size) DECLSPEC_HIDDEN; > DEVICE_OBJECT *bus_find_hid_device(const platform_vtbl *vtbl, void *platform_dev) DECLSPEC_HIDDEN; > void bus_remove_hid_device(DEVICE_OBJECT *device) DECLSPEC_HIDDEN; > +NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp) DECLSPEC_HIDDEN; > diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c > index 93e56f3..a01d88e 100644 > --- a/dlls/winebus.sys/bus_udev.c > +++ b/dlls/winebus.sys/bus_udev.c > @@ -299,6 +299,7 @@ NTSTATUS WINAPI udev_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registry > > udev_driver_obj = driver; > driver->MajorFunction[IRP_MJ_PNP] = common_pnp_dispatch; > + driver->MajorFunction[IRP_MJ_INTERNAL_DEVICE_CONTROL] = hid_internal_dispatch; I guess you intentionally did not name it "common_*" because non-HID devices might need a different handler, is that correct? > > if (!(events[0] = CreateEventW(NULL, TRUE, FALSE, NULL))) > goto error; > diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c > index 09a59d6..ef19322 100644 > --- a/dlls/winebus.sys/main.c > +++ b/dlls/winebus.sys/main.c > @@ -30,7 +30,9 @@ > #include "winternl.h" > #include "winreg.h" > #include "setupapi.h" > +#include "winioctl.h" > #include "ddk/wdm.h" > +#include "ddk/hidport.h" > #include "wine/debug.h" > #include "wine/unicode.h" > #include "wine/list.h" > @@ -346,6 +348,45 @@ NTSTATUS WINAPI common_pnp_dispatch(DEVICE_OBJECT *device, IRP *irp) > return status; > } > > +NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp) > +{ > + NTSTATUS rc = irp->IoStatus.u.Status; I would suggest to use rename the variable to "status". > + IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation(irp); > + struct device_extension *extension = (struct device_extension *)device->DeviceExtension; > + > + TRACE("(%p, %p)\n", device, irp); > + > + switch(irpsp->Parameters.DeviceIoControl.IoControlCode) There is a space missing after "switch". > + { > + case IOCTL_HID_GET_DEVICE_ATTRIBUTES: > + { > + PHID_DEVICE_ATTRIBUTES attr = (PHID_DEVICE_ATTRIBUTES)irp->UserBuffer; It would be better to use HID_DEVICE_ATTRIBUTES * instead of the version with the "P" prefix. Also, please add a check for Parameters.DeviceIoControl.OutputBufferLength to make sure the buffer is big enough. > + TRACE("IOCTL_HID_GET_DEVICE_ATTRIBUTES\n"); > + > + RtlZeroMemory(attr, sizeof(HID_DEVICE_ATTRIBUTES)); Is there any specific reason to use RtlZeroMemory instead of memset? Something like memset(attr, 0, sizeof(*attr)) would be easier. ;) > + attr->Size = sizeof(HID_DEVICE_ATTRIBUTES); > + attr->VendorID = extension->vid; > + attr->ProductID = extension->pid; > + attr->VersionNumber = extension->version; > + irp->IoStatus.u.Status = rc = STATUS_SUCCESS; > + irp->IoStatus.Information = sizeof(HID_DEVICE_ATTRIBUTES); > + break; > + } > + default: > + { > + ULONG code = irpsp->Parameters.DeviceIoControl.IoControlCode; > + FIXME("Unsupported ioctl %x (device=%x access=%x func=%x method=%x)\n", > + code, code >> 16, (code >> 14) & 3, (code >> 2) & 0xfff, code & 3); > + break; > + } > + } > + > + if (rc != STATUS_PENDING) > + IoCompleteRequest( irp, IO_NO_INCREMENT ); Do you need to handle asynchronous requests in this function? If not you can remove the if check. For consistency reasons, you should probably remove the space after "(" and before ")" in the IoCompleteRequest call. > + > + return rc; > +} > + > NTSTATUS WINAPI DriverEntry( DRIVER_OBJECT *driver, UNICODE_STRING *path ) > { > static const WCHAR udevW[] = {'\\','D','r','i','v','e','r','\\','U','D','E','V',0}; > > >

Oct. 6, 2016

Re: Problems with WM_DROPFILES

by Michael Stefaniuc

On 10/06/2016 12:46 PM, John Found wrote: > On Thu, 6 Oct 2016 10:28:14 +0300 > John Found <johnfound(a)asm32.info> wrote: > >> Any ideas about fixing all this mess? Writing own WindowFromPoint especially for the drag&drop operations? >> > > Well, that was exactly what I tried and everything seems to work good. In the attachment is the modified file: "dlls/winex11.drv/xdnd.c" > > Please someone to review it and if it fits the standards of the project, commit it to the repository (or what the git people do with the modified files)... Diffs are easier to review so here it is (generated with just 'git diff') bye michael

Oct. 6, 2016

Re: Problems with WM_DROPFILES

by John Found

On Thu, 6 Oct 2016 10:28:14 +0300 John Found <johnfound(a)asm32.info> wrote: > Any ideas about fixing all this mess? Writing own WindowFromPoint especially for the drag&drop operations? > Well, that was exactly what I tried and everything seems to work good. In the attachment is the modified file: "dlls/winex11.drv/xdnd.c" Please someone to review it and if it fits the standards of the project, commit it to the repository (or what the git people do with the modified files)... If not, give me some directions what to do further. -- http://fresh.flatassembler.net http://asm32.info John Found <johnfound(a)asm32.info>

Oct. 6, 2016

Re: [PATCH 5/7] wined3d: Avoid destroying active contexts.

by Józef Kucia

On Thu, Oct 6, 2016 at 11:46 AM, Henri Verbeet <hverbeet(a)gmail.com> wrote: > On 6 October 2016 at 11:45, Józef Kucia <joseph.kucia(a)gmail.com> wrote: >> FWIW, a copy of the wined3d creation flags is stored in d3d_info, so >> "context->d3d_info.wined3d_creation_flags" is the proper way to get >> them. > Right, d3d_info is "new". Yeah, wined3d_creation_flags is even newer. It was introduced together with WINED3D_SRGB_READ_WRITE_CONTROL.

Oct. 6, 2016

Re: [PATCH 5/7] wined3d: Avoid destroying active contexts.

by Henri Verbeet

On 6 October 2016 at 11:45, Józef Kucia <joseph.kucia(a)gmail.com> wrote: > FWIW, a copy of the wined3d creation flags is stored in d3d_info, so > "context->d3d_info.wined3d_creation_flags" is the proper way to get > them. Right, d3d_info is "new".

Oct. 6, 2016