diff mbox series

[v2,8/8] RISC-V: Assign hwcap only according to current cpu.

Message ID 1546940318-9752-9-git-send-email-atish.patra@wdc.com (mailing list archive)
State New, archived
Headers show
Series Various SMP related fixes | expand

Commit Message

Atish Patra Jan. 8, 2019, 9:38 a.m. UTC
Currently, we set hwcap based on first valid cpu from
DT. This may not be correct always as that CPU might not
be current booting cpu.

Set hwcap based on the current cpu instead of first
valid CPU from DT.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 arch/riscv/kernel/cpufeature.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Comments

Atish Patra Jan. 8, 2019, 10:33 a.m. UTC | #1
On 1/8/19 1:38 AM, Atish Patra wrote:
> Currently, we set hwcap based on first valid cpu from
> DT. This may not be correct always as that CPU might not
> be current booting cpu.
> 
> Set hwcap based on the current cpu instead of first
> valid CPU from DT.
> 
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
>   arch/riscv/kernel/cpufeature.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index a6e369ed..ed6122ff 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -43,12 +43,15 @@ void riscv_fill_hwcap(void)
>   	elf_hwcap = 0;
>   
>   	/*
> -	 * We don't support running Linux on hertergenous ISA systems.  For
> -	 * now, we just check the ISA of the first "okay" processor.
> +	 * We don't support running Linux on hertergenous ISA systems.
> +	 * But first "okay" processor might not be the boot cpu.
> +	 * Check the ISA of boot cpu.
>   	 */
> -	while ((node = of_find_node_by_type(node, "cpu")))
> -		if (riscv_of_processor_hartid(node) >= 0)
> +	while ((node = of_find_node_by_type(node, "cpu"))) {
> +		if (riscv_of_processor_hartid(node) == boot_cpu_hartid)
>   			break;
> +	}
> +
>   	if (!node) {
>   		pr_warning("Unable to find \"cpu\" devicetree entry");
>   		return;
> 

Argh..Missed an include while rebasing. Following edit is required for 
non SMP config. I will fix it in v2.

diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index ed6122ff..78379ea3 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -20,6 +20,7 @@
  #include <linux/of.h>
  #include <asm/processor.h>
  #include <asm/hwcap.h>
+#include <asm/smp.h>

Regards,
Atish
Anup Patel Jan. 8, 2019, 12:01 p.m. UTC | #2
On Tue, Jan 8, 2019 at 3:08 PM Atish Patra <atish.patra@wdc.com> wrote:
>
> Currently, we set hwcap based on first valid cpu from
> DT. This may not be correct always as that CPU might not
> be current booting cpu.
>
> Set hwcap based on the current cpu instead of first
> valid CPU from DT.
>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
>  arch/riscv/kernel/cpufeature.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index a6e369ed..ed6122ff 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -43,12 +43,15 @@ void riscv_fill_hwcap(void)
>         elf_hwcap = 0;
>
>         /*
> -        * We don't support running Linux on hertergenous ISA systems.  For
> -        * now, we just check the ISA of the first "okay" processor.
> +        * We don't support running Linux on hertergenous ISA systems.
> +        * But first "okay" processor might not be the boot cpu.
> +        * Check the ISA of boot cpu.
>          */
> -       while ((node = of_find_node_by_type(node, "cpu")))
> -               if (riscv_of_processor_hartid(node) >= 0)
> +       while ((node = of_find_node_by_type(node, "cpu"))) {
> +               if (riscv_of_processor_hartid(node) == boot_cpu_hartid)
>                         break;
> +       }
> +
>         if (!node) {
>                 pr_warning("Unable to find \"cpu\" devicetree entry");
>                 return;
> --
> 2.7.4
>

Looks good to me.

Reviewed-by: Anup Patel <anup@brainfault.org>

Regards,
Anup
Christoph Hellwig Jan. 15, 2019, 1:56 p.m. UTC | #3
On Tue, Jan 08, 2019 at 01:38:38AM -0800, Atish Patra wrote:
> Currently, we set hwcap based on first valid cpu from
> DT. This may not be correct always as that CPU might not
> be current booting cpu.
> 
> Set hwcap based on the current cpu instead of first
> valid CPU from DT.

This is generally the right thing to do.  But can the kernel even cope
with different hwcaps per hart?  I know arm land and I think x86 as well
don't, so we might want to add a sanity check that they match or reduce
them to the common subset.
Atish Patra Jan. 18, 2019, 2:13 a.m. UTC | #4
On 1/15/19 5:56 AM, Christoph Hellwig wrote:
> On Tue, Jan 08, 2019 at 01:38:38AM -0800, Atish Patra wrote:
>> Currently, we set hwcap based on first valid cpu from
>> DT. This may not be correct always as that CPU might not
>> be current booting cpu.
>>
>> Set hwcap based on the current cpu instead of first
>> valid CPU from DT.
> 
> This is generally the right thing to do.  But can the kernel even cope
> with different hwcaps per hart?

I don't think so.
   I know arm land and I think x86 as well
> don't, so we might want to add a sanity check that they match or reduce
> them to the common subset.
> 

I will add a sanity check that they match and throw a warning if they 
don't.

Regards,
Atish
diff mbox series

Patch

diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index a6e369ed..ed6122ff 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -43,12 +43,15 @@  void riscv_fill_hwcap(void)
 	elf_hwcap = 0;
 
 	/*
-	 * We don't support running Linux on hertergenous ISA systems.  For
-	 * now, we just check the ISA of the first "okay" processor.
+	 * We don't support running Linux on hertergenous ISA systems.
+	 * But first "okay" processor might not be the boot cpu.
+	 * Check the ISA of boot cpu.
 	 */
-	while ((node = of_find_node_by_type(node, "cpu")))
-		if (riscv_of_processor_hartid(node) >= 0)
+	while ((node = of_find_node_by_type(node, "cpu"))) {
+		if (riscv_of_processor_hartid(node) == boot_cpu_hartid)
 			break;
+	}
+
 	if (!node) {
 		pr_warning("Unable to find \"cpu\" devicetree entry");
 		return;