* [PATCH] RISC-V: Check clint_time_val before use
@ 2020-09-26 7:27 Anup Patel
2020-09-26 9:25 ` Damien Le Moal
0 siblings, 1 reply; 9+ messages in thread
From: Anup Patel @ 2020-09-26 7:27 UTC (permalink / raw)
To: Palmer Dabbelt, Palmer Dabbelt, Paul Walmsley, Albert Ou
Cc: Damien Le Moal, Anup Patel, Anup Patel, linux-kernel,
Atish Patra, Alistair Francis, linux-riscv
The NoMMU kernel is broken for QEMU virt machine from Linux-5.9-rc6
because the get_cycles() and friends are called very early from
rand_initialize() before CLINT driver is probed. To fix this, we
should check clint_time_val before use in get_cycles() and friends.
Fixes: d5be89a8d118 ("RISC-V: Resurrect the MMIO timer implementation
for M-mode systems")
Signed-off-by: Anup Patel <anup.patel@wdc.com>
---
arch/riscv/include/asm/timex.h | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/arch/riscv/include/asm/timex.h b/arch/riscv/include/asm/timex.h
index 7f659dda0032..52b42bb1602c 100644
--- a/arch/riscv/include/asm/timex.h
+++ b/arch/riscv/include/asm/timex.h
@@ -17,18 +17,24 @@ typedef unsigned long cycles_t;
#ifdef CONFIG_64BIT
static inline cycles_t get_cycles(void)
{
- return readq_relaxed(clint_time_val);
+ if (clint_time_val)
+ return readq_relaxed(clint_time_val);
+ return 0;
}
#else /* !CONFIG_64BIT */
static inline u32 get_cycles(void)
{
- return readl_relaxed(((u32 *)clint_time_val));
+ if (clint_time_val)
+ return readl_relaxed(((u32 *)clint_time_val));
+ return 0;
}
#define get_cycles get_cycles
static inline u32 get_cycles_hi(void)
{
- return readl_relaxed(((u32 *)clint_time_val) + 1);
+ if (clint_time_val)
+ return readl_relaxed(((u32 *)clint_time_val) + 1);
+ return 0
}
#define get_cycles_hi get_cycles_hi
#endif /* CONFIG_64BIT */
--
2.25.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] RISC-V: Check clint_time_val before use
2020-09-26 7:27 [PATCH] RISC-V: Check clint_time_val before use Anup Patel
@ 2020-09-26 9:25 ` Damien Le Moal
2020-09-26 9:31 ` Anup Patel
0 siblings, 1 reply; 9+ messages in thread
From: Damien Le Moal @ 2020-09-26 9:25 UTC (permalink / raw)
To: paul.walmsley, palmer, palmerdabbelt, Anup Patel, aou
Cc: anup, linux-riscv, Alistair Francis, linux-kernel, Atish Patra
On Sat, 2020-09-26 at 12:57 +0530, Anup Patel wrote:
> The NoMMU kernel is broken for QEMU virt machine from Linux-5.9-rc6
> because the get_cycles() and friends are called very early from
> rand_initialize() before CLINT driver is probed. To fix this, we
> should check clint_time_val before use in get_cycles() and friends.
>
> Fixes: d5be89a8d118 ("RISC-V: Resurrect the MMIO timer implementation
> for M-mode systems")
> Signed-off-by: Anup Patel <anup.patel@wdc.com>
> ---
> arch/riscv/include/asm/timex.h | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/arch/riscv/include/asm/timex.h b/arch/riscv/include/asm/timex.h
> index 7f659dda0032..52b42bb1602c 100644
> --- a/arch/riscv/include/asm/timex.h
> +++ b/arch/riscv/include/asm/timex.h
> @@ -17,18 +17,24 @@ typedef unsigned long cycles_t;
> #ifdef CONFIG_64BIT
> static inline cycles_t get_cycles(void)
> {
> - return readq_relaxed(clint_time_val);
> + if (clint_time_val)
> + return readq_relaxed(clint_time_val);
> + return 0;
> }
> #else /* !CONFIG_64BIT */
> static inline u32 get_cycles(void)
> {
> - return readl_relaxed(((u32 *)clint_time_val));
> + if (clint_time_val)
> + return readl_relaxed(((u32 *)clint_time_val));
> + return 0;
> }
> #define get_cycles get_cycles
>
> static inline u32 get_cycles_hi(void)
> {
> - return readl_relaxed(((u32 *)clint_time_val) + 1);
> + if (clint_time_val)
> + return readl_relaxed(((u32 *)clint_time_val) + 1);
> + return 0
> }
> #define get_cycles_hi get_cycles_hi
> #endif /* CONFIG_64BIT */
Applying this on top of rc6, I now get a hang on Kendryte boot...
No problems without the patch on the other hand.
--
Damien Le Moal
Western Digital
_______________________________________________
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] RISC-V: Check clint_time_val before use
2020-09-26 9:25 ` Damien Le Moal
@ 2020-09-26 9:31 ` Anup Patel
2020-09-26 9:46 ` Damien Le Moal
0 siblings, 1 reply; 9+ messages in thread
From: Anup Patel @ 2020-09-26 9:31 UTC (permalink / raw)
To: Damien Le Moal, paul.walmsley, palmer, palmerdabbelt, aou
Cc: anup, linux-riscv, Alistair Francis, linux-kernel, Atish Patra
> -----Original Message-----
> From: Damien Le Moal <Damien.LeMoal@wdc.com>
> Sent: 26 September 2020 14:55
> To: paul.walmsley@sifive.com; palmer@dabbelt.com;
> palmerdabbelt@google.com; Anup Patel <Anup.Patel@wdc.com>;
> aou@eecs.berkeley.edu
> Cc: anup@brainfault.org; linux-riscv@lists.infradead.org; Atish Patra
> <Atish.Patra@wdc.com>; Alistair Francis <Alistair.Francis@wdc.com>; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH] RISC-V: Check clint_time_val before use
>
> On Sat, 2020-09-26 at 12:57 +0530, Anup Patel wrote:
> > The NoMMU kernel is broken for QEMU virt machine from Linux-5.9-rc6
> > because the get_cycles() and friends are called very early from
> > rand_initialize() before CLINT driver is probed. To fix this, we
> > should check clint_time_val before use in get_cycles() and friends.
> >
> > Fixes: d5be89a8d118 ("RISC-V: Resurrect the MMIO timer implementation
> > for M-mode systems")
> > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> > ---
> > arch/riscv/include/asm/timex.h | 12 +++++++++---
> > 1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/timex.h
> > b/arch/riscv/include/asm/timex.h index 7f659dda0032..52b42bb1602c
> > 100644
> > --- a/arch/riscv/include/asm/timex.h
> > +++ b/arch/riscv/include/asm/timex.h
> > @@ -17,18 +17,24 @@ typedef unsigned long cycles_t; #ifdef
> > CONFIG_64BIT static inline cycles_t get_cycles(void) {
> > - return readq_relaxed(clint_time_val);
> > + if (clint_time_val)
> > + return readq_relaxed(clint_time_val);
> > + return 0;
> > }
> > #else /* !CONFIG_64BIT */
> > static inline u32 get_cycles(void)
> > {
> > - return readl_relaxed(((u32 *)clint_time_val));
> > + if (clint_time_val)
> > + return readl_relaxed(((u32 *)clint_time_val));
> > + return 0;
> > }
> > #define get_cycles get_cycles
> >
> > static inline u32 get_cycles_hi(void) {
> > - return readl_relaxed(((u32 *)clint_time_val) + 1);
> > + if (clint_time_val)
> > + return readl_relaxed(((u32 *)clint_time_val) + 1);
> > + return 0
> > }
> > #define get_cycles_hi get_cycles_hi
> > #endif /* CONFIG_64BIT */
>
> Applying this on top of rc6, I now get a hang on Kendryte boot...
> No problems without the patch on the other hand.
Not sure about the issue with Kendryte but I get a crash on
QEMU virt machine without this patch.
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] RISC-V: Check clint_time_val before use
2020-09-26 9:31 ` Anup Patel
@ 2020-09-26 9:46 ` Damien Le Moal
2020-09-26 9:57 ` Anup Patel
2020-09-26 10:09 ` Maciej W. Rozycki
0 siblings, 2 replies; 9+ messages in thread
From: Damien Le Moal @ 2020-09-26 9:46 UTC (permalink / raw)
To: paul.walmsley, palmer, palmerdabbelt, Anup Patel, aou
Cc: anup, linux-riscv, Alistair Francis, linux-kernel, Atish Patra
On Sat, 2020-09-26 at 09:31 +0000, Anup Patel wrote:
> > -----Original Message-----
> > From: Damien Le Moal <Damien.LeMoal@wdc.com>
> > Sent: 26 September 2020 14:55
> > To: paul.walmsley@sifive.com; palmer@dabbelt.com;
> > palmerdabbelt@google.com; Anup Patel <Anup.Patel@wdc.com>;
> > aou@eecs.berkeley.edu
> > Cc: anup@brainfault.org; linux-riscv@lists.infradead.org; Atish Patra
> > <Atish.Patra@wdc.com>; Alistair Francis <Alistair.Francis@wdc.com>; linux-
> > kernel@vger.kernel.org
> > Subject: Re: [PATCH] RISC-V: Check clint_time_val before use
> >
> > On Sat, 2020-09-26 at 12:57 +0530, Anup Patel wrote:
> > > The NoMMU kernel is broken for QEMU virt machine from Linux-5.9-rc6
> > > because the get_cycles() and friends are called very early from
> > > rand_initialize() before CLINT driver is probed. To fix this, we
> > > should check clint_time_val before use in get_cycles() and friends.
> > >
> > > Fixes: d5be89a8d118 ("RISC-V: Resurrect the MMIO timer implementation
> > > for M-mode systems")
> > > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> > > ---
> > > arch/riscv/include/asm/timex.h | 12 +++++++++---
> > > 1 file changed, 9 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/riscv/include/asm/timex.h
> > > b/arch/riscv/include/asm/timex.h index 7f659dda0032..52b42bb1602c
> > > 100644
> > > --- a/arch/riscv/include/asm/timex.h
> > > +++ b/arch/riscv/include/asm/timex.h
> > > @@ -17,18 +17,24 @@ typedef unsigned long cycles_t; #ifdef
> > > CONFIG_64BIT static inline cycles_t get_cycles(void) {
> > > - return readq_relaxed(clint_time_val);
> > > + if (clint_time_val)
> > > + return readq_relaxed(clint_time_val);
> > > + return 0;
> > > }
> > > #else /* !CONFIG_64BIT */
> > > static inline u32 get_cycles(void)
> > > {
> > > - return readl_relaxed(((u32 *)clint_time_val));
> > > + if (clint_time_val)
> > > + return readl_relaxed(((u32 *)clint_time_val));
> > > + return 0;
> > > }
> > > #define get_cycles get_cycles
> > >
> > > static inline u32 get_cycles_hi(void) {
> > > - return readl_relaxed(((u32 *)clint_time_val) + 1);
> > > + if (clint_time_val)
> > > + return readl_relaxed(((u32 *)clint_time_val) + 1);
> > > + return 0
> > > }
> > > #define get_cycles_hi get_cycles_hi
> > > #endif /* CONFIG_64BIT */
> >
> > Applying this on top of rc6, I now get a hang on Kendryte boot...
> > No problems without the patch on the other hand.
>
> Not sure about the issue with Kendryte but I get a crash on
> QEMU virt machine without this patch.
With this applied in addition to your patch, it works.
diff --git a/drivers/clocksource/timer-clint.c b/drivers/clocksource/timer-
clint.c
index d17367dee02c..8dbec85979fd 100644
--- a/drivers/clocksource/timer-clint.c
+++ b/drivers/clocksource/timer-clint.c
@@ -37,7 +37,7 @@ static unsigned long clint_timer_freq;
static unsigned int clint_timer_irq;
#ifdef CONFIG_RISCV_M_MODE
-u64 __iomem *clint_time_val;
+u64 __iomem *clint_time_val = NULL;
#endif
static void clint_send_ipi(const struct cpumask *target)
--
Damien Le Moal
Western Digital
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] RISC-V: Check clint_time_val before use
2020-09-26 9:46 ` Damien Le Moal
@ 2020-09-26 9:57 ` Anup Patel
2020-09-26 10:01 ` Damien Le Moal
2020-09-26 10:09 ` Maciej W. Rozycki
1 sibling, 1 reply; 9+ messages in thread
From: Anup Patel @ 2020-09-26 9:57 UTC (permalink / raw)
To: Damien Le Moal
Cc: palmerdabbelt, Anup Patel, linux-kernel, Atish Patra, aou,
palmer, paul.walmsley, Alistair Francis, linux-riscv
On Sat, Sep 26, 2020 at 3:16 PM Damien Le Moal <Damien.LeMoal@wdc.com> wrote:
>
> On Sat, 2020-09-26 at 09:31 +0000, Anup Patel wrote:
> > > -----Original Message-----
> > > From: Damien Le Moal <Damien.LeMoal@wdc.com>
> > > Sent: 26 September 2020 14:55
> > > To: paul.walmsley@sifive.com; palmer@dabbelt.com;
> > > palmerdabbelt@google.com; Anup Patel <Anup.Patel@wdc.com>;
> > > aou@eecs.berkeley.edu
> > > Cc: anup@brainfault.org; linux-riscv@lists.infradead.org; Atish Patra
> > > <Atish.Patra@wdc.com>; Alistair Francis <Alistair.Francis@wdc.com>; linux-
> > > kernel@vger.kernel.org
> > > Subject: Re: [PATCH] RISC-V: Check clint_time_val before use
> > >
> > > On Sat, 2020-09-26 at 12:57 +0530, Anup Patel wrote:
> > > > The NoMMU kernel is broken for QEMU virt machine from Linux-5.9-rc6
> > > > because the get_cycles() and friends are called very early from
> > > > rand_initialize() before CLINT driver is probed. To fix this, we
> > > > should check clint_time_val before use in get_cycles() and friends.
> > > >
> > > > Fixes: d5be89a8d118 ("RISC-V: Resurrect the MMIO timer implementation
> > > > for M-mode systems")
> > > > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> > > > ---
> > > > arch/riscv/include/asm/timex.h | 12 +++++++++---
> > > > 1 file changed, 9 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/arch/riscv/include/asm/timex.h
> > > > b/arch/riscv/include/asm/timex.h index 7f659dda0032..52b42bb1602c
> > > > 100644
> > > > --- a/arch/riscv/include/asm/timex.h
> > > > +++ b/arch/riscv/include/asm/timex.h
> > > > @@ -17,18 +17,24 @@ typedef unsigned long cycles_t; #ifdef
> > > > CONFIG_64BIT static inline cycles_t get_cycles(void) {
> > > > - return readq_relaxed(clint_time_val);
> > > > + if (clint_time_val)
> > > > + return readq_relaxed(clint_time_val);
> > > > + return 0;
> > > > }
> > > > #else /* !CONFIG_64BIT */
> > > > static inline u32 get_cycles(void)
> > > > {
> > > > - return readl_relaxed(((u32 *)clint_time_val));
> > > > + if (clint_time_val)
> > > > + return readl_relaxed(((u32 *)clint_time_val));
> > > > + return 0;
> > > > }
> > > > #define get_cycles get_cycles
> > > >
> > > > static inline u32 get_cycles_hi(void) {
> > > > - return readl_relaxed(((u32 *)clint_time_val) + 1);
> > > > + if (clint_time_val)
> > > > + return readl_relaxed(((u32 *)clint_time_val) + 1);
> > > > + return 0
> > > > }
> > > > #define get_cycles_hi get_cycles_hi
> > > > #endif /* CONFIG_64BIT */
> > >
> > > Applying this on top of rc6, I now get a hang on Kendryte boot...
> > > No problems without the patch on the other hand.
> >
> > Not sure about the issue with Kendryte but I get a crash on
> > QEMU virt machine without this patch.
>
> With this applied in addition to your patch, it works.
>
> diff --git a/drivers/clocksource/timer-clint.c b/drivers/clocksource/timer-
> clint.c
> index d17367dee02c..8dbec85979fd 100644
> --- a/drivers/clocksource/timer-clint.c
> +++ b/drivers/clocksource/timer-clint.c
> @@ -37,7 +37,7 @@ static unsigned long clint_timer_freq;
> static unsigned int clint_timer_irq;
>
> #ifdef CONFIG_RISCV_M_MODE
> -u64 __iomem *clint_time_val;
> +u64 __iomem *clint_time_val = NULL;
> #endif
>
> static void clint_send_ipi(const struct cpumask *target)
Ahh, clint_time_val is an uninitialized variable.
I will send a v2 with your SoB.
Regards,
Anup
>
> --
> Damien Le Moal
> Western Digital
_______________________________________________
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] RISC-V: Check clint_time_val before use
2020-09-26 9:57 ` Anup Patel
@ 2020-09-26 10:01 ` Damien Le Moal
0 siblings, 0 replies; 9+ messages in thread
From: Damien Le Moal @ 2020-09-26 10:01 UTC (permalink / raw)
To: anup
Cc: aou, palmerdabbelt, linux-kernel, Atish Patra, Anup Patel,
Alistair Francis, paul.walmsley, palmer, linux-riscv
On Sat, 2020-09-26 at 15:27 +0530, Anup Patel wrote:
> On Sat, Sep 26, 2020 at 3:16 PM Damien Le Moal <Damien.LeMoal@wdc.com> wrote:
> > On Sat, 2020-09-26 at 09:31 +0000, Anup Patel wrote:
> > > > -----Original Message-----
> > > > From: Damien Le Moal <Damien.LeMoal@wdc.com>
> > > > Sent: 26 September 2020 14:55
> > > > To: paul.walmsley@sifive.com; palmer@dabbelt.com;
> > > > palmerdabbelt@google.com; Anup Patel <Anup.Patel@wdc.com>;
> > > > aou@eecs.berkeley.edu
> > > > Cc: anup@brainfault.org; linux-riscv@lists.infradead.org; Atish Patra
> > > > <Atish.Patra@wdc.com>; Alistair Francis <Alistair.Francis@wdc.com>; linux-
> > > > kernel@vger.kernel.org
> > > > Subject: Re: [PATCH] RISC-V: Check clint_time_val before use
> > > >
> > > > On Sat, 2020-09-26 at 12:57 +0530, Anup Patel wrote:
> > > > > The NoMMU kernel is broken for QEMU virt machine from Linux-5.9-rc6
> > > > > because the get_cycles() and friends are called very early from
> > > > > rand_initialize() before CLINT driver is probed. To fix this, we
> > > > > should check clint_time_val before use in get_cycles() and friends.
> > > > >
> > > > > Fixes: d5be89a8d118 ("RISC-V: Resurrect the MMIO timer implementation
> > > > > for M-mode systems")
> > > > > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> > > > > ---
> > > > > arch/riscv/include/asm/timex.h | 12 +++++++++---
> > > > > 1 file changed, 9 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/arch/riscv/include/asm/timex.h
> > > > > b/arch/riscv/include/asm/timex.h index 7f659dda0032..52b42bb1602c
> > > > > 100644
> > > > > --- a/arch/riscv/include/asm/timex.h
> > > > > +++ b/arch/riscv/include/asm/timex.h
> > > > > @@ -17,18 +17,24 @@ typedef unsigned long cycles_t; #ifdef
> > > > > CONFIG_64BIT static inline cycles_t get_cycles(void) {
> > > > > - return readq_relaxed(clint_time_val);
> > > > > + if (clint_time_val)
> > > > > + return readq_relaxed(clint_time_val);
> > > > > + return 0;
> > > > > }
> > > > > #else /* !CONFIG_64BIT */
> > > > > static inline u32 get_cycles(void)
> > > > > {
> > > > > - return readl_relaxed(((u32 *)clint_time_val));
> > > > > + if (clint_time_val)
> > > > > + return readl_relaxed(((u32 *)clint_time_val));
> > > > > + return 0;
> > > > > }
> > > > > #define get_cycles get_cycles
> > > > >
> > > > > static inline u32 get_cycles_hi(void) {
> > > > > - return readl_relaxed(((u32 *)clint_time_val) + 1);
> > > > > + if (clint_time_val)
> > > > > + return readl_relaxed(((u32 *)clint_time_val) + 1);
> > > > > + return 0
> > > > > }
> > > > > #define get_cycles_hi get_cycles_hi
> > > > > #endif /* CONFIG_64BIT */
> > > >
> > > > Applying this on top of rc6, I now get a hang on Kendryte boot...
> > > > No problems without the patch on the other hand.
> > >
> > > Not sure about the issue with Kendryte but I get a crash on
> > > QEMU virt machine without this patch.
> >
> > With this applied in addition to your patch, it works.
> >
> > diff --git a/drivers/clocksource/timer-clint.c b/drivers/clocksource/timer-
> > clint.c
> > index d17367dee02c..8dbec85979fd 100644
> > --- a/drivers/clocksource/timer-clint.c
> > +++ b/drivers/clocksource/timer-clint.c
> > @@ -37,7 +37,7 @@ static unsigned long clint_timer_freq;
> > static unsigned int clint_timer_irq;
> >
> > #ifdef CONFIG_RISCV_M_MODE
> > -u64 __iomem *clint_time_val;
> > +u64 __iomem *clint_time_val = NULL;
> > #endif
> >
> > static void clint_send_ipi(const struct cpumask *target)
>
> Ahh, clint_time_val is an uninitialized variable.
>
> I will send a v2 with your SoB.
No need for my sob. Just merge that in your patch.
>
> Regards,
> Anup
>
> > --
> > Damien Le Moal
> > Western Digital
--
Damien Le Moal
Western Digital
_______________________________________________
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] RISC-V: Check clint_time_val before use
2020-09-26 9:46 ` Damien Le Moal
2020-09-26 9:57 ` Anup Patel
@ 2020-09-26 10:09 ` Maciej W. Rozycki
2020-09-26 10:25 ` Damien Le Moal
1 sibling, 1 reply; 9+ messages in thread
From: Maciej W. Rozycki @ 2020-09-26 10:09 UTC (permalink / raw)
To: Damien Le Moal
Cc: palmerdabbelt, anup, Anup Patel, linux-kernel, Atish Patra, aou,
palmer, paul.walmsley, Alistair Francis, linux-riscv
On Sat, 26 Sep 2020, Damien Le Moal wrote:
> > > Applying this on top of rc6, I now get a hang on Kendryte boot...
> > > No problems without the patch on the other hand.
> >
> > Not sure about the issue with Kendryte but I get a crash on
> > QEMU virt machine without this patch.
>
> With this applied in addition to your patch, it works.
>
> diff --git a/drivers/clocksource/timer-clint.c b/drivers/clocksource/timer-
> clint.c
> index d17367dee02c..8dbec85979fd 100644
> --- a/drivers/clocksource/timer-clint.c
> +++ b/drivers/clocksource/timer-clint.c
> @@ -37,7 +37,7 @@ static unsigned long clint_timer_freq;
> static unsigned int clint_timer_irq;
>
> #ifdef CONFIG_RISCV_M_MODE
> -u64 __iomem *clint_time_val;
> +u64 __iomem *clint_time_val = NULL;
> #endif
Hmm, BSS initialisation issue?
Maciej
_______________________________________________
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] RISC-V: Check clint_time_val before use
2020-09-26 10:09 ` Maciej W. Rozycki
@ 2020-09-26 10:25 ` Damien Le Moal
2020-09-26 10:52 ` Maciej W. Rozycki
0 siblings, 1 reply; 9+ messages in thread
From: Damien Le Moal @ 2020-09-26 10:25 UTC (permalink / raw)
To: macro
Cc: palmerdabbelt, anup, Anup Patel, linux-kernel, Atish Patra, aou,
Alistair Francis, paul.walmsley, palmer, linux-riscv
On Sat, 2020-09-26 at 11:09 +0100, Maciej W. Rozycki wrote:
> On Sat, 26 Sep 2020, Damien Le Moal wrote:
>
> > > > Applying this on top of rc6, I now get a hang on Kendryte boot...
> > > > No problems without the patch on the other hand.
> > >
> > > Not sure about the issue with Kendryte but I get a crash on
> > > QEMU virt machine without this patch.
> >
> > With this applied in addition to your patch, it works.
> >
> > diff --git a/drivers/clocksource/timer-clint.c b/drivers/clocksource/timer-
> > clint.c
> > index d17367dee02c..8dbec85979fd 100644
> > --- a/drivers/clocksource/timer-clint.c
> > +++ b/drivers/clocksource/timer-clint.c
> > @@ -37,7 +37,7 @@ static unsigned long clint_timer_freq;
> > static unsigned int clint_timer_irq;
> >
> > #ifdef CONFIG_RISCV_M_MODE
> > -u64 __iomem *clint_time_val;
> > +u64 __iomem *clint_time_val = NULL;
> > #endif
>
> Hmm, BSS initialisation issue?
Not a static variable, so it is not in BSS, no ?
>
> Maciej
--
Damien Le Moal
Western Digital
_______________________________________________
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] RISC-V: Check clint_time_val before use
2020-09-26 10:25 ` Damien Le Moal
@ 2020-09-26 10:52 ` Maciej W. Rozycki
0 siblings, 0 replies; 9+ messages in thread
From: Maciej W. Rozycki @ 2020-09-26 10:52 UTC (permalink / raw)
To: Damien Le Moal
Cc: palmerdabbelt, anup, Anup Patel, linux-kernel, Atish Patra, aou,
Alistair Francis, paul.walmsley, palmer, linux-riscv
On Sat, 26 Sep 2020, Damien Le Moal wrote:
> > > With this applied in addition to your patch, it works.
> > >
> > > diff --git a/drivers/clocksource/timer-clint.c b/drivers/clocksource/timer-
> > > clint.c
> > > index d17367dee02c..8dbec85979fd 100644
> > > --- a/drivers/clocksource/timer-clint.c
> > > +++ b/drivers/clocksource/timer-clint.c
> > > @@ -37,7 +37,7 @@ static unsigned long clint_timer_freq;
> > > static unsigned int clint_timer_irq;
> > >
> > > #ifdef CONFIG_RISCV_M_MODE
> > > -u64 __iomem *clint_time_val;
> > > +u64 __iomem *clint_time_val = NULL;
> > > #endif
> >
> > Hmm, BSS initialisation issue?
>
> Not a static variable, so it is not in BSS, no ?
Maybe it has a weird declaration elsewhere which messes up things (I
haven't checked), but it looks to me like it does have static storage
(rather than automatic or thread one), so if uninitialised it goes to BSS,
and it is supposed to be all-zeros whether explicitly assigned a NULL
value or not. It does have external rather than internal linkage (as it
would if it had the `static' keyword), but it does not matter. Best check
with `objdump'/`readelf'.
Maciej
_______________________________________________
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, other threads:[~2020-09-26 10:52 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-26 7:27 [PATCH] RISC-V: Check clint_time_val before use Anup Patel
2020-09-26 9:25 ` Damien Le Moal
2020-09-26 9:31 ` Anup Patel
2020-09-26 9:46 ` Damien Le Moal
2020-09-26 9:57 ` Anup Patel
2020-09-26 10:01 ` Damien Le Moal
2020-09-26 10:09 ` Maciej W. Rozycki
2020-09-26 10:25 ` Damien Le Moal
2020-09-26 10:52 ` Maciej W. Rozycki
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).