On Thu, May 19, 2022 at 12:32 PM Greg KH [off-list ref] wrote:

On Thu, May 19, 2022 at 01:38:58AM -0700, Christoph Hellwig wrote:

quoted

On Thu, May 19, 2022 at 10:20:35AM +0200, Greg KH wrote:

quoted

quoted

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.

quoted

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.

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.)

And that "exception handling" is most of the driver.

For a number of "small" drivers, yes, that's all there is as the
hardware is "broken" and needs to be fixed up in order to work properly
with the hid core code.  An example of that would be hid-samsung.c which
rewrites the descriptors to be sane and maps the mouse buttons properly.

But that's it, after initialization that driver gets out of the way and
doesn't actually control anything.  From what I can tell, this patchset
would allow us to write those "fixup the mappings and reports before the
HID driver takes over" into ebpf programs.

It would not replace "real" HID drivers like hid-rmi.c that has to
handle the events and do other "real" work here.

Or I could be reading this code all wrong, Benjamin?

You get it right. hid-rmi is a good example of something that can not
sanely be done with eBPF.
We can do some filtering on the events (dropping one event, changing
one other), but anything outside that would not be possible. This
driver does a lot of scheduling, synchronisation, and various setup
that would require a lot of back and forth between userspace and
BPF/kernel, which makes it definitively not fit for a BPF
implementation.

But even if it would allow us to write HID drivers as ebpf, what is
wrong with that?  It's not a licensing issue (this api is only allowed
for GPL ebpf programs), it should allow us to move a bunch of in-kernel
drivers into smaller ebpf programs instead.

The one thing I also like with eBPF is that it is safe. When you
declare an array of bytes, the verifier enforces we don't go outside
of the boundaries.
I know rust is coming, but compared to plain C, that is much better,
even if more restrictive.
And it will also prevent some potential bugs where we have a report
fixup that writes outside of the reserved memory.

It's not like this ebpf HID driver would actually work on any other
operating system, right?  I guess Microsoft could create a gpl-licensed
ebpf HID layer as well?  As Windows allows vendors to do all of this
horrible HID fixups in userspace today anyway, I strongly doubt they
would go through the effort to add a new api like this for no valid
reason.

OTOH, managing to push Microsoft to implement HID-BPF would be some
achievement :) (just kidding).

Cheers,
Benjamin