2024-04-06 11:35:33

by Jisheng Zhang

[permalink] [raw]
Subject: [PATCH v2 0/3] riscv: improve nommu and timer-clint

As is known, the sophgo CV1800B contains so called little core, which
is C906 w/o MMU, so I want to run nommu linux on it. This series is
the result of the bring up. After this series, w/ proper dts, we can
run nommu linux on milkv duo's little core.

First of all, patch1 removes the PAGE_OFFSET hardcoding by introducing
DRAM_BASE Kconfig option.

As pointed out by commit ca7810aecdba ("lib: utils/timer: mtimer: add a
quirk for lacking mtime register") of opensbi:
"T-Head developers surely have a different understanding of time CSR and
CLINT's mtime register with SiFive ones, that they did not implement
the mtime register at all -- as shown in openC906 source code, their
time CSR value is just exposed at the top of their processor IP block
and expects an external continous counter, which makes it not
overrideable, and thus mtime register is not implemented, even not for
reading. However, if CLINTEE is not enabled in T-Head's MXSTATUS
extended CSR, these systems still rely on the mtimecmp registers to
generate timer interrupts. This makes it necessary to implement T-Head
C9xx CLINT support in OpenSBI MTIMER driver, which skips implementing
reading mtime register and falls back to default code that reads time
CSR."
So the second patch adds an option CONFIG_CLINT_USE_CSR_INSTEADOF_MTIME
for this purpose.

The last patch adds T-Head C9xxx clint support to timer-clint driver.

Since v1:
- fix c900_clint_timer_init_dt() defined but not used build warning
- add option CONFIG_CLINT_USE_CSR_INSTEADOF_MTIME instead of removing
mtime usage for all platforms, since not all platforms implement the
time CSR in HW in M mode.
- rebase on the timer-clint improvement series
https://lore.kernel.org/linux-riscv/[email protected]/T/#t

Jisheng Zhang (3):
riscv: nommu: remove PAGE_OFFSET hardcoding
clocksource/drivers/timer-clint: Add option to use CSR instead of
mtime
clocksource/drivers/timer-clint: Add T-Head C9xx clint

arch/riscv/Kconfig | 8 +++++++-
arch/riscv/include/asm/timex.h | 6 +++---
drivers/clocksource/Kconfig | 9 +++++++++
drivers/clocksource/timer-clint.c | 31 +++++++++++++++++++++++++++++++
4 files changed, 50 insertions(+), 4 deletions(-)

--
2.43.0



2024-04-06 11:35:40

by Jisheng Zhang

[permalink] [raw]
Subject: [PATCH v2 1/3] riscv: nommu: remove PAGE_OFFSET hardcoding

Currently, PAGE_OFFSET is hardcoded as 0x8000_0000, it works fine since
there's only one nommu platform in the mainline. However, there are
many cases where the (S)DRAM base address isn't 0x8000_0000, so remove
the hardcoding value, and introduce DRAM_BASE which will be set by
users during configuring. DRAM_BASE is 0x8000_0000 by default.

Signed-off-by: Jisheng Zhang <[email protected]>
---
arch/riscv/Kconfig | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 7895c77545f1..b4af1df86352 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -247,10 +247,16 @@ config MMU
Select if you want MMU-based virtualised addressing space
support by paged memory management. If unsure, say 'Y'.

+if !MMU
+config DRAM_BASE
+ hex '(S)DRAM Base Address'
+ default 0x80000000
+endif
+
config PAGE_OFFSET
hex
default 0xC0000000 if 32BIT && MMU
- default 0x80000000 if !MMU
+ default DRAM_BASE if !MMU
default 0xff60000000000000 if 64BIT

config KASAN_SHADOW_OFFSET
--
2.43.0


2024-04-06 11:35:52

by Jisheng Zhang

[permalink] [raw]
Subject: [PATCH v2 2/3] clocksource/drivers/timer-clint: Add option to use CSR instead of mtime

As pointed out by commit ca7810aecdba ("lib: utils/timer: mtimer: add a
quirk for lacking mtime register") of opensbi:

"T-Head developers surely have a different understanding of time CSR and
CLINT's mtime register with SiFive ones, that they did not implement
the mtime register at all -- as shown in openC906 source code, their
time CSR value is just exposed at the top of their processor IP block
and expects an external continous counter, which makes it not
overrideable, and thus mtime register is not implemented, even not for
reading. However, if CLINTEE is not enabled in T-Head's MXSTATUS
extended CSR, these systems still rely on the mtimecmp registers to
generate timer interrupts. This makes it necessary to implement T-Head
C9xx CLINT support in OpenSBI MTIMER driver, which skips implementing
reading mtime register and falls back to default code that reads time
CSR."

To use the clint in RISCV-M NOMMU env on Milkv Duo little core, we
need to fall back to read time CSR instead of mtime register. Add the
option for this purpose.

Signed-off-by: Jisheng Zhang <[email protected]>
---
arch/riscv/include/asm/timex.h | 6 +++---
drivers/clocksource/Kconfig | 9 +++++++++
drivers/clocksource/timer-clint.c | 7 +++++++
3 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/include/asm/timex.h b/arch/riscv/include/asm/timex.h
index a06697846e69..1c3eed4263cd 100644
--- a/arch/riscv/include/asm/timex.h
+++ b/arch/riscv/include/asm/timex.h
@@ -10,7 +10,7 @@

typedef unsigned long cycles_t;

-#ifdef CONFIG_RISCV_M_MODE
+#if defined(CONFIG_RISCV_M_MODE) && !defined(CONFIG_CLINT_USE_CSR_INSTEADOF_MTIME)

#include <asm/clint.h>

@@ -46,7 +46,7 @@ static inline unsigned long random_get_entropy(void)
}
#define random_get_entropy() random_get_entropy()

-#else /* CONFIG_RISCV_M_MODE */
+#else /* CONFIG_RISCV_M_MODE && !CONFIG_CLINT_USE_CSR_INSTEADOF_MTIME */

static inline cycles_t get_cycles(void)
{
@@ -60,7 +60,7 @@ static inline u32 get_cycles_hi(void)
}
#define get_cycles_hi get_cycles_hi

-#endif /* !CONFIG_RISCV_M_MODE */
+#endif /* !CONFIG_RISCV_M_MODE || CONFIG_CLINT_USE_CSR_INSTEADOF_MTIME */

#ifdef CONFIG_64BIT
static inline u64 get_cycles64(void)
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 34faa0320ece..7bbdbf2f96a8 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -650,6 +650,15 @@ config CLINT_TIMER
This option enables the CLINT timer for RISC-V systems. The CLINT
driver is usually used for NoMMU RISC-V systems.

+config CLINT_USE_CSR_INSTEADOF_MTIME
+ bool "Use TIME CSR instead of the mtime register"
+ depends on CLINT_TIMER
+ help
+ Use TIME CSR instead of mtime register. Enable this option if
+ prefer TIME CSR over MTIME register, or if the implementation
+ doesn't implement the mtime register in CLINT, so fall back on
+ TIME CSR.
+
config CSKY_MP_TIMER
bool "SMP Timer for the C-SKY platform" if COMPILE_TEST
depends on CSKY
diff --git a/drivers/clocksource/timer-clint.c b/drivers/clocksource/timer-clint.c
index f468fa8bf5f0..0d3890e00b75 100644
--- a/drivers/clocksource/timer-clint.c
+++ b/drivers/clocksource/timer-clint.c
@@ -76,6 +76,12 @@ static void clint_ipi_interrupt(struct irq_desc *desc)
#define clint_get_cycles_hi() readl_relaxed(((u32 *)clint_timer_val) + 1)
#endif

+#ifdef CONFIG_CLINT_USE_CSR_INSTEADOF_MTIME
+static u64 notrace clint_get_cycles64(void)
+{
+ return get_cycles64();
+}
+#else
#ifdef CONFIG_64BIT
static u64 notrace clint_get_cycles64(void)
{
@@ -94,6 +100,7 @@ static u64 notrace clint_get_cycles64(void)
return ((u64)hi << 32) | lo;
}
#endif /* CONFIG_64BIT */
+#endif /* CONFIG_CLINT_USE_CSR_INSTEADOF_MTIME */

static u64 clint_rdtime(struct clocksource *cs)
{
--
2.43.0


2024-04-06 11:36:04

by Jisheng Zhang

[permalink] [raw]
Subject: [PATCH v2 3/3] clocksource/drivers/timer-clint: Add T-Head C9xx clint

The mtimecmp in T-Head C9xx clint only supports 32bit read/write,
implement such support.

Signed-off-by: Jisheng Zhang <[email protected]>
---
drivers/clocksource/timer-clint.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)

diff --git a/drivers/clocksource/timer-clint.c b/drivers/clocksource/timer-clint.c
index 0d3890e00b75..655ea81071ff 100644
--- a/drivers/clocksource/timer-clint.c
+++ b/drivers/clocksource/timer-clint.c
@@ -39,6 +39,7 @@ static u64 __iomem *clint_timer_cmp;
static u64 __iomem *clint_timer_val;
static unsigned long clint_timer_freq;
static unsigned int clint_timer_irq;
+static bool is_c900_clint;

#ifdef CONFIG_RISCV_M_MODE
u64 __iomem *clint_time_val;
@@ -135,6 +136,19 @@ static int clint_clock_shutdown(struct clock_event_device *evt)
return 0;
}

+static int c900_clint_clock_next_event(unsigned long delta,
+ struct clock_event_device *ce)
+{
+ void __iomem *r = clint_timer_cmp +
+ cpuid_to_hartid_map(smp_processor_id());
+ u64 val = clint_get_cycles64() + delta;
+
+ csr_set(CSR_IE, IE_TIE);
+ writel_relaxed(val, r);
+ writel_relaxed(val >> 32, r + 4);
+ return 0;
+}
+
static DEFINE_PER_CPU(struct clock_event_device, clint_clock_event) = {
.name = "clint_clockevent",
.features = CLOCK_EVT_FEAT_ONESHOT,
@@ -148,6 +162,9 @@ static int clint_timer_starting_cpu(unsigned int cpu)
{
struct clock_event_device *ce = per_cpu_ptr(&clint_clock_event, cpu);

+ if (is_c900_clint)
+ ce->set_next_event = c900_clint_clock_next_event;
+
ce->cpumask = cpumask_of(cpu);
clockevents_config_and_register(ce, clint_timer_freq, 100, ULONG_MAX);

@@ -291,5 +308,12 @@ static int __init clint_timer_init_dt(struct device_node *np)
return rc;
}

+static int __init c900_clint_timer_init_dt(struct device_node *np)
+{
+ is_c900_clint = true;
+ return clint_timer_init_dt(np);
+}
+
TIMER_OF_DECLARE(clint_timer, "riscv,clint0", clint_timer_init_dt);
TIMER_OF_DECLARE(clint_timer1, "sifive,clint0", clint_timer_init_dt);
+TIMER_OF_DECLARE(clint_timer2, "thead,c900-clint", c900_clint_timer_init_dt);
--
2.43.0


2024-04-09 14:49:14

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] clocksource/drivers/timer-clint: Add option to use CSR instead of mtime

On Sat, Apr 06, 2024 at 07:21:58PM +0800, Jisheng Zhang wrote:
> As pointed out by commit ca7810aecdba ("lib: utils/timer: mtimer: add a
> quirk for lacking mtime register") of opensbi:
>
> "T-Head developers surely have a different understanding of time CSR and
> CLINT's mtime register with SiFive ones, that they did not implement
> the mtime register at all -- as shown in openC906 source code, their
> time CSR value is just exposed at the top of their processor IP block
> and expects an external continous counter, which makes it not
> overrideable, and thus mtime register is not implemented, even not for
> reading. However, if CLINTEE is not enabled in T-Head's MXSTATUS
> extended CSR, these systems still rely on the mtimecmp registers to
> generate timer interrupts. This makes it necessary to implement T-Head
> C9xx CLINT support in OpenSBI MTIMER driver, which skips implementing
> reading mtime register and falls back to default code that reads time
> CSR."
>
> To use the clint in RISCV-M NOMMU env on Milkv Duo little core, we
> need to fall back to read time CSR instead of mtime register. Add the
> option for this purpose.
>
> Signed-off-by: Jisheng Zhang <[email protected]>

> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 34faa0320ece..7bbdbf2f96a8 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -650,6 +650,15 @@ config CLINT_TIMER
> This option enables the CLINT timer for RISC-V systems. The CLINT
> driver is usually used for NoMMU RISC-V systems.
>
> +config CLINT_USE_CSR_INSTEADOF_MTIME
> + bool "Use TIME CSR instead of the mtime register"
> + depends on CLINT_TIMER
> + help
> + Use TIME CSR instead of mtime register. Enable this option if
> + prefer TIME CSR over MTIME register, or if the implementation
> + doesn't implement the mtime register in CLINT, so fall back on
> + TIME CSR.

This, as a Kconfig option, seems a bit strange to me. We know at runtime
if we are on a T-Head device without the mtime register and should be
able decide to use the CSR implementation dynamically in that case,
right?


Attachments:
(No filename) (2.13 kB)
signature.asc (235.00 B)
Download all attachments

2024-04-09 15:50:16

by Jisheng Zhang

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] clocksource/drivers/timer-clint: Add option to use CSR instead of mtime

On Tue, Apr 09, 2024 at 03:26:18PM +0100, Conor Dooley wrote:
> On Sat, Apr 06, 2024 at 07:21:58PM +0800, Jisheng Zhang wrote:
> > As pointed out by commit ca7810aecdba ("lib: utils/timer: mtimer: add a
> > quirk for lacking mtime register") of opensbi:
> >
> > "T-Head developers surely have a different understanding of time CSR and
> > CLINT's mtime register with SiFive ones, that they did not implement
> > the mtime register at all -- as shown in openC906 source code, their
> > time CSR value is just exposed at the top of their processor IP block
> > and expects an external continous counter, which makes it not
> > overrideable, and thus mtime register is not implemented, even not for
> > reading. However, if CLINTEE is not enabled in T-Head's MXSTATUS
> > extended CSR, these systems still rely on the mtimecmp registers to
> > generate timer interrupts. This makes it necessary to implement T-Head
> > C9xx CLINT support in OpenSBI MTIMER driver, which skips implementing
> > reading mtime register and falls back to default code that reads time
> > CSR."
> >
> > To use the clint in RISCV-M NOMMU env on Milkv Duo little core, we
> > need to fall back to read time CSR instead of mtime register. Add the
> > option for this purpose.
> >
> > Signed-off-by: Jisheng Zhang <[email protected]>
>
> > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> > index 34faa0320ece..7bbdbf2f96a8 100644
> > --- a/drivers/clocksource/Kconfig
> > +++ b/drivers/clocksource/Kconfig
> > @@ -650,6 +650,15 @@ config CLINT_TIMER
> > This option enables the CLINT timer for RISC-V systems. The CLINT
> > driver is usually used for NoMMU RISC-V systems.
> >
> > +config CLINT_USE_CSR_INSTEADOF_MTIME
> > + bool "Use TIME CSR instead of the mtime register"
> > + depends on CLINT_TIMER
> > + help
> > + Use TIME CSR instead of mtime register. Enable this option if
> > + prefer TIME CSR over MTIME register, or if the implementation
> > + doesn't implement the mtime register in CLINT, so fall back on
> > + TIME CSR.
>
> This, as a Kconfig option, seems a bit strange to me. We know at runtime
> if we are on a T-Head device without the mtime register and should be
> able decide to use the CSR implementation dynamically in that case,
> right?

Dynamically decision can be done in clocksource/clockevnt:
I can patch clint_clocksource.read to point to different clint_rdtime()
implementation.

But clint timer is also used in NOMMU RISCV-M's get_cycles(), this
can't be dynamically chosen w/o an ugly "if (is_c900)"
check, and I'm not sure whether this check in get_cycles() will
introduce non-trival overhead or not. Or use code patching technology
here?

Or introduce a function pointer such as unsigned long (*rdtime)(void)
for RISCV_M_MODE, then point it to different implementation?

Any suggestion is welcome.
Thanks

2024-04-10 10:43:45

by Jisheng Zhang

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] clocksource/drivers/timer-clint: Add option to use CSR instead of mtime

On Tue, Apr 09, 2024 at 10:48:33PM +0800, Jisheng Zhang wrote:
> On Tue, Apr 09, 2024 at 03:26:18PM +0100, Conor Dooley wrote:
> > On Sat, Apr 06, 2024 at 07:21:58PM +0800, Jisheng Zhang wrote:
> > > As pointed out by commit ca7810aecdba ("lib: utils/timer: mtimer: add a
> > > quirk for lacking mtime register") of opensbi:
> > >
> > > "T-Head developers surely have a different understanding of time CSR and
> > > CLINT's mtime register with SiFive ones, that they did not implement
> > > the mtime register at all -- as shown in openC906 source code, their
> > > time CSR value is just exposed at the top of their processor IP block
> > > and expects an external continous counter, which makes it not
> > > overrideable, and thus mtime register is not implemented, even not for
> > > reading. However, if CLINTEE is not enabled in T-Head's MXSTATUS
> > > extended CSR, these systems still rely on the mtimecmp registers to
> > > generate timer interrupts. This makes it necessary to implement T-Head
> > > C9xx CLINT support in OpenSBI MTIMER driver, which skips implementing
> > > reading mtime register and falls back to default code that reads time
> > > CSR."
> > >
> > > To use the clint in RISCV-M NOMMU env on Milkv Duo little core, we
> > > need to fall back to read time CSR instead of mtime register. Add the
> > > option for this purpose.
> > >
> > > Signed-off-by: Jisheng Zhang <[email protected]>
> >
> > > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> > > index 34faa0320ece..7bbdbf2f96a8 100644
> > > --- a/drivers/clocksource/Kconfig
> > > +++ b/drivers/clocksource/Kconfig
> > > @@ -650,6 +650,15 @@ config CLINT_TIMER
> > > This option enables the CLINT timer for RISC-V systems. The CLINT
> > > driver is usually used for NoMMU RISC-V systems.
> > >
> > > +config CLINT_USE_CSR_INSTEADOF_MTIME
> > > + bool "Use TIME CSR instead of the mtime register"
> > > + depends on CLINT_TIMER
> > > + help
> > > + Use TIME CSR instead of mtime register. Enable this option if
> > > + prefer TIME CSR over MTIME register, or if the implementation
> > > + doesn't implement the mtime register in CLINT, so fall back on
> > > + TIME CSR.
> >
> > This, as a Kconfig option, seems a bit strange to me. We know at runtime
> > if we are on a T-Head device without the mtime register and should be
> > able decide to use the CSR implementation dynamically in that case,
> > right?
>
> Dynamically decision can be done in clocksource/clockevnt:
> I can patch clint_clocksource.read to point to different clint_rdtime()
> implementation.
>
> But clint timer is also used in NOMMU RISCV-M's get_cycles(), this
> can't be dynamically chosen w/o an ugly "if (is_c900)"
> check, and I'm not sure whether this check in get_cycles() will
> introduce non-trival overhead or not. Or use code patching technology
> here?

Hi,

After some tests, I think will go with code patching path, I.E use
static_branch in get_cycles(). New version is under cooking.

Thanks
>
> Or introduce a function pointer such as unsigned long (*rdtime)(void)
> for RISCV_M_MODE, then point it to different implementation?
>
> Any suggestion is welcome.
> Thanks

2024-04-24 11:02:09

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] clocksource/drivers/timer-clint: Add option to use CSR instead of mtime

On Wed, Apr 10, 2024 at 06:30:13PM +0800, Jisheng Zhang wrote:
> On Tue, Apr 09, 2024 at 10:48:33PM +0800, Jisheng Zhang wrote:
> > On Tue, Apr 09, 2024 at 03:26:18PM +0100, Conor Dooley wrote:
> > > On Sat, Apr 06, 2024 at 07:21:58PM +0800, Jisheng Zhang wrote:
> > > > As pointed out by commit ca7810aecdba ("lib: utils/timer: mtimer: add a
> > > > quirk for lacking mtime register") of opensbi:
> > > >
> > > > "T-Head developers surely have a different understanding of time CSR and
> > > > CLINT's mtime register with SiFive ones, that they did not implement
> > > > the mtime register at all -- as shown in openC906 source code, their
> > > > time CSR value is just exposed at the top of their processor IP block
> > > > and expects an external continous counter, which makes it not
> > > > overrideable, and thus mtime register is not implemented, even not for
> > > > reading. However, if CLINTEE is not enabled in T-Head's MXSTATUS
> > > > extended CSR, these systems still rely on the mtimecmp registers to
> > > > generate timer interrupts. This makes it necessary to implement T-Head
> > > > C9xx CLINT support in OpenSBI MTIMER driver, which skips implementing
> > > > reading mtime register and falls back to default code that reads time
> > > > CSR."
> > > >
> > > > To use the clint in RISCV-M NOMMU env on Milkv Duo little core, we
> > > > need to fall back to read time CSR instead of mtime register. Add the
> > > > option for this purpose.
> > > >
> > > > Signed-off-by: Jisheng Zhang <[email protected]>
> > >
> > > > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> > > > index 34faa0320ece..7bbdbf2f96a8 100644
> > > > --- a/drivers/clocksource/Kconfig
> > > > +++ b/drivers/clocksource/Kconfig
> > > > @@ -650,6 +650,15 @@ config CLINT_TIMER
> > > > This option enables the CLINT timer for RISC-V systems. The CLINT
> > > > driver is usually used for NoMMU RISC-V systems.
> > > >
> > > > +config CLINT_USE_CSR_INSTEADOF_MTIME
> > > > + bool "Use TIME CSR instead of the mtime register"
> > > > + depends on CLINT_TIMER
> > > > + help
> > > > + Use TIME CSR instead of mtime register. Enable this option if
> > > > + prefer TIME CSR over MTIME register, or if the implementation
> > > > + doesn't implement the mtime register in CLINT, so fall back on
> > > > + TIME CSR.
> > >
> > > This, as a Kconfig option, seems a bit strange to me. We know at runtime
> > > if we are on a T-Head device without the mtime register and should be
> > > able decide to use the CSR implementation dynamically in that case,
> > > right?
> >
> > Dynamically decision can be done in clocksource/clockevnt:
> > I can patch clint_clocksource.read to point to different clint_rdtime()
> > implementation.
> >
> > But clint timer is also used in NOMMU RISCV-M's get_cycles(), this
> > can't be dynamically chosen w/o an ugly "if (is_c900)"
> > check, and I'm not sure whether this check in get_cycles() will
> > introduce non-trival overhead or not. Or use code patching technology
> > here?
>
> Hi,
>
> After some tests, I think will go with code patching path, I.E use
> static_branch in get_cycles(). New version is under cooking.

Whoops, sorry for the delay getting back to you - been busier than I
would like of late. Patching sounds good to me if your testing went
well.


Attachments:
(No filename) (3.32 kB)
signature.asc (235.00 B)
Download all attachments