| Message ID | 20240903-hid-bpf-hid-generic-v1-4-9511a565b2da@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | HID: bpf: add a new hook to control hid-generic | expand |
On Sep 03 2024, Peter Hutterer wrote: > On Tue, Sep 03, 2024 at 01:14:34AM +0900, Benjamin Tissoires wrote: > > The use case is when we fix a device through HID-BPF, 99% of the cases > > we want the device to use hid-generic now instead of a dedicated device. > > s/dedicated device/dedicated driver/ in the commit message > > > That's because the dedicated device might also want to change the report > > descriptor, or will be handling the device in a different way the new > > fixed device is using. > > > > In hid-core, after matching for the device (so that we only call this new > > hook on compatible drivers), we call for `.hid_bpf_driver_probe`. > > The function can not communicate with the device because it is not yet > > started, but it can make educated guesses and decide to: > > - let hid-core decide by itself > > - force the use of this driver (by comparing the provided name with > > "hid-generic" for instance) > > - force hid-core to ignore this driver for this device. > > > > For API stability, we don't rely on a bitfield or a return value for > > chosing hid-core behavior. We simply have a couple of writeable fields > > in the new struct hid_bpf_driver, and then hid-core can make its educated > > decision. > > > > Signed-off-by: Benjamin Tissoires <bentiss@kernel.org> > > --- > > Documentation/hid/hid-bpf.rst | 2 +- > > drivers/hid/bpf/hid_bpf_dispatch.c | 31 ++++++++++++++++++++++++++++ > > drivers/hid/bpf/hid_bpf_struct_ops.c | 3 +++ > > drivers/hid/hid-core.c | 6 ++++++ > > include/linux/hid_bpf.h | 40 ++++++++++++++++++++++++++++++++++++ > > 5 files changed, 81 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/hid/hid-bpf.rst b/Documentation/hid/hid-bpf.rst > > index 5939eeafb361..05a43f11cdab 100644 > > --- a/Documentation/hid/hid-bpf.rst > > +++ b/Documentation/hid/hid-bpf.rst > > @@ -190,7 +190,7 @@ User API data structures available in programs: > > ----------------------------------------------- > > > > .. kernel-doc:: include/linux/hid_bpf.h > > - :identifiers: hid_bpf_ctx > > + :identifiers: hid_bpf_ctx hid_bpf_driver > > > > Available API that can be used in all HID-BPF struct_ops programs: > > ------------------------------------------------------------------ > > diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c > > index a272a086c950..2df136d64152 100644 > > --- a/drivers/hid/bpf/hid_bpf_dispatch.c > > +++ b/drivers/hid/bpf/hid_bpf_dispatch.c > > @@ -189,6 +189,37 @@ u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *s > > } > > EXPORT_SYMBOL_GPL(call_hid_bpf_rdesc_fixup); > > > > +int call_hid_bpf_driver_probe(struct hid_device *hdev, struct hid_driver *hdrv, > > + const struct hid_device_id *id) > > +{ > > + struct hid_bpf_driver drv = { 0 }; > > + struct hid_bpf_ops *e; > > + int idx; > + > > + if (strscpy(drv.name, hdrv->name, sizeof(drv.name)) < 0) > > + return 0; > > + > > + idx = srcu_read_lock(&hdev->bpf.srcu); > > + list_for_each_entry_srcu(e, &hdev->bpf.prog_list, list, > > + srcu_read_lock_held(&hdev->bpf.srcu)) { > > + if (!e->hid_driver_probe) > > + continue; > > + > > + e->hid_driver_probe(hdev, &drv, id); > > + } > > + > > + srcu_read_unlock(&hdev->bpf.srcu, idx); > > + > > + if (drv.force_driver) > > + return 1; > > + > > + if (drv.ignore_driver) > > + return -1; > > + > > + return 0; > > +} > > +EXPORT_SYMBOL_GPL(call_hid_bpf_driver_probe); > > + > > static int device_match_id(struct device *dev, const void *id) > > { > > struct hid_device *hdev = to_hid_device(dev); > > diff --git a/drivers/hid/bpf/hid_bpf_struct_ops.c b/drivers/hid/bpf/hid_bpf_struct_ops.c > > index cd696c59ba0f..1e13a22f73a1 100644 > > --- a/drivers/hid/bpf/hid_bpf_struct_ops.c > > +++ b/drivers/hid/bpf/hid_bpf_struct_ops.c > > @@ -46,6 +46,7 @@ static int hid_bpf_ops_check_member(const struct btf_type *t, > > case offsetof(struct hid_bpf_ops, hid_rdesc_fixup): > > case offsetof(struct hid_bpf_ops, hid_hw_request): > > case offsetof(struct hid_bpf_ops, hid_hw_output_report): > > + case offsetof(struct hid_bpf_ops, hid_driver_probe): > > break; > > default: > > if (prog->sleepable) > > @@ -79,6 +80,8 @@ static int hid_bpf_ops_btf_struct_access(struct bpf_verifier_log *log, > > WRITE_RANGE(hid_device, name, true), > > WRITE_RANGE(hid_device, uniq, true), > > WRITE_RANGE(hid_device, phys, true), > > + WRITE_RANGE(hid_bpf_driver, force_driver, false), > > + WRITE_RANGE(hid_bpf_driver, ignore_driver, false), > > }; > > #undef WRITE_RANGE > > const struct btf_type *state = NULL; > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > > index 988d0acbdf04..7845f0a789ec 100644 > > --- a/drivers/hid/hid-core.c > > +++ b/drivers/hid/hid-core.c > > @@ -2639,10 +2639,16 @@ static bool hid_check_device_match(struct hid_device *hdev, > > struct hid_driver *hdrv, > > const struct hid_device_id **id) > > { > > + int ret; > > + > > *id = hid_match_device(hdev, hdrv); > > if (!*id) > > return false; > > > > + ret = call_hid_bpf_driver_probe(hdev, hdrv, *id); > > + if (ret) > > + return ret > 0; > > + > > if (hdrv->match) > > return hdrv->match(hdev, hid_ignore_special_drivers); > > > > diff --git a/include/linux/hid_bpf.h b/include/linux/hid_bpf.h > > index d4d063cf63b5..20693c218857 100644 > > --- a/include/linux/hid_bpf.h > > +++ b/include/linux/hid_bpf.h > > @@ -9,6 +9,7 @@ > > #include <uapi/linux/hid.h> > > > > struct hid_device; > > +struct hid_driver; > > > > /* > > * The following is the user facing HID BPF API. > > @@ -80,6 +81,22 @@ struct hid_ops { > > > > extern struct hid_ops *hid_ops; > > > > +/** > > + * struct hid_bpf_driver - User accessible data for the ``hid_bpf_probe`` > > + * struct_ops > > + * > > + * @name: the name of the driver currently being treated > > + * @force_driver: set this to ``true`` to force hid-core to use this driver, > > + * bypassing any further decision made by this driver > > + * @ignore_driver: set this to ``true`` to force hid-core to ignore this driver, > > + * bypassing any further decision made by this driver > > If I set both to false or true, what happens? The two seem to be force_driver has priority over ignore_driver. > mutually exclusive, in userspace I'd use an enum here to have a > NOOP/FORCE_DRIVER/IGNORE_DRIVER value range (that can be extended later). > Maybe something like that is an option? enum also has the advantage to be exported in vmlinux.h. FWIW, the idea behind adding new fields in a struct was to get the backward compatibility for free. Because the verifier/relocator will see if we are using the correct field entries. OTOH, maybe we can make the function return the afformended enum, and drop those two fields. I think we should probably abort processing of any bpf sets the return value to anything else than NOOP. I'll work a little bit more on that. Cheers, Benjamin > > > + */ > > +struct hid_bpf_driver { > > + __u8 name[64]; > > + bool force_driver; > > + bool ignore_driver; > > +}; > > + > > /** > > * struct hid_bpf_ops - A BPF struct_ops of callbacks allowing to attach HID-BPF > > * programs to a HID device > > @@ -178,6 +195,25 @@ struct hid_bpf_ops { > > */ > > int (*hid_hw_output_report)(struct hid_bpf_ctx *ctx, u64 source); > > > > + /** > > + * @hid_driver_probe: called before the kernel ``.probe()`` function > > + * > > + * It has the following arguments: > > + * > > + * ``hdev``: The HID device kernel representation > > + * > > + * ``hdrv``: A BPF partially writeable representation of a HID driver > > + * > > + * ``id``: The device match structure found in the driver > > + * > > + * Note that the device has not been started yet, and thus kfuncs like > > + * ``hid_hw_output_report`` will likely fail. > > Just to confirm, I can access the device's report descriptor though? For > the devices that we're looking at (e.g. the foot pedals pretending to be > an apple keyboard) the driver name and what we can set in HID_BPF_CONFIG > are not going to be enough, we'll have to check the rdesc too. > > Cheers, > Peter > > > + * > > + * This function is useful to force/ignore a given supported HID driver, > > + * by writing ``true`` in ``hdrv->force_driver`` or ``hdrv->ignore_driver`` > > + */ > > + void (*hid_driver_probe)(struct hid_device *hdev, struct hid_bpf_driver *hdrv, > > + const struct hid_device_id *id); > > > > /* private: do not show up in the docs */ > > struct hid_device *hdev; > > @@ -213,6 +249,8 @@ void hid_bpf_disconnect_device(struct hid_device *hdev); > > void hid_bpf_destroy_device(struct hid_device *hid); > > int hid_bpf_device_init(struct hid_device *hid); > > u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *size); > > +int call_hid_bpf_driver_probe(struct hid_device *hdev, struct hid_driver *hdrv, > > + const struct hid_device_id *id); > > #else /* CONFIG_HID_BPF */ > > static inline u8 *dispatch_hid_bpf_device_event(struct hid_device *hid, enum hid_report_type type, > > u8 *data, u32 *size, int interrupt, > > @@ -228,6 +266,8 @@ static inline int hid_bpf_connect_device(struct hid_device *hdev) { return 0; } > > static inline void hid_bpf_disconnect_device(struct hid_device *hdev) {} > > static inline void hid_bpf_destroy_device(struct hid_device *hid) {} > > static inline int hid_bpf_device_init(struct hid_device *hid) { return 0; } > > +static inline int call_hid_bpf_driver_probe(struct hid_device *hdev, struct hid_driver *hdrv, > > + const struct hid_device_id *id) { return 0; } > > /* > > * This specialized allocator has to be a macro for its allocations to be > > * accounted separately (to have a separate alloc_tag). The typecast is > > > > -- > > 2.46.0 > >
On Sep 03 2024, Benjamin Tissoires wrote: > On Sep 03 2024, Peter Hutterer wrote: > > On Tue, Sep 03, 2024 at 01:14:34AM +0900, Benjamin Tissoires wrote: > > > The use case is when we fix a device through HID-BPF, 99% of the cases > > > we want the device to use hid-generic now instead of a dedicated device. > > > > s/dedicated device/dedicated driver/ in the commit message > > > > > That's because the dedicated device might also want to change the report > > > descriptor, or will be handling the device in a different way the new > > > fixed device is using. > > > > > > In hid-core, after matching for the device (so that we only call this new > > > hook on compatible drivers), we call for `.hid_bpf_driver_probe`. > > > The function can not communicate with the device because it is not yet > > > started, but it can make educated guesses and decide to: > > > - let hid-core decide by itself > > > - force the use of this driver (by comparing the provided name with > > > "hid-generic" for instance) > > > - force hid-core to ignore this driver for this device. > > > > > > For API stability, we don't rely on a bitfield or a return value for > > > chosing hid-core behavior. We simply have a couple of writeable fields > > > in the new struct hid_bpf_driver, and then hid-core can make its educated > > > decision. > > > > > > Signed-off-by: Benjamin Tissoires <bentiss@kernel.org> > > > --- > > > Documentation/hid/hid-bpf.rst | 2 +- > > > drivers/hid/bpf/hid_bpf_dispatch.c | 31 ++++++++++++++++++++++++++++ > > > drivers/hid/bpf/hid_bpf_struct_ops.c | 3 +++ > > > drivers/hid/hid-core.c | 6 ++++++ > > > include/linux/hid_bpf.h | 40 ++++++++++++++++++++++++++++++++++++ > > > 5 files changed, 81 insertions(+), 1 deletion(-) > > > > > > diff --git a/Documentation/hid/hid-bpf.rst b/Documentation/hid/hid-bpf.rst > > > index 5939eeafb361..05a43f11cdab 100644 > > > --- a/Documentation/hid/hid-bpf.rst > > > +++ b/Documentation/hid/hid-bpf.rst > > > @@ -190,7 +190,7 @@ User API data structures available in programs: > > > ----------------------------------------------- > > > > > > .. kernel-doc:: include/linux/hid_bpf.h > > > - :identifiers: hid_bpf_ctx > > > + :identifiers: hid_bpf_ctx hid_bpf_driver > > > > > > Available API that can be used in all HID-BPF struct_ops programs: > > > ------------------------------------------------------------------ > > > diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c > > > index a272a086c950..2df136d64152 100644 > > > --- a/drivers/hid/bpf/hid_bpf_dispatch.c > > > +++ b/drivers/hid/bpf/hid_bpf_dispatch.c > > > @@ -189,6 +189,37 @@ u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *s > > > } > > > EXPORT_SYMBOL_GPL(call_hid_bpf_rdesc_fixup); > > > > > > +int call_hid_bpf_driver_probe(struct hid_device *hdev, struct hid_driver *hdrv, > > > + const struct hid_device_id *id) > > > +{ > > > + struct hid_bpf_driver drv = { 0 }; > > > + struct hid_bpf_ops *e; > > > + int idx; > > + > > > + if (strscpy(drv.name, hdrv->name, sizeof(drv.name)) < 0) > > > + return 0; > > > + > > > + idx = srcu_read_lock(&hdev->bpf.srcu); > > > + list_for_each_entry_srcu(e, &hdev->bpf.prog_list, list, > > > + srcu_read_lock_held(&hdev->bpf.srcu)) { > > > + if (!e->hid_driver_probe) > > > + continue; > > > + > > > + e->hid_driver_probe(hdev, &drv, id); > > > + } > > > + > > > + srcu_read_unlock(&hdev->bpf.srcu, idx); > > > + > > > + if (drv.force_driver) > > > + return 1; > > > + > > > + if (drv.ignore_driver) > > > + return -1; > > > + > > > + return 0; > > > +} > > > +EXPORT_SYMBOL_GPL(call_hid_bpf_driver_probe); > > > + > > > static int device_match_id(struct device *dev, const void *id) > > > { > > > struct hid_device *hdev = to_hid_device(dev); > > > diff --git a/drivers/hid/bpf/hid_bpf_struct_ops.c b/drivers/hid/bpf/hid_bpf_struct_ops.c > > > index cd696c59ba0f..1e13a22f73a1 100644 > > > --- a/drivers/hid/bpf/hid_bpf_struct_ops.c > > > +++ b/drivers/hid/bpf/hid_bpf_struct_ops.c > > > @@ -46,6 +46,7 @@ static int hid_bpf_ops_check_member(const struct btf_type *t, > > > case offsetof(struct hid_bpf_ops, hid_rdesc_fixup): > > > case offsetof(struct hid_bpf_ops, hid_hw_request): > > > case offsetof(struct hid_bpf_ops, hid_hw_output_report): > > > + case offsetof(struct hid_bpf_ops, hid_driver_probe): > > > break; > > > default: > > > if (prog->sleepable) > > > @@ -79,6 +80,8 @@ static int hid_bpf_ops_btf_struct_access(struct bpf_verifier_log *log, > > > WRITE_RANGE(hid_device, name, true), > > > WRITE_RANGE(hid_device, uniq, true), > > > WRITE_RANGE(hid_device, phys, true), > > > + WRITE_RANGE(hid_bpf_driver, force_driver, false), > > > + WRITE_RANGE(hid_bpf_driver, ignore_driver, false), > > > }; > > > #undef WRITE_RANGE > > > const struct btf_type *state = NULL; > > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > > > index 988d0acbdf04..7845f0a789ec 100644 > > > --- a/drivers/hid/hid-core.c > > > +++ b/drivers/hid/hid-core.c > > > @@ -2639,10 +2639,16 @@ static bool hid_check_device_match(struct hid_device *hdev, > > > struct hid_driver *hdrv, > > > const struct hid_device_id **id) > > > { > > > + int ret; > > > + > > > *id = hid_match_device(hdev, hdrv); > > > if (!*id) > > > return false; > > > > > > + ret = call_hid_bpf_driver_probe(hdev, hdrv, *id); > > > + if (ret) > > > + return ret > 0; > > > + > > > if (hdrv->match) > > > return hdrv->match(hdev, hid_ignore_special_drivers); > > > > > > diff --git a/include/linux/hid_bpf.h b/include/linux/hid_bpf.h > > > index d4d063cf63b5..20693c218857 100644 > > > --- a/include/linux/hid_bpf.h > > > +++ b/include/linux/hid_bpf.h > > > @@ -9,6 +9,7 @@ > > > #include <uapi/linux/hid.h> > > > > > > struct hid_device; > > > +struct hid_driver; > > > > > > /* > > > * The following is the user facing HID BPF API. > > > @@ -80,6 +81,22 @@ struct hid_ops { > > > > > > extern struct hid_ops *hid_ops; > > > > > > +/** > > > + * struct hid_bpf_driver - User accessible data for the ``hid_bpf_probe`` > > > + * struct_ops > > > + * > > > + * @name: the name of the driver currently being treated > > > + * @force_driver: set this to ``true`` to force hid-core to use this driver, > > > + * bypassing any further decision made by this driver > > > + * @ignore_driver: set this to ``true`` to force hid-core to ignore this driver, > > > + * bypassing any further decision made by this driver > > > > If I set both to false or true, what happens? The two seem to be > > force_driver has priority over ignore_driver. > > > mutually exclusive, in userspace I'd use an enum here to have a > > NOOP/FORCE_DRIVER/IGNORE_DRIVER value range (that can be extended later). > > Maybe something like that is an option? > > enum also has the advantage to be exported in vmlinux.h. > > FWIW, the idea behind adding new fields in a struct was to get the > backward compatibility for free. Because the verifier/relocator will see > if we are using the correct field entries. > > OTOH, maybe we can make the function return the afformended enum, and > drop those two fields. > > I think we should probably abort processing of any bpf sets the return > value to anything else than NOOP. > > I'll work a little bit more on that. > > Cheers, > Benjamin > > > > > > + */ > > > +struct hid_bpf_driver { > > > + __u8 name[64]; > > > + bool force_driver; > > > + bool ignore_driver; > > > +}; > > > + > > > /** > > > * struct hid_bpf_ops - A BPF struct_ops of callbacks allowing to attach HID-BPF > > > * programs to a HID device > > > @@ -178,6 +195,25 @@ struct hid_bpf_ops { > > > */ > > > int (*hid_hw_output_report)(struct hid_bpf_ctx *ctx, u64 source); > > > > > > + /** > > > + * @hid_driver_probe: called before the kernel ``.probe()`` function > > > + * > > > + * It has the following arguments: > > > + * > > > + * ``hdev``: The HID device kernel representation > > > + * > > > + * ``hdrv``: A BPF partially writeable representation of a HID driver > > > + * > > > + * ``id``: The device match structure found in the driver > > > + * > > > + * Note that the device has not been started yet, and thus kfuncs like > > > + * ``hid_hw_output_report`` will likely fail. > > > > Just to confirm, I can access the device's report descriptor though? For I forgot to reply to this comment: no, you don't :) > > the devices that we're looking at (e.g. the foot pedals pretending to be > > an apple keyboard) the driver name and what we can set in HID_BPF_CONFIG > > are not going to be enough, we'll have to check the rdesc too. You can check this in the probe syscall before unbinding/rebinding the device. The device is uniquely linked to the bpf program you loaded, so in theory this is sufficient. Cheers, Benjamin > > > > Cheers, > > Peter > > > > > + * > > > + * This function is useful to force/ignore a given supported HID driver, > > > + * by writing ``true`` in ``hdrv->force_driver`` or ``hdrv->ignore_driver`` > > > + */ > > > + void (*hid_driver_probe)(struct hid_device *hdev, struct hid_bpf_driver *hdrv, > > > + const struct hid_device_id *id); > > > > > > /* private: do not show up in the docs */ > > > struct hid_device *hdev; > > > @@ -213,6 +249,8 @@ void hid_bpf_disconnect_device(struct hid_device *hdev); > > > void hid_bpf_destroy_device(struct hid_device *hid); > > > int hid_bpf_device_init(struct hid_device *hid); > > > u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *size); > > > +int call_hid_bpf_driver_probe(struct hid_device *hdev, struct hid_driver *hdrv, > > > + const struct hid_device_id *id); > > > #else /* CONFIG_HID_BPF */ > > > static inline u8 *dispatch_hid_bpf_device_event(struct hid_device *hid, enum hid_report_type type, > > > u8 *data, u32 *size, int interrupt, > > > @@ -228,6 +266,8 @@ static inline int hid_bpf_connect_device(struct hid_device *hdev) { return 0; } > > > static inline void hid_bpf_disconnect_device(struct hid_device *hdev) {} > > > static inline void hid_bpf_destroy_device(struct hid_device *hid) {} > > > static inline int hid_bpf_device_init(struct hid_device *hid) { return 0; } > > > +static inline int call_hid_bpf_driver_probe(struct hid_device *hdev, struct hid_driver *hdrv, > > > + const struct hid_device_id *id) { return 0; } > > > /* > > > * This specialized allocator has to be a macro for its allocations to be > > > * accounted separately (to have a separate alloc_tag). The typecast is > > > > > > -- > > > 2.46.0 > > >
diff --git a/Documentation/hid/hid-bpf.rst b/Documentation/hid/hid-bpf.rst index 5939eeafb361..05a43f11cdab 100644 --- a/Documentation/hid/hid-bpf.rst +++ b/Documentation/hid/hid-bpf.rst @@ -190,7 +190,7 @@ User API data structures available in programs: ----------------------------------------------- .. kernel-doc:: include/linux/hid_bpf.h - :identifiers: hid_bpf_ctx + :identifiers: hid_bpf_ctx hid_bpf_driver Available API that can be used in all HID-BPF struct_ops programs: ------------------------------------------------------------------ diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c index a272a086c950..2df136d64152 100644 --- a/drivers/hid/bpf/hid_bpf_dispatch.c +++ b/drivers/hid/bpf/hid_bpf_dispatch.c @@ -189,6 +189,37 @@ u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *s } EXPORT_SYMBOL_GPL(call_hid_bpf_rdesc_fixup); +int call_hid_bpf_driver_probe(struct hid_device *hdev, struct hid_driver *hdrv, + const struct hid_device_id *id) +{ + struct hid_bpf_driver drv = { 0 }; + struct hid_bpf_ops *e; + int idx; + + if (strscpy(drv.name, hdrv->name, sizeof(drv.name)) < 0) + return 0; + + idx = srcu_read_lock(&hdev->bpf.srcu); + list_for_each_entry_srcu(e, &hdev->bpf.prog_list, list, + srcu_read_lock_held(&hdev->bpf.srcu)) { + if (!e->hid_driver_probe) + continue; + + e->hid_driver_probe(hdev, &drv, id); + } + + srcu_read_unlock(&hdev->bpf.srcu, idx); + + if (drv.force_driver) + return 1; + + if (drv.ignore_driver) + return -1; + + return 0; +} +EXPORT_SYMBOL_GPL(call_hid_bpf_driver_probe); + static int device_match_id(struct device *dev, const void *id) { struct hid_device *hdev = to_hid_device(dev); diff --git a/drivers/hid/bpf/hid_bpf_struct_ops.c b/drivers/hid/bpf/hid_bpf_struct_ops.c index cd696c59ba0f..1e13a22f73a1 100644 --- a/drivers/hid/bpf/hid_bpf_struct_ops.c +++ b/drivers/hid/bpf/hid_bpf_struct_ops.c @@ -46,6 +46,7 @@ static int hid_bpf_ops_check_member(const struct btf_type *t, case offsetof(struct hid_bpf_ops, hid_rdesc_fixup): case offsetof(struct hid_bpf_ops, hid_hw_request): case offsetof(struct hid_bpf_ops, hid_hw_output_report): + case offsetof(struct hid_bpf_ops, hid_driver_probe): break; default: if (prog->sleepable) @@ -79,6 +80,8 @@ static int hid_bpf_ops_btf_struct_access(struct bpf_verifier_log *log, WRITE_RANGE(hid_device, name, true), WRITE_RANGE(hid_device, uniq, true), WRITE_RANGE(hid_device, phys, true), + WRITE_RANGE(hid_bpf_driver, force_driver, false), + WRITE_RANGE(hid_bpf_driver, ignore_driver, false), }; #undef WRITE_RANGE const struct btf_type *state = NULL; diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 988d0acbdf04..7845f0a789ec 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -2639,10 +2639,16 @@ static bool hid_check_device_match(struct hid_device *hdev, struct hid_driver *hdrv, const struct hid_device_id **id) { + int ret; + *id = hid_match_device(hdev, hdrv); if (!*id) return false; + ret = call_hid_bpf_driver_probe(hdev, hdrv, *id); + if (ret) + return ret > 0; + if (hdrv->match) return hdrv->match(hdev, hid_ignore_special_drivers); diff --git a/include/linux/hid_bpf.h b/include/linux/hid_bpf.h index d4d063cf63b5..20693c218857 100644 --- a/include/linux/hid_bpf.h +++ b/include/linux/hid_bpf.h @@ -9,6 +9,7 @@ #include <uapi/linux/hid.h> struct hid_device; +struct hid_driver; /* * The following is the user facing HID BPF API. @@ -80,6 +81,22 @@ struct hid_ops { extern struct hid_ops *hid_ops; +/** + * struct hid_bpf_driver - User accessible data for the ``hid_bpf_probe`` + * struct_ops + * + * @name: the name of the driver currently being treated + * @force_driver: set this to ``true`` to force hid-core to use this driver, + * bypassing any further decision made by this driver + * @ignore_driver: set this to ``true`` to force hid-core to ignore this driver, + * bypassing any further decision made by this driver + */ +struct hid_bpf_driver { + __u8 name[64]; + bool force_driver; + bool ignore_driver; +}; + /** * struct hid_bpf_ops - A BPF struct_ops of callbacks allowing to attach HID-BPF * programs to a HID device @@ -178,6 +195,25 @@ struct hid_bpf_ops { */ int (*hid_hw_output_report)(struct hid_bpf_ctx *ctx, u64 source); + /** + * @hid_driver_probe: called before the kernel ``.probe()`` function + * + * It has the following arguments: + * + * ``hdev``: The HID device kernel representation + * + * ``hdrv``: A BPF partially writeable representation of a HID driver + * + * ``id``: The device match structure found in the driver + * + * Note that the device has not been started yet, and thus kfuncs like + * ``hid_hw_output_report`` will likely fail. + * + * This function is useful to force/ignore a given supported HID driver, + * by writing ``true`` in ``hdrv->force_driver`` or ``hdrv->ignore_driver`` + */ + void (*hid_driver_probe)(struct hid_device *hdev, struct hid_bpf_driver *hdrv, + const struct hid_device_id *id); /* private: do not show up in the docs */ struct hid_device *hdev; @@ -213,6 +249,8 @@ void hid_bpf_disconnect_device(struct hid_device *hdev); void hid_bpf_destroy_device(struct hid_device *hid); int hid_bpf_device_init(struct hid_device *hid); u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *size); +int call_hid_bpf_driver_probe(struct hid_device *hdev, struct hid_driver *hdrv, + const struct hid_device_id *id); #else /* CONFIG_HID_BPF */ static inline u8 *dispatch_hid_bpf_device_event(struct hid_device *hid, enum hid_report_type type, u8 *data, u32 *size, int interrupt, @@ -228,6 +266,8 @@ static inline int hid_bpf_connect_device(struct hid_device *hdev) { return 0; } static inline void hid_bpf_disconnect_device(struct hid_device *hdev) {} static inline void hid_bpf_destroy_device(struct hid_device *hid) {} static inline int hid_bpf_device_init(struct hid_device *hid) { return 0; } +static inline int call_hid_bpf_driver_probe(struct hid_device *hdev, struct hid_driver *hdrv, + const struct hid_device_id *id) { return 0; } /* * This specialized allocator has to be a macro for its allocations to be * accounted separately (to have a separate alloc_tag). The typecast is
The use case is when we fix a device through HID-BPF, 99% of the cases we want the device to use hid-generic now instead of a dedicated device. That's because the dedicated device might also want to change the report descriptor, or will be handling the device in a different way the new fixed device is using. In hid-core, after matching for the device (so that we only call this new hook on compatible drivers), we call for `.hid_bpf_driver_probe`. The function can not communicate with the device because it is not yet started, but it can make educated guesses and decide to: - let hid-core decide by itself - force the use of this driver (by comparing the provided name with "hid-generic" for instance) - force hid-core to ignore this driver for this device. For API stability, we don't rely on a bitfield or a return value for chosing hid-core behavior. We simply have a couple of writeable fields in the new struct hid_bpf_driver, and then hid-core can make its educated decision. Signed-off-by: Benjamin Tissoires <bentiss@kernel.org> --- Documentation/hid/hid-bpf.rst | 2 +- drivers/hid/bpf/hid_bpf_dispatch.c | 31 ++++++++++++++++++++++++++++ drivers/hid/bpf/hid_bpf_struct_ops.c | 3 +++ drivers/hid/hid-core.c | 6 ++++++ include/linux/hid_bpf.h | 40 ++++++++++++++++++++++++++++++++++++ 5 files changed, 81 insertions(+), 1 deletion(-)