On Thu, May 19, 2022 at 10:39 AM Christoph Hellwig hch@infradead.org wrote:
On Thu, May 19, 2022 at 10:20:35AM +0200, Greg KH wrote:
are written using a hip new VM?
Ugh, don't mention UDI, that's a bad flashback...
But that is very much what we are doing here.
I thought the goal here was to move a lot of the quirk handling and "fixup the broken HID decriptors in this device" out of kernel .c code and into BPF code instead, which this patchset would allow.
Yes, quirks are a big motivation for this work. Right now half of the HID drivers are less than 100 lines of code, and are just trivial fixes (one byte in the report descriptor, one key mapping, etc...). Using eBPF for those would simplify the process from the user point of view: you drop a "firmware fix" as an eBPF program in your system and you can continue working on your existing kernel.
The other important aspect is being able to do filtering on the event streams themselves. This would mean for instance that you allow some applications to have access to part of the device features and you reject some of them. The main use case I have is to prevent applications to switch a device into its bootloader mode and mess up with the firmware.
So that would just be exception handling. I don't think you can write a real HID driver here at all, but I could be wrong as I have not read the new patchset (older versions of this series could not do that.)
Well, to be fair, yes and no. HID-BPF can only talk HID, and so we only act on arrays of bytes. You can mess up with the report descriptor or the events themselves, but you don't have access to other kernel APIs. So no, you can not write a HID-BPF driver that would manually create LEDs sysfs endpoints, input endpoints and battery endpoints.
However, HID is very versatile in how you can describe a device. And the kernel now supports a lot of those features. So if you really want, you can entirely change the look of the device (its report descriptor), and rely on hid-core to export those LEDs, inputs and battery endpoints.
But we already have this available by making use of hidraw+uhid. This involves userspace and there are already projects (for handling Corsair keyboard for example) which are doing exactly that, with a big security whole in the middle because the application is reading *all* events as they are flowing.
One of the most important things here is that this work allows for context driven behavior. We can now control how a device is behaving depending on the actual application without having to design and maintain forever kernel APIs. For example, the Surface Dial is a puck that can have some haptic feedback when you turn it. However, when you enable the haptic feedback you have to reduce the resolution to one event every 5 degrees or the haptic feedback feels just wrong. But the device is capable of sub-degrees of event notifications. Which means you want the high resolution mode without haptic, and low res with haptic.
Of course, you can use some new FF capabilities to enable/disable haptic, but we have nothing to change the resolution on the fly of a HID device, so we'll likely have to create another kernel API through a sysfs node or a kernel parameter. But then we need to teach userspace to use it and this kernel API is not standard, so it won't be used outside of this particular device. BPF in that case allows the application which needs it to do the changes it requires depending on the context. And when I say application, it is mostly either the compositor or a daemon, not gimp.
And that "exception handling" is most of the driver.
Well, it depends. If hardware makers would not make crappy decisions based on the fact that it somehow works under Windows, we wouldn't have to do anything to support those devices. But for half of the drivers, we are doing dumb things to fix those devices in the kernel.
On the other hand, we do have generic protocols in HID that can not be replaced by BPF. For the exercise, I tried to think about what it would take to rewrite the multitouch logic in eBPF, and trust me, you don't want that. The code would be a lot of spaghetti and would require access to many kernel APIs to handle it properly.
Cheers, Benjamin