[PATCH 2/2] hidclass.sys: Create Driver reg key for HID devices.

[Zebediah Figura (she/her)]

On 4/13/21 2:03 PM, Arkadiusz Hiler wrote:
> On Tue, Apr 13, 2021 at 10:54:12AM -0500, Zebediah Figura (she/her) wrote:
>> On 4/13/21 10:31 AM, Arkadiusz Hiler wrote:
>>> SDL doesn't use HID devices directly if they don't have SPDRP_DRIVER
>>> registry property set. It uses another input method as a fallback instead.
>>>
>>> Since SDL_Joystick GUIDs are dependant on the input method this can mess
>>> with mappings and controller type detection.
>>>
>>> The change fixes Hades wrongly displaying Xbox prompts with Sony controllers
>>> over winebus.sys/bus_udev.c.
>>
>> It seems that Windows actually installs a null function driver (i.e. one
>> with the SPSVCINST_ASSOCSERVICE flag set) for HID devices, so setupapi will
>> create the key automatically in this case.
> 
> I was just following what we already do for the display drivers
> (winex11.drv, winemac.drv).
> 
> So we would need to add an .inf with an appropriate AddService directive
> and install it instead of just adding the driver property pointing
> to an empty driver key?

Right. On Windows this is c:\windows\input.inf.

> 
>> I'm not sure how that interacts with drivers that also want to layer onto
>> the HID device; probably it means our SetupDiSelectBestCompatDrv()
>> implementation needs to be less stubby.
> 
> I don't understand this comment. I tried to follow the function's usage
> + see how we end up creating a HID device out of the underlaying one but
> the call chains are long and it gets confusing quickly.
> 
> Looks like SetupDiSelectBestCompatDrv() is used by
> SetupDiCallClassInstaller(DIF_SELECTBESTCOMPATDRV) and that in turn is
> used by ntoskrnl.exe with each function when adding a new PNP device.
> 
> It seems that whenever a specific driver (one of the implemenations in
> winebus.sys) detects a new hardware it creates a device on it's own bus
> (e.g. HIDRAW for udev) and then calls IoInvalidateDeviceRelations()
> 
> IoInvalidateDeviceRelations() -> handle_bus_relations() ->
> enumerate_new_device() -> install_device_driver() ->
> SetupDiCallClassInstaller(DIF_REGISTER_COINSTLLERS) ->
> SetupDiRegisterCoDeviceInstllers() -> create_driver_key().
> 
> Which installs the winehid.ini for the device on the underlaying HIDRAW bus.
> 
> Then enumerate_new_device() ends up calling start_device() which creates
> HID device by invoking hidclass.sys/pnp.c:PNP_AddDevice().
> 
> Right now hidclass.sys (just like the display drivers) seems to be in
> charge of its own SetupAPI entries.
> 
> How exactly does SetupDiSelectBestCompatDrv() tie into that?
> 

It is long and complicated ;-)

The basic point of .inf files is to detect a layered driver for a 
device. input.inf contains an section that says "the layered driver for 
this device is no driver at all", which isn't the same as a driver not 
being present. But it's possible for a third-party driver to layer on 
top of HID (or even a first-party driver, such as mouhid.sys), in which 
case that driver *should* take precedence.

SetupDiSelectBestCompatDrv() is supposed to pick the "best" driver. 
According to [1] it should try to match hardware IDs in order, so that a 
driver targeting a specific hardware ID "HID\VID_1234&PID_5678&MI_00" is 
selected instead of a driver targeting a less specific "HID_DEVICE" ID.

DIF_SELECTBESTCOMPATDRV is done by UpdateDriverForPlugAndPlayDevices(), 
or the ntoskrnl equivalent install_device_driver(). Basically the order 
goes:

1. Call SetupDiBuildDriverInfoList(), which scans all installed .inf 
files for matching drivers and stores them in a list.
2. Call SetupDiCallClassInstaller(DIF_SELECTBESTCOMPATDRV), which picks 
the best driver. In Wine this currently picks the first driver, since we 
don't really expect there to be more than one. We should probably print 
an explicit FIXME instead of a WARN if there are.
3. Call SetupDiCallClassInstaller with other DIF_* values, these 
populate various registry keys and copy files, etc.

Honestly, I think with the aforementioned FIXME, it's probably not 
necessary to improve SetupDiSelectBestCompatDrv() yet. On the other 
hand, it probably wouldn't be that hard.

[1] 
https://docs.microsoft.com/en-us/windows-hardware/drivers/install/identifier-score--windows-vista-and-later-