[PATCH 1/3] ARC: mcip: halt GFRC together with ARC cores

[PATCH 1/3] ARC: mcip: halt GFRC together with ARC cores

[Eugeniy Paltsev]

Currently GFRC is running regardless state of ARC cores in the SMP cluster.
That means even if ARC cores are halted when doing JTAG debugging GFRC
[our source of wall-time] continues to run giving us unexpected warnings
once we allow ARC cores to run due to some tasks being stuck for too
long.

Starting from ARC HS v3.0 it's possible to tie GFRC to state of up-to 4
ARC cores with help of GFRC's CORE register where we set a mask for
cores which state we need to rely on.

Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
---
NOTE: with this patch previous patch is not required:
http://patchwork.ozlabs.org/patch/875091/

 arch/arc/kernel/mcip.c | 23 +++++++++++++++++++++++
 include/soc/arc/mcip.h |  2 ++
 2 files changed, 25 insertions(+)

diff --git a/arch/arc/kernel/mcip.c b/arch/arc/kernel/mcip.c
index f61a52b..e87a4ea 100644
--- a/arch/arc/kernel/mcip.c
+++ b/arch/arc/kernel/mcip.c
@@ -22,10 +22,33 @@ static DEFINE_RAW_SPINLOCK(mcip_lock);
 
 static char smp_cpuinfo_buf[128];
 
+/*
+ * Set mask to halt GFRC if any online core in SMP cluster is halted.
+ * Only works for ARC HS v3.0+, on earlier versions has no effect.
+ */
+static void mcip_update_gfrc_halt_mask(int cpu)
+{
+	struct mcip_bcr mp;
+	u32 gfrc_halt_mask;
+
+	READ_BCR(ARC_REG_MCIP_BCR, mp);
+
+	if (!mp.gfrc)
+		return;
+
+	__mcip_cmd(CMD_GFRC_READ_CORE, 0);
+	gfrc_halt_mask = read_aux_reg(ARC_REG_MCIP_READBACK);
+	gfrc_halt_mask |= BIT(cpu);
+	__mcip_cmd_data(CMD_GFRC_SET_CORE, 0, gfrc_halt_mask);
+}
+
 static void mcip_setup_per_cpu(int cpu)
 {
 	smp_ipi_irq_setup(cpu, IPI_IRQ);
 	smp_ipi_irq_setup(cpu, SOFTIRQ_IRQ);
+
+	/* Update GFRC halt mask as new CPU came online */
+	mcip_update_gfrc_halt_mask(cpu);
 }
 
 static void mcip_ipi_send(int cpu)
diff --git a/include/soc/arc/mcip.h b/include/soc/arc/mcip.h
index c2d1b15..a156fa5 100644
--- a/include/soc/arc/mcip.h
+++ b/include/soc/arc/mcip.h
@@ -40,6 +40,8 @@ struct mcip_cmd {
 
 #define CMD_GFRC_READ_LO		0x42
 #define CMD_GFRC_READ_HI		0x43
+#define CMD_GFRC_SET_CORE		0x47
+#define CMD_GFRC_READ_CORE		0x48
 
 #define CMD_IDU_ENABLE			0x71
 #define CMD_IDU_DISABLE			0x72
-- 
2.9.3

[PATCH 2/3] ARC: mcip: setup MCIP debug mask according to cpu possible mask

[Eugeniy Paltsev]

Setup MCIP debug mask according cpu possible mask instead of
use hardcoded one.

Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
---
 arch/arc/kernel/mcip.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/arch/arc/kernel/mcip.c b/arch/arc/kernel/mcip.c
index e87a4ea..da73258 100644
--- a/arch/arc/kernel/mcip.c
+++ b/arch/arc/kernel/mcip.c
@@ -112,6 +112,7 @@ static void mcip_ipi_clear(int irq)
 static void mcip_probe_n_setup(void)
 {
 	struct mcip_bcr mp;
+	u32 i, mcip_mask = 0;
 
 	READ_BCR(ARC_REG_MCIP_BCR, mp);
 
@@ -126,8 +127,16 @@ static void mcip_probe_n_setup(void)
 	cpuinfo_arc700[0].extn.gfrc = mp.gfrc;
 
 	if (mp.dbg) {
-		__mcip_cmd_data(CMD_DEBUG_SET_SELECT, 0, 0xf);
-		__mcip_cmd_data(CMD_DEBUG_SET_MASK, 0xf, 0xf);
+		for_each_possible_cpu(i)
+			mcip_mask |= BIT(i);
+
+		__mcip_cmd_data(CMD_DEBUG_SET_SELECT, 0, mcip_mask);
+		/*
+		 * Parameter specified halt cause:
+		 * STATUS32[H]/actionpoint/breakpoint/self-halt
+		 * We choose all of them (0xF).
+		 */
+		__mcip_cmd_data(CMD_DEBUG_SET_MASK, 0xF, mcip_mask);
 	}
 }
 
-- 
2.9.3

[PATCH 3/3] ARC: setup cpu possible mask according to possible-cpus dts property

[Eugeniy Paltsev]

As we have option in u-boot to set CPU mask for running linux,
we want to pass information to kernel about CPU cores should
be brought up. So we patch kernel dtb in u-boot to set
possible-cpus property.

This also allows us to have correctly setuped MCIP debug mask.

Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
---
 arch/arc/kernel/smp.c | 50 ++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 40 insertions(+), 10 deletions(-)

diff --git a/arch/arc/kernel/smp.c b/arch/arc/kernel/smp.c
index efe8b42..21d86c3 100644
--- a/arch/arc/kernel/smp.c
+++ b/arch/arc/kernel/smp.c
@@ -24,6 +24,7 @@
 #include <linux/reboot.h>
 #include <linux/irqdomain.h>
 #include <linux/export.h>
+#include <linux/of_fdt.h>
 
 #include <asm/processor.h>
 #include <asm/setup.h>
@@ -47,6 +48,42 @@ void __init smp_prepare_boot_cpu(void)
 {
 }
 
+static int __init arc_get_cpu_map(const char *name, struct cpumask *cpumask)
+{
+	unsigned long dt_root = of_get_flat_dt_root();
+	const char *buf;
+
+	buf = of_get_flat_dt_prop(dt_root, name, NULL);
+	if (!buf)
+		return -EINVAL;
+
+	if (cpulist_parse(buf, cpumask))
+		return -EINVAL;
+
+	return 0;
+}
+
+/*
+ * Read from DeviceTree and setup cpu possible mask. If there is no
+ * "possible-cpus" property in DeviceTree pretend all [0..NR_CPUS-1] exist.
+ */
+static void __init arc_init_cpu_possible(void)
+{
+	struct cpumask cpumask;
+
+	if (arc_get_cpu_map("possible-cpus", &cpumask)) {
+		pr_warn("Failed to get possible-cpus from dtb, pretending all %u cpus exist\n",
+			NR_CPUS);
+
+		cpumask_setall(&cpumask);
+	}
+
+	if (!cpumask_test_cpu(0, &cpumask))
+		panic("Master cpu (cpu[0]) is missed in cpu possible mask!");
+
+	init_cpu_possible(&cpumask);
+}
+
 /*
  * Called from setup_arch() before calling setup_processor()
  *
@@ -58,10 +95,7 @@ void __init smp_prepare_boot_cpu(void)
  */
 void __init smp_init_cpus(void)
 {
-	unsigned int i;
-
-	for (i = 0; i < NR_CPUS; i++)
-		set_cpu_possible(i, true);
+	arc_init_cpu_possible();
 
 	if (plat_smp_ops.init_early_smp)
 		plat_smp_ops.init_early_smp();
@@ -70,16 +104,12 @@ void __init smp_init_cpus(void)
 /* called from init ( ) =>  process 1 */
 void __init smp_prepare_cpus(unsigned int max_cpus)
 {
-	int i;
-
 	/*
 	 * if platform didn't set the present map already, do it now
 	 * boot cpu is set to present already by init/main.c
 	 */
-	if (num_present_cpus() <= 1) {
-		for (i = 0; i < max_cpus; i++)
-			set_cpu_present(i, true);
-	}
+	if (num_present_cpus() <= 1)
+		init_cpu_present(cpu_possible_mask);
 }
 
 void __init smp_cpus_done(unsigned int max_cpus)
-- 
2.9.3

Re: [PATCH 1/3] ARC: mcip: halt GFRC together with ARC cores

[Vineet Gupta]

Hi Eugeniy,

On 02/21/2018 01:40 AM, Eugeniy Paltsev wrote:
> Currently GFRC is running regardless state of ARC cores in the SMP cluster.
> That means even if ARC cores are halted when doing JTAG debugging GFRC
> [our source of wall-time] continues to run giving us unexpected warnings
> once we allow ARC cores to run due to some tasks being stuck for too
> long.

This patch is definitely nicer than previous version. Thx.


> Starting from ARC HS v3.0

 From the STAR fix, it seem this was fixed in HS 2.1c, so you should be able to 
test it on HSDK, which was my next question: where and how did you test this and 
verify that it works as we think it does. I tried the patch on HSDK and I still 
see the rcu_preempt self-detected stall error splat when running hackbench and 
pausing the target with Metaware debugger. Perhaps we need to write a small test 
case to check what's going on. Also try that on AXS103 release which is definitely 
HS 3.0 !


> it's possible to tie GFRC to state of up-to 4
> ARC cores with help of GFRC's CORE register where we set a mask for
> cores which state we need to rely on.
> 
> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
> ---
> NOTE: with this patch previous patch is not required:
> http://patchwork.ozlabs.org/patch/875091/
> 
>   arch/arc/kernel/mcip.c | 23 +++++++++++++++++++++++
>   include/soc/arc/mcip.h |  2 ++
>   2 files changed, 25 insertions(+)
> 
> diff --git a/arch/arc/kernel/mcip.c b/arch/arc/kernel/mcip.c
> index f61a52b..e87a4ea 100644
> --- a/arch/arc/kernel/mcip.c
> +++ b/arch/arc/kernel/mcip.c
> @@ -22,10 +22,33 @@ static DEFINE_RAW_SPINLOCK(mcip_lock);
>   
>   static char smp_cpuinfo_buf[128];
>   
> +/*
> + * Set mask to halt GFRC if any online core in SMP cluster is halted.
> + * Only works for ARC HS v3.0+, on earlier versions has no effect.
> + */
> +static void mcip_update_gfrc_halt_mask(int cpu)
> +{
> +	struct mcip_bcr mp;
> +	u32 gfrc_halt_mask;
> +
> +	READ_BCR(ARC_REG_MCIP_BCR, mp);
> +
> +	if (!mp.gfrc)
> +		return;
> +

In theory this could be called concurrently by multiple cpus and mcip doesn't 
guarantee any internal serialization/buffering. Granted, current use case is fine 
as mcip_setup_per_cpu --> plat_smp_ops.init_per_cpu is serialized by master core, 
we could run into issue when say cpu hot plug etc works. So better to wrap this 
inside the spinlock which we already have.

> +	__mcip_cmd(CMD_GFRC_READ_CORE, 0);
> +	gfrc_halt_mask = read_aux_reg(ARC_REG_MCIP_READBACK);
> +	gfrc_halt_mask |= BIT(cpu);
> +	__mcip_cmd_data(CMD_GFRC_SET_CORE, 0, gfrc_halt_mask);
> +}
> +
>   static void mcip_setup_per_cpu(int cpu)
>   {
>   	smp_ipi_irq_setup(cpu, IPI_IRQ);
>   	smp_ipi_irq_setup(cpu, SOFTIRQ_IRQ);

Please move the bcr readout / gfrc check here so in future we can do any more 
checks as well.

> +
> +	/* Update GFRC halt mask as new CPU came online */
> +	mcip_update_gfrc_halt_mask(cpu);
>   }
>   
>   static void mcip_ipi_send(int cpu)
> diff --git a/include/soc/arc/mcip.h b/include/soc/arc/mcip.h
> index c2d1b15..a156fa5 100644
> --- a/include/soc/arc/mcip.h
> +++ b/include/soc/arc/mcip.h
> @@ -40,6 +40,8 @@ struct mcip_cmd {
>   
>   #define CMD_GFRC_READ_LO		0x42
>   #define CMD_GFRC_READ_HI		0x43
> +#define CMD_GFRC_SET_CORE		0x47
> +#define CMD_GFRC_READ_CORE		0x48
>   
>   #define CMD_IDU_ENABLE			0x71
>   #define CMD_IDU_DISABLE			0x72
> 

Re: [PATCH 1/3] ARC: mcip: halt GFRC together with ARC cores

[Vineet Gupta]

On 02/21/2018 12:31 PM, Vineet Gupta wrote:
> Hi Eugeniy,
> 
>> Starting from ARC HS v3.0
> 
>  From the STAR fix, it seem this was fixed in HS 2.1c, so you should be able to 
> test it on HSDK, which was my next question: where and how did you test this and 
> verify that it works as we think it does. I tried the patch on HSDK and I still 
> see the rcu_preempt self-detected stall error splat when running hackbench and 
> pausing the target with Metaware debugger. Perhaps we need to write a small test 
> case to check what's going on. Also try that on AXS103 release which is definitely 
> HS 3.0 !

So I tried this on both.
  - HSDK   (HS 2.1c): Doesn't work
  - AXS103 (HS 3.0) : Works

Fortunately we can read (yet another BCR: GFRC_BUILD) and infer whether this is 
supported or not. So add that check in mcip_update_gfrc_halt_mask()

>> it's possible to tie GFRC to state of up-to 4
>> ARC cores with help of GFRC's CORE register where we set a mask for
>> cores which state we need to rely on.

On second thoughts, do we really have to do this per cpu. Just write 0xf once just 
as Alexey did in first iteration.

-Vineet

Re: [PATCH 1/3] ARC: mcip: halt GFRC together with ARC cores

[Eugeniy Paltsev]

Hi Vineet,

On Wed, 2018-02-21 at 15:42 -0800, Vineet Gupta wrote:
> On 02/21/2018 12:31 PM, Vineet Gupta wrote:
> > Hi Eugeniy,
> > 
> > > Starting from ARC HS v3.0
> > 
> >  From the STAR fix, it seem this was fixed in HS 2.1c, so you should be able to 
> > test it on HSDK, which was my next question: where and how did you test this and 
> > verify that it works as we think it does. I tried the patch on HSDK and I still 
> > see the rcu_preempt self-detected stall error splat when running hackbench and 
> > pausing the target with Metaware debugger. Perhaps we need to write a small test 
> > case to check what's going on. Also try that on AXS103 release which is definitely 
> > HS 3.0 !
> 
> So I tried this on both.
>   - HSDK   (HS 2.1c): Doesn't work
>   - AXS103 (HS 3.0) : Works

I checked the HS_3.00a and HS_2.1c documentation - GFRC HALT commands/settings exist
only in HS_3.00a.

> 
> Fortunately we can read (yet another BCR: GFRC_BUILD) and infer whether this is 
> supported or not. So add that check in mcip_update_gfrc_halt_mask()

Ok, I'll add GFRC_BUILD read.

> > > it's possible to tie GFRC to state of up-to 4
> > > ARC cores with help of GFRC's CORE register where we set a mask for
> > > cores which state we need to rely on.
> 
> On second thoughts, do we really have to do this per cpu. Just write 0xf once just 
> as Alexey did in first iteration.

And we will face with same problems like with MCIP debug.
Remember what happens when we launch kernel on one CPU on board which has several CPUs.

> In theory this could be called concurrently by multiple cpus and mcip doesn't 
> guarantee any internal serialization/buffering. Granted, current use
case is fine 
> as mcip_setup_per_cpu --> plat_smp_ops.init_per_cpu is serialized by master core, 
> we could run into issue when say cpu hot plug etc
works. So better to wrap this 
> inside the spinlock which we already have.

Yep, I was also thinking about adding the spinlock here...
I'll add it in next patch version.

> -Vineet
-- 
 Eugeniy Paltsev