* [PATCH 0/2] asus vivobook tablet mode
From: Helge Bahmann @ 2026-07-21 14:28 UTC (permalink / raw)
To: Jiri Kosina, linux-input, Basavaraj Natikar; +Cc: Basavaraj Natikar
In-Reply-To: <ecadaac8-e222-420a-ac5b-4d529fb3317e@amd.com>
Hi Basavaraj,
Here the two patches implementing the split hid/input
driver for the tablet mode support. I mulled over what
exactly to use as report descriptor in the hid driver, and
there is no clean and easy solution -- I hope this is what
you had in mind? I could not figure a way how to do
a stand-alone input driver without doing any hid indirection,
as I simply could not find a way to bind to the physical
dev twice.
Please let me know in comments to the patches how you
would like to proceed.
Thanks!
Helge
^ permalink raw reply
* Re: [PATCH 2/2] amd-sfh-tabletmode: interpret sfh tablet mode hid report
From: sashiko-bot @ 2026-07-21 14:41 UTC (permalink / raw)
To: Helge Bahmann; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <14202195.uLZWGnKmhe@zephyr>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unconditional `field->hidinput` dereference and tablet mode state corruption due to unconstrained `.event` callback.
- [High] The Kconfig symbol `INPUT_AMD_SFH_TABLETMODE` is missing a dependency on `HID`.
--
--- Patch [2]: [PATCH 2/2] amd-sfh-tabletmode: interpret sfh tablet mode hid report ---
commit 981d61514e0f554ad31b2f14f7fb391c059df340
Author: Helge Bahmann <hcb@chaoticmind.net>
amd-sfh-tabletmode: interpret sfh tablet mode hid report
Add a driver that hooks into the tablet mode hid reports
generated by the amd-sfh-hid driver. This enables tablet
mode switching on e.g. Asus Vivobook devices.
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 94a753fcb64fa..5a1dacd6f7b9c 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -1003,4 +1003,15 @@ config INPUT_STPMIC1_ONKEY
> To compile this driver as a module, choose M here: the
> module will be called stpmic1_onkey.
>
> +config INPUT_AMD_SFH_TABLETMODE
> + tristate "AMD sensor fusion hub tablet mode driver"
[Severity: High]
Is it possible this Kconfig entry is missing a depends on HID?
If the HID subsystem is disabled or configured as a module while this driver
is built-in, could this result in linking errors for HID core functions like
hid_register_driver() and hid_parse()?
> + help
> + Say Y to enable support of the tablet mode switch driver for
[ ... ]
> diff --git a/drivers/input/misc/amd_sfh_tabletmode.c b/drivers/input/misc/amd_sfh_tabletmode.c
> new file mode 100644
> index 0000000000000..ee1207372207c
> --- /dev/null
> +++ b/drivers/input/misc/amd_sfh_tabletmode.c
[ ... ]
> +static int amd_sfh_tabletmode_event(struct hid_device *hid, struct hid_field *field,
> + struct hid_usage *usage, __s32 value)
> +{
> + input_report_switch(field->hidinput->input, SW_TABLET_MODE, value);
[Severity: High]
Does this unconditionally dereference field->hidinput, which could be NULL
for unmapped usages?
Because the driver struct below lacks a .usage_table, hid_match_usage()
will default to true for all usages, meaning this callback intercepts every
incoming HID report field. This could lead to a NULL pointer dereference,
and will also blindly emit a SW_TABLET_MODE event for any report, causing
standard buttons or battery reports to randomly toggle tablet mode.
Since .input_mapping already maps the usage to EV_SW, allowing the generic
HID core to automatically report the event, could this callback be removed
entirely?
> +
> + return 0;
> +}
> +
> +static const struct hid_device_id amd_sfh_tabletmode_devices[] = {
> + { HID_DEVICE(BUS_AMD_SFH, HID_GROUP_GENERIC, AMD_SFH_HID_VENDOR, AMD_SFH_HID_PRODUCT) },
> + { }
> +};
> +MODULE_DEVICE_TABLE(hid, amd_sfh_tabletmode_devices);
> +
> +static struct hid_driver amd_sfh_tabletmode_driver = {
> + .name = "amd_sfh_tabletmode",
> + .id_table = amd_sfh_tabletmode_devices,
> + .probe = amd_sfh_tabletmode_probe,
> + .remove = amd_sfh_tabletmode_remove,
> + .input_mapping = amd_sfh_tabletmode_input_mapping,
> + .event = amd_sfh_tabletmode_event,
[Severity: High]
Should this struct include a .usage_table?
Without it, all device usages are routed to the .event callback above.
> +};
--
Sashiko AI review · https://sashiko.dev/#/patchset/ecadaac8-e222-420a-ac5b-4d529fb3317e@amd.com?part=2
^ permalink raw reply
* Re: [PATCH 1/2] amd-sfh-hid: tablet mode hid report and asus quirk
From: sashiko-bot @ 2026-07-21 14:47 UTC (permalink / raw)
To: Helge Bahmann; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <2108873.tdWV9SEqCh@zephyr>
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
New issues:
- [Medium] Missing HID Input Mapping for tablet mode switch
- [Low] Incomplete Implementation / Dead Code for Feature Report
- [Low] Unintentional Parameter Override via DMI Quirk
- [Low] Unused Macros for Tablet State Evaluated by Magic Number
- [Low] Introduction of Non-English Comments
Pre-existing issues:
- [High] Double Free / Use-After-Free in amdtp_hid_probe error path
--
--- Patch [1]: [PATCH 1/2] amd-sfh-hid: tablet mode hid report and asus quirk ---
commit 8025fc07a2e48355e8daeee6c47f30c940eb5a30
Author: Helge Bahmann <hcb@chaoticmind.net>
amd-sfh-hid: tablet mode hid report and asus quirk
Add an input driver that interprets the "operation mode" sensor offered
by the amd sfh as a hid driver to generate a custom hid report (tablet
mode switch).
Add a quirk to restore compatibility of the driver with the Asus
VivoBook (turn off the "disable interrupts flag). Expose the
intr_disable flag as a module parameter in case it turns out to be
needed on further laptop models.
> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_client.c b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
> index 96ae792beeb6c..8c03279646023 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_client.c
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
[ ... ]
> @@ -327,12 +314,6 @@ int amd_sfh_hid_client_init(struct amd_mp2_dev *privdata)
>
> for (i = 0; i < cl_data->num_hid_devices; i++) {
> cl_data->cur_hid_dev = i;
> - if (cl_data->sensor_idx[i] == op_idx) {
> - dev_dbg(dev, "sid 0x%x (%s) status 0x%x\n",
> - cl_data->sensor_idx[i], get_sensor_name(cl_data->sensor_idx[i]),
> - cl_data->sensor_sts[i]);
> - continue;
> - }
>
> if (cl_data->sensor_sts[i] == SENSOR_ENABLED) {
> rc = amdtp_hid_probe(i, cl_data);
[Severity: High]
This isn't a bug introduced by this patch, but is a pre-existing issue in the
driver's error-handling path.
If amdtp_hid_probe() fails (e.g., in hid_add_device()), it destroys the hid
device but does not clear the dangling pointer in
cli_data->hid_sensor_hubs[cur_hid_dev].
When this failure propagates up to amd_sfh_hid_client_init(), it jumps to the
cleanup label and invokes amd_sfh_hid_client_deinit() -> amdtp_hid_remove().
amdtp_hid_remove() will then iterate over the hid_sensor_hubs array and call
hid_destroy_device() again on the dangling pointer, which can cause a double
free.
Should the pointer be set to NULL in amdtp_hid_probe() on error?
> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> index 4b81cebdc3359..924329bfef8fe 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> @@ -39,6 +39,8 @@ module_param_named(sensor_mask, sensor_mask_override, int, 0444);
> MODULE_PARM_DESC(sensor_mask, "override the detected sensors mask");
>
> static bool intr_disable = true;
> +module_param_named(intr_disable, intr_disable, bool, 0444);
> +MODULE_PARM_DESC(intr_disable, "override the interrupt disable sensor bit");
[Severity: Low]
Because intr_disable is implemented as a simple boolean, the DMI match quirk
for Asus VivoBook unconditionally invokes mp2_disable_intr(), which
overwrites intr_disable to false.
Does this prevent the driver from differentiating between the default state
and an explicit user override on the command line (e.g.,
amd_sfh.intr_disable=1)?
> diff --git a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c
> index ef1f9be8b8938..7e807b92d2960 100644
> --- a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c
> +++ b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c
[ ... ]
> @@ -164,6 +180,11 @@ static u8 get_feature_report(int sensor_idx, int report_id, u8 *feature_report)
> memcpy(feature_report, &gyro_feature, sizeof(gyro_feature));
> report_size = sizeof(gyro_feature);
> break;
> + case op_idx: /* op */
> + get_common_features(&tablet_mode_feature.common_property, report_id);
> + memcpy(feature_report, &tablet_mode_feature, sizeof(tablet_mode_feature));
> + report_size = sizeof(tablet_mode_feature);
> + break;
[Severity: Low]
The driver allocates and populates a Feature report
(tablet_mode_feature_report) here, but the corresponding
tablet_mode_report_descriptor lacks any Feature items (0xB1).
Since the HID core strictly relies on the parsed descriptor to create reports,
will a feature report with ID 0x11 ever be requested, or is this unused
dead code?
> @@ -238,6 +260,15 @@ static u8 get_input_report(u8 current_index, int sensor_idx, int report_id,
> memcpy(input_report, &gyro_input, sizeof(gyro_input));
> report_size = sizeof(gyro_input);
> break;
> + case op_idx: /* op */
> + tablet_mode_input.report_id = 0x11;
> + if (sensor_virt_addr[0] == 3)
> + tablet_mode_input.tablet_state = 1;
> + else
> + tablet_mode_input.tablet_state = 0;
[Severity: Low]
This code relies on a hardcoded magic number 3. Should this evaluate the
sensor payload using the newly defined AMD_SFH_OP_IDX_MODE_TABLET macro
instead?
> diff --git a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h
> index 882434b1501ff..ff82e47eb3ca4 100644
> --- a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h
> +++ b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h
> @@ -114,4 +114,13 @@ struct hpd_input_report {
> u8 human_presence;
> } __packed;
>
> +struct tablet_mode_feature_report {
> + struct common_feature_property common_property;
> +} __packed;
> +
> +struct tablet_mode_input_report {
> + u8 report_id; /* Muss 0x11 (17) sein */
[Severity: Low]
Is this German comment intentional? Kernel coding style typically requires all
comments to be written in English.
> diff --git a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h
> index 67ec2d6a417de..f95368534fc60 100644
> --- a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h
> +++ b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h
> @@ -776,4 +776,22 @@ static const u8 hpd_report_descriptor[] = {
> 0X81, 0x02, /* HID Input (Data_Var_Abs) */
> 0xC0 /* HID end collection */
> };
> +
> +/* tablet mode switch */
> +static const u8 tablet_mode_report_descriptor[] = {
> +0x05, 0x0C, /* HID usage consumer electronics */
> +0x09, 0x01, /* HID usage */
> +0xA1, 0x00, /* HID collection (Physical) */
> +0x85, 0x11, /* HID report id */
> +0x0A, 0xFF, 0x02, /* HID usage (unallocated -- (ab)used for tablet mode) */
[Severity: Medium]
Because the driver does not provide a custom .input_mapping callback to
intercept and map this unallocated Consumer usage to SW_TABLET_MODE, will the
standard hid-input core ignore it or map it to KEY_UNKNOWN (240)?
Could this result in physical changes in tablet mode emitting spurious unknown
key events instead of a standard EV_SW switch event?
--
Sashiko AI review · https://sashiko.dev/#/patchset/ecadaac8-e222-420a-ac5b-4d529fb3317e@amd.com?part=1
^ permalink raw reply
* Re: [PATCH v3 3/4] dt-bindings: input: Convert TI TPS65217 power button to DT schema
From: Rob Herring @ 2026-07-21 15:02 UTC (permalink / raw)
To: sashiko-reviews
Cc: Eduard Bostina, conor+dt, linux-input, linux-leds, dri-devel,
devicetree, lee, dmitry.torokhov
In-Reply-To: <20260721113408.655C01F000E9@smtp.kernel.org>
On Tue, Jul 21, 2026 at 11:34:07AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Missing NULL check on `dev_get_regmap` can lead to a kernel panic in the IRQ handler.
> - [High] The driver incorrectly masks `platform_get_irq()` error codes with `-EINVAL`, breaking probe deferral.
> --
Why is sashiko pointing out driver issues on a DT binding patch? This
series doesn't even touch the driver. This seems to be a re-occurring
problem.
Rob
^ permalink raw reply
* Re: [PATCH v6 1/4] HID: wacom: Advertise SW_MUTE_DEVICE capability prior to registration
From: Jason Gerecke @ 2026-07-21 15:33 UTC (permalink / raw)
To: Lee Jones
Cc: Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Aaron Skomra, Dmitry Torokhov, Peter Hutterer, linux-input,
linux-kernel
In-Reply-To: <20260720122156.GA11805@google.com>
On Mon, Jul 20, 2026 at 5:54 AM Lee Jones <lee@kernel.org> wrote:
>
> Is Jason back in this week?
>
> Note that all of the Sashiko review comments are pre-existing issues.
>
I'm back now :)
> > Input subsystem guidelines require that device capabilities are advertised
> > before the input device is registered. The Wacom driver was violating
> > this by advertising the SW_MUTE_DEVICE capability post-registration in
> > wacom_set_shared_values() (and duplicating it in device-specific setup
> > cases).
> >
> > Resolve this by moving the SW_MUTE_DEVICE capability setup to
> > wacom_setup_touch_input_capabilities() for all touch devices that support
> > it, excluding TabletPC devices.
This paragraph is no longer accurate. The TabletPC-specific checks
were removed. It would be more correct to say something more like "for
all HID_GENERIC touch devices that support it" given the new logic.
> >
> > Additionally, replace the lookup-dependent
> > 'wacom_wac->shared->touch->product' references with 'hdev->product'
> > inside wacom_setup_touch_input_capabilities() as 'hdev' is already
> > available and represents the touch device itself.
> >
> > Fixes: d2ec58aee8b1 ("HID: wacom: generic: support generic touch switch")
> > Signed-off-by: Lee Jones <lee@kernel.org>
> > ---
> >
> > v4 -> v5: New patch used to split out SW_MUTE_DEVICE as per Jason's request
> > v5 -> v6: Unconditionally advertise SW_MUTE_DEVICE on generic touch devices
> >
> > drivers/hid/wacom_sys.c | 6 ------
> > drivers/hid/wacom_wac.c | 19 +++++++++++--------
> > 2 files changed, 11 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> > index 0eafa483b7f7..6c2a3ddfe366 100644
> > --- a/drivers/hid/wacom_sys.c
> > +++ b/drivers/hid/wacom_sys.c
> > @@ -2359,12 +2359,6 @@ static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
> > wacom_wac->shared->is_touch_on = true;
> > }
> >
> > - if (wacom_wac->shared->has_mute_touch_switch &&
> > - wacom_wac->shared->touch_input) {
> > - set_bit(EV_SW, wacom_wac->shared->touch_input->evbit);
> > - input_set_capability(wacom_wac->shared->touch_input, EV_SW,
> > - SW_MUTE_DEVICE);
> > - }
> > }
> >
> > static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
> > diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> > index a29bf051ada7..32c4fd03479e 100644
> > --- a/drivers/hid/wacom_wac.c
> > +++ b/drivers/hid/wacom_wac.c
> > @@ -3953,6 +3953,8 @@ int wacom_setup_pen_input_capabilities(struct input_dev *input_dev,
> > int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
> > struct wacom_wac *wacom_wac)
> > {
> > + struct wacom *wacom = container_of(wacom_wac, struct wacom, wacom_wac);
> > + struct hid_device *hdev = wacom->hdev;
> > struct wacom_features *features = &wacom_wac->features;
> >
> > if (!(features->device_type & WACOM_DEVICETYPE_TOUCH))
> > @@ -3963,9 +3965,12 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
> > else
> > __set_bit(INPUT_PROP_POINTER, input_dev->propbit);
> >
> > - if (features->type == HID_GENERIC)
> > + if (features->type == HID_GENERIC) {
> > + input_dev->evbit[0] |= BIT_MASK(EV_SW);
> > + __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
> > /* setup has already been done */
> > return 0;
> > + }
I'm still concerned about this not working properly. I don't see how
you are preserving the desired behavior where SW_MUTE_DEVICE is only
set on devices where `has_mute_touch_switch` is true (e.g. when a
WACOM_HID_WD_TOUCHONOFF usage is discovered in the HID descriptor). It
seems like this change will result in *all* HID_GENERIC touch devices
gaining SW_MUTE_DEVICE, which is not correct.
For devices that have SW_MUTE_DEVICE on the touch interface, I think
it would be sufficient to simply add a `has_mute_touch_switch` check
here, since the HID descriptor will have already been parsed by this
point. **But** I think the trickier part is handling the case where
SW_MUTE_DEVICE for the touch appears on a different interface,
endpoint, or (potentially) sibling USB device... (The SW_MUTE_DEVICE
usage is usually found in the "pad" collection, which is appears in
the HID descriptor for the pen interface/endpoint/device)
Jason (she/they)
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one /
(That is to say, eight) to the two, /
But you can’t take seven from three, /
So you look at the sixty-fours....
> >
> > input_dev->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> > __set_bit(BTN_TOUCH, input_dev->keybit);
> > @@ -3997,19 +4002,17 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
> > input_dev->evbit[0] |= BIT_MASK(EV_SW);
> > __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
> >
> > - if (wacom_wac->shared->touch->product == 0x361) {
> > + if (hdev->product == 0x361) {
> > input_set_abs_params(input_dev, ABS_MT_POSITION_X,
> > 0, 12440, 4, 0);
> > input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
> > 0, 8640, 4, 0);
> > - }
> > - else if (wacom_wac->shared->touch->product == 0x360) {
> > + } else if (hdev->product == 0x360) {
> > input_set_abs_params(input_dev, ABS_MT_POSITION_X,
> > 0, 8960, 4, 0);
> > input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
> > 0, 5920, 4, 0);
> > - }
> > - else if (wacom_wac->shared->touch->product == 0x393) {
> > + } else if (hdev->product == 0x393) {
> > input_set_abs_params(input_dev, ABS_MT_POSITION_X,
> > 0, 6400, 4, 0);
> > input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
> > @@ -4039,8 +4042,8 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
> > fallthrough;
> >
> > case WACOM_27QHDT:
> > - if (wacom_wac->shared->touch->product == 0x32C ||
> > - wacom_wac->shared->touch->product == 0xF6) {
> > + if (hdev->product == 0x32C ||
> > + hdev->product == 0xF6) {
> > input_dev->evbit[0] |= BIT_MASK(EV_SW);
> > __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
> > wacom_wac->has_mute_touch_switch = true;
> > --
> > 2.55.0.141.g00534a21ce-goog
> >
>
> --
> Lee Jones
>
^ permalink raw reply
* [PATCH] HID: logitech-hidpp: Add support for G502 X Plus USB mouse
From: Stuart Hayhurst @ 2026-07-21 17:12 UTC (permalink / raw)
To: linux-kernel, Jiri Kosina
Cc: Stuart Hayhurst, linux-input, Benjamin Tissoires, Bastien Nocera,
Filipe Laíns
The wireless dongle is already supported, this adds detection for
the mouse in wired mode. Supports battery reporting.
Signed-off-by: Stuart Hayhurst <stuart.a.hayhurst@gmail.com>
---
drivers/hid/hid-logitech-hidpp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 90b0184df777..25620494e741 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4647,6 +4647,8 @@ static const struct hid_device_id hidpp_devices[] = {
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC082) },
{ /* Logitech G502 Lightspeed Wireless Gaming Mouse over USB */
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC08D) },
+ { /* Logitech G502 X Plus Wireless Gaming Mouse over USB */
+ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC095) },
{ /* Logitech G703 Gaming Mouse over USB */
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC087) },
{ /* Logitech G703 Hero Gaming Mouse over USB */
--
2.53.0
^ permalink raw reply related
* [PATCH] HID: logitech-hidpp: Remove duplicate ID for the G703 Hero over USB
From: Stuart Hayhurst @ 2026-07-21 17:37 UTC (permalink / raw)
To: linux-kernel, Jiri Kosina
Cc: Stuart Hayhurst, linux-input, Benjamin Tissoires, Bastien Nocera,
Filipe Laíns
The ID appears to be given twice, remove the duplicate
Signed-off-by: Stuart Hayhurst <stuart.a.hayhurst@gmail.com>
---
drivers/hid/hid-logitech-hidpp.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 90b0184df777..b1b1c5cb657c 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4659,8 +4659,6 @@ static const struct hid_device_id hidpp_devices[] = {
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC088) },
{ /* MX Vertical over USB */
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC08A) },
- { /* Logitech G703 Hero Gaming Mouse over USB */
- HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC090) },
{ /* Logitech G903 Hero Gaming Mouse over USB */
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC091) },
{ /* Logitech G915 TKL Keyboard over USB */
--
2.53.0
^ permalink raw reply related
* [PATCH 2/3] HID: amd_sfh: Register tablet-mode auxiliary device
From: Basavaraj Natikar @ 2026-07-21 17:44 UTC (permalink / raw)
To: jikos, bentiss, dmitry.torokhov; +Cc: linux-input, Basavaraj Natikar
In-Reply-To: <20260721174422.3109166-1-Basavaraj.Natikar@amd.com>
Register an auxiliary device when the operating-mode sensor is present,
so a dedicated input driver can bind to it and report the device
posture. This keeps the input handling out of the sensor transport
driver.
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
drivers/hid/amd-sfh-hid/Kconfig | 1 +
drivers/hid/amd-sfh-hid/amd_sfh_common.h | 2 ++
drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 30 ++++++++++++++++++++++++
3 files changed, 33 insertions(+)
diff --git a/drivers/hid/amd-sfh-hid/Kconfig b/drivers/hid/amd-sfh-hid/Kconfig
index 3291786a5ee6..d86d83ffebd7 100644
--- a/drivers/hid/amd-sfh-hid/Kconfig
+++ b/drivers/hid/amd-sfh-hid/Kconfig
@@ -6,6 +6,7 @@ menu "AMD SFH HID Support"
config AMD_SFH_HID
tristate "AMD Sensor Fusion Hub"
depends on X86
+ select AUXILIARY_BUS
help
If you say yes to this option, support will be included for the
AMD Sensor Fusion Hub.
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_common.h b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
index cad7d5f8fd4d..34270ad95cf0 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_common.h
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
@@ -10,6 +10,7 @@
#ifndef AMD_SFH_COMMON_H
#define AMD_SFH_COMMON_H
+#include <linux/auxiliary_bus.h>
#include <linux/mutex.h>
#include <linux/pci.h>
#include "amd_sfh_hid.h"
@@ -64,6 +65,7 @@ struct amd_mp2_dev {
struct mutex lock;
u8 init_done;
u8 rver;
+ struct auxiliary_device *tm_auxdev;
};
struct amd_mp2_ops {
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
index a3672a9302c5..826fd762514f 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
@@ -8,6 +8,7 @@
* Basavaraj Natikar <Basavaraj.Natikar@amd.com>
*/
+#include <linux/auxiliary_bus.h>
#include <linux/bitops.h>
#include <linux/delay.h>
#include <linux/devm-helpers.h>
@@ -391,6 +392,32 @@ static const struct attribute_group *amd_sfh_groups[] = {
NULL,
};
+static void amd_sfh_maybe_register_tm(struct amd_mp2_dev *mp2)
+{
+ bool present;
+
+ if (mp2->tm_auxdev)
+ return;
+
+ present = mp2->sfh1_1_ops ? mp2->dev_en.is_sra_present
+ : amd_sfh_op_idx_enabled(mp2);
+ if (!present)
+ return;
+
+ mp2->tm_auxdev = auxiliary_device_create(&mp2->pdev->dev, KBUILD_MODNAME,
+ "tabletmode", NULL, 0);
+ if (!mp2->tm_auxdev)
+ dev_warn(&mp2->pdev->dev, "tabletmode auxdev create failed\n");
+}
+
+static void amd_sfh_unregister_tm(struct amd_mp2_dev *mp2)
+{
+ if (mp2->tm_auxdev) {
+ auxiliary_device_destroy(mp2->tm_auxdev);
+ mp2->tm_auxdev = NULL;
+ }
+}
+
static void sfh1_1_init_work(struct work_struct *work)
{
struct amd_mp2_dev *mp2 = container_of(work, struct amd_mp2_dev, work);
@@ -407,6 +434,7 @@ static void sfh1_1_init_work(struct work_struct *work)
if (rc)
dev_warn(&mp2->pdev->dev, "failed to update sysfs group\n");
+ amd_sfh_maybe_register_tm(mp2);
}
static void sfh_init_work(struct work_struct *work)
@@ -425,6 +453,7 @@ static void sfh_init_work(struct work_struct *work)
amd_sfh_clear_intr(mp2);
mp2->init_done = 1;
+ amd_sfh_maybe_register_tm(mp2);
}
static void amd_sfh_remove(struct pci_dev *pdev)
@@ -432,6 +461,7 @@ static void amd_sfh_remove(struct pci_dev *pdev)
struct amd_mp2_dev *mp2 = pci_get_drvdata(pdev);
flush_work(&mp2->work);
+ amd_sfh_unregister_tm(mp2);
if (mp2->init_done)
mp2->mp2_ops->remove(mp2);
}
--
2.34.1
^ permalink raw reply related
* [PATCH 1/3] HID: amd_sfh: Add accessor to read the operating-mode sensor
From: Basavaraj Natikar @ 2026-07-21 17:44 UTC (permalink / raw)
To: jikos, bentiss, dmitry.torokhov; +Cc: linux-input, Basavaraj Natikar
In-Reply-To: <20260721174422.3109166-1-Basavaraj.Natikar@amd.com>
Allow other drivers to query the operating mode (laptop or tablet)
reported by the Sensor Fusion Hub. This is the interface used by the
tablet-mode switch driver to report the device posture to userspace.
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
drivers/hid/amd-sfh-hid/amd_sfh_client.c | 16 +++++++++
drivers/hid/amd-sfh-hid/amd_sfh_common.h | 5 +++
drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 12 +++++++
.../amd-sfh-hid/sfh1_1/amd_sfh_interface.c | 33 +++++++++++++++++++
.../amd-sfh-hid/sfh1_1/amd_sfh_interface.h | 1 -
include/linux/amd-pmf-io.h | 14 ++++++++
6 files changed, 80 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_client.c b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
index 96ae792beeb6..0f5346d2174c 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_client.c
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
@@ -383,3 +383,19 @@ int amd_sfh_hid_client_deinit(struct amd_mp2_dev *privdata)
return 0;
}
+
+bool amd_sfh_op_idx_enabled(struct amd_mp2_dev *mp2)
+{
+ struct amdtp_cl_data *cl = mp2->cl_data;
+ int i;
+
+ if (!cl)
+ return false;
+
+ for (i = 0; i < cl->num_hid_devices; i++)
+ if (cl->sensor_idx[i] == op_idx &&
+ cl->sensor_sts[i] == SENSOR_ENABLED)
+ return true;
+
+ return false;
+}
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_common.h b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
index 78f830c133e5..cad7d5f8fd4d 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_common.h
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
@@ -100,4 +100,9 @@ static inline u64 amd_get_p2c_val(struct amd_mp2_dev *mp2, u32 idx)
{
return mp2->rver == 1 ? AMD_P2C_MSG_V1(idx) : AMD_P2C_MSG(idx);
}
+
+bool amd_sfh_op_idx_enabled(struct amd_mp2_dev *mp2);
+void sfh_set_emp2(struct amd_mp2_dev *mp2);
+void sfh_deinit_emp2(void);
+
#endif
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
index 4b81cebdc335..a3672a9302c5 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
@@ -248,6 +248,11 @@ int amd_mp2_get_sensor_num(struct amd_mp2_dev *privdata, u8 *sensor_id)
return num_of_sensors;
}
+static void amd_sfh_deinit_emp2_action(void *unused)
+{
+ sfh_deinit_emp2();
+}
+
static void amd_mp2_pci_remove(void *privdata)
{
struct amd_mp2_dev *mp2 = privdata;
@@ -447,6 +452,7 @@ static int amd_mp2_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
privdata->pdev = pdev;
dev_set_drvdata(&pdev->dev, privdata);
+
rc = pcim_enable_device(pdev);
if (rc)
return rc;
@@ -471,6 +477,10 @@ static int amd_mp2_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
if (rc)
return rc;
+ rc = devm_add_action_or_reset(&pdev->dev, amd_sfh_deinit_emp2_action, NULL);
+ if (rc)
+ return rc;
+
privdata->sfh1_1_ops = (const struct amd_sfh1_1_ops *)id->driver_data;
if (privdata->sfh1_1_ops) {
if (boot_cpu_data.x86 >= 0x1A)
@@ -480,6 +490,7 @@ static int amd_mp2_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
if (rc)
return rc;
+ sfh_set_emp2(privdata);
schedule_work(&privdata->work);
return 0;
}
@@ -498,6 +509,7 @@ static int amd_mp2_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
return rc;
}
+ sfh_set_emp2(privdata);
schedule_work(&privdata->work);
return 0;
}
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
index 837d59e7a661..74b3d3382a82 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
@@ -81,12 +81,43 @@ void sfh_deinit_emp2(void)
emp2 = NULL;
}
+void sfh_set_emp2(struct amd_mp2_dev *mp2)
+{
+ emp2 = mp2;
+}
+
void sfh_interface_init(struct amd_mp2_dev *mp2)
{
mp2->mp2_ops = &amd_sfh_ops;
emp2 = mp2;
}
+static int amd_sfh_op_mode_info(u32 *op_mode)
+{
+ struct sfh_op_mode mode;
+ bool present;
+
+ if (!op_mode)
+ return -EINVAL;
+ if (!emp2)
+ return -ENODEV;
+
+ present = emp2->sfh1_1_ops ? emp2->dev_en.is_sra_present
+ : amd_sfh_op_idx_enabled(emp2);
+ if (!present)
+ return -ENODEV;
+
+ mode.val = readl(emp2->mmio + amd_get_c2p_val(emp2, 3));
+ dev_dbg(&emp2->pdev->dev,
+ "op-mode: raw=0x%08x mode=%u lid=%u angle=%u ontable=%u devmode=%u\n",
+ mode.val, mode.op_mode.mode, mode.op_mode.lidstatus,
+ mode.op_mode.angle, mode.op_mode.ontablestate,
+ mode.op_mode.devicemode);
+ *op_mode = mode.op_mode.mode;
+
+ return 0;
+}
+
static int amd_sfh_mode_info(u32 *platform_type, u32 *laptop_placement)
{
struct sfh_op_mode mode;
@@ -169,6 +200,8 @@ int amd_get_sfh_info(struct amd_sfh_info *sfh_info, enum sfh_message_type op)
case MT_SRA:
return amd_sfh_mode_info(&sfh_info->platform_type,
&sfh_info->laptop_placement);
+ case MT_OP_MODE:
+ return amd_sfh_op_mode_info(&sfh_info->op_mode);
}
}
return -EINVAL;
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
index 665c99ad779f..56258c4d1b3a 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
@@ -185,7 +185,6 @@ struct sfh_op_mode {
};
void sfh_interface_init(struct amd_mp2_dev *mp2);
-void sfh_deinit_emp2(void);
void amd_sfh1_1_set_desc_ops(struct amd_mp2_ops *mp2_ops);
int amd_sfh_float_to_int(u32 flt32_val);
#endif
diff --git a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h
index 55198d2875cc..dc59c43bd8f2 100644
--- a/include/linux/amd-pmf-io.h
+++ b/include/linux/amd-pmf-io.h
@@ -19,11 +19,13 @@
* @MT_HPD: Message ID to know the Human presence info from MP2 FW
* @MT_ALS: Message ID to know the Ambient light info from MP2 FW
* @MT_SRA: Message ID to know the SRA data from MP2 FW
+ * @MT_OP_MODE: Message ID to know the operating-mode (tablet/laptop) info
*/
enum sfh_message_type {
MT_HPD,
MT_ALS,
MT_SRA,
+ MT_OP_MODE,
};
/**
@@ -44,12 +46,24 @@ enum sfh_hpd_info {
* @user_present: Populates the user presence information
* @platform_type: Operating modes (clamshell, flat, tent, etc.)
* @laptop_placement: Device states (ontable, onlap, outbag)
+ * @op_mode: Operating-mode field (see enum sfh_dev_mode); used for tablet detection
*/
struct amd_sfh_info {
u32 ambient_light;
u8 user_present;
u32 platform_type;
u32 laptop_placement;
+ u32 op_mode;
+};
+
+/**
+ * enum sfh_dev_mode - SFH operating-mode field (sfh_op_mode.mode, bits 0-2)
+ * @SFH_MODE_LAPTOP: Device is in laptop/clamshell posture
+ * @SFH_MODE_TABLET: Device is in tablet posture
+ */
+enum sfh_dev_mode {
+ SFH_MODE_LAPTOP = 1,
+ SFH_MODE_TABLET = 3,
};
enum laptop_placement {
--
2.34.1
^ permalink raw reply related
* [PATCH 0/3] Add SW_TABLET_MODE support for AMD SFH convertibles
From: Basavaraj Natikar @ 2026-07-21 17:44 UTC (permalink / raw)
To: jikos, bentiss, dmitry.torokhov; +Cc: linux-input, Basavaraj Natikar
Add SW_TABLET_MODE support for AMD convertibles whose operating mode
(laptop or tablet) is reported by the Sensor Fusion Hub.
The first patch adds a way to read the operating mode. The second
registers an auxiliary device when the sensor is present, and the third
is a small input driver that binds to it and reports the posture to
userspace. Keeping the input handling in its own driver avoids pulling
input/evdev into the sensor transport driver.
Basavaraj Natikar (3):
HID: amd_sfh: Add accessor to read the operating-mode sensor
HID: amd_sfh: Register tablet-mode auxiliary device
Input: misc: Add AMD SFH tablet-mode switch driver
MAINTAINERS | 1 +
drivers/hid/amd-sfh-hid/Kconfig | 1 +
drivers/hid/amd-sfh-hid/amd_sfh_client.c | 16 ++++
drivers/hid/amd-sfh-hid/amd_sfh_common.h | 7 ++
drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 42 ++++++++++
.../amd-sfh-hid/sfh1_1/amd_sfh_interface.c | 33 ++++++++
.../amd-sfh-hid/sfh1_1/amd_sfh_interface.h | 1 -
drivers/input/misc/Kconfig | 15 ++++
drivers/input/misc/Makefile | 1 +
drivers/input/misc/amd_sfh_tabletmode.c | 81 +++++++++++++++++++
include/linux/amd-pmf-io.h | 14 ++++
11 files changed, 211 insertions(+), 1 deletion(-)
create mode 100644 drivers/input/misc/amd_sfh_tabletmode.c
--
2.34.1
^ permalink raw reply
* [PATCH 3/3] Input: misc: Add AMD SFH tablet-mode switch driver
From: Basavaraj Natikar @ 2026-07-21 17:44 UTC (permalink / raw)
To: jikos, bentiss, dmitry.torokhov
Cc: linux-input, Basavaraj Natikar, Helge Bahmann
In-Reply-To: <20260721174422.3109166-1-Basavaraj.Natikar@amd.com>
Report whether an AMD convertible is in laptop or tablet mode using the
operating-mode sensor provided by the Sensor Fusion Hub, and expose it
to userspace as SW_TABLET_MODE, so userspace can react to the device
being folded into tablet posture.
Cc: Helge Bahmann <hcb@chaoticmind.net>
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
MAINTAINERS | 1 +
drivers/input/misc/Kconfig | 15 +++++
drivers/input/misc/Makefile | 1 +
drivers/input/misc/amd_sfh_tabletmode.c | 81 +++++++++++++++++++++++++
4 files changed, 98 insertions(+)
create mode 100644 drivers/input/misc/amd_sfh_tabletmode.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 1ab8736850ea..30cd879e4a3e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1301,6 +1301,7 @@ L: linux-input@vger.kernel.org
S: Maintained
F: Documentation/hid/amd-sfh*
F: drivers/hid/amd-sfh-hid/
+F: drivers/input/misc/amd_sfh_tabletmode.c
AMD SPI DRIVER
M: Raju Rangoju <Raju.Rangoju@amd.com>
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 1f6c57dba030..5b76be31f687 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -899,6 +899,21 @@ config INPUT_SOC_BUTTON_ARRAY
To compile this driver as a module, choose M here: the
module will be called soc_button_array.
+config INPUT_AMD_SFH_TABLETMODE
+ tristate "AMD SFH tablet-mode switch"
+ depends on AMD_SFH_HID
+ select AUXILIARY_BUS
+ help
+ Expose SW_TABLET_MODE for AMD convertible laptops whose
+ operation-mode sensor is provided by the AMD Sensor Fusion Hub.
+
+ Userspace components such as systemd-logind and modern desktop
+ environments react to SW_TABLET_MODE when the device is folded
+ into tablet posture.
+
+ To compile this driver as a module, choose M here: the module
+ will be called amd_sfh_tabletmode.
+
config INPUT_DRV260X_HAPTICS
tristate "TI DRV260X haptics support"
depends on INPUT && I2C
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 2281d6803fce..72aa0658ebb3 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -81,6 +81,7 @@ obj-$(CONFIG_INPUT_RK805_PWRKEY) += rk805-pwrkey.o
obj-$(CONFIG_INPUT_SC27XX_VIBRA) += sc27xx-vibra.o
obj-$(CONFIG_INPUT_SGI_BTNS) += sgi_btns.o
obj-$(CONFIG_INPUT_SOC_BUTTON_ARRAY) += soc_button_array.o
+obj-$(CONFIG_INPUT_AMD_SFH_TABLETMODE) += amd_sfh_tabletmode.o
obj-$(CONFIG_INPUT_SPARCSPKR) += sparcspkr.o
obj-$(CONFIG_INPUT_STPMIC1_ONKEY) += stpmic1_onkey.o
obj-$(CONFIG_INPUT_TPS65218_PWRBUTTON) += tps65218-pwrbutton.o
diff --git a/drivers/input/misc/amd_sfh_tabletmode.c b/drivers/input/misc/amd_sfh_tabletmode.c
new file mode 100644
index 000000000000..f88edebbf35d
--- /dev/null
+++ b/drivers/input/misc/amd_sfh_tabletmode.c
@@ -0,0 +1,81 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * AMD SFH tablet-mode switch driver
+ *
+ * Copyright (c) 2026, Advanced Micro Devices, Inc.
+ * All Rights Reserved.
+ *
+ * Author: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
+ */
+
+#include <linux/amd-pmf-io.h>
+#include <linux/auxiliary_bus.h>
+#include <linux/input.h>
+#include <linux/module.h>
+#include <linux/pci_ids.h>
+
+#define POLL_INTERVAL_MS 200
+
+static void sfh_tm_poll(struct input_dev *input)
+{
+ struct amd_sfh_info info = {};
+
+ if (amd_get_sfh_info(&info, MT_OP_MODE))
+ return;
+
+ input_report_switch(input, SW_TABLET_MODE,
+ info.op_mode == SFH_MODE_TABLET);
+ input_sync(input);
+}
+
+static int sfh_tm_probe(struct auxiliary_device *auxdev,
+ const struct auxiliary_device_id *id)
+{
+ struct amd_sfh_info info = {};
+ struct device *dev = &auxdev->dev;
+ struct input_dev *input;
+ int rc;
+
+ rc = amd_get_sfh_info(&info, MT_OP_MODE);
+ if (rc)
+ return rc == -EINVAL ? -ENODEV : rc;
+
+ input = devm_input_allocate_device(dev);
+ if (!input)
+ return -ENOMEM;
+
+ input->name = "AMD SFH tablet mode switch";
+ input->phys = "amd-sfh/tabletmode";
+ input->id.bustype = BUS_HOST;
+ input->id.vendor = PCI_VENDOR_ID_AMD;
+ input_set_capability(input, EV_SW, SW_TABLET_MODE);
+
+ rc = input_setup_polling(input, sfh_tm_poll);
+ if (rc)
+ return rc;
+ input_set_poll_interval(input, POLL_INTERVAL_MS);
+
+ rc = input_register_device(input);
+ if (rc)
+ return rc;
+
+ sfh_tm_poll(input);
+
+ return 0;
+}
+
+static const struct auxiliary_device_id sfh_tm_id_table[] = {
+ { .name = "amd_sfh.tabletmode" },
+ {}
+};
+MODULE_DEVICE_TABLE(auxiliary, sfh_tm_id_table);
+
+static struct auxiliary_driver sfh_tm_driver = {
+ .name = "tabletmode",
+ .id_table = sfh_tm_id_table,
+ .probe = sfh_tm_probe,
+};
+module_auxiliary_driver(sfh_tm_driver);
+
+MODULE_DESCRIPTION("AMD SFH tablet mode switch");
+MODULE_LICENSE("GPL");
--
2.34.1
^ permalink raw reply related
* [PATCH v2 0/4] selftests: shared lib.bpf.mk for building BPF progs and skeletons
From: Ziyang Men @ 2026-07-21 17:48 UTC (permalink / raw)
To: Shuah Khan, Tejun Heo, Johannes Weiner, Michal Koutný,
Jiri Kosina, Benjamin Tissoires, David Vernet, Eduard Zingerman
Cc: Andrea Righi, Changwoo Min, Michal Hocko, Roman Gushchin,
Shakeel Butt, Muchun Song, Andrew Morton, JP Kobryn,
Mykola Lysenko, Nathan Chancellor, linux-kselftest, cgroups,
linux-input, sched-ext, linux-mm, kernel-team, bpf, llvm,
linux-kernel, Ziyang Men
This series re-factors the bpf-related toolchain for selftests, which
currently duplicated across selftests/{bpf,sched_ext,hid}/. Unify them
into a single includable fragment, tools/testing/selftests/lib.bpf.mk,
and wires up three consumers. We believe this will simplify the work for
configuraing selftests with the bpf, for both the existing and future ones.
Patch 1 adds lib.bpf.mk: it builds the in-tree libbpf.a and a host
bpftool, generates vmlinux.h from the kernel's BTF, compiles *.bpf.c into
BPF objects and skeletons, and exports the include paths, a bpf_link
helper, and BPF_LDLIBS. Use can control the behavior by several knobs
(source layout/suffix, extra headers/cflags, skeleton suffix,
subskeletons, output dirs) that can be used commonly in general cases.
Patch 2 adds a new selftest, test_memcg_stat_cross_cpu, under
selftests/cgroup. This test is a memory-cgroup statistics correctness
test that compares the memcg BPF kfuncs against memory.stat over a whole
cgroup subtree charged across many CPUs, and checks the cross-CPU rstat
flush. It is the first in-tree consumer of lib.bpf.mk outside
selftests/bpf.
Patches 3 and 4 convert the duplicated parts in Makefile for two
existing duplicating bpf consumers, the selftests/hid and
selftests/sched_ext, to lib.bpf.mk. The generated skeletons keep
a byte-identical public API and land in the same locations as before.
The folders build cleanly under both plain make and LLVM=1.
Changes since v1:
- Generalized lib.bpf.mk (source layout/suffix, extra hdrs/cflags,
skeleton suffix, subskeletons, configurable output dirs) so it can
serve hid and sched_ext, not just cgroup.
- Added patch 3 (hid) and patch 4 (sched_ext), converting those folders
to the shared fragment.
Ziyang Men (4):
selftests: add shared lib.bpf.mk to build BPF progs and skeletons
selftests/cgroup: add memcg_stat_cross_cpu correctness test for flush
selftests/hid: build the BPF program via the shared lib.bpf.mk
selftests/sched_ext: build BPF schedulers via the shared lib.bpf.mk
tools/testing/selftests/cgroup/.gitignore | 6 +
tools/testing/selftests/cgroup/Makefile | 21 +
tools/testing/selftests/cgroup/config | 4 +
.../selftests/cgroup/lib/cgroup_util.c | 48 ++
.../cgroup/lib/include/cgroup_util.h | 1 +
.../cgroup/memcg_stat_cross_cpu.bpf.c | 86 ++
.../selftests/cgroup/memcg_stat_cross_cpu.h | 27 +
.../cgroup/test_memcg_stat_cross_cpu.c | 780 ++++++++++++++++++
tools/testing/selftests/hid/Makefile | 182 +---
tools/testing/selftests/lib.bpf.mk | 247 ++++++
tools/testing/selftests/sched_ext/Makefile | 153 +---
11 files changed, 1275 insertions(+), 280 deletions(-)
create mode 100644 tools/testing/selftests/cgroup/memcg_stat_cross_cpu.bpf.c
create mode 100644 tools/testing/selftests/cgroup/memcg_stat_cross_cpu.h
create mode 100644 tools/testing/selftests/cgroup/test_memcg_stat_cross_cpu.c
create mode 100644 tools/testing/selftests/lib.bpf.mk
base-commit: 6eb8711ece2ce27e52e327a5b7a628ed39b97f45
--
2.53.0-Meta
^ permalink raw reply
* [PATCH v2 1/4] selftests: add shared lib.bpf.mk to build BPF progs and skeletons
From: Ziyang Men @ 2026-07-21 17:48 UTC (permalink / raw)
To: Shuah Khan, Tejun Heo, Johannes Weiner, Michal Koutný,
Jiri Kosina, Benjamin Tissoires, David Vernet, Eduard Zingerman
Cc: Andrea Righi, Changwoo Min, Michal Hocko, Roman Gushchin,
Shakeel Butt, Muchun Song, Andrew Morton, JP Kobryn,
Mykola Lysenko, Nathan Chancellor, linux-kselftest, cgroups,
linux-input, sched-ext, linux-mm, kernel-team, bpf, llvm,
linux-kernel, Ziyang Men
In-Reply-To: <20260721174833.1232771-1-ziyang.meme@gmail.com>
The libbpf + bpftool + vmlinux.h + BPF-object + skeleton build tool-chain
is currently duplicated across tools/testing/selftests/{bpf,sched_ext,
hid}/, each carrying ~100-140 lines of near-identical Makefile. As more
subsystems grow BPF-based selftests, the duplication scales poorly.
Add tools/testing/selftests/lib.bpf.mk, a single includable fragment that
provides the whole chain end-to-end. It builds the in-tree libbpf.a and a
host bpftool, generates vmlinux.h from the kernel's BTF, compiles *.bpf.c
into BPF objects (clang --target=bpf) and generates their skeletons. It
also provides what the user-space test binary needs to build: the header
search paths (so #includes resolve), a bpf_link macro that wraps the link
command, and BPF_LDLIBS, so the test can be statically linked against
libbpf.a.
To use: set BPF_SRCS and OVERRIDE_TARGETS := 1 before including ../lib.mk
(so lib.mk's default link rule is suppressed), then include ../lib.bpf.mk
and list $(BPF_SKELS) as prerequisites of the test binary, e.g.,:
BPF_SRCS := progs/foo.bpf.c
OVERRIDE_TARGETS := 1
include ../lib.mk
include ../lib.bpf.mk
$(OUTPUT)/foo_test: foo_test.c $(BPF_SKELS)
$(call bpf_link,$@,$<)
This saves much work for configuring selftests in other folder, such the cgroup.
net/bpf.mk (which only builds *.bpf.o, without skeleton or vmlinux.h
generation) is left unchanged; replacing the existing duplication
mentioned above is the next step.
Suggested-by: Shakeel Butt <shakeel.butt@linux.dev>
Suggested-by: Eduard Zingerman <eddyz87@gmail.com>
Suggested-by: Mykola Lysenko <mykolal@meta.com>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Ziyang Men <ziyang.meme@gmail.com>
---
tools/testing/selftests/lib.bpf.mk | 247 +++++++++++++++++++++++++++++
1 file changed, 247 insertions(+)
create mode 100644 tools/testing/selftests/lib.bpf.mk
diff --git a/tools/testing/selftests/lib.bpf.mk b/tools/testing/selftests/lib.bpf.mk
new file mode 100644
index 000000000000..6f175c6568e9
--- /dev/null
+++ b/tools/testing/selftests/lib.bpf.mk
@@ -0,0 +1,247 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Shared fragment for selftests that compile *.bpf.c into BPF objects +
+# skeletons and link them into userspace test binaries, without each
+# subsystem's Makefile re-implementing the libbpf/bpftool/vmlinux.h machinery.
+#
+# Caller contract (per-test Makefile):
+#
+# BPF_SRCS := foo.bpf.c bar.bpf.c
+# TEST_GEN_PROGS := foo_test
+# OVERRIDE_TARGETS := 1 # MUST be set before lib.mk
+# include ../lib.mk # defines OUTPUT, CC, Q, msg,
+# include ../lib.bpf.mk # selfdir, top_srcdir; honours OVERRIDE
+#
+# $(OUTPUT)/foo_test: foo_test.c $(BPF_SKELS)
+# $(call bpf_link,$@,$<)
+#
+# Optional knobs (set before including lib.bpf.mk):
+# BPF_PROG_EXT - source suffix, default .bpf.c; set to .c for the legacy
+# "progs/foo.c -> foo.bpf.o" layout.
+# BPF_EXTRA_HDRS - extra prerequisites (headers) for the BPF objects.
+# BPF_EXTRA_CFLAGS - appended to BPF_CFLAGS for the BPF compile.
+# BPF_SKEL_EXT - skeleton suffix, default .skel.h (e.g. .bpf.skel.h).
+# BPF_GEN_SUBSKEL - if set, also emit a subskeleton next to each skeleton.
+# BPF_OBJ_DIR - dir for generated *.bpf.o (default $(OUTPUT)).
+# BPF_SKEL_DIR - dir for generated skeletons (default $(OUTPUT)).
+# A caller with unusual compile needs may override BPF_CFLAGS wholesale after
+# the include (the object recipe expands it lazily).
+# BPF_SRCS entries may live in a subdirectory (e.g. progs/foo.bpf.c); the
+# objects and skeletons are always emitted flat under $(OUTPUT), keyed by the
+# source basename (foo.bpf.o / foo.skel.h).
+#
+# lib.mk MUST be included first: this fragment consumes the vars it defines
+# (OUTPUT, top_srcdir, CC, CLANG, Q, msg) and needs OVERRIDE_TARGETS to have
+# already suppressed lib.mk's default link rule.
+
+include $(top_srcdir)/tools/scripts/Makefile.arch # ARCH / SRCARCH
+# Pull in the shared toolchain definitions (HOSTCC/HOSTLD/CLANG) so this
+# fragment picks the *same* host compiler the libbpf and bpftool sub-makes
+# will: in particular HOSTCC becomes clang under LLVM=1 (gcc otherwise).
+# Without this the host bpftool and its bootstrap libbpf can end up built with
+# a mix of gcc and clang, which trips clang on gcc-only flags (-Wstrict-aliasing=3).
+#
+# Makefile.include's allow-override resets CC to a bare "clang", which would
+# drop the --target=<arch> flag lib.mk set for LLVM=1 cross builds (make LLVM=1
+# ARCH=arm64). lib.mk ran first and configured CC for the target, so save it
+# across the include and restore it afterwards.
+lib_bpf_mk_saved_cc := $(CC)
+include $(top_srcdir)/tools/scripts/Makefile.include
+CC := $(lib_bpf_mk_saved_cc)
+
+CLANG ?= clang
+HOSTCC ?= gcc
+HOSTLD ?= ld
+ifneq ($(V),1)
+submake_extras := feature_display=0
+endif
+
+# ---- paths & tools --------------------------------------------------------
+TOOLSDIR := $(top_srcdir)/tools
+LIBDIR := $(TOOLSDIR)/lib
+BPFDIR := $(LIBDIR)/bpf
+TOOLSINCDIR := $(TOOLSDIR)/include
+BPFTOOLDIR := $(TOOLSDIR)/bpf/bpftool
+APIDIR := $(TOOLSINCDIR)/uapi
+
+# Everything generated lives under $(OUTPUT) so O= and in-tree both work and
+# per-test builds (distinct $(OUTPUT)) never collide.
+SCRATCH_DIR := $(OUTPUT)/tools
+BUILD_DIR := $(SCRATCH_DIR)/build
+INCLUDE_DIR := $(SCRATCH_DIR)/include
+BPFOBJ := $(BUILD_DIR)/libbpf/libbpf.a
+
+# bpftool must run on the *host*; split the host toolchain out when cross-building.
+ifneq ($(CROSS_COMPILE),)
+HOST_BUILD_DIR := $(BUILD_DIR)/host
+HOST_SCRATCH_DIR := $(OUTPUT)/host-tools
+else
+HOST_BUILD_DIR := $(BUILD_DIR)
+HOST_SCRATCH_DIR := $(SCRATCH_DIR)
+endif
+HOST_BPFOBJ := $(HOST_BUILD_DIR)/libbpf/libbpf.a
+DEFAULT_BPFTOOL := $(HOST_SCRATCH_DIR)/sbin/bpftool
+BPFTOOL ?= $(DEFAULT_BPFTOOL)
+
+# ---- vmlinux BTF discovery -----------------------------------------------
+VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux) \
+ $(if $(KBUILD_OUTPUT),$(KBUILD_OUTPUT)/vmlinux) \
+ $(top_srcdir)/vmlinux \
+ /sys/kernel/btf/vmlinux \
+ /boot/vmlinux-$(shell uname -r)
+VMLINUX_BTF ?= $(abspath $(firstword $(wildcard $(VMLINUX_BTF_PATHS))))
+ifeq ($(VMLINUX_BTF),)
+$(error Cannot find a vmlinux for VMLINUX_BTF at any of "$(VMLINUX_BTF_PATHS)")
+endif
+
+# ---- clang flags ----------------------------------------------------------
+# Clang's default system includes (not the ones seen under --target=bpf); fixes
+# "missing" asm/byteorder.h etc. '-idirafter' so we never shadow real includes.
+define get_sys_includes
+$(shell $(1) $(2) -v -E - </dev/null 2>&1 \
+ | sed -n '/<...> search starts here:/,/End of search list./{ s| \(/.*\)|-idirafter \1|p }') \
+$(shell $(1) $(2) -dM -E - </dev/null | grep '__riscv_xlen ' | awk '{printf("-D__riscv_xlen=%d -D__BITS_PER_LONG=%d", $$3, $$3)}')
+endef
+ifneq ($(CROSS_COMPILE),)
+CLANG_TARGET_ARCH = --target=$(notdir $(CROSS_COMPILE:%-=%))
+endif
+CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG),$(CLANG_TARGET_ARCH))
+
+IS_LITTLE_ENDIAN = $(shell $(CC) -dM -E - </dev/null | \
+ grep 'define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__')
+MENDIAN = $(if $(IS_LITTLE_ENDIAN),-mlittle-endian,-mbig-endian)
+
+# Prefer -mcpu=v3; fall back to v2 on clang too old to know it (probed once).
+CLANG_BPF_CPU := $(shell $(CLANG) --target=bpf -mcpu=help 2>&1 | grep -q 'v3' \
+ && echo v3 || echo v2)
+
+# -fms-extensions + -Wno-microsoft-anon-tag: required so clang accepts the
+# anonymous nested struct/union members bpftool emits into vmlinux.h.
+BPF_CFLAGS = -g -Wall -Werror -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN) \
+ -I$(INCLUDE_DIR) -I$(APIDIR) -I$(TOOLSINCDIR) \
+ -std=gnu11 \
+ -fno-strict-aliasing \
+ -fms-extensions -Wno-microsoft-anon-tag \
+ -Wno-compare-distinct-pointer-types \
+ $(CLANG_SYS_INCLUDES) $(BPF_EXTRA_CFLAGS)
+
+# $1 = src .bpf.c, $2 = dst .bpf.o
+define BPF_BUILD_RULE
+ $(call msg,CLNG-BPF,,$2)
+ $(Q)$(CLANG) $(BPF_CFLAGS) -O2 --target=bpf -mcpu=$(CLANG_BPF_CPU) -c $1 -o $2
+endef
+
+# ---- output dirs for generated objects/skeletons --------------------------
+# Default: flat under $(OUTPUT) (what bpf/, hid/, cgroup/ do). A caller may
+# segregate the generated files into subdirs, e.g. BPF_SKEL_DIR := $(OUTPUT)/...
+BPF_OBJ_DIR ?= $(OUTPUT)
+BPF_SKEL_DIR ?= $(OUTPUT)
+
+# ---- scratch dirs ---------------------------------------------------------
+MAKE_DIRS := $(sort $(BUILD_DIR)/libbpf $(HOST_BUILD_DIR)/libbpf \
+ $(HOST_BUILD_DIR)/bpftool $(INCLUDE_DIR) \
+ $(filter-out $(OUTPUT),$(BPF_OBJ_DIR) $(BPF_SKEL_DIR)))
+$(MAKE_DIRS):
+ $(call msg,MKDIR,,$@)
+ $(Q)mkdir -p $@
+
+# ---- libbpf (target) ------------------------------------------------------
+# Pass ARCH/CROSS_COMPILE/CC through: lib.mk's CC is file-origin and is not
+# exported, so without this the libbpf sub-make would rebuild for the host under
+# a pure-LLVM cross build (make LLVM=1 ARCH=<arch>). -fPIC keeps the static
+# libbpf linkable into position-independent (PIE) test binaries.
+$(BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile) \
+ $(APIDIR)/linux/bpf.h | $(BUILD_DIR)/libbpf
+ $(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) OUTPUT=$(BUILD_DIR)/libbpf/ \
+ ARCH=$(ARCH) CROSS_COMPILE=$(CROSS_COMPILE) CC="$(CC)" \
+ EXTRA_CFLAGS='-g -O0 -fPIC' \
+ DESTDIR=$(SCRATCH_DIR) prefix= all install_headers
+
+# ---- libbpf (host) -- a distinct rule only when cross-compiling -----------
+ifneq ($(BPFOBJ),$(HOST_BPFOBJ))
+$(HOST_BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile) \
+ | $(HOST_BUILD_DIR)/libbpf
+ $(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) ARCH= CROSS_COMPILE= \
+ OUTPUT=$(HOST_BUILD_DIR)/libbpf/ CC=$(HOSTCC) LD=$(HOSTLD) \
+ EXTRA_CFLAGS='-g -O0' \
+ DESTDIR=$(HOST_SCRATCH_DIR) prefix= all install_headers
+endif
+
+# ---- bpftool (host) -------------------------------------------------------
+$(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
+ $(HOST_BPFOBJ) | $(HOST_BUILD_DIR)/bpftool
+ $(Q)$(MAKE) $(submake_extras) -C $(BPFTOOLDIR) \
+ ARCH= CROSS_COMPILE= CC=$(HOSTCC) LD=$(HOSTLD) \
+ EXTRA_CFLAGS='-g -O0' \
+ OUTPUT=$(HOST_BUILD_DIR)/bpftool/ \
+ LIBBPF_OUTPUT=$(HOST_BUILD_DIR)/libbpf/ \
+ LIBBPF_DESTDIR=$(HOST_SCRATCH_DIR)/ \
+ prefix= DESTDIR=$(HOST_SCRATCH_DIR)/ install-bin
+
+# ---- vmlinux.h ------------------------------------------------------------
+$(INCLUDE_DIR)/vmlinux.h: $(VMLINUX_BTF) $(BPFTOOL) | $(INCLUDE_DIR)
+ifeq ($(VMLINUX_H),)
+ $(call msg,GEN,,$@)
+ $(Q)$(BPFTOOL) btf dump file $(VMLINUX_BTF) format c > $@
+else
+ $(call msg,CP,,$@)
+ $(Q)cp "$(VMLINUX_H)" $@
+endif
+
+# ---- BPF objects + skeletons ---------------------------------------------
+# Sources may sit in a subdir and use the modern *.bpf.c or the legacy *.c
+# suffix (BPF_PROG_EXT); objects go in $(BPF_OBJ_DIR) and skeletons in
+# $(BPF_SKEL_DIR) (both default to $(OUTPUT)), keyed by the source basename.
+BPF_PROG_EXT ?= .bpf.c
+bpf_stems := $(patsubst %$(BPF_PROG_EXT),%,$(notdir $(BPF_SRCS)))
+# The output namespace is flat, so two sources with the same basename would
+# collapse into one object/skeleton; fail loudly instead of silently dropping.
+ifneq ($(words $(bpf_stems)),$(words $(sort $(bpf_stems))))
+$(error lib.bpf.mk: BPF_SRCS has colliding basenames: $(BPF_SRCS))
+endif
+# BPF_SKEL_EXT lets a caller pick the skeleton suffix (default .skel.h; e.g.
+# .bpf.skel.h). With BPF_GEN_SUBSKEL set, a matching subskeleton is emitted
+# alongside (foo.subskel.h / foo.bpf.subskel.h).
+BPF_SKEL_EXT ?= .skel.h
+BPF_SUBSKEL_EXT := $(patsubst %skel.h,%subskel.h,$(BPF_SKEL_EXT))
+BPF_OBJS := $(addprefix $(BPF_OBJ_DIR)/,$(addsuffix .bpf.o,$(bpf_stems)))
+BPF_SKELS := $(addprefix $(BPF_SKEL_DIR)/,$(addsuffix $(BPF_SKEL_EXT),$(bpf_stems)))
+
+# Locate the sources wherever the caller keeps them (e.g. progs/).
+vpath %$(BPF_PROG_EXT) $(sort $(dir $(BPF_SRCS)))
+
+$(BPF_OBJS): $(BPF_OBJ_DIR)/%.bpf.o: %$(BPF_PROG_EXT) $(BPF_EXTRA_HDRS) \
+ $(wildcard *.bpf.h) $(INCLUDE_DIR)/vmlinux.h | $(BPF_OBJ_DIR) $(BPFOBJ)
+ $(call BPF_BUILD_RULE,$<,$@)
+
+$(BPF_SKELS): $(BPF_SKEL_DIR)/%$(BPF_SKEL_EXT): $(BPF_OBJ_DIR)/%.bpf.o $(BPFTOOL) | $(BPF_SKEL_DIR)
+ $(call msg,GEN-SKEL,,$@)
+ $(Q)$(BPFTOOL) gen object $(<:.o=.linked.o) $<
+ $(Q)$(BPFTOOL) gen skeleton $(<:.o=.linked.o) name $(notdir $(<:.bpf.o=)) > $@
+ifneq ($(BPF_GEN_SUBSKEL),)
+ $(Q)$(BPFTOOL) gen subskeleton $(<:.o=.linked.o) name $(notdir $(<:.bpf.o=)) > $(@:$(BPF_SKEL_EXT)=$(BPF_SUBSKEL_EXT))
+endif
+
+# ---- exports consumed by the caller --------------------------------------
+# -I$(OUTPUT)/-I$(BPF_SKEL_DIR): so the test .c can #include "foo.skel.h".
+# -I$(INCLUDE_DIR): so userspace can pull in the generated vmlinux.h if needed.
+CFLAGS += -I$(OUTPUT) -I$(BPF_SKEL_DIR) -I$(INCLUDE_DIR)
+
+# Static libbpf.a first, then its deps. libbpf may pull in zstd (BTF decompress)
+# only when built against it; link -lzstd only if libzstd is present.
+BPF_LDLIBS := $(BPFOBJ) -lelf -lz
+ifneq ($(shell pkg-config --exists libzstd 2>/dev/null && echo y),)
+BPF_LDLIBS += -lzstd
+endif
+
+TEST_GEN_FILES += $(BPF_OBJS)
+
+# Link helper: $1 = output binary, $2 = test .c (skels are the target's deps).
+define bpf_link
+ $(call msg,BINARY,,$1)
+ $(Q)$(CC) $(CFLAGS) $2 $(BPF_LDLIBS) $(LDLIBS) -o $1
+endef
+
+EXTRA_CLEAN += $(SCRATCH_DIR) $(HOST_SCRATCH_DIR) \
+ $(addprefix $(BPF_OBJ_DIR)/,*.bpf.o *.linked.o) \
+ $(addprefix $(BPF_SKEL_DIR)/,*$(BPF_SKEL_EXT) *$(BPF_SUBSKEL_EXT))
--
2.53.0-Meta
^ permalink raw reply related
* [PATCH v2 2/4] selftests/cgroup: add memcg_stat_cross_cpu correctness test for flush
From: Ziyang Men @ 2026-07-21 17:48 UTC (permalink / raw)
To: Shuah Khan, Tejun Heo, Johannes Weiner, Michal Koutný,
Jiri Kosina, Benjamin Tissoires, David Vernet, Eduard Zingerman
Cc: Andrea Righi, Changwoo Min, Michal Hocko, Roman Gushchin,
Shakeel Butt, Muchun Song, Andrew Morton, JP Kobryn,
Mykola Lysenko, Nathan Chancellor, linux-kselftest, cgroups,
linux-input, sched-ext, linux-mm, kernel-team, bpf, llvm,
linux-kernel, Ziyang Men
In-Reply-To: <20260721174833.1232771-1-ziyang.meme@gmail.com>
Add a test_progs selftest that verifies the memory-cgroup BPF kfuncs
return values that agree with what userspace reads from cgroupfs, across
a whole cgroup subtree that has been charged on many CPUs.
It complements the existing cgroup_iter_memcg test. cgroup_iter_memcg
calls the memcg kfuncs on a single cgroup (BPF_CGROUP_ITER_SELF_ONLY) and
only asserts each value is greater than zero, so it never checks that a
value is actually correct. This test compares the kfunc values against
what userspace reads from memory.stat, and checks every node in the
cgroup tree.
Moreover, cgroup_iter_memcg does not exercise the rstat flush
(mem_cgroup_flush_stats()). This test does: it launches a process on
each leaf that charges and holds memory across multiple CPUs (so the
leaf's rstat is dirty on K per-cpu trees), then confirms the flush works
for both the BPF and the file path by checking two properties:
1. after the flush, each leaf's anon is at least what the process
charged there;
2. after the flush, the sum of the leaves' charged memory equals the
amount at the root.
The BPF reader and the file reader run in two separate rounds. Each
round builds the same cgroup tree from scratch and charges the same
amount of memory, so both rounds start from the same state and the
numbers are comparable. In detail:
- round 1 (BPF) builds the subtree, forks one child per leaf that
charges the leaf across K CPUs and then blocks holding the charge,
walks the tree with a SEC("iter.s/cgroup") program that flushes the
subtree at the root and reads each cgroup via the memcg kfuncs
(bpf_get_mem_cgroup, bpf_mem_cgroup_flush_stats,
bpf_mem_cgroup_page_state, bpf_mem_cgroup_vm_events,
bpf_put_mem_cgroup) into a hash map;
- round 2 (cgroupfs) builds and charges an identical tree the same way,
then reads every cgroup's memory.stat / memory.current from
userspace.
The subtests differ in how many CPUs each leaf is charged on (a single
CPU or across K CPUs), and run on two cgroup tree sizes. The charging
children pin CPUs and there is one per leaf, so the test is registered
serial.
The traditional path reads memory.stat / memory.current through a new
read_cgroup_file() helper added to cgroup_helpers (the read counterpart
of write_cgroup_file). When the memcg kfuncs are unavailable
(CONFIG_MEMCG=n) the test skips cleanly; the base selftest config now
selects CONFIG_MEMCG=y.
Suggested-by: Shakeel Butt <shakeel.butt@linux.dev>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Ziyang Men <ziyang.meme@gmail.com>
---
tools/testing/selftests/cgroup/.gitignore | 6 +
tools/testing/selftests/cgroup/Makefile | 21 +
tools/testing/selftests/cgroup/config | 4 +
.../selftests/cgroup/lib/cgroup_util.c | 48 ++
.../cgroup/lib/include/cgroup_util.h | 1 +
.../cgroup/memcg_stat_cross_cpu.bpf.c | 86 ++
.../selftests/cgroup/memcg_stat_cross_cpu.h | 27 +
.../cgroup/test_memcg_stat_cross_cpu.c | 780 ++++++++++++++++++
8 files changed, 973 insertions(+)
create mode 100644 tools/testing/selftests/cgroup/memcg_stat_cross_cpu.bpf.c
create mode 100644 tools/testing/selftests/cgroup/memcg_stat_cross_cpu.h
create mode 100644 tools/testing/selftests/cgroup/test_memcg_stat_cross_cpu.c
diff --git a/tools/testing/selftests/cgroup/.gitignore b/tools/testing/selftests/cgroup/.gitignore
index 952e4448bf07..9e6616af832e 100644
--- a/tools/testing/selftests/cgroup/.gitignore
+++ b/tools/testing/selftests/cgroup/.gitignore
@@ -6,7 +6,13 @@ test_freezer
test_hugetlb_memcg
test_kill
test_kmem
+test_memcg_stat_cross_cpu
test_memcontrol
test_pids
test_zswap
wait_inotify
+# Artifacts generated by lib.bpf.mk
+/tools
+*.bpf.o
+*.linked.o
+*.skel.h
diff --git a/tools/testing/selftests/cgroup/Makefile b/tools/testing/selftests/cgroup/Makefile
index e01584c2189a..daebd35aec79 100644
--- a/tools/testing/selftests/cgroup/Makefile
+++ b/tools/testing/selftests/cgroup/Makefile
@@ -14,14 +14,29 @@ TEST_GEN_PROGS += test_freezer
TEST_GEN_PROGS += test_hugetlb_memcg
TEST_GEN_PROGS += test_kill
TEST_GEN_PROGS += test_kmem
+TEST_GEN_PROGS += test_memcg_stat_cross_cpu
TEST_GEN_PROGS += test_memcontrol
TEST_GEN_PROGS += test_pids
TEST_GEN_PROGS += test_zswap
LOCAL_HDRS += $(selfdir)/clone3/clone3_selftests.h $(selfdir)/pidfd/pidfd.h
+# test_memcg_stat_cross_cpu builds a BPF program + skeleton through lib.bpf.mk.
+# OVERRIDE_TARGETS suppresses lib.mk's default C link rule (re-supplied below);
+# it must be set before ../lib.mk is included.
+BPF_SRCS := memcg_stat_cross_cpu.bpf.c
+OVERRIDE_TARGETS := 1
+
include ../lib.mk
include lib/libcgroup.mk
+include ../lib.bpf.mk
+
+# Re-supply the default C link rule that OVERRIDE_TARGETS removed, for the plain
+# cgroup tests. test_memcg_stat_cross_cpu has its own recipe further below.
+LOCAL_HDRS += $(selfdir)/kselftest_harness.h $(selfdir)/kselftest.h
+$(OUTPUT)/%: %.c $(LOCAL_HDRS)
+ $(call msg,CC,,$@)
+ $(Q)$(LINK.c) $(filter-out $(LOCAL_HDRS),$^) $(LDLIBS) -o $@
$(OUTPUT)/test_core: $(LIBCGROUP_O)
$(OUTPUT)/test_cpu: $(LIBCGROUP_O)
@@ -33,3 +48,9 @@ $(OUTPUT)/test_kmem: $(LIBCGROUP_O)
$(OUTPUT)/test_memcontrol: $(LIBCGROUP_O)
$(OUTPUT)/test_pids: $(LIBCGROUP_O)
$(OUTPUT)/test_zswap: $(LIBCGROUP_O)
+
+# test_memcg_stat_cross_cpu links cgroup_util and the generated BPF skeleton
+# against the in-tree static libbpf that lib.bpf.mk built.
+$(OUTPUT)/test_memcg_stat_cross_cpu: test_memcg_stat_cross_cpu.c \
+ $(BPF_SKELS) $(LIBCGROUP_O)
+ $(call bpf_link,$@,$< $(LIBCGROUP_O))
diff --git a/tools/testing/selftests/cgroup/config b/tools/testing/selftests/cgroup/config
index 39f979690dd3..9457bf604b23 100644
--- a/tools/testing/selftests/cgroup/config
+++ b/tools/testing/selftests/cgroup/config
@@ -4,3 +4,7 @@ CONFIG_CGROUP_FREEZER=y
CONFIG_CGROUP_SCHED=y
CONFIG_MEMCG=y
CONFIG_PAGE_COUNTER=y
+CONFIG_BPF=y
+CONFIG_BPF_SYSCALL=y
+CONFIG_CGROUP_BPF=y
+CONFIG_DEBUG_INFO_BTF=y
diff --git a/tools/testing/selftests/cgroup/lib/cgroup_util.c b/tools/testing/selftests/cgroup/lib/cgroup_util.c
index 2596c12cd864..3a30557855d3 100644
--- a/tools/testing/selftests/cgroup/lib/cgroup_util.c
+++ b/tools/testing/selftests/cgroup/lib/cgroup_util.c
@@ -54,6 +54,54 @@ ssize_t write_text(const char *path, char *buf, ssize_t len)
return len < 0 ? -errno : len;
}
+/*
+ * cg_get_id - return the kernfs id of a cgroup directory
+ * @cgroup: absolute path to the cgroup directory
+ *
+ * Returns the cgroup's kernfs node id (cgrp->kn->id) -- the same value the
+ * kernel exposes to BPF as cgrp->kn->id and via bpf_get_current_cgroup_id().
+ * This is obtained from the cgroupfs file handle and is NOT the directory's
+ * st_ino. Returns 0 (an invalid id) on failure.
+ */
+unsigned long long cg_get_id(const char *cgroup)
+{
+ union {
+ unsigned long long id;
+ unsigned char raw[8];
+ } handle;
+ struct file_handle *fhp, *fhp2;
+ int mount_id, fhsize, err;
+ unsigned long long ret = 0;
+
+ fhsize = sizeof(*fhp);
+ fhp = calloc(1, fhsize);
+ if (!fhp)
+ return 0;
+
+ /*
+ * The probe call is expected to fail (EOVERFLOW) and report the real
+ * handle size in fhp->handle_bytes; a cgroupfs handle is always 8 bytes.
+ */
+ err = name_to_handle_at(AT_FDCWD, cgroup, fhp, &mount_id, 0);
+ if (err >= 0 || fhp->handle_bytes != 8)
+ goto out;
+
+ fhsize = sizeof(*fhp) + fhp->handle_bytes;
+ fhp2 = realloc(fhp, fhsize);
+ if (!fhp2)
+ goto out;
+ fhp = fhp2;
+
+ if (name_to_handle_at(AT_FDCWD, cgroup, fhp, &mount_id, 0) < 0)
+ goto out;
+
+ memcpy(handle.raw, fhp->f_handle, 8);
+ ret = handle.id;
+out:
+ free(fhp);
+ return ret;
+}
+
char *cg_name(const char *root, const char *name)
{
size_t len = strlen(root) + strlen(name) + 2;
diff --git a/tools/testing/selftests/cgroup/lib/include/cgroup_util.h b/tools/testing/selftests/cgroup/lib/include/cgroup_util.h
index 8ebb2b4d4ec0..832fcefe60a4 100644
--- a/tools/testing/selftests/cgroup/lib/include/cgroup_util.h
+++ b/tools/testing/selftests/cgroup/lib/include/cgroup_util.h
@@ -54,6 +54,7 @@ extern ssize_t write_text(const char *path, char *buf, ssize_t len);
extern int cg_find_controller_root(char *root, size_t len, const char *controller);
extern int cg_find_unified_root(char *root, size_t len, bool *nsdelegate);
extern char *cg_name(const char *root, const char *name);
+extern unsigned long long cg_get_id(const char *cgroup);
extern char *cg_name_indexed(const char *root, const char *name, int index);
extern char *cg_control(const char *cgroup, const char *control);
extern int cg_create(const char *cgroup);
diff --git a/tools/testing/selftests/cgroup/memcg_stat_cross_cpu.bpf.c b/tools/testing/selftests/cgroup/memcg_stat_cross_cpu.bpf.c
new file mode 100644
index 000000000000..3b8c716e8d01
--- /dev/null
+++ b/tools/testing/selftests/cgroup/memcg_stat_cross_cpu.bpf.c
@@ -0,0 +1,86 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_core_read.h>
+#include "memcg_stat_cross_cpu.h"
+
+char _license[] SEC("license") = "GPL";
+
+/*
+ * Per-cgroup results, keyed by cgroup id. The BPF-side id (cgrp->kn->id)
+ * equals the userspace get_cgroup_id() value, so the test can correlate map
+ * entries back to the cgroups it created. max_entries is resized by userspace
+ * (bpf_map__set_max_entries) to the size of the subtree before load.
+ */
+struct {
+ __uint(type, BPF_MAP_TYPE_HASH);
+ __uint(max_entries, 1);
+ __type(key, __u64);
+ __type(value, struct memcg_stat_snapshot);
+} results SEC(".maps");
+
+/*
+ * Sleepable cgroup iterator: flush the subtree's rstat once at the root, then
+ * for every cgroup in the walked subtree read a fixed set of memcg statistics
+ * through the memcg kfuncs and stash them in the hash map for userspace to
+ * compare against memory.stat.
+ *
+ * The flush kfunc may sleep, hence SEC("iter.s/cgroup").
+ */
+SEC("iter.s/cgroup")
+int cgroup_memcg_stat_cross_cpu(struct bpf_iter__cgroup *ctx)
+{
+ struct cgroup *cgrp = ctx->cgroup;
+ struct memcg_stat_snapshot snap = {};
+ struct cgroup_subsys_state *css;
+ struct mem_cgroup *memcg;
+ int idx_anon, idx_file, idx_shmem, idx_fmapped, idx_pgfault;
+ __u64 cg_id;
+
+ /*
+ * DESCENDANTS_PRE ends with a terminal element where cgroup == NULL.
+ * Return 0 (not 1) so the walk runs to completion.
+ */
+ if (!cgrp)
+ return 0;
+
+ css = &cgrp->self;
+ memcg = bpf_get_mem_cgroup(css);
+ if (!memcg)
+ return 0;
+
+ /*
+ * Flush once, at the subtree root -- the first element visited in
+ * DESCENDANTS_PRE order (seq_num == 0). css_rstat_flush() is
+ * subtree-wide, so this one flush brings the whole walked subtree
+ * up to date and every descendant read afterwards is accurate;
+ * flushing again per-cgroup would only hit the no-op threshold gate.
+ */
+ if (ctx->meta->seq_num == 0)
+ bpf_mem_cgroup_flush_stats(memcg);
+
+ cg_id = BPF_CORE_READ(cgrp, kn, id);
+ snap.cgroup_id = cg_id;
+
+ idx_anon = bpf_core_enum_value(enum node_stat_item, NR_ANON_MAPPED);
+ idx_file = bpf_core_enum_value(enum node_stat_item, NR_FILE_PAGES);
+ idx_shmem = bpf_core_enum_value(enum node_stat_item, NR_SHMEM);
+ idx_fmapped = bpf_core_enum_value(enum node_stat_item, NR_FILE_MAPPED);
+ idx_pgfault = bpf_core_enum_value(enum vm_event_item, PGFAULT);
+
+ snap.anon = bpf_mem_cgroup_page_state(memcg, idx_anon);
+ snap.file = bpf_mem_cgroup_page_state(memcg, idx_file);
+ snap.shmem = bpf_mem_cgroup_page_state(memcg, idx_shmem);
+ snap.file_mapped = bpf_mem_cgroup_page_state(memcg, idx_fmapped);
+ snap.pgfault = bpf_mem_cgroup_vm_events(memcg, idx_pgfault);
+
+ /* page_counter fields need no kfunc; read them off the trusted ptr. */
+ snap.usage_pages = BPF_CORE_READ(memcg, memory.usage.counter);
+ snap.max_pages = BPF_CORE_READ(memcg, memory.max);
+
+ bpf_map_update_elem(&results, &cg_id, &snap, BPF_ANY);
+
+ bpf_put_mem_cgroup(memcg);
+ return 0;
+}
diff --git a/tools/testing/selftests/cgroup/memcg_stat_cross_cpu.h b/tools/testing/selftests/cgroup/memcg_stat_cross_cpu.h
new file mode 100644
index 000000000000..1583cafdab7e
--- /dev/null
+++ b/tools/testing/selftests/cgroup/memcg_stat_cross_cpu.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
+#ifndef __MEMCG_STAT_CROSS_CPU_H
+#define __MEMCG_STAT_CROSS_CPU_H
+
+/*
+ * One per-cgroup snapshot, produced by the BPF cgroup iterator and read back
+ * from a BPF hash map keyed by cgroup id. After the charge has quiesced the
+ * test compares every field against what userspace parses from
+ * memory.stat / memory.current / memory.max, so the two must agree.
+ *
+ * Page-state values are in bytes (already unit-scaled by the kernel), so they
+ * compare directly against memory.stat. usage_pages / max_pages come straight
+ * off the page_counter and are in PAGES.
+ */
+struct memcg_stat_snapshot {
+ __u64 cgroup_id;
+ __u64 anon; /* NR_ANON_MAPPED, bytes */
+ __u64 file; /* NR_FILE_PAGES, bytes */
+ __u64 shmem; /* NR_SHMEM, bytes */
+ __u64 file_mapped; /* NR_FILE_MAPPED, bytes */
+ __u64 pgfault; /* PGFAULT, count */
+ __u64 usage_pages; /* page_counter memory.usage, in PAGES */
+ __u64 max_pages; /* page_counter memory.max, in PAGES */
+};
+
+#endif /* __MEMCG_STAT_CROSS_CPU_H */
diff --git a/tools/testing/selftests/cgroup/test_memcg_stat_cross_cpu.c b/tools/testing/selftests/cgroup/test_memcg_stat_cross_cpu.c
new file mode 100644
index 000000000000..673b3da467ba
--- /dev/null
+++ b/tools/testing/selftests/cgroup/test_memcg_stat_cross_cpu.c
@@ -0,0 +1,780 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
+
+/*
+ * memcg_stat_cross_cpu
+ * ====================
+ * A memory-cgroup statistics correctness test. It compares the memcg
+ * statistics read through the BPF memcg kfuncs against what userspace reads
+ * from memory.stat, over a whole cgroup tree that has been charged across many
+ * CPUs. Where a plain kfunc smoke test only checks that a single cgroup's
+ * values are non-zero, this test checks the values are actually correct and
+ * that a cross-CPU rstat flush aggregates every per-CPU slice.
+ *
+ * The BPF reader and the file reader are run in two separate rounds, each on
+ * its own freshly built and charged tree:
+ *
+ * round 1: build the subtree; fork one process per leaf that charges the leaf
+ * across K CPUs and then blocks holding the charge; walk the tree
+ * with a SEC("iter.s/cgroup") program that flushes and reads each
+ * cgroup via the memcg kfuncs into a hash map; record one snapshot
+ * per node; clean the tree.
+ * round 2: build and charge an identical tree the same way, then read every
+ * cgroup's memory.stat / memory.current from userspace.
+ *
+ * The correctness is ensured as follows: each leaf was charged a known amount
+ * of anon spread over K CPUs, so the flushed anon must be at least that amount,
+ * and the subtree root's recursive anon must equal the sum of the leaves' anon.
+ *
+ * The charging children are CPU-pinning and there is one per leaf.
+ */
+#define _GNU_SOURCE
+
+#include <linux/limits.h>
+#include <fcntl.h>
+#include <sched.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <errno.h>
+#include <sys/mman.h>
+#include <sys/wait.h>
+#include <sys/prctl.h>
+
+#include <linux/bpf.h>
+#include <bpf/bpf.h>
+#include <bpf/libbpf.h>
+#include <bpf/btf.h>
+
+#include "kselftest.h"
+#include "cgroup_util.h"
+#include "memcg_stat_cross_cpu.h"
+#include "memcg_stat_cross_cpu.skel.h"
+
+#define SUBTREE_NAME "mcg_xcpu"
+
+static char root[PATH_MAX]; /* cgroup2 mount root */
+static char *subtree_root; /* <root>/mcg_xcpu, from cg_name() */
+
+struct cg_node {
+ char path[PATH_MAX];
+ __u64 id;
+ bool is_leaf;
+};
+
+/* Field subset parsed from memory.stat / memory.current. */
+struct file_snap {
+ __u64 anon, file, shmem, file_mapped, pgfault;
+ __u64 current; /* memory.current, bytes */
+ __u64 max; /* memory.max, bytes (valid unless max_is_max) */
+ bool max_is_max;
+};
+
+static long page_size;
+
+/* ---- allowed CPU set --------------------------------------------------- */
+
+static int *cpu_list; /* ids of the CPUs this process may run on */
+static int n_cpu; /* number of such CPUs */
+
+/*
+ * Collect the CPUs the test is allowed to run on.
+ * Bounded by CPU_SETSIZE (1024).
+ */
+static int collect_cpus(void)
+{
+ cpu_set_t set;
+ int i, want, n = 0;
+
+ CPU_ZERO(&set);
+ if (sched_getaffinity(0, sizeof(set), &set))
+ return -1;
+ want = CPU_COUNT(&set);
+ if (want <= 0)
+ return -1;
+ cpu_list = calloc(want, sizeof(*cpu_list));
+ if (!cpu_list)
+ return -1;
+ for (i = 0; i < CPU_SETSIZE && n < want; i++)
+ if (CPU_ISSET(i, &set))
+ cpu_list[n++] = i;
+ n_cpu = n;
+ return 0;
+}
+
+/* Pin the calling task to a single CPU. */
+static int pin_cpu(int cpu)
+{
+ cpu_set_t set;
+
+ CPU_ZERO(&set);
+ CPU_SET(cpu, &set);
+ return sched_setaffinity(0, sizeof(set), &set);
+}
+
+/* ---- tree construction ------------------------------------------------- */
+
+static struct cg_node *nodes;
+static int n_nodes;
+static int n_leaves;
+
+static int add_node(const char *path, bool is_leaf, int *keep_fd)
+{
+ if (cg_create(path))
+ return -1;
+ if (keep_fd) {
+ *keep_fd = open(path, O_RDONLY);
+ if (*keep_fd < 0)
+ return -1;
+ }
+
+ strncpy(nodes[n_nodes].path, path, sizeof(nodes[n_nodes].path) - 1);
+ nodes[n_nodes].path[sizeof(nodes[n_nodes].path) - 1] = '\0';
+ nodes[n_nodes].id = cg_get_id(path);
+ nodes[n_nodes].is_leaf = is_leaf;
+ if (is_leaf)
+ n_leaves++;
+ n_nodes++;
+ return 0;
+}
+
+/* Recursively create children of @path. @path must already exist and be recorded. */
+static int build_children(const char *path, int fanout, int depth)
+{
+ char child[PATH_MAX];
+ int i;
+
+ if (depth == 0)
+ return 0;
+
+ /* Enable memory on this interior node so its children get a memcg. */
+ if (cg_write(path, "cgroup.subtree_control", "+memory"))
+ return -1;
+
+ for (i = 0; i < fanout; i++) {
+ snprintf(child, sizeof(child), "%s/c%d", path, i);
+ if (add_node(child, depth == 1, NULL))
+ return -1;
+ if (build_children(child, fanout, depth - 1))
+ return -1;
+ }
+ return 0;
+}
+
+static size_t tree_capacity(int fanout, int depth)
+{
+ size_t total = 1, level = 1;
+ int d;
+
+ for (d = 0; d < depth; d++) {
+ level *= fanout;
+ total += level;
+ }
+ return total;
+}
+
+/* The tree is arranged in the DFS order within an array */
+static int build_tree(int fanout, int depth, int *root_fd)
+{
+ n_nodes = 0;
+ n_leaves = 0;
+ nodes = calloc(tree_capacity(fanout, depth), sizeof(*nodes));
+ if (!nodes)
+ return -1;
+
+ if (add_node(subtree_root, depth == 0, root_fd))
+ return -1;
+ return build_children(subtree_root, fanout, depth);
+}
+
+/* ---- cross-CPU charge (one pinning child per leaf) --------------------- */
+
+static pid_t *charger_pids;
+static int n_chargers;
+/* parent pid, for the children's PR_SET_PDEATHSIG race check */
+static pid_t test_pid;
+static int charge_ready[2] = { -1, -1 }; /* child -> parent "ready" barrier */
+static int charge_ctrl[2] = { -1, -1 }; /* parent -> child "exit" (close to signal) */
+
+/*
+ * One charging child, dedicated to a single leaf and spread over K CPUs. It
+ * joins its leaf, maps a resident anon region, then faults the region in K
+ * slices, each on a different CPU, so this leaf's rstat ends up dirty on K
+ * per-cpu trees. The region stays mapped, so the charge persists while the
+ * parent reads. After signalling readiness the child blocks (holding the
+ * charge) until the parent closes the control pipe. Never returns.
+ *
+ * @base is this child's starting index into cpu_list; its K CPUs are
+ * (base + 0..K-1) mod n_cpu.
+ */
+static void charger_child(const struct cg_node *leaf, int base, int k,
+ size_t resident_bytes)
+{
+ size_t per, off;
+ char *region;
+ char c;
+ int j;
+
+ /*
+ * If the parent dies without running the cleanup that closes the control
+ * pipe -- e.g. a CI timeout SIGKILLs the whole test -- ask the kernel to
+ * SIGKILL this child too, so it can never hang as an orphan holding a
+ * charge.
+ */
+ prctl(PR_SET_PDEATHSIG, SIGKILL);
+ if (getppid() != test_pid)
+ _exit(0);
+
+ close(charge_ready[0]);
+ close(charge_ctrl[1]);
+
+ if (cg_enter_current(leaf->path))
+ _exit(1);
+
+ region = mmap(NULL, resident_bytes, PROT_READ | PROT_WRITE,
+ MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
+ if (region == MAP_FAILED)
+ _exit(2);
+
+ /*
+ * Fault the region in K slices, each on a different CPU, so the charge
+ * for this leaf is scattered across K per-cpu rstat trees. A correct
+ * flush must gather all K slices.
+ */
+ per = resident_bytes / k;
+ for (j = 0; j < k; j++) {
+ off = (size_t)j * per;
+ if (pin_cpu(cpu_list[(base + j) % n_cpu]))
+ _exit(3);
+ memset(region + off, 1,
+ (j == k - 1) ? resident_bytes - off : per);
+ }
+
+ /* Ready: the charge is in place and spread across K CPUs. */
+ if (write(charge_ready[1], "x", 1) != 1)
+ _exit(4);
+ close(charge_ready[1]);
+
+ /* Hold the charge (region stays mapped) until the parent tells
+ * us to exit by closing the control pipe.
+ */
+ while (read(charge_ctrl[0], &c, 1) > 0)
+ ;
+
+ munmap(region, resident_bytes);
+ _exit(0);
+}
+
+/*
+ * Fork one charging child per leaf, each spread over K CPUs. Returns 0 once
+ * every child has charged its leaf and is holding the charge, so the tree is
+ * under a steady, quiesced load. On failure the caller's cleanup path calls
+ * stop_chargers().
+ */
+static int start_chargers(int cpus_per_leaf, size_t resident_bytes)
+{
+ int k_eff, i, h = 0;
+
+ k_eff = (cpus_per_leaf > 0 && cpus_per_leaf <= n_cpu) ? cpus_per_leaf :
+ n_cpu;
+
+ if (pipe(charge_ready) || pipe(charge_ctrl)) {
+ ksft_print_msg("pipe: %s\n", strerror(errno));
+ return -1;
+ }
+
+ charger_pids = calloc(n_leaves, sizeof(*charger_pids));
+ if (!charger_pids) {
+ ksft_print_msg("calloc charger_pids failed\n");
+ return -1;
+ }
+
+ /* recorded before the fork so each child can PR_SET_PDEATHSIG against us */
+ test_pid = getpid();
+
+ for (i = 0; i < n_nodes; i++) {
+ pid_t pid;
+
+ if (!nodes[i].is_leaf)
+ continue;
+
+ pid = fork();
+ if (pid < 0) {
+ ksft_print_msg("fork charger: %s\n", strerror(errno));
+ return -1;
+ }
+ if (pid == 0)
+ charger_child(&nodes[i], h * k_eff, k_eff,
+ resident_bytes);
+
+ charger_pids[n_chargers++] = pid;
+ h++;
+ }
+
+ /* parent: keeps only the ready-read end and the ctrl-write end */
+ close(charge_ready[1]);
+ charge_ready[1] = -1;
+ close(charge_ctrl[0]);
+ charge_ctrl[0] = -1;
+
+ /* wait until every child has charged its leaf and is holding it */
+ for (i = 0; i < n_chargers; i++) {
+ char c;
+ ssize_t r = read(charge_ready[0], &c, 1);
+
+ if (r != 1) {
+ ksft_print_msg("charger exited before ready (setup failed?)\n");
+ return -1;
+ }
+ }
+ return 0;
+}
+
+static void stop_chargers(void)
+{
+ int i, status;
+
+ /* closing the ctrl write end unblocks every child -> they munmap + exit */
+ if (charge_ctrl[1] >= 0) {
+ close(charge_ctrl[1]);
+ charge_ctrl[1] = -1;
+ }
+ if (charge_ctrl[0] >= 0) {
+ close(charge_ctrl[0]);
+ charge_ctrl[0] = -1;
+ }
+ if (charge_ready[0] >= 0) {
+ close(charge_ready[0]);
+ charge_ready[0] = -1;
+ }
+ if (charge_ready[1] >= 0) {
+ close(charge_ready[1]);
+ charge_ready[1] = -1;
+ }
+
+ for (i = 0; i < n_chargers; i++) {
+ if (!charger_pids || charger_pids[i] <= 0)
+ continue;
+ if (waitpid(charger_pids[i], &status, 0) == charger_pids[i] &&
+ (!WIFEXITED(status) || WEXITSTATUS(status) != 0))
+ ksft_print_msg("charger %d exited abnormally (status=0x%x)\n",
+ charger_pids[i], status);
+ }
+
+ free(charger_pids);
+ charger_pids = NULL;
+ n_chargers = 0;
+}
+
+/* ---- file (traditional) reader ----------------------------------------- */
+
+static void parse_stat(char *buf, struct file_snap *o)
+{
+ char *save, *line;
+
+ for (line = strtok_r(buf, "\n", &save); line;
+ line = strtok_r(NULL, "\n", &save)) {
+ unsigned long long val;
+ char name[64];
+
+ if (sscanf(line, "%63s %llu", name, &val) != 2)
+ continue;
+ if (!strcmp(name, "anon"))
+ o->anon = val;
+ else if (!strcmp(name, "file"))
+ o->file = val;
+ else if (!strcmp(name, "shmem"))
+ o->shmem = val;
+ else if (!strcmp(name, "file_mapped"))
+ o->file_mapped = val;
+ else if (!strcmp(name, "pgfault"))
+ o->pgfault = val;
+ }
+}
+
+static int file_read_node(const char *path, struct file_snap *o)
+{
+ char buf[8192];
+
+ memset(o, 0, sizeof(*o));
+
+ if (cg_read(path, "memory.stat", buf, sizeof(buf)))
+ return -1;
+ parse_stat(buf, o);
+
+ if (!cg_read(path, "memory.current", buf, sizeof(buf)))
+ o->current = strtoull(buf, NULL, 10);
+ if (!cg_read(path, "memory.max", buf, sizeof(buf))) {
+ if (!strncmp(buf, "max", 3))
+ o->max_is_max = true;
+ else
+ o->max = strtoull(buf, NULL, 10);
+ }
+ return 0;
+}
+
+/* ---- BPF reader -------------------------------------------------------- */
+
+static int bpf_walk_once(struct bpf_link *link)
+{
+ char buf[4096];
+ ssize_t r;
+ int fd;
+
+ fd = bpf_iter_create(bpf_link__fd(link));
+ if (fd < 0)
+ return -1;
+ while ((r = read(fd, buf, sizeof(buf))) > 0)
+ ;
+ close(fd);
+ return r == 0 ? 0 : -1;
+}
+
+/* ---- correctness comparison -------------------------------------------- */
+
+static bool close_enough(__u64 a, __u64 b, __u64 tol)
+{
+ return (a > b ? a - b : b - a) <= tol;
+}
+
+/* Dump one node's bpf-vs-file stats; called when a mismatch is detected. */
+static void dump_node(int i, const struct memcg_stat_snapshot *b,
+ const struct file_snap *f)
+{
+ ksft_print_msg("node %d bpf : anon=%llu file=%llu shmem=%llu fmapped=%llu pgfault=%llu\n",
+ i, b->anon, b->file, b->shmem, b->file_mapped, b->pgfault);
+ ksft_print_msg("node %d file: anon=%llu file=%llu shmem=%llu fmapped=%llu pgfault=%llu\n",
+ i, f->anon, f->file, f->shmem, f->file_mapped, f->pgfault);
+}
+
+/*
+ * Compare the BPF kfunc snapshots (round 1) against the memory.stat values
+ * (round 2), node by node. The two rounds are independent, equivalently
+ * charged trees, so the flushed stats are compared within a small tolerance
+ * that absorbs per-round overhead (i.e., a charging child's own stack pages).
+ * A wrong unit, enum or field in the kfunc path would miss by far more.
+ *
+ * Two per-round checks verify the flush itself: each leaf was charged
+ * resident_bytes of anon spread over K CPUs, so its flushed anon must be at
+ * least that; and the root's recursive anon must equal the sum of the leaves'
+ * anon (rstat propagated the charge up the tree).
+ *
+ * Returns 0 if every check passes, -1 otherwise.
+ */
+static int check_correctness(const struct memcg_stat_snapshot *bpf,
+ const struct file_snap *file, const bool *is_leaf,
+ int n, size_t resident_bytes)
+{
+ __u64 stat_tol = 64 * page_size;
+ __u64 pgf_tol = 1024;
+ __u64 broot = 0, bsum = 0, froot = 0, fsum = 0;
+ int i, mism = 0, flush_bad = 0;
+
+ for (i = 0; i < n; i++) {
+ const struct memcg_stat_snapshot *b = &bpf[i];
+ const struct file_snap *f = &file[i];
+ __u64 bcur = b->usage_pages * page_size;
+
+ /* kfunc path (round 1) compared with memory.stat path (round 2) */
+ if (!close_enough(b->anon, f->anon, stat_tol) ||
+ !close_enough(b->file, f->file, stat_tol) ||
+ !close_enough(b->shmem, f->shmem, stat_tol) ||
+ !close_enough(b->file_mapped, f->file_mapped, stat_tol) ||
+ !close_enough(b->pgfault, f->pgfault, pgf_tol)) {
+ mism++;
+ dump_node(i, b, f);
+ }
+
+ /* memory.current is live (no flush) and must bound the flushed anon */
+ if (b->anon == 0 || b->anon > bcur) {
+ flush_bad++;
+ ksft_print_msg("node %d: anon=%llu exceeds current=%llu\n",
+ i, b->anon, bcur);
+ }
+
+ /* each leaf's flush must have gathered the full cross-CPU charge */
+ if (is_leaf[i]) {
+ if (b->anon < resident_bytes || f->anon < resident_bytes) {
+ flush_bad++;
+ ksft_print_msg("node %d: short flush bpf=%llu file=%llu\n",
+ i, b->anon, f->anon);
+ }
+ bsum += b->anon;
+ fsum += f->anon;
+ }
+ if (i == 0) { /* nodes[0] == subtree_root */
+ broot = b->anon;
+ froot = f->anon;
+ }
+ }
+
+ if (mism) {
+ ksft_print_msg("bpf (round 1) disagrees with memory.stat (round 2)\n");
+ return -1;
+ }
+ if (flush_bad) {
+ ksft_print_msg("flush did not aggregate the cross-cpu charge\n");
+ return -1;
+ }
+ if (broot != bsum || froot != fsum) {
+ ksft_print_msg("root anon != sum of leaf anon: bpf %llu/%llu file %llu/%llu\n",
+ broot, bsum, froot, fsum);
+ return -1;
+ }
+ if (bsum == 0) {
+ ksft_print_msg("tree carries no anon\n");
+ return -1;
+ }
+ return 0;
+}
+
+/* ---- one case ---------------------------------------------------------- */
+
+struct testcase {
+ const char *name;
+ int fanout;
+ int depth;
+ int cpus_per_leaf; /* K: CPUs each leaf is charged on; 0 = all CPUs */
+ size_t resident_bytes; /* anon charged per leaf */
+};
+
+/*
+ * Remove the subtree in reverse creation order. Nodes are recorded in DFS
+ * order (a parent precedes all its descendants), so iterating backwards
+ * removes every child before its parent.
+ */
+static void destroy_tree(void)
+{
+ int i;
+
+ if (!nodes)
+ return;
+ for (i = n_nodes - 1; i >= 0; i--)
+ cg_destroy(nodes[i].path);
+ free(nodes);
+ nodes = NULL;
+}
+
+/*
+ * Round 1: build and charge a fresh tree, walk it with the BPF iterator (which
+ * flushes and reads each cgroup via the memcg kfuncs), and capture one snapshot
+ * per node into @snap. @is_leaf records the tree shape so the later comparison
+ * can run after the tree is gone. Returns the node count, or -1 on failure.
+ * The tree is always torn down before returning.
+ */
+static int capture_bpf_round(const struct testcase *tc,
+ struct memcg_stat_snapshot *snap, bool *is_leaf)
+{
+ struct memcg_stat_cross_cpu *skel = NULL;
+ struct bpf_link *link = NULL;
+ int root_fd = -1, ret = -1, i, mfd;
+
+ if (build_tree(tc->fanout, tc->depth, &root_fd)) {
+ ksft_print_msg("build tree (bpf) failed\n");
+ goto out;
+ }
+ if (start_chargers(tc->cpus_per_leaf, tc->resident_bytes))
+ goto out;
+
+ skel = memcg_stat_cross_cpu__open();
+ if (!skel) {
+ ksft_print_msg("skel open failed\n");
+ goto out;
+ }
+ if (bpf_map__set_max_entries(skel->maps.results, n_nodes + 8)) {
+ ksft_print_msg("set max_entries failed\n");
+ goto out;
+ }
+ if (memcg_stat_cross_cpu__load(skel)) {
+ ksft_print_msg("skel load failed\n");
+ goto out;
+ }
+
+ DECLARE_LIBBPF_OPTS(bpf_iter_attach_opts, opts);
+ union bpf_iter_link_info linfo = {};
+
+ linfo.cgroup.cgroup_fd = root_fd;
+ linfo.cgroup.order = BPF_CGROUP_ITER_DESCENDANTS_PRE;
+ opts.link_info = &linfo;
+ opts.link_info_len = sizeof(linfo);
+
+ link = bpf_program__attach_iter(skel->progs.cgroup_memcg_stat_cross_cpu,
+ &opts);
+ if (!link) {
+ ksft_print_msg("attach iter failed\n");
+ goto out;
+ }
+
+ /* bpf walk through the cgroup tree and fetch result */
+ if (bpf_walk_once(link)) {
+ ksft_print_msg("bpf walk failed\n");
+ goto out;
+ }
+
+ mfd = bpf_map__fd(skel->maps.results);
+ for (i = 0; i < n_nodes; i++) {
+ /* Save the position for the leaf, used for later correctness check */
+ is_leaf[i] = nodes[i].is_leaf;
+ if (bpf_map_lookup_elem(mfd, &nodes[i].id, &snap[i])) {
+ ksft_print_msg("map lookup failed for node %d\n", i);
+ goto out;
+ }
+ }
+ ret = n_nodes;
+out:
+ bpf_link__destroy(link);
+ memcg_stat_cross_cpu__destroy(skel);
+ if (root_fd >= 0)
+ close(root_fd);
+ stop_chargers();
+ destroy_tree();
+ return ret;
+}
+
+/*
+ * Round 2: build and charge an identical fresh tree, then read every cgroup via
+ * memory.stat / memory.current. The tree is reset after bpf read, so each
+ * read does a real rstat flush. Returns the node count, or -1 on failure.
+ * The tree is always clear before returning.
+ */
+static int capture_file_round(const struct testcase *tc, struct file_snap *snap)
+{
+ int root_fd = -1, ret = -1, i;
+
+ if (build_tree(tc->fanout, tc->depth, &root_fd)) {
+ ksft_print_msg("build tree (file) failed\n");
+ goto out;
+ }
+ if (start_chargers(tc->cpus_per_leaf, tc->resident_bytes))
+ goto out;
+
+ for (i = 0; i < n_nodes; i++)
+ if (file_read_node(nodes[i].path, &snap[i])) {
+ ksft_print_msg("file read failed for node %d\n", i);
+ goto out;
+ }
+ ret = n_nodes;
+out:
+ if (root_fd >= 0)
+ close(root_fd);
+ stop_chargers();
+ destroy_tree();
+ return ret;
+}
+
+static int run_case(const struct testcase *tc)
+{
+ struct memcg_stat_snapshot *bpf = NULL;
+ struct file_snap *file = NULL;
+ bool *is_leaf = NULL;
+ size_t cap = tree_capacity(tc->fanout, tc->depth);
+ int nb, nf, ret = KSFT_FAIL;
+
+ bpf = calloc(cap, sizeof(*bpf));
+ file = calloc(cap, sizeof(*file));
+ is_leaf = calloc(cap, sizeof(*is_leaf));
+ if (!bpf || !file || !is_leaf) {
+ ksft_print_msg("calloc failed\n");
+ goto out;
+ }
+
+ ksft_print_msg("%s: fanout=%d depth=%d cpus=%d/%d resident=%zuKB/leaf\n",
+ tc->name, tc->fanout, tc->depth, n_cpu, tc->cpus_per_leaf,
+ tc->resident_bytes >> 10);
+
+ /* round 1: BPF reader flushes and reads its own charged tree */
+ nb = capture_bpf_round(tc, bpf, is_leaf);
+ if (nb < 0)
+ goto out;
+
+ /* round 2: file reader flushes and reads a fresh, equivalent tree */
+ nf = capture_file_round(tc, file);
+ if (nf < 0)
+ goto out;
+
+ if (nb != nf) {
+ ksft_print_msg("node count differs between rounds: %d vs %d\n",
+ nb, nf);
+ goto out;
+ }
+
+ if (!check_correctness(bpf, file, is_leaf, nb, tc->resident_bytes))
+ ret = KSFT_PASS;
+out:
+ free(bpf);
+ free(file);
+ free(is_leaf);
+ return ret;
+}
+
+static const struct testcase cases[] = {
+ /* name, fan, depth, K, resident anon */
+ { "single_cpu_small_tree", 4, 2, 1, 2 << 20 },
+ { "cross_cpu_small_tree", 4, 2, 0, 2 << 20 },
+ { "single_cpu_large_tree", 10, 3, 1, 256 << 10 },
+ { "cross_cpu_large_tree", 10, 3, 0, 256 << 10 },
+};
+
+static bool memcg_kfuncs_available(void)
+{
+ struct btf *btf;
+ bool ok;
+
+ btf = btf__load_vmlinux_btf();
+ if (!btf)
+ return false;
+ ok = btf__find_by_name_kind(btf, "bpf_get_mem_cgroup", BTF_KIND_FUNC) > 0;
+ btf__free(btf);
+ return ok;
+}
+
+int main(int argc, char **argv)
+{
+ int i;
+
+ ksft_print_header();
+ ksft_set_plan(ARRAY_SIZE(cases));
+
+ /* Feature gate first: a read-only BTF probe, no privilege needed. */
+ if (!memcg_kfuncs_available())
+ ksft_exit_skip("memcg BPF kfuncs are not available\n");
+
+ if (cg_find_unified_root(root, sizeof(root), NULL))
+ ksft_exit_skip("cgroup v2 isn't mounted\n");
+
+ if (cg_read_strstr(root, "cgroup.controllers", "memory"))
+ ksft_exit_skip("memory controller isn't available\n");
+
+ if (cg_read_strstr(root, "cgroup.subtree_control", "memory"))
+ if (cg_write(root, "cgroup.subtree_control", "+memory"))
+ ksft_exit_skip("Failed to set memory controller\n");
+
+ if (collect_cpus())
+ ksft_exit_skip("cannot read CPU affinity\n");
+
+ page_size = sysconf(_SC_PAGESIZE);
+ subtree_root = cg_name(root, SUBTREE_NAME);
+ if (!subtree_root)
+ ksft_exit_skip("cannot build subtree root path\n");
+
+ for (i = 0; i < ARRAY_SIZE(cases); i++) {
+ switch (run_case(&cases[i])) {
+ case KSFT_PASS:
+ ksft_test_result_pass("%s\n", cases[i].name);
+ break;
+ case KSFT_SKIP:
+ ksft_test_result_skip("%s\n", cases[i].name);
+ break;
+ default:
+ ksft_test_result_fail("%s\n", cases[i].name);
+ break;
+ }
+ }
+
+ free(cpu_list);
+ cpu_list = NULL;
+ n_cpu = 0;
+
+ ksft_finished();
+}
--
2.53.0-Meta
^ permalink raw reply related
* [PATCH v2 3/4] selftests/hid: build the BPF program via the shared lib.bpf.mk
From: Ziyang Men @ 2026-07-21 17:48 UTC (permalink / raw)
To: Shuah Khan, Tejun Heo, Johannes Weiner, Michal Koutný,
Jiri Kosina, Benjamin Tissoires, David Vernet, Eduard Zingerman
Cc: Andrea Righi, Changwoo Min, Michal Hocko, Roman Gushchin,
Shakeel Butt, Muchun Song, Andrew Morton, JP Kobryn,
Mykola Lysenko, Nathan Chancellor, linux-kselftest, cgroups,
linux-input, sched-ext, linux-mm, kernel-team, bpf, llvm,
linux-kernel, Ziyang Men
In-Reply-To: <20260721174833.1232771-1-ziyang.meme@gmail.com>
hid carries its own ~150 lines of libbpf + bpftool + vmlinux.h +
BPF-object + skeleton build machinery, copied from selftests/bpf.
Replace it with an include of the shared
tools/testing/selftests/lib.bpf.mk, so that the previous ~150 lines of
BPF build configuration can now be achieved in only ~10 lines.
hid keeps the legacy progs/<name>.c layout, so it sets BPF_PROG_EXT := .c
and passes its shared BPF headers through BPF_EXTRA_HDRS. It also gains
the fragment's -Wall on the BPF compile, which its old Makefile did not
set; hid.c has a few pre-existing unused locals, so
BPF_EXTRA_CFLAGS := -Wno-unused-variable keeps the previous behaviour
without touching the program source.
The conversion drops two pieces of dead machinery hid had copied from
selftests/bpf: a resolve_btfids build that was never a prerequisite of
any target, and a CLANG_CFLAGS variable that was never passed to any
compile.
No functional change: the generated hid.skel.h public API is
byte-identical before and after, hid_bpf and hidraw are built and linked
the same way, and the folder builds cleanly under both plain make and
LLVM=1.
Suggested-by: Shakeel Butt <shakeel.butt@linux.dev>
Suggested-by: Eduard Zingerman <eddyz87@gmail.com>
Suggested-by: Mykola Lysenko <mykolal@meta.com>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Ziyang Men <ziyang.meme@gmail.com>
---
tools/testing/selftests/hid/Makefile | 182 +++------------------------
1 file changed, 20 insertions(+), 162 deletions(-)
diff --git a/tools/testing/selftests/hid/Makefile b/tools/testing/selftests/hid/Makefile
index 2f423de83147..48def484703c 100644
--- a/tools/testing/selftests/hid/Makefile
+++ b/tools/testing/selftests/hid/Makefile
@@ -58,172 +58,31 @@ override define CLEAN
$(Q)$(RM) -r $(EXTRA_CLEAN)
endef
-include ../lib.mk
+# The BPF program (progs/hid.c) is compiled into a skeleton by the shared
+# tools/testing/selftests/lib.bpf.mk fragment. hid keeps the legacy
+# progs/<name>.c layout, so point BPF_PROG_EXT at .c and hand the fragment
+# hid's shared BPF headers as extra prerequisites. BPF_EXTRA_HDRS is deferred
+# because it references $(BPFDIR), which lib.bpf.mk defines.
+BPF_SRCS := progs/hid.c
+BPF_PROG_EXT := .c
+BPF_EXTRA_HDRS = $(wildcard progs/*.h) $(wildcard $(BPFDIR)/hid_bpf_*.h) \
+ $(wildcard $(BPFDIR)/*.bpf.h)
+# hid.c predates the shared -Wall (hid's old Makefile didn't set it) and has a
+# few unused locals; tolerate that pre-existing warning without touching the
+# program source.
+BPF_EXTRA_CFLAGS := -Wno-unused-variable
-TOOLSDIR := $(top_srcdir)/tools
-LIBDIR := $(TOOLSDIR)/lib
-BPFDIR := $(LIBDIR)/bpf
-TOOLSINCDIR := $(TOOLSDIR)/include
-BPFTOOLDIR := $(TOOLSDIR)/bpf/bpftool
-SCRATCH_DIR := $(OUTPUT)/tools
-BUILD_DIR := $(SCRATCH_DIR)/build
-INCLUDE_DIR := $(SCRATCH_DIR)/include
-BPFOBJ := $(BUILD_DIR)/libbpf/libbpf.a
-ifneq ($(CROSS_COMPILE),)
-HOST_BUILD_DIR := $(BUILD_DIR)/host
-HOST_SCRATCH_DIR := $(OUTPUT)/host-tools
-HOST_INCLUDE_DIR := $(HOST_SCRATCH_DIR)/include
-else
-HOST_BUILD_DIR := $(BUILD_DIR)
-HOST_SCRATCH_DIR := $(SCRATCH_DIR)
-HOST_INCLUDE_DIR := $(INCLUDE_DIR)
-endif
-HOST_BPFOBJ := $(HOST_BUILD_DIR)/libbpf/libbpf.a
-RESOLVE_BTFIDS := $(HOST_BUILD_DIR)/resolve_btfids/resolve_btfids
-
-VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux) \
- $(if $(KBUILD_OUTPUT),$(KBUILD_OUTPUT)/vmlinux) \
- ../../../../vmlinux \
- /sys/kernel/btf/vmlinux \
- /boot/vmlinux-$(shell uname -r)
-VMLINUX_BTF ?= $(abspath $(firstword $(wildcard $(VMLINUX_BTF_PATHS))))
-ifeq ($(VMLINUX_BTF),)
-$(error Cannot find a vmlinux for VMLINUX_BTF at any of "$(VMLINUX_BTF_PATHS)")
-endif
+include ../lib.mk
+include ../lib.bpf.mk
-# Define simple and short `make test_progs`, `make test_sysctl`, etc targets
-# to build individual tests.
+# Define simple and short `make hid_bpf`, `make hidraw` targets.
# NOTE: Semicolon at the end is critical to override lib.mk's default static
# rule for binaries.
$(notdir $(TEST_GEN_PROGS)): %: $(OUTPUT)/% ;
-# sort removes libbpf duplicates when not cross-building
-MAKE_DIRS := $(sort $(BUILD_DIR)/libbpf $(HOST_BUILD_DIR)/libbpf \
- $(HOST_BUILD_DIR)/bpftool $(HOST_BUILD_DIR)/resolve_btfids \
- $(INCLUDE_DIR))
-$(MAKE_DIRS):
- $(call msg,MKDIR,,$@)
- $(Q)mkdir -p $@
-
-DEFAULT_BPFTOOL := $(HOST_SCRATCH_DIR)/sbin/bpftool
-
-TEST_GEN_PROGS_EXTENDED += $(DEFAULT_BPFTOOL)
-
-$(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED): $(BPFOBJ)
-
-BPFTOOL ?= $(DEFAULT_BPFTOOL)
-$(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
- $(HOST_BPFOBJ) | $(HOST_BUILD_DIR)/bpftool
- $(Q)$(MAKE) $(submake_extras) -C $(BPFTOOLDIR) \
- ARCH= CROSS_COMPILE= CC=$(HOSTCC) LD=$(HOSTLD) \
- EXTRA_CFLAGS='-g -O0' \
- OUTPUT=$(HOST_BUILD_DIR)/bpftool/ \
- LIBBPF_OUTPUT=$(HOST_BUILD_DIR)/libbpf/ \
- LIBBPF_DESTDIR=$(HOST_SCRATCH_DIR)/ \
- prefix= DESTDIR=$(HOST_SCRATCH_DIR)/ install-bin
-
-$(BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile) \
- | $(BUILD_DIR)/libbpf
- $(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) OUTPUT=$(BUILD_DIR)/libbpf/ \
- EXTRA_CFLAGS='-g -O0' \
- DESTDIR=$(SCRATCH_DIR) prefix= all install_headers
-
-ifneq ($(BPFOBJ),$(HOST_BPFOBJ))
-$(HOST_BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile) \
- | $(HOST_BUILD_DIR)/libbpf
- $(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) \
- EXTRA_CFLAGS='-g -O0' ARCH= CROSS_COMPILE= \
- OUTPUT=$(HOST_BUILD_DIR)/libbpf/ CC=$(HOSTCC) LD=$(HOSTLD) \
- DESTDIR=$(HOST_SCRATCH_DIR)/ prefix= all install_headers
-endif
-
-$(INCLUDE_DIR)/vmlinux.h: $(VMLINUX_BTF) $(BPFTOOL) | $(INCLUDE_DIR)
-ifeq ($(VMLINUX_H),)
- $(call msg,GEN,,$@)
- $(Q)$(BPFTOOL) btf dump file $(VMLINUX_BTF) format c > $@
-else
- $(call msg,CP,,$@)
- $(Q)cp "$(VMLINUX_H)" $@
-endif
-
-$(RESOLVE_BTFIDS): $(HOST_BPFOBJ) | $(HOST_BUILD_DIR)/resolve_btfids \
- $(TOOLSDIR)/bpf/resolve_btfids/main.c \
- $(TOOLSDIR)/lib/rbtree.c \
- $(TOOLSDIR)/lib/zalloc.c \
- $(TOOLSDIR)/lib/string.c \
- $(TOOLSDIR)/lib/ctype.c \
- $(TOOLSDIR)/lib/str_error_r.c
- $(Q)$(MAKE) $(submake_extras) -C $(TOOLSDIR)/bpf/resolve_btfids \
- CC=$(HOSTCC) LD=$(HOSTLD) AR=$(HOSTAR) \
- LIBBPF_INCLUDE=$(HOST_INCLUDE_DIR) \
- OUTPUT=$(HOST_BUILD_DIR)/resolve_btfids/ BPFOBJ=$(HOST_BPFOBJ)
-
-# Get Clang's default includes on this system, as opposed to those seen by
-# '--target=bpf'. This fixes "missing" files on some architectures/distros,
-# such as asm/byteorder.h, asm/socket.h, asm/sockios.h, sys/cdefs.h etc.
-#
-# Use '-idirafter': Don't interfere with include mechanics except where the
-# build would have failed anyways.
-define get_sys_includes
-$(shell $(1) -v -E - </dev/null 2>&1 \
- | sed -n '/<...> search starts here:/,/End of search list./{ s| \(/.*\)|-idirafter \1|p }') \
-$(shell $(1) -dM -E - </dev/null | grep '__riscv_xlen ' | awk '{printf("-D__riscv_xlen=%d -D__BITS_PER_LONG=%d", $$3, $$3)}')
-endef
-
-# Determine target endianness.
-IS_LITTLE_ENDIAN = $(shell $(CC) -dM -E - </dev/null | \
- grep 'define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__')
-MENDIAN=$(if $(IS_LITTLE_ENDIAN),-mlittle-endian,-mbig-endian)
-
-CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG))
-BPF_CFLAGS = -g -Werror -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN) \
- -Wno-microsoft-anon-tag \
- -fms-extensions \
- -I$(INCLUDE_DIR)
-
-CLANG_CFLAGS = $(CLANG_SYS_INCLUDES) \
- -Wno-compare-distinct-pointer-types
-
-# Build BPF object using Clang
-# $1 - input .c file
-# $2 - output .o file
-# $3 - CFLAGS
-define CLANG_BPF_BUILD_RULE
- $(call msg,CLNG-BPF,$(TRUNNER_BINARY),$2)
- $(Q)$(CLANG) $3 -O2 --target=bpf -c $1 -mcpu=v3 -o $2
-endef
-# Similar to CLANG_BPF_BUILD_RULE, but with disabled alu32
-define CLANG_NOALU32_BPF_BUILD_RULE
- $(call msg,CLNG-BPF,$(TRUNNER_BINARY),$2)
- $(Q)$(CLANG) $3 -O2 --target=bpf -c $1 -mcpu=v2 -o $2
-endef
-# Build BPF object using GCC
-define GCC_BPF_BUILD_RULE
- $(call msg,GCC-BPF,$(TRUNNER_BINARY),$2)
- $(Q)$(BPF_GCC) $3 -O2 -c $1 -o $2
-endef
-
-BPF_PROGS_DIR := progs
-BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE
-BPF_SRCS := $(notdir $(wildcard $(BPF_PROGS_DIR)/*.c))
-BPF_OBJS := $(patsubst %.c,$(OUTPUT)/%.bpf.o, $(BPF_SRCS))
-BPF_SKELS := $(patsubst %.c,$(OUTPUT)/%.skel.h, $(BPF_SRCS))
-TEST_GEN_FILES += $(BPF_OBJS)
-
-$(BPF_PROGS_DIR)-bpfobjs := y
-$(BPF_OBJS): $(OUTPUT)/%.bpf.o: \
- $(BPF_PROGS_DIR)/%.c \
- $(wildcard $(BPF_PROGS_DIR)/*.h) \
- $(INCLUDE_DIR)/vmlinux.h \
- $(wildcard $(BPFDIR)/hid_bpf_*.h) \
- $(wildcard $(BPFDIR)/*.bpf.h) \
- | $(OUTPUT) $(BPFOBJ)
- $(call $(BPF_BUILD_RULE),$<,$@, $(BPF_CFLAGS))
-
-$(BPF_SKELS): %.skel.h: %.bpf.o $(BPFTOOL) | $(OUTPUT)
- $(call msg,GEN-SKEL,$(BINARY),$@)
- $(Q)$(BPFTOOL) gen object $(<:.o=.linked1.o) $<
- $(Q)$(BPFTOOL) gen skeleton $(<:.o=.linked1.o) name $(notdir $(<:.bpf.o=)) > $@
+# Each test binary links against the in-tree static libbpf that lib.bpf.mk
+# built (lib.mk has already prefixed TEST_GEN_PROGS with $(OUTPUT)/).
+$(TEST_GEN_PROGS): $(BPFOBJ)
$(OUTPUT)/%.o: %.c $(BPF_SKELS) hid_common.h
$(call msg,CC,,$@)
@@ -233,5 +92,4 @@ $(OUTPUT)/%: $(OUTPUT)/%.o
$(call msg,BINARY,,$@)
$(Q)$(LINK.c) $^ $(LDLIBS) -o $@
-EXTRA_CLEAN := $(SCRATCH_DIR) $(HOST_SCRATCH_DIR) feature bpftool \
- $(addprefix $(OUTPUT)/,*.o *.skel.h no_alu32)
+EXTRA_CLEAN += feature bpftool $(addprefix $(OUTPUT)/,*.o no_alu32)
--
2.53.0-Meta
^ permalink raw reply related
* [PATCH v2 4/4] selftests/sched_ext: build BPF schedulers via the shared lib.bpf.mk
From: Ziyang Men @ 2026-07-21 17:48 UTC (permalink / raw)
To: Shuah Khan, Tejun Heo, Johannes Weiner, Michal Koutný,
Jiri Kosina, Benjamin Tissoires, David Vernet, Eduard Zingerman
Cc: Andrea Righi, Changwoo Min, Michal Hocko, Roman Gushchin,
Shakeel Butt, Muchun Song, Andrew Morton, JP Kobryn,
Mykola Lysenko, Nathan Chancellor, linux-kselftest, cgroups,
linux-input, sched-ext, linux-mm, kernel-team, bpf, llvm,
linux-kernel, Ziyang Men
In-Reply-To: <20260721174833.1232771-1-ziyang.meme@gmail.com>
sched_ext carried its own ~130 lines of libbpf + bpftool + vmlinux.h +
BPF-object + skeleton build machinery. Replace it with an include of the
shared tools/testing/selftests/lib.bpf.mk, making sched_ext the third
in-tree consumer of that fragment, after selftests/cgroup and
selftests/hid.
sched_ext emits a skeleton and a subskeleton per scheduler with a
.bpf.skel.h suffix, so it sets BPF_SKEL_EXT := .bpf.skel.h and
BPF_GEN_SUBSKEL := 1. Its BPF programs need scheduler-specific include
paths and are not -Werror clean, so it overrides BPF_CFLAGS wholesale
after the include; the userspace runner/testcase rules are kept and
repointed at the fragment's flat $(OUTPUT) skeletons.
No functional change: all 28 skeletons and 28 subskeletons keep a
byte-identical public API before and after, the runner builds and links
the same way, and the folder builds cleanly under both plain make and
LLVM=1.
Suggested-by: Shakeel Butt <shakeel.butt@linux.dev>
Suggested-by: Eduard Zingerman <eddyz87@gmail.com>
Suggested-by: Mykola Lysenko <mykolal@meta.com>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Ziyang Men <ziyang.meme@gmail.com>
---
tools/testing/selftests/sched_ext/Makefile | 153 +++++----------------
1 file changed, 35 insertions(+), 118 deletions(-)
diff --git a/tools/testing/selftests/sched_ext/Makefile b/tools/testing/selftests/sched_ext/Makefile
index 5d2dffca0e91..67ff96bf34e8 100644
--- a/tools/testing/selftests/sched_ext/Makefile
+++ b/tools/testing/selftests/sched_ext/Makefile
@@ -14,48 +14,36 @@ CURDIR := $(abspath .)
REPOROOT := $(abspath ../../../..)
TOOLSDIR := $(REPOROOT)/tools
LIBDIR := $(TOOLSDIR)/lib
-BPFDIR := $(LIBDIR)/bpf
TOOLSINCDIR := $(TOOLSDIR)/include
-BPFTOOLDIR := $(TOOLSDIR)/bpf/bpftool
APIDIR := $(TOOLSINCDIR)/uapi
GENDIR := $(REPOROOT)/include/generated
GENHDR := $(GENDIR)/autoconf.h
-SCXTOOLSDIR := $(TOOLSDIR)/sched_ext
SCXTOOLSINCDIR := $(TOOLSDIR)/sched_ext/include
-OUTPUT_DIR := $(OUTPUT)/build
-OBJ_DIR := $(OUTPUT_DIR)/obj
-INCLUDE_DIR := $(OUTPUT_DIR)/include
-BPFOBJ_DIR := $(OBJ_DIR)/libbpf
-SCXOBJ_DIR := $(OBJ_DIR)/sched_ext
-BPFOBJ := $(BPFOBJ_DIR)/libbpf.a
-LIBBPF_OUTPUT := $(OBJ_DIR)/libbpf/libbpf.a
-
-DEFAULT_BPFTOOL := $(OUTPUT_DIR)/host/sbin/bpftool
-HOST_OBJ_DIR := $(OBJ_DIR)/host/bpftool
-HOST_LIBBPF_OUTPUT := $(OBJ_DIR)/host/libbpf/
-HOST_LIBBPF_DESTDIR := $(OUTPUT_DIR)/host/
-HOST_DESTDIR := $(OUTPUT_DIR)/host/
-
-VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux) \
- $(if $(KBUILD_OUTPUT),$(KBUILD_OUTPUT)/vmlinux) \
- ../../../../vmlinux \
- /sys/kernel/btf/vmlinux \
- /boot/vmlinux-$(shell uname -r)
-VMLINUX_BTF ?= $(abspath $(firstword $(wildcard $(VMLINUX_BTF_PATHS))))
-ifeq ($(VMLINUX_BTF),)
-$(error Cannot find a vmlinux for VMLINUX_BTF at any of "$(VMLINUX_BTF_PATHS)")
-endif
-
-BPFTOOL ?= $(DEFAULT_BPFTOOL)
+# The BPF schedulers are compiled into skeletons + subskeletons by the shared
+# lib.bpf.mk fragment. They use the modern *.bpf.c layout with a .bpf.skel.h
+# suffix and need subskeletons.
+BPF_SRCS := $(wildcard *.bpf.c)
+BPF_SKEL_EXT := .bpf.skel.h
+BPF_GEN_SUBSKEL := 1
+BPF_EXTRA_HDRS = $(wildcard $(CURDIR)/include/scx/*.h) \
+ $(wildcard $(CURDIR)/include/*.h)
+# Keep generated files under build/ as before (skeletons in build/include,
+# objects in build/obj) rather than flat in the source dir. lib.bpf.mk creates
+# these dirs; SCXOBJ_DIR (userspace objects) reuses the BPF object dir.
+BPF_OBJ_DIR := $(OUTPUT)/build/obj/sched_ext
+BPF_SKEL_DIR := $(OUTPUT)/build/include
+SCXOBJ_DIR := $(BPF_OBJ_DIR)
+
+include ../lib.bpf.mk
ifneq ($(wildcard $(GENHDR)),)
GENFLAGS := -DHAVE_GENHDR
endif
CFLAGS += -g -O2 -rdynamic -pthread -Wall -Werror $(GENFLAGS) \
- -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR) \
- -I$(TOOLSINCDIR) -I$(APIDIR) -I$(CURDIR)/include -I$(SCXTOOLSINCDIR)
+ -I$(GENDIR) -I$(LIBDIR) -I$(TOOLSINCDIR) -I$(APIDIR) \
+ -I$(CURDIR)/include -I$(SCXTOOLSINCDIR)
# Silence some warnings when compiled with clang
ifneq ($(LLVM),)
@@ -64,102 +52,29 @@ endif
LDFLAGS = -lelf -lz -lpthread -lzstd
-IS_LITTLE_ENDIAN = $(shell $(CC) -dM -E - </dev/null | \
- grep 'define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__')
-
-# Get Clang's default includes on this system, as opposed to those seen by
-# '-target bpf'. This fixes "missing" files on some architectures/distros,
-# such as asm/byteorder.h, asm/socket.h, asm/sockios.h, sys/cdefs.h etc.
-#
-# Use '-idirafter': Don't interfere with include mechanics except where the
-# build would have failed anyways.
-define get_sys_includes
-$(shell $(1) $(2) -v -E - </dev/null 2>&1 \
- | sed -n '/<...> search starts here:/,/End of search list./{ s| \(/.*\)|-idirafter \1|p }') \
-$(shell $(1) $(2) -dM -E - </dev/null | grep '__riscv_xlen ' | awk '{printf("-D__riscv_xlen=%d -D__BITS_PER_LONG=%d", $$3, $$3)}')
-endef
-
-ifneq ($(CROSS_COMPILE),)
-CLANG_TARGET_ARCH = --target=$(notdir $(CROSS_COMPILE:%-=%))
-endif
-
-CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG),$(CLANG_TARGET_ARCH))
-
-BPF_CFLAGS = -g -D__TARGET_ARCH_$(SRCARCH) \
- $(if $(IS_LITTLE_ENDIAN),-mlittle-endian,-mbig-endian) \
+# The BPF schedulers need scheduler-specific includes and are not -Werror
+# clean, so override the shared BPF_CFLAGS wholesale (lib.bpf.mk expands it
+# lazily in the object recipe; -O2/--target/-mcpu are added by the rule).
+BPF_CFLAGS = -g -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN) \
-I$(CURDIR)/include -I$(CURDIR)/include/bpf-compat \
-I$(INCLUDE_DIR) -I$(APIDIR) -I$(SCXTOOLSINCDIR) \
-I$(REPOROOT)/include \
- $(CLANG_SYS_INCLUDES) \
+ $(CLANG_SYS_INCLUDES) \
-Wall -Wno-compare-distinct-pointer-types \
-Wno-incompatible-function-pointer-types \
-Wno-microsoft-anon-tag \
- -fms-extensions \
- -O2 -mcpu=v3
-
-# sort removes libbpf duplicates when not cross-building
-MAKE_DIRS := $(sort $(OBJ_DIR)/libbpf $(OBJ_DIR)/libbpf \
- $(OBJ_DIR)/bpftool $(OBJ_DIR)/resolve_btfids \
- $(HOST_OBJ_DIR) $(INCLUDE_DIR) $(SCXOBJ_DIR))
-
-$(MAKE_DIRS):
- $(call msg,MKDIR,,$@)
- $(Q)mkdir -p $@
-
-$(BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile) \
- $(APIDIR)/linux/bpf.h \
- | $(OBJ_DIR)/libbpf
- $(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) OUTPUT=$(OBJ_DIR)/libbpf/ \
- ARCH=$(ARCH) CC="$(CC)" CROSS_COMPILE=$(CROSS_COMPILE) \
- EXTRA_CFLAGS='-g -O0 -fPIC' \
- DESTDIR=$(OUTPUT_DIR) prefix= all install_headers
-
-$(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
- $(LIBBPF_OUTPUT) | $(HOST_OBJ_DIR)
- $(Q)$(MAKE) $(submake_extras) -C $(BPFTOOLDIR) \
- ARCH= CROSS_COMPILE= CC=$(HOSTCC) LD=$(HOSTLD) \
- EXTRA_CFLAGS='-g -O0' \
- OUTPUT=$(HOST_OBJ_DIR)/ \
- LIBBPF_OUTPUT=$(HOST_LIBBPF_OUTPUT) \
- LIBBPF_DESTDIR=$(HOST_LIBBPF_DESTDIR) \
- prefix= DESTDIR=$(HOST_DESTDIR) install-bin
-
-$(INCLUDE_DIR)/vmlinux.h: $(VMLINUX_BTF) $(BPFTOOL) | $(INCLUDE_DIR)
-ifeq ($(VMLINUX_H),)
- $(call msg,GEN,,$@)
- $(Q)$(BPFTOOL) btf dump file $(VMLINUX_BTF) format c > $@
-else
- $(call msg,CP,,$@)
- $(Q)cp "$(VMLINUX_H)" $@
-endif
+ -fms-extensions
-$(SCXOBJ_DIR)/%.bpf.o: %.bpf.c $(INCLUDE_DIR)/vmlinux.h | $(BPFOBJ) $(SCXOBJ_DIR)
- $(call msg,CLNG-BPF,,$(notdir $@))
- $(Q)$(CLANG) $(BPF_CFLAGS) -target bpf -c $< -o $@
-
-$(INCLUDE_DIR)/%.bpf.skel.h: $(SCXOBJ_DIR)/%.bpf.o $(INCLUDE_DIR)/vmlinux.h $(BPFTOOL) | $(INCLUDE_DIR)
- $(eval sched=$(notdir $@))
- $(call msg,GEN-SKEL,,$(sched))
- $(Q)$(BPFTOOL) gen object $(<:.o=.linked1.o) $<
- $(Q)$(BPFTOOL) gen object $(<:.o=.linked2.o) $(<:.o=.linked1.o)
- $(Q)$(BPFTOOL) gen object $(<:.o=.linked3.o) $(<:.o=.linked2.o)
- $(Q)diff $(<:.o=.linked2.o) $(<:.o=.linked3.o)
- $(Q)$(BPFTOOL) gen skeleton $(<:.o=.linked3.o) name $(subst .bpf.skel.h,,$(sched)) > $@
- $(Q)$(BPFTOOL) gen subskeleton $(<:.o=.linked3.o) name $(subst .bpf.skel.h,,$(sched)) > $(@:.skel.h=.subskel.h)
+EXTRA_CLEAN += $(OUTPUT)/build
################
# C schedulers #
################
-override define CLEAN
- rm -rf $(OUTPUT_DIR)
- rm -f $(TEST_GEN_PROGS)
-endef
-
-# Every testcase takes all of the BPF progs are dependencies by default. This
+# Every testcase takes all of the BPF progs as dependencies by default. This
# allows testcases to load any BPF scheduler, which is useful for testcases
# that don't need their own prog to run their test.
-all_test_bpfprogs := $(foreach prog,$(wildcard *.bpf.c),$(INCLUDE_DIR)/$(patsubst %.c,%.skel.h,$(prog)))
+all_test_bpfprogs := $(BPF_SKELS)
auto-test-targets := \
create_dsq \
@@ -195,7 +110,8 @@ auto-test-targets := \
testcase-targets := $(addsuffix .o,$(addprefix $(SCXOBJ_DIR)/,$(auto-test-targets)))
$(SCXOBJ_DIR)/runner.o: runner.c | $(SCXOBJ_DIR) $(BPFOBJ)
- $(CC) $(CFLAGS) -c $< -o $@
+ $(call msg,CC,,$@)
+ $(Q)$(CC) $(CFLAGS) -c $< -o $@
# Create all of the test targets object files, whose testcase objects will be
# registered into the runner in ELF constructors.
@@ -204,15 +120,16 @@ $(SCXOBJ_DIR)/runner.o: runner.c | $(SCXOBJ_DIR) $(BPFOBJ)
# compiling BPF object files only if one is present, as the wildcard Make
# function doesn't support using implicit rules otherwise.
$(testcase-targets): $(SCXOBJ_DIR)/%.o: %.c $(SCXOBJ_DIR)/runner.o $(all_test_bpfprogs) | $(SCXOBJ_DIR)
- $(eval test=$(patsubst %.o,%.c,$(notdir $@)))
- $(CC) $(CFLAGS) -c $< -o $@
+ $(call msg,CC,,$@)
+ $(Q)$(CC) $(CFLAGS) -c $< -o $@
$(SCXOBJ_DIR)/util.o: util.c | $(SCXOBJ_DIR)
- $(CC) $(CFLAGS) -c $< -o $@
+ $(call msg,CC,,$@)
+ $(Q)$(CC) $(CFLAGS) -c $< -o $@
$(OUTPUT)/runner: $(SCXOBJ_DIR)/runner.o $(SCXOBJ_DIR)/util.o $(BPFOBJ) $(testcase-targets)
- @echo "$(testcase-targets)"
- $(CC) $(CFLAGS) -o $@ $^ $(LDFLAGS)
+ $(call msg,BINARY,,$@)
+ $(Q)$(CC) $(CFLAGS) -o $@ $^ $(LDFLAGS)
.DEFAULT_GOAL := all
--
2.53.0-Meta
^ permalink raw reply related
* Re: [PATCH 0/2] asus vivobook tablet mode
From: Basavaraj Natikar @ 2026-07-21 17:52 UTC (permalink / raw)
To: Helge Bahmann, Jiri Kosina, linux-input; +Cc: Basavaraj Natikar
In-Reply-To: <5234335.GXAFRqVoOG@zephyr>
On 7/21/2026 7:58 PM, Helge Bahmann wrote:
> Hi Basavaraj,
>
> Here the two patches implementing the split hid/input
> gdriver for the tablet mode support. I mulled over what
> exactly to use as report descriptor in the hid driver, and
> there is no clean and easy solution -- I hope this is what
> you had in mind? I could not figure a way how to do
> a stand-alone input driver without doing any hid indirection,
Hi Helge,
Please find below the series adding SW_TABLET_MODE support via the AMD
Sensor Fusion Hub, covering all currently supported AMD convertibles
https://lore.kernel.org/all/20260721174422.3109166-1-Basavaraj.Natikar@amd.com/
Once this is merged, the ASUS VivoBook quirk can go on top of it.
Thanks,
--
Basavaraj
> as I simply could not find a way to bind to the physical
> dev twice.
>
> Please let me know in comments to the patches how you
> would like to proceed.
>
> Thanks!
> Helge
>
>
^ permalink raw reply
* Re: [PATCH 3/3] Input: misc: Add AMD SFH tablet-mode switch driver
From: sashiko-bot @ 2026-07-21 17:54 UTC (permalink / raw)
To: Basavaraj Natikar; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260721174422.3109166-4-Basavaraj.Natikar@amd.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The initial hardware state of the tablet mode switch is redundantly queried and synchronized after `input_register_device()` exposes the device to userspace, creating a race condition.
- [Low] Violation of input subsystem naming convention for error variables.
--
--- Patch [3]: [PATCH 3/3] Input: misc: Add AMD SFH tablet-mode switch driver ---
commit e88c97b2482f6ed434b346336b9c6f9164d73473
Author: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
Input: misc: Add AMD SFH tablet-mode switch driver
This commit introduces a new driver for AMD convertible laptops. It exposes
the SW_TABLET_MODE switch by polling the operating-mode sensor provided by
the AMD Sensor Fusion Hub.
> diff --git a/drivers/input/misc/amd_sfh_tabletmode.c b/drivers/input/misc/amd_sfh_tabletmode.c
> new file mode 100644
> index 0000000000000..f88edebbf35d5
> --- /dev/null
> +++ b/drivers/input/misc/amd_sfh_tabletmode.c
[ ... ]
> +static int sfh_tm_probe(struct auxiliary_device *auxdev,
> + const struct auxiliary_device_id *id)
> +{
> + struct amd_sfh_info info = {};
> + struct device *dev = &auxdev->dev;
> + struct input_dev *input;
> + int rc;
[Severity: Low]
This isn't a bug, but does using 'rc' instead of 'error' or 'err' violate
the input subsystem maintainer preference?
The input subsystem guidelines suggest using 'error' or 'err' for variables
that only hold negative error codes and 0 for success.
[ ... ]
> + rc = input_setup_polling(input, sfh_tm_poll);
> + if (rc)
> + return rc;
> + input_set_poll_interval(input, POLL_INTERVAL_MS);
> +
> + rc = input_register_device(input);
> + if (rc)
> + return rc;
> +
> + sfh_tm_poll(input);
[Severity: Medium]
Should sfh_tm_poll() be called before input_register_device()?
Calling it after registration exposes a race condition where userspace
components reacting to the device creation uevent might read the
SW_TABLET_MODE state before sfh_tm_poll() executes.
They will read the default uninitialized state (0), which may incorrectly
leave the keyboard and touchpad active if the device is actually folded into
tablet mode.
> +
> + return 0;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260721174422.3109166-1-Basavaraj.Natikar@amd.com?part=3
^ permalink raw reply
* Re: [PATCH v2 1/4] selftests: add shared lib.bpf.mk to build BPF progs and skeletons
From: sashiko-bot @ 2026-07-21 17:58 UTC (permalink / raw)
To: Ziyang Men
Cc: sched-ext, mkoutny, linux-input, dmitry.torokhov, bpf, hannes, tj
In-Reply-To: <20260721174833.1232771-2-ziyang.meme@gmail.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] The `bpf_link` macro drops `LDFLAGS` and `CPPFLAGS` when linking user-space test binaries.
- [Medium] Hardcoded `pkg-config` breaks target feature detection for `libzstd` when cross-compiling.
- [Medium] Missing compiler-generated dependency tracking for BPF objects leads to stale builds.
- [Medium] The documented caller contract omits `$(BPFOBJ)` as a prerequisite for test binaries, breaking incremental relinks.
--
--- Patch [1]: [PATCH v2 1/4] selftests: add shared lib.bpf.mk to build BPF progs and skeletons ---
commit 5037b1678e6c542784cb0b94b1da28ba597740a3
Author: Ziyang Men <ziyang.meme@gmail.com>
selftests: add shared lib.bpf.mk to build BPF progs and skeletons
This patch introduces tools/testing/selftests/lib.bpf.mk, a shared Makefile
fragment to standardize building BPF programs, skeletons, and the libbpf
toolchain. It provides a common configuration and a bpf_link macro for
test binaries to easily link against libbpf.
> diff --git a/tools/testing/selftests/lib.bpf.mk b/tools/testing/selftests/lib.bpf.mk
> new file mode 100644
> --- /dev/null
> +++ b/tools/testing/selftests/lib.bpf.mk
> @@ -0,0 +1,247 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Shared fragment for selftests that compile *.bpf.c into BPF objects +
> +# skeletons and link them into userspace test binaries, without each
> +# subsystem's Makefile re-implementing the libbpf/bpftool/vmlinux.h machinery.
> +#
> +# Caller contract (per-test Makefile):
> +#
> +# BPF_SRCS := foo.bpf.c bar.bpf.c
> +# TEST_GEN_PROGS := foo_test
> +# OVERRIDE_TARGETS := 1 # MUST be set before lib.mk
> +# include ../lib.mk # defines OUTPUT, CC, Q, msg,
> +# include ../lib.bpf.mk # selfdir, top_srcdir; honours OVERRIDE
> +#
> +# $(OUTPUT)/foo_test: foo_test.c $(BPF_SKELS)
> +# $(call bpf_link,$@,$<)
[Severity: Medium]
Should $(BPFOBJ) be included as a prerequisite in this caller contract
template? If a user follows this example and libbpf.a is rebuilt, make
will not relink the test binaries that depend on it.
[ ... ]
> +# Locate the sources wherever the caller keeps them (e.g. progs/).
> +vpath %$(BPF_PROG_EXT) $(sort $(dir $(BPF_SRCS)))
> +
> +$(BPF_OBJS): $(BPF_OBJ_DIR)/%.bpf.o: %$(BPF_PROG_EXT) $(BPF_EXTRA_HDRS) \
> + $(wildcard *.bpf.h) $(INCLUDE_DIR)/vmlinux.h | $(BPF_OBJ_DIR) $(BPFOBJ)
> + $(call BPF_BUILD_RULE,$<,$@)
[Severity: Medium]
Will relying on this wildcard approach fail to track included headers if they
are located alongside sources in subdirectories like progs/?
Should this use compiler-generated dependency tracking (-MD/-MMD) instead
so developers don't end up testing stale code?
[ ... ]
> +# Static libbpf.a first, then its deps. libbpf may pull in zstd (BTF decompress)
> +# only when built against it; link -lzstd only if libzstd is present.
> +BPF_LDLIBS := $(BPFOBJ) -lelf -lz
> +ifneq ($(shell pkg-config --exists libzstd 2>/dev/null && echo y),)
> +BPF_LDLIBS += -lzstd
> +endif
[Severity: Medium]
Can this hardcoded pkg-config break target feature detection when
cross-compiling?
This queries the host's pkg-config environment instead of the target's
sysroot, which might cause link errors if libzstd availability differs
between the host and target. Should it use $(PKG_CONFIG) instead?
[ ... ]
> +TEST_GEN_FILES += $(BPF_OBJS)
> +
> +# Link helper: $1 = output binary, $2 = test .c (skels are the target's deps).
> +define bpf_link
> + $(call msg,BINARY,,$1)
> + $(Q)$(CC) $(CFLAGS) $2 $(BPF_LDLIBS) $(LDLIBS) -o $1
> +endef
[Severity: Medium]
Does this compilation command drop $(LDFLAGS) and $(CPPFLAGS)?
These are normally included in lib.mk's default link rule. Skipping them
might ignore user-specified flags like static linking, RPATHs, or sanitizers.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260721174833.1232771-1-ziyang.meme@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH v2 2/4] selftests/cgroup: add memcg_stat_cross_cpu correctness test for flush
From: sashiko-bot @ 2026-07-21 18:01 UTC (permalink / raw)
To: Ziyang Men
Cc: sched-ext, tj, hannes, bpf, dmitry.torokhov, mkoutny, linux-input
In-Reply-To: <20260721174833.1232771-3-ziyang.meme@gmail.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [Medium] Cross-CPU charge scattering fails to trigger page faults on all K CPUs when resident_bytes / k < PAGE_SIZE.
- [Medium] Commit message contains inaccurate claims regarding the test framework and utility helpers.
- [Low] Multi-line comment does not follow the required BPF subsystem style.
Pre-existing issues:
- [High] Missing bounds checks for negative enum values in kfuncs bpf_mem_cgroup_vm_events and bpf_mem_cgroup_memory_events allow out-of-bounds array access.
--
--- Patch [2]: [PATCH v2 2/4] selftests/cgroup: add memcg_stat_cross_cpu correctness test for flush ---
commit c0bc72e5d098a9cdecf85dcaee9fd0bb8671e354
Author: Ziyang Men <ziyang.meme@gmail.com>
selftests/cgroup: add memcg_stat_cross_cpu correctness test for flush
This commit adds a memory-cgroup BPF selftest that verifies if the
memory-cgroup BPF kfuncs return values that match what userspace reads
from cgroupfs. It builds a cgroup tree, charges memory across multiple
CPUs, and compares the flushed kfunc stats to the traditional memory.stat
reads.
> Add a test_progs selftest that verifies the memory-cgroup BPF kfuncs
[ ... ]
> The traditional path reads memory.stat / memory.current through a new
> read_cgroup_file() helper added to cgroup_helpers (the read counterpart
> of write_cgroup_file).
[Severity: Medium]
The commit message states that a test_progs selftest was added and a new
read_cgroup_file() helper was introduced. However, the patch implements a
standalone kselftest via TEST_GEN_PROGS and relies on the pre-existing
cg_read() helper instead. Could the commit message be updated to accurately
reflect the implemented code?
[ ... ]
> diff --git a/tools/testing/selftests/cgroup/memcg_stat_cross_cpu.bpf.c b/tools/testing/selftests/cgroup/memcg_stat_cross_cpu.bpf.c
[ ... ]
> + snap.file = bpf_mem_cgroup_page_state(memcg, idx_file);
> + snap.shmem = bpf_mem_cgroup_page_state(memcg, idx_shmem);
> + snap.file_mapped = bpf_mem_cgroup_page_state(memcg, idx_fmapped);
> + snap.pgfault = bpf_mem_cgroup_vm_events(memcg, idx_pgfault);
[Severity: High]
This is a pre-existing issue, but the called kfuncs
bpf_mem_cgroup_vm_events() and bpf_mem_cgroup_memory_events() in
mm/bpf_memcontrol.c lack negative bounds checks for their enum arguments.
Since the BPF verifier does not enforce enum ranges, a BPF program could
pass a negative enum value which bypasses the upper-bound check (e.g.,
idx >= NR_VM_EVENT_ITEMS). This happens because the C code evaluates the
signed integer, leading to an out-of-bounds array access.
Can we add the required negative bounds checks to these kfuncs?
> +
> + /* page_counter fields need no kfunc; read them off the trusted ptr. */
[ ... ]
> diff --git a/tools/testing/selftests/cgroup/test_memcg_stat_cross_cpu.c b/tools/testing/selftests/cgroup/test_memcg_stat_cross_cpu.c
[ ... ]
> + /*
> + * Fault the region in K slices, each on a different CPU, so the charge
> + * for this leaf is scattered across K per-cpu rstat trees. A correct
> + * flush must gather all K slices.
> + */
> + per = resident_bytes / k;
> + for (j = 0; j < k; j++) {
> + off = (size_t)j * per;
> + if (pin_cpu(cpu_list[(base + j) % n_cpu]))
> + _exit(3);
> + memset(region + off, 1,
> + (j == k - 1) ? resident_bytes - off : per);
> + }
[Severity: Medium]
If the system has a large number of CPUs, can per (resident_bytes / k)
become smaller than the page size?
For example, in the cross_cpu_large_tree test case with 256KB
resident_bytes and >64 CPUs, multiple CPUs would be assigned offsets
within the same 4KB page. The first CPU writing to the page causes a page
fault, but subsequent CPUs writing to the same page will find it already
mapped and won't trigger faults, defeating the cross-CPU scattering
mechanism.
> +
> + /* Ready: the charge is in place and spread across K CPUs. */
> + if (write(charge_ready[1], "x", 1) != 1)
> + _exit(4);
> + close(charge_ready[1]);
> +
> + /* Hold the charge (region stays mapped) until the parent tells
> + * us to exit by closing the control pipe.
> + */
[Severity: Low]
Does this multi-line comment format match the BPF subsystem style? The BPF
subsystem requires multi-line comments to have the opening /* on its own
line, even in selftests.
> + while (read(charge_ctrl[0], &c, 1) > 0)
> + ;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260721174833.1232771-1-ziyang.meme@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH 1/3] HID: amd_sfh: Add accessor to read the operating-mode sensor
From: sashiko-bot @ 2026-07-21 18:04 UTC (permalink / raw)
To: Basavaraj Natikar; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260721174422.3109166-2-Basavaraj.Natikar@amd.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [High] The auxiliary device created for the tablet-mode switch is never destroyed, leaking memory and leaving dangling device registrations.
- [High] Reading the operating mode on a v1.0 device accesses a register that is corrupted by DMA addresses, returning invalid data.
- [Medium] The `amd_sfh_deinit_emp2_action` devres action is registered prematurely and unconditionally clears the global `emp2` pointer, potentially corrupting global state.
- [Medium] The global `emp2` pointer is published before asynchronous initialization completes, causing data races on dynamically populated structures.
Pre-existing issues:
- [High] Exported APIs accessing the global `emp2` pointer lack synchronization, leading to a potential use-after-free and unmapped MMIO access if the device is concurrently unbound.
--
--- Patch [1]: [PATCH 1/3] HID: amd_sfh: Add accessor to read the operating-mode sensor ---
commit 1243ba22a0e430d7c20b9d71ce0f64ee46332ff2
Author: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
HID: amd_sfh: Add accessor to read the operating-mode sensor
Allow other drivers to query the operating mode (laptop or tablet)
reported by the Sensor Fusion Hub. This is the interface used by the
tablet-mode switch driver to report the device posture to userspace.
> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_client.c b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
> index 96ae792beeb6c..0f5346d2174c2 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_client.c
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
[ ... ]
> @@ -383,3 +383,19 @@ int amd_sfh_hid_client_deinit(struct amd_mp2_dev *privdata)
>
> return 0;
> }
> +
> +bool amd_sfh_op_idx_enabled(struct amd_mp2_dev *mp2)
> +{
> + struct amdtp_cl_data *cl = mp2->cl_data;
> + int i;
> +
> + if (!cl)
> + return false;
> +
> + for (i = 0; i < cl->num_hid_devices; i++)
> + if (cl->sensor_idx[i] == op_idx &&
> + cl->sensor_sts[i] == SENSOR_ENABLED)
> + return true;
> +
> + return false;
> +}
[Severity: High]
Does enabling this operating mode index leak the auxiliary device?
When amd_sfh_op_idx_enabled() returns true, it appears to trigger the
creation of mp2->tm_auxdev in amd_sfh_maybe_register_tm():
drivers/hid/amd-sfh-hid/amd_sfh_pcie.c:amd_sfh_maybe_register_tm() {
...
present = mp2->sfh1_1_ops ? mp2->dev_en.is_sra_present
: amd_sfh_op_idx_enabled(mp2);
if (!present)
return;
mp2->tm_auxdev = auxiliary_device_create(&mp2->pdev->dev, KBUILD_MODNAME,
"tabletmode", NULL, 0);
...
}
However, amd_mp2_pci_remove() does not seem to destroy this auxiliary
device, which could leave a dangling sysfs registration and leak memory
if the device is unbound.
> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> index 4b81cebdc3359..a3672a9302c5d 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
[ ... ]
> @@ -471,6 +477,10 @@ static int amd_mp2_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
> if (rc)
> return rc;
>
> + rc = devm_add_action_or_reset(&pdev->dev, amd_sfh_deinit_emp2_action, NULL);
> + if (rc)
> + return rc;
> +
[Severity: Medium]
Could this unconditionally clear the global emp2 pointer on a probe failure?
If devm_add_action_or_reset() is called before the device successfully
assigns its privdata to emp2 (which happens later in probe), and a
subsequent probe step fails, the devres cleanup will fire.
Will this wipe the emp2 pointer via sfh_deinit_emp2() even if it is
currently owned by another active device in a multi-device scenario?
> privdata->sfh1_1_ops = (const struct amd_sfh1_1_ops *)id->driver_data;
> if (privdata->sfh1_1_ops) {
> if (boot_cpu_data.x86 >= 0x1A)
> @@ -480,6 +490,7 @@ static int amd_mp2_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
> if (rc)
> return rc;
>
> + sfh_set_emp2(privdata);
> schedule_work(&privdata->work);
> return 0;
> }
[Severity: Medium]
Is there a data race when publishing the device globally before the workqueue
finishes?
sfh_set_emp2() exposes the device to exported APIs like amd_get_sfh_info().
If that API is called concurrently, amd_sfh_op_idx_enabled() might locklessly
iterate over cl->num_hid_devices and cl->sensor_idx[] while they are still
being populated by the workqueue in amd_sfh_hid_client_init().
> diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
> index 837d59e7a6610..74b3d3382a82c 100644
> --- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
> +++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
[ ... ]
> @@ -81,12 +81,43 @@ void sfh_deinit_emp2(void)
> emp2 = NULL;
> }
>
> +void sfh_set_emp2(struct amd_mp2_dev *mp2)
> +{
> + emp2 = mp2;
> +}
> +
> +static int amd_sfh_op_mode_info(u32 *op_mode)
> +{
> + struct sfh_op_mode mode;
> + bool present;
> +
> + if (!op_mode)
> + return -EINVAL;
> + if (!emp2)
> + return -ENODEV;
[Severity: High]
This is a pre-existing issue, but does this lockless check of emp2 create a
race condition with device unbind?
If a privileged user unbinds the SFH PCI driver concurrently, the teardown
path unmaps the MMIO, frees privdata, and the devres action clears emp2.
Could the querying thread dereference emp2 after it has been freed if the
unbind happens immediately after this !emp2 check?
> +
> + present = emp2->sfh1_1_ops ? emp2->dev_en.is_sra_present
> + : amd_sfh_op_idx_enabled(emp2);
> + if (!present)
> + return -ENODEV;
> +
> + mode.val = readl(emp2->mmio + amd_get_c2p_val(emp2, 3));
[Severity: High]
Will this read corrupted data on v1.0 hardware?
On v1.0 hardware, amd_start_sensor() uses a 64-bit writeq() to AMD_C2P_MSG2
to program the sensor's DMA address:
drivers/hid/amd-sfh-hid/amd_sfh_pcie.c:amd_start_sensor() {
...
writeq(info.dma_address, privdata->mmio + AMD_C2P_MSG2);
...
}
This 64-bit write spans both AMD_C2P_MSG2 and AMD_C2P_MSG3. Since
amd_get_c2p_val(emp2, 3) accesses AMD_C2P_MSG3, will it return the upper
32 bits of a DMA address instead of the actual operating mode?
> + dev_dbg(&emp2->pdev->dev,
> + "op-mode: raw=0x%08x mode=%u lid=%u angle=%u ontable=%u devmode=%u\n",
> + mode.val, mode.op_mode.mode, mode.op_mode.lidstatus,
> + mode.op_mode.angle, mode.op_mode.ontablestate,
> + mode.op_mode.devicemode);
> + *op_mode = mode.op_mode.mode;
> +
> + return 0;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260721174422.3109166-1-Basavaraj.Natikar@amd.com?part=1
^ permalink raw reply
* [PATCH] Input: wm831x-ts - drain pen-down work at teardown
From: Fan Wu @ 2026-07-22 2:45 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, linux-kernel, Fan Wu, stable
The pen-down and data IRQ handlers queue pd_data_work to switch between the
two IRQs. The worker obtains wm831x_ts with container_of() and calls
enable_irq(). free_irq() synchronizes an IRQ handler, but does not drain a
work item already queued by that handler.
wm831x_ts_remove() can therefore free the IRQ actions while pd_data_work is
pending or running. The work may then dereference wm831x_ts after devres
frees it, or enable an IRQ after its action has been freed.
Disable both IRQs before cancelling the work, so neither handler can queue
another instance while cancel_work_sync() drains it. Free the IRQ actions
only after the work has stopped. Apply the same sequence to err_pd_irq:
both IRQ actions exist there when input_register_device() fails.
This issue was found by an in-house static analysis tool and confirmed by
manual code review.
Fixes: f5346668150c ("Input: wm831x-ts - fix races with IRQ management")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
drivers/input/touchscreen/wm831x-ts.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/input/touchscreen/wm831x-ts.c b/drivers/input/touchscreen/wm831x-ts.c
index 98f8ec408cad..705772025648 100644
--- a/drivers/input/touchscreen/wm831x-ts.c
+++ b/drivers/input/touchscreen/wm831x-ts.c
@@ -366,6 +366,10 @@ static int wm831x_ts_probe(struct platform_device *pdev)
return 0;
err_pd_irq:
+ disable_irq(wm831x_ts->pd_irq);
+ disable_irq(wm831x_ts->data_irq);
+ cancel_work_sync(&wm831x_ts->pd_data_work);
+
free_irq(wm831x_ts->pd_irq, wm831x_ts);
err_data_irq:
free_irq(wm831x_ts->data_irq, wm831x_ts);
@@ -378,6 +382,10 @@ static void wm831x_ts_remove(struct platform_device *pdev)
{
struct wm831x_ts *wm831x_ts = platform_get_drvdata(pdev);
+ disable_irq(wm831x_ts->pd_irq);
+ disable_irq(wm831x_ts->data_irq);
+ cancel_work_sync(&wm831x_ts->pd_data_work);
+
free_irq(wm831x_ts->pd_irq, wm831x_ts);
free_irq(wm831x_ts->data_irq, wm831x_ts);
}
--
2.34.1
^ permalink raw reply related
* Re: [PATCH] Input: wm831x-ts - drain pen-down work at teardown
From: sashiko-bot @ 2026-07-22 3:03 UTC (permalink / raw)
To: Fan Wu; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260722024518.3253280-1-fanwu01@zju.edu.cn>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Use-After-Free / Lifecycle Bug due to devres and manual IRQ teardown mismatch.
--
--- Patch [1]: [PATCH] Input: wm831x-ts - drain pen-down work at teardown ---
commit 084d696661fe76ff6ec20cb061d22484763d2522
Author: Fan Wu <fanwu01@zju.edu.cn>
Input: wm831x-ts - drain pen-down work at teardown
This patch explicitly disables the pen-down and data IRQs and cancels the
pd_data_work before freeing the IRQs in the error path and during device
removal. This aims to fix races where the work might run or be pending
while the IRQs are being freed.
> diff --git a/drivers/input/touchscreen/wm831x-ts.c b/drivers/input/touchscreen/wm831x-ts.c
> index 98f8ec408cad6..7057720256487 100644
> --- a/drivers/input/touchscreen/wm831x-ts.c
> +++ b/drivers/input/touchscreen/wm831x-ts.c
[ ... ]
> @@ -378,6 +382,10 @@ static void wm831x_ts_remove(struct platform_device *pdev)
> {
> struct wm831x_ts *wm831x_ts = platform_get_drvdata(pdev);
>
> + disable_irq(wm831x_ts->pd_irq);
> + disable_irq(wm831x_ts->data_irq);
> + cancel_work_sync(&wm831x_ts->pd_data_work);
> +
> free_irq(wm831x_ts->pd_irq, wm831x_ts);
> free_irq(wm831x_ts->data_irq, wm831x_ts);
> }
[Severity: High]
This is a pre-existing issue, but does freeing the IRQs here lead to a
use-after-free when the input device is closed during devres cleanup?
Because the input device is managed via devres (allocated with
devm_input_allocate_device()), the input core will automatically unregister
it after wm831x_ts_remove() returns.
If the input device is held open by userspace while the driver is unbound,
the unregistration process will call the device's close callback. That callback
operates on the IRQs that have already been freed:
wm831x_ts_input_close() {
...
if (wm831x_ts->pen_down) {
disable_irq(wm831x_ts->data_irq);
enable_irq(wm831x_ts->pd_irq);
wm831x_ts->pen_down = false;
}
...
}
Calling enable_irq() on an already freed IRQ can drop its depth to 0 and
unmask it without an action handler, leading to an IRQ shutdown and a loud
kernel 'irq X: nobody cared' warning. In a shared IRQ environment (like an MFD),
could this disrupt other devices sharing the same line?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260722024518.3253280-1-fanwu01@zju.edu.cn?part=1
^ permalink raw reply
* Re: [PATCH v3 7/9] platform/chrome: of_hw_prober: Add delay for hana trackpads
From: Chen-Yu Tsai @ 2026-07-22 3:33 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Matthias Brugger, AngeloGioacchino Del Regno, Benson Leung,
Tzung-Bi Shih, Dmitry Torokhov, Jiri Kosina, Andi Shyti,
linux-mediatek, devicetree, linux-arm-kernel, chrome-platform,
linux-input, linux-i2c, linux-kernel
In-Reply-To: <al9JivV0SZCiOgK8@ashevche-desk.local>
On Tue, Jul 21, 2026 at 6:27 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Jul 21, 2026 at 03:52:21PM +0800, Chen-Yu Tsai wrote:
> > Up until now, the MT8173 elm/hana device tree has set the dedicated
> > regulator supplying the trackpad as always-on, simply because the Elan
> > driver was missing proper delays. As a result the delay for the
> > Synaptics trackpad was also omitted, as it was not strictly required
> > under such a model and delayed the availability of the trackpad to the
> > user.
> >
> > The Elan driver recently gained proper delays after power-up, with
> > adaptive skipping of the delay if the regulator was originally
> > on. The I2C HID driver and I2C OF component prober library gained
> > similar adaptive delay skipping. The device tree will be fixed to have
> > the regulator not be always on, and proper post-power-on delay time
> > added to the I2C HID device.
> >
> > Also add the post-power-on delay to the ChromeOS OF component prober,
> > so that if the regulator is off at the time of probing, the prober knows
> > to wait for the hardware to initialize.
>
> ...
>
> > +++ b/drivers/platform/chrome/chromeos_of_hw_prober.c
>
> > /*
> > * ELAN trackpad needs 2 ms for H/W init and 100 ms for F/W init.
> > * Synaptics trackpad needs 100 ms.
> > - * However, the regulator is set to "always-on", presumably to
> > - * avoid this delay. The ELAN driver is also missing delays.
> > */
> > - .post_power_on_delay_ms = 0,
> > + .post_power_on_delay_ms = 110,
>
> This and the other delay in the previous patches are all HW-related. What if
> another touchpad gets connected? I mean that the delays for the certain HW
> should not affect other possible devices connected to the platform.
This is a complete laptop, not a development board. You can't just randomly
swap out the trackpad. The model AFAIK is no longer in production, and we
have already added all possible configurations to the device tree. And
there's no extra connector or pin header to attach additional peripherals.
If someone hacks their own device to swap out a trackpad, then handling
the additional bring-up should fall on them.
If you are referring to the delay for the other trackpad option, that is
100 ms, and is described in the device tree. See patch 8 of this series.
Any other device with different delay requirements should have its own
entry, not reuse this one. The information in the entry is tightly
coupled to the device tree anyway. I already corrected the other device
that incorrectly reused this entry [1]. We have some additional offenders
downstream, but nothing that can't be corrected in the same manner.
ChenYu
[1] https://lore.kernel.org/all/20260625060859.1020483-1-wenst@chromium.org/
^ permalink raw reply
* [PATCH v2] Input: wm831x-ts - drain pen-down work at teardown
From: Fan Wu @ 2026-07-22 4:58 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, linux-kernel, Fan Wu, stable
In-Reply-To: <20260722024518.3253280-1-fanwu01@zju.edu.cn>
The pen-down and data IRQ handlers queue pd_data_work to switch between the
two IRQs. The worker obtains wm831x_ts with container_of() and calls
enable_irq(). free_irq() synchronizes an IRQ handler, but does not drain a
work item already queued by that handler.
wm831x_ts_remove() can therefore free the IRQ actions while pd_data_work is
pending or running. The work may then dereference wm831x_ts after devres
frees it (use-after-free), or re-enable an IRQ whose action has been freed.
The input device is devm-allocated, so the input core unregisters it after
wm831x_ts_remove() returns. If a userspace handle is still open, that
unregister drives the input close callback, which on pen-down calls
enable_irq(pd_irq) - after wm831x_ts_remove() has already freed the pd_irq
action. Unregister the input device before freeing the IRQs, so the close
callback runs while the actions are still installed. This must precede the
IRQ teardown: disabling and cancelling first would still let the close
callback re-enable pd_irq after its action is gone.
After unregistering, drain the work: disable both IRQs so neither handler
can queue another instance, cancel_work_sync() to make the drain final,
then free the actions. Apply the same drain to err_pd_irq; the input
device is not registered there, so it is not unregistered.
This issue was found by an in-house static analysis tool and confirmed by
manual code review.
Fixes: f5346668150c ("Input: wm831x-ts - fix races with IRQ management")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
Changes since v1:
- Unregister the input device before freeing the IRQs. v1 drained the
work in wm831x_ts_remove(), but the input device is devm-managed and is
unregistered by the input core only after remove() returns; with a
userspace handle open, that unregister drives the close callback, which
on pen-down calls enable_irq(pd_irq) after its action has been freed.
drivers/input/touchscreen/wm831x-ts.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/input/touchscreen/wm831x-ts.c b/drivers/input/touchscreen/wm831x-ts.c
index 98f8ec408cad..a465b1b96fd8 100644
--- a/drivers/input/touchscreen/wm831x-ts.c
+++ b/drivers/input/touchscreen/wm831x-ts.c
@@ -366,6 +366,10 @@ static int wm831x_ts_probe(struct platform_device *pdev)
return 0;
err_pd_irq:
+ disable_irq(wm831x_ts->pd_irq);
+ disable_irq(wm831x_ts->data_irq);
+ cancel_work_sync(&wm831x_ts->pd_data_work);
+
free_irq(wm831x_ts->pd_irq, wm831x_ts);
err_data_irq:
free_irq(wm831x_ts->data_irq, wm831x_ts);
@@ -378,6 +382,12 @@ static void wm831x_ts_remove(struct platform_device *pdev)
{
struct wm831x_ts *wm831x_ts = platform_get_drvdata(pdev);
+ input_unregister_device(wm831x_ts->input_dev);
+
+ disable_irq(wm831x_ts->pd_irq);
+ disable_irq(wm831x_ts->data_irq);
+ cancel_work_sync(&wm831x_ts->pd_data_work);
+
free_irq(wm831x_ts->pd_irq, wm831x_ts);
free_irq(wm831x_ts->data_irq, wm831x_ts);
}
--
2.34.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox