Hi all,
sending this as an RFC because it's not complete, but I'd like to start
the discussion:
While presenting HID-BPF, I always mentioned that device fixes should be
integrated in the kernel. And I am trying to do that in this series.
Load a generic bpf from the kernel:
===================================
The first step is to be able to load that bpf program without knowing
its interface. So I studied the output of the light skeletons and
squized them into a simple C array. Then I wrote a BPF loader based on
that same skeleton, and now I can iterate over an array of BPF programs
and load the ones that match the device.
The step 0 in that translation was to generate a json instead of a
proper C header for the light skeleton. The idea is that I can then
transform that json into whatever I want, without having to mess up with
bpftool. IIRC this was briefly discussed at plumbers, so I hope this is
not too weird.
Pin the program to the bpffs:
=============================
AFAICT, the infrastructure is not completely ready to pin programs from
the kernel itself (outside of bpf_preload).
I encountered a few hiccups and I'd like to know if I am on the correct
path:
- to be able to pin to the bpffs, it first needs to be mounted by
userspace. Should I add some sort of list of already available
programs that would be picked up by the kernel when bpffs is mounted?
- I am not sure the way I added the pinned program is correct: I am
reusing the skeleton of bpf_iter_link_pin_kernel(), but using
kernel_path_create() in the same way bpf_obj_pin_user() does seems
better, though I always get -ENOENT even with bpffs mounted.
- I also need to be able to add a hierarchy of directories in bpffs from
the kernel, and this requires some more code digging... :)
Produce the actual "compiled" bpf program:
==========================================
The current code here relies on the user to run `make` in the
drivers/hid/bpf/progs directory to regenerate the files.
Leaving aside the fact that I need to check on how to make this step
integrated by the generic root-level make, I wonder if using python to
generate that file is OK.
I am not very happy to add a requirement to build the whole kernel, but
OTOH, writing the same tool in C is desperately annoying. I would rather
have a tool written in rust TBH, if rust is now part of the required
toolchain.
Ship the "compiled" bpf programs:
=================================
In this version, the bpf program is embedded in vmlinux, for no other
reasons that splitting that out in a module would require some more
effort before submitting that RFC (and subject to concurrency races when
a device has several interfaces at once).
However, I wonder what should be the final "product":
- In a first pass, I can keep the current form and have a dedicated
kernel module that contains all of the bpf fixes. The kernel would
load it, check for any match, pin the programs, and unload this kernel
module.
This works but isn't very modular as we just enable/ship all of the
fixes or nothing.
- another idea I had, was to rely on the firmware kernel interface. Now
that I have a simple "bpf module" format (or even the json file), I
could "compile" it into a binary and then have the kernel request the
firmware on a device plug. This way we don't load/unload a module at
each plug, and we rely on the existing firmware capabilities.
I really like this idea, but then I wonder how I can ship those
firmwares. I'd like them to be tied to the currently running kernel,
so should I namespace them in the firmware directory on install? Is
there any other way to be able to have 2 or more firmwares depending
on the kernel version?
I think that's it. Again, this series is just a PoC on top of
hid.git/for-6.2/hid-bpf, and I can change everything if I am not headed
to the correct direction.
Cheers,
Benjamin
Benjamin Tissoires (10):
bpftool: generate json output of skeletons
WIP: bpf: allow to pin programs from the kernel when bpffs is mounted
HID: add a tool to convert a bpf source into a generic bpf loader
HID: add the bpf loader that can attach a generic hid-bpf program
HID: add report descriptor override for the X-Keys XK24
selftests: hid: add vmtest.sh
selftests: hid: Add a variant parameter so we can emulate specific
devices
selftests: hid: add XK-24 tests
selftests: hid: ensure the program is correctly pinned
wip: vmtest aarch64
MAINTAINERS | 1 +
drivers/hid/bpf/Makefile | 3 +-
drivers/hid/bpf/hid_bpf_dispatch.c | 3 +-
drivers/hid/bpf/hid_bpf_loader.c | 243 +++++++++++++++
drivers/hid/bpf/progs/Makefile | 105 +++++++
.../bpf/progs/b0003g0001v05F3p0405-xk24.bpf.c | 106 +++++++
.../progs/b0003g0001v05F3p0405-xk24.hidbpf.h | 292 ++++++++++++++++++
drivers/hid/bpf/progs/hid_bpf.h | 15 +
drivers/hid/bpf/progs/hid_bpf_helpers.h | 22 ++
drivers/hid/bpf/progs/hid_bpf_progs.h | 50 +++
drivers/hid/hid-core.c | 2 +
include/linux/bpf.h | 1 +
include/linux/hid_bpf.h | 2 +
kernel/bpf/inode.c | 41 ++-
tools/bpf/bpftool/gen.c | 95 ++++++
tools/hid/build_progs_list.py | 231 ++++++++++++++
tools/testing/selftests/hid/.gitignore | 1 +
tools/testing/selftests/hid/config.aarch64 | 39 +++
tools/testing/selftests/hid/config.common | 241 +++++++++++++++
tools/testing/selftests/hid/config.x86_64 | 4 +
tools/testing/selftests/hid/hid_bpf.c | 150 +++++++--
tools/testing/selftests/hid/vmtest.sh | 286 +++++++++++++++++
22 files changed, 1907 insertions(+), 26 deletions(-)
create mode 100644 drivers/hid/bpf/hid_bpf_loader.c
create mode 100644 drivers/hid/bpf/progs/Makefile
create mode 100644 drivers/hid/bpf/progs/b0003g0001v05F3p0405-xk24.bpf.c
create mode 100644 drivers/hid/bpf/progs/b0003g0001v05F3p0405-xk24.hidbpf.h
create mode 100644 drivers/hid/bpf/progs/hid_bpf.h
create mode 100644 drivers/hid/bpf/progs/hid_bpf_helpers.h
create mode 100644 drivers/hid/bpf/progs/hid_bpf_progs.h
create mode 100755 tools/hid/build_progs_list.py
create mode 100644 tools/testing/selftests/hid/config.aarch64
create mode 100644 tools/testing/selftests/hid/config.common
create mode 100644 tools/testing/selftests/hid/config.x86_64
create mode 100755 tools/testing/selftests/hid/vmtest.sh
--
2.38.1
When testing with in-kernel bpf programs, we need to emulate a specific
device, because otherwise we won't be loading the matching bpf program.
Add a variant parameter to the fixture hid_bpf so that we can re-use it
later with different devices.
Signed-off-by: Benjamin Tissoires <[email protected]>
---
tools/testing/selftests/hid/hid_bpf.c | 75 +++++++++++++++++++--------
1 file changed, 53 insertions(+), 22 deletions(-)
diff --git a/tools/testing/selftests/hid/hid_bpf.c b/tools/testing/selftests/hid/hid_bpf.c
index 6615c26fb5dd..738ddb2c6a62 100644
--- a/tools/testing/selftests/hid/hid_bpf.c
+++ b/tools/testing/selftests/hid/hid_bpf.c
@@ -114,6 +114,15 @@ static pthread_cond_t uhid_started = PTHREAD_COND_INITIALIZER;
/* no need to protect uhid_stopped, only one thread accesses it */
static bool uhid_stopped;
+struct hid_device_id {
+ int bus;
+ int vendor;
+ int product;
+ int version;
+ unsigned char *rdesc;
+ const size_t rdesc_sz;
+};
+
static int uhid_write(struct __test_metadata *_metadata, int fd, const struct uhid_event *ev)
{
ssize_t ret;
@@ -131,7 +140,8 @@ static int uhid_write(struct __test_metadata *_metadata, int fd, const struct uh
}
}
-static int uhid_create(struct __test_metadata *_metadata, int fd, int rand_nb)
+static int uhid_create(struct __test_metadata *_metadata, const struct hid_device_id *variant,
+ int fd, int rand_nb)
{
struct uhid_event ev;
char buf[25];
@@ -141,12 +151,12 @@ static int uhid_create(struct __test_metadata *_metadata, int fd, int rand_nb)
memset(&ev, 0, sizeof(ev));
ev.type = UHID_CREATE;
strcpy((char *)ev.u.create.name, buf);
- ev.u.create.rd_data = rdesc;
- ev.u.create.rd_size = sizeof(rdesc);
- ev.u.create.bus = BUS_USB;
- ev.u.create.vendor = 0x0001;
- ev.u.create.product = 0x0a37;
- ev.u.create.version = 0;
+ ev.u.create.rd_data = variant->rdesc;
+ ev.u.create.rd_size = variant->rdesc_sz;
+ ev.u.create.bus = variant->bus;
+ ev.u.create.vendor = variant->vendor;
+ ev.u.create.product = variant->product;
+ ev.u.create.version = variant->version;
ev.u.create.country = 0;
sprintf(buf, "%d", rand_nb);
@@ -299,7 +309,8 @@ static int uhid_send_event(struct __test_metadata *_metadata, int fd, __u8 *buf,
return uhid_write(_metadata, fd, &ev);
}
-static int setup_uhid(struct __test_metadata *_metadata, int rand_nb)
+static int setup_uhid(struct __test_metadata *_metadata, const struct hid_device_id *variant,
+ int rand_nb)
{
int fd;
const char *path = "/dev/uhid";
@@ -308,7 +319,7 @@ static int setup_uhid(struct __test_metadata *_metadata, int rand_nb)
fd = open(path, O_RDWR | O_CLOEXEC);
ASSERT_GE(fd, 0) TH_LOG("open uhid-cdev failed; %d", fd);
- ret = uhid_create(_metadata, fd, rand_nb);
+ ret = uhid_create(_metadata, variant, fd, rand_nb);
ASSERT_EQ(0, ret) {
TH_LOG("create uhid device failed: %d", ret);
close(fd);
@@ -317,15 +328,19 @@ static int setup_uhid(struct __test_metadata *_metadata, int rand_nb)
return fd;
}
-static bool match_sysfs_device(int dev_id, const char *workdir, struct dirent *dir)
+static bool match_sysfs_device(int dev_id, const struct hid_device_id *variant,
+ const char *workdir, struct dirent *dir)
{
- const char *target = "0003:0001:0A37.*";
+ char target[17] = "0003:0001:0A37.*";
char phys[512];
char uevent[1024];
char temp[512];
int fd, nread;
bool found = false;
+ snprintf(target, sizeof(target), "0003:%04X:%04X.*",
+ variant->vendor, variant->product);
+
if (fnmatch(target, dir->d_name, 0))
return false;
@@ -347,7 +362,7 @@ static bool match_sysfs_device(int dev_id, const char *workdir, struct dirent *d
return found;
}
-static int get_hid_id(int dev_id)
+static int get_hid_id(int dev_id, const struct hid_device_id *variant)
{
const char *workdir = "/sys/devices/virtual/misc/uhid";
const char *str_id;
@@ -362,7 +377,7 @@ static int get_hid_id(int dev_id)
d = opendir(workdir);
if (d) {
while ((dir = readdir(d)) != NULL) {
- if (!match_sysfs_device(dev_id, workdir, dir))
+ if (!match_sysfs_device(dev_id, variant, workdir, dir))
continue;
str_id = dir->d_name + sizeof("0003:0001:0A37.");
@@ -379,7 +394,7 @@ static int get_hid_id(int dev_id)
return found;
}
-static int get_hidraw(int dev_id)
+static int get_hidraw(int dev_id, const struct hid_device_id *variant)
{
const char *workdir = "/sys/devices/virtual/misc/uhid";
char sysfs[1024];
@@ -396,7 +411,7 @@ static int get_hidraw(int dev_id)
continue;
while ((dir = readdir(d)) != NULL) {
- if (!match_sysfs_device(dev_id, workdir, dir))
+ if (!match_sysfs_device(dev_id, variant, workdir, dir))
continue;
sprintf(sysfs, "%s/%s/hidraw", workdir, dir->d_name);
@@ -423,12 +438,12 @@ static int get_hidraw(int dev_id)
return found;
}
-static int open_hidraw(int dev_id)
+static int open_hidraw(int dev_id, const struct hid_device_id *variant)
{
int hidraw_number;
char hidraw_path[64] = { 0 };
- hidraw_number = get_hidraw(dev_id);
+ hidraw_number = get_hidraw(dev_id, variant);
if (hidraw_number < 0)
return hidraw_number;
@@ -445,6 +460,22 @@ FIXTURE(hid_bpf) {
pthread_t tid;
struct hid *skel;
};
+
+FIXTURE_VARIANT(hid_bpf) {
+ const struct hid_device_id id;
+};
+
+FIXTURE_VARIANT_ADD(hid_bpf, generic_device) {
+ .id = {
+ .bus = BUS_USB,
+ .vendor = 0x0001,
+ .product = 0x0a37,
+ .version = 0,
+ .rdesc = rdesc,
+ .rdesc_sz = sizeof(rdesc),
+ },
+};
+
static void detach_bpf(FIXTURE_DATA(hid_bpf) * self)
{
if (self->hidraw_fd)
@@ -478,10 +509,10 @@ FIXTURE_SETUP(hid_bpf)
self->dev_id = rand() % 1024;
- self->uhid_fd = setup_uhid(_metadata, self->dev_id);
+ self->uhid_fd = setup_uhid(_metadata, &variant->id, self->dev_id);
- /* locate the uev, self, variant);ent file of the created device */
- self->hid_id = get_hid_id(self->dev_id);
+ /* locate the file of the created device */
+ self->hid_id = get_hid_id(self->dev_id, &variant->id);
ASSERT_GT(self->hid_id, 0)
TEARDOWN_LOG("Could not locate uhid device id: %d", self->hid_id);
@@ -546,7 +577,7 @@ static void load_programs(const struct test_program programs[],
ASSERT_OK(args.retval) TH_LOG("attach_hid(%s): %d", programs[i].name, args.retval);
}
- self->hidraw_fd = open_hidraw(self->dev_id);
+ self->hidraw_fd = open_hidraw(self->dev_id, &variant->id);
ASSERT_GE(self->hidraw_fd, 0) TH_LOG("open_hidraw");
}
@@ -644,7 +675,7 @@ TEST_F(hid_bpf, test_attach_detach)
/* detach the program */
detach_bpf(self);
- self->hidraw_fd = open_hidraw(self->dev_id);
+ self->hidraw_fd = open_hidraw(self->dev_id, &variant->id);
ASSERT_GE(self->hidraw_fd, 0) TH_LOG("open_hidraw");
/* inject another event */
--
2.38.1
Turns out that if bpffs was not mounted, the test was silently passing.
So ensure it passes, but also force the mount of the bpffs in vmtest.sh
so we get passing results.
Signed-off-by: Benjamin Tissoires <[email protected]>
---
tools/testing/selftests/hid/hid_bpf.c | 3 ++-
tools/testing/selftests/hid/vmtest.sh | 4 +++-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/hid/hid_bpf.c b/tools/testing/selftests/hid/hid_bpf.c
index 4bdd1cfa7d13..3ae544bf24fe 100644
--- a/tools/testing/selftests/hid/hid_bpf.c
+++ b/tools/testing/selftests/hid/hid_bpf.c
@@ -712,7 +712,8 @@ TEST_F(hid_bpf, test_attach_detach)
/* pin the program and immediately unpin it */
#define PIN_PATH "/sys/fs/bpf/hid_first_event"
- bpf_program__pin(self->skel->progs.hid_first_event, PIN_PATH);
+ err = bpf_program__pin(self->skel->progs.hid_first_event, PIN_PATH);
+ ASSERT_OK(err) TH_LOG("error while calling bpf_program__pin");
remove(PIN_PATH);
#undef PIN_PATH
usleep(100000);
diff --git a/tools/testing/selftests/hid/vmtest.sh b/tools/testing/selftests/hid/vmtest.sh
index f124cf6b0d0f..bd60f65acb72 100755
--- a/tools/testing/selftests/hid/vmtest.sh
+++ b/tools/testing/selftests/hid/vmtest.sh
@@ -108,8 +108,10 @@ EOF
if [[ "${debug_shell}" != "yes" ]]
then
touch ${OUTPUT_DIR}/${LOG_FILE}
- command="set -o pipefail ; ${command} 2>&1 | tee ${OUTPUT_DIR}/${LOG_FILE}"
+ command="mount bpffs -t bpf /sys/fs/bpf/; set -o pipefail ; ${command} 2>&1 | tee ${OUTPUT_DIR}/${LOG_FILE}"
post_command="cat ${OUTPUT_DIR}/${LOG_FILE}"
+ else
+ command="mount bpffs -t bpf /sys/fs/bpf/; ${command}"
fi
set +e
--
2.38.1
this is the first device to be added in the kernel with a bpf program
associated, so we better use it to ensure we get the things right.
We define another fixture that will reuse everything from hid_bpf except
for the variant parameters. And then we can use that easily in a new
dedicated test.
Signed-off-by: Benjamin Tissoires <[email protected]>
---
tools/testing/selftests/hid/hid_bpf.c | 72 +++++++++++++++++++++++++++
1 file changed, 72 insertions(+)
diff --git a/tools/testing/selftests/hid/hid_bpf.c b/tools/testing/selftests/hid/hid_bpf.c
index 738ddb2c6a62..4bdd1cfa7d13 100644
--- a/tools/testing/selftests/hid/hid_bpf.c
+++ b/tools/testing/selftests/hid/hid_bpf.c
@@ -520,6 +520,51 @@ FIXTURE_SETUP(hid_bpf)
ASSERT_EQ(0, err) TEARDOWN_LOG("could not start udev listener: %d", err);
}
+static unsigned char xk24_rdesc[] = {
+ 0x05, 0x0c, // Usage Page (Consumer Devices) 0
+ 0x09, 0x01, // Usage (Consumer Control) 2
+ 0xa1, 0x01, // Collection (Application) 4
+ 0xa1, 0x02, // Collection (Logical) 6
+ 0x05, 0x08, // Usage Page (LEDs) 8
+ 0x09, 0x4b, // Usage (Generic Indicator) 10
+ 0x75, 0x08, // Report Size (8) 12
+ 0x95, 0x23, // Report Count (35) 14
+ 0x91, 0x02, // Output (Data,Var,Abs) 16
+ 0xc0, // End Collection 18
+ 0xa1, 0x02, // Collection (Logical) 19
+ 0x05, 0x09, // Usage Page (Button) 21
+ 0x09, 0x4b, // Usage (Vendor Usage 0x4b) 23
+ 0x75, 0x08, // Report Size (8) 25
+ 0x95, 0x20, // Report Count (32) 27
+ 0x81, 0x02, // Input (Data,Var,Abs) 29
+ 0xc0, // End Collection 31
+ 0xc0, // End Collection 32
+};
+
+#define _test_data_hid_bpf_xkeys _test_data_hid_bpf
+#define _fixture_variant_hid_bpf_xkeys _fixture_variant_hid_bpf
+FIXTURE(hid_bpf_xkeys);
+FIXTURE_VARIANT(hid_bpf_xkeys);
+FIXTURE_VARIANT_ADD(hid_bpf_xkeys, xk24) {
+ .id = {
+ .bus = BUS_USB,
+ .vendor = 0x05F3,
+ .product = 0x0405,
+ .version = 0,
+ .rdesc = xk24_rdesc,
+ .rdesc_sz = sizeof(xk24_rdesc),
+ },
+};
+
+FIXTURE_SETUP(hid_bpf_xkeys)
+{
+ hid_bpf_setup(_metadata, self, variant);
+}
+FIXTURE_TEARDOWN(hid_bpf_xkeys)
+{
+ hid_bpf_teardown(_metadata, self, variant);
+}
+
struct test_program {
const char *name;
int insert_head;
@@ -846,6 +891,33 @@ TEST_F(hid_bpf, test_rdesc_fixup)
ASSERT_EQ(rpt_desc.value[4], 0x42);
}
+/*
+ * Attach an emulated XK24, which has an in-tree eBPF program, and ensure
+ * we got it loaded.
+ */
+TEST_F(hid_bpf_xkeys, test_xk24)
+{
+ struct hidraw_report_descriptor rpt_desc = {0};
+ int err, desc_size;
+
+ /* open the kernel provided hidraw node */
+ self->hidraw_fd = open_hidraw(self->dev_id, &variant->id);
+ ASSERT_GE(self->hidraw_fd, 0) TH_LOG("open_hidraw");
+
+ /* read the exposed report descriptor from hidraw */
+ err = ioctl(self->hidraw_fd, HIDIOCGRDESCSIZE, &desc_size);
+ ASSERT_GE(err, 0) TH_LOG("error while reading HIDIOCGRDESCSIZE: %d", err);
+
+ /* ensure the new size of the rdesc is bigger than the old one */
+ ASSERT_GT(desc_size, sizeof(xk24_rdesc));
+
+ rpt_desc.size = desc_size;
+ err = ioctl(self->hidraw_fd, HIDIOCGRDESC, &rpt_desc);
+ ASSERT_GE(err, 0) TH_LOG("error while reading HIDIOCGRDESC: %d", err);
+
+ ASSERT_EQ(rpt_desc.value[21], 0x09);
+}
+
static int libbpf_print_fn(enum libbpf_print_level level,
const char *format, va_list args)
{
--
2.38.1
untested
Signed-off-by: Benjamin Tissoires <[email protected]>
---
tools/testing/selftests/hid/config.aarch64 | 39 ++++++++++++++++++++++
tools/testing/selftests/hid/vmtest.sh | 6 +++-
2 files changed, 44 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/hid/config.aarch64
diff --git a/tools/testing/selftests/hid/config.aarch64 b/tools/testing/selftests/hid/config.aarch64
new file mode 100644
index 000000000000..76581bc6395b
--- /dev/null
+++ b/tools/testing/selftests/hid/config.aarch64
@@ -0,0 +1,39 @@
+# EFI
+CONFIG_EFI_PARAMS_FROM_FDT=y
+CONFIG_EFI_GENERIC_STUB=y
+CONFIG_EFI_ARMSTUB_DTB_LOADER=y
+
+# Disable unsupported AARCH64 platforms
+CONFIG_ARCH_ACTIONS=n
+CONFIG_ARCH_SUNXI=n
+CONFIG_ARCH_ALPINE=n
+CONFIG_ARCH_APPLE=n
+CONFIG_ARCH_BERLIN=n
+CONFIG_ARCH_EXYNOS=n
+CONFIG_ARCH_K3=n
+CONFIG_ARCH_LAYERSCAPE=n
+CONFIG_ARCH_LG1K=n
+CONFIG_ARCH_HISI=n
+CONFIG_ARCH_KEEMBAY=n
+CONFIG_ARCH_MEDIATEK=n
+CONFIG_ARCH_MESON=n
+CONFIG_ARCH_MVEBU=n
+CONFIG_ARCH_MXC=n
+CONFIG_ARCH_QCOM=n
+CONFIG_ARCH_RENESAS=n
+CONFIG_ARCH_S32=n
+CONFIG_ARCH_SEATTLE=n
+CONFIG_ARCH_INTEL_SOCFPGA=n
+CONFIG_ARCH_SYNQUACER=n
+CONFIG_ARCH_TEGRA=n
+CONFIG_ARCH_TESLA_FSD=n
+CONFIG_ARCH_SPRD=n
+CONFIG_ARCH_THUNDER=n
+CONFIG_ARCH_THUNDER2=n
+CONFIG_ARCH_UNIPHIER=n
+CONFIG_ARCH_VEXPRESS=n
+CONFIG_ARCH_VISCONTI=n
+CONFIG_ARCH_XGENE=n
+CONFIG_ARCH_ZYNQMP=n
+
+CONFIG_NET_VENDOR_MELLANOX=n
diff --git a/tools/testing/selftests/hid/vmtest.sh b/tools/testing/selftests/hid/vmtest.sh
index bd60f65acb72..d9e4a4c0557a 100755
--- a/tools/testing/selftests/hid/vmtest.sh
+++ b/tools/testing/selftests/hid/vmtest.sh
@@ -4,9 +4,13 @@
set -u
set -e
-# This script currently only works for x86_64
+# This script currently only works for x86_64 and aarch64.
ARCH="$(uname -m)"
case "${ARCH}" in
+aarch64)
+ QEMU_BINARY=qemu-system-aarch64
+ BZIMAGE="arch/aarch64/boot/bzImage"
+ ;;
x86_64)
QEMU_BINARY=qemu-system-x86_64
BZIMAGE="arch/x86/boot/bzImage"
--
2.38.1