Nicholas Piggin <npig...@gmail.com> writes:

> Provide a dt_cpu_ftrs= cmdline option to disable the dt_cpu_ftrs CPU
> feature discovery, and fall back to the "cputable" based version.

I don't think this works properly.

> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c 
> b/arch/powerpc/kernel/dt_cpu_ftrs.c
> index fcc7588a96d6..050925b5b451 100644
> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
> @@ -775,6 +785,16 @@ bool __init dt_cpu_ftrs_in_use(void)
>  
>  bool __init dt_cpu_ftrs_init(void *fdt)
>  {
> +     if (!using_dt_cpu_ftrs) {
> +             /*
> +              * This should never happen because this runs before
> +              * early_praam, however if the init ordering changes,
> +              * test if early_param has disabled this.
> +              */
> +             return false;
> +     }
> +     using_dt_cpu_ftrs = false;
> +
>       /* Setup and verify the FDT, if it fails we just bail */
>       if (!early_init_dt_verify(fdt))
>               return false;
...
        return true;

Because this runs before early_param(), as you mention,
dt_cpu_ftrs_init() returns true, which means we skip calling
identify_cpu().

So although passing dt_cpu_ftrs=off will skip the later logic, it
doesn't cause us to call identify_cpu() in early_setup() which it
should.

In practice it works because the base CPU spec that we initialise in
dt_cpu_ftrs.c is mostly OK, and skiboot also adds the cpu-version
property which causes us to call identify_cpu() again later, but that's
all a bit fragile IMHO.

So unfortunately I think we need to add logic in dt_cpu_ftrs_init() to
look for dt_cpu_ftrs=off on the command line.

The patch below seems to work, but would appreciate more eyes on it.

cheers

diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c 
b/arch/powerpc/kernel/dt_cpu_ftrs.c
index d6f05e4dc328..9a560954498a 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -8,6 +8,7 @@
 #include <linux/export.h>
 #include <linux/init.h>
 #include <linux/jump_label.h>
+#include <linux/libfdt.h>
 #include <linux/memblock.h>
 #include <linux/printk.h>
 #include <linux/sched.h>
@@ -767,6 +770,26 @@ static void __init cpufeatures_setup_finished(void)
                cur_cpu_spec->cpu_features, cur_cpu_spec->mmu_features);
 }
 
+static int __init disabled_on_cmdline(void)
+{
+       unsigned long root, chosen;
+       const char *p;
+
+       root = of_get_flat_dt_root();
+       chosen = of_get_flat_dt_subnode_by_name(root, "chosen");
+       if (chosen == -FDT_ERR_NOTFOUND)
+               return false;
+
+       p = of_get_flat_dt_prop(chosen, "bootargs", NULL);
+       if (!p)
+               return false;
+
+       if (strstr(p, "dt_cpu_ftrs=off"))
+               return true;
+
+       return false;
+}
+
 static int __init fdt_find_cpu_features(unsigned long node, const char *uname,
                                        int depth, void *data)
 {
@@ -801,6 +826,9 @@ bool __init dt_cpu_ftrs_init(void *fdt)
        if (!of_scan_flat_dt(fdt_find_cpu_features, NULL))
                return false;
 
+       if (disabled_on_cmdline())
+               return false;
+
        cpufeatures_setup_cpu();
 
        using_dt_cpu_ftrs = true;

Reply via email to