Message-ID: <YhjbzxxgxtSxFLe/@kroah.com>
Date:   Fri, 25 Feb 2022 14:38:23 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Benjamin Tissoires <benjamin.tissoires@...hat.com>
Cc:     Jiri Kosina <jikos@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>, Shuah Khan <shuah@...nel.org>,
        Dave Marchevsky <davemarchevsky@...com>,
        Joe Stringer <joe@...ium.io>,
        Tero Kristo <tero.kristo@...ux.intel.com>,
        lkml <linux-kernel@...r.kernel.org>,
        "open list:HID CORE LAYER" <linux-input@...r.kernel.org>,
        netdev@...r.kernel.org, bpf@...r.kernel.org,
        linux-kselftest@...r.kernel.org,
        Peter Hutterer <peter.hutterer@...hat.com>
Subject: Re: [PATCH bpf-next v1 0/6] Introduce eBPF support for HID devices

On Thu, Feb 24, 2022 at 02:49:21PM +0100, Benjamin Tissoires wrote:
> Hi Greg,
> 
> Thanks for the quick answer :)
> 
> On Thu, Feb 24, 2022 at 12:31 PM Greg KH <gregkh@...uxfoundation.org> wrote:
> >
> > On Thu, Feb 24, 2022 at 12:08:22PM +0100, Benjamin Tissoires wrote:
> > > Hi there,
> > >
> > > This series introduces support of eBPF for HID devices.
> > >
> > > I have several use cases where eBPF could be interesting for those
> > > input devices:
> > >
> > > - simple fixup of report descriptor:
> > >
> > > In the HID tree, we have half of the drivers that are "simple" and
> > > that just fix one key or one byte in the report descriptor.
> > > Currently, for users of such devices, the process of fixing them
> > > is long and painful.
> > > With eBPF, we could externalize those fixups in one external repo,
> > > ship various CoRe bpf programs and have those programs loaded at boot
> > > time without having to install a new kernel (and wait 6 months for the
> > > fix to land in the distro kernel)
> >
> > Why would a distro update such an external repo faster than they update
> > the kernel?  Many sane distros update their kernel faster than other
> > packages already, how about fixing your distro?  :)
> 
> Heh, I'm going to try to dodge the incoming rhel bullet :)
> 
> It's true that thanks to the work of the stable folks we don't have to
> wait 6 months for a fix to come in. However, I think having a single
> file to drop in a directory would be easier for development/testing
> (and distribution of that file between developers/testers) than
> requiring people to recompile their kernel.
> 
> Brain fart: is there any chance we could keep the validated bpf
> programs in the kernel tree?

That would make the most sense to me.  And allow "slow" distros to
override the HID bpf quirks easily if they need to.  If you do that,
then most of my objections of the "now the code is in two places that
you have to track" goes away :)

> > I'm all for the idea of using ebpf for HID devices, but now we have to
> > keep track of multiple packages to be in sync here.  Is this making
> > things harder overall?
> 
> Probably, and this is also maybe opening a can of worms. Vendors will
> be able to say "use that bpf program for my HID device because the
> firmware is bogus".
> 
> OTOH, as far as I understand, you can not load a BPF program in the
> kernel that uses GPL-declared functions if your BPF program is not
> GPL. Which means that if firmware vendors want to distribute blobs
> through BPF, either it's GPL and they have to provide the sources, or
> it's not happening.

You can make the new HID bpf api only availble to GPL programs, and if I
were you, that's what I would do just to keep any legal issues from
coming up.  Also bundling it with the kernel makes it easier.

> I am not entirely clear on which plan I want to have for userspace.
> I'd like to have libinput on board, but right now, Peter's stance is
> "not in my garden" (and he has good reasons for it).
> So my initial plan is to cook and hold the bpf programs in hid-tools,
> which is the repo I am using for the regression tests on HID.

Why isn't the hid regression tests in the kernel tree also?  That would
allow all of the testers out there to test things much easier than
having to suck down another test repo (like Linaro and 0-day and
kernelci would be forced to do).

> I plan on building a systemd intrinsic that would detect the HID
> VID/PID and then load the various BPF programs associated with the
> small fixes.
> Note that everything can not be fixed through eBPF, simply because at
> boot we don't always have the root partition mounted.

Root partitions are now on HID devices?  :)

> > > - Universal Stylus Interface (or any other new fancy feature that
> > >   requires a new kernel API)
> > >
> > > See [0].
> > > Basically, USI pens are requiring a new kernel API because there are
> > > some channels of communication our HID and input stack are not capable
> > > of. Instead of using hidraw or creating new sysfs or ioctls, we can rely
> > > on eBPF to have the kernel API controlled by the consumer and to not
> > > impact the performances by waking up userspace every time there is an
> > > event.
> >
> > How is userspace supposed to interact with these devices in a unified
> > way then?  This would allow random new interfaces to be created, one
> > each for each device, and again, be a pain to track for a distro to keep
> > in sync.  And how are you going to keep the ebpf interface these
> > provides in sync with the userspace program?
> 
> Right now, the idea we have is to export the USI specifics through
> dbus. This has a couple of advantages: we are not tied to USI and can
> "emulate" those parameters by storing them on disk instead of in the
> pen, and this is easily accessible from all applications directly.
> 
> I am trying to push to have one implementation of that dbus service
> with the Intel and ChromeOS folks so general linux doesn't have to
> recreate it. But if you look at it, with hidraw nothing prevents
> someone from writing such a library/daemon in its own world without
> sharing it with anybody.
> 
> The main advantage of eBPF compared to hidraw is that you can analyse
> the incoming event without waking userspace and only wake it up when
> there is something noticeable.

That is a very good benefit, and one that many battery-powered devices
would like.

> In terms of random interfaces, yes, this is a good point. But the way
> I see it is that we can provide one kernel API (eBPF for HID) which we
> will maintain and not have to maintain forever a badly designed kernel
> API for a specific device. Though also note that USI is a HID standard
> (I think there is a second one), so AFAICT, the same bpf program
> should be able to be generic enough to be cross vendor. So there will
> be one provider only for USI.

Ok, that's good to know.

<good stuff snipped>

> Yeah, I completely understand the view. However, please keep in mind
> that most of it (though not firewall and some corner cases of tracing)
> is already possible to do through hidraw.
> One other example of that is SDL. We got Sony involved to create a
> nice driver for the DualSense controller (the PS5 one), but they
> simply ignore it and use plain HID (through hidraw). They have the
> advantage of this being cross-platform and can provide a consistent
> experience across platforms. And as a result, in the kernel, we have
> to hands up the handling of the device whenever somebody opens a
> hidraw node for those devices (Steam is also doing the same FWIW).
> 
> Which reminds me that I also have another use-case: joystick
> dead-zone. You can have a small filter that configures the dead zone
> and doesn't even wake up userspace for those hardware glitches...

hidraw is a big issue, and I understand why vendors use that and prefer
it over writing a kernel driver.  They can control it and ship it to
users and it makes life easier for them.  It's also what Windows has
been doing for decades now, so it's a comfortable interface for them to
write their code in userspace.

But, now you are going to ask them to use bpf instead?  Why would they
switch off of hidraw to use this api?  What benefit are you going to
provide them here for that?

This is why I've delayed doing bpf for USB.  Right now we have a nice
cross-platform way to write userspace USB drivers using usbfs/libusb.
All a bpf api to USB would be doing is much the same thing that libusb
does, for almost no real added benefit that I can tell.

USB, is a really "simple" networking like protocol (send/recieve
packets).  So it ties into the bpf model well.  But the need to use bpf
for USB so far I don't have a real justification.

And the same thing here.  Yes it is cool, and personally I love it, but
what is going to get people off of hidraw to use this instead?  You
can'd drop hidraw now (just like I can't drop usbfs), so all this means
is we have yet-another-way to do something on the system.  Is that a
good idea?  I don't know.

Also you mention dbus as the carrier for the HID information to
userspace programs.  Is dbus really the right thing for sending streams
of input data around?  Yes it will probably work, but I don't think it
was designed for that at all, so the overhead involved might just
overshadow any of the improvements you made using bpf.  And also, you
could do this today with hidraw, right?

> Anyway, IOW, I think the bpf approach will allow kernel-like
> performances of hidraw applications, and I would be more inclined to
> ask people to move their weird issue in userspace thanks to that.

I like this from a "everyone should use bpf" point of view, but how are
you going to tell people "use bpf over hidraw" in a way that gets them
to do so?  If you have a good answer for that, I might just steal it for
the bpf-USB interface as well :)

thanks,

greg k-h

Powered by blists - more mailing lists