Hello,
This v2 addresses some issues observed when running the ACPI probe kselftest proposed in v1[1] across various devices and improves the overall reliability of the test.
The acpi-extract-ids script has been improved to: - Parse both .c and .h files - Add an option to print only IDs matched by a driver (i.e. defined in an ACPI match tables or in lists of IDs provided by the drivers)
The test_unprobed_devices.sh script relies on sysfs information to determine if a device was successfully bound to a driver. Not all devices listed in /sys/devices are expected to have a driver folder, so the script has been adjusted to handle these cases and avoid generating false negatives.
The test_unprobed_devices.sh test script logic has been modified to: - Check the status attribute (when available) to exclusively test hardware devices that are physically present, enabled and operational - Traverse only ACPI objects with a physical_node* link, to ensure testing of correctly enumerated devices - Skip devices whose HID or CID are not matched by any driver, as determined by the list generated through the acpi-extract-ids script - Skip devices with HID or CID listed in the ignored IDs list. This list has been added to contain IDs of devices that don't require a driver or cannot be represented as platform devices (e.g. ACPI container and module devices). - Skip devices that are natively enumerated and don't need a driver, such as certain PCI bridges - Skip devices unassigned to any subsystem, devices linked to other devices and class devices
Some of the heuristics used by the script are suboptimal and might require adjustments over time. This kind of tests would greatly benefit from a dedicated interface that exposes information about devices expected to be matched by drivers and their probe status. Discussion regarding this matter was initiated in v1.
As of now, I have not identified a suitable method for exposing this information; I plan on submitting a separate RFC to propose some options and engage in discussion. Meanwhile, this v2 focuses on utilizing already available information to provide an ACPI equivalent of the existing DT kselftest [2].
Adding in CC the people involved in the discussion at Plumbers [3], feel free to add anyone that might be interested in this.
This series depends on: - https://lore.kernel.org/all/20240102141528.169947-1-laura.nao@collabora.com/... - https://lore.kernel.org/all/20240131-ktap-sh-helpers-extend-v1-0-98ffb468712...
Thanks,
Laura
[1] https://lore.kernel.org/all/20230925155806.1812249-2-laura.nao@collabora.com... [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tool... [3] https://www.youtube.com/watch?v=oE73eVSyFXQ&t=9377s
Original cover letter:
Regressions that prevent a driver from probing a device can significantly affect the functionality of a platform.
A kselftest to verify if devices on a DT-based platform are probed correctly was recently introduced [4], but no such generic test is available for ACPI platforms yet. bootrr [5] provides device probe testing, but relies on a pre-defined list of the peripherals present on each DUT.
On ACPI based hardware, a complete description of the platform is provided to the OS by the system firmware. ACPI namespace objects are mapped by the Linux ACPI subsystem into a device tree in /sys/devices/LNXSYSTEM:00; the information in this subtree can be parsed to build a list of the hw peripherals present on the DUT dynamically.
This series adds a test to verify if the devices declared in the ACPI namespace and supported by the kernel are probed correctly.
This work follows a similar approach to [4], adapted for the ACPI use case.
The first patch introduces a script that builds a list of all ACPI device IDs supported by the kernel, by inspecting the acpi_device_id structs in the sources. This list can be used to avoid testing ACPI-enumerated devices that don't have a matching driver in the kernel. This script was highly inspired by the dt-extract-compatibles script [6].
In the second patch, a new kselftest is added. It parses the /sys/devices/LNXSYSTEM:00 tree to obtain a list of all platform peripherals and verifies which of those, if supported, are correctly bound to a driver.
Feedback is much appreciated,
Thank you,
Laura
[4] https://lore.kernel.org/all/20230828211424.2964562-1-nfraprado@collabora.com... [5] https://github.com/kernelci/bootr [6] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scri...
Laura Nao (2): acpi: Add script to extract ACPI device ids in the kernel kselftest: Add test to detect unprobed devices on ACPI platforms
MAINTAINERS | 2 + scripts/acpi/acpi-extract-ids | 99 +++++++++++++ tools/testing/selftests/Makefile | 1 + tools/testing/selftests/acpi/.gitignore | 1 + tools/testing/selftests/acpi/Makefile | 21 +++ tools/testing/selftests/acpi/id_ignore_list | 3 + .../selftests/acpi/test_unprobed_devices.sh | 138 ++++++++++++++++++ 7 files changed, 265 insertions(+) create mode 100755 scripts/acpi/acpi-extract-ids create mode 100644 tools/testing/selftests/acpi/.gitignore create mode 100644 tools/testing/selftests/acpi/Makefile create mode 100644 tools/testing/selftests/acpi/id_ignore_list create mode 100755 tools/testing/selftests/acpi/test_unprobed_devices.sh
Add a script to extract all the supported acpi device ids from kernel sources.
The script looks for IDs defined in acpi_device_id structs within both .c and .h files and prints them. If the -d option is used, the script only shows the IDs that are matched by a driver, identified through either an ACPI match table or a list of supported IDs provided by the driver.
The list of IDs returned by the script can be used as a reference to determine if a device declared in the ACPI namespace with certain _HID/_CID is supported by the kernel or not.
Note: this script cannot identify IDs defined via macros.
Signed-off-by: Laura Nao laura.nao@collabora.com --- MAINTAINERS | 1 + scripts/acpi/acpi-extract-ids | 99 +++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+) create mode 100755 scripts/acpi/acpi-extract-ids
diff --git a/MAINTAINERS b/MAINTAINERS index 375d34363777..8333ead448c4 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -293,6 +293,7 @@ F: include/linux/acpi.h F: include/linux/fwnode.h F: include/linux/fw_table.h F: lib/fw_table.c +F: scripts/acpi/acpi-extract-ids F: tools/power/acpi/
ACPI APEI diff --git a/scripts/acpi/acpi-extract-ids b/scripts/acpi/acpi-extract-ids new file mode 100755 index 000000000000..4c492d384a35 --- /dev/null +++ b/scripts/acpi/acpi-extract-ids @@ -0,0 +1,99 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: GPL-2.0-only +# +# Heavily inspired by the scripts/dtc/dt-extract-compatibles script, +# adapted for the ACPI use case. +# + +import fnmatch +import os +import glob +import re +import argparse + + +def parse_acpi_device_id(data, match_list=None): + """ Find all device ids in acpi_device_id structs """ + acpi_device_id_list = [] + + for m in re.finditer(r'acpi_device_id(\s+\S+)?\s+(\S+)[](\s+\S+)?\s*=\s*({.*?);', data): + if match_list is not None and m[2] not in match_list: + continue + acpi_device_id_list += re.findall(r'"(\S+)"', m[4]) + + return acpi_device_id_list + +def parse_acpi_match_table(data): + """ Find all driver's acpi_match_table """ + match_table_list = [] + for m in re.finditer(r'.acpi_match_table\s+=\s+(ACPI_PTR()?([a-zA-Z0-9_-]+)', data): + match_table_list.append(m[2]) + + return match_table_list + +def parse_acpi_driver_ids(data): + """ Find all driver's ids """ + id_list = [] + for m in re.finditer(r'.ids\s+=\s+([a-zA-Z0-9_-]+)', data): + id_list.append(m[1]) + + return id_list + +def is_header_file(file): + _, extension = os.path.splitext(file) + return extension.lower() == ".h" + +def parse_ids(file, driver_match=False): + with open(file, 'r', encoding='utf-8') as f: + data = f.read().replace('\n', '') + + if is_header_file(file) or not driver_match: + return parse_acpi_device_id(data) + else: + match_list = parse_acpi_match_table(data) + parse_acpi_driver_ids(data) + return parse_acpi_device_id(data, match_list) + +def print_ids(filename, id_list): + if not id_list: + return + if show_filename: + compat_str = ' '.join(id_list) + print(filename + ": ID(s): " + compat_str) + else: + print(*id_list, sep='\n') + +def glob_without_symlinks(root, glob): + for path, dirs, files in os.walk(root): + # Ignore hidden directories + for d in dirs: + if fnmatch.fnmatch(d, ".*"): + dirs.remove(d) + for f in files: + if fnmatch.fnmatch(f, glob): + yield os.path.join(path, f) + +def files_to_parse(path_args): + for f in path_args: + if os.path.isdir(f): + for filename in glob_without_symlinks(f, "*.[ch]"): + yield filename + else: + yield f + + +show_filename = False + +if __name__ == "__main__": + ap = argparse.ArgumentParser() + ap.add_argument("cfile", type=str, nargs='*', + help="C source files or directories to parse") + ap.add_argument('-H', '--with-filename', + help="Print filename with device ids", action="store_true") + ap.add_argument('-d', '--driver-match', help="Only print ids that should match to a driver", action="store_true") + args = ap.parse_args() + + show_filename = args.with_filename + + for f in files_to_parse(args.cfile): + id_list = parse_ids(f, args.driver_match) + print_ids(f, id_list)
Add new kselftest that tests whether devices declared in the ACPI namespace and supported by the kernel are correctly bound to a driver.
The test traverses the ACPI sysfs tree to get a list of all the devices defined in the ACPI namespace and verifies whether the physical devices linked to each ACPI object are bound to a driver. The test relies on two lists to skip devices not expected to be bound to a driver: - List generated by the acpi-extract-ids script: includes the ACPI IDs matched by a driver - Manual list of ignored IDs: includes the ID of devices that may be discovered only via the platform firmware and that don't require a driver or cannot be represented as platform devices
The test also examines the sysfs attributes of the target device objects linked by physical_node* to exclude other devices that should not be bound to a driver. This includes: - Devices not assigned to any subsystem - Devices that are linked to other devices - Class devices - Specific PCI bridges that do not require a driver
Signed-off-by: Laura Nao laura.nao@collabora.com --- MAINTAINERS | 1 + tools/testing/selftests/Makefile | 1 + tools/testing/selftests/acpi/.gitignore | 1 + tools/testing/selftests/acpi/Makefile | 21 +++ tools/testing/selftests/acpi/id_ignore_list | 3 + .../selftests/acpi/test_unprobed_devices.sh | 138 ++++++++++++++++++ 6 files changed, 165 insertions(+) create mode 100644 tools/testing/selftests/acpi/.gitignore create mode 100644 tools/testing/selftests/acpi/Makefile create mode 100644 tools/testing/selftests/acpi/id_ignore_list create mode 100755 tools/testing/selftests/acpi/test_unprobed_devices.sh
diff --git a/MAINTAINERS b/MAINTAINERS index 8333ead448c4..1f58949c9e51 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -294,6 +294,7 @@ F: include/linux/fwnode.h F: include/linux/fw_table.h F: lib/fw_table.c F: scripts/acpi/acpi-extract-ids +F: tools/testing/selftests/acpi/ F: tools/power/acpi/
ACPI APEI diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index e1504833654d..3107301ea4f3 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -1,4 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 +TARGETS += acpi TARGETS += alsa TARGETS += amd-pstate TARGETS += arm64 diff --git a/tools/testing/selftests/acpi/.gitignore b/tools/testing/selftests/acpi/.gitignore new file mode 100644 index 000000000000..3c520e8a1962 --- /dev/null +++ b/tools/testing/selftests/acpi/.gitignore @@ -0,0 +1 @@ +id_list diff --git a/tools/testing/selftests/acpi/Makefile b/tools/testing/selftests/acpi/Makefile new file mode 100644 index 000000000000..b80d4fb797ac --- /dev/null +++ b/tools/testing/selftests/acpi/Makefile @@ -0,0 +1,21 @@ +PY3 = $(shell which python3 2>/dev/null) + +ifneq ($(PY3),) + +TEST_PROGS := test_unprobed_devices.sh +TEST_GEN_FILES := id_list +TEST_FILES := id_ignore_list + +include ../lib.mk + +$(OUTPUT)/id_list: + $(top_srcdir)/scripts/acpi/acpi-extract-ids -d $(top_srcdir) > $@ + +else + +all: no_py3_warning + +no_py3_warning: + @echo "Missing python3. This test will be skipped." + +endif \ No newline at end of file diff --git a/tools/testing/selftests/acpi/id_ignore_list b/tools/testing/selftests/acpi/id_ignore_list new file mode 100644 index 000000000000..86ddf4b0a55a --- /dev/null +++ b/tools/testing/selftests/acpi/id_ignore_list @@ -0,0 +1,3 @@ +PNP0A05 +PNP0A06 +ACPI0004 \ No newline at end of file diff --git a/tools/testing/selftests/acpi/test_unprobed_devices.sh b/tools/testing/selftests/acpi/test_unprobed_devices.sh new file mode 100755 index 000000000000..23e52833c475 --- /dev/null +++ b/tools/testing/selftests/acpi/test_unprobed_devices.sh @@ -0,0 +1,138 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# +# Copyright (c) 2023 Collabora Ltd +# +# Inspired by the tools/testing/selftests/dt/test_unprobed_devices.sh +# script, adapted for the ACPI use case. +# +# This script checks whether devices declared in the ACPI namespace and +# supported by the kernel are correctly bound to a driver. +# +# To do this, two lists are used: +# * a list of ACPI IDs matched by existing drivers +# * a list of IDs that should be ignored +# + +DIR="$(dirname "$(readlink -f "$0")")" + +KTAP_HELPERS="${DIR}/../kselftest/ktap_helpers.sh" +if ! source "$KTAP_HELPERS"; then + exit 4 +fi + +ACPI_SYSTEM_DIR="/sys/devices/LNXSYSTM:00" +ID_IGNORE_LIST="${DIR}"/id_ignore_list +ID_LIST="${DIR}"/id_list + +PCI_CLASS_BRIDGE_HOST="0x0600" +PCI_CLASS_BRIDGE_ISA="0x0601" + +ktap_print_header + +if [[ ! -d "${ACPI_SYSTEM_DIR}" ]]; then + ktap_skip_all "${ACPI_SYSTEM_DIR} doesn't exist." + exit "${KSFT_SKIP}" +fi + +# The ACPI specification mandates that ACPI objects representing devices on +# non-enumerable and enumerable busses contain a _HID or an _ADR +# identification object respectively. Get a list of devices of both types, +# by searching the ACPI sysfs subtree for directories containing a hid or +# adr attribute. +supp_dev_paths=$(while IFS=$'\n' read -r dev_path; do + if [ ! -f "${dev_path}"/hid ] && [ ! -f "${dev_path}"/adr ]; then + continue + fi + + # Check if the device is present, enabled, and functioning properly + status="${dev_path}/status" + if [ -f "${status}" ]; then + status_hex=$(($(cat "${status}"))) + + if [ $((status_hex & 1)) -eq 0 ] || + [ $((status_hex >> 1 & 1)) -eq 0 ] || + [ $((status_hex >> 3 & 1)) -eq 0 ]; then + continue + fi + fi + + if [ -n "$(find -L "${dev_path}" -maxdepth 1 -name "physical_node*" -print -quit)" ]; then + for node in "${dev_path}"/physical_node*; do + # Ignore devices without a subsystem, devices that link to + # other devices, and class devices + if [ ! -d "${node}/subsystem" ] || + [ -d "${node}/device" ] || + [[ "$(readlink -f "${node}/subsystem")" == /sys/class/* ]]; then + continue + fi + + echo "${node}" + done + fi +done < <(find ${ACPI_SYSTEM_DIR} -name uevent -exec dirname {} ;)) + +supp_dev_paths_num=$(echo "${supp_dev_paths}" | wc -w) +ktap_set_plan "${supp_dev_paths_num}" + +# Iterate over ACPI devices +for dev_path in ${supp_dev_paths}; do + if [ -f "${dev_path}/firmware_node/path" ]; then + acpi_path="$(<"${dev_path}"/firmware_node/path)" + fi + + dev_link=$(readlink -f "${dev_path}") + desc="${acpi_path}-${dev_link#/sys/devices/}" + + if [ -f "${dev_path}/firmware_node/hid" ]; then + hid="$(<"${dev_path}"/firmware_node/hid)" + + if [ -f "${dev_path}/firmware_node/modalias" ]; then + modalias=$(<"${dev_path}/firmware_node/modalias") + cid=$(echo "${modalias}" | cut -d':' -f3) + + # Skip devices with ignored HID/CID + if ignored_id=$(grep -i "${hid}" "${ID_IGNORE_LIST}" || + { [ -n "${cid}" ] && grep -i "${cid}" "${ID_IGNORE_LIST}"; }); then + ktap_print_msg "ID ${ignored_id} ignored [SKIP]" + ktap_test_skip "${desc}" + continue + fi + # Skip devices with unsupported HID/CID + if [[ "${hid}" != LNX* ]] && ! grep -x -q -i "${hid}" "${ID_LIST}"; then + if [ -z "${cid}" ] || ! grep -x -q -i "${cid}" "${ID_LIST}"; then + ktap_print_msg "no match for ${hid}${cid:+:${cid}} found \ + in the supported IDs list [SKIP]" + ktap_test_skip "${desc}" + continue + fi + fi + fi + fi + + # Skip bridges that don't require a driver + if [ -f "${dev_path}/class" ]; then + class=$(<"${dev_path}"/class) + if [[ ${class} == ${PCI_CLASS_BRIDGE_HOST}* ]] || + [[ ${class} == ${PCI_CLASS_BRIDGE_ISA}* ]]; then + ktap_print_msg "device linked to ${desc} does not require a driver [SKIP]" + ktap_test_skip "${desc}" + continue + fi + fi + + # Search for the driver in both the device folder and the companion's folder + if [ -d "${dev_path}/driver" ] || [ -d "${dev_path}/firmware_node/driver" ]; then + ktap_test_pass "${desc}" + # Skip char devices + elif [ -f "${dev_path}/dev" ]; then + ktap_print_msg "${desc} is a char device [SKIP]" + ktap_test_skip "${desc}" + continue + else + ktap_test_fail "${desc}" + fi + +done + +ktap_finished
Hi Shuah and Rafael,
On 3/8/24 15:49, Laura Nao wrote:
Hello,
This v2 addresses some issues observed when running the ACPI probe kselftest proposed in v1[1] across various devices and improves the overall reliability of the test.
The acpi-extract-ids script has been improved to:
- Parse both .c and .h files
- Add an option to print only IDs matched by a driver (i.e. defined in an
ACPI match tables or in lists of IDs provided by the drivers)
The test_unprobed_devices.sh script relies on sysfs information to determine if a device was successfully bound to a driver. Not all devices listed in /sys/devices are expected to have a driver folder, so the script has been adjusted to handle these cases and avoid generating false negatives.
The test_unprobed_devices.sh test script logic has been modified to:
- Check the status attribute (when available) to exclusively test hardware devices that are physically present, enabled and operational
- Traverse only ACPI objects with a physical_node* link, to ensure testing of correctly enumerated devices
- Skip devices whose HID or CID are not matched by any driver, as determined by the list generated through the acpi-extract-ids script
- Skip devices with HID or CID listed in the ignored IDs list. This list has been added to contain IDs of devices that don't require a driver or cannot be represented as platform devices (e.g. ACPI container and module devices).
- Skip devices that are natively enumerated and don't need a driver, such as certain PCI bridges
- Skip devices unassigned to any subsystem, devices linked to other devices and class devices
Some of the heuristics used by the script are suboptimal and might require adjustments over time. This kind of tests would greatly benefit from a dedicated interface that exposes information about devices expected to be matched by drivers and their probe status. Discussion regarding this matter was initiated in v1.
As of now, I have not identified a suitable method for exposing this information; I plan on submitting a separate RFC to propose some options and engage in discussion. Meanwhile, this v2 focuses on utilizing already available information to provide an ACPI equivalent of the existing DT kselftest [2].
Adding in CC the people involved in the discussion at Plumbers [3], feel free to add anyone that might be interested in this.
This series depends on:
- https://lore.kernel.org/all/20240102141528.169947-1-laura.nao@collabora.com/...
- https://lore.kernel.org/all/20240131-ktap-sh-helpers-extend-v1-0-98ffb468712...
Thanks,
Laura
[1] https://lore.kernel.org/all/20230925155806.1812249-2-laura.nao@collabora.com... [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tool... [3] https://www.youtube.com/watch?v=oE73eVSyFXQ&t=9377s
Just wanted to gently check in on your thoughts regarding this series. We've conducted some initial testing with it in KernelCI and it's proven its worth by catching a driver probe regression [1] on some x86_64 platforms. Your feedback would be greatly appreciated.
Thanks!
Laura
[1] https://lore.kernel.org/all/20240530153727.843378-1-laura.nao@collabora.com/
linux-kselftest-mirror@lists.linaro.org
-
Laura Nao