Linux-RISC-V Archive on lore.kernel.org
 help / Atom feed
* [PATCH 0/3] Non-smp configuration fix
@ 2018-12-26 23:08 Atish Patra
  2018-12-26 23:08 ` [PATCH 1/3] RISC-V: Do not wait indefinitely in __cpu_up Atish Patra
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Atish Patra @ 2018-12-26 23:08 UTC (permalink / raw)
  To: linux-riscv
  Cc: Rob Herring, Patrick Stählin, Albert Ou, Damien Le Moal,
	Jason Cooper, Dmitriy Cherkasov, Anup Patel, Daniel Lezcano,
	linux-kernel, Michael Clark, Atish Patra, Palmer Dabbelt,
	Marc Zyngier, Thomas Gleixner

The existing upstream kernel doesn't boot for non-smp
configuration. This patch series address various issues
with non-smp configurations.

Tested on QEMU and HiFive Unleashed board.

Atish Patra (3):
RISC-V: Do not wait indefinitely in __cpu_up
RISC-V: Move cpuid to hartid mapping to SMP.
RISC-V: Fix non-smp kernel boot on SMP systems

arch/riscv/include/asm/smp.h      | 13 +++++++++++--
arch/riscv/kernel/cpu.c           |  4 ----
arch/riscv/kernel/setup.c         |  2 ++
arch/riscv/kernel/smp.c           |  1 -
arch/riscv/kernel/smpboot.c       | 19 ++++++++++++++++---
drivers/clocksource/riscv_timer.c | 21 ++++++++++++++++++---
drivers/irqchip/irq-sifive-plic.c |  5 +++++
7 files changed, 52 insertions(+), 13 deletions(-)

--
2.7.4


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/3] RISC-V: Do not wait indefinitely in __cpu_up
  2018-12-26 23:08 [PATCH 0/3] Non-smp configuration fix Atish Patra
@ 2018-12-26 23:08 ` Atish Patra
  2018-12-27  3:36   ` Anup Patel
  2018-12-26 23:09 ` [PATCH 2/3] RISC-V: Move cpuid to hartid mapping to SMP Atish Patra
  2018-12-26 23:09 ` [PATCH 3/3] RISC-V: Fix non-smp kernel boot on SMP systems Atish Patra
  2 siblings, 1 reply; 9+ messages in thread
From: Atish Patra @ 2018-12-26 23:08 UTC (permalink / raw)
  To: linux-riscv
  Cc: Rob Herring, Patrick Stählin, Albert Ou, Damien Le Moal,
	Jason Cooper, Dmitriy Cherkasov, Anup Patel, Daniel Lezcano,
	linux-kernel, Michael Clark, Atish Patra, Palmer Dabbelt,
	Marc Zyngier, Thomas Gleixner

In SMP path, __cpu_up waits for other CPU to come online
indefinitely. This is wrong as other CPU might be disabled
in machine mode and possible CPU is set to the cpus present
in DT.

Introduce a completion variable and waits only for a second.

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

diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 18cda0e8..bb8cd242 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -39,6 +39,7 @@
 
 void *__cpu_up_stack_pointer[NR_CPUS];
 void *__cpu_up_task_pointer[NR_CPUS];
+static DECLARE_COMPLETION(cpu_running);
 
 void __init smp_prepare_boot_cpu(void)
 {
@@ -77,6 +78,7 @@ void __init setup_smp(void)
 
 int __cpu_up(unsigned int cpu, struct task_struct *tidle)
 {
+	int ret = 0;
 	int hartid = cpuid_to_hartid_map(cpu);
 	tidle->thread_info.cpu = cpu;
 
@@ -92,10 +94,15 @@ int __cpu_up(unsigned int cpu, struct task_struct *tidle)
 		  task_stack_page(tidle) + THREAD_SIZE);
 	WRITE_ONCE(__cpu_up_task_pointer[hartid], tidle);
 
-	while (!cpu_online(cpu))
-		cpu_relax();
+	wait_for_completion_timeout(&cpu_running,
+					    msecs_to_jiffies(1000));
 
-	return 0;
+	if (!cpu_online(cpu)) {
+		pr_crit("CPU%u: failed to come online\n", cpu);
+		ret = -EIO;
+	}
+
+	return ret;
 }
 
 void __init smp_cpus_done(unsigned int max_cpus)
@@ -121,6 +128,7 @@ asmlinkage void __init smp_callin(void)
 	 * a local TLB flush right now just in case.
 	 */
 	local_flush_tlb_all();
+	complete(&cpu_running);
 	/*
 	 * Disable preemption before enabling interrupts, so we don't try to
 	 * schedule a CPU that hasn't actually started yet.
-- 
2.7.4


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/3] RISC-V: Move cpuid to hartid mapping to SMP.
  2018-12-26 23:08 [PATCH 0/3] Non-smp configuration fix Atish Patra
  2018-12-26 23:08 ` [PATCH 1/3] RISC-V: Do not wait indefinitely in __cpu_up Atish Patra
@ 2018-12-26 23:09 ` Atish Patra
  2018-12-27  3:37   ` Anup Patel
  2018-12-26 23:09 ` [PATCH 3/3] RISC-V: Fix non-smp kernel boot on SMP systems Atish Patra
  2 siblings, 1 reply; 9+ messages in thread
From: Atish Patra @ 2018-12-26 23:09 UTC (permalink / raw)
  To: linux-riscv
  Cc: Rob Herring, Patrick Stählin, Albert Ou, Damien Le Moal,
	Jason Cooper, Dmitriy Cherkasov, Anup Patel, Daniel Lezcano,
	linux-kernel, Michael Clark, Atish Patra, Palmer Dabbelt,
	Marc Zyngier, Thomas Gleixner

Currently, logical CPU id to physical hartid mapping is
defined for both smp and non-smp configurations. This
is not required as we need this only for smp configuration.
The mapping function can define directly boot_cpu_hartid
for non-smp usecase.

The reverse mapping function i.e. hartid to cpuid can be called
for any valid but not booted harts. So it should return default
cpu 0 only if it is a boot hartid

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 arch/riscv/include/asm/smp.h | 13 +++++++++++--
 arch/riscv/kernel/setup.c    |  2 ++
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h
index 41aa73b4..8f30300f 100644
--- a/arch/riscv/include/asm/smp.h
+++ b/arch/riscv/include/asm/smp.h
@@ -22,12 +22,13 @@
 /*
  * Mapping between linux logical cpu index and hartid.
  */
-extern unsigned long __cpuid_to_hartid_map[NR_CPUS];
-#define cpuid_to_hartid_map(cpu)    __cpuid_to_hartid_map[cpu]
 
+extern unsigned long boot_cpu_hartid;
 struct seq_file;
 
 #ifdef CONFIG_SMP
+extern unsigned long __cpuid_to_hartid_map[NR_CPUS];
+#define cpuid_to_hartid_map(cpu)    __cpuid_to_hartid_map[cpu]
 
 /* print IPI stats */
 void show_ipi_stats(struct seq_file *p, int prec);
@@ -58,7 +59,15 @@ static inline void show_ipi_stats(struct seq_file *p, int prec)
 
 static inline int riscv_hartid_to_cpuid(int hartid)
 {
+	if (hartid == boot_cpu_hartid)
 		return 0;
+	else
+		return -1;
+}
+static inline unsigned long cpuid_to_hartid_map(int cpu)
+{
+
+	return boot_cpu_hartid;
 }
 
 static inline void riscv_cpuid_to_hartid_mask(const struct cpumask *in,
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index 2c290e6a..bd4d7b85 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -83,6 +83,7 @@ EXPORT_SYMBOL(empty_zero_page);
 atomic_t hart_lottery;
 unsigned long boot_cpu_hartid;
 
+#ifdef CONFIG_SMP
 unsigned long __cpuid_to_hartid_map[NR_CPUS] = {
 	[0 ... NR_CPUS-1] = INVALID_HARTID
 };
@@ -91,6 +92,7 @@ void __init smp_setup_processor_id(void)
 {
 	cpuid_to_hartid_map(0) = boot_cpu_hartid;
 }
+#endif
 
 #ifdef CONFIG_BLK_DEV_INITRD
 static void __init setup_initrd(void)
-- 
2.7.4


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3/3] RISC-V: Fix non-smp kernel boot on SMP systems
  2018-12-26 23:08 [PATCH 0/3] Non-smp configuration fix Atish Patra
  2018-12-26 23:08 ` [PATCH 1/3] RISC-V: Do not wait indefinitely in __cpu_up Atish Patra
  2018-12-26 23:09 ` [PATCH 2/3] RISC-V: Move cpuid to hartid mapping to SMP Atish Patra
@ 2018-12-26 23:09 ` Atish Patra
  2018-12-27  3:58   ` Anup Patel
  2 siblings, 1 reply; 9+ messages in thread
From: Atish Patra @ 2018-12-26 23:09 UTC (permalink / raw)
  To: linux-riscv
  Cc: Rob Herring, Patrick Stählin, Albert Ou, Damien Le Moal,
	Jason Cooper, Dmitriy Cherkasov, Anup Patel, Daniel Lezcano,
	linux-kernel, Michael Clark, Atish Patra, Palmer Dabbelt,
	Marc Zyngier, Thomas Gleixner

In non-smp configuration, hartid can be higher that NR_CPUS.
riscv_of_processor_hartid should not be compared to hartid to
NR_CPUS in that case. Moreover, this function checks all the
DT properties of a hart node. NR_CPUS comparison seems out of
place.

Do cpuid comparison with NR_CPUs in smp setup code. Update the
drivers to handle appropriate code as well.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 arch/riscv/kernel/cpu.c           |  4 ----
 arch/riscv/kernel/smp.c           |  1 -
 arch/riscv/kernel/smpboot.c       |  5 +++++
 drivers/clocksource/riscv_timer.c | 21 ++++++++++++++++++---
 drivers/irqchip/irq-sifive-plic.c |  5 +++++
 5 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index b4a7d442..251ffab6 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -34,10 +34,6 @@ int riscv_of_processor_hartid(struct device_node *node)
 		pr_warn("Found CPU without hart ID\n");
 		return -(ENODEV);
 	}
-	if (hart >= NR_CPUS) {
-		pr_info("Found hart ID %d, which is above NR_CPUs.  Disabling this hart\n", hart);
-		return -(ENODEV);
-	}
 
 	if (of_property_read_string(node, "status", &status)) {
 		pr_warn("CPU with hartid=%d has no \"status\" property\n", hart);
diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
index 57b1383e..9ea7ac7d 100644
--- a/arch/riscv/kernel/smp.c
+++ b/arch/riscv/kernel/smp.c
@@ -49,7 +49,6 @@ int riscv_hartid_to_cpuid(int hartid)
 			return i;
 
 	pr_err("Couldn't find cpu id for hartid [%d]\n", hartid);
-	BUG();
 	return i;
 }
 
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index bb8cd242..05291840 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -66,6 +66,11 @@ void __init setup_smp(void)
 			found_boot_cpu = 1;
 			continue;
 		}
+		if (cpuid >= NR_CPUS) {
+			pr_warn("Invalid cpuid [%d] for hartid [%d]\n",
+				cpuid, hart);
+			break;
+		}
 
 		cpuid_to_hartid_map(cpuid) = hart;
 		set_cpu_possible(cpuid, true);
diff --git a/drivers/clocksource/riscv_timer.c b/drivers/clocksource/riscv_timer.c
index 084e97dc..acf2af10 100644
--- a/drivers/clocksource/riscv_timer.c
+++ b/drivers/clocksource/riscv_timer.c
@@ -89,20 +89,35 @@ static int __init riscv_timer_init_dt(struct device_node *n)
 	struct clocksource *cs;
 
 	hartid = riscv_of_processor_hartid(n);
+	if (hartid < 0) {
+		pr_warn("Not valid hartid for node [%pOF] error = [%d]\n",
+			n, hartid);
+		return hartid;
+	}
 	cpuid = riscv_hartid_to_cpuid(hartid);
 
+	if (cpuid < 0)
+		pr_warn("Invalid cpuid for hartid [%d]\n", hartid);
+
 	if (cpuid != smp_processor_id())
 		return 0;
 
+	pr_err("%s: Registering clocksource cpuid [%d] hartid [%d]\n",
+	       __func__, cpuid, hartid);
 	cs = per_cpu_ptr(&riscv_clocksource, cpuid);
-	clocksource_register_hz(cs, riscv_timebase);
+	error = clocksource_register_hz(cs, riscv_timebase);
 
+	if (error) {
+		pr_err("RISCV timer register failed [%d] for cpu = [%d]\n",
+		       error, cpuid);
+		return error;
+	}
 	error = cpuhp_setup_state(CPUHP_AP_RISCV_TIMER_STARTING,
 			 "clockevents/riscv/timer:starting",
 			 riscv_timer_starting_cpu, riscv_timer_dying_cpu);
 	if (error)
-		pr_err("RISCV timer register failed [%d] for cpu = [%d]\n",
-		       error, cpuid);
+		pr_err("cpu hp setup state failed for RISCV timer [%d]\n",
+		       error);
 	return error;
 }
 
diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
index 357e9daf..254ecd76 100644
--- a/drivers/irqchip/irq-sifive-plic.c
+++ b/drivers/irqchip/irq-sifive-plic.c
@@ -237,6 +237,11 @@ static int __init plic_init(struct device_node *node,
 		}
 
 		cpu = riscv_hartid_to_cpuid(hartid);
+		if (cpu < 0) {
+			pr_warn("Invalid cpuid for context %d\n", i);
+			continue;
+		}
+
 		handler = per_cpu_ptr(&plic_handlers, cpu);
 		handler->present = true;
 		handler->ctxid = i;
-- 
2.7.4


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] RISC-V: Do not wait indefinitely in __cpu_up
  2018-12-26 23:08 ` [PATCH 1/3] RISC-V: Do not wait indefinitely in __cpu_up Atish Patra
@ 2018-12-27  3:36   ` Anup Patel
  0 siblings, 0 replies; 9+ messages in thread
From: Anup Patel @ 2018-12-27  3:36 UTC (permalink / raw)
  To: Atish Patra
  Cc: Rob Herring, Patrick Stählin, Albert Ou, Jason Cooper,
	Dmitriy Cherkasov, Damien Le Moal, Daniel Lezcano,
	linux-kernel@vger.kernel.org List, Michael Clark, Marc Zyngier,
	Palmer Dabbelt, linux-riscv, Thomas Gleixner

On Thu, Dec 27, 2018 at 4:39 AM Atish Patra <atish.patra@wdc.com> wrote:
>
> In SMP path, __cpu_up waits for other CPU to come online
> indefinitely. This is wrong as other CPU might be disabled
> in machine mode and possible CPU is set to the cpus present
> in DT.
>
> Introduce a completion variable and waits only for a second.
>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
>  arch/riscv/kernel/smpboot.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> index 18cda0e8..bb8cd242 100644
> --- a/arch/riscv/kernel/smpboot.c
> +++ b/arch/riscv/kernel/smpboot.c
> @@ -39,6 +39,7 @@
>
>  void *__cpu_up_stack_pointer[NR_CPUS];
>  void *__cpu_up_task_pointer[NR_CPUS];
> +static DECLARE_COMPLETION(cpu_running);
>
>  void __init smp_prepare_boot_cpu(void)
>  {
> @@ -77,6 +78,7 @@ void __init setup_smp(void)
>
>  int __cpu_up(unsigned int cpu, struct task_struct *tidle)
>  {
> +       int ret = 0;
>         int hartid = cpuid_to_hartid_map(cpu);
>         tidle->thread_info.cpu = cpu;
>
> @@ -92,10 +94,15 @@ int __cpu_up(unsigned int cpu, struct task_struct *tidle)
>                   task_stack_page(tidle) + THREAD_SIZE);
>         WRITE_ONCE(__cpu_up_task_pointer[hartid], tidle);
>
> -       while (!cpu_online(cpu))
> -               cpu_relax();
> +       wait_for_completion_timeout(&cpu_running,
> +                                           msecs_to_jiffies(1000));
>
> -       return 0;
> +       if (!cpu_online(cpu)) {
> +               pr_crit("CPU%u: failed to come online\n", cpu);
> +               ret = -EIO;
> +       }
> +
> +       return ret;
>  }
>
>  void __init smp_cpus_done(unsigned int max_cpus)
> @@ -121,6 +128,7 @@ asmlinkage void __init smp_callin(void)
>          * a local TLB flush right now just in case.
>          */
>         local_flush_tlb_all();
> +       complete(&cpu_running);
>         /*
>          * Disable preemption before enabling interrupts, so we don't try to
>          * schedule a CPU that hasn't actually started yet.
> --
> 2.7.4
>

Looks good to me.

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] RISC-V: Move cpuid to hartid mapping to SMP.
  2018-12-26 23:09 ` [PATCH 2/3] RISC-V: Move cpuid to hartid mapping to SMP Atish Patra
@ 2018-12-27  3:37   ` Anup Patel
  2019-01-06  2:13     ` Atish Patra
  0 siblings, 1 reply; 9+ messages in thread
From: Anup Patel @ 2018-12-27  3:37 UTC (permalink / raw)
  To: Atish Patra
  Cc: Rob Herring, Patrick Stählin, Albert Ou, Jason Cooper,
	Dmitriy Cherkasov, Damien Le Moal, Daniel Lezcano,
	linux-kernel@vger.kernel.org List, Michael Clark, Marc Zyngier,
	Palmer Dabbelt, linux-riscv, Thomas Gleixner

On Thu, Dec 27, 2018 at 4:39 AM Atish Patra <atish.patra@wdc.com> wrote:
>
> Currently, logical CPU id to physical hartid mapping is
> defined for both smp and non-smp configurations. This
> is not required as we need this only for smp configuration.
> The mapping function can define directly boot_cpu_hartid
> for non-smp usecase.
>
> The reverse mapping function i.e. hartid to cpuid can be called
> for any valid but not booted harts. So it should return default
> cpu 0 only if it is a boot hartid
>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
>  arch/riscv/include/asm/smp.h | 13 +++++++++++--
>  arch/riscv/kernel/setup.c    |  2 ++
>  2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h
> index 41aa73b4..8f30300f 100644
> --- a/arch/riscv/include/asm/smp.h
> +++ b/arch/riscv/include/asm/smp.h
> @@ -22,12 +22,13 @@
>  /*
>   * Mapping between linux logical cpu index and hartid.
>   */
> -extern unsigned long __cpuid_to_hartid_map[NR_CPUS];
> -#define cpuid_to_hartid_map(cpu)    __cpuid_to_hartid_map[cpu]
>
> +extern unsigned long boot_cpu_hartid;
>  struct seq_file;
>
>  #ifdef CONFIG_SMP
> +extern unsigned long __cpuid_to_hartid_map[NR_CPUS];
> +#define cpuid_to_hartid_map(cpu)    __cpuid_to_hartid_map[cpu]
>
>  /* print IPI stats */
>  void show_ipi_stats(struct seq_file *p, int prec);
> @@ -58,7 +59,15 @@ static inline void show_ipi_stats(struct seq_file *p, int prec)
>
>  static inline int riscv_hartid_to_cpuid(int hartid)
>  {
> +       if (hartid == boot_cpu_hartid)
>                 return 0;
> +       else
> +               return -1;
> +}
> +static inline unsigned long cpuid_to_hartid_map(int cpu)
> +{
> +
> +       return boot_cpu_hartid;
>  }
>
>  static inline void riscv_cpuid_to_hartid_mask(const struct cpumask *in,
> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> index 2c290e6a..bd4d7b85 100644
> --- a/arch/riscv/kernel/setup.c
> +++ b/arch/riscv/kernel/setup.c
> @@ -83,6 +83,7 @@ EXPORT_SYMBOL(empty_zero_page);
>  atomic_t hart_lottery;
>  unsigned long boot_cpu_hartid;
>
> +#ifdef CONFIG_SMP
>  unsigned long __cpuid_to_hartid_map[NR_CPUS] = {
>         [0 ... NR_CPUS-1] = INVALID_HARTID
>  };
> @@ -91,6 +92,7 @@ void __init smp_setup_processor_id(void)
>  {
>         cpuid_to_hartid_map(0) = boot_cpu_hartid;
>  }
> +#endif

Please move __cpuid_to_hartid_map[] and smp_setup_processor_id()
to arch/riscv/kernel/smp.c

Otherwise, looks good to me.

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] RISC-V: Fix non-smp kernel boot on SMP systems
  2018-12-26 23:09 ` [PATCH 3/3] RISC-V: Fix non-smp kernel boot on SMP systems Atish Patra
@ 2018-12-27  3:58   ` Anup Patel
  2019-01-06  2:14     ` Atish Patra
  0 siblings, 1 reply; 9+ messages in thread
From: Anup Patel @ 2018-12-27  3:58 UTC (permalink / raw)
  To: Atish Patra
  Cc: Rob Herring, Patrick Stählin, Albert Ou, Jason Cooper,
	Dmitriy Cherkasov, Damien Le Moal, Daniel Lezcano,
	linux-kernel@vger.kernel.org List, Michael Clark, Marc Zyngier,
	Palmer Dabbelt, linux-riscv, Thomas Gleixner

On Thu, Dec 27, 2018 at 4:39 AM Atish Patra <atish.patra@wdc.com> wrote:
>
> In non-smp configuration, hartid can be higher that NR_CPUS.
> riscv_of_processor_hartid should not be compared to hartid to
> NR_CPUS in that case. Moreover, this function checks all the
> DT properties of a hart node. NR_CPUS comparison seems out of
> place.

This only explains change in arch/riscv/kernel/cpu.c

Create separate patch for it.

>
> Do cpuid comparison with NR_CPUs in smp setup code. Update the

Create separate patch for change in arch/riscv/kernel/smp.c

> drivers to handle appropriate code as well.

Create separate patches for riscv_timer and irq-sifive-plic.c
because they will probably go via different gitrepos.

>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
>  arch/riscv/kernel/cpu.c           |  4 ----
>  arch/riscv/kernel/smp.c           |  1 -
>  arch/riscv/kernel/smpboot.c       |  5 +++++
>  drivers/clocksource/riscv_timer.c | 21 ++++++++++++++++++---
>  drivers/irqchip/irq-sifive-plic.c |  5 +++++
>  5 files changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index b4a7d442..251ffab6 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -34,10 +34,6 @@ int riscv_of_processor_hartid(struct device_node *node)
>                 pr_warn("Found CPU without hart ID\n");
>                 return -(ENODEV);
>         }
> -       if (hart >= NR_CPUS) {
> -               pr_info("Found hart ID %d, which is above NR_CPUs.  Disabling this hart\n", hart);
> -               return -(ENODEV);
> -       }
>
>         if (of_property_read_string(node, "status", &status)) {
>                 pr_warn("CPU with hartid=%d has no \"status\" property\n", hart);
> diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
> index 57b1383e..9ea7ac7d 100644
> --- a/arch/riscv/kernel/smp.c
> +++ b/arch/riscv/kernel/smp.c
> @@ -49,7 +49,6 @@ int riscv_hartid_to_cpuid(int hartid)
>                         return i;
>
>         pr_err("Couldn't find cpu id for hartid [%d]\n", hartid);
> -       BUG();

Have a separate patch with explanation about why
we don't need BUG() here.

>         return i;
>  }
>
> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> index bb8cd242..05291840 100644
> --- a/arch/riscv/kernel/smpboot.c
> +++ b/arch/riscv/kernel/smpboot.c
> @@ -66,6 +66,11 @@ void __init setup_smp(void)
>                         found_boot_cpu = 1;
>                         continue;
>                 }
> +               if (cpuid >= NR_CPUS) {
> +                       pr_warn("Invalid cpuid [%d] for hartid [%d]\n",
> +                               cpuid, hart);
> +                       break;
> +               }
>
>                 cpuid_to_hartid_map(cpuid) = hart;
>                 set_cpu_possible(cpuid, true);
> diff --git a/drivers/clocksource/riscv_timer.c b/drivers/clocksource/riscv_timer.c
> index 084e97dc..acf2af10 100644
> --- a/drivers/clocksource/riscv_timer.c
> +++ b/drivers/clocksource/riscv_timer.c
> @@ -89,20 +89,35 @@ static int __init riscv_timer_init_dt(struct device_node *n)
>         struct clocksource *cs;
>
>         hartid = riscv_of_processor_hartid(n);
> +       if (hartid < 0) {
> +               pr_warn("Not valid hartid for node [%pOF] error = [%d]\n",
> +                       n, hartid);
> +               return hartid;
> +       }
>         cpuid = riscv_hartid_to_cpuid(hartid);
>
> +       if (cpuid < 0)
> +               pr_warn("Invalid cpuid for hartid [%d]\n", hartid);
> +
>         if (cpuid != smp_processor_id())
>                 return 0;
>
> +       pr_err("%s: Registering clocksource cpuid [%d] hartid [%d]\n",
> +              __func__, cpuid, hartid);
>         cs = per_cpu_ptr(&riscv_clocksource, cpuid);
> -       clocksource_register_hz(cs, riscv_timebase);
> +       error = clocksource_register_hz(cs, riscv_timebase);
>
> +       if (error) {
> +               pr_err("RISCV timer register failed [%d] for cpu = [%d]\n",
> +                      error, cpuid);
> +               return error;
> +       }
>         error = cpuhp_setup_state(CPUHP_AP_RISCV_TIMER_STARTING,
>                          "clockevents/riscv/timer:starting",
>                          riscv_timer_starting_cpu, riscv_timer_dying_cpu);
>         if (error)
> -               pr_err("RISCV timer register failed [%d] for cpu = [%d]\n",
> -                      error, cpuid);
> +               pr_err("cpu hp setup state failed for RISCV timer [%d]\n",
> +                      error);
>         return error;
>  }
>
> diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
> index 357e9daf..254ecd76 100644
> --- a/drivers/irqchip/irq-sifive-plic.c
> +++ b/drivers/irqchip/irq-sifive-plic.c
> @@ -237,6 +237,11 @@ static int __init plic_init(struct device_node *node,
>                 }
>
>                 cpu = riscv_hartid_to_cpuid(hartid);
> +               if (cpu < 0) {
> +                       pr_warn("Invalid cpuid for context %d\n", i);
> +                       continue;
> +               }
> +
>                 handler = per_cpu_ptr(&plic_handlers, cpu);
>                 handler->present = true;
>                 handler->ctxid = i;
> --
> 2.7.4
>

Regards,
Anup

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] RISC-V: Move cpuid to hartid mapping to SMP.
  2018-12-27  3:37   ` Anup Patel
@ 2019-01-06  2:13     ` Atish Patra
  0 siblings, 0 replies; 9+ messages in thread
From: Atish Patra @ 2019-01-06  2:13 UTC (permalink / raw)
  To: Anup Patel
  Cc: Rob Herring, Patrick Stählin, Albert Ou, Jason Cooper,
	Dmitriy Cherkasov, Damien Le Moal, Daniel Lezcano,
	linux-kernel@vger.kernel.org List, Michael Clark, Marc Zyngier,
	Palmer Dabbelt, linux-riscv, Thomas Gleixner

On 12/26/18 7:38 PM, Anup Patel wrote:
> On Thu, Dec 27, 2018 at 4:39 AM Atish Patra <atish.patra@wdc.com> wrote:
>>
>> Currently, logical CPU id to physical hartid mapping is
>> defined for both smp and non-smp configurations. This
>> is not required as we need this only for smp configuration.
>> The mapping function can define directly boot_cpu_hartid
>> for non-smp usecase.
>>
>> The reverse mapping function i.e. hartid to cpuid can be called
>> for any valid but not booted harts. So it should return default
>> cpu 0 only if it is a boot hartid
>>
>> Signed-off-by: Atish Patra <atish.patra@wdc.com>
>> ---
>>   arch/riscv/include/asm/smp.h | 13 +++++++++++--
>>   arch/riscv/kernel/setup.c    |  2 ++
>>   2 files changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h
>> index 41aa73b4..8f30300f 100644
>> --- a/arch/riscv/include/asm/smp.h
>> +++ b/arch/riscv/include/asm/smp.h
>> @@ -22,12 +22,13 @@
>>   /*
>>    * Mapping between linux logical cpu index and hartid.
>>    */
>> -extern unsigned long __cpuid_to_hartid_map[NR_CPUS];
>> -#define cpuid_to_hartid_map(cpu)    __cpuid_to_hartid_map[cpu]
>>
>> +extern unsigned long boot_cpu_hartid;
>>   struct seq_file;
>>
>>   #ifdef CONFIG_SMP
>> +extern unsigned long __cpuid_to_hartid_map[NR_CPUS];
>> +#define cpuid_to_hartid_map(cpu)    __cpuid_to_hartid_map[cpu]
>>
>>   /* print IPI stats */
>>   void show_ipi_stats(struct seq_file *p, int prec);
>> @@ -58,7 +59,15 @@ static inline void show_ipi_stats(struct seq_file *p, int prec)
>>
>>   static inline int riscv_hartid_to_cpuid(int hartid)
>>   {
>> +       if (hartid == boot_cpu_hartid)
>>                  return 0;
>> +       else
>> +               return -1;
>> +}
>> +static inline unsigned long cpuid_to_hartid_map(int cpu)
>> +{
>> +
>> +       return boot_cpu_hartid;
>>   }
>>
>>   static inline void riscv_cpuid_to_hartid_mask(const struct cpumask *in,
>> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
>> index 2c290e6a..bd4d7b85 100644
>> --- a/arch/riscv/kernel/setup.c
>> +++ b/arch/riscv/kernel/setup.c
>> @@ -83,6 +83,7 @@ EXPORT_SYMBOL(empty_zero_page);
>>   atomic_t hart_lottery;
>>   unsigned long boot_cpu_hartid;
>>
>> +#ifdef CONFIG_SMP
>>   unsigned long __cpuid_to_hartid_map[NR_CPUS] = {
>>          [0 ... NR_CPUS-1] = INVALID_HARTID
>>   };
>> @@ -91,6 +92,7 @@ void __init smp_setup_processor_id(void)
>>   {
>>          cpuid_to_hartid_map(0) = boot_cpu_hartid;
>>   }
>> +#endif
> 
> Please move __cpuid_to_hartid_map[] and smp_setup_processor_id()
> to arch/riscv/kernel/smp.c
> 

Will do. Thanks for the review.

> Otherwise, looks good to me.
> 
> Reviewed-by: Anup Patel <anup@brainfault.org>
> 

Regards,
Atish

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] RISC-V: Fix non-smp kernel boot on SMP systems
  2018-12-27  3:58   ` Anup Patel
@ 2019-01-06  2:14     ` Atish Patra
  0 siblings, 0 replies; 9+ messages in thread
From: Atish Patra @ 2019-01-06  2:14 UTC (permalink / raw)
  To: Anup Patel
  Cc: Rob Herring, Patrick Stählin, Albert Ou, Jason Cooper,
	Dmitriy Cherkasov, Damien Le Moal, Daniel Lezcano,
	linux-kernel@vger.kernel.org List, Michael Clark, Marc Zyngier,
	Palmer Dabbelt, linux-riscv, Thomas Gleixner

On 12/26/18 7:58 PM, Anup Patel wrote:
> On Thu, Dec 27, 2018 at 4:39 AM Atish Patra <atish.patra@wdc.com> wrote:
>>
>> In non-smp configuration, hartid can be higher that NR_CPUS.
>> riscv_of_processor_hartid should not be compared to hartid to
>> NR_CPUS in that case. Moreover, this function checks all the
>> DT properties of a hart node. NR_CPUS comparison seems out of
>> place.
> 
> This only explains change in arch/riscv/kernel/cpu.c
> 
> Create separate patch for it.
> 
>>
>> Do cpuid comparison with NR_CPUs in smp setup code. Update the
> 
> Create separate patch for change in arch/riscv/kernel/smp.c
> 
>> drivers to handle appropriate code as well.
> 
> Create separate patches for riscv_timer and irq-sifive-plic.c
> because they will probably go via different gitrepos.
> 
>>
>> Signed-off-by: Atish Patra <atish.patra@wdc.com>
>> ---
>>   arch/riscv/kernel/cpu.c           |  4 ----
>>   arch/riscv/kernel/smp.c           |  1 -
>>   arch/riscv/kernel/smpboot.c       |  5 +++++
>>   drivers/clocksource/riscv_timer.c | 21 ++++++++++++++++++---
>>   drivers/irqchip/irq-sifive-plic.c |  5 +++++
>>   5 files changed, 28 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
>> index b4a7d442..251ffab6 100644
>> --- a/arch/riscv/kernel/cpu.c
>> +++ b/arch/riscv/kernel/cpu.c
>> @@ -34,10 +34,6 @@ int riscv_of_processor_hartid(struct device_node *node)
>>                  pr_warn("Found CPU without hart ID\n");
>>                  return -(ENODEV);
>>          }
>> -       if (hart >= NR_CPUS) {
>> -               pr_info("Found hart ID %d, which is above NR_CPUs.  Disabling this hart\n", hart);
>> -               return -(ENODEV);
>> -       }
>>
>>          if (of_property_read_string(node, "status", &status)) {
>>                  pr_warn("CPU with hartid=%d has no \"status\" property\n", hart);
>> diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
>> index 57b1383e..9ea7ac7d 100644
>> --- a/arch/riscv/kernel/smp.c
>> +++ b/arch/riscv/kernel/smp.c
>> @@ -49,7 +49,6 @@ int riscv_hartid_to_cpuid(int hartid)
>>                          return i;
>>
>>          pr_err("Couldn't find cpu id for hartid [%d]\n", hartid);
>> -       BUG();
> 
> Have a separate patch with explanation about why
> we don't need BUG() here.
> 

Ok. I will split the patch into multiple ones as suggested.

Regards,
Atish
>>          return i;
>>   }
>>
>> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
>> index bb8cd242..05291840 100644
>> --- a/arch/riscv/kernel/smpboot.c
>> +++ b/arch/riscv/kernel/smpboot.c
>> @@ -66,6 +66,11 @@ void __init setup_smp(void)
>>                          found_boot_cpu = 1;
>>                          continue;
>>                  }
>> +               if (cpuid >= NR_CPUS) {
>> +                       pr_warn("Invalid cpuid [%d] for hartid [%d]\n",
>> +                               cpuid, hart);
>> +                       break;
>> +               }
>>
>>                  cpuid_to_hartid_map(cpuid) = hart;
>>                  set_cpu_possible(cpuid, true);
>> diff --git a/drivers/clocksource/riscv_timer.c b/drivers/clocksource/riscv_timer.c
>> index 084e97dc..acf2af10 100644
>> --- a/drivers/clocksource/riscv_timer.c
>> +++ b/drivers/clocksource/riscv_timer.c
>> @@ -89,20 +89,35 @@ static int __init riscv_timer_init_dt(struct device_node *n)
>>          struct clocksource *cs;
>>
>>          hartid = riscv_of_processor_hartid(n);
>> +       if (hartid < 0) {
>> +               pr_warn("Not valid hartid for node [%pOF] error = [%d]\n",
>> +                       n, hartid);
>> +               return hartid;
>> +       }
>>          cpuid = riscv_hartid_to_cpuid(hartid);
>>
>> +       if (cpuid < 0)
>> +               pr_warn("Invalid cpuid for hartid [%d]\n", hartid);
>> +
>>          if (cpuid != smp_processor_id())
>>                  return 0;
>>
>> +       pr_err("%s: Registering clocksource cpuid [%d] hartid [%d]\n",
>> +              __func__, cpuid, hartid);
>>          cs = per_cpu_ptr(&riscv_clocksource, cpuid);
>> -       clocksource_register_hz(cs, riscv_timebase);
>> +       error = clocksource_register_hz(cs, riscv_timebase);
>>
>> +       if (error) {
>> +               pr_err("RISCV timer register failed [%d] for cpu = [%d]\n",
>> +                      error, cpuid);
>> +               return error;
>> +       }
>>          error = cpuhp_setup_state(CPUHP_AP_RISCV_TIMER_STARTING,
>>                           "clockevents/riscv/timer:starting",
>>                           riscv_timer_starting_cpu, riscv_timer_dying_cpu);
>>          if (error)
>> -               pr_err("RISCV timer register failed [%d] for cpu = [%d]\n",
>> -                      error, cpuid);
>> +               pr_err("cpu hp setup state failed for RISCV timer [%d]\n",
>> +                      error);
>>          return error;
>>   }
>>
>> diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
>> index 357e9daf..254ecd76 100644
>> --- a/drivers/irqchip/irq-sifive-plic.c
>> +++ b/drivers/irqchip/irq-sifive-plic.c
>> @@ -237,6 +237,11 @@ static int __init plic_init(struct device_node *node,
>>                  }
>>
>>                  cpu = riscv_hartid_to_cpuid(hartid);
>> +               if (cpu < 0) {
>> +                       pr_warn("Invalid cpuid for context %d\n", i);
>> +                       continue;
>> +               }
>> +
>>                  handler = per_cpu_ptr(&plic_handlers, cpu);
>>                  handler->present = true;
>>                  handler->ctxid = i;
>> --
>> 2.7.4
>>
> 
> Regards,
> Anup
> 


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-26 23:08 [PATCH 0/3] Non-smp configuration fix Atish Patra
2018-12-26 23:08 ` [PATCH 1/3] RISC-V: Do not wait indefinitely in __cpu_up Atish Patra
2018-12-27  3:36   ` Anup Patel
2018-12-26 23:09 ` [PATCH 2/3] RISC-V: Move cpuid to hartid mapping to SMP Atish Patra
2018-12-27  3:37   ` Anup Patel
2019-01-06  2:13     ` Atish Patra
2018-12-26 23:09 ` [PATCH 3/3] RISC-V: Fix non-smp kernel boot on SMP systems Atish Patra
2018-12-27  3:58   ` Anup Patel
2019-01-06  2:14     ` Atish Patra

Linux-RISC-V Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-riscv/0 linux-riscv/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-riscv linux-riscv/ https://lore.kernel.org/linux-riscv \
		linux-riscv@lists.infradead.org infradead-linux-riscv@archiver.kernel.org
	public-inbox-index linux-riscv


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-riscv


AGPL code for this site: git clone https://public-inbox.org/ public-inbox