linux-mips.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] MIPS Booting fixes
@ 2023-02-25 22:10 Jiaxun Yang
  2023-02-25 22:10 ` [PATCH 1/2] MIPS: smp-cps: Don't rely on CP0_CMGCRBASE Jiaxun Yang
  2023-02-25 22:10 ` [PATCH 2/2] MIPS: cevt-r4k: Offset counter value for clearing compare interrupt Jiaxun Yang
  0 siblings, 2 replies; 10+ messages in thread
From: Jiaxun Yang @ 2023-02-25 22:10 UTC (permalink / raw)
  To: linux-mips; +Cc: tsbogend, Jiaxun Yang

Hi all,

This patchset fixes two issues that was found when doing reboot
stress test on Malta/Boston board with various MIPS cores.

Perhaps they should go through the mips-fixes tree.

Thanks

Jiaxun Yang (2):
  MIPS: smp-cps: Don't rely on CP0_CMGCRBASE
  MIPS: cevt-r4k: Offset counter value for clearing compare interrupt

 arch/mips/include/asm/smp-cps.h |  4 ++++
 arch/mips/kernel/cevt-r4k.c     |  3 +++
 arch/mips/kernel/cps-vec.S      | 35 ++++++++++++++-------------------
 arch/mips/kernel/smp-cps.c      |  2 ++
 4 files changed, 24 insertions(+), 20 deletions(-)

-- 
2.37.1 (Apple Git-137.1)


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

* [PATCH 1/2] MIPS: smp-cps: Don't rely on CP0_CMGCRBASE
  2023-02-25 22:10 [PATCH 0/2] MIPS Booting fixes Jiaxun Yang
@ 2023-02-25 22:10 ` Jiaxun Yang
  2023-02-27 11:59   ` Thomas Bogendoerfer
  2023-02-25 22:10 ` [PATCH 2/2] MIPS: cevt-r4k: Offset counter value for clearing compare interrupt Jiaxun Yang
  1 sibling, 1 reply; 10+ messages in thread
From: Jiaxun Yang @ 2023-02-25 22:10 UTC (permalink / raw)
  To: linux-mips; +Cc: tsbogend, Jiaxun Yang

CP0_CMGCRBASE is not always available on CPS enabled system
such as early proAptiv.

We patch the entry of CPS NMI vector to inject CMGCR address
directly into register during early core bringup.

For VPE bringup as the core is already coherenct at that point
we just read the variable to obtain the address.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 arch/mips/include/asm/smp-cps.h |  4 ++++
 arch/mips/kernel/cps-vec.S      | 35 ++++++++++++++-------------------
 arch/mips/kernel/smp-cps.c      |  2 ++
 3 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/arch/mips/include/asm/smp-cps.h b/arch/mips/include/asm/smp-cps.h
index 7e5b9411faee..22a572b70fe3 100644
--- a/arch/mips/include/asm/smp-cps.h
+++ b/arch/mips/include/asm/smp-cps.h
@@ -7,6 +7,8 @@
 #ifndef __MIPS_ASM_SMP_CPS_H__
 #define __MIPS_ASM_SMP_CPS_H__
 
+#define CPS_ENTRY_PATCH_INSNS	6
+
 #ifndef __ASSEMBLY__
 
 struct vpe_boot_config {
@@ -30,6 +32,8 @@ extern void mips_cps_boot_vpes(struct core_boot_config *cfg, unsigned vpe);
 extern void mips_cps_pm_save(void);
 extern void mips_cps_pm_restore(void);
 
+extern void *mips_cps_core_entry_patch_end;
+
 #ifdef CONFIG_MIPS_CPS
 
 extern bool mips_cps_smp_in_use(void);
diff --git a/arch/mips/kernel/cps-vec.S b/arch/mips/kernel/cps-vec.S
index 975343240148..fece0cca5548 100644
--- a/arch/mips/kernel/cps-vec.S
+++ b/arch/mips/kernel/cps-vec.S
@@ -13,6 +13,7 @@
 #include <asm/mipsregs.h>
 #include <asm/mipsmtregs.h>
 #include <asm/pm.h>
+#include <asm/smp-cps.h>
 
 #define GCR_CPC_BASE_OFS	0x0088
 #define GCR_CL_COHERENCE_OFS	0x2008
@@ -80,25 +81,20 @@
 	 nop
 	.endm
 
-	/* Calculate an uncached address for the CM GCRs */
-	.macro	cmgcrb	dest
-	.set	push
-	.set	noat
-	MFC0	$1, CP0_CMGCRBASE
-	PTR_SLL	$1, $1, 4
-	PTR_LI	\dest, UNCAC_BASE
-	PTR_ADDU \dest, \dest, $1
-	.set	pop
-	.endm
 
 .balign 0x1000
 
 LEAF(mips_cps_core_entry)
 	/*
-	 * These first 4 bytes will be patched by cps_smp_setup to load the
-	 * CCA to use into register s0.
+	 * These first several instructions will be patched by cps_smp_setup to load the
+	 * CCA to use into register s0 and GCR base address to register s1.
 	 */
-	.word	0
+	.rept   CPS_ENTRY_PATCH_INSNS
+    nop
+   .endr
+
+	.global mips_cps_core_entry_patch_end
+mips_cps_core_entry_patch_end:
 
 	/* Check whether we're here due to an NMI */
 	mfc0	k0, CP0_STATUS
@@ -121,8 +117,7 @@ not_nmi:
 	mtc0	t0, CP0_STATUS
 
 	/* Skip cache & coherence setup if we're already coherent */
-	cmgcrb	v1
-	lw	s7, GCR_CL_COHERENCE_OFS(v1)
+	lw	s7, GCR_CL_COHERENCE_OFS(s1)
 	bnez	s7, 1f
 	 nop
 
@@ -132,7 +127,7 @@ not_nmi:
 
 	/* Enter the coherent domain */
 	li	t0, 0xff
-	sw	t0, GCR_CL_COHERENCE_OFS(v1)
+	sw	t0, GCR_CL_COHERENCE_OFS(s1)
 	ehb
 
 	/* Set Kseg0 CCA to that in s0 */
@@ -305,8 +300,7 @@ LEAF(mips_cps_core_init)
  */
 LEAF(mips_cps_get_bootcfg)
 	/* Calculate a pointer to this cores struct core_boot_config */
-	cmgcrb	t0
-	lw	t0, GCR_CL_ID_OFS(t0)
+	lw	t0, GCR_CL_ID_OFS(s1)
 	li	t1, COREBOOTCFG_SIZE
 	mul	t0, t0, t1
 	PTR_LA	t1, mips_cps_core_bootcfg
@@ -366,8 +360,9 @@ LEAF(mips_cps_boot_vpes)
 	has_vp	t0, 5f
 
 	/* Find base address of CPC */
-	cmgcrb	t3
-	PTR_L	t1, GCR_CPC_BASE_OFS(t3)
+	PTR_LA	t1, mips_gcr_base
+	PTR_L	t1, 0(t1)
+	PTR_L	t1, GCR_CPC_BASE_OFS(t1)
 	PTR_LI	t2, ~0x7fff
 	and	t1, t1, t2
 	PTR_LI	t2, UNCAC_BASE
diff --git a/arch/mips/kernel/smp-cps.c b/arch/mips/kernel/smp-cps.c
index f2df0cae1b4d..4fc288bb85b9 100644
--- a/arch/mips/kernel/smp-cps.c
+++ b/arch/mips/kernel/smp-cps.c
@@ -162,6 +162,8 @@ static void __init cps_prepare_cpus(unsigned int max_cpus)
 	 */
 	entry_code = (u32 *)&mips_cps_core_entry;
 	uasm_i_addiu(&entry_code, 16, 0, cca);
+	UASM_i_LA(&entry_code, 17, (long)mips_gcr_base);
+	BUG_ON((void *)entry_code > (void *)&mips_cps_core_entry_patch_end);
 	blast_dcache_range((unsigned long)&mips_cps_core_entry,
 			   (unsigned long)entry_code);
 	bc_wback_inv((unsigned long)&mips_cps_core_entry,
-- 
2.37.1 (Apple Git-137.1)


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

* [PATCH 2/2] MIPS: cevt-r4k: Offset counter value for clearing compare interrupt
  2023-02-25 22:10 [PATCH 0/2] MIPS Booting fixes Jiaxun Yang
  2023-02-25 22:10 ` [PATCH 1/2] MIPS: smp-cps: Don't rely on CP0_CMGCRBASE Jiaxun Yang
@ 2023-02-25 22:10 ` Jiaxun Yang
  2023-02-26 22:04   ` Philippe Mathieu-Daudé
  2023-02-26 23:23   ` Thomas Bogendoerfer
  1 sibling, 2 replies; 10+ messages in thread
From: Jiaxun Yang @ 2023-02-25 22:10 UTC (permalink / raw)
  To: linux-mips; +Cc: tsbogend, Jiaxun Yang

In c0_compare_int_usable we clear compare interrupt by write value
just read out from counter to compare register.

However sometimes if those all instructions are graduated together
then it's possible that at the time compare register is written, the
counter haven't progressed, thus the interrupt is triggered again.

It also applies to QEMU that instructions is execuated significantly
faster then counter.

Offset the counter value a litlle bit to prevent that happen.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 arch/mips/kernel/cevt-r4k.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/mips/kernel/cevt-r4k.c b/arch/mips/kernel/cevt-r4k.c
index 32ec67c9ab67..bbc422376e97 100644
--- a/arch/mips/kernel/cevt-r4k.c
+++ b/arch/mips/kernel/cevt-r4k.c
@@ -200,6 +200,8 @@ int c0_compare_int_usable(void)
 	 */
 	if (c0_compare_int_pending()) {
 		cnt = read_c0_count();
+		// Drawdown a little bit in case counter haven't progressed
+		cnt -= COMPARE_INT_SEEN_TICKS;
 		write_c0_compare(cnt);
 		back_to_back_c0_hazard();
 		while (read_c0_count() < (cnt  + COMPARE_INT_SEEN_TICKS))
@@ -228,6 +230,7 @@ int c0_compare_int_usable(void)
 	if (!c0_compare_int_pending())
 		return 0;
 	cnt = read_c0_count();
+	cnt -= COMPARE_INT_SEEN_TICKS;
 	write_c0_compare(cnt);
 	back_to_back_c0_hazard();
 	while (read_c0_count() < (cnt + COMPARE_INT_SEEN_TICKS))
-- 
2.37.1 (Apple Git-137.1)


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

* Re: [PATCH 2/2] MIPS: cevt-r4k: Offset counter value for clearing compare interrupt
  2023-02-25 22:10 ` [PATCH 2/2] MIPS: cevt-r4k: Offset counter value for clearing compare interrupt Jiaxun Yang
@ 2023-02-26 22:04   ` Philippe Mathieu-Daudé
  2023-02-26 23:23   ` Thomas Bogendoerfer
  1 sibling, 0 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-26 22:04 UTC (permalink / raw)
  To: Jiaxun Yang, linux-mips; +Cc: tsbogend

On 25/2/23 23:10, Jiaxun Yang wrote:
> In c0_compare_int_usable we clear compare interrupt by write value
> just read out from counter to compare register.
> 
> However sometimes if those all instructions are graduated together
> then it's possible that at the time compare register is written, the
> counter haven't progressed, thus the interrupt is triggered again.
> 
> It also applies to QEMU that instructions is execuated significantly

Typo "executed",

> faster then counter.
> 
> Offset the counter value a litlle bit to prevent that happen.

"little".

> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>   arch/mips/kernel/cevt-r4k.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/arch/mips/kernel/cevt-r4k.c b/arch/mips/kernel/cevt-r4k.c
> index 32ec67c9ab67..bbc422376e97 100644
> --- a/arch/mips/kernel/cevt-r4k.c
> +++ b/arch/mips/kernel/cevt-r4k.c
> @@ -200,6 +200,8 @@ int c0_compare_int_usable(void)
>   	 */
>   	if (c0_compare_int_pending()) {
>   		cnt = read_c0_count();
> +		// Drawdown a little bit in case counter haven't progressed
> +		cnt -= COMPARE_INT_SEEN_TICKS;
>   		write_c0_compare(cnt);
>   		back_to_back_c0_hazard();
>   		while (read_c0_count() < (cnt  + COMPARE_INT_SEEN_TICKS))
> @@ -228,6 +230,7 @@ int c0_compare_int_usable(void)
>   	if (!c0_compare_int_pending())
>   		return 0;
>   	cnt = read_c0_count();
> +	cnt -= COMPARE_INT_SEEN_TICKS;
>   	write_c0_compare(cnt);
>   	back_to_back_c0_hazard();
>   	while (read_c0_count() < (cnt + COMPARE_INT_SEEN_TICKS))


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

* Re: [PATCH 2/2] MIPS: cevt-r4k: Offset counter value for clearing compare interrupt
  2023-02-25 22:10 ` [PATCH 2/2] MIPS: cevt-r4k: Offset counter value for clearing compare interrupt Jiaxun Yang
  2023-02-26 22:04   ` Philippe Mathieu-Daudé
@ 2023-02-26 23:23   ` Thomas Bogendoerfer
  2023-02-27  1:22     ` Jiaxun Yang
  1 sibling, 1 reply; 10+ messages in thread
From: Thomas Bogendoerfer @ 2023-02-26 23:23 UTC (permalink / raw)
  To: Jiaxun Yang; +Cc: linux-mips

On Sat, Feb 25, 2023 at 10:10:08PM +0000, Jiaxun Yang wrote:
> In c0_compare_int_usable we clear compare interrupt by write value
> just read out from counter to compare register.
> 
> However sometimes if those all instructions are graduated together
> then it's possible that at the time compare register is written, the
> counter haven't progressed, thus the interrupt is triggered again.
> 
> It also applies to QEMU that instructions is execuated significantly
> faster then counter.
> 
> Offset the counter value a litlle bit to prevent that happen.
> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>  arch/mips/kernel/cevt-r4k.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/mips/kernel/cevt-r4k.c b/arch/mips/kernel/cevt-r4k.c
> index 32ec67c9ab67..bbc422376e97 100644
> --- a/arch/mips/kernel/cevt-r4k.c
> +++ b/arch/mips/kernel/cevt-r4k.c
> @@ -200,6 +200,8 @@ int c0_compare_int_usable(void)
>  	 */
>  	if (c0_compare_int_pending()) {
>  		cnt = read_c0_count();
> +		// Drawdown a little bit in case counter haven't progressed

no C++ comments

> +		cnt -= COMPARE_INT_SEEN_TICKS;
>  		write_c0_compare(cnt);
>  		back_to_back_c0_hazard();
>  		while (read_c0_count() < (cnt  + COMPARE_INT_SEEN_TICKS))

this doesn't make sense. clearing of the interrupts happes because of
this loop. So either COMPARE_INT_SEEN_TICKS is too small or you are
hunting a different bug.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

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

* Re: [PATCH 2/2] MIPS: cevt-r4k: Offset counter value for clearing compare interrupt
  2023-02-26 23:23   ` Thomas Bogendoerfer
@ 2023-02-27  1:22     ` Jiaxun Yang
  2023-02-27  9:31       ` Thomas Bogendoerfer
  0 siblings, 1 reply; 10+ messages in thread
From: Jiaxun Yang @ 2023-02-27  1:22 UTC (permalink / raw)
  To: Thomas Bogendoerfer; +Cc: linux-mips



> 2023年2月26日 23:23,Thomas Bogendoerfer <tsbogend@alpha.franken.de> 写道:
> 
> On Sat, Feb 25, 2023 at 10:10:08PM +0000, Jiaxun Yang wrote:
>> In c0_compare_int_usable we clear compare interrupt by write value
>> just read out from counter to compare register.
>> 
>> However sometimes if those all instructions are graduated together
>> then it's possible that at the time compare register is written, the
>> counter haven't progressed, thus the interrupt is triggered again.
>> 
>> It also applies to QEMU that instructions is execuated significantly
>> faster then counter.
>> 
>> Offset the counter value a litlle bit to prevent that happen.
>> 
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> ---
>> arch/mips/kernel/cevt-r4k.c | 3 +++
>> 1 file changed, 3 insertions(+)
>> 
>> diff --git a/arch/mips/kernel/cevt-r4k.c b/arch/mips/kernel/cevt-r4k.c
>> index 32ec67c9ab67..bbc422376e97 100644
>> --- a/arch/mips/kernel/cevt-r4k.c
>> +++ b/arch/mips/kernel/cevt-r4k.c
>> @@ -200,6 +200,8 @@ int c0_compare_int_usable(void)
>> */
>> if (c0_compare_int_pending()) {
>> cnt = read_c0_count();
>> + // Drawdown a little bit in case counter haven't progressed
> 
> no C++ comments
> 
>> + cnt -= COMPARE_INT_SEEN_TICKS;
>> write_c0_compare(cnt);
>> back_to_back_c0_hazard();
>> while (read_c0_count() < (cnt  + COMPARE_INT_SEEN_TICKS))
> 
> this doesn't make sense. clearing of the interrupts happes because of
> this loop. So either COMPARE_INT_SEEN_TICKS is too small or you are
> hunting a different bug.

Clearing interrupt happens in write_c0_compare but the problem is the interrupt
does really get cleared because it triggered again straight away.

Had confirmed issue on MIPS I6500 uarch simulation trace.

Ah I see, it makes more sense if I move minus into write_c0_compare so the
loop will still run. Though this loop is not required on any MTI cores, TI is
always clear immediately since very early 4Kc.

Will send v2 later.

Thanks.

 

> 
> Thomas.
> 
> -- 
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea.                                                [ RFC1925, 2.3 ]


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

* Re: [PATCH 2/2] MIPS: cevt-r4k: Offset counter value for clearing compare interrupt
  2023-02-27  1:22     ` Jiaxun Yang
@ 2023-02-27  9:31       ` Thomas Bogendoerfer
  2023-02-27 11:44         ` Jiaxun Yang
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Bogendoerfer @ 2023-02-27  9:31 UTC (permalink / raw)
  To: Jiaxun Yang; +Cc: linux-mips

On Mon, Feb 27, 2023 at 01:22:47AM +0000, Jiaxun Yang wrote:
> 
> 
> > 2023年2月26日 23:23,Thomas Bogendoerfer <tsbogend@alpha.franken.de> 写道:
> > 
> > On Sat, Feb 25, 2023 at 10:10:08PM +0000, Jiaxun Yang wrote:
> >> In c0_compare_int_usable we clear compare interrupt by write value
> >> just read out from counter to compare register.
> >> 
> >> However sometimes if those all instructions are graduated together
> >> then it's possible that at the time compare register is written, the
> >> counter haven't progressed, thus the interrupt is triggered again.
> >> 
> >> It also applies to QEMU that instructions is execuated significantly
> >> faster then counter.
> >> 
> >> Offset the counter value a litlle bit to prevent that happen.
> >> 
> >> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> >> ---
> >> arch/mips/kernel/cevt-r4k.c | 3 +++
> >> 1 file changed, 3 insertions(+)
> >> 
> >> diff --git a/arch/mips/kernel/cevt-r4k.c b/arch/mips/kernel/cevt-r4k.c
> >> index 32ec67c9ab67..bbc422376e97 100644
> >> --- a/arch/mips/kernel/cevt-r4k.c
> >> +++ b/arch/mips/kernel/cevt-r4k.c
> >> @@ -200,6 +200,8 @@ int c0_compare_int_usable(void)
> >> */
> >> if (c0_compare_int_pending()) {
> >> cnt = read_c0_count();
> >> + // Drawdown a little bit in case counter haven't progressed
> > 
> > no C++ comments
> > 
> >> + cnt -= COMPARE_INT_SEEN_TICKS;
> >> write_c0_compare(cnt);
> >> back_to_back_c0_hazard();
> >> while (read_c0_count() < (cnt  + COMPARE_INT_SEEN_TICKS))
> > 
> > this doesn't make sense. clearing of the interrupts happes because of
> > this loop. So either COMPARE_INT_SEEN_TICKS is too small or you are
> > hunting a different bug.
> 
> Clearing interrupt happens in write_c0_compare but the problem is the interrupt
> does really get cleared because it triggered again straight away.

the function you are patching is a test function whether counter/compare
is usable, so it's not in the normal timer handling. See a problem there
would more to broken hardware than to a bug in that function.

> Had confirmed issue on MIPS I6500 uarch simulation trace.

so not seen on real hardware ?

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

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

* Re: [PATCH 2/2] MIPS: cevt-r4k: Offset counter value for clearing compare interrupt
  2023-02-27  9:31       ` Thomas Bogendoerfer
@ 2023-02-27 11:44         ` Jiaxun Yang
  2023-02-27 16:48           ` Thomas Bogendoerfer
  0 siblings, 1 reply; 10+ messages in thread
From: Jiaxun Yang @ 2023-02-27 11:44 UTC (permalink / raw)
  To: Thomas Bogendoerfer; +Cc: linux-mips



> 2023年2月27日 09:31,Thomas Bogendoerfer <tsbogend@alpha.franken.de> 写道:
> 
> On Mon, Feb 27, 2023 at 01:22:47AM +0000, Jiaxun Yang wrote:
>> 
>> 
>> 
[...]
>> Clearing interrupt happens in write_c0_compare but the problem is the interrupt
>> does really get cleared because it triggered again straight away.
> 
> the function you are patching is a test function whether counter/compare
> is usable, so it's not in the normal timer handling. See a problem there
> would more to broken hardware than to a bug in that function.

The problem is sometimes on a multi-core system this function managed
to run on one of the core but then fail on another.

In our real use of compare interrupt, we are clearing interrupt by writing back
old value of compare, which is guarenteed to be smaller then current count
because there are so many instructions being executed from exception vector
to here.

Also as we have no way to disable compare interrupt, if this function determined
compare to be broken then it won’t register event_handler function, which will
cause panic for null pointer when unexpected-ish interrupt comes.

> 
>> Had confirmed issue on MIPS I6500 uarch simulation trace.
> 
> so not seen on real hardware ?

It was discovered on FPGA and then I tried to debug with simulation to reveal what’s
going on hardware side.

Thanks
- Jiaxun

> 
> Thomas.
> 
> -- 
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea.                                                [ RFC1925, 2.3 ]



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

* Re: [PATCH 1/2] MIPS: smp-cps: Don't rely on CP0_CMGCRBASE
  2023-02-25 22:10 ` [PATCH 1/2] MIPS: smp-cps: Don't rely on CP0_CMGCRBASE Jiaxun Yang
@ 2023-02-27 11:59   ` Thomas Bogendoerfer
  0 siblings, 0 replies; 10+ messages in thread
From: Thomas Bogendoerfer @ 2023-02-27 11:59 UTC (permalink / raw)
  To: Jiaxun Yang; +Cc: linux-mips

On Sat, Feb 25, 2023 at 10:10:07PM +0000, Jiaxun Yang wrote:
> CP0_CMGCRBASE is not always available on CPS enabled system
> such as early proAptiv.
> 
> We patch the entry of CPS NMI vector to inject CMGCR address
> directly into register during early core bringup.
> 
> For VPE bringup as the core is already coherenct at that point
> we just read the variable to obtain the address.
> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>  arch/mips/include/asm/smp-cps.h |  4 ++++
>  arch/mips/kernel/cps-vec.S      | 35 ++++++++++++++-------------------
>  arch/mips/kernel/smp-cps.c      |  2 ++
>  3 files changed, 21 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/mips/include/asm/smp-cps.h b/arch/mips/include/asm/smp-cps.h
> index 7e5b9411faee..22a572b70fe3 100644
> --- a/arch/mips/include/asm/smp-cps.h
> +++ b/arch/mips/include/asm/smp-cps.h
> @@ -7,6 +7,8 @@
>  #ifndef __MIPS_ASM_SMP_CPS_H__
>  #define __MIPS_ASM_SMP_CPS_H__
>  
> +#define CPS_ENTRY_PATCH_INSNS	6
> +
>  #ifndef __ASSEMBLY__
>  
>  struct vpe_boot_config {
> @@ -30,6 +32,8 @@ extern void mips_cps_boot_vpes(struct core_boot_config *cfg, unsigned vpe);
>  extern void mips_cps_pm_save(void);
>  extern void mips_cps_pm_restore(void);
>  
> +extern void *mips_cps_core_entry_patch_end;
> +
>  #ifdef CONFIG_MIPS_CPS
>  
>  extern bool mips_cps_smp_in_use(void);
> diff --git a/arch/mips/kernel/cps-vec.S b/arch/mips/kernel/cps-vec.S
> index 975343240148..fece0cca5548 100644
> --- a/arch/mips/kernel/cps-vec.S
> +++ b/arch/mips/kernel/cps-vec.S
> @@ -13,6 +13,7 @@
>  #include <asm/mipsregs.h>
>  #include <asm/mipsmtregs.h>
>  #include <asm/pm.h>
> +#include <asm/smp-cps.h>
>  
>  #define GCR_CPC_BASE_OFS	0x0088
>  #define GCR_CL_COHERENCE_OFS	0x2008
> @@ -80,25 +81,20 @@
>  	 nop
>  	.endm
>  
> -	/* Calculate an uncached address for the CM GCRs */
> -	.macro	cmgcrb	dest
> -	.set	push
> -	.set	noat
> -	MFC0	$1, CP0_CMGCRBASE
> -	PTR_SLL	$1, $1, 4
> -	PTR_LI	\dest, UNCAC_BASE
> -	PTR_ADDU \dest, \dest, $1
> -	.set	pop
> -	.endm
>  
>  .balign 0x1000
>  
>  LEAF(mips_cps_core_entry)
>  	/*
> -	 * These first 4 bytes will be patched by cps_smp_setup to load the
> -	 * CCA to use into register s0.
> +	 * These first several instructions will be patched by cps_smp_setup to load the
> +	 * CCA to use into register s0 and GCR base address to register s1.
>  	 */
> -	.word	0
> +	.rept   CPS_ENTRY_PATCH_INSNS
> +    nop
> +   .endr
> +
> +	.global mips_cps_core_entry_patch_end
> +mips_cps_core_entry_patch_end:
>  
>  	/* Check whether we're here due to an NMI */
>  	mfc0	k0, CP0_STATUS
> @@ -121,8 +117,7 @@ not_nmi:
>  	mtc0	t0, CP0_STATUS
>  
>  	/* Skip cache & coherence setup if we're already coherent */
> -	cmgcrb	v1
> -	lw	s7, GCR_CL_COHERENCE_OFS(v1)
> +	lw	s7, GCR_CL_COHERENCE_OFS(s1)
>  	bnez	s7, 1f
>  	 nop
>  
> @@ -132,7 +127,7 @@ not_nmi:
>  
>  	/* Enter the coherent domain */
>  	li	t0, 0xff
> -	sw	t0, GCR_CL_COHERENCE_OFS(v1)
> +	sw	t0, GCR_CL_COHERENCE_OFS(s1)
>  	ehb
>  
>  	/* Set Kseg0 CCA to that in s0 */
> @@ -305,8 +300,7 @@ LEAF(mips_cps_core_init)
>   */
>  LEAF(mips_cps_get_bootcfg)
>  	/* Calculate a pointer to this cores struct core_boot_config */
> -	cmgcrb	t0
> -	lw	t0, GCR_CL_ID_OFS(t0)
> +	lw	t0, GCR_CL_ID_OFS(s1)
>  	li	t1, COREBOOTCFG_SIZE
>  	mul	t0, t0, t1
>  	PTR_LA	t1, mips_cps_core_bootcfg
> @@ -366,8 +360,9 @@ LEAF(mips_cps_boot_vpes)
>  	has_vp	t0, 5f
>  
>  	/* Find base address of CPC */
> -	cmgcrb	t3
> -	PTR_L	t1, GCR_CPC_BASE_OFS(t3)
> +	PTR_LA	t1, mips_gcr_base
> +	PTR_L	t1, 0(t1)
> +	PTR_L	t1, GCR_CPC_BASE_OFS(t1)
>  	PTR_LI	t2, ~0x7fff
>  	and	t1, t1, t2
>  	PTR_LI	t2, UNCAC_BASE
> diff --git a/arch/mips/kernel/smp-cps.c b/arch/mips/kernel/smp-cps.c
> index f2df0cae1b4d..4fc288bb85b9 100644
> --- a/arch/mips/kernel/smp-cps.c
> +++ b/arch/mips/kernel/smp-cps.c
> @@ -162,6 +162,8 @@ static void __init cps_prepare_cpus(unsigned int max_cpus)
>  	 */
>  	entry_code = (u32 *)&mips_cps_core_entry;
>  	uasm_i_addiu(&entry_code, 16, 0, cca);
> +	UASM_i_LA(&entry_code, 17, (long)mips_gcr_base);
> +	BUG_ON((void *)entry_code > (void *)&mips_cps_core_entry_patch_end);
>  	blast_dcache_range((unsigned long)&mips_cps_core_entry,
>  			   (unsigned long)entry_code);
>  	bc_wback_inv((unsigned long)&mips_cps_core_entry,

why do you need to patch that in ? Would replacing cmgcrb by a
PTR_LA  t1, mips_gcr_base in arch/mips/kernel/cps-vec.S do the same thing ?

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

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

* Re: [PATCH 2/2] MIPS: cevt-r4k: Offset counter value for clearing compare interrupt
  2023-02-27 11:44         ` Jiaxun Yang
@ 2023-02-27 16:48           ` Thomas Bogendoerfer
  0 siblings, 0 replies; 10+ messages in thread
From: Thomas Bogendoerfer @ 2023-02-27 16:48 UTC (permalink / raw)
  To: Jiaxun Yang; +Cc: linux-mips

On Mon, Feb 27, 2023 at 11:44:48AM +0000, Jiaxun Yang wrote:
> > so not seen on real hardware ?
> 
> It was discovered on FPGA and then I tried to debug with simulation to
> reveal what’s going on hardware side.

ic, I think the best fix would be to do a write_c0_compare(cnt - 1).
Subtracting COMPARE_INT_SEEN_TICKS feels like overkill to me.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

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

end of thread, other threads:[~2023-02-27 16:49 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-25 22:10 [PATCH 0/2] MIPS Booting fixes Jiaxun Yang
2023-02-25 22:10 ` [PATCH 1/2] MIPS: smp-cps: Don't rely on CP0_CMGCRBASE Jiaxun Yang
2023-02-27 11:59   ` Thomas Bogendoerfer
2023-02-25 22:10 ` [PATCH 2/2] MIPS: cevt-r4k: Offset counter value for clearing compare interrupt Jiaxun Yang
2023-02-26 22:04   ` Philippe Mathieu-Daudé
2023-02-26 23:23   ` Thomas Bogendoerfer
2023-02-27  1:22     ` Jiaxun Yang
2023-02-27  9:31       ` Thomas Bogendoerfer
2023-02-27 11:44         ` Jiaxun Yang
2023-02-27 16:48           ` Thomas Bogendoerfer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).