[PATCH v3] winebus.sys: Handle device reports for hidraw devices

[Aric Stewart]

On 10/19/16 10:21 PM, Sebastian Lackner wrote:
> On 19.10.2016 21:32, Aric Stewart wrote:
>> Hi,
>>
>> It is important to remember that there is a big difference between
>> the user level communications with the HID device and the minidriver
>> level communications. The top of the stack, the HID device, has a
>> ring buffer and such.
>>
> [...]
>>
>> Depends on the call that is made. IOCTL_HID_GET_INPUT_REPORT should
>> always return immediately, if there is no report then it returns 0.
>> If there is a report it returns the last report from the device.
>>
>> IOCTL_HID_READ_REPORT is expected to pend until a report is
>> available. In the grand scheme of things IOCTL_HID_READ_REPORT is
>> expected to block until a report is available because the general use
>> is that hidclass will be in a loop where it does a
>> IOCTL_HID_READ_REPORT call and when the call succeeds it places that
>> report into the ring buffer and then immediately calls again to get
>> the next report.
>>
>> IOCTL_HID_GET_INPUT_REPORT is generated by the corresponding
>> IOCTL_HID_GET_INPUT_REPORT in the class driver and
>> HidD_GetInputReport.
>>
>> IOCTL_HID_READ_REPORT is like doing a FileRead.
> 
> I'm aware of those differences. The part I'm more interested in is how
> IOCTL_HID_READ_REPORT is implemented in Windows drivers. ;)
> Even if Windows does not explicitly mention any kind of buffering, I
> believe that data loss is much more unlikely than with the proposed Wine
> implementation.
> 
> Usually hardware events trigger an interrupt service routine, which can
> immediately complete the request. The code responsible for queuing a new
> IOCTL_HID_READ_REPORT ioctl also runs in kernel mode, maybe even with a
> higher priority to avoid any interruptions by user mode processes.
> In addition, devices might also do their own buffering, to ensure that
> the transmission works reliable.
> 
>>
>> So they need to behave quite a bit differently.
>>
>>>> https://www.julianloehr.de/wp-content/uploads/2014/07/Paper_HIDWiimote.pdf
>>>
>>>>
>> """In case the Wiimote state changes when there is no read request
>>>> currently buffered, a flag is used to signal whether a newly
>>>> received read request shall be processed and completed
>>>> immediately."""
>>>>
>> That would not be hard, just a flag that showed if the last reports
>> has been read or not. In all practical terms the minidriver only has
>> 1 consumer, the hidclass class driver devices, so in many ways the
>> whole IRP queue is overkill as there should only ever be 1 client.
>> It would not be hard to add that.
>>
>>>> In Wine we have to cache the last report anyway, so we could
>>>> implement something similar very easy. Do you think it makes
>>>> sense? The implementation of IOCTL_HID_GET_INPUT_REPORT and
>>>> IOCTL_HID_READ_REPORT would also not be much more difficult, as
>>>> shown in the following pseudocode:
>>>>
>>>> --- snip --- case IOCTL_HID_GET_INPUT_REPORT: case
>>>> IOCTL_HID_READ_REPORT: EnterCriticalSection(report_cs); ... start
>>>> processing if not done yet ... if (ioctl == IOCTL_HID_READ_REPORT
>>>> && (!last_report || last_report_seen)) { ... async handling ... 
>>>> break; } ... synchronous handlng ... last_report_seen = TRUE; 
>>>> LeaveCriticalSection(report_cs); break; --- snip ---
>> I will have to explore this when I have more time to code things but
>> the basic idea looks pretty good though IOCTL_HID_GET_INPUT_REPORT
>> uses HID_XFER_PACKETs and IOCTL_HID_READ_REPORT does not so that
>> complicates things a fair bit more.
> 
> You are right, it would be a bit more complicated. Probably we could
> move some of the code to a helper function, but I guess we will have
> to try it out. ;)
> 
> Another idea we could consider (at least for Linux) would be to use
> wine_server_fd_to_handle(), and then NtReadFile(). This would allow
> to get rid of the thread, and we could use an APC to run IoCompleteRequest().
> Have you actually looked into that? And which mechanism is required
> on other platforms, do they also require a separate thread?
> 

I have not explored wine_server_fd_to_handle as I was not really aware of it, Mac and IOHID uses a callback, so a run loop thread is needed, but no read of any sort.  If you thinking doing that wine magic is correct then I would be up for exploring it, but it seems like the sort of thing that Alexandre would frown on. 

-aric


>>
>> -aric
>>
>