[Skiboot] [PATCH 1/7] Add hwprobe: no longer hard code probe order
Dan Horák
dan at danny.cz
Mon Jan 25 19:48:22 AEDT 2021
On Sun, 24 Jan 2021 16:58:00 -0800
Stewart Smith <stewart at flamingspork.com> wrote:
> hwprobe is a little system to have different hardware probing modules
> run in the dependency order they choose rather than hard coding
> that order in core/init.c
not sure if skiboot will ever switch to use LTO, but using the ELF
sections for storing data like the hwprobes here together with start/end
markers is not LTO-safe. The solution is to add an explicit "no_reorder"
attribute to the markers, please see for example
https://sigrok.org/bugzilla/show_bug.cgi?id=1433 for more details.
Dan
> Signed-off-by: Stewart Smith <stewart at flamingspork.com>
> ---
> core/Makefile.inc | 1 +
> core/hwprobe.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++
> core/init.c | 12 ++------
> hw/npu.c | 8 ++++++
> hw/npu2-common.c | 8 ++++++
> hw/npu3.c | 8 ++++++
> hw/phb3.c | 6 +++-
> hw/phb4.c | 6 ++++
> include/skiboot.h | 29 +++++++++++++++++++-
> skiboot.lds.S | 6 ++++
> 10 files changed, 142 insertions(+), 12 deletions(-)
> create mode 100644 core/hwprobe.c
>
> diff --git a/core/Makefile.inc b/core/Makefile.inc
> index 829800e5..f80019b6 100644
> --- a/core/Makefile.inc
> +++ b/core/Makefile.inc
> @@ -13,6 +13,7 @@ CORE_OBJS += timer.o i2c.o rtc.o flash.o sensor.o ipmi-opal.o
> CORE_OBJS += flash-subpartition.o bitmap.o buddy.o pci-quirk.o powercap.o psr.o
> CORE_OBJS += pci-dt-slot.o direct-controls.o cpufeatures.o
> CORE_OBJS += flash-firmware-versions.o opal-dump.o
> +CORE_OBJS += hwprobe.o
>
> ifeq ($(SKIBOOT_GCOV),1)
> CORE_OBJS += gcov-profiling.o
> diff --git a/core/hwprobe.c b/core/hwprobe.c
> new file mode 100644
> index 00000000..de331af4
> --- /dev/null
> +++ b/core/hwprobe.c
> @@ -0,0 +1,70 @@
> +// SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
> +/* Copyright 2021 Stewart Smith */
> +
> +#define pr_fmt(fmt) "HWPROBE: " fmt
> +#include <skiboot.h>
> +#include <string.h>
> +
> +static bool hwprobe_deps_satisfied(const struct hwprobe *hwp)
> +{
> + struct hwprobe *hwprobe;
> + const char *dep;
> + unsigned int i;
> +
> + if (hwp->deps == NULL)
> + return true;
> +
> + dep = hwp->deps[0];
> +
> + prlog(PR_TRACE, "Checking deps for %s\n", hwp->name);
> +
> + while (dep != NULL) {
> + prlog(PR_TRACE, "Checking %s dep %s\n", hwp->name, dep);
> + hwprobe = &__hwprobes_start;
> + for (i = 0; &hwprobe[i] < &__hwprobes_end; i++) {
> + if(strcmp(hwprobe[i].name,dep) == 0 &&
> + !hwprobe[i].probed)
> + return false;
> + }
> + dep++;
> + }
> +
> + prlog(PR_TRACE, "deps for %s are satisfied!\n", hwp->name);
> + return true;
> +
> +}
> +
> +void probe_hardware(void)
> +{
> + struct hwprobe *hwprobe;
> + unsigned int i;
> + bool work_todo = true;
> + bool did_something = true;
> +
> + while (work_todo) {
> + work_todo = false;
> + did_something = false;
> + hwprobe = &__hwprobes_start;
> + prlog(PR_DEBUG, "Begin loop\n");
> + for (i = 0; &hwprobe[i] < &__hwprobes_end; i++) {
> + if (hwprobe[i].probed)
> + continue;
> + if (hwprobe_deps_satisfied(&hwprobe[i])) {
> + prlog(PR_DEBUG, "Probing %s...\n", hwprobe[i].name);
> + if (hwprobe[i].probe)
> + hwprobe[i].probe();
> + did_something = true;
> + hwprobe[i].probed = true;
> + } else {
> + prlog(PR_DEBUG, "Dependencies for %s not yet satisfied, skipping\n",
> + hwprobe[i].name);
> + work_todo = true;
> + }
> + }
> +
> + if (work_todo && !did_something) {
> + prlog(PR_ERR, "Cannot satisfy dependencies! Bailing out\n");
> + break;
> + }
> + }
> +}
> diff --git a/core/init.c b/core/init.c
> index 4cb8f989..92811aa4 100644
> --- a/core/init.c
> +++ b/core/init.c
> @@ -1325,16 +1325,8 @@ void __noreturn __nomcount main_cpu_entry(const void *fdt)
> /* Init In-Memory Collection related stuff (load the IMC dtb into memory) */
> imc_init();
>
> - /* Probe PHB3 on P8 */
> - probe_phb3();
> -
> - /* Probe PHB4 on P9 */
> - probe_phb4();
> -
> - /* Probe NPUs */
> - probe_npu();
> - probe_npu2();
> - probe_npu3();
> + /* Probe all hardware we have code linked for (PHBs, NPUs etc)*/
> + probe_hardware();
>
> /* Initialize PCI */
> pci_init_slots();
> diff --git a/hw/npu.c b/hw/npu.c
> index dba7ee50..b747df2d 100644
> --- a/hw/npu.c
> +++ b/hw/npu.c
> @@ -1691,3 +1691,11 @@ void probe_npu(void)
> dt_for_each_compatible(dt_root, np, "ibm,power8-npu-pciex")
> npu_create_phb(np);
> }
> +
> +static const char* npu_deps[] = { "phb3", NULL };
> +
> +DECLARE_HWPROBE(npu) = {
> + .name = "npu",
> + .probe = probe_npu,
> + .deps = npu_deps
> +};
> diff --git a/hw/npu2-common.c b/hw/npu2-common.c
> index 3bc9bcee..aa71e16c 100644
> --- a/hw/npu2-common.c
> +++ b/hw/npu2-common.c
> @@ -679,3 +679,11 @@ void probe_npu2(void)
> setup_devices(npu);
> }
> }
> +
> +static const char* npu2_deps[] = { "phb4", NULL };
> +
> +DECLARE_HWPROBE(npu2) = {
> + .name = "npu2",
> + .probe = probe_npu2,
> + .deps = npu2_deps
> +};
> diff --git a/hw/npu3.c b/hw/npu3.c
> index 03461373..b0b957a7 100644
> --- a/hw/npu3.c
> +++ b/hw/npu3.c
> @@ -547,3 +547,11 @@ void probe_npu3(void)
> npu3_init(npu);
> }
> }
> +
> +static const char* npu3_deps[] = { "phb4", NULL };
> +
> +DECLARE_HWPROBE(npu3) = {
> + .name = "npu3",
> + .probe = probe_npu3,
> + .deps = npu3_deps
> +};
> diff --git a/hw/phb3.c b/hw/phb3.c
> index 8af6b616..d1ce8b97 100644
> --- a/hw/phb3.c
> +++ b/hw/phb3.c
> @@ -5049,4 +5049,8 @@ void probe_phb3(void)
> phb3_create(np);
> }
>
> -
> +DECLARE_HWPROBE(phb3) = {
> + .name = "phb3",
> + .probe = probe_phb3,
> + .deps = NULL
> +};
> diff --git a/hw/phb4.c b/hw/phb4.c
> index e7758d34..9060335a 100644
> --- a/hw/phb4.c
> +++ b/hw/phb4.c
> @@ -6082,3 +6082,9 @@ void probe_phb4(void)
> dt_for_each_compatible(dt_root, np, "ibm,power9-pciex")
> phb4_create(np);
> }
> +
> +DECLARE_HWPROBE(phb4) = {
> + .name = "phb4",
> + .probe = probe_phb4,
> + .deps = NULL
> +};
> diff --git a/include/skiboot.h b/include/skiboot.h
> index d33c0250..e2274041 100644
> --- a/include/skiboot.h
> +++ b/include/skiboot.h
> @@ -1,5 +1,7 @@
> // SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
> -/* Copyright 2013-2019 IBM Corp. */
> +/* Copyright 2013-2019 IBM Corp.
> + * Copyright 2021 Stewart Smith
> + */
>
> #ifndef __SKIBOOT_H
> #define __SKIBOOT_H
> @@ -345,4 +347,29 @@ extern int fake_nvram_info(uint32_t *total_size);
> extern int fake_nvram_start_read(void *dst, uint32_t src, uint32_t len);
> extern int fake_nvram_write(uint32_t offset, void *src, uint32_t size);
>
> +/*
> + * A bunch of hardware needs to be probed, sometimes in a particular order.
> + * Very simple dependency graph, with a even simpler way to resolve it.
> + * But it means we can now at link time choose what hardware we support.
> + */
> +struct hwprobe {
> + const char *name;
> + void (*probe)(void);
> +
> + bool probed; /* Internal use only */
> +
> + /*
> + * Must be NULL terminated list of strings
> + */
> + const char **deps;
> +};
> +
> +#define DECLARE_HWPROBE(name)\
> +static const struct hwprobe __used __section(".hwprobes") name ##_hwprobe
> +
> +extern struct hwprobe __hwprobes_start;
> +extern struct hwprobe __hwprobes_end;
> +
> +extern void probe_hardware(void);
> +
> #endif /* __SKIBOOT_H */
> diff --git a/skiboot.lds.S b/skiboot.lds.S
> index 5a7f9e31..c8e6e747 100644
> --- a/skiboot.lds.S
> +++ b/skiboot.lds.S
> @@ -164,6 +164,12 @@ SECTIONS
> __platforms_end = .;
> }
>
> + .hwprobes : {
> + __hwprobes_start = .;
> + KEEP(*(.hwprobes))
> + __hwprobes_end = .;
> + }
> +
> /* Relocations */
> . = ALIGN(0x10);
> .dynamic : {
> --
> 2.29.2
>
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot
More information about the Skiboot
mailing list