On Fri, 7 May 2021 13:34:30 +0200 Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx> 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@xxxxxxxxx> > > --- > > 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@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 _______________________________________________ barebox mailing list barebox@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/barebox
