2020-08-11 06:42:06

by Qiu Wenbo

[permalink] [raw]
Subject: [PATCH] riscv: Setup exception vector for K210 properly

Exception vector is missing on nommu platform and it is a big issue.
This patch is tested in Sipeed MAIX Bit Dev Board.

Fixes: 79b1feba5455 ("RISC-V: Setup exception vector early")
Signed-off-by: Qiu Wenbo <[email protected]>
---
arch/riscv/kernel/smpboot.c | 1 +
arch/riscv/kernel/traps.c | 11 ++++++++++-
2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 356825a57551..23cde0ceb39d 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -154,6 +154,7 @@ asmlinkage __visible void smp_callin(void)
mmgrab(mm);
current->active_mm = mm;

+ trap_init();
notify_cpu_starting(curr_cpuid);
update_siblings_masks(curr_cpuid);
set_cpu_online(curr_cpuid, 1);
diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
index ad14f4466d92..a390239818ae 100644
--- a/arch/riscv/kernel/traps.c
+++ b/arch/riscv/kernel/traps.c
@@ -174,7 +174,16 @@ int is_valid_bugaddr(unsigned long pc)
}
#endif /* CONFIG_GENERIC_BUG */

-/* stvec & scratch is already set from head.S */
+/* stvec & scratch is already set from head.S when mmu is enabled */
void trap_init(void)
{
+#ifndef CONFIG_MMU
+ /*
+ * Set sup0 scratch register to 0, indicating to exception vector
+ * that we are presently executing in the kernel
+ */
+ csr_write(CSR_SCRATCH, 0);
+ /* Set the exception vector address */
+ csr_write(CSR_TVEC, &handle_exception);
+#endif
}
--
2.28.0


2020-08-11 06:43:46

by Damien Le Moal

[permalink] [raw]
Subject: Re: [PATCH] riscv: Setup exception vector for K210 properly

On 2020/08/11 15:38, Qiu Wenbo wrote:
> Exception vector is missing on nommu platform and it is a big issue.
> This patch is tested in Sipeed MAIX Bit Dev Board.
>
> Fixes: 79b1feba5455 ("RISC-V: Setup exception vector early")
> Signed-off-by: Qiu Wenbo <[email protected]>
> ---
> arch/riscv/kernel/smpboot.c | 1 +
> arch/riscv/kernel/traps.c | 11 ++++++++++-
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> index 356825a57551..23cde0ceb39d 100644
> --- a/arch/riscv/kernel/smpboot.c
> +++ b/arch/riscv/kernel/smpboot.c
> @@ -154,6 +154,7 @@ asmlinkage __visible void smp_callin(void)
> mmgrab(mm);
> current->active_mm = mm;
>
> + trap_init();
> notify_cpu_starting(curr_cpuid);
> update_siblings_masks(curr_cpuid);
> set_cpu_online(curr_cpuid, 1);
> diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
> index ad14f4466d92..a390239818ae 100644
> --- a/arch/riscv/kernel/traps.c
> +++ b/arch/riscv/kernel/traps.c
> @@ -174,7 +174,16 @@ int is_valid_bugaddr(unsigned long pc)
> }
> #endif /* CONFIG_GENERIC_BUG */
>
> -/* stvec & scratch is already set from head.S */
> +/* stvec & scratch is already set from head.S when mmu is enabled */
> void trap_init(void)
> {
> +#ifndef CONFIG_MMU
> + /*
> + * Set sup0 scratch register to 0, indicating to exception vector
> + * that we are presently executing in the kernel
> + */
> + csr_write(CSR_SCRATCH, 0);
> + /* Set the exception vector address */
> + csr_write(CSR_TVEC, &handle_exception);
> +#endif
> }
>

Looks OK to me. But out of curiosity, how did you trigger a problem ? I never
got any weird exceptions with my busybox userspace.

--
Damien Le Moal
Western Digital Research

2020-08-11 06:44:54

by Damien Le Moal

[permalink] [raw]
Subject: Re: [PATCH] riscv: Setup exception vector for K210 properly

On 2020/08/11 15:38, Qiu Wenbo wrote:
> Exception vector is missing on nommu platform and it is a big issue.
> This patch is tested in Sipeed MAIX Bit Dev Board.
>
> Fixes: 79b1feba5455 ("RISC-V: Setup exception vector early")

I think this needs a "Cc: [email protected] #5.8" too.

> Signed-off-by: Qiu Wenbo <[email protected]>
> ---
> arch/riscv/kernel/smpboot.c | 1 +
> arch/riscv/kernel/traps.c | 11 ++++++++++-
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> index 356825a57551..23cde0ceb39d 100644
> --- a/arch/riscv/kernel/smpboot.c
> +++ b/arch/riscv/kernel/smpboot.c
> @@ -154,6 +154,7 @@ asmlinkage __visible void smp_callin(void)
> mmgrab(mm);
> current->active_mm = mm;
>
> + trap_init();
> notify_cpu_starting(curr_cpuid);
> update_siblings_masks(curr_cpuid);
> set_cpu_online(curr_cpuid, 1);
> diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
> index ad14f4466d92..a390239818ae 100644
> --- a/arch/riscv/kernel/traps.c
> +++ b/arch/riscv/kernel/traps.c
> @@ -174,7 +174,16 @@ int is_valid_bugaddr(unsigned long pc)
> }
> #endif /* CONFIG_GENERIC_BUG */
>
> -/* stvec & scratch is already set from head.S */
> +/* stvec & scratch is already set from head.S when mmu is enabled */
> void trap_init(void)
> {
> +#ifndef CONFIG_MMU
> + /*
> + * Set sup0 scratch register to 0, indicating to exception vector
> + * that we are presently executing in the kernel
> + */
> + csr_write(CSR_SCRATCH, 0);
> + /* Set the exception vector address */
> + csr_write(CSR_TVEC, &handle_exception);
> +#endif
> }
>


--
Damien Le Moal
Western Digital Research

2020-08-11 07:07:08

by Qiu Wenbo

[permalink] [raw]
Subject: Re: Re: [PATCH] riscv: Setup exception vector for K210 properly

The serial port did not print anything after early console.

[ 0.000000] Sorting __ex_table...
[ 0.000000] mem auto-init: stack:off, heap alloc:off, heap free:off
[ 0.000000] Memory: 6480K/8192K available (1024K kernel code, 111K rwdata, 170K rodata, 101K init, 97K bss, 1712K reserved, 0K cma-reserved)
[ 0.000000] rcu: Hierarchical RCU implementation.
[ 0.000000] rcu: RCU calculated value of scheduler-enlistment delay is 25 jiffies.
[ 0.000000] NR_IRQS: 64, nr_irqs: 64, preallocated irqs: 0
[ 0.000000] riscv-intc: 64 local interrupts mapped
[ 0.000000] plic: interrupt-controller@c000000: mapped 65 interrupts with 2 handlers for 4 contexts.
[ 0.000000] random: get_random_bytes called from 0x00000000800019a4 with crng_init=0
[ 0.000000] riscv_timer_init_dt: Registering clocksource cpuid [0] hartid [0]
[ 0.000000] clocksource: riscv_clocksource: mask: 0xffffffffffffffff max_cycles: 0x3990be68b, max_idle_ns: 881590404272 ns
[ 0.000015] sched_clock: 64 bits at 7MHz, resolution 128ns, wraps every 4398046511054ns
[ 0.008254] Console: colour dummy device 80x25



&gt; -----原始邮件-----
&gt; 发件人: "Damien Le Moal" <[email protected]>
&gt; 发送时间: 2020-08-11 14:42:15 (星期二)
&gt; 收件人: "Qiu Wenbo" <[email protected]>, "Palmer Dabbelt" <[email protected]>, "Paul Walmsley" <[email protected]>, "[email protected]" <[email protected]>
&gt; 抄送: "Albert Ou" <[email protected]>, "Atish Patra" <[email protected]>, "Anup
&gt; Patel" <[email protected]>, "Guo Ren" <[email protected]>, "Zong Li" <[email protected]>, "Greentime Hu" <[email protected]>, "Vincent Chen" <[email protected]>, "[email protected]" <[email protected]>
&gt; 主题: Re: [PATCH] riscv: Setup exception vector for K210 properly
&gt;
&gt; On 2020/08/11 15:38, Qiu Wenbo wrote:
&gt; &gt; Exception vector is missing on nommu platform and it is a big issue.
&gt; &gt; This patch is tested in Sipeed MAIX Bit Dev Board.
&gt; &gt;
&gt; &gt; Fixes: 79b1feba5455 ("RISC-V: Setup exception vector early")
&gt; &gt; Signed-off-by: Qiu Wenbo <[email protected]>
&gt; &gt; ---
&gt; &gt; arch/riscv/kernel/smpboot.c | 1 +
&gt; &gt; arch/riscv/kernel/traps.c | 11 ++++++++++-
&gt; &gt; 2 files changed, 11 insertions(+), 1 deletion(-)
&gt; &gt;
&gt; &gt; diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
&gt; &gt; index 356825a57551..23cde0ceb39d 100644
&gt; &gt; --- a/arch/riscv/kernel/smpboot.c
&gt; &gt; +++ b/arch/riscv/kernel/smpboot.c
&gt; &gt; @@ -154,6 +154,7 @@ asmlinkage __visible void smp_callin(void)
&gt; &gt; mmgrab(mm);
&gt; &gt; current-&gt;active_mm = mm;
&gt; &gt;
&gt; &gt; + trap_init();
&gt; &gt; notify_cpu_starting(curr_cpuid);
&gt; &gt; update_siblings_masks(curr_cpuid);
&gt; &gt; set_cpu_online(curr_cpuid, 1);
&gt; &gt; diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
&gt; &gt; index ad14f4466d92..a390239818ae 100644
&gt; &gt; --- a/arch/riscv/kernel/traps.c
&gt; &gt; +++ b/arch/riscv/kernel/traps.c
&gt; &gt; @@ -174,7 +174,16 @@ int is_valid_bugaddr(unsigned long pc)
&gt; &gt; }
&gt; &gt; #endif /* CONFIG_GENERIC_BUG */
&gt; &gt;
&gt; &gt; -/* stvec &amp; scratch is already set from head.S */
&gt; &gt; +/* stvec &amp; scratch is already set from head.S when mmu is enabled */
&gt; &gt; void trap_init(void)
&gt; &gt; {
&gt; &gt; +#ifndef CONFIG_MMU
&gt; &gt; + /*
&gt; &gt; + * Set sup0 scratch register to 0, indicating to exception vector
&gt; &gt; + * that we are presently executing in the kernel
&gt; &gt; + */
&gt; &gt; + csr_write(CSR_SCRATCH, 0);
&gt; &gt; + /* Set the exception vector address */
&gt; &gt; + csr_write(CSR_TVEC, &amp;handle_exception);
&gt; &gt; +#endif
&gt; &gt; }
&gt; &gt;
&gt;
&gt; Looks OK to me. But out of curiosity, how did you trigger a problem ? I never
&gt; got any weird exceptions with my busybox userspace.
&gt;
&gt; --
&gt; Damien Le Moal
&gt; Western Digital Research
</[email protected]></[email protected]></[email protected]></[email protected]></[email protected]></[email protected]></[email protected]></[email protected]></[email protected]></[email protected]></[email protected]></[email protected]></[email protected]></[email protected]>




2020-08-11 07:48:03

by Damien Le Moal

[permalink] [raw]
Subject: Re: [PATCH] riscv: Setup exception vector for K210 properly

On 2020/08/11 16:06, ???IJ? wrote:
> The serial port did not print anything after early console.
>
> [ 0.000000] Sorting __ex_table...
> [ 0.000000] mem auto-init: stack:off, heap alloc:off, heap free:off
> [ 0.000000] Memory: 6480K/8192K available (1024K kernel code, 111K rwdata, 170K rodata, 101K init, 97K bss, 1712K reserved, 0K cma-reserved)
> [ 0.000000] rcu: Hierarchical RCU implementation.
> [ 0.000000] rcu: RCU calculated value of scheduler-enlistment delay is 25 jiffies.
> [ 0.000000] NR_IRQS: 64, nr_irqs: 64, preallocated irqs: 0
> [ 0.000000] riscv-intc: 64 local interrupts mapped
> [ 0.000000] plic: interrupt-controller@c000000: mapped 65 interrupts with 2 handlers for 4 contexts.
> [ 0.000000] random: get_random_bytes called from 0x00000000800019a4 with crng_init=0
> [ 0.000000] riscv_timer_init_dt: Registering clocksource cpuid [0] hartid [0]
> [ 0.000000] clocksource: riscv_clocksource: mask: 0xffffffffffffffff max_cycles: 0x3990be68b, max_idle_ns: 881590404272 ns
> [ 0.000015] sched_clock: 64 bits at 7MHz, resolution 128ns, wraps every 4398046511054ns
> [ 0.008254] Console: colour dummy device 80x25

Interesting. Never saw that happening... Thanks !

>
>
>
> &gt; -----ԭʼ?ʼ?-----
> &gt; ??????: "Damien Le Moal" <[email protected]>
> &gt; ????ʱ??: 2020-08-11 14:42:15 (???ڶ?)
> &gt; ?ռ???: "Qiu Wenbo" <[email protected]>, "Palmer Dabbelt" <[email protected]>, "Paul Walmsley" <[email protected]>, "[email protected]" <[email protected]>
> &gt; ????: "Albert Ou" <[email protected]>, "Atish Patra" <[email protected]>, "Anup
> &gt; Patel" <[email protected]>, "Guo Ren" <[email protected]>, "Zong Li" <[email protected]>, "Greentime Hu" <[email protected]>, "Vincent Chen" <[email protected]>, "[email protected]" <[email protected]>
> &gt; ????: Re: [PATCH] riscv: Setup exception vector for K210 properly
> &gt;
> &gt; On 2020/08/11 15:38, Qiu Wenbo wrote:
> &gt; &gt; Exception vector is missing on nommu platform and it is a big issue.
> &gt; &gt; This patch is tested in Sipeed MAIX Bit Dev Board.
> &gt; &gt;
> &gt; &gt; Fixes: 79b1feba5455 ("RISC-V: Setup exception vector early")
> &gt; &gt; Signed-off-by: Qiu Wenbo <[email protected]>
> &gt; &gt; ---
> &gt; &gt; arch/riscv/kernel/smpboot.c | 1 +
> &gt; &gt; arch/riscv/kernel/traps.c | 11 ++++++++++-
> &gt; &gt; 2 files changed, 11 insertions(+), 1 deletion(-)
> &gt; &gt;
> &gt; &gt; diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> &gt; &gt; index 356825a57551..23cde0ceb39d 100644
> &gt; &gt; --- a/arch/riscv/kernel/smpboot.c
> &gt; &gt; +++ b/arch/riscv/kernel/smpboot.c
> &gt; &gt; @@ -154,6 +154,7 @@ asmlinkage __visible void smp_callin(void)
> &gt; &gt; mmgrab(mm);
> &gt; &gt; current-&gt;active_mm = mm;
> &gt; &gt;
> &gt; &gt; + trap_init();
> &gt; &gt; notify_cpu_starting(curr_cpuid);
> &gt; &gt; update_siblings_masks(curr_cpuid);
> &gt; &gt; set_cpu_online(curr_cpuid, 1);
> &gt; &gt; diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
> &gt; &gt; index ad14f4466d92..a390239818ae 100644
> &gt; &gt; --- a/arch/riscv/kernel/traps.c
> &gt; &gt; +++ b/arch/riscv/kernel/traps.c
> &gt; &gt; @@ -174,7 +174,16 @@ int is_valid_bugaddr(unsigned long pc)
> &gt; &gt; }
> &gt; &gt; #endif /* CONFIG_GENERIC_BUG */
> &gt; &gt;
> &gt; &gt; -/* stvec &amp; scratch is already set from head.S */
> &gt; &gt; +/* stvec &amp; scratch is already set from head.S when mmu is enabled */
> &gt; &gt; void trap_init(void)
> &gt; &gt; {
> &gt; &gt; +#ifndef CONFIG_MMU
> &gt; &gt; + /*
> &gt; &gt; + * Set sup0 scratch register to 0, indicating to exception vector
> &gt; &gt; + * that we are presently executing in the kernel
> &gt; &gt; + */
> &gt; &gt; + csr_write(CSR_SCRATCH, 0);
> &gt; &gt; + /* Set the exception vector address */
> &gt; &gt; + csr_write(CSR_TVEC, &amp;handle_exception);
> &gt; &gt; +#endif
> &gt; &gt; }
> &gt; &gt;
> &gt;
> &gt; Looks OK to me. But out of curiosity, how did you trigger a problem ? I never
> &gt; got any weird exceptions with my busybox userspace.
> &gt;
> &gt; --
> &gt; Damien Le Moal
> &gt; Western Digital Research
> </[email protected]></[email protected]></[email protected]></[email protected]></[email protected]></[email protected]></[email protected]></[email protected]></[email protected]></[email protected]></[email protected]></[email protected]></[email protected]></[email protected]>
>
>
>
>
>


--
Damien Le Moal
Western Digital Research

2020-08-11 08:43:59

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH] riscv: Setup exception vector for K210 properly

On Tue, Aug 11, 2020 at 12:07 PM Qiu Wenbo <[email protected]> wrote:
>
> Exception vector is missing on nommu platform and it is a big issue.
> This patch is tested in Sipeed MAIX Bit Dev Board.
>
> Fixes: 79b1feba5455 ("RISC-V: Setup exception vector early")
> Signed-off-by: Qiu Wenbo <[email protected]>
> ---
> arch/riscv/kernel/smpboot.c | 1 +
> arch/riscv/kernel/traps.c | 11 ++++++++++-
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> index 356825a57551..23cde0ceb39d 100644
> --- a/arch/riscv/kernel/smpboot.c
> +++ b/arch/riscv/kernel/smpboot.c
> @@ -154,6 +154,7 @@ asmlinkage __visible void smp_callin(void)
> mmgrab(mm);
> current->active_mm = mm;
>
> + trap_init();
> notify_cpu_starting(curr_cpuid);
> update_siblings_masks(curr_cpuid);
> set_cpu_online(curr_cpuid, 1);
> diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
> index ad14f4466d92..a390239818ae 100644
> --- a/arch/riscv/kernel/traps.c
> +++ b/arch/riscv/kernel/traps.c
> @@ -174,7 +174,16 @@ int is_valid_bugaddr(unsigned long pc)
> }
> #endif /* CONFIG_GENERIC_BUG */
>
> -/* stvec & scratch is already set from head.S */
> +/* stvec & scratch is already set from head.S when mmu is enabled */
> void trap_init(void)
> {
> +#ifndef CONFIG_MMU
> + /*
> + * Set sup0 scratch register to 0, indicating to exception vector
> + * that we are presently executing in the kernel
> + */
> + csr_write(CSR_SCRATCH, 0);
> + /* Set the exception vector address */
> + csr_write(CSR_TVEC, &handle_exception);
> +#endif
> }
> --
> 2.28.0
>

This issue seems to be only on the latest master branch of
Linux stable tree so this fix need not be a stable fix.

For MMU kernel, the CSR_TVEC is setup in relocate() function
called from secondary_start_common() function of head.S

For NoMMU kernel, we should set CSR_TVEC directly in
secondary_start_common() function as "#else" case of the
"#ifdef CONFIG_MMU".

Regards,
Anup

2020-08-11 18:47:07

by Atish Patra

[permalink] [raw]
Subject: Re: [PATCH] riscv: Setup exception vector for K210 properly

On Tue, Aug 11, 2020 at 1:41 AM Anup Patel <[email protected]> wrote:
>
> On Tue, Aug 11, 2020 at 12:07 PM Qiu Wenbo <[email protected]> wrote:
> >
> > Exception vector is missing on nommu platform and it is a big issue.
> > This patch is tested in Sipeed MAIX Bit Dev Board.
> >
> > Fixes: 79b1feba5455 ("RISC-V: Setup exception vector early")
> > Signed-off-by: Qiu Wenbo <[email protected]>

Thanks for testing it on the kendryte board.

> > ---
> > arch/riscv/kernel/smpboot.c | 1 +
> > arch/riscv/kernel/traps.c | 11 ++++++++++-
> > 2 files changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> > index 356825a57551..23cde0ceb39d 100644
> > --- a/arch/riscv/kernel/smpboot.c
> > +++ b/arch/riscv/kernel/smpboot.c
> > @@ -154,6 +154,7 @@ asmlinkage __visible void smp_callin(void)
> > mmgrab(mm);
> > current->active_mm = mm;
> >
> > + trap_init();
> > notify_cpu_starting(curr_cpuid);
> > update_siblings_masks(curr_cpuid);
> > set_cpu_online(curr_cpuid, 1);
> > diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
> > index ad14f4466d92..a390239818ae 100644
> > --- a/arch/riscv/kernel/traps.c
> > +++ b/arch/riscv/kernel/traps.c
> > @@ -174,7 +174,16 @@ int is_valid_bugaddr(unsigned long pc)
> > }
> > #endif /* CONFIG_GENERIC_BUG */
> >
> > -/* stvec & scratch is already set from head.S */
> > +/* stvec & scratch is already set from head.S when mmu is enabled */
> > void trap_init(void)
> > {
> > +#ifndef CONFIG_MMU
> > + /*
> > + * Set sup0 scratch register to 0, indicating to exception vector
> > + * that we are presently executing in the kernel
> > + */
> > + csr_write(CSR_SCRATCH, 0);
> > + /* Set the exception vector address */
> > + csr_write(CSR_TVEC, &handle_exception);
> > +#endif
> > }
> > --
> > 2.28.0
> >
>
> This issue seems to be only on the latest master branch of
> Linux stable tree so this fix need not be a stable fix.
>
> For MMU kernel, the CSR_TVEC is setup in relocate() function
> called from secondary_start_common() function of head.S
>
> For NoMMU kernel, we should set CSR_TVEC directly in
> secondary_start_common() function as "#else" case of the
> "#ifdef CONFIG_MMU".
>

That would enable the trap only for secondary harts. But the exception
vector on boot hart
is still uninitialized. How about this change ?

diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index d0c5c316e9bb..7822054dbd88 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -77,16 +77,6 @@ relocate:
csrw CSR_SATP, a0
.align 2
1:
- /* Set trap vector to exception handler */
- la a0, handle_exception
- csrw CSR_TVEC, a0
-
- /*
- * Set sup0 scratch register to 0, indicating to exception vector that
- * we are presently executing in kernel.
- */
- csrw CSR_SCRATCH, zero
-
/* Reload the global pointer */
.option push
.option norelax
@@ -144,9 +134,23 @@ secondary_start_common:
la a0, swapper_pg_dir
call relocate
#endif
+ call setup_trap_vector
tail smp_callin
#endif /* CONFIG_SMP */

+.align 2
+setup_trap_vector:
+ /* Set trap vector to exception handler */
+ la a0, handle_exception
+ csrw CSR_TVEC, a0
+
+ /*
+ * Set sup0 scratch register to 0, indicating to exception vector that
+ * we are presently executing in kernel.
+ */
+ csrw CSR_SCRATCH, zero
+ ret
+
.Lsecondary_park:
/* We lack SMP support or have too many harts, so park this hart */
wfi
@@ -240,6 +244,7 @@ clear_bss_done:
call relocate
#endif /* CONFIG_MMU */

+ call setup_trap_vector
/* Restore C environment */
la tp, init_task
sw zero, TASK_TI_CPU(tp)


> Regards,
> Anup
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv



--
Regards,
Atish

2020-08-12 01:11:04

by Qiu Wenbo

[permalink] [raw]
Subject: Re: [PATCH] riscv: Setup exception vector for K210 properly

That is a more reasonable approach.

On 8/12/20 2:45 AM, Atish Patra wrote:
> On Tue, Aug 11, 2020 at 1:41 AM Anup Patel <[email protected]> wrote:
>> On Tue, Aug 11, 2020 at 12:07 PM Qiu Wenbo <[email protected]> wrote:
>>> Exception vector is missing on nommu platform and it is a big issue.
>>> This patch is tested in Sipeed MAIX Bit Dev Board.
>>>
>>> Fixes: 79b1feba5455 ("RISC-V: Setup exception vector early")
>>> Signed-off-by: Qiu Wenbo <[email protected]>
> Thanks for testing it on the kendryte board.
>
>>> ---
>>> arch/riscv/kernel/smpboot.c | 1 +
>>> arch/riscv/kernel/traps.c | 11 ++++++++++-
>>> 2 files changed, 11 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
>>> index 356825a57551..23cde0ceb39d 100644
>>> --- a/arch/riscv/kernel/smpboot.c
>>> +++ b/arch/riscv/kernel/smpboot.c
>>> @@ -154,6 +154,7 @@ asmlinkage __visible void smp_callin(void)
>>> mmgrab(mm);
>>> current->active_mm = mm;
>>>
>>> + trap_init();
>>> notify_cpu_starting(curr_cpuid);
>>> update_siblings_masks(curr_cpuid);
>>> set_cpu_online(curr_cpuid, 1);
>>> diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
>>> index ad14f4466d92..a390239818ae 100644
>>> --- a/arch/riscv/kernel/traps.c
>>> +++ b/arch/riscv/kernel/traps.c
>>> @@ -174,7 +174,16 @@ int is_valid_bugaddr(unsigned long pc)
>>> }
>>> #endif /* CONFIG_GENERIC_BUG */
>>>
>>> -/* stvec & scratch is already set from head.S */
>>> +/* stvec & scratch is already set from head.S when mmu is enabled */
>>> void trap_init(void)
>>> {
>>> +#ifndef CONFIG_MMU
>>> + /*
>>> + * Set sup0 scratch register to 0, indicating to exception vector
>>> + * that we are presently executing in the kernel
>>> + */
>>> + csr_write(CSR_SCRATCH, 0);
>>> + /* Set the exception vector address */
>>> + csr_write(CSR_TVEC, &handle_exception);
>>> +#endif
>>> }
>>> --
>>> 2.28.0
>>>
>> This issue seems to be only on the latest master branch of
>> Linux stable tree so this fix need not be a stable fix.
>>
>> For MMU kernel, the CSR_TVEC is setup in relocate() function
>> called from secondary_start_common() function of head.S
>>
>> For NoMMU kernel, we should set CSR_TVEC directly in
>> secondary_start_common() function as "#else" case of the
>> "#ifdef CONFIG_MMU".
>>
> That would enable the trap only for secondary harts. But the exception
> vector on boot hart
> is still uninitialized. How about this change ?
>
> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> index d0c5c316e9bb..7822054dbd88 100644
> --- a/arch/riscv/kernel/head.S
> +++ b/arch/riscv/kernel/head.S
> @@ -77,16 +77,6 @@ relocate:
> csrw CSR_SATP, a0
> .align 2
> 1:
> - /* Set trap vector to exception handler */
> - la a0, handle_exception
> - csrw CSR_TVEC, a0
> -
> - /*
> - * Set sup0 scratch register to 0, indicating to exception vector that
> - * we are presently executing in kernel.
> - */
> - csrw CSR_SCRATCH, zero
> -
> /* Reload the global pointer */
> .option push
> .option norelax
> @@ -144,9 +134,23 @@ secondary_start_common:
> la a0, swapper_pg_dir
> call relocate
> #endif
> + call setup_trap_vector
> tail smp_callin
> #endif /* CONFIG_SMP */
>
> +.align 2
> +setup_trap_vector:
> + /* Set trap vector to exception handler */
> + la a0, handle_exception
> + csrw CSR_TVEC, a0
> +
> + /*
> + * Set sup0 scratch register to 0, indicating to exception vector that
> + * we are presently executing in kernel.
> + */
> + csrw CSR_SCRATCH, zero
> + ret
> +
> .Lsecondary_park:
> /* We lack SMP support or have too many harts, so park this hart */
> wfi
> @@ -240,6 +244,7 @@ clear_bss_done:
> call relocate
> #endif /* CONFIG_MMU */
>
> + call setup_trap_vector
> /* Restore C environment */
> la tp, init_task
> sw zero, TASK_TI_CPU(tp)
>
>
>> Regards,
>> Anup
>>
>> _______________________________________________
>> linux-riscv mailing list
>> [email protected]
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>
>

2020-08-12 01:59:45

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH] riscv: Setup exception vector for K210 properly

On Wed, Aug 12, 2020 at 12:16 AM Atish Patra <[email protected]> wrote:
>
> On Tue, Aug 11, 2020 at 1:41 AM Anup Patel <[email protected]> wrote:
> >
> > On Tue, Aug 11, 2020 at 12:07 PM Qiu Wenbo <[email protected]> wrote:
> > >
> > > Exception vector is missing on nommu platform and it is a big issue.
> > > This patch is tested in Sipeed MAIX Bit Dev Board.
> > >
> > > Fixes: 79b1feba5455 ("RISC-V: Setup exception vector early")
> > > Signed-off-by: Qiu Wenbo <[email protected]>
>
> Thanks for testing it on the kendryte board.
>
> > > ---
> > > arch/riscv/kernel/smpboot.c | 1 +
> > > arch/riscv/kernel/traps.c | 11 ++++++++++-
> > > 2 files changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> > > index 356825a57551..23cde0ceb39d 100644
> > > --- a/arch/riscv/kernel/smpboot.c
> > > +++ b/arch/riscv/kernel/smpboot.c
> > > @@ -154,6 +154,7 @@ asmlinkage __visible void smp_callin(void)
> > > mmgrab(mm);
> > > current->active_mm = mm;
> > >
> > > + trap_init();
> > > notify_cpu_starting(curr_cpuid);
> > > update_siblings_masks(curr_cpuid);
> > > set_cpu_online(curr_cpuid, 1);
> > > diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
> > > index ad14f4466d92..a390239818ae 100644
> > > --- a/arch/riscv/kernel/traps.c
> > > +++ b/arch/riscv/kernel/traps.c
> > > @@ -174,7 +174,16 @@ int is_valid_bugaddr(unsigned long pc)
> > > }
> > > #endif /* CONFIG_GENERIC_BUG */
> > >
> > > -/* stvec & scratch is already set from head.S */
> > > +/* stvec & scratch is already set from head.S when mmu is enabled */
> > > void trap_init(void)
> > > {
> > > +#ifndef CONFIG_MMU
> > > + /*
> > > + * Set sup0 scratch register to 0, indicating to exception vector
> > > + * that we are presently executing in the kernel
> > > + */
> > > + csr_write(CSR_SCRATCH, 0);
> > > + /* Set the exception vector address */
> > > + csr_write(CSR_TVEC, &handle_exception);
> > > +#endif
> > > }
> > > --
> > > 2.28.0
> > >
> >
> > This issue seems to be only on the latest master branch of
> > Linux stable tree so this fix need not be a stable fix.
> >
> > For MMU kernel, the CSR_TVEC is setup in relocate() function
> > called from secondary_start_common() function of head.S
> >
> > For NoMMU kernel, we should set CSR_TVEC directly in
> > secondary_start_common() function as "#else" case of the
> > "#ifdef CONFIG_MMU".
> >
>
> That would enable the trap only for secondary harts. But the exception
> vector on boot hart
> is still uninitialized. How about this change ?
>
> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> index d0c5c316e9bb..7822054dbd88 100644
> --- a/arch/riscv/kernel/head.S
> +++ b/arch/riscv/kernel/head.S
> @@ -77,16 +77,6 @@ relocate:
> csrw CSR_SATP, a0
> .align 2
> 1:
> - /* Set trap vector to exception handler */
> - la a0, handle_exception
> - csrw CSR_TVEC, a0
> -
> - /*
> - * Set sup0 scratch register to 0, indicating to exception vector that
> - * we are presently executing in kernel.
> - */
> - csrw CSR_SCRATCH, zero
> -

Instead of having no trap vector setup here, we should
at least have dummy trap vector (just like original code).

/* Set trap vector to spin forever to help debug */
la a0, .Lsecondary_park
csrw CSR_TVEC, a0

> /* Reload the global pointer */
> .option push
> .option norelax
> @@ -144,9 +134,23 @@ secondary_start_common:
> la a0, swapper_pg_dir
> call relocate
> #endif
> + call setup_trap_vector
> tail smp_callin
> #endif /* CONFIG_SMP */
>
> +.align 2
> +setup_trap_vector:
> + /* Set trap vector to exception handler */
> + la a0, handle_exception
> + csrw CSR_TVEC, a0
> +
> + /*
> + * Set sup0 scratch register to 0, indicating to exception vector that
> + * we are presently executing in kernel.
> + */
> + csrw CSR_SCRATCH, zero
> + ret
> +
> .Lsecondary_park:
> /* We lack SMP support or have too many harts, so park this hart */
> wfi
> @@ -240,6 +244,7 @@ clear_bss_done:
> call relocate
> #endif /* CONFIG_MMU */
>
> + call setup_trap_vector
> /* Restore C environment */
> la tp, init_task
> sw zero, TASK_TI_CPU(tp)

Apart from above, this looks good.

Regards,
Anup

2020-08-12 07:43:25

by Atish Patra

[permalink] [raw]
Subject: Re: [PATCH] riscv: Setup exception vector for K210 properly

On Tue, Aug 11, 2020 at 6:57 PM Anup Patel <[email protected]> wrote:
>
> On Wed, Aug 12, 2020 at 12:16 AM Atish Patra <[email protected]> wrote:
> >
> > On Tue, Aug 11, 2020 at 1:41 AM Anup Patel <[email protected]> wrote:
> > >
> > > On Tue, Aug 11, 2020 at 12:07 PM Qiu Wenbo <[email protected]> wrote:
> > > >
> > > > Exception vector is missing on nommu platform and it is a big issue.
> > > > This patch is tested in Sipeed MAIX Bit Dev Board.
> > > >
> > > > Fixes: 79b1feba5455 ("RISC-V: Setup exception vector early")
> > > > Signed-off-by: Qiu Wenbo <[email protected]>
> >
> > Thanks for testing it on the kendryte board.
> >
> > > > ---
> > > > arch/riscv/kernel/smpboot.c | 1 +
> > > > arch/riscv/kernel/traps.c | 11 ++++++++++-
> > > > 2 files changed, 11 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> > > > index 356825a57551..23cde0ceb39d 100644
> > > > --- a/arch/riscv/kernel/smpboot.c
> > > > +++ b/arch/riscv/kernel/smpboot.c
> > > > @@ -154,6 +154,7 @@ asmlinkage __visible void smp_callin(void)
> > > > mmgrab(mm);
> > > > current->active_mm = mm;
> > > >
> > > > + trap_init();
> > > > notify_cpu_starting(curr_cpuid);
> > > > update_siblings_masks(curr_cpuid);
> > > > set_cpu_online(curr_cpuid, 1);
> > > > diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
> > > > index ad14f4466d92..a390239818ae 100644
> > > > --- a/arch/riscv/kernel/traps.c
> > > > +++ b/arch/riscv/kernel/traps.c
> > > > @@ -174,7 +174,16 @@ int is_valid_bugaddr(unsigned long pc)
> > > > }
> > > > #endif /* CONFIG_GENERIC_BUG */
> > > >
> > > > -/* stvec & scratch is already set from head.S */
> > > > +/* stvec & scratch is already set from head.S when mmu is enabled */
> > > > void trap_init(void)
> > > > {
> > > > +#ifndef CONFIG_MMU
> > > > + /*
> > > > + * Set sup0 scratch register to 0, indicating to exception vector
> > > > + * that we are presently executing in the kernel
> > > > + */
> > > > + csr_write(CSR_SCRATCH, 0);
> > > > + /* Set the exception vector address */
> > > > + csr_write(CSR_TVEC, &handle_exception);
> > > > +#endif
> > > > }
> > > > --
> > > > 2.28.0
> > > >
> > >
> > > This issue seems to be only on the latest master branch of
> > > Linux stable tree so this fix need not be a stable fix.
> > >
> > > For MMU kernel, the CSR_TVEC is setup in relocate() function
> > > called from secondary_start_common() function of head.S
> > >
> > > For NoMMU kernel, we should set CSR_TVEC directly in
> > > secondary_start_common() function as "#else" case of the
> > > "#ifdef CONFIG_MMU".
> > >
> >
> > That would enable the trap only for secondary harts. But the exception
> > vector on boot hart
> > is still uninitialized. How about this change ?
> >
> > diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> > index d0c5c316e9bb..7822054dbd88 100644
> > --- a/arch/riscv/kernel/head.S
> > +++ b/arch/riscv/kernel/head.S
> > @@ -77,16 +77,6 @@ relocate:
> > csrw CSR_SATP, a0
> > .align 2
> > 1:
> > - /* Set trap vector to exception handler */
> > - la a0, handle_exception
> > - csrw CSR_TVEC, a0
> > -
> > - /*
> > - * Set sup0 scratch register to 0, indicating to exception vector that
> > - * we are presently executing in kernel.
> > - */
> > - csrw CSR_SCRATCH, zero
> > -
>
> Instead of having no trap vector setup here, we should
> at least have dummy trap vector (just like original code).
>

Ahh yes. We should include that.

> /* Set trap vector to spin forever to help debug */
> la a0, .Lsecondary_park
> csrw CSR_TVEC, a0
>
> > /* Reload the global pointer */
> > .option push
> > .option norelax
> > @@ -144,9 +134,23 @@ secondary_start_common:
> > la a0, swapper_pg_dir
> > call relocate
> > #endif
> > + call setup_trap_vector
> > tail smp_callin
> > #endif /* CONFIG_SMP */
> >
> > +.align 2
> > +setup_trap_vector:
> > + /* Set trap vector to exception handler */
> > + la a0, handle_exception
> > + csrw CSR_TVEC, a0
> > +
> > + /*
> > + * Set sup0 scratch register to 0, indicating to exception vector that
> > + * we are presently executing in kernel.
> > + */
> > + csrw CSR_SCRATCH, zero
> > + ret
> > +
> > .Lsecondary_park:
> > /* We lack SMP support or have too many harts, so park this hart */
> > wfi
> > @@ -240,6 +244,7 @@ clear_bss_done:
> > call relocate
> > #endif /* CONFIG_MMU */
> >
> > + call setup_trap_vector
> > /* Restore C environment */
> > la tp, init_task
> > sw zero, TASK_TI_CPU(tp)
>
> Apart from above, this looks good.
>
> Regards,
> Anup



--
Regards,
Atish