This series primarily adds support for DECLARE_PCI_FIXUP_*() in modules. There are a few drivers that already use this, and so they are presumably broken when built as modules.
While at it, I wrote some unit tests that emulate a fake PCI device, and let the PCI framework match/not-match its vendor/device IDs. This test can be built into the kernel or built as a module.
I also include some infrastructure changes (patch 3 and 4), so that ARCH=um (the default for kunit.py), ARCH=arm, and ARCH=arm64 will run these tests by default. These patches have different maintainers and are independent, so they can probably be picked up separately. I included them because otherwise the tests in patch 2 aren't so easy to run.
Brian Norris (4): PCI: Support FIXUP quirks in modules PCI: Add KUnit tests for FIXUP quirks um: Select PCI_DOMAINS_GENERIC kunit: qemu_configs: Add PCI to arm, arm64
arch/um/Kconfig | 1 + drivers/pci/Kconfig | 11 ++ drivers/pci/Makefile | 1 + drivers/pci/fixup-test.c | 197 ++++++++++++++++++++++ drivers/pci/quirks.c | 62 +++++++ include/linux/module.h | 18 ++ kernel/module/main.c | 26 +++ tools/testing/kunit/qemu_configs/arm.py | 1 + tools/testing/kunit/qemu_configs/arm64.py | 1 + 9 files changed, 318 insertions(+) create mode 100644 drivers/pci/fixup-test.c
The PCI framework supports "quirks" for PCI devices via several DECLARE_PCI_FIXUP_*() macros. These macros allow arch or driver code to match device IDs to provide customizations or workarounds for broken devices.
This mechanism is generally used in code that can only be built into the kernel, but there are a few occasions where this mechanism is used in drivers that can be modules. For example, see commit 574f29036fce ("PCI: iproc: Apply quirk_paxc_bridge() for module as well as built-in").
The PCI fixup mechanism only works for built-in code, however, because pci_fixup_device() only scans the ".pci_fixup_*" linker sections found in the main kernel; it never touches modules.
Extend the fixup approach to modules.
I don't attempt to be clever here; the algorithm here scales with the number of modules in the system.
Signed-off-by: Brian Norris briannorris@chromium.org ---
drivers/pci/quirks.c | 62 ++++++++++++++++++++++++++++++++++++++++++ include/linux/module.h | 18 ++++++++++++ kernel/module/main.c | 26 ++++++++++++++++++ 3 files changed, 106 insertions(+)
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index d97335a40193..db5e0ac82ed7 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -207,6 +207,62 @@ extern struct pci_fixup __end_pci_fixups_suspend_late[];
static bool pci_apply_fixup_final_quirks;
+struct pci_fixup_arg { + struct pci_dev *dev; + enum pci_fixup_pass pass; +}; + +static int pci_module_fixup(struct module *mod, void *parm) +{ + struct pci_fixup_arg *arg = parm; + void *start; + unsigned int size; + + switch (arg->pass) { + case pci_fixup_early: + start = mod->pci_fixup_early; + size = mod->pci_fixup_early_size; + break; + case pci_fixup_header: + start = mod->pci_fixup_header; + size = mod->pci_fixup_header_size; + break; + case pci_fixup_final: + start = mod->pci_fixup_final; + size = mod->pci_fixup_final_size; + break; + case pci_fixup_enable: + start = mod->pci_fixup_enable; + size = mod->pci_fixup_enable_size; + break; + case pci_fixup_resume: + start = mod->pci_fixup_resume; + size = mod->pci_fixup_resume_size; + break; + case pci_fixup_suspend: + start = mod->pci_fixup_suspend; + size = mod->pci_fixup_suspend_size; + break; + case pci_fixup_resume_early: + start = mod->pci_fixup_resume_early; + size = mod->pci_fixup_resume_early_size; + break; + case pci_fixup_suspend_late: + start = mod->pci_fixup_suspend_late; + size = mod->pci_fixup_suspend_late_size; + break; + default: + return 0; + } + + if (!size) + return 0; + + pci_do_fixups(arg->dev, start, start + size); + + return 0; +} + void pci_fixup_device(enum pci_fixup_pass pass, struct pci_dev *dev) { struct pci_fixup *start, *end; @@ -259,6 +315,12 @@ void pci_fixup_device(enum pci_fixup_pass pass, struct pci_dev *dev) return; } pci_do_fixups(dev, start, end); + + struct pci_fixup_arg arg = { + .dev = dev, + .pass = pass, + }; + module_for_each_mod(pci_module_fixup, &arg); } EXPORT_SYMBOL(pci_fixup_device);
diff --git a/include/linux/module.h b/include/linux/module.h index 3319a5269d28..7faa8987b9eb 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -539,6 +539,24 @@ struct module { int num_kunit_suites; struct kunit_suite **kunit_suites; #endif +#ifdef CONFIG_PCI_QUIRKS + void *pci_fixup_early; + unsigned int pci_fixup_early_size; + void *pci_fixup_header; + unsigned int pci_fixup_header_size; + void *pci_fixup_final; + unsigned int pci_fixup_final_size; + void *pci_fixup_enable; + unsigned int pci_fixup_enable_size; + void *pci_fixup_resume; + unsigned int pci_fixup_resume_size; + void *pci_fixup_suspend; + unsigned int pci_fixup_suspend_size; + void *pci_fixup_resume_early; + unsigned int pci_fixup_resume_early_size; + void *pci_fixup_suspend_late; + unsigned int pci_fixup_suspend_late_size; +#endif
#ifdef CONFIG_LIVEPATCH diff --git a/kernel/module/main.c b/kernel/module/main.c index c66b26184936..50a80c875adc 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -2702,6 +2702,32 @@ static int find_module_sections(struct module *mod, struct load_info *info) sizeof(*mod->kunit_init_suites), &mod->num_kunit_init_suites); #endif +#ifdef CONFIG_PCI_QUIRKS + mod->pci_fixup_early = section_objs(info, ".pci_fixup_early", + sizeof(*mod->pci_fixup_early), + &mod->pci_fixup_early_size); + mod->pci_fixup_header = section_objs(info, ".pci_fixup_header", + sizeof(*mod->pci_fixup_header), + &mod->pci_fixup_header_size); + mod->pci_fixup_final = section_objs(info, ".pci_fixup_final", + sizeof(*mod->pci_fixup_final), + &mod->pci_fixup_final_size); + mod->pci_fixup_enable = section_objs(info, ".pci_fixup_enable", + sizeof(*mod->pci_fixup_enable), + &mod->pci_fixup_enable_size); + mod->pci_fixup_resume = section_objs(info, ".pci_fixup_resume", + sizeof(*mod->pci_fixup_resume), + &mod->pci_fixup_resume_size); + mod->pci_fixup_suspend = section_objs(info, ".pci_fixup_suspend", + sizeof(*mod->pci_fixup_suspend), + &mod->pci_fixup_suspend_size); + mod->pci_fixup_resume_early = section_objs(info, ".pci_fixup_resume_early", + sizeof(*mod->pci_fixup_resume_early), + &mod->pci_fixup_resume_early_size); + mod->pci_fixup_suspend_late = section_objs(info, ".pci_fixup_suspend_late", + sizeof(*mod->pci_fixup_suspend_late), + &mod->pci_fixup_suspend_late_size); +#endif
mod->extable = section_objs(info, "__ex_table", sizeof(*mod->extable), &mod->num_exentries);
The PCI framework supports device quirks via a series of macros and linker sections. This support previously did not work when used in modules. Add a basic set of tests for matching/non-matching devices.
Example run:
$ ./tools/testing/kunit/kunit.py run 'pci_fixup*' [...] [15:31:30] ============ pci_fixup_test_cases (2 subtests) ============= [15:31:30] [PASSED] pci_fixup_match_test [15:31:30] [PASSED] pci_fixup_nomatch_test [15:31:30] ============== [PASSED] pci_fixup_test_cases =============== [15:31:30] ============================================================ [15:31:30] Testing complete. Ran 2 tests: passed: 2 [15:31:30] Elapsed time: 11.197s total, 0.001s configuring, 9.870s building, 1.299s running
Signed-off-by: Brian Norris briannorris@chromium.org ---
drivers/pci/Kconfig | 11 +++ drivers/pci/Makefile | 1 + drivers/pci/fixup-test.c | 197 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 209 insertions(+) create mode 100644 drivers/pci/fixup-test.c
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig index 9a249c65aedc..a4fa9be797e7 100644 --- a/drivers/pci/Kconfig +++ b/drivers/pci/Kconfig @@ -68,6 +68,17 @@ config PCI_QUIRKS Disable this only if your target machine is unaffected by PCI quirks.
+config PCI_FIXUP_KUNIT_TEST + tristate "KUnit tests for PCI fixup code" if !KUNIT_ALL_TESTS + depends on KUNIT + depends on PCI_DOMAINS_GENERIC + default KUNIT_ALL_TESTS + help + This builds unit tests for the PCI quirk/fixup framework. Recommended + only for kernel developers. + + When in doubt, say N. + config PCI_DEBUG bool "PCI Debugging" depends on DEBUG_KERNEL diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile index 67647f1880fb..ade400250ceb 100644 --- a/drivers/pci/Makefile +++ b/drivers/pci/Makefile @@ -20,6 +20,7 @@ endif
obj-$(CONFIG_OF) += of.o obj-$(CONFIG_PCI_QUIRKS) += quirks.o +obj-$(CONFIG_PCI_FIXUP_KUNIT_TEST) += fixup-test.o obj-$(CONFIG_HOTPLUG_PCI) += hotplug/ obj-$(CONFIG_PCI_ATS) += ats.o obj-$(CONFIG_PCI_IOV) += iov.o diff --git a/drivers/pci/fixup-test.c b/drivers/pci/fixup-test.c new file mode 100644 index 000000000000..54b895fc8f3e --- /dev/null +++ b/drivers/pci/fixup-test.c @@ -0,0 +1,197 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright 2025 Google, Inc. + */ + +#include <kunit/device.h> +#include <kunit/test.h> +#include <linux/cleanup.h> +#include <linux/pci.h> + +#define DEVICE_NAME "pci_fixup_test_device" +#define TEST_VENDOR_ID 0xdead +#define TEST_DEVICE_ID 0xbeef + +#define TEST_CONF_SIZE 4096 +static u8 *test_conf_space; + +#define test_readb(addr) (*(u8 *)(addr)) +#define test_readw(addr) le16_to_cpu(*((__force __le16 *)(addr))) +#define test_readl(addr) le32_to_cpu(*((__force __le32 *)(addr))) +#define test_writeb(addr, v) (*(u8 *)(addr) = (v)) +#define test_writew(addr, v) (*((__force __le16 *)(addr)) = cpu_to_le16(v)) +#define test_writel(addr, v) (*((__force __le32 *)(addr)) = cpu_to_le32(v)) + +static int test_config_read(struct pci_bus *bus, unsigned int devfn, int where, + int size, u32 *val) +{ + if (PCI_SLOT(devfn) > 0) + return PCIBIOS_DEVICE_NOT_FOUND; + + if (where + size > TEST_CONF_SIZE) + return PCIBIOS_BUFFER_TOO_SMALL; + + if (size == 1) + *val = test_readb(test_conf_space + where); + else if (size == 2) + *val = test_readw(test_conf_space + where); + else if (size == 4) + *val = test_readl(test_conf_space + where); + + return PCIBIOS_SUCCESSFUL; +} + +static int test_config_write(struct pci_bus *bus, unsigned int devfn, int where, + int size, u32 val) +{ + if (PCI_SLOT(devfn) > 0) + return PCIBIOS_DEVICE_NOT_FOUND; + + if (where + size > TEST_CONF_SIZE) + return PCIBIOS_BUFFER_TOO_SMALL; + + if (size == 1) + test_writeb(test_conf_space + where, val); + else if (size == 2) + test_writew(test_conf_space + where, val); + else if (size == 4) + test_writel(test_conf_space + where, val); + + return PCIBIOS_SUCCESSFUL; +} + +static struct pci_ops test_ops = { + .read = test_config_read, + .write = test_config_write, +}; + +static struct pci_dev *hook_device_early; +static struct pci_dev *hook_device_header; +static struct pci_dev *hook_device_final; +static struct pci_dev *hook_device_enable; + +static void pci_fixup_early_hook(struct pci_dev *pdev) +{ + hook_device_early = pdev; +} +DECLARE_PCI_FIXUP_EARLY(TEST_VENDOR_ID, TEST_DEVICE_ID, pci_fixup_early_hook); + +static void pci_fixup_header_hook(struct pci_dev *pdev) +{ + hook_device_header = pdev; +} +DECLARE_PCI_FIXUP_HEADER(TEST_VENDOR_ID, TEST_DEVICE_ID, pci_fixup_header_hook); + +static void pci_fixup_final_hook(struct pci_dev *pdev) +{ + hook_device_final = pdev; +} +DECLARE_PCI_FIXUP_FINAL(TEST_VENDOR_ID, TEST_DEVICE_ID, pci_fixup_final_hook); + +static void pci_fixup_enable_hook(struct pci_dev *pdev) +{ + hook_device_enable = pdev; +} +DECLARE_PCI_FIXUP_ENABLE(TEST_VENDOR_ID, TEST_DEVICE_ID, pci_fixup_enable_hook); + +static int pci_fixup_test_init(struct kunit *test) +{ + hook_device_early = NULL; + hook_device_header = NULL; + hook_device_final = NULL; + hook_device_enable = NULL; + + return 0; +} + +static void pci_fixup_match_test(struct kunit *test) +{ + struct device *dev = kunit_device_register(test, DEVICE_NAME); + + KUNIT_ASSERT_PTR_NE(test, NULL, dev); + + test_conf_space = kunit_kzalloc(test, TEST_CONF_SIZE, GFP_KERNEL); + KUNIT_ASSERT_PTR_NE(test, NULL, test_conf_space); + + /* Minimal configuration space: a stub vendor and device ID */ + test_writew(test_conf_space + PCI_VENDOR_ID, TEST_VENDOR_ID); + test_writew(test_conf_space + PCI_DEVICE_ID, TEST_DEVICE_ID); + + struct pci_host_bridge *bridge = devm_pci_alloc_host_bridge(dev, 0); + + KUNIT_ASSERT_PTR_NE(test, NULL, bridge); + bridge->ops = &test_ops; + + KUNIT_EXPECT_PTR_EQ(test, NULL, hook_device_early); + KUNIT_EXPECT_PTR_EQ(test, NULL, hook_device_header); + KUNIT_EXPECT_PTR_EQ(test, NULL, hook_device_final); + KUNIT_EXPECT_PTR_EQ(test, NULL, hook_device_enable); + + KUNIT_EXPECT_EQ(test, 0, pci_host_probe(bridge)); + + struct pci_dev *pdev __free(pci_dev_put) = + pci_get_slot(bridge->bus, PCI_DEVFN(0, 0)); + KUNIT_ASSERT_PTR_NE(test, NULL, pdev); + + KUNIT_EXPECT_PTR_EQ(test, pdev, hook_device_early); + KUNIT_EXPECT_PTR_EQ(test, pdev, hook_device_header); + KUNIT_EXPECT_PTR_EQ(test, pdev, hook_device_final); + KUNIT_EXPECT_PTR_EQ(test, NULL, hook_device_enable); + + KUNIT_EXPECT_EQ(test, 0, pci_enable_device(pdev)); + KUNIT_EXPECT_PTR_EQ(test, pdev, hook_device_enable); +} + +static void pci_fixup_nomatch_test(struct kunit *test) +{ + struct device *dev = kunit_device_register(test, DEVICE_NAME); + + KUNIT_ASSERT_PTR_NE(test, NULL, dev); + + test_conf_space = kunit_kzalloc(test, TEST_CONF_SIZE, GFP_KERNEL); + KUNIT_ASSERT_PTR_NE(test, NULL, test_conf_space); + + /* Minimal configuration space: a stub vendor and device ID */ + test_writew(test_conf_space + PCI_VENDOR_ID, TEST_VENDOR_ID + 1); + test_writew(test_conf_space + PCI_DEVICE_ID, TEST_DEVICE_ID); + + struct pci_host_bridge *bridge = devm_pci_alloc_host_bridge(dev, 0); + + KUNIT_ASSERT_PTR_NE(test, NULL, bridge); + bridge->ops = &test_ops; + + KUNIT_EXPECT_PTR_EQ(test, NULL, hook_device_early); + KUNIT_EXPECT_PTR_EQ(test, NULL, hook_device_header); + KUNIT_EXPECT_PTR_EQ(test, NULL, hook_device_final); + KUNIT_EXPECT_PTR_EQ(test, NULL, hook_device_enable); + + KUNIT_EXPECT_EQ(test, 0, pci_host_probe(bridge)); + + struct pci_dev *pdev __free(pci_dev_put) = + pci_get_slot(bridge->bus, PCI_DEVFN(0, 0)); + KUNIT_ASSERT_PTR_NE(test, NULL, pdev); + + KUNIT_EXPECT_PTR_EQ(test, NULL, hook_device_early); + KUNIT_EXPECT_PTR_EQ(test, NULL, hook_device_header); + KUNIT_EXPECT_PTR_EQ(test, NULL, hook_device_final); + KUNIT_EXPECT_PTR_EQ(test, NULL, hook_device_enable); + + KUNIT_EXPECT_EQ(test, 0, pci_enable_device(pdev)); + KUNIT_EXPECT_PTR_EQ(test, NULL, hook_device_enable); +} + +static struct kunit_case pci_fixup_test_cases[] = { + KUNIT_CASE(pci_fixup_match_test), + KUNIT_CASE(pci_fixup_nomatch_test), + {} +}; + +static struct kunit_suite pci_fixup_test_suite = { + .name = "pci_fixup_test_cases", + .test_cases = pci_fixup_test_cases, + .init = pci_fixup_test_init, +}; + +kunit_test_suite(pci_fixup_test_suite); +MODULE_DESCRIPTION("PCI fixups unit test suite"); +MODULE_LICENSE("GPL");
This is useful especially for KUnit tests, where we may want to dynamically add/remove PCI domains.
Signed-off-by: Brian Norris briannorris@chromium.org ---
arch/um/Kconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/um/Kconfig b/arch/um/Kconfig index 9083bfdb7735..7fccd63c3229 100644 --- a/arch/um/Kconfig +++ b/arch/um/Kconfig @@ -38,6 +38,7 @@ config UML select HAVE_ARCH_TRACEHOOK select HAVE_SYSCALL_TRACEPOINTS select THREAD_INFO_IN_TASK + select PCI_DOMAINS_GENERIC if PCI
config MMU bool
To get some more test coverage on PCI tests.
Signed-off-by: Brian Norris briannorris@chromium.org ---
tools/testing/kunit/qemu_configs/arm.py | 1 + tools/testing/kunit/qemu_configs/arm64.py | 1 + 2 files changed, 2 insertions(+)
diff --git a/tools/testing/kunit/qemu_configs/arm.py b/tools/testing/kunit/qemu_configs/arm.py index db2160200566..101d67e5157c 100644 --- a/tools/testing/kunit/qemu_configs/arm.py +++ b/tools/testing/kunit/qemu_configs/arm.py @@ -3,6 +3,7 @@ from ..qemu_config import QemuArchParams QEMU_ARCH = QemuArchParams(linux_arch='arm', kconfig=''' CONFIG_ARCH_VIRT=y +CONFIG_PCI=y CONFIG_SERIAL_AMBA_PL010=y CONFIG_SERIAL_AMBA_PL010_CONSOLE=y CONFIG_SERIAL_AMBA_PL011=y diff --git a/tools/testing/kunit/qemu_configs/arm64.py b/tools/testing/kunit/qemu_configs/arm64.py index 5c44d3a87e6d..ba2b4e660ba7 100644 --- a/tools/testing/kunit/qemu_configs/arm64.py +++ b/tools/testing/kunit/qemu_configs/arm64.py @@ -2,6 +2,7 @@ from ..qemu_config import QemuArchParams
QEMU_ARCH = QemuArchParams(linux_arch='arm64', kconfig=''' +CONFIG_PCI=y CONFIG_SERIAL_AMBA_PL010=y CONFIG_SERIAL_AMBA_PL010_CONSOLE=y CONFIG_SERIAL_AMBA_PL011=y
linux-kselftest-mirror@lists.linaro.org
-
Brian Norris