[PATCH v2 01/11] clocksource: timer-riscv: select CSR from device tree
Ahmad Fatoum
a.fatoum at pengutronix.de
Fri May 7 11:05:17 PDT 2021
Hello Antony,
On 07.05.21 19:41, Antony Pavlov wrote:
> On Fri, 7 May 2021 13:34:30 +0200
> Ahmad Fatoum <a.fatoum at pengutronix.de> wrote:
>>> 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.
Thanks for clearing this up :)
>>> 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?
Because riscv_timebase is also used for CLINT clocksource, but this one
here would be exclusive to riscv timer, so it should be read there.
>> 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 |
>
>
--
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 |
More information about the barebox
mailing list