[PATCH v2 01/11] clocksource: timer-riscv: select CSR from device tree
Antony Pavlov
antonynpavlov at gmail.com
Fri May 7 10:41:36 PDT 2021
On Fri, 7 May 2021 13:34:30 +0200
Ahmad Fatoum <a.fatoum at pengutronix.de> wrote:
> Hello Antony,
>
> On 07.05.21 00:08, Antony Pavlov wrote:
> > barebox timer-riscv driver supports one of user counters:
> >
> > * 'cycle', counter for RDCYCLE instruction (CSR 0xc00);
> > * 'time', timer for RDTIME instruction (CSR 0xc01).
> >
> > At the moment in S-mode timer-riscv uses the 'cycle' counter,
> > and in M-mode timer-riscv uses the 'time' timer.
>
> Other way round, right? cycle for M-Mode, time for S-Mode?
Yes, you are right, my bad.
> >
> > Alas picorv32 CPU core supports only the 'cycle' counter.
> > VexRiscV CPU core supports only the 'time' timer.
> >
> > This patch makes it possible to use the 'time' timer
> > for VexRiscV CPU in M-mode.
>
> Is that allowed by the ISA? To provide time, but not cycle?
> Can VexRiscV boot Linux? If so, how does Linux handle lack
> of this CSR?
Here is an unobvious answer for all these questions at once.
The RISC-V Instruction Set Manual, Volume II: Privileged Architecture
Document Version 20190608-Priv-MSU-Ratified states that
<<
Attempts to access a non-existent CSR raise an illegal instruction exception
>>
So you can even realize in hardware very few CSR for exception handling (mstatus, mcause, mtvec, mie, mepc)
and try to emulate all other CSR in software !
As a result exception handler for VexRiscv emulates access to 'cycle' CSR (RDCYCLE pseudo-op)
by reading the 'time' CSR !
Please see https://github.com/SpinalHDL/VexRiscv/blob/master/src/main/c/emulator/src/main.c#L239
opensbi do near the same ! Please see https://github.com/riscv/opensbi/blob/master/lib/sbi/sbi_emulate_csr.c#L64
Can VexRiscV boot Linux? Yes, it can boot linux!
Please see this demo: https://asciinema.org/a/7fLu84ytgtVd3rjPm7Li3GD4r
In this demo barebox loads emulator.bin into the RAM.
This emulator.bin realizes SBI for linux kernel and emulates all missed features
necessary for running linux.
>
> > Signed-off-by: Antony Pavlov <antonynpavlov at gmail.com>
> > ---
> > arch/riscv/cpu/time.c | 7 +++++++
> > arch/riscv/dts/erizo.dtsi | 2 ++
> > arch/riscv/include/asm/timer.h | 1 +
> > drivers/clocksource/timer-riscv.c | 19 ++++++++-----------
> > 4 files changed, 18 insertions(+), 11 deletions(-)
> >
> > diff --git a/arch/riscv/cpu/time.c b/arch/riscv/cpu/time.c
> > index 39bb6a5112..59c8ca61d6 100644
> > --- a/arch/riscv/cpu/time.c
> > +++ b/arch/riscv/cpu/time.c
> > @@ -18,6 +18,7 @@
> > #include <asm/timer.h>
> >
> > unsigned long riscv_timebase;
> > +unsigned long riscv_use_csr_cycle;
> >
> > int timer_init(void)
> > {
> > @@ -32,6 +33,12 @@ int timer_init(void)
> >
> > riscv_timebase = prop;
> >
> > + if (of_property_read_bool(cpu, "csr-cycle")) {
> > + riscv_use_csr_cycle = 1;
> > + } else {
> > + riscv_use_csr_cycle = 0;
> > + }
> > +
>
> Any reason this couldn't happen in driver probe?
Because we already have riscv_timebase parse routine outside of driver probe.
I tryed to avoid code duplication.
Why we couldn't parse riscv_timebase in driver probe?
> I'd also prefer another name, e.g. barebox,use-csr-cycle ?
Yes, barebox,use-csr-cycle is a better name.
> > of_platform_populate(cpu, NULL, NULL);
> >
> > return 0;
> > diff --git a/arch/riscv/dts/erizo.dtsi b/arch/riscv/dts/erizo.dtsi
> > index 228711bd69..b3ccf281f2 100644
> > --- a/arch/riscv/dts/erizo.dtsi
> > +++ b/arch/riscv/dts/erizo.dtsi
> > @@ -22,6 +22,8 @@
> >
> > timebase-frequency = <24000000>;
> >
> > + csr-cycle;
> > +
> > cpu at 0 {
> > device_type = "cpu";
> > compatible = "cliffordwolf,picorv32", "riscv";
> > diff --git a/arch/riscv/include/asm/timer.h b/arch/riscv/include/asm/timer.h
> > index 1f78ef4c00..555b3f5989 100644
> > --- a/arch/riscv/include/asm/timer.h
> > +++ b/arch/riscv/include/asm/timer.h
> > @@ -5,5 +5,6 @@
> >
> > int timer_init(void);
> > extern unsigned long riscv_timebase;
> > +extern unsigned long riscv_use_csr_cycle;
> >
> > #endif /* _ASM_RISCV_DELAY_H */
> > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
> > index ef67cff475..c0deed40eb 100644
> > --- a/drivers/clocksource/timer-riscv.c
> > +++ b/drivers/clocksource/timer-riscv.c
> > @@ -13,7 +13,7 @@
> > #include <asm/timer.h>
> > #include <asm/csr.h>
> >
> > -static u64 notrace riscv_timer_get_count_sbi(void)
> > +static u64 notrace riscv_timer_get_count_time(void)
> > {
> > __maybe_unused u32 hi, lo;
> >
> > @@ -28,7 +28,7 @@ static u64 notrace riscv_timer_get_count_sbi(void)
> > return ((u64)hi << 32) | lo;
> > }
> >
> > -static u64 notrace riscv_timer_get_count_rdcycle(void)
> > +static u64 notrace riscv_timer_get_count_cycle(void)
> > {
> > __maybe_unused u32 hi, lo;
> >
> > @@ -43,16 +43,7 @@ static u64 notrace riscv_timer_get_count_rdcycle(void)
> > return ((u64)hi << 32) | lo;
> > }
> >
> > -static u64 notrace riscv_timer_get_count(void)
> > -{
> > - if (IS_ENABLED(CONFIG_RISCV_SBI))
> > - return riscv_timer_get_count_sbi();
> > - else
> > - return riscv_timer_get_count_rdcycle();
> > -}
> > -
> > static struct clocksource riscv_clocksource = {
> > - .read = riscv_timer_get_count,
> > .mask = CLOCKSOURCE_MASK(64),
> > .priority = 100,
> > };
> > @@ -61,6 +52,12 @@ static int riscv_timer_init(struct device_d* dev)
> > {
> > dev_info(dev, "running at %lu Hz\n", riscv_timebase);
> >
> > + if (riscv_use_csr_cycle) {
> > + riscv_clocksource.read = riscv_timer_get_count_cycle;
> > + } else {
> > + riscv_clocksource.read = riscv_timer_get_count_time;
> > + }
> > +
> > riscv_clocksource.mult = clocksource_hz2mult(riscv_timebase, riscv_clocksource.shift);
> >
> > return init_clock(&riscv_clocksource);
> >
>
> --
> Pengutronix e.K. | |
> Steuerwalder Str. 21 | http://www.pengutronix.de/ |
> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
--
Best regards,
Antony Pavlov
More information about the barebox
mailing list