[U-Boot] [PATCH 00/19] riscv: Adding RISC-V CPU and timer driver

Discussion:

[U-Boot] [PATCH 00/19] riscv: Adding RISC-V CPU and timer driver

Bin Meng

2018-11-13 08:21:48 UTC

This adds DM drivers to support RISC-V CPU and timer.

The U-Boot RISC-V SBI support is still working in progress.
Some patches in this series like adding CSR numbers, exception
numbers, are prerequisites for the SBI implementation, but it
does no harm to include them as part of this series.

This series is dependent on Lukas's riscv series @
http://patchwork.ozlabs.org/project/uboot/list/?series=74999

This series is available at u-boot-x86/riscv-working for testing.

Bin Meng (18):
dm: cpu: Add timebase frequency to the platdata
riscv: qemu: Create a simple-bus driver for the soc node
cpu: Add a RISC-V CPU driver
riscv: Add a SYSCON driver for Core Local Interruptor
timer: Add driver for RISC-V privileged architecture defined timer
riscv: kconfig: Allow platform to specify Kconfig options
riscv: Enlarge the default SYS_MALLOC_F_LEN
riscv: qemu: Probe cpus during boot
riscv: Add CSR numbers
riscv: Add exception codes for xcause register
riscv: Do some basic architecture level cpu initialization
riscv: Move trap handler codes to mtrap.S
riscv: Fix context restore before returning from trap handler
riscv: Return to previous privilege level after trap handling
riscv: Adjust the _exit_trap() position to come before handle_trap()
riscv: Pass correct exception code to _exit_trap()
riscv: Refactor handle_trap() a little for future extension
riscv: Allow U-Boot to run on hart 0 only

Lukas Auer (1):
riscv: add Kconfig entries for the code model

arch/riscv/Kconfig | 35 ++++++
arch/riscv/Makefile | 9 +-
arch/riscv/cpu/Makefile | 2 +-
arch/riscv/cpu/cpu.c | 21 ++++
arch/riscv/cpu/mtrap.S | 103 ++++++++++++++++
arch/riscv/cpu/qemu/Kconfig | 10 ++
arch/riscv/cpu/qemu/cpu.c | 27 +++++
arch/riscv/cpu/start.S | 86 +-------------
arch/riscv/include/asm/clint.h | 24 ++++
arch/riscv/include/asm/encoding.h | 234 +++++++++++++++++++++++++++++++++++++
arch/riscv/lib/Makefile | 1 +
arch/riscv/lib/clint.c | 69 +++++++++++
arch/riscv/lib/interrupts.c | 78 +++++++------
board/emulation/qemu-riscv/Kconfig | 1 +
drivers/cpu/Kconfig | 6 +
drivers/cpu/Makefile | 1 +
drivers/cpu/riscv_cpu.c | 117 +++++++++++++++++++
drivers/timer/Kconfig | 8 ++
drivers/timer/Makefile | 1 +
drivers/timer/riscv_timer.c | 50 ++++++++
include/cpu.h | 3 +
21 files changed, 764 insertions(+), 122 deletions(-)
create mode 100644 arch/riscv/cpu/mtrap.S
create mode 100644 arch/riscv/cpu/qemu/Kconfig
create mode 100644 arch/riscv/include/asm/clint.h
create mode 100644 arch/riscv/lib/clint.c
create mode 100644 drivers/cpu/riscv_cpu.c
create mode 100644 drivers/timer/riscv_timer.c

--
2.7.4

Bin Meng

2018-11-13 08:21:50 UTC

This adds a timebase_freq member to the 'struct cpu_platdata', to
hold the "timebase-frequency" value in the cpu or /cpus node.

Signed-off-by: Bin Meng <***@gmail.com>
---

include/cpu.h | 3 +++
1 file changed, 3 insertions(+)

diff --git a/include/cpu.h b/include/cpu.h
index 367c5f4..176a276 100644
--- a/include/cpu.h
+++ b/include/cpu.h
@@ -14,6 +14,8 @@
* @device_id: Driver-defined device identifier
* @family: DMTF CPU Family identifier
* @id: DMTF CPU Processor identifier
+ * @timebase_freq: the current frequency at which the cpu timer timebase
+ * registers are updated (in HZ)
*
* This can be accessed with dev_get_parent_platdata() for any UCLASS_CPU
* device.
@@ -24,6 +26,7 @@ struct cpu_platdata {
ulong device_id;
u16 family;
u32 id[2];
+ u32 timebase_freq;
};

/* CPU features - mostly just a placeholder for now */

--
2.7.4

Simon Glass

2018-11-13 20:01:59 UTC

Post by Bin Meng
This adds a timebase_freq member to the 'struct cpu_platdata', to
hold the "timebase-frequency" value in the cpu or /cpus node.
---
include/cpu.h | 3 +++
1 file changed, 3 insertions(+)

Reviewed-by: Simon Glass <***@chromium.org>

Could we have comments for the struct members?

Bin Meng

2018-11-14 01:14:09 UTC

Hi Simon,

Post by Simon Glass

Post by Bin Meng
This adds a timebase_freq member to the 'struct cpu_platdata', to
hold the "timebase-frequency" value in the cpu or /cpus node.
---
include/cpu.h | 3 +++
1 file changed, 3 insertions(+)

Could we have comments for the struct members?

There are already comments added for the struct members. I am not sure
what additional comments do you want to add?

Regards,
Bin

Simon Glass

2018-11-15 19:21:13 UTC

Hi Bin,

Post by Bin Meng
Hi Simon,

Post by Simon Glass

Post by Bin Meng
This adds a timebase_freq member to the 'struct cpu_platdata', to
hold the "timebase-frequency" value in the cpu or /cpus node.
---
include/cpu.h | 3 +++
1 file changed, 3 insertions(+)

Could we have comments for the struct members?

There are already comments added for the struct members. I am not sure
what additional comments do you want to add?

Yes. I'm not sure what I was thinking there.

Regards,
Simon

Auer, Lukas

2018-11-14 21:17:40 UTC

Hi Bin,

Post by Bin Meng
This adds a timebase_freq member to the 'struct cpu_platdata', to
hold the "timebase-frequency" value in the cpu or /cpus node.
---
include/cpu.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/cpu.h b/include/cpu.h
index 367c5f4..176a276 100644
--- a/include/cpu.h
+++ b/include/cpu.h
@@ -14,6 +14,8 @@
+ * registers are updated (in HZ)

nit: Hz

Thanks,
Lukas

Post by Bin Meng
*
* This can be accessed with dev_get_parent_platdata() for any UCLASS_CPU
* device.
@@ -24,6 +26,7 @@ struct cpu_platdata {
ulong device_id;
u16 family;
u32 id[2];
+ u32 timebase_freq;
};
/* CPU features - mostly just a placeholder for now */

Bin Meng

2018-11-13 08:21:49 UTC

From: Lukas Auer <***@aisec.fraunhofer.de>

RISC-V has two code models, medium low (medlow) and medium any (medany).
Medlow limits addressable memory to a single 2 GiB range between the
absolute addresses -2 GiB and +2 GiB. Medany limits addressable memory
to any single 2 GiB address range.

By default, medlow is selected for U-Boot on both 32-bit and 64-bit
systems.

The -mcmodel compiler flag is selected according to the Kconfig
configuration.

Signed-off-by: Lukas Auer <***@aisec.fraunhofer.de>
[bmeng: adjust to make medlow the default code model for U-Boot]
Signed-off-by: Bin Meng <***@gmail.com>

---

arch/riscv/Kconfig | 18 ++++++++++++++++++
arch/riscv/Makefile | 9 ++++++++-
2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 0de77a7..a37e895 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -38,6 +38,24 @@ config ARCH_RV64I

endchoice

+choice
+ prompt "Code Model"
+ default CMODEL_MEDLOW
+
+config CMODEL_MEDLOW
+ bool "medium low code model"
+ help
+ U-Boot and its statically defined symbols must lie within a single 2 GiB
+ address range and must lie between absolute addresses -2 GiB and +2 GiB.
+
+config CMODEL_MEDANY
+ bool "medium any code model"
+ help
+ U-Boot and its statically defined symbols must be within any single 2 GiB
+ address range.
+
+endchoice
+
config RISCV_ISA_C
bool "Emit compressed instructions"
default y
diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index 55d7c65..0b80eb8 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -17,8 +17,15 @@ endif
ifeq ($(CONFIG_RISCV_ISA_C),y)
ARCH_C = c
endif
+ifeq ($(CONFIG_CMODEL_MEDLOW),y)
+ CMODEL = medlow
+endif
+ifeq ($(CONFIG_CMODEL_MEDANY),y)
+ CMODEL = medany
+endif

-ARCH_FLAGS = -march=$(ARCH_BASE)$(ARCH_A)$(ARCH_C) -mabi=$(ABI)
+ARCH_FLAGS = -march=$(ARCH_BASE)$(ARCH_A)$(ARCH_C) -mabi=$(ABI) \
+ -mcmodel=$(CMODEL)

PLATFORM_CPPFLAGS += $(ARCH_FLAGS)
CFLAGS_EFI += $(ARCH_FLAGS)

--
2.7.4

Bin Meng

2018-11-13 08:21:53 UTC

This post might be inappropriate. Click to display it.

Auer, Lukas

2018-11-13 14:45:10 UTC

Hi Bin,

Post by Bin Meng
This adds U-Boot syscon driver for RISC-V Core Local Interruptor
(CLINT). The CLINT block holds memory-mapped control and status
registers associated with software and timer interrupts.
3 APIs are provided for U-Boot to implement Supervisor Binary
Interface (SBI) as defined by the RISC-V privileged architecture
spec v1.10.
---

Would it make sense to also abstract the functions provided by the
CLINT more? The reason why I am asking is because the CLINT is
(unfortunately) not specified as part of RISC-V. It is developing into
a de facto standard since other platforms are following SiFive's
implementation, but there is nothing that would prevent them from
implementing something else.

Two immediate examples I can think of would be mtime and the IPI
implementation. Many SoC vendors will likely already have a suitable
timer IP block for mtime. I can imagine that they would choose to re-
use their memory map instead of following that of the CLINT.
For the IPI implementation there is already an alternative, the SBI. If
U-Boot should be able to run in supervisor mode, it would be helpful to
support both the SBI and the CLINT interface.

So what we would need is some kind of API for the functionality
provided by the CLINT. For the multi-core support (I will try to send
out a series soon) I am re-using the IRQ uclass ID (it is only used in
arch code) to define a irq-uclass for RISC-V, which handles everything
with IPIs. The CLINT0 and SBI are then just different driver
implementing the uclass.

This is just an idea right now, what do you think?

Thanks,
Lukas

Bin Meng

2018-11-14 01:48:51 UTC

Hi Lukas,

On Tue, Nov 13, 2018 at 10:45 PM Auer, Lukas

Post by Auer, Lukas
Hi Bin,

Post by Bin Meng
This adds U-Boot syscon driver for RISC-V Core Local Interruptor
(CLINT). The CLINT block holds memory-mapped control and status
registers associated with software and timer interrupts.
3 APIs are provided for U-Boot to implement Supervisor Binary
Interface (SBI) as defined by the RISC-V privileged architecture
spec v1.10.
---

Would it make sense to also abstract the functions provided by the
CLINT more? The reason why I am asking is because the CLINT is
(unfortunately) not specified as part of RISC-V. It is developing into
a de facto standard since other platforms are following SiFive's
implementation, but there is nothing that would prevent them from
implementing something else.

I think your observation is correct about CLINT. Rick, does Andes's
RISC-V processor also follow SiFive's CLINT model?

Post by Auer, Lukas
Two immediate examples I can think of would be mtime and the IPI
implementation. Many SoC vendors will likely already have a suitable
timer IP block for mtime. I can imagine that they would choose to re-
use their memory map instead of following that of the CLINT.
For the IPI implementation there is already an alternative, the SBI. If
U-Boot should be able to run in supervisor mode, it would be helpful to
support both the SBI and the CLINT interface.

I am not sure I followed you here. This driver provides 3 APIs:
riscv_send_ipi() is for IPI, and the other 2 are for mtime/mtimecmp.

Post by Auer, Lukas
So what we would need is some kind of API for the functionality
provided by the CLINT. For the multi-core support (I will try to send
out a series soon) I am re-using the IRQ uclass ID (it is only used in
arch code) to define a irq-uclass for RISC-V, which handles everything
with IPIs. The CLINT0 and SBI are then just different driver
implementing the uclass.
This is just an idea right now, what do you think?

Regards,
Bin

Rick Chen

2018-11-14 08:02:13 UTC

Post by Bin Meng
Hi Lukas,

Post by Auer, Lukas
Hi Bin,

Post by Bin Meng
This adds U-Boot syscon driver for RISC-V Core Local Interruptor
(CLINT). The CLINT block holds memory-mapped control and status
registers associated with software and timer interrupts.
3 APIs are provided for U-Boot to implement Supervisor Binary
Interface (SBI) as defined by the RISC-V privileged architecture
spec v1.10.
---

Would it make sense to also abstract the functions provided by the
CLINT more? The reason why I am asking is because the CLINT is
(unfortunately) not specified as part of RISC-V. It is developing into
a de facto standard since other platforms are following SiFive's
implementation, but there is nothing that would prevent them from
implementing something else.

I think your observation is correct about CLINT. Rick, does Andes's RISC-V
processor also follow SiFive's CLINT model?

Hi Ben

As I know the mtime is the same as SiFive, but ipi is different.
They all implemented via IP block instead of CSR.

And the following base definition seem to be fixed as some kind of HW implement.
Maybe it can be more flexible.

/* MSIP registers */
#define MSIP_REG(base, hart) ((ulong)(base) + (hart) * 4)
/* Timer compare register */
#define MTIMECMP_REG(base, hart) ((ulong)(base) + 0x4000 + (hart) * 8)
/* Timer register */
#define MTIME_REG(base) ((ulong)(base) + 0xbff8)

Andes's ipi memory map
plic0: interrupt-***@e4000000 {
compatible = "riscv,plic0";
#address-cells = <2>;
#interrupt-cells = <2>;
interrupt-controller;
reg = <0x0 0xe4000000 0x0 0x2000000>;
riscv,ndev=<71>;
interrupts-extended = <&CPU0_intc 11 &CPU0_intc 9>;
};

Andes's mt memory map
***@e6000000 {
compatible = "riscv,plmt0";
interrupts-extended = <&CPU0_intc 7>;
reg = <0x0 0xe6000000 0x0 0x100000>;
};
};

It is obviously to be found that their base is different here.

Rick

Post by Bin Meng

Post by Auer, Lukas
Two immediate examples I can think of would be mtime and the IPI
implementation. Many SoC vendors will likely already have a suitable
timer IP block for mtime. I can imagine that they would choose to re-
use their memory map instead of following that of the CLINT.
For the IPI implementation there is already an alternative, the SBI.
If U-Boot should be able to run in supervisor mode, it would be
helpful to support both the SBI and the CLINT interface.

riscv_send_ipi() is for IPI, and the other 2 are for mtime/mtimecmp.

Post by Auer, Lukas
So what we would need is some kind of API for the functionality
provided by the CLINT. For the multi-core support (I will try to send
out a series soon) I am re-using the IRQ uclass ID (it is only used in
arch code) to define a irq-uclass for RISC-V, which handles everything
with IPIs. The CLINT0 and SBI are then just different driver
implementing the uclass.
This is just an idea right now, what do you think?

Regards,
Bin

Auer, Lukas

2018-11-14 10:33:02 UTC

Hi Bin,

Post by Bin Meng
Hi Lukas,
On Tue, Nov 13, 2018 at 10:45 PM Auer, Lukas

Post by Auer, Lukas
Hi Bin,

Post by Bin Meng
This adds U-Boot syscon driver for RISC-V Core Local Interruptor
(CLINT). The CLINT block holds memory-mapped control and status
registers associated with software and timer interrupts.
3 APIs are provided for U-Boot to implement Supervisor Binary
Interface (SBI) as defined by the RISC-V privileged architecture
spec v1.10.
---

Would it make sense to also abstract the functions provided by the
CLINT more? The reason why I am asking is because the CLINT is
(unfortunately) not specified as part of RISC-V. It is developing into
a de facto standard since other platforms are following SiFive's
implementation, but there is nothing that would prevent them from
implementing something else.

I think your observation is correct about CLINT. Rick, does Andes's
RISC-V processor also follow SiFive's CLINT model?

Post by Auer, Lukas
Two immediate examples I can think of would be mtime and the IPI
implementation. Many SoC vendors will likely already have a
suitable
timer IP block for mtime. I can imagine that they would choose to re-
use their memory map instead of following that of the CLINT.
For the IPI implementation there is already an alternative, the SBI. If
U-Boot should be able to run in supervisor mode, it would be
helpful to
support both the SBI and the CLINT interface.

riscv_send_ipi() is for IPI, and the other 2 are for mtime/mtimecmp.

It does, but I am not sure how easy it is to support different devices.
Supporting the SBI is not going to be an issue, more problematic would
be if we have two different devices and device tree nodes to implement
the functionality of the APIs. Now we have to bind this driver to two
devices and call the APIs from the correct instantiation, which would
not work.

Thinking about this a little more, I think the only issue is that we
have both IPI- and mtime-related APIs in one driver. How about
something like this? Instead of binding this driver to riscv,clint0, we
add a new misc driver for the clint0. The only thing the driver does,
is to bind the syscon driver and the timer driver (see for example
tegra-car.c). Other architectures with separate device tree nodes for
the API functionality won't need the misc driver and can just bind the
devices to the syscon driver and a timer driver.

Thanks,
Lukas

Post by Bin Meng

Post by Auer, Lukas
So what we would need is some kind of API for the functionality
provided by the CLINT. For the multi-core support (I will try to send
out a series soon) I am re-using the IRQ uclass ID (it is only used in
arch code) to define a irq-uclass for RISC-V, which handles
everything
with IPIs. The CLINT0 and SBI are then just different driver
implementing the uclass.
This is just an idea right now, what do you think?

Regards,
Bin

Bin Meng

2018-12-05 09:59:34 UTC

Hi Lukas,

On Wed, Nov 14, 2018 at 6:33 PM Auer, Lukas

Post by Auer, Lukas
Hi Bin,

Post by Bin Meng
Hi Lukas,
On Tue, Nov 13, 2018 at 10:45 PM Auer, Lukas

Post by Auer, Lukas
Hi Bin,

Post by Bin Meng
This adds U-Boot syscon driver for RISC-V Core Local Interruptor
(CLINT). The CLINT block holds memory-mapped control and status
registers associated with software and timer interrupts.
3 APIs are provided for U-Boot to implement Supervisor Binary
Interface (SBI) as defined by the RISC-V privileged architecture
spec v1.10.
---

Would it make sense to also abstract the functions provided by the
CLINT more? The reason why I am asking is because the CLINT is
(unfortunately) not specified as part of RISC-V. It is developing into
a de facto standard since other platforms are following SiFive's
implementation, but there is nothing that would prevent them from
implementing something else.

I think your observation is correct about CLINT. Rick, does Andes's
RISC-V processor also follow SiFive's CLINT model?

Post by Auer, Lukas
Two immediate examples I can think of would be mtime and the IPI
implementation. Many SoC vendors will likely already have a suitable
timer IP block for mtime. I can imagine that they would choose to re-
use their memory map instead of following that of the CLINT.
For the IPI implementation there is already an alternative, the SBI. If
U-Boot should be able to run in supervisor mode, it would be helpful to
support both the SBI and the CLINT interface.

riscv_send_ipi() is for IPI, and the other 2 are for mtime/mtimecmp.

It does, but I am not sure how easy it is to support different devices.
Supporting the SBI is not going to be an issue, more problematic would
be if we have two different devices and device tree nodes to implement
the functionality of the APIs. Now we have to bind this driver to two
devices and call the APIs from the correct instantiation, which would
not work.
Thinking about this a little more, I think the only issue is that we
have both IPI- and mtime-related APIs in one driver. How about
something like this? Instead of binding this driver to riscv,clint0, we
add a new misc driver for the clint0. The only thing the driver does,
is to bind the syscon driver and the timer driver (see for example
tegra-car.c). Other architectures with separate device tree nodes for
the API functionality won't need the misc driver and can just bind the
devices to the syscon driver and a timer driver.

Sorry it took a long time before replying this. I did have a look at
the tegra-car.c driver, and also tried various experiments. As Rick
pointed out we have to handle mixed IP blocks like Andes chip for
mtimer and IPIs. So it looks we need be able to flexibly handle the
following cases (sigh):

- SiFive's clint model which implements mtimer and IPI
- mtimer following SiFive's clint model, but IPI does not (Andes chip)
- IPI following SiFive's clint model, but mtimer follows (hypothetical
model which does not exist today)
- completely different mtimer and IPI models from SiFive's clint model

I don't quite follow your idea of implementing clint as a misc
driver, then binding syscon and timer devices in the misc driver. But
I think we can only have the misc driver, and use misc uclass's
ioctl() to implement the SBI required calls (set_timecmp, get_time,
send_ipi), and we can have another ioctl code to report its capability
to support mtimer and/or IPI functionality. This can support
flexibility. However I believe this may affect performance if we go
through many uclass function calls when doing SBI.

The solution does not look clean to me :(

Regards,
Bin

Auer, Lukas

2018-12-05 23:11:24 UTC

Hi Bin,

Post by Bin Meng
Hi Lukas,
On Wed, Nov 14, 2018 at 6:33 PM Auer, Lukas

Post by Auer, Lukas
Hi Bin,

Post by Bin Meng
Hi Lukas,
On Tue, Nov 13, 2018 at 10:45 PM Auer, Lukas

Post by Auer, Lukas
Hi Bin,

Post by Bin Meng
This adds U-Boot syscon driver for RISC-V Core Local
Interruptor
(CLINT). The CLINT block holds memory-mapped control and status
registers associated with software and timer interrupts.
3 APIs are provided for U-Boot to implement Supervisor Binary
Interface (SBI) as defined by the RISC-V privileged
architecture
spec v1.10.
---

Would it make sense to also abstract the functions provided by the
CLINT more? The reason why I am asking is because the CLINT is
(unfortunately) not specified as part of RISC-V. It is
developing
into
a de facto standard since other platforms are following
SiFive's
implementation, but there is nothing that would prevent them from
implementing something else.

I think your observation is correct about CLINT. Rick, does Andes's
RISC-V processor also follow SiFive's CLINT model?

Post by Auer, Lukas
Two immediate examples I can think of would be mtime and the IPI
implementation. Many SoC vendors will likely already have a suitable
timer IP block for mtime. I can imagine that they would choose
to
re-
use their memory map instead of following that of the CLINT.
For the IPI implementation there is already an alternative, the SBI. If
U-Boot should be able to run in supervisor mode, it would be helpful to
support both the SBI and the CLINT interface.

riscv_send_ipi() is for IPI, and the other 2 are for
mtime/mtimecmp.

It does, but I am not sure how easy it is to support different devices.
Supporting the SBI is not going to be an issue, more problematic would
be if we have two different devices and device tree nodes to
implement
the functionality of the APIs. Now we have to bind this driver to two
devices and call the APIs from the correct instantiation, which would
not work.
Thinking about this a little more, I think the only issue is that we
have both IPI- and mtime-related APIs in one driver. How about
something like this? Instead of binding this driver to
riscv,clint0, we
add a new misc driver for the clint0. The only thing the driver does,
is to bind the syscon driver and the timer driver (see for example
tegra-car.c). Other architectures with separate device tree nodes for
the API functionality won't need the misc driver and can just bind the
devices to the syscon driver and a timer driver.

Sorry it took a long time before replying this. I did have a look at
the tegra-car.c driver, and also tried various experiments. As Rick
pointed out we have to handle mixed IP blocks like Andes chip for
mtimer and IPIs. So it looks we need be able to flexibly handle the
- SiFive's clint model which implements mtimer and IPI
- mtimer following SiFive's clint model, but IPI does not (Andes chip)
- IPI following SiFive's clint model, but mtimer follows
(hypothetical
model which does not exist today)
- completely different mtimer and IPI models from SiFive's clint model

This really is not ideal. I don't think there is a nice way of handling
this since there is no way to detect which version we have. A cleaner
solution would be to get everyone to use separate compatible strings so
that we can load the correct driver or use the correct register
offsets.
I can't actually find a device node for the CLINT in the device tree of
Andes' chip. If they already use a different compatible string then
this should work.

Post by Bin Meng
I don't quite follow your idea of implementing clint as a misc
driver, then binding syscon and timer devices in the misc driver. But
I think we can only have the misc driver, and use misc uclass's
ioctl() to implement the SBI required calls (set_timecmp, get_time,
send_ipi), and we can have another ioctl code to report its
capability
to support mtimer and/or IPI functionality. This can support
flexibility. However I believe this may affect performance if we go
through many uclass function calls when doing SBI.
The solution does not look clean to me :(

Yes I agree, that seems too complex. My idea was to only use the misc
driver to bind different sub-devices, so that we can separate the
functionality of the CLINT into separate drivers.
If you are interested, I have pushed my WIP patch series for SMP
support to Github [1]. It works, but is not finished yet and, in
particular, must be rebased on top of your series. The misc driver for
the CLINT0 is added in [2].

Thinking about this a bit more, I think your implementation might be
best. RISC-V specifies that all implementation must have an mtime CSR.
So it makes sense to have just one timer driver for RISC-V, which
accesses mtime using an API. We can then have different syscon drivers
to implement the IPI and timer related APIs. So for qemu, we would have
one syscon driver for the CLINT0 (when U-Boot is running in machine
mode) and one using the SBI functions (when U-Boot is running in
supervisor mode).
We would need more error handling though to, for example, handle the
case where no syscon driver is bound. Essentially an interface for the
syscon drivers to implement, similar to a uclass.

Thanks,
Lukas

[1]: https://github.com/lukasauer/u-boot/commits/riscv-smp
[2]:
https://github.com/lukasauer/u-boot/commit/7901e68ed25ac8e3008f76a8f2c5a11e1b8e2952

Bin Meng

2018-12-06 10:07:40 UTC

Hi Lukas,

On Thu, Dec 6, 2018 at 7:11 AM Auer, Lukas

Post by Auer, Lukas
Hi Bin,

Post by Bin Meng
Hi Lukas,
On Wed, Nov 14, 2018 at 6:33 PM Auer, Lukas

Post by Auer, Lukas
Hi Bin,

Post by Bin Meng
Hi Lukas,
On Tue, Nov 13, 2018 at 10:45 PM Auer, Lukas

Post by Auer, Lukas
Hi Bin,

Post by Bin Meng
This adds U-Boot syscon driver for RISC-V Core Local
Interruptor
(CLINT). The CLINT block holds memory-mapped control and status
registers associated with software and timer interrupts.
3 APIs are provided for U-Boot to implement Supervisor Binary
Interface (SBI) as defined by the RISC-V privileged
architecture
spec v1.10.
---

Would it make sense to also abstract the functions provided by the
CLINT more? The reason why I am asking is because the CLINT is
(unfortunately) not specified as part of RISC-V. It is
developing
into
a de facto standard since other platforms are following SiFive's
implementation, but there is nothing that would prevent them from
implementing something else.

I think your observation is correct about CLINT. Rick, does Andes's
RISC-V processor also follow SiFive's CLINT model?

Post by Auer, Lukas
Two immediate examples I can think of would be mtime and the IPI
implementation. Many SoC vendors will likely already have a suitable
timer IP block for mtime. I can imagine that they would choose
to
re-
use their memory map instead of following that of the CLINT.
For the IPI implementation there is already an alternative, the SBI. If
U-Boot should be able to run in supervisor mode, it would be helpful to
support both the SBI and the CLINT interface.

riscv_send_ipi() is for IPI, and the other 2 are for
mtime/mtimecmp.

It does, but I am not sure how easy it is to support different devices.
Supporting the SBI is not going to be an issue, more problematic would
be if we have two different devices and device tree nodes to implement
the functionality of the APIs. Now we have to bind this driver to two
devices and call the APIs from the correct instantiation, which would
not work.
Thinking about this a little more, I think the only issue is that we
have both IPI- and mtime-related APIs in one driver. How about
something like this? Instead of binding this driver to
riscv,clint0, we
add a new misc driver for the clint0. The only thing the driver does,
is to bind the syscon driver and the timer driver (see for example
tegra-car.c). Other architectures with separate device tree nodes for
the API functionality won't need the misc driver and can just bind the
devices to the syscon driver and a timer driver.

Sorry it took a long time before replying this. I did have a look at
the tegra-car.c driver, and also tried various experiments. As Rick
pointed out we have to handle mixed IP blocks like Andes chip for
mtimer and IPIs. So it looks we need be able to flexibly handle the
- SiFive's clint model which implements mtimer and IPI
- mtimer following SiFive's clint model, but IPI does not (Andes chip)
- IPI following SiFive's clint model, but mtimer follows
(hypothetical
model which does not exist today)
- completely different mtimer and IPI models from SiFive's clint model

This really is not ideal. I don't think there is a nice way of handling
this since there is no way to detect which version we have. A cleaner
solution would be to get everyone to use separate compatible strings so
that we can load the correct driver or use the correct register
offsets.
I can't actually find a device node for the CLINT in the device tree of
Andes' chip. If they already use a different compatible string then
this should work.

I cannot find the clint compatible string for Andes chip too ...

Post by Auer, Lukas

Post by Bin Meng
I don't quite follow your idea of implementing clint as a misc
driver, then binding syscon and timer devices in the misc driver. But
I think we can only have the misc driver, and use misc uclass's
ioctl() to implement the SBI required calls (set_timecmp, get_time,
send_ipi), and we can have another ioctl code to report its
capability
to support mtimer and/or IPI functionality. This can support
flexibility. However I believe this may affect performance if we go
through many uclass function calls when doing SBI.
The solution does not look clean to me :(

Yes I agree, that seems too complex. My idea was to only use the misc
driver to bind different sub-devices, so that we can separate the
functionality of the CLINT into separate drivers.
If you are interested, I have pushed my WIP patch series for SMP
support to Github [1]. It works, but is not finished yet and, in
particular, must be rebased on top of your series. The misc driver for
the CLINT0 is added in [2].
Thinking about this a bit more, I think your implementation might be
best. RISC-V specifies that all implementation must have an mtime CSR.
So it makes sense to have just one timer driver for RISC-V, which
accesses mtime using an API. We can then have different syscon drivers
to implement the IPI and timer related APIs. So for qemu, we would have
one syscon driver for the CLINT0 (when U-Boot is running in machine
mode) and one using the SBI functions (when U-Boot is running in
supervisor mode).

Yes, having just one timer driver for RISC-V seems doable. I will try
to refine current implementation in v2.

Post by Auer, Lukas
We would need more error handling though to, for example, handle the
case where no syscon driver is bound. Essentially an interface for the
syscon drivers to implement, similar to a uclass.
Thanks,
Lukas
[1]: https://github.com/lukasauer/u-boot/commits/riscv-smp
https://github.com/lukasauer/u-boot/commit/7901e68ed25ac8e3008f76a8f2c5a11e1b8e2952

BTW: I think I can drop my last patch in my series and let you handle
the SMP boot in your series. Good work!

Regards,
Bin

Auer, Lukas

2018-12-06 12:30:55 UTC

Hi Bin,

Post by Bin Meng
Hi Lukas,
On Thu, Dec 6, 2018 at 7:11 AM Auer, Lukas

Post by Auer, Lukas
Hi Bin,

Post by Bin Meng
Hi Lukas,
On Wed, Nov 14, 2018 at 6:33 PM Auer, Lukas

Post by Auer, Lukas
Hi Bin,

Post by Bin Meng
Hi Lukas,
On Tue, Nov 13, 2018 at 10:45 PM Auer, Lukas

Post by Auer, Lukas
Hi Bin,

Post by Bin Meng
This adds U-Boot syscon driver for RISC-V Core Local Interruptor
(CLINT). The CLINT block holds memory-mapped control and status
registers associated with software and timer interrupts.
3 APIs are provided for U-Boot to implement Supervisor Binary
Interface (SBI) as defined by the RISC-V privileged architecture
spec v1.10.
---

Would it make sense to also abstract the functions provided
by
the
CLINT more? The reason why I am asking is because the CLINT is
(unfortunately) not specified as part of RISC-V. It is
developing
into
a de facto standard since other platforms are following SiFive's
implementation, but there is nothing that would prevent
them
from
implementing something else.

I think your observation is correct about CLINT. Rick, does Andes's
RISC-V processor also follow SiFive's CLINT model?

Post by Auer, Lukas
Two immediate examples I can think of would be mtime and
the
IPI
implementation. Many SoC vendors will likely already have a suitable
timer IP block for mtime. I can imagine that they would choose
to
re-
use their memory map instead of following that of the CLINT.
For the IPI implementation there is already an alternative,
the
SBI. If
U-Boot should be able to run in supervisor mode, it would be
helpful to
support both the SBI and the CLINT interface.

riscv_send_ipi() is for IPI, and the other 2 are for
mtime/mtimecmp.

It does, but I am not sure how easy it is to support different devices.
Supporting the SBI is not going to be an issue, more
problematic
would
be if we have two different devices and device tree nodes to implement
the functionality of the APIs. Now we have to bind this driver
to
two
devices and call the APIs from the correct instantiation, which would
not work.
Thinking about this a little more, I think the only issue is
that
we
have both IPI- and mtime-related APIs in one driver. How about
something like this? Instead of binding this driver to
riscv,clint0, we
add a new misc driver for the clint0. The only thing the driver does,
is to bind the syscon driver and the timer driver (see for example
tegra-car.c). Other architectures with separate device tree
nodes
for
the API functionality won't need the misc driver and can just
bind
the
devices to the syscon driver and a timer driver.

Sorry it took a long time before replying this. I did have a look at
the tegra-car.c driver, and also tried various experiments. As Rick
pointed out we have to handle mixed IP blocks like Andes chip for
mtimer and IPIs. So it looks we need be able to flexibly handle the
- SiFive's clint model which implements mtimer and IPI
- mtimer following SiFive's clint model, but IPI does not (Andes chip)
- IPI following SiFive's clint model, but mtimer follows
(hypothetical
model which does not exist today)
- completely different mtimer and IPI models from SiFive's clint model

This really is not ideal. I don't think there is a nice way of handling
this since there is no way to detect which version we have. A cleaner
solution would be to get everyone to use separate compatible
strings so
that we can load the correct driver or use the correct register
offsets.
I can't actually find a device node for the CLINT in the device tree of
Andes' chip. If they already use a different compatible string then
this should work.

I cannot find the clint compatible string for Andes chip too ...

Post by Auer, Lukas

Post by Bin Meng
I don't quite follow your idea of implementing clint as a misc
driver, then binding syscon and timer devices in the misc driver. But
I think we can only have the misc driver, and use misc uclass's
ioctl() to implement the SBI required calls (set_timecmp,
get_time,
send_ipi), and we can have another ioctl code to report its capability
to support mtimer and/or IPI functionality. This can support
flexibility. However I believe this may affect performance if we go
through many uclass function calls when doing SBI.
The solution does not look clean to me :(

Yes I agree, that seems too complex. My idea was to only use the misc
driver to bind different sub-devices, so that we can separate the
functionality of the CLINT into separate drivers.
If you are interested, I have pushed my WIP patch series for SMP
support to Github [1]. It works, but is not finished yet and, in
particular, must be rebased on top of your series. The misc driver for
the CLINT0 is added in [2].
Thinking about this a bit more, I think your implementation might be
best. RISC-V specifies that all implementation must have an mtime CSR.
So it makes sense to have just one timer driver for RISC-V, which
accesses mtime using an API. We can then have different syscon drivers
to implement the IPI and timer related APIs. So for qemu, we would have
one syscon driver for the CLINT0 (when U-Boot is running in machine
mode) and one using the SBI functions (when U-Boot is running in
supervisor mode).

Yes, having just one timer driver for RISC-V seems doable. I will try
to refine current implementation in v2.

Sounds good, looking forward to it. One more question, can you also add
an API to clear the IPI bit?

Post by Bin Meng

Post by Auer, Lukas
We would need more error handling though to, for example, handle the
case where no syscon driver is bound. Essentially an interface for the
syscon drivers to implement, similar to a uclass.
Thanks,
Lukas
[1]: https://github.com/lukasauer/u-boot/commits/riscv-smp

https://github.com/lukasauer/u-boot/commit/7901e68ed25ac8e3008f76a8f2c5a11e1b8e2952

Post by Bin Meng
BTW: I think I can drop my last patch in my series and let you handle
the SMP boot in your series. Good work!

Thanks!

Lukas

Bin Meng

2018-12-07 14:00:02 UTC

Hi Lukas,

On Thu, Dec 6, 2018 at 8:30 PM Auer, Lukas

Post by Auer, Lukas
Hi Bin,

Post by Bin Meng
Hi Lukas,
On Thu, Dec 6, 2018 at 7:11 AM Auer, Lukas

Post by Auer, Lukas
Hi Bin,

Post by Bin Meng
Hi Lukas,
On Wed, Nov 14, 2018 at 6:33 PM Auer, Lukas

Post by Auer, Lukas
Hi Bin,

Post by Bin Meng
Hi Lukas,
On Tue, Nov 13, 2018 at 10:45 PM Auer, Lukas

Post by Auer, Lukas
Hi Bin,

Post by Bin Meng
This adds U-Boot syscon driver for RISC-V Core Local
Interruptor
(CLINT). The CLINT block holds memory-mapped control and status
registers associated with software and timer interrupts.
3 APIs are provided for U-Boot to implement Supervisor Binary
Interface (SBI) as defined by the RISC-V privileged
architecture
spec v1.10.
---

Would it make sense to also abstract the functions provided
by
the
CLINT more? The reason why I am asking is because the CLINT is
(unfortunately) not specified as part of RISC-V. It is
developing
into
a de facto standard since other platforms are following SiFive's
implementation, but there is nothing that would prevent
them
from
implementing something else.

I think your observation is correct about CLINT. Rick, does Andes's
RISC-V processor also follow SiFive's CLINT model?

Post by Auer, Lukas
Two immediate examples I can think of would be mtime and
the
IPI
implementation. Many SoC vendors will likely already have a
suitable
timer IP block for mtime. I can imagine that they would choose
to
re-
use their memory map instead of following that of the CLINT.
For the IPI implementation there is already an alternative,
the
SBI. If
U-Boot should be able to run in supervisor mode, it would be
helpful to
support both the SBI and the CLINT interface.

riscv_send_ipi() is for IPI, and the other 2 are for
mtime/mtimecmp.

It does, but I am not sure how easy it is to support different devices.
Supporting the SBI is not going to be an issue, more
problematic
would
be if we have two different devices and device tree nodes to implement
the functionality of the APIs. Now we have to bind this driver
to
two
devices and call the APIs from the correct instantiation, which would
not work.
Thinking about this a little more, I think the only issue is
that
we
have both IPI- and mtime-related APIs in one driver. How about
something like this? Instead of binding this driver to
riscv,clint0, we
add a new misc driver for the clint0. The only thing the driver does,
is to bind the syscon driver and the timer driver (see for example
tegra-car.c). Other architectures with separate device tree
nodes
for
the API functionality won't need the misc driver and can just
bind
the
devices to the syscon driver and a timer driver.

Sorry it took a long time before replying this. I did have a look at
the tegra-car.c driver, and also tried various experiments. As Rick
pointed out we have to handle mixed IP blocks like Andes chip for
mtimer and IPIs. So it looks we need be able to flexibly handle the
- SiFive's clint model which implements mtimer and IPI
- mtimer following SiFive's clint model, but IPI does not (Andes chip)
- IPI following SiFive's clint model, but mtimer follows
(hypothetical
model which does not exist today)
- completely different mtimer and IPI models from SiFive's clint model

This really is not ideal. I don't think there is a nice way of handling
this since there is no way to detect which version we have. A cleaner
solution would be to get everyone to use separate compatible strings so
that we can load the correct driver or use the correct register
offsets.
I can't actually find a device node for the CLINT in the device tree of
Andes' chip. If they already use a different compatible string then
this should work.

I cannot find the clint compatible string for Andes chip too ...

Post by Auer, Lukas

Post by Bin Meng
I don't quite follow your idea of implementing clint as a misc
driver, then binding syscon and timer devices in the misc driver. But
I think we can only have the misc driver, and use misc uclass's
ioctl() to implement the SBI required calls (set_timecmp, get_time,
send_ipi), and we can have another ioctl code to report its capability
to support mtimer and/or IPI functionality. This can support
flexibility. However I believe this may affect performance if we go
through many uclass function calls when doing SBI.
The solution does not look clean to me :(

Yes I agree, that seems too complex. My idea was to only use the misc
driver to bind different sub-devices, so that we can separate the
functionality of the CLINT into separate drivers.
If you are interested, I have pushed my WIP patch series for SMP
support to Github [1]. It works, but is not finished yet and, in
particular, must be rebased on top of your series. The misc driver for
the CLINT0 is added in [2].
Thinking about this a bit more, I think your implementation might be
best. RISC-V specifies that all implementation must have an mtime CSR.
So it makes sense to have just one timer driver for RISC-V, which
accesses mtime using an API. We can then have different syscon drivers
to implement the IPI and timer related APIs. So for qemu, we would have
one syscon driver for the CLINT0 (when U-Boot is running in machine
mode) and one using the SBI functions (when U-Boot is running in
supervisor mode).

Yes, having just one timer driver for RISC-V seems doable. I will try
to refine current implementation in v2.

Sounds good, looking forward to it. One more question, can you also add
an API to clear the IPI bit?

Will do in v2.

Post by Auer, Lukas

Post by Bin Meng

Post by Auer, Lukas
We would need more error handling though to, for example, handle the
case where no syscon driver is bound. Essentially an interface for the
syscon drivers to implement, similar to a uclass.
Thanks,
Lukas
[1]: https://github.com/lukasauer/u-boot/commits/riscv-smp

https://github.com/lukasauer/u-boot/commit/7901e68ed25ac8e3008f76a8f2c5a11e1b8e2952

Post by Bin Meng
BTW: I think I can drop my last patch in my series and let you handle
the SMP boot in your series. Good work!

Thanks!

Regards,
Bin

Philipp Tomsich

2018-12-06 14:33:42 UTC

Post by Bin Meng
This adds U-Boot syscon driver for RISC-V Core Local Interruptor
(CLINT). The CLINT block holds memory-mapped control and status
registers associated with software and timer interrupts.
3 APIs are provided for U-Boot to implement Supervisor Binary
Interface (SBI) as defined by the RISC-V privileged architecture
spec v1.10.
---
arch/riscv/Kconfig | 8 +++++
arch/riscv/include/asm/clint.h | 24 +++++++++++++++
arch/riscv/lib/Makefile | 1 +
arch/riscv/lib/clint.c | 69 ++++++++++++++++++++++++++++++++++++++++++
4 files changed, 102 insertions(+)
create mode 100644 arch/riscv/include/asm/clint.h
create mode 100644 arch/riscv/lib/clint.c
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index a37e895..abfc083 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -73,4 +73,12 @@ config 32BIT
config 64BIT
bool
+config RISCV_CLINT
+ bool "Support Core Local Interruptor (CLINT)"
+ select REGMAP
+ select SYSCON
+ help
+ The CLINT block holds memory-mapped control and status registers
+ associated with software and timer interrupts.
+
endmenu
diff --git a/arch/riscv/include/asm/clint.h b/arch/riscv/include/asm/clint.h
new file mode 100644
index 0000000..1c6024f
--- /dev/null
+++ b/arch/riscv/include/asm/clint.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ */
+
+#ifndef _ASM_RISCV_CLINT_H
+#define _ASM_RISCV_CLINT_H
+
+/*
+ * System controllers in a RISC-V system
+ *
+ * So far only Core Local Interruptor (CLINT) is defined. If new system
+ * controller is added, we may need move the defines to somewhere else.
+ */
+enum {
+ RISCV_NONE,
+ RISCV_SYSCON_CLINT, /* Core Local Interruptor (CLINT) */
+};
+
+void riscv_send_ipi(int hart);
+void riscv_set_timecmp(int hart, u64 cmp);
+u64 riscv_get_time(void);
+
+#endif /* _ASM_RISCV_CLINT_H */
diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile
index b58db89..b5a77c2 100644
--- a/arch/riscv/lib/Makefile
+++ b/arch/riscv/lib/Makefile
@@ -9,6 +9,7 @@
obj-$(CONFIG_CMD_BOOTM) += bootm.o
obj-$(CONFIG_CMD_GO) += boot.o
obj-y += cache.o
+obj-$(CONFIG_RISCV_CLINT) += clint.o
obj-y += interrupts.o
obj-y += reset.o
obj-y += setjmp.o
diff --git a/arch/riscv/lib/clint.c b/arch/riscv/lib/clint.c
new file mode 100644
index 0000000..369aa1d
--- /dev/null
+++ b/arch/riscv/lib/clint.c
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *
+ * U-Boot syscon driver for RISC-V Core Local Interruptor (CLINT)
+ * The CLINT block holds memory-mapped control and status registers
+ * associated with software and timer interrupts.
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <regmap.h>
+#include <syscon.h>
+#include <asm/clint.h>
+#include <asm/io.h>
+
+/* MSIP registers */
+#define MSIP_REG(base, hart) ((ulong)(base) + (hart) * 4)
+/* Timer compare register */
+#define MTIMECMP_REG(base, hart) ((ulong)(base) + 0x4000 + (hart) * 8)
+/* Timer register */
+#define MTIME_REG(base) ((ulong)(base) + 0xbff8)
+
+static void __iomem *clint_base;
+
+/*
+ * The following 3 APIs are used to implement Supervisor Binary Interface (SBI)
+ * as defined by the RISC-V privileged architecture spec v1.10.
+ *
+ * For performance reasons we don't want to get the CLINT register base via
+ * syscon_get_first_range() each time we enter in those functions, instead
+ * the base address was saved to a global variable during the clint driver
+ * probe phase, so that we can use it directly.
+ */
+
+void riscv_send_ipi(int hart)
+{
+ writel(1, (void __iomem *)MSIP_REG(clint_base, hart));
+}
+
+void riscv_set_timecmp(int hart, u64 cmp)
+{
+ writeq(cmp, (void __iomem *)MTIMECMP_REG(clint_base, hart));
+}
+
+u64 riscv_get_time(void)
+{
+ return readq((void __iomem *)MTIME_REG(clint_base));
+}
+
+static int clint_probe(struct udevice *dev)
+{
+ clint_base = syscon_get_first_range(RISCV_SYSCON_CLINT);
+
+ return 0;
+}
+
+static const struct udevice_id clint_ids[] = {
+ { .compatible = "riscv,clint0", .data = RISCV_SYSCON_CLINT },
+ { }
+};
+
+U_BOOT_DRIVER(riscv_clint) = {
+ .name = "riscv_clint",
+ .id = UCLASS_SYSCON,

Wouldn’t a separate UCLASS be more appropriate?

Post by Bin Meng
+ .of_match = clint_ids,
+ .probe = clint_probe,
+ .flags = DM_FLAG_PRE_RELOC,
+};
--
2.7.4
_______________________________________________
U-Boot mailing list
https://lists.denx.de/listinfo/u-boot

Bin Meng

2018-12-07 14:01:16 UTC

Hi Philipp,

On Thu, Dec 6, 2018 at 10:33 PM Philipp Tomsich

Post by Philipp Tomsich

Post by Bin Meng
This adds U-Boot syscon driver for RISC-V Core Local Interruptor
(CLINT). The CLINT block holds memory-mapped control and status
registers associated with software and timer interrupts.
3 APIs are provided for U-Boot to implement Supervisor Binary
Interface (SBI) as defined by the RISC-V privileged architecture
spec v1.10.
---
arch/riscv/Kconfig | 8 +++++
arch/riscv/include/asm/clint.h | 24 +++++++++++++++
arch/riscv/lib/Makefile | 1 +
arch/riscv/lib/clint.c | 69 ++++++++++++++++++++++++++++++++++++++++++
4 files changed, 102 insertions(+)
create mode 100644 arch/riscv/include/asm/clint.h
create mode 100644 arch/riscv/lib/clint.c
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index a37e895..abfc083 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -73,4 +73,12 @@ config 32BIT
config 64BIT
bool
+config RISCV_CLINT
+ bool "Support Core Local Interruptor (CLINT)"
+ select REGMAP
+ select SYSCON
+ help
+ The CLINT block holds memory-mapped control and status registers
+ associated with software and timer interrupts.
+
endmenu
diff --git a/arch/riscv/include/asm/clint.h b/arch/riscv/include/asm/clint.h
new file mode 100644
index 0000000..1c6024f
--- /dev/null
+++ b/arch/riscv/include/asm/clint.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ */
+
+#ifndef _ASM_RISCV_CLINT_H
+#define _ASM_RISCV_CLINT_H
+
+/*
+ * System controllers in a RISC-V system
+ *
+ * So far only Core Local Interruptor (CLINT) is defined. If new system
+ * controller is added, we may need move the defines to somewhere else.
+ */
+enum {
+ RISCV_NONE,
+ RISCV_SYSCON_CLINT, /* Core Local Interruptor (CLINT) */
+};
+
+void riscv_send_ipi(int hart);
+void riscv_set_timecmp(int hart, u64 cmp);
+u64 riscv_get_time(void);
+
+#endif /* _ASM_RISCV_CLINT_H */
diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile
index b58db89..b5a77c2 100644
--- a/arch/riscv/lib/Makefile
+++ b/arch/riscv/lib/Makefile
@@ -9,6 +9,7 @@
obj-$(CONFIG_CMD_BOOTM) += bootm.o
obj-$(CONFIG_CMD_GO) += boot.o
obj-y += cache.o
+obj-$(CONFIG_RISCV_CLINT) += clint.o
obj-y += interrupts.o
obj-y += reset.o
obj-y += setjmp.o
diff --git a/arch/riscv/lib/clint.c b/arch/riscv/lib/clint.c
new file mode 100644
index 0000000..369aa1d
--- /dev/null
+++ b/arch/riscv/lib/clint.c
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *
+ * U-Boot syscon driver for RISC-V Core Local Interruptor (CLINT)
+ * The CLINT block holds memory-mapped control and status registers
+ * associated with software and timer interrupts.
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <regmap.h>
+#include <syscon.h>
+#include <asm/clint.h>
+#include <asm/io.h>
+
+/* MSIP registers */
+#define MSIP_REG(base, hart) ((ulong)(base) + (hart) * 4)
+/* Timer compare register */
+#define MTIMECMP_REG(base, hart) ((ulong)(base) + 0x4000 + (hart) * 8)
+/* Timer register */
+#define MTIME_REG(base) ((ulong)(base) + 0xbff8)
+
+static void __iomem *clint_base;
+
+/*
+ * The following 3 APIs are used to implement Supervisor Binary Interface (SBI)
+ * as defined by the RISC-V privileged architecture spec v1.10.
+ *
+ * For performance reasons we don't want to get the CLINT register base via
+ * syscon_get_first_range() each time we enter in those functions, instead
+ * the base address was saved to a global variable during the clint driver
+ * probe phase, so that we can use it directly.
+ */
+
+void riscv_send_ipi(int hart)
+{
+ writel(1, (void __iomem *)MSIP_REG(clint_base, hart));
+}
+
+void riscv_set_timecmp(int hart, u64 cmp)
+{
+ writeq(cmp, (void __iomem *)MTIMECMP_REG(clint_base, hart));
+}
+
+u64 riscv_get_time(void)
+{
+ return readq((void __iomem *)MTIME_REG(clint_base));
+}
+
+static int clint_probe(struct udevice *dev)
+{
+ clint_base = syscon_get_first_range(RISCV_SYSCON_CLINT);
+
+ return 0;
+}
+
+static const struct udevice_id clint_ids[] = {
+ { .compatible = "riscv,clint0", .data = RISCV_SYSCON_CLINT },
+ { }
+};
+
+U_BOOT_DRIVER(riscv_clint) = {
+ .name = "riscv_clint",
+ .id = UCLASS_SYSCON,

Wouldn’t a separate UCLASS be more appropriate?

I am afraid this CLINT model is not that common to deserve a separate UCLASS.

Regards,
Bin

Bin Meng

2018-11-13 08:21:59 UTC

This adds all exception codes in encoding.h.

Signed-off-by: Bin Meng <***@gmail.com>
---

arch/riscv/include/asm/encoding.h | 15 +++++++++++++++
1 file changed, 15 insertions(+)

diff --git a/arch/riscv/include/asm/encoding.h b/arch/riscv/include/asm/encoding.h
index 0c47c53..85a11c4 100644
--- a/arch/riscv/include/asm/encoding.h
+++ b/arch/riscv/include/asm/encoding.h
@@ -79,6 +79,21 @@
#define IRQ_COP 12
#define IRQ_HOST 13

+#define CAUSE_MISALIGNED_FETCH 0
+#define CAUSE_FETCH_ACCESS 1
+#define CAUSE_ILLEGAL_INSTRUCTION 2
+#define CAUSE_BREAKPOINT 3
+#define CAUSE_MISALIGNED_LOAD 4
+#define CAUSE_LOAD_ACCESS 5
+#define CAUSE_MISALIGNED_STORE 6
+#define CAUSE_STORE_ACCESS 7
+#define CAUSE_USER_ECALL 8
+#define CAUSE_SUPERVISOR_ECALL 9
+#define CAUSE_MACHINE_ECALL 11
+#define CAUSE_FETCH_PAGE_FAULT 12
+#define CAUSE_LOAD_PAGE_FAULT 13
+#define CAUSE_STORE_PAGE_FAULT 15
+
#define DEFAULT_RSTVEC 0x00001000
#define DEFAULT_NMIVEC 0x00001004
#define DEFAULT_MTVEC 0x00001010

--
2.7.4

Bin Meng

2018-11-13 08:21:52 UTC

This adds a driver for RISC-V CPU. Note the driver will bind
a RISC-V timer driver if "timebase-frequency" property is
present in the device tree.

Signed-off-by: Bin Meng <***@gmail.com>
---

drivers/cpu/Kconfig | 6 +++
drivers/cpu/Makefile | 1 +
drivers/cpu/riscv_cpu.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 124 insertions(+)
create mode 100644 drivers/cpu/riscv_cpu.c

diff --git a/drivers/cpu/Kconfig b/drivers/cpu/Kconfig
index d405200..3d5729f 100644
--- a/drivers/cpu/Kconfig
+++ b/drivers/cpu/Kconfig
@@ -13,3 +13,9 @@ config CPU_MPC83XX
select CLK_MPC83XX
help
Support CPU cores for SoCs of the MPC83xx series.
+
+config CPU_RISCV
+ bool "Enable RISC-V CPU driver"
+ depends on CPU && RISCV
+ help
+ Support CPU cores for RISC-V architecture.
diff --git a/drivers/cpu/Makefile b/drivers/cpu/Makefile
index 858b037..be0300c 100644
--- a/drivers/cpu/Makefile
+++ b/drivers/cpu/Makefile
@@ -8,4 +8,5 @@ obj-$(CONFIG_CPU) += cpu-uclass.o

obj-$(CONFIG_ARCH_BMIPS) += bmips_cpu.o
obj-$(CONFIG_CPU_MPC83XX) += mpc83xx_cpu.o
+obj-$(CONFIG_CPU_RISCV) += riscv_cpu.o
obj-$(CONFIG_SANDBOX) += cpu_sandbox.o
diff --git a/drivers/cpu/riscv_cpu.c b/drivers/cpu/riscv_cpu.c
new file mode 100644
index 0000000..23917db
--- /dev/null
+++ b/drivers/cpu/riscv_cpu.c
@@ -0,0 +1,117 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2018, Bin Meng <***@gmail.com>
+ */
+
+#include <common.h>
+#include <cpu.h>
+#include <dm.h>
+#include <errno.h>
+#include <dm/device-internal.h>
+#include <dm/lists.h>
+
+static int riscv_cpu_get_desc(struct udevice *dev, char *buf, int size)
+{
+ const char *isa;
+
+ isa = dev_read_string(dev, "riscv,isa");
+ if (size < (strlen(isa) + 1))
+ return -ENOSPC;
+
+ strcpy(buf, isa);
+
+ return 0;
+}
+
+static int riscv_cpu_get_info(struct udevice *dev, struct cpu_info *info)
+{
+ const char *mmu;
+
+ dev_read_u32(dev, "clock-frequency", (u32 *)&info->cpu_freq);
+
+ mmu = dev_read_string(dev, "mmu-type");
+ if (!mmu)
+ info->features |= BIT(CPU_FEAT_MMU);
+
+ return 0;
+}
+
+static int riscv_cpu_get_count(struct udevice *dev)
+{
+ ofnode node;
+ int num = 0;
+
+ ofnode_for_each_subnode(node, dev_ofnode(dev->parent)) {
+ const char *device_type;
+
+ device_type = ofnode_read_string(node, "device_type");
+ if (!device_type)
+ continue;
+ if (strcmp(device_type, "cpu") == 0)
+ num++;
+ }
+
+ return num;
+}
+
+static int riscv_cpu_bind(struct udevice *dev)
+{
+ struct cpu_platdata *plat = dev_get_parent_platdata(dev);
+ struct driver *drv;
+ struct udevice *timer;
+ int ret;
+
+ /* save the hart id */
+ plat->cpu_id = dev_read_addr(dev);
+
+ /* first examine the property in current cpu node */
+ ret = dev_read_u32(dev, "timebase-frequency", &plat->timebase_freq);
+ /* if not found, then look at the parent /cpus node */
+ if (ret)
+ dev_read_u32(dev->parent, "timebase-frequency",
+ &plat->timebase_freq);
+
+ /*
+ * Bind riscv-timer driver on hart 0
+ *
+ * We only instantiate one timer device which is enough for U-Boot.
+ * Pass the "timebase-frequency" value as the driver data for the
+ * timer device.
+ *
+ * Return value is not checked since it's possible that the timer
+ * driver is not included.
+ */
+ if (!plat->cpu_id && plat->timebase_freq) {
+ drv = lists_driver_lookup_name("riscv_timer");
+ if (!drv) {
+ debug("Cannot find the timer driver, not included?\n");
+ return 0;
+ }
+
+ device_bind_with_driver_data(dev, drv, "riscv_timer",
+ plat->timebase_freq, ofnode_null(),
+ &timer);
+ }
+
+ return 0;
+}
+
+static const struct cpu_ops riscv_cpu_ops = {
+ .get_desc = riscv_cpu_get_desc,
+ .get_info = riscv_cpu_get_info,
+ .get_count = riscv_cpu_get_count,
+};
+
+static const struct udevice_id riscv_cpu_ids[] = {
+ { .compatible = "riscv" },
+ { }
+};
+
+U_BOOT_DRIVER(riscv_cpu) = {
+ .name = "riscv_cpu",
+ .id = UCLASS_CPU,
+ .of_match = riscv_cpu_ids,
+ .bind = riscv_cpu_bind,
+ .ops = &riscv_cpu_ops,
+ .flags = DM_FLAG_PRE_RELOC,
+};

--
2.7.4

Auer, Lukas

2018-11-14 21:57:20 UTC

Hi Bin,

Post by Bin Meng
This adds a driver for RISC-V CPU. Note the driver will bind
a RISC-V timer driver if "timebase-frequency" property is
present in the device tree.
---

Since we have the CPU driver, we could also enable CMD_CPU.

Post by Bin Meng
drivers/cpu/Kconfig | 6 +++
drivers/cpu/Makefile | 1 +
drivers/cpu/riscv_cpu.c | 117
++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 124 insertions(+)
create mode 100644 drivers/cpu/riscv_cpu.c
diff --git a/drivers/cpu/Kconfig b/drivers/cpu/Kconfig
index d405200..3d5729f 100644
--- a/drivers/cpu/Kconfig
+++ b/drivers/cpu/Kconfig
@@ -13,3 +13,9 @@ config CPU_MPC83XX
select CLK_MPC83XX
help
Support CPU cores for SoCs of the MPC83xx series.
+
+config CPU_RISCV
+ bool "Enable RISC-V CPU driver"
+ depends on CPU && RISCV
+ help
+ Support CPU cores for RISC-V architecture.
diff --git a/drivers/cpu/Makefile b/drivers/cpu/Makefile
index 858b037..be0300c 100644
--- a/drivers/cpu/Makefile
+++ b/drivers/cpu/Makefile
@@ -8,4 +8,5 @@ obj-$(CONFIG_CPU) += cpu-uclass.o
obj-$(CONFIG_ARCH_BMIPS) += bmips_cpu.o
obj-$(CONFIG_CPU_MPC83XX) += mpc83xx_cpu.o
+obj-$(CONFIG_CPU_RISCV) += riscv_cpu.o
obj-$(CONFIG_SANDBOX) += cpu_sandbox.o
diff --git a/drivers/cpu/riscv_cpu.c b/drivers/cpu/riscv_cpu.c
new file mode 100644
index 0000000..23917db
--- /dev/null
+++ b/drivers/cpu/riscv_cpu.c
@@ -0,0 +1,117 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ */
+
+#include <common.h>
+#include <cpu.h>
+#include <dm.h>
+#include <errno.h>
+#include <dm/device-internal.h>
+#include <dm/lists.h>
+
+static int riscv_cpu_get_desc(struct udevice *dev, char *buf, int size)
+{
+ const char *isa;
+
+ isa = dev_read_string(dev, "riscv,isa");
+ if (size < (strlen(isa) + 1))
+ return -ENOSPC;
+
+ strcpy(buf, isa);
+
+ return 0;
+}
+
+static int riscv_cpu_get_info(struct udevice *dev, struct cpu_info *info)
+{
+ const char *mmu;
+
+ dev_read_u32(dev, "clock-frequency", (u32 *)&info->cpu_freq);
+
+ mmu = dev_read_string(dev, "mmu-type");
+ if (!mmu)
+ info->features |= BIT(CPU_FEAT_MMU);
+

BBL also disables CPUs without an MMU, so it is good to have this
information. Where would you disable the CPU in U-Boot? Maybe in
arch_fixup_fdt() or in the CPU driver?

Post by Bin Meng
+ return 0;
+}
+
+static int riscv_cpu_get_count(struct udevice *dev)
+{
+ ofnode node;
+ int num = 0;
+
+ ofnode_for_each_subnode(node, dev_ofnode(dev->parent)) {
+ const char *device_type;
+
+ device_type = ofnode_read_string(node, "device_type");
+ if (!device_type)
+ continue;
+ if (strcmp(device_type, "cpu") == 0)
+ num++;
+ }
+
+ return num;
+}
+
+static int riscv_cpu_bind(struct udevice *dev)
+{
+ struct cpu_platdata *plat = dev_get_parent_platdata(dev);
+ struct driver *drv;
+ struct udevice *timer;
+ int ret;
+
+ /* save the hart id */
+ plat->cpu_id = dev_read_addr(dev);
+
+ /* first examine the property in current cpu node */
+ ret = dev_read_u32(dev, "timebase-frequency", &plat-

Post by Bin Meng
timebase_freq);

+ /* if not found, then look at the parent /cpus node */
+ if (ret)
+ dev_read_u32(dev->parent, "timebase-frequency",
+ &plat->timebase_freq);
+
+ /*
+ * Bind riscv-timer driver on hart 0
+ *
+ * We only instantiate one timer device which is enough for U-
Boot.
+ * Pass the "timebase-frequency" value as the driver data for the
+ * timer device.
+ *
+ * Return value is not checked since it's possible that the
timer
+ * driver is not included.
+ */
+ if (!plat->cpu_id && plat->timebase_freq) {
+ drv = lists_driver_lookup_name("riscv_timer");
+ if (!drv) {
+ debug("Cannot find the timer driver, not
included?\n");
+ return 0;
+ }
+
+ device_bind_with_driver_data(dev, drv, "riscv_timer",
+ plat->timebase_freq,
ofnode_null(),
+ &timer);

You don't need the timer variable here, you can just pass NULL.

I did not see that you are not checking the return value here, when I
wrote my previous email regarding the syscon driver. So it is not
actually a problem if two different device implement the functionality
for the syscon APIs. I think it is still worth considering to implement
something like the misc driver I suggested, however.

Thanks,
Lukas

Post by Bin Meng
+ }
+
+ return 0;
+}
+
+static const struct cpu_ops riscv_cpu_ops = {
+ .get_desc = riscv_cpu_get_desc,
+ .get_info = riscv_cpu_get_info,
+ .get_count = riscv_cpu_get_count,
+};
+
+static const struct udevice_id riscv_cpu_ids[] = {
+ { .compatible = "riscv" },
+ { }
+};
+
+U_BOOT_DRIVER(riscv_cpu) = {
+ .name = "riscv_cpu",
+ .id = UCLASS_CPU,
+ .of_match = riscv_cpu_ids,
+ .bind = riscv_cpu_bind,
+ .ops = &riscv_cpu_ops,
+ .flags = DM_FLAG_PRE_RELOC,
+};

Bin Meng

2018-12-07 13:59:44 UTC

Hi Lukas,

On Thu, Nov 15, 2018 at 5:57 AM Auer, Lukas

Post by Auer, Lukas
Hi Bin,

Post by Bin Meng
This adds a driver for RISC-V CPU. Note the driver will bind
a RISC-V timer driver if "timebase-frequency" property is
present in the device tree.
---

Since we have the CPU driver, we could also enable CMD_CPU.

Agreed. Will do in v2.

Post by Auer, Lukas

Post by Bin Meng
drivers/cpu/Kconfig | 6 +++
drivers/cpu/Makefile | 1 +
drivers/cpu/riscv_cpu.c | 117
++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 124 insertions(+)
create mode 100644 drivers/cpu/riscv_cpu.c
diff --git a/drivers/cpu/Kconfig b/drivers/cpu/Kconfig
index d405200..3d5729f 100644
--- a/drivers/cpu/Kconfig
+++ b/drivers/cpu/Kconfig
@@ -13,3 +13,9 @@ config CPU_MPC83XX
select CLK_MPC83XX
help
Support CPU cores for SoCs of the MPC83xx series.
+
+config CPU_RISCV
+ bool "Enable RISC-V CPU driver"
+ depends on CPU && RISCV
+ help
+ Support CPU cores for RISC-V architecture.
diff --git a/drivers/cpu/Makefile b/drivers/cpu/Makefile
index 858b037..be0300c 100644
--- a/drivers/cpu/Makefile
+++ b/drivers/cpu/Makefile
@@ -8,4 +8,5 @@ obj-$(CONFIG_CPU) += cpu-uclass.o
obj-$(CONFIG_ARCH_BMIPS) += bmips_cpu.o
obj-$(CONFIG_CPU_MPC83XX) += mpc83xx_cpu.o
+obj-$(CONFIG_CPU_RISCV) += riscv_cpu.o
obj-$(CONFIG_SANDBOX) += cpu_sandbox.o
diff --git a/drivers/cpu/riscv_cpu.c b/drivers/cpu/riscv_cpu.c
new file mode 100644
index 0000000..23917db
--- /dev/null
+++ b/drivers/cpu/riscv_cpu.c
@@ -0,0 +1,117 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ */
+
+#include <common.h>
+#include <cpu.h>
+#include <dm.h>
+#include <errno.h>
+#include <dm/device-internal.h>
+#include <dm/lists.h>
+
+static int riscv_cpu_get_desc(struct udevice *dev, char *buf, int size)
+{
+ const char *isa;
+
+ isa = dev_read_string(dev, "riscv,isa");
+ if (size < (strlen(isa) + 1))
+ return -ENOSPC;
+
+ strcpy(buf, isa);
+
+ return 0;
+}
+
+static int riscv_cpu_get_info(struct udevice *dev, struct cpu_info *info)
+{
+ const char *mmu;
+
+ dev_read_u32(dev, "clock-frequency", (u32 *)&info->cpu_freq);
+
+ mmu = dev_read_string(dev, "mmu-type");
+ if (!mmu)
+ info->features |= BIT(CPU_FEAT_MMU);
+

BBL also disables CPUs without an MMU, so it is good to have this
information. Where would you disable the CPU in U-Boot? Maybe in
arch_fixup_fdt() or in the CPU driver?

I think disabling CPU only needs to be done if booting to S-mode
payload. If booting to M-mode payload we should leave it as it is.
arch_fixup_fdt() can be a good place to fix up these sort of things
but I think that should be another patch.

Post by Auer, Lukas

Post by Bin Meng
+ return 0;
+}
+
+static int riscv_cpu_get_count(struct udevice *dev)
+{
+ ofnode node;
+ int num = 0;
+
+ ofnode_for_each_subnode(node, dev_ofnode(dev->parent)) {
+ const char *device_type;
+
+ device_type = ofnode_read_string(node, "device_type");
+ if (!device_type)
+ continue;
+ if (strcmp(device_type, "cpu") == 0)
+ num++;
+ }
+
+ return num;
+}
+
+static int riscv_cpu_bind(struct udevice *dev)
+{
+ struct cpu_platdata *plat = dev_get_parent_platdata(dev);
+ struct driver *drv;
+ struct udevice *timer;
+ int ret;
+
+ /* save the hart id */
+ plat->cpu_id = dev_read_addr(dev);
+
+ /* first examine the property in current cpu node */
+ ret = dev_read_u32(dev, "timebase-frequency", &plat-

Post by Bin Meng
timebase_freq);

+ /* if not found, then look at the parent /cpus node */
+ if (ret)
+ dev_read_u32(dev->parent, "timebase-frequency",
+ &plat->timebase_freq);
+
+ /*
+ * Bind riscv-timer driver on hart 0
+ *
+ * We only instantiate one timer device which is enough for U-
Boot.
+ * Pass the "timebase-frequency" value as the driver data for
the
+ * timer device.
+ *
+ * Return value is not checked since it's possible that the
timer
+ * driver is not included.
+ */
+ if (!plat->cpu_id && plat->timebase_freq) {
+ drv = lists_driver_lookup_name("riscv_timer");
+ if (!drv) {
+ debug("Cannot find the timer driver, not
included?\n");
+ return 0;
+ }
+
+ device_bind_with_driver_data(dev, drv, "riscv_timer",
+ plat->timebase_freq,
ofnode_null(),
+ &timer);

You don't need the timer variable here, you can just pass NULL.

Yes, smarter!

Post by Auer, Lukas
I did not see that you are not checking the return value here, when I
wrote my previous email regarding the syscon driver. So it is not
actually a problem if two different device implement the functionality
for the syscon APIs. I think it is still worth considering to implement
something like the misc driver I suggested, however.

Regards,
Bin

Auer, Lukas

2018-12-11 00:03:36 UTC

Hi Bin,

Post by Bin Meng
Hi Lukas,
On Thu, Nov 15, 2018 at 5:57 AM Auer, Lukas

Post by Auer, Lukas
Hi Bin,

Post by Bin Meng
This adds a driver for RISC-V CPU. Note the driver will bind
a RISC-V timer driver if "timebase-frequency" property is
present in the device tree.
---

Since we have the CPU driver, we could also enable CMD_CPU.

Agreed. Will do in v2.

Post by Auer, Lukas

Post by Bin Meng
drivers/cpu/Kconfig | 6 +++
drivers/cpu/Makefile | 1 +
drivers/cpu/riscv_cpu.c | 117
++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 124 insertions(+)
create mode 100644 drivers/cpu/riscv_cpu.c
diff --git a/drivers/cpu/Kconfig b/drivers/cpu/Kconfig
index d405200..3d5729f 100644
--- a/drivers/cpu/Kconfig
+++ b/drivers/cpu/Kconfig
@@ -13,3 +13,9 @@ config CPU_MPC83XX
select CLK_MPC83XX
help
Support CPU cores for SoCs of the MPC83xx series.
+
+config CPU_RISCV
+ bool "Enable RISC-V CPU driver"
+ depends on CPU && RISCV
+ help
+ Support CPU cores for RISC-V architecture.
diff --git a/drivers/cpu/Makefile b/drivers/cpu/Makefile
index 858b037..be0300c 100644
--- a/drivers/cpu/Makefile
+++ b/drivers/cpu/Makefile
@@ -8,4 +8,5 @@ obj-$(CONFIG_CPU) += cpu-uclass.o
obj-$(CONFIG_ARCH_BMIPS) += bmips_cpu.o
obj-$(CONFIG_CPU_MPC83XX) += mpc83xx_cpu.o
+obj-$(CONFIG_CPU_RISCV) += riscv_cpu.o
obj-$(CONFIG_SANDBOX) += cpu_sandbox.o
diff --git a/drivers/cpu/riscv_cpu.c b/drivers/cpu/riscv_cpu.c
new file mode 100644
index 0000000..23917db
--- /dev/null
+++ b/drivers/cpu/riscv_cpu.c
@@ -0,0 +1,117 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ */
+
+#include <common.h>
+#include <cpu.h>
+#include <dm.h>
+#include <errno.h>
+#include <dm/device-internal.h>
+#include <dm/lists.h>
+
+static int riscv_cpu_get_desc(struct udevice *dev, char *buf,
int
size)
+{
+ const char *isa;
+
+ isa = dev_read_string(dev, "riscv,isa");
+ if (size < (strlen(isa) + 1))
+ return -ENOSPC;
+
+ strcpy(buf, isa);
+
+ return 0;
+}
+
+static int riscv_cpu_get_info(struct udevice *dev, struct
cpu_info
*info)
+{
+ const char *mmu;
+
+ dev_read_u32(dev, "clock-frequency", (u32 *)&info-

Post by Bin Meng
cpu_freq);

+
+ mmu = dev_read_string(dev, "mmu-type");
+ if (!mmu)
+ info->features |= BIT(CPU_FEAT_MMU);
+

BBL also disables CPUs without an MMU, so it is good to have this
information. Where would you disable the CPU in U-Boot? Maybe in
arch_fixup_fdt() or in the CPU driver?

I think disabling CPU only needs to be done if booting to S-mode
payload. If booting to M-mode payload we should leave it as it is.
arch_fixup_fdt() can be a good place to fix up these sort of things
but I think that should be another patch.

Makes sense. At the moment this is done in BBL anyways, so this is
something we only have to add once we have the OpenSBI.

Thanks,
Lukas

Post by Bin Meng

Post by Auer, Lukas

Post by Bin Meng
+ return 0;
+}
+
+static int riscv_cpu_get_count(struct udevice *dev)
+{
+ ofnode node;
+ int num = 0;
+
+ ofnode_for_each_subnode(node, dev_ofnode(dev->parent)) {
+ const char *device_type;
+
+ device_type = ofnode_read_string(node,
"device_type");
+ if (!device_type)
+ continue;
+ if (strcmp(device_type, "cpu") == 0)
+ num++;
+ }
+
+ return num;
+}
+
+static int riscv_cpu_bind(struct udevice *dev)
+{
+ struct cpu_platdata *plat = dev_get_parent_platdata(dev);
+ struct driver *drv;
+ struct udevice *timer;
+ int ret;
+
+ /* save the hart id */
+ plat->cpu_id = dev_read_addr(dev);
+
+ /* first examine the property in current cpu node */
+ ret = dev_read_u32(dev, "timebase-frequency", &plat-

Post by Bin Meng
timebase_freq);

+ /* if not found, then look at the parent /cpus node */
+ if (ret)
+ dev_read_u32(dev->parent, "timebase-frequency",
+ &plat->timebase_freq);
+
+ /*
+ * Bind riscv-timer driver on hart 0
+ *
+ * We only instantiate one timer device which is enough for U-
Boot.
+ * Pass the "timebase-frequency" value as the driver data for
the
+ * timer device.
+ *
+ * Return value is not checked since it's possible that the
timer
+ * driver is not included.
+ */
+ if (!plat->cpu_id && plat->timebase_freq) {
+ drv = lists_driver_lookup_name("riscv_timer");
+ if (!drv) {
+ debug("Cannot find the timer driver, not
included?\n");
+ return 0;
+ }
+
+ device_bind_with_driver_data(dev, drv,
"riscv_timer",
+ plat->timebase_freq,
ofnode_null(),
+ &timer);

You don't need the timer variable here, you can just pass NULL.

Yes, smarter!

Post by Auer, Lukas
I did not see that you are not checking the return value here, when I
wrote my previous email regarding the syscon driver. So it is not
actually a problem if two different device implement the
functionality
for the syscon APIs. I think it is still worth considering to implement
something like the misc driver I suggested, however.

Regards,
Bin

Bin Meng

2018-11-13 08:22:01 UTC

Currently the M-mode trap handler codes are in start.S. For future
extension, move them to a separate file mtrap.S.

Signed-off-by: Bin Meng <***@gmail.com>
---

arch/riscv/cpu/Makefile | 2 +-
arch/riscv/cpu/mtrap.S | 106 ++++++++++++++++++++++++++++++++++++++++++++++++
arch/riscv/cpu/start.S | 82 -------------------------------------
3 files changed, 107 insertions(+), 83 deletions(-)
create mode 100644 arch/riscv/cpu/mtrap.S

diff --git a/arch/riscv/cpu/Makefile b/arch/riscv/cpu/Makefile
index 2cc6757..6bf6f91 100644
--- a/arch/riscv/cpu/Makefile
+++ b/arch/riscv/cpu/Makefile
@@ -4,4 +4,4 @@

extra-y = start.o

-obj-y += cpu.o
+obj-y += cpu.o mtrap.o
diff --git a/arch/riscv/cpu/mtrap.S b/arch/riscv/cpu/mtrap.S
new file mode 100644
index 0000000..ba6462f
--- /dev/null
+++ b/arch/riscv/cpu/mtrap.S
@@ -0,0 +1,106 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * M-mode Trap Handler Code for RISC-V Core
+ *
+ * Copyright (c) 2017 Microsemi Corporation.
+ * Copyright (c) 2017 Padmarao Begari <***@microsemi.com>
+ *
+ * Copyright (C) 2017 Andes Technology Corporation
+ * Rick Chen, Andes Technology Corporation <***@andestech.com>
+ *
+ * Copyright (C) 2018, Bin Meng <***@gmail.com>
+ */
+
+#include <common.h>
+#include <asm/encoding.h>
+
+#ifdef CONFIG_32BIT
+#define LREG lw
+#define SREG sw
+#define REGBYTES 4
+#else
+#define LREG ld
+#define SREG sd
+#define REGBYTES 8
+#endif
+
+ .text
+
+ /* trap entry */
+ .align 2
+ .global trap_entry
+trap_entry:
+ addi sp, sp, -32 * REGBYTES
+ SREG x1, 1 * REGBYTES(sp)
+ SREG x2, 2 * REGBYTES(sp)
+ SREG x3, 3 * REGBYTES(sp)
+ SREG x4, 4 * REGBYTES(sp)
+ SREG x5, 5 * REGBYTES(sp)
+ SREG x6, 6 * REGBYTES(sp)
+ SREG x7, 7 * REGBYTES(sp)
+ SREG x8, 8 * REGBYTES(sp)
+ SREG x9, 9 * REGBYTES(sp)
+ SREG x10, 10 * REGBYTES(sp)
+ SREG x11, 11 * REGBYTES(sp)
+ SREG x12, 12 * REGBYTES(sp)
+ SREG x13, 13 * REGBYTES(sp)
+ SREG x14, 14 * REGBYTES(sp)
+ SREG x15, 15 * REGBYTES(sp)
+ SREG x16, 16 * REGBYTES(sp)
+ SREG x17, 17 * REGBYTES(sp)
+ SREG x18, 18 * REGBYTES(sp)
+ SREG x19, 19 * REGBYTES(sp)
+ SREG x20, 20 * REGBYTES(sp)
+ SREG x21, 21 * REGBYTES(sp)
+ SREG x22, 22 * REGBYTES(sp)
+ SREG x23, 23 * REGBYTES(sp)
+ SREG x24, 24 * REGBYTES(sp)
+ SREG x25, 25 * REGBYTES(sp)
+ SREG x26, 26 * REGBYTES(sp)
+ SREG x27, 27 * REGBYTES(sp)
+ SREG x28, 28 * REGBYTES(sp)
+ SREG x29, 29 * REGBYTES(sp)
+ SREG x30, 30 * REGBYTES(sp)
+ SREG x31, 31 * REGBYTES(sp)
+ csrr a0, mcause
+ csrr a1, mepc
+ mv a2, sp
+ jal handle_trap
+ csrw mepc, a0
+
+ /* Remain in M-mode after mret */
+ li t0, MSTATUS_MPP
+ csrs mstatus, t0
+ LREG x1, 1 * REGBYTES(sp)
+ LREG x2, 2 * REGBYTES(sp)
+ LREG x3, 3 * REGBYTES(sp)
+ LREG x4, 4 * REGBYTES(sp)
+ LREG x5, 5 * REGBYTES(sp)
+ LREG x6, 6 * REGBYTES(sp)
+ LREG x7, 7 * REGBYTES(sp)
+ LREG x8, 8 * REGBYTES(sp)
+ LREG x9, 9 * REGBYTES(sp)
+ LREG x10, 10 * REGBYTES(sp)
+ LREG x11, 11 * REGBYTES(sp)
+ LREG x12, 12 * REGBYTES(sp)
+ LREG x13, 13 * REGBYTES(sp)
+ LREG x14, 14 * REGBYTES(sp)
+ LREG x15, 15 * REGBYTES(sp)
+ LREG x16, 16 * REGBYTES(sp)
+ LREG x17, 17 * REGBYTES(sp)
+ LREG x18, 18 * REGBYTES(sp)
+ LREG x19, 19 * REGBYTES(sp)
+ LREG x20, 20 * REGBYTES(sp)
+ LREG x21, 21 * REGBYTES(sp)
+ LREG x22, 22 * REGBYTES(sp)
+ LREG x23, 23 * REGBYTES(sp)
+ LREG x24, 24 * REGBYTES(sp)
+ LREG x25, 25 * REGBYTES(sp)
+ LREG x26, 26 * REGBYTES(sp)
+ LREG x27, 27 * REGBYTES(sp)
+ LREG x28, 28 * REGBYTES(sp)
+ LREG x29, 29 * REGBYTES(sp)
+ LREG x30, 30 * REGBYTES(sp)
+ LREG x31, 31 * REGBYTES(sp)
+ addi sp, sp, 32 * REGBYTES
+ mret
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
index 331a534..9858058 100644
--- a/arch/riscv/cpu/start.S
+++ b/arch/riscv/cpu/start.S
@@ -194,85 +194,3 @@ call_board_init_r:
* jump to it ...
*/
jr t4 /* jump to board_init_r() */
-
-/*
- * trap entry
- */
-.align 2
-trap_entry:
- addi sp, sp, -32*REGBYTES
- SREG x1, 1*REGBYTES(sp)
- SREG x2, 2*REGBYTES(sp)
- SREG x3, 3*REGBYTES(sp)
- SREG x4, 4*REGBYTES(sp)
- SREG x5, 5*REGBYTES(sp)
- SREG x6, 6*REGBYTES(sp)
- SREG x7, 7*REGBYTES(sp)
- SREG x8, 8*REGBYTES(sp)
- SREG x9, 9*REGBYTES(sp)
- SREG x10, 10*REGBYTES(sp)
- SREG x11, 11*REGBYTES(sp)
- SREG x12, 12*REGBYTES(sp)
- SREG x13, 13*REGBYTES(sp)
- SREG x14, 14*REGBYTES(sp)
- SREG x15, 15*REGBYTES(sp)
- SREG x16, 16*REGBYTES(sp)
- SREG x17, 17*REGBYTES(sp)
- SREG x18, 18*REGBYTES(sp)
- SREG x19, 19*REGBYTES(sp)
- SREG x20, 20*REGBYTES(sp)
- SREG x21, 21*REGBYTES(sp)
- SREG x22, 22*REGBYTES(sp)
- SREG x23, 23*REGBYTES(sp)
- SREG x24, 24*REGBYTES(sp)
- SREG x25, 25*REGBYTES(sp)
- SREG x26, 26*REGBYTES(sp)
- SREG x27, 27*REGBYTES(sp)
- SREG x28, 28*REGBYTES(sp)
- SREG x29, 29*REGBYTES(sp)
- SREG x30, 30*REGBYTES(sp)
- SREG x31, 31*REGBYTES(sp)
- csrr a0, mcause
- csrr a1, mepc
- mv a2, sp
- jal handle_trap
- csrw mepc, a0
-
-/*
- * Remain in M-mode after mret
- */
- li t0, MSTATUS_MPP
- csrs mstatus, t0
- LREG x1, 1*REGBYTES(sp)
- LREG x2, 2*REGBYTES(sp)
- LREG x3, 3*REGBYTES(sp)
- LREG x4, 4*REGBYTES(sp)
- LREG x5, 5*REGBYTES(sp)
- LREG x6, 6*REGBYTES(sp)
- LREG x7, 7*REGBYTES(sp)
- LREG x8, 8*REGBYTES(sp)
- LREG x9, 9*REGBYTES(sp)
- LREG x10, 10*REGBYTES(sp)
- LREG x11, 11*REGBYTES(sp)
- LREG x12, 12*REGBYTES(sp)
- LREG x13, 13*REGBYTES(sp)
- LREG x14, 14*REGBYTES(sp)
- LREG x15, 15*REGBYTES(sp)
- LREG x16, 16*REGBYTES(sp)
- LREG x17, 17*REGBYTES(sp)
- LREG x18, 18*REGBYTES(sp)
- LREG x19, 19*REGBYTES(sp)
- LREG x20, 20*REGBYTES(sp)
- LREG x21, 21*REGBYTES(sp)
- LREG x22, 22*REGBYTES(sp)
- LREG x23, 23*REGBYTES(sp)
- LREG x24, 24*REGBYTES(sp)
- LREG x25, 25*REGBYTES(sp)
- LREG x26, 26*REGBYTES(sp)
- LREG x27, 27*REGBYTES(sp)
- LREG x28, 28*REGBYTES(sp)
- LREG x29, 29*REGBYTES(sp)
- LREG x30, 30*REGBYTES(sp)
- LREG x31, 31*REGBYTES(sp)
- addi sp, sp, 32*REGBYTES
- mret

--
2.7.4

Auer, Lukas

2018-11-14 22:44:53 UTC

Post by Bin Meng
Currently the M-mode trap handler codes are in start.S. For future
extension, move them to a separate file mtrap.S.
---
arch/riscv/cpu/Makefile | 2 +-
arch/riscv/cpu/mtrap.S | 106
++++++++++++++++++++++++++++++++++++++++++++++++
arch/riscv/cpu/start.S | 82 -------------------------------------
3 files changed, 107 insertions(+), 83 deletions(-)
create mode 100644 arch/riscv/cpu/mtrap.S

Reviewed-by: Lukas Auer <***@aisec.fraunhofer.de>

Bin Meng

2018-11-13 08:21:51 UTC

To enumerate devices on the /soc/ node, create a "simple-bus"
driver to match "riscv-virtio-soc".

Signed-off-by: Bin Meng <***@gmail.com>
---

arch/riscv/cpu/qemu/cpu.c | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/arch/riscv/cpu/qemu/cpu.c b/arch/riscv/cpu/qemu/cpu.c
index 6c7a327..221f3a8 100644
--- a/arch/riscv/cpu/qemu/cpu.c
+++ b/arch/riscv/cpu/qemu/cpu.c
@@ -4,6 +4,7 @@
*/

#include <common.h>
+#include <dm.h>

/*
* cleanup_before_linux() is called just before we call linux
@@ -19,3 +20,15 @@ int cleanup_before_linux(void)

return 0;
}
+
+/* To enumerate devices on the /soc/ node, create a "simple-bus" driver */
+static const struct udevice_id riscv_virtio_soc_ids[] = {
+ { .compatible = "riscv-virtio-soc" },
+ { }
+};
+
+U_BOOT_DRIVER(riscv_virtio_soc) = {
+ .name = "riscv_virtio_soc",
+ .id = UCLASS_SIMPLE_BUS,
+ .of_match = riscv_virtio_soc_ids,
+};

--
2.7.4

Auer, Lukas

2018-11-14 21:26:26 UTC

Hi Bin,

Post by Bin Meng
To enumerate devices on the /soc/ node, create a "simple-bus"
driver to match "riscv-virtio-soc".
---
arch/riscv/cpu/qemu/cpu.c | 13 +++++++++++++
1 file changed, 13 insertions(+)

Reviewed-by: Lukas Auer <***@aisec.fraunhofer.de>

Would it makes sense to move this to cpu/ to make this driver available
to all RISC-V CPUs? I think most CPUs will need this driver to make
devices under the soc/ node available before relocation.

Post by Bin Meng
diff --git a/arch/riscv/cpu/qemu/cpu.c b/arch/riscv/cpu/qemu/cpu.c
index 6c7a327..221f3a8 100644
--- a/arch/riscv/cpu/qemu/cpu.c
+++ b/arch/riscv/cpu/qemu/cpu.c
@@ -4,6 +4,7 @@
*/
#include <common.h>
+#include <dm.h>
/*
* cleanup_before_linux() is called just before we call linux
@@ -19,3 +20,15 @@ int cleanup_before_linux(void)
return 0;
}
+
+/* To enumerate devices on the /soc/ node, create a "simple-bus" driver */
+static const struct udevice_id riscv_virtio_soc_ids[] = {
+ { .compatible = "riscv-virtio-soc" },
+ { }
+};
+
+U_BOOT_DRIVER(riscv_virtio_soc) = {
+ .name = "riscv_virtio_soc",
+ .id = UCLASS_SIMPLE_BUS,
+ .of_match = riscv_virtio_soc_ids,
+};

I think the DM_FLAG_PRE_RELOC flag should be set, since it is set for
the syscon driver for the clint0.

Thanks,
Lukas

Bin Meng

2018-11-30 09:47:58 UTC

Hi Lukas,

On Thu, Nov 15, 2018 at 5:26 AM Auer, Lukas

Post by Auer, Lukas
Hi Bin,

Post by Bin Meng
To enumerate devices on the /soc/ node, create a "simple-bus"
driver to match "riscv-virtio-soc".
---
arch/riscv/cpu/qemu/cpu.c | 13 +++++++++++++
1 file changed, 13 insertions(+)

Would it makes sense to move this to cpu/ to make this driver available
to all RISC-V CPUs? I think most CPUs will need this driver to make
devices under the soc/ node available before relocation.

I suspect this can apply to other RISC-V CPUs because it compatible
string says: "riscv-virtio-soc"

Post by Auer, Lukas

Post by Bin Meng
diff --git a/arch/riscv/cpu/qemu/cpu.c b/arch/riscv/cpu/qemu/cpu.c
index 6c7a327..221f3a8 100644
--- a/arch/riscv/cpu/qemu/cpu.c
+++ b/arch/riscv/cpu/qemu/cpu.c
@@ -4,6 +4,7 @@
*/
#include <common.h>
+#include <dm.h>
/*
* cleanup_before_linux() is called just before we call linux
@@ -19,3 +20,15 @@ int cleanup_before_linux(void)
return 0;
}
+
+/* To enumerate devices on the /soc/ node, create a "simple-bus" driver */
+static const struct udevice_id riscv_virtio_soc_ids[] = {
+ { .compatible = "riscv-virtio-soc" },
+ { }
+};
+
+U_BOOT_DRIVER(riscv_virtio_soc) = {
+ .name = "riscv_virtio_soc",
+ .id = UCLASS_SIMPLE_BUS,
+ .of_match = riscv_virtio_soc_ids,
+};

I think the DM_FLAG_PRE_RELOC flag should be set, since it is set for
the syscon driver for the clint0.

Will fix in v2.

Regards,
Bin

Bin Meng

2018-11-13 08:22:02 UTC

sp cannot be loaded before restoring other registers.

Signed-off-by: Bin Meng <***@gmail.com>
---

arch/riscv/cpu/mtrap.S | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/cpu/mtrap.S b/arch/riscv/cpu/mtrap.S
index ba6462f..6c0eac6 100644
--- a/arch/riscv/cpu/mtrap.S
+++ b/arch/riscv/cpu/mtrap.S
@@ -72,7 +72,6 @@ trap_entry:
li t0, MSTATUS_MPP
csrs mstatus, t0
LREG x1, 1 * REGBYTES(sp)
- LREG x2, 2 * REGBYTES(sp)
LREG x3, 3 * REGBYTES(sp)
LREG x4, 4 * REGBYTES(sp)
LREG x5, 5 * REGBYTES(sp)
@@ -102,5 +101,6 @@ trap_entry:
LREG x29, 29 * REGBYTES(sp)
LREG x30, 30 * REGBYTES(sp)
LREG x31, 31 * REGBYTES(sp)
+ LREG x2, 2 * REGBYTES(sp)
addi sp, sp, 32 * REGBYTES
mret

--
2.7.4

Auer, Lukas

2018-11-14 22:46:46 UTC

Post by Bin Meng
sp cannot be loaded before restoring other registers.
---
arch/riscv/cpu/mtrap.S | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Lukas Auer <***@aisec.fraunhofer.de>

Good catch!

Post by Bin Meng
diff --git a/arch/riscv/cpu/mtrap.S b/arch/riscv/cpu/mtrap.S
index ba6462f..6c0eac6 100644
--- a/arch/riscv/cpu/mtrap.S
+++ b/arch/riscv/cpu/mtrap.S
li t0, MSTATUS_MPP
csrs mstatus, t0
LREG x1, 1 * REGBYTES(sp)
- LREG x2, 2 * REGBYTES(sp)
LREG x3, 3 * REGBYTES(sp)
LREG x4, 4 * REGBYTES(sp)
LREG x5, 5 * REGBYTES(sp)
LREG x29, 29 * REGBYTES(sp)
LREG x30, 30 * REGBYTES(sp)
LREG x31, 31 * REGBYTES(sp)
+ LREG x2, 2 * REGBYTES(sp)
addi sp, sp, 32 * REGBYTES
mret

Bin Meng

2018-11-13 08:21:58 UTC

The standard RISC-V ISA sets aside a 12-bit encoding space for up
to 4096 CSRs. This adds all known CSR numbers as defined in the
RISC-V Privileged Architecture Version 1.10.

Signed-off-by: Bin Meng <***@gmail.com>
---

arch/riscv/include/asm/encoding.h | 219 ++++++++++++++++++++++++++++++++++++++
1 file changed, 219 insertions(+)

diff --git a/arch/riscv/include/asm/encoding.h b/arch/riscv/include/asm/encoding.h
index 9ea50ce..0c47c53 100644
--- a/arch/riscv/include/asm/encoding.h
+++ b/arch/riscv/include/asm/encoding.h
@@ -146,6 +146,225 @@
#define RISCV_PGSHIFT 12
#define RISCV_PGSIZE BIT(RISCV_PGSHIFT)

+/* CSR numbers */
+#define CSR_FFLAGS 0x1
+#define CSR_FRM 0x2
+#define CSR_FCSR 0x3
+
+#define CSR_SSTATUS 0x100
+#define CSR_SIE 0x104
+#define CSR_STVEC 0x105
+#define CSR_SCOUNTEREN 0x106
+#define CSR_SSCRATCH 0x140
+#define CSR_SEPC 0x141
+#define CSR_SCAUSE 0x142
+#define CSR_STVAL 0x143
+#define CSR_SIP 0x144
+#define CSR_SATP 0x180
+
+#define CSR_MSTATUS 0x300
+#define CSR_MISA 0x301
+#define CSR_MEDELEG 0x302
+#define CSR_MIDELEG 0x303
+#define CSR_MIE 0x304
+#define CSR_MTVEC 0x305
+#define CSR_MCOUNTEREN 0x306
+#define CSR_MHPMEVENT3 0x323
+#define CSR_MHPMEVENT4 0x324
+#define CSR_MHPMEVENT5 0x325
+#define CSR_MHPMEVENT6 0x326
+#define CSR_MHPMEVENT7 0x327
+#define CSR_MHPMEVENT8 0x328
+#define CSR_MHPMEVENT9 0x329
+#define CSR_MHPMEVENT10 0x32a
+#define CSR_MHPMEVENT11 0x32b
+#define CSR_MHPMEVENT12 0x32c
+#define CSR_MHPMEVENT13 0x32d
+#define CSR_MHPMEVENT14 0x32e
+#define CSR_MHPMEVENT15 0x32f
+#define CSR_MHPMEVENT16 0x330
+#define CSR_MHPMEVENT17 0x331
+#define CSR_MHPMEVENT18 0x332
+#define CSR_MHPMEVENT19 0x333
+#define CSR_MHPMEVENT20 0x334
+#define CSR_MHPMEVENT21 0x335
+#define CSR_MHPMEVENT22 0x336
+#define CSR_MHPMEVENT23 0x337
+#define CSR_MHPMEVENT24 0x338
+#define CSR_MHPMEVENT25 0x339
+#define CSR_MHPMEVENT26 0x33a
+#define CSR_MHPMEVENT27 0x33b
+#define CSR_MHPMEVENT28 0x33c
+#define CSR_MHPMEVENT29 0x33d
+#define CSR_MHPMEVENT30 0x33e
+#define CSR_MHPMEVENT31 0x33f
+#define CSR_MSCRATCH 0x340
+#define CSR_MEPC 0x341
+#define CSR_MCAUSE 0x342
+#define CSR_MTVAL 0x343
+#define CSR_MIP 0x344
+#define CSR_PMPCFG0 0x3a0
+#define CSR_PMPCFG1 0x3a1
+#define CSR_PMPCFG2 0x3a2
+#define CSR_PMPCFG3 0x3a3
+#define CSR_PMPADDR0 0x3b0
+#define CSR_PMPADDR1 0x3b1
+#define CSR_PMPADDR2 0x3b2
+#define CSR_PMPADDR3 0x3b3
+#define CSR_PMPADDR4 0x3b4
+#define CSR_PMPADDR5 0x3b5
+#define CSR_PMPADDR6 0x3b6
+#define CSR_PMPADDR7 0x3b7
+#define CSR_PMPADDR8 0x3b8
+#define CSR_PMPADDR9 0x3b9
+#define CSR_PMPADDR10 0x3ba
+#define CSR_PMPADDR11 0x3bb
+#define CSR_PMPADDR12 0x3bc
+#define CSR_PMPADDR13 0x3bd
+#define CSR_PMPADDR14 0x3be
+#define CSR_PMPADDR15 0x3bf
+
+#define CSR_TSELECT 0x7a0
+#define CSR_TDATA1 0x7a1
+#define CSR_TDATA2 0x7a2
+#define CSR_TDATA3 0x7a3
+#define CSR_DCSR 0x7b0
+#define CSR_DPC 0x7b1
+#define CSR_DSCRATCH 0x7b2
+
+#define CSR_MCYCLE 0xb00
+#define CSR_MINSTRET 0xb02
+#define CSR_MHPMCOUNTER3 0xb03
+#define CSR_MHPMCOUNTER4 0xb04
+#define CSR_MHPMCOUNTER5 0xb05
+#define CSR_MHPMCOUNTER6 0xb06
+#define CSR_MHPMCOUNTER7 0xb07
+#define CSR_MHPMCOUNTER8 0xb08
+#define CSR_MHPMCOUNTER9 0xb09
+#define CSR_MHPMCOUNTER10 0xb0a
+#define CSR_MHPMCOUNTER11 0xb0b
+#define CSR_MHPMCOUNTER12 0xb0c
+#define CSR_MHPMCOUNTER13 0xb0d
+#define CSR_MHPMCOUNTER14 0xb0e
+#define CSR_MHPMCOUNTER15 0xb0f
+#define CSR_MHPMCOUNTER16 0xb10
+#define CSR_MHPMCOUNTER17 0xb11
+#define CSR_MHPMCOUNTER18 0xb12
+#define CSR_MHPMCOUNTER19 0xb13
+#define CSR_MHPMCOUNTER20 0xb14
+#define CSR_MHPMCOUNTER21 0xb15
+#define CSR_MHPMCOUNTER22 0xb16
+#define CSR_MHPMCOUNTER23 0xb17
+#define CSR_MHPMCOUNTER24 0xb18
+#define CSR_MHPMCOUNTER25 0xb19
+#define CSR_MHPMCOUNTER26 0xb1a
+#define CSR_MHPMCOUNTER27 0xb1b
+#define CSR_MHPMCOUNTER28 0xb1c
+#define CSR_MHPMCOUNTER29 0xb1d
+#define CSR_MHPMCOUNTER30 0xb1e
+#define CSR_MHPMCOUNTER31 0xb1f
+#define CSR_MCYCLEH 0xb80
+#define CSR_MINSTRETH 0xb82
+#define CSR_MHPMCOUNTER3H 0xb83
+#define CSR_MHPMCOUNTER4H 0xb84
+#define CSR_MHPMCOUNTER5H 0xb85
+#define CSR_MHPMCOUNTER6H 0xb86
+#define CSR_MHPMCOUNTER7H 0xb87
+#define CSR_MHPMCOUNTER8H 0xb88
+#define CSR_MHPMCOUNTER9H 0xb89
+#define CSR_MHPMCOUNTER10H 0xb8a
+#define CSR_MHPMCOUNTER11H 0xb8b
+#define CSR_MHPMCOUNTER12H 0xb8c
+#define CSR_MHPMCOUNTER13H 0xb8d
+#define CSR_MHPMCOUNTER14H 0xb8e
+#define CSR_MHPMCOUNTER15H 0xb8f
+#define CSR_MHPMCOUNTER16H 0xb90
+#define CSR_MHPMCOUNTER17H 0xb91
+#define CSR_MHPMCOUNTER18H 0xb92
+#define CSR_MHPMCOUNTER19H 0xb93
+#define CSR_MHPMCOUNTER20H 0xb94
+#define CSR_MHPMCOUNTER21H 0xb95
+#define CSR_MHPMCOUNTER22H 0xb96
+#define CSR_MHPMCOUNTER23H 0xb97
+#define CSR_MHPMCOUNTER24H 0xb98
+#define CSR_MHPMCOUNTER25H 0xb99
+#define CSR_MHPMCOUNTER26H 0xb9a
+#define CSR_MHPMCOUNTER27H 0xb9b
+#define CSR_MHPMCOUNTER28H 0xb9c
+#define CSR_MHPMCOUNTER29H 0xb9d
+#define CSR_MHPMCOUNTER30H 0xb9e
+#define CSR_MHPMCOUNTER31H 0xb9f
+
+#define CSR_CYCLE 0xc00
+#define CSR_TIME 0xc01
+#define CSR_INSTRET 0xc02
+#define CSR_HPMCOUNTER3 0xc03
+#define CSR_HPMCOUNTER4 0xc04
+#define CSR_HPMCOUNTER5 0xc05
+#define CSR_HPMCOUNTER6 0xc06
+#define CSR_HPMCOUNTER7 0xc07
+#define CSR_HPMCOUNTER8 0xc08
+#define CSR_HPMCOUNTER9 0xc09
+#define CSR_HPMCOUNTER10 0xc0a
+#define CSR_HPMCOUNTER11 0xc0b
+#define CSR_HPMCOUNTER12 0xc0c
+#define CSR_HPMCOUNTER13 0xc0d
+#define CSR_HPMCOUNTER14 0xc0e
+#define CSR_HPMCOUNTER15 0xc0f
+#define CSR_HPMCOUNTER16 0xc10
+#define CSR_HPMCOUNTER17 0xc11
+#define CSR_HPMCOUNTER18 0xc12
+#define CSR_HPMCOUNTER19 0xc13
+#define CSR_HPMCOUNTER20 0xc14
+#define CSR_HPMCOUNTER21 0xc15
+#define CSR_HPMCOUNTER22 0xc16
+#define CSR_HPMCOUNTER23 0xc17
+#define CSR_HPMCOUNTER24 0xc18
+#define CSR_HPMCOUNTER25 0xc19
+#define CSR_HPMCOUNTER26 0xc1a
+#define CSR_HPMCOUNTER27 0xc1b
+#define CSR_HPMCOUNTER28 0xc1c
+#define CSR_HPMCOUNTER29 0xc1d
+#define CSR_HPMCOUNTER30 0xc1e
+#define CSR_HPMCOUNTER31 0xc1f
+#define CSR_CYCLEH 0xc80
+#define CSR_TIMEH 0xc81
+#define CSR_INSTRETH 0xc82
+#define CSR_HPMCOUNTER3H 0xc83
+#define CSR_HPMCOUNTER4H 0xc84
+#define CSR_HPMCOUNTER5H 0xc85
+#define CSR_HPMCOUNTER6H 0xc86
+#define CSR_HPMCOUNTER7H 0xc87
+#define CSR_HPMCOUNTER8H 0xc88
+#define CSR_HPMCOUNTER9H 0xc89
+#define CSR_HPMCOUNTER10H 0xc8a
+#define CSR_HPMCOUNTER11H 0xc8b
+#define CSR_HPMCOUNTER12H 0xc8c
+#define CSR_HPMCOUNTER13H 0xc8d
+#define CSR_HPMCOUNTER14H 0xc8e
+#define CSR_HPMCOUNTER15H 0xc8f
+#define CSR_HPMCOUNTER16H 0xc90
+#define CSR_HPMCOUNTER17H 0xc91
+#define CSR_HPMCOUNTER18H 0xc92
+#define CSR_HPMCOUNTER19H 0xc93
+#define CSR_HPMCOUNTER20H 0xc94
+#define CSR_HPMCOUNTER21H 0xc95
+#define CSR_HPMCOUNTER22H 0xc96
+#define CSR_HPMCOUNTER23H 0xc97
+#define CSR_HPMCOUNTER24H 0xc98
+#define CSR_HPMCOUNTER25H 0xc99
+#define CSR_HPMCOUNTER26H 0xc9a
+#define CSR_HPMCOUNTER27H 0xc9b
+#define CSR_HPMCOUNTER28H 0xc9c
+#define CSR_HPMCOUNTER29H 0xc9d
+#define CSR_HPMCOUNTER30H 0xc9e
+#define CSR_HPMCOUNTER31H 0xc9f
+
+#define CSR_MVENDORID 0xf11
+#define CSR_MARCHID 0xf12
+#define CSR_MIMPID 0xf13
+#define CSR_MHARTID 0xf14
+
#endif /* __riscv */

#endif /* RISCV_CSR_ENCODING_H */

--
2.7.4

Auer, Lukas

2018-11-14 22:26:24 UTC

Hi Bin,

Post by Bin Meng
The standard RISC-V ISA sets aside a 12-bit encoding space for up
to 4096 CSRs. This adds all known CSR numbers as defined in the
RISC-V Privileged Architecture Version 1.10.
---
arch/riscv/include/asm/encoding.h | 219
++++++++++++++++++++++++++++++++++++++
1 file changed, 219 insertions(+)

What is the reason for adding these and also the exception code
definitions in the next patch?

Thanks,
Lukas

Post by Bin Meng
diff --git a/arch/riscv/include/asm/encoding.h
b/arch/riscv/include/asm/encoding.h
index 9ea50ce..0c47c53 100644
--- a/arch/riscv/include/asm/encoding.h
+++ b/arch/riscv/include/asm/encoding.h
@@ -146,6 +146,225 @@
#define RISCV_PGSHIFT 12
#define RISCV_PGSIZE BIT(RISCV_PGSHIFT)
+/* CSR numbers */
+#define CSR_FFLAGS 0x1
+#define CSR_FRM 0x2
+#define CSR_FCSR 0x3
+
+#define CSR_SSTATUS 0x100
+#define CSR_SIE 0x104
+#define CSR_STVEC 0x105
+#define CSR_SCOUNTEREN 0x106
+#define CSR_SSCRATCH 0x140
+#define CSR_SEPC 0x141
+#define CSR_SCAUSE 0x142
+#define CSR_STVAL 0x143
+#define CSR_SIP 0x144
+#define CSR_SATP 0x180
+
+#define CSR_MSTATUS 0x300
+#define CSR_MISA 0x301
+#define CSR_MEDELEG 0x302
+#define CSR_MIDELEG 0x303
+#define CSR_MIE 0x304
+#define CSR_MTVEC 0x305
+#define CSR_MCOUNTEREN 0x306
+#define CSR_MHPMEVENT3 0x323
+#define CSR_MHPMEVENT4 0x324
+#define CSR_MHPMEVENT5 0x325
+#define CSR_MHPMEVENT6 0x326
+#define CSR_MHPMEVENT7 0x327
+#define CSR_MHPMEVENT8 0x328
+#define CSR_MHPMEVENT9 0x329
+#define CSR_MHPMEVENT10 0x32a
+#define CSR_MHPMEVENT11 0x32b
+#define CSR_MHPMEVENT12 0x32c
+#define CSR_MHPMEVENT13 0x32d
+#define CSR_MHPMEVENT14 0x32e
+#define CSR_MHPMEVENT15 0x32f
+#define CSR_MHPMEVENT16 0x330
+#define CSR_MHPMEVENT17 0x331
+#define CSR_MHPMEVENT18 0x332
+#define CSR_MHPMEVENT19 0x333
+#define CSR_MHPMEVENT20 0x334
+#define CSR_MHPMEVENT21 0x335
+#define CSR_MHPMEVENT22 0x336
+#define CSR_MHPMEVENT23 0x337
+#define CSR_MHPMEVENT24 0x338
+#define CSR_MHPMEVENT25 0x339
+#define CSR_MHPMEVENT26 0x33a
+#define CSR_MHPMEVENT27 0x33b
+#define CSR_MHPMEVENT28 0x33c
+#define CSR_MHPMEVENT29 0x33d
+#define CSR_MHPMEVENT30 0x33e
+#define CSR_MHPMEVENT31 0x33f
+#define CSR_MSCRATCH 0x340
+#define CSR_MEPC 0x341
+#define CSR_MCAUSE 0x342
+#define CSR_MTVAL 0x343
+#define CSR_MIP 0x344
+#define CSR_PMPCFG0 0x3a0
+#define CSR_PMPCFG1 0x3a1
+#define CSR_PMPCFG2 0x3a2
+#define CSR_PMPCFG3 0x3a3
+#define CSR_PMPADDR0 0x3b0
+#define CSR_PMPADDR1 0x3b1
+#define CSR_PMPADDR2 0x3b2
+#define CSR_PMPADDR3 0x3b3
+#define CSR_PMPADDR4 0x3b4
+#define CSR_PMPADDR5 0x3b5
+#define CSR_PMPADDR6 0x3b6
+#define CSR_PMPADDR7 0x3b7
+#define CSR_PMPADDR8 0x3b8
+#define CSR_PMPADDR9 0x3b9
+#define CSR_PMPADDR10 0x3ba
+#define CSR_PMPADDR11 0x3bb
+#define CSR_PMPADDR12 0x3bc
+#define CSR_PMPADDR13 0x3bd
+#define CSR_PMPADDR14 0x3be
+#define CSR_PMPADDR15 0x3bf
+
+#define CSR_TSELECT 0x7a0
+#define CSR_TDATA1 0x7a1
+#define CSR_TDATA2 0x7a2
+#define CSR_TDATA3 0x7a3
+#define CSR_DCSR 0x7b0
+#define CSR_DPC 0x7b1
+#define CSR_DSCRATCH 0x7b2
+
+#define CSR_MCYCLE 0xb00
+#define CSR_MINSTRET 0xb02
+#define CSR_MHPMCOUNTER3 0xb03
+#define CSR_MHPMCOUNTER4 0xb04
+#define CSR_MHPMCOUNTER5 0xb05
+#define CSR_MHPMCOUNTER6 0xb06
+#define CSR_MHPMCOUNTER7 0xb07
+#define CSR_MHPMCOUNTER8 0xb08
+#define CSR_MHPMCOUNTER9 0xb09
+#define CSR_MHPMCOUNTER10 0xb0a
+#define CSR_MHPMCOUNTER11 0xb0b
+#define CSR_MHPMCOUNTER12 0xb0c
+#define CSR_MHPMCOUNTER13 0xb0d
+#define CSR_MHPMCOUNTER14 0xb0e
+#define CSR_MHPMCOUNTER15 0xb0f
+#define CSR_MHPMCOUNTER16 0xb10
+#define CSR_MHPMCOUNTER17 0xb11
+#define CSR_MHPMCOUNTER18 0xb12
+#define CSR_MHPMCOUNTER19 0xb13
+#define CSR_MHPMCOUNTER20 0xb14
+#define CSR_MHPMCOUNTER21 0xb15
+#define CSR_MHPMCOUNTER22 0xb16
+#define CSR_MHPMCOUNTER23 0xb17
+#define CSR_MHPMCOUNTER24 0xb18
+#define CSR_MHPMCOUNTER25 0xb19
+#define CSR_MHPMCOUNTER26 0xb1a
+#define CSR_MHPMCOUNTER27 0xb1b
+#define CSR_MHPMCOUNTER28 0xb1c
+#define CSR_MHPMCOUNTER29 0xb1d
+#define CSR_MHPMCOUNTER30 0xb1e
+#define CSR_MHPMCOUNTER31 0xb1f
+#define CSR_MCYCLEH 0xb80
+#define CSR_MINSTRETH 0xb82
+#define CSR_MHPMCOUNTER3H 0xb83
+#define CSR_MHPMCOUNTER4H 0xb84
+#define CSR_MHPMCOUNTER5H 0xb85
+#define CSR_MHPMCOUNTER6H 0xb86
+#define CSR_MHPMCOUNTER7H 0xb87
+#define CSR_MHPMCOUNTER8H 0xb88
+#define CSR_MHPMCOUNTER9H 0xb89
+#define CSR_MHPMCOUNTER10H 0xb8a
+#define CSR_MHPMCOUNTER11H 0xb8b
+#define CSR_MHPMCOUNTER12H 0xb8c
+#define CSR_MHPMCOUNTER13H 0xb8d
+#define CSR_MHPMCOUNTER14H 0xb8e
+#define CSR_MHPMCOUNTER15H 0xb8f
+#define CSR_MHPMCOUNTER16H 0xb90
+#define CSR_MHPMCOUNTER17H 0xb91
+#define CSR_MHPMCOUNTER18H 0xb92
+#define CSR_MHPMCOUNTER19H 0xb93
+#define CSR_MHPMCOUNTER20H 0xb94
+#define CSR_MHPMCOUNTER21H 0xb95
+#define CSR_MHPMCOUNTER22H 0xb96
+#define CSR_MHPMCOUNTER23H 0xb97
+#define CSR_MHPMCOUNTER24H 0xb98
+#define CSR_MHPMCOUNTER25H 0xb99
+#define CSR_MHPMCOUNTER26H 0xb9a
+#define CSR_MHPMCOUNTER27H 0xb9b
+#define CSR_MHPMCOUNTER28H 0xb9c
+#define CSR_MHPMCOUNTER29H 0xb9d
+#define CSR_MHPMCOUNTER30H 0xb9e
+#define CSR_MHPMCOUNTER31H 0xb9f
+
+#define CSR_CYCLE 0xc00
+#define CSR_TIME 0xc01
+#define CSR_INSTRET 0xc02
+#define CSR_HPMCOUNTER3 0xc03
+#define CSR_HPMCOUNTER4 0xc04
+#define CSR_HPMCOUNTER5 0xc05
+#define CSR_HPMCOUNTER6 0xc06
+#define CSR_HPMCOUNTER7 0xc07
+#define CSR_HPMCOUNTER8 0xc08
+#define CSR_HPMCOUNTER9 0xc09
+#define CSR_HPMCOUNTER10 0xc0a
+#define CSR_HPMCOUNTER11 0xc0b
+#define CSR_HPMCOUNTER12 0xc0c
+#define CSR_HPMCOUNTER13 0xc0d
+#define CSR_HPMCOUNTER14 0xc0e
+#define CSR_HPMCOUNTER15 0xc0f
+#define CSR_HPMCOUNTER16 0xc10
+#define CSR_HPMCOUNTER17 0xc11
+#define CSR_HPMCOUNTER18 0xc12
+#define CSR_HPMCOUNTER19 0xc13
+#define CSR_HPMCOUNTER20 0xc14
+#define CSR_HPMCOUNTER21 0xc15
+#define CSR_HPMCOUNTER22 0xc16
+#define CSR_HPMCOUNTER23 0xc17
+#define CSR_HPMCOUNTER24 0xc18
+#define CSR_HPMCOUNTER25 0xc19
+#define CSR_HPMCOUNTER26 0xc1a
+#define CSR_HPMCOUNTER27 0xc1b
+#define CSR_HPMCOUNTER28 0xc1c
+#define CSR_HPMCOUNTER29 0xc1d
+#define CSR_HPMCOUNTER30 0xc1e
+#define CSR_HPMCOUNTER31 0xc1f
+#define CSR_CYCLEH 0xc80
+#define CSR_TIMEH 0xc81
+#define CSR_INSTRETH 0xc82
+#define CSR_HPMCOUNTER3H 0xc83
+#define CSR_HPMCOUNTER4H 0xc84
+#define CSR_HPMCOUNTER5H 0xc85
+#define CSR_HPMCOUNTER6H 0xc86
+#define CSR_HPMCOUNTER7H 0xc87
+#define CSR_HPMCOUNTER8H 0xc88
+#define CSR_HPMCOUNTER9H 0xc89
+#define CSR_HPMCOUNTER10H 0xc8a
+#define CSR_HPMCOUNTER11H 0xc8b
+#define CSR_HPMCOUNTER12H 0xc8c
+#define CSR_HPMCOUNTER13H 0xc8d
+#define CSR_HPMCOUNTER14H 0xc8e
+#define CSR_HPMCOUNTER15H 0xc8f
+#define CSR_HPMCOUNTER16H 0xc90
+#define CSR_HPMCOUNTER17H 0xc91
+#define CSR_HPMCOUNTER18H 0xc92
+#define CSR_HPMCOUNTER19H 0xc93
+#define CSR_HPMCOUNTER20H 0xc94
+#define CSR_HPMCOUNTER21H 0xc95
+#define CSR_HPMCOUNTER22H 0xc96
+#define CSR_HPMCOUNTER23H 0xc97
+#define CSR_HPMCOUNTER24H 0xc98
+#define CSR_HPMCOUNTER25H 0xc99
+#define CSR_HPMCOUNTER26H 0xc9a
+#define CSR_HPMCOUNTER27H 0xc9b
+#define CSR_HPMCOUNTER28H 0xc9c
+#define CSR_HPMCOUNTER29H 0xc9d
+#define CSR_HPMCOUNTER30H 0xc9e
+#define CSR_HPMCOUNTER31H 0xc9f
+
+#define CSR_MVENDORID 0xf11
+#define CSR_MARCHID 0xf12
+#define CSR_MIMPID 0xf13
+#define CSR_MHARTID 0xf14
+
#endif /* __riscv */
#endif /* RISCV_CSR_ENCODING_H */

Bin Meng

2018-11-30 09:48:12 UTC

Hi Lukas,

On Thu, Nov 15, 2018 at 6:26 AM Auer, Lukas

Post by Auer, Lukas
Hi Bin,

Post by Bin Meng
The standard RISC-V ISA sets aside a 12-bit encoding space for up
to 4096 CSRs. This adds all known CSR numbers as defined in the
RISC-V Privileged Architecture Version 1.10.
---
arch/riscv/include/asm/encoding.h | 219
++++++++++++++++++++++++++++++++++++++
1 file changed, 219 insertions(+)

What is the reason for adding these and also the exception code
definitions in the next patch?

These are needed for the SBI and CSR instruction emulation.

Regards,
Bin

Bin Meng

2018-11-13 08:21:56 UTC

This post might be inappropriate. Click to display it.

Auer, Lukas

2018-11-14 22:11:31 UTC

Post by Bin Meng
Increase the heap size for the pre-relocation stage, so that CPU
driver can be loaded.
---
arch/riscv/Kconfig | 3 +++
1 file changed, 3 insertions(+)

Reviewed-by: Lukas Auer <***@aisec.fraunhofer.de>

Bin Meng

2018-11-13 08:22:06 UTC

Use a variable 'code' to store the exception code to simplify the
codes in handle_trap().

Signed-off-by: Bin Meng <***@gmail.com>
---

arch/riscv/lib/interrupts.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/arch/riscv/lib/interrupts.c b/arch/riscv/lib/interrupts.c
index 5e09196..0c13588 100644
--- a/arch/riscv/lib/interrupts.c
+++ b/arch/riscv/lib/interrupts.c
@@ -66,14 +66,18 @@ int disable_interrupts(void)
ulong handle_trap(ulong mcause, ulong epc, struct pt_regs *regs)
{
ulong is_int;
+ ulong code;

is_int = (mcause & MCAUSE_INT);
- if ((is_int) && ((mcause & MCAUSE_CAUSE) == IRQ_M_EXT))
- external_interrupt(0); /* handle_m_ext_interrupt */
- else if ((is_int) && ((mcause & MCAUSE_CAUSE) == IRQ_M_TIMER))
- timer_interrupt(0); /* handle_m_timer_interrupt */
- else
- _exit_trap(mcause & MCAUSE_CAUSE, epc, regs);
+ code = mcause & MCAUSE_CAUSE;
+ if (is_int) {
+ if (code == IRQ_M_EXT)
+ external_interrupt(0); /* handle_m_ext_interrupt */
+ else if (code == IRQ_M_TIMER)
+ timer_interrupt(0); /* handle_m_timer_interrupt */
+ } else {
+ _exit_trap(code, epc, regs);
+ }

return epc;
}

--
2.7.4

Auer, Lukas

2018-11-14 23:01:16 UTC

Hi Bin,

Post by Bin Meng
Use a variable 'code' to store the exception code to simplify the
codes in handle_trap().
---
arch/riscv/lib/interrupts.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/arch/riscv/lib/interrupts.c
b/arch/riscv/lib/interrupts.c
index 5e09196..0c13588 100644
--- a/arch/riscv/lib/interrupts.c
+++ b/arch/riscv/lib/interrupts.c
@@ -66,14 +66,18 @@ int disable_interrupts(void)
ulong handle_trap(ulong mcause, ulong epc, struct pt_regs *regs)
{
ulong is_int;
+ ulong code;
is_int = (mcause & MCAUSE_INT);
- if ((is_int) && ((mcause & MCAUSE_CAUSE) == IRQ_M_EXT))
- external_interrupt(0); /* handle_m_ext_interrupt */
- else if ((is_int) && ((mcause & MCAUSE_CAUSE) == IRQ_M_TIMER))
- timer_interrupt(0); /* handle_m_timer_interrupt
*/
- else
- _exit_trap(mcause & MCAUSE_CAUSE, epc, regs);
+ code = mcause & MCAUSE_CAUSE;
+ if (is_int) {
+ if (code == IRQ_M_EXT)
+ external_interrupt(0); /*
handle_m_ext_interrupt */
+ else if (code == IRQ_M_TIMER)
+ timer_interrupt(0); /*
handle_m_timer_interrupt */
+ } else {
+ _exit_trap(code, epc, regs);\

This should use mcause instead of code (see my comments on your
previous patch).

Thanks,
Lukas

Post by Bin Meng
+ }
return epc;
}

Bin Meng

2018-11-13 08:21:54 UTC

RISC-V privileged architecture v1.10 defines a real-time counter,
exposed as a memory-mapped machine-mode register - mtime. mtime must
run at constant frequency, and the platform must provide a mechanism
for determining the timebase of mtime. The mtime register has a
64-bit precision on all RV32, RV64, and RV128 systems.

The mtime is currently implemented by the RISC-V CLINT module. This
adds a U-Boot timer driver so that timer functionalities like delay
works correctly now.

Signed-off-by: Bin Meng <***@gmail.com>
---

drivers/timer/Kconfig | 8 ++++++++
drivers/timer/Makefile | 1 +
drivers/timer/riscv_timer.c | 50 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 59 insertions(+)
create mode 100644 drivers/timer/riscv_timer.c

diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig
index d0cfc35..188b433 100644
--- a/drivers/timer/Kconfig
+++ b/drivers/timer/Kconfig
@@ -126,6 +126,14 @@ config OMAP_TIMER
help
Select this to enable an timer for Omap devices.

+config RISCV_TIMER
+ bool "RISC-V timer support"
+ depends on RISCV && TIMER
+ select RISCV_CLINT
+ help
+ Select this to enable support for the timer as defined
+ by the RISC-V privileged architecture spec v1.10.
+
config ROCKCHIP_TIMER
bool "Rockchip timer support"
depends on TIMER
diff --git a/drivers/timer/Makefile b/drivers/timer/Makefile
index 7f19c49..9fc075b 100644
--- a/drivers/timer/Makefile
+++ b/drivers/timer/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_CADENCE_TTC_TIMER) += cadence-ttc.o
obj-$(CONFIG_DESIGNWARE_APB_TIMER) += dw-apb-timer.o
obj-$(CONFIG_MPC83XX_TIMER) += mpc83xx_timer.o
obj-$(CONFIG_OMAP_TIMER) += omap-timer.o
+obj-$(CONFIG_RISCV_TIMER) += riscv_timer.o
obj-$(CONFIG_ROCKCHIP_TIMER) += rockchip_timer.o
obj-$(CONFIG_SANDBOX_TIMER) += sandbox_timer.o
obj-$(CONFIG_STI_TIMER) += sti-timer.o
diff --git a/drivers/timer/riscv_timer.c b/drivers/timer/riscv_timer.c
new file mode 100644
index 0000000..8bede76
--- /dev/null
+++ b/drivers/timer/riscv_timer.c
@@ -0,0 +1,50 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2018, Bin Meng <***@gmail.com>
+ *
+ * RISC-V privileged architecture timer
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <syscon.h>
+#include <timer.h>
+#include <asm/io.h>
+#include <asm/clint.h>
+
+static int riscv_timer_get_count(struct udevice *dev, u64 *count)
+{
+ *count = riscv_get_time();
+
+ return 0;
+}
+
+static int riscv_timer_probe(struct udevice *dev)
+{
+ struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
+ struct udevice *clint;
+ int ret;
+
+ /* make sure clint driver is loaded */
+ ret = syscon_get_by_driver_data(RISCV_SYSCON_CLINT, &clint);
+ if (ret)
+ return ret;
+
+ /* clock frequency was passed from the cpu driver as driver data */
+ uc_priv->clock_rate = dev->driver_data;
+
+ return 0;
+}
+
+static const struct timer_ops riscv_timer_ops = {
+ .get_count = riscv_timer_get_count,
+};
+
+U_BOOT_DRIVER(riscv_timer) = {
+ .name = "riscv_timer",
+ .id = UCLASS_TIMER,
+ .probe = riscv_timer_probe,
+ .ops = &riscv_timer_ops,
+ .flags = DM_FLAG_PRE_RELOC,
+};

--
2.7.4

Bin Meng

2018-11-13 08:21:55 UTC

At present there are just two levels of Kconfig option hierarchy in
RISC-V. This adds a new level for platform to specify additional
options. It is organized in a way that platform-specific options
followed by board-specific ones, so that when it comes to the same
Kconfig option, board-specific one takes take the highest precedence,
then platform-specific one, and finally architecture-specific one.

As an example, add the QEMU RISC-V platform-specific Kconfig options.

Signed-off-by: Bin Meng <***@gmail.com>
---

arch/riscv/Kconfig | 6 ++++++
arch/riscv/cpu/qemu/Kconfig | 9 +++++++++
board/emulation/qemu-riscv/Kconfig | 1 +
3 files changed, 16 insertions(+)
create mode 100644 arch/riscv/cpu/qemu/Kconfig

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index abfc083..4292ffd 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -16,9 +16,15 @@ config TARGET_QEMU_VIRT

endchoice

+# board-specific options below
source "board/AndesTech/ax25-ae350/Kconfig"
source "board/emulation/qemu-riscv/Kconfig"

+# platform-specific options below
+source "arch/riscv/cpu/qemu/Kconfig"
+
+# architecture-specific options below
+
choice
prompt "Base ISA"
default ARCH_RV32I
diff --git a/arch/riscv/cpu/qemu/Kconfig b/arch/riscv/cpu/qemu/Kconfig
new file mode 100644
index 0000000..ec5d934
--- /dev/null
+++ b/arch/riscv/cpu/qemu/Kconfig
@@ -0,0 +1,9 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright (C) 2018, Bin Meng <***@gmail.com>
+
+config QEMU_RISCV
+ bool
+ imply CPU
+ imply CPU_RISCV
+ imply RISCV_TIMER
diff --git a/board/emulation/qemu-riscv/Kconfig b/board/emulation/qemu-riscv/Kconfig
index 33ca253..bfe050c 100644
--- a/board/emulation/qemu-riscv/Kconfig
+++ b/board/emulation/qemu-riscv/Kconfig
@@ -17,6 +17,7 @@ config SYS_TEXT_BASE

config BOARD_SPECIFIC_OPTIONS # dummy
def_bool y
+ select QEMU_RISCV
imply SYS_NS16550
imply VIRTIO_MMIO
imply VIRTIO_NET

--
2.7.4

Auer, Lukas

2018-11-14 22:05:21 UTC

Hi Bin,

Post by Bin Meng
At present there are just two levels of Kconfig option hierarchy in
RISC-V. This adds a new level for platform to specify additional
options. It is organized in a way that platform-specific options
followed by board-specific ones, so that when it comes to the same
Kconfig option, board-specific one takes take the highest precedence,

nit: you have an extra take here

Post by Bin Meng
then platform-specific one, and finally architecture-specific one.
As an example, add the QEMU RISC-V platform-specific Kconfig options.
---
arch/riscv/Kconfig | 6 ++++++
arch/riscv/cpu/qemu/Kconfig | 9 +++++++++
board/emulation/qemu-riscv/Kconfig | 1 +
3 files changed, 16 insertions(+)
create mode 100644 arch/riscv/cpu/qemu/Kconfig

Reviewed-by: Lukas Auer <***@aisec.fraunhofer.de>

Bin Meng

2018-11-13 08:21:57 UTC

This calls cpu_probe_all() to probe all available cpus.

Signed-off-by: Bin Meng <***@gmail.com>
---

arch/riscv/cpu/qemu/Kconfig | 1 +
arch/riscv/cpu/qemu/cpu.c | 14 ++++++++++++++
2 files changed, 15 insertions(+)

diff --git a/arch/riscv/cpu/qemu/Kconfig b/arch/riscv/cpu/qemu/Kconfig
index ec5d934..e91cff5 100644
--- a/arch/riscv/cpu/qemu/Kconfig
+++ b/arch/riscv/cpu/qemu/Kconfig
@@ -4,6 +4,7 @@

config QEMU_RISCV
bool
+ select ARCH_EARLY_INIT_R
imply CPU
imply CPU_RISCV
imply RISCV_TIMER
diff --git a/arch/riscv/cpu/qemu/cpu.c b/arch/riscv/cpu/qemu/cpu.c
index 221f3a8..e98f624 100644
--- a/arch/riscv/cpu/qemu/cpu.c
+++ b/arch/riscv/cpu/qemu/cpu.c
@@ -4,7 +4,9 @@
*/

#include <common.h>
+#include <cpu.h>
#include <dm.h>
+#include <log.h>

/*
* cleanup_before_linux() is called just before we call linux
@@ -21,6 +23,18 @@ int cleanup_before_linux(void)
return 0;
}

+int arch_early_init_r(void)
+{
+ int ret;
+
+ /* probe cpus so that risc-v timer can be bound */
+ ret = cpu_probe_all();
+ if (ret)
+ return log_msg_ret("risc-v cpus probe fails\n", ret);
+
+ return 0;
+}
+
/* To enumerate devices on the /soc/ node, create a "simple-bus" driver */
static const struct udevice_id riscv_virtio_soc_ids[] = {
{ .compatible = "riscv-virtio-soc" },

--
2.7.4

Auer, Lukas

2018-11-14 22:21:51 UTC

Hi Bin,

Post by Bin Meng
This calls cpu_probe_all() to probe all available cpus.
---
arch/riscv/cpu/qemu/Kconfig | 1 +
arch/riscv/cpu/qemu/cpu.c | 14 ++++++++++++++
2 files changed, 15 insertions(+)

Reviewed-by: Lukas Auer <***@aisec.fraunhofer.de>

This could also go into the generic cpu/cpu.c, what do you think?

Post by Bin Meng
diff --git a/arch/riscv/cpu/qemu/Kconfig
b/arch/riscv/cpu/qemu/Kconfig
index ec5d934..e91cff5 100644
--- a/arch/riscv/cpu/qemu/Kconfig
+++ b/arch/riscv/cpu/qemu/Kconfig
@@ -4,6 +4,7 @@
config QEMU_RISCV
bool
+ select ARCH_EARLY_INIT_R
imply CPU
imply CPU_RISCV
imply RISCV_TIMER
diff --git a/arch/riscv/cpu/qemu/cpu.c b/arch/riscv/cpu/qemu/cpu.c
index 221f3a8..e98f624 100644
--- a/arch/riscv/cpu/qemu/cpu.c
+++ b/arch/riscv/cpu/qemu/cpu.c
@@ -4,7 +4,9 @@
*/
#include <common.h>
+#include <cpu.h>
#include <dm.h>
+#include <log.h>
/*
* cleanup_before_linux() is called just before we call linux
@@ -21,6 +23,18 @@ int cleanup_before_linux(void)
return 0;
}
+int arch_early_init_r(void)
+{
+ int ret;
+
+ /* probe cpus so that risc-v timer can be bound */
+ ret = cpu_probe_all();
+ if (ret)
+ return log_msg_ret("risc-v cpus probe fails\n", ret);

nit: RISC-V (here and in the comment above), failed instead of fails

Thanks,
Lukas

Post by Bin Meng
+
+ return 0;
+}
+
/* To enumerate devices on the /soc/ node, create a "simple-bus" driver */
static const struct udevice_id riscv_virtio_soc_ids[] = {
{ .compatible = "riscv-virtio-soc" },

Bin Meng

2018-11-30 09:48:08 UTC

Hi Lukas,

On Thu, Nov 15, 2018 at 6:22 AM Auer, Lukas

Post by Auer, Lukas
Hi Bin,

Post by Bin Meng
This calls cpu_probe_all() to probe all available cpus.
---
arch/riscv/cpu/qemu/Kconfig | 1 +
arch/riscv/cpu/qemu/cpu.c | 14 ++++++++++++++
2 files changed, 15 insertions(+)

This could also go into the generic cpu/cpu.c, what do you think?

Yes, I think so. Let's do this in v2.

Post by Auer, Lukas

Post by Bin Meng
diff --git a/arch/riscv/cpu/qemu/Kconfig
b/arch/riscv/cpu/qemu/Kconfig
index ec5d934..e91cff5 100644
--- a/arch/riscv/cpu/qemu/Kconfig
+++ b/arch/riscv/cpu/qemu/Kconfig
@@ -4,6 +4,7 @@
config QEMU_RISCV
bool
+ select ARCH_EARLY_INIT_R
imply CPU
imply CPU_RISCV
imply RISCV_TIMER
diff --git a/arch/riscv/cpu/qemu/cpu.c b/arch/riscv/cpu/qemu/cpu.c
index 221f3a8..e98f624 100644
--- a/arch/riscv/cpu/qemu/cpu.c
+++ b/arch/riscv/cpu/qemu/cpu.c
@@ -4,7 +4,9 @@
*/
#include <common.h>
+#include <cpu.h>
#include <dm.h>
+#include <log.h>
/*
* cleanup_before_linux() is called just before we call linux
@@ -21,6 +23,18 @@ int cleanup_before_linux(void)
return 0;
}
+int arch_early_init_r(void)
+{
+ int ret;
+
+ /* probe cpus so that risc-v timer can be bound */
+ ret = cpu_probe_all();
+ if (ret)
+ return log_msg_ret("risc-v cpus probe fails\n", ret);

nit: RISC-V (here and in the comment above), failed instead of fails

Will fix in v2.

Regards,
Bin

Bin Meng

2018-11-13 08:22:04 UTC

With this change, we can avoid a forward declaration.

Signed-off-by: Bin Meng <***@gmail.com>
---

arch/riscv/lib/interrupts.c | 62 ++++++++++++++++++++++-----------------------
1 file changed, 30 insertions(+), 32 deletions(-)

diff --git a/arch/riscv/lib/interrupts.c b/arch/riscv/lib/interrupts.c
index 903a1c4..c568706 100644
--- a/arch/riscv/lib/interrupts.c
+++ b/arch/riscv/lib/interrupts.c
@@ -12,7 +12,36 @@
#include <asm/system.h>
#include <asm/encoding.h>

-static void _exit_trap(ulong code, ulong epc, struct pt_regs *regs);
+static void _exit_trap(ulong code, ulong epc, struct pt_regs *regs)
+{
+ static const char * const exception_code[] = {
+ "Instruction address misaligned",
+ "Instruction access fault",
+ "Illegal instruction",
+ "Breakpoint",
+ "Load address misaligned",
+ "Load access fault",
+ "Store/AMO address misaligned",
+ "Store/AMO access fault",
+ "Environment call from U-mode",
+ "Environment call from S-mode",
+ "Reserved",
+ "Environment call from M-mode",
+ "Instruction page fault",
+ "Load page fault",
+ "Reserved",
+ "Store/AMO page fault",
+ };
+
+ if (code < ARRAY_SIZE(exception_code)) {
+ printf("exception code: %ld , %s , epc %lx , ra %lx\n",
+ code, exception_code[code], epc, regs->ra);
+ } else {
+ printf("Reserved\n");
+ }
+
+ hang();
+}

int interrupt_init(void)
{
@@ -59,34 +88,3 @@ __attribute__((weak)) void external_interrupt(struct pt_regs *regs)
__attribute__((weak)) void timer_interrupt(struct pt_regs *regs)
{
}
-
-static void _exit_trap(ulong code, ulong epc, struct pt_regs *regs)
-{
- static const char * const exception_code[] = {
- "Instruction address misaligned",
- "Instruction access fault",
- "Illegal instruction",
- "Breakpoint",
- "Load address misaligned",
- "Load access fault",
- "Store/AMO address misaligned",
- "Store/AMO access fault",
- "Environment call from U-mode",
- "Environment call from S-mode",
- "Reserved",
- "Environment call from M-mode",
- "Instruction page fault",
- "Load page fault",
- "Reserved",
- "Store/AMO page fault",
- };
-
- if (code < ARRAY_SIZE(exception_code)) {
- printf("exception code: %ld , %s , epc %lx , ra %lx\n",
- code, exception_code[code], epc, regs->ra);
- } else {
- printf("Reserved\n");
- }
-
- hang();
-}

--
2.7.4

Auer, Lukas

2018-11-14 22:50:05 UTC

Post by Bin Meng
With this change, we can avoid a forward declaration.
---
arch/riscv/lib/interrupts.c | 62 ++++++++++++++++++++++-------------
----------
1 file changed, 30 insertions(+), 32 deletions(-)

Reviewed-by: Lukas Auer <***@aisec.fraunhofer.de>

Bin Meng

2018-11-13 08:22:03 UTC

At present the trap handler returns to M-mode only. Change to
returning to previous privilege level instead.

Signed-off-by: Bin Meng <***@gmail.com>
---

arch/riscv/cpu/mtrap.S | 3 ---
1 file changed, 3 deletions(-)

diff --git a/arch/riscv/cpu/mtrap.S b/arch/riscv/cpu/mtrap.S
index 6c0eac6..c9010c7 100644
--- a/arch/riscv/cpu/mtrap.S
+++ b/arch/riscv/cpu/mtrap.S
@@ -68,9 +68,6 @@ trap_entry:
jal handle_trap
csrw mepc, a0

- /* Remain in M-mode after mret */
- li t0, MSTATUS_MPP
- csrs mstatus, t0
LREG x1, 1 * REGBYTES(sp)
LREG x3, 3 * REGBYTES(sp)
LREG x4, 4 * REGBYTES(sp)

--
2.7.4

Auer, Lukas

2018-11-14 22:49:29 UTC

Post by Bin Meng
At present the trap handler returns to M-mode only. Change to
returning to previous privilege level instead.
---
arch/riscv/cpu/mtrap.S | 3 ---
1 file changed, 3 deletions(-)

Reviewed-by: Lukas Auer <***@aisec.fraunhofer.de>

Bin Meng

2018-11-13 08:22:05 UTC

The most significant bit in mcause register should be masked to
form the exception code for _exit_trap().

Signed-off-by: Bin Meng <***@gmail.com>
---

arch/riscv/lib/interrupts.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/lib/interrupts.c b/arch/riscv/lib/interrupts.c
index c568706..5e09196 100644
--- a/arch/riscv/lib/interrupts.c
+++ b/arch/riscv/lib/interrupts.c
@@ -73,7 +73,7 @@ ulong handle_trap(ulong mcause, ulong epc, struct pt_regs *regs)
else if ((is_int) && ((mcause & MCAUSE_CAUSE) == IRQ_M_TIMER))
timer_interrupt(0); /* handle_m_timer_interrupt */
else
- _exit_trap(mcause, epc, regs);
+ _exit_trap(mcause & MCAUSE_CAUSE, epc, regs);

return epc;
}

--
2.7.4

Auer, Lukas

2018-11-14 22:58:59 UTC

Hi Bin,

Post by Bin Meng
The most significant bit in mcause register should be masked to
form the exception code for _exit_trap().
---
arch/riscv/lib/interrupts.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/riscv/lib/interrupts.c
b/arch/riscv/lib/interrupts.c
index c568706..5e09196 100644
--- a/arch/riscv/lib/interrupts.c
+++ b/arch/riscv/lib/interrupts.c
@@ -73,7 +73,7 @@ ulong handle_trap(ulong mcause, ulong epc, struct pt_regs *regs)
else if ((is_int) && ((mcause & MCAUSE_CAUSE) == IRQ_M_TIMER))
timer_interrupt(0); /* handle_m_timer_interrupt
*/
else
- _exit_trap(mcause, epc, regs);
+ _exit_trap(mcause & MCAUSE_CAUSE, epc, regs);

The exception codes differ between traps caused by an interrupt (MSB
set) and those that are not. Besides software interrupts, the
handle_trap already checks for all possible machine-mode interrupts.

Thanks,
Lukas

Post by Bin Meng
return epc;
}

Bin Meng

2018-11-30 09:56:32 UTC

Hi Lukas,

On Thu, Nov 15, 2018 at 6:59 AM Auer, Lukas

Post by Auer, Lukas
Hi Bin,

Post by Bin Meng
The most significant bit in mcause register should be masked to
form the exception code for _exit_trap().
---
arch/riscv/lib/interrupts.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/riscv/lib/interrupts.c
b/arch/riscv/lib/interrupts.c
index c568706..5e09196 100644
--- a/arch/riscv/lib/interrupts.c
+++ b/arch/riscv/lib/interrupts.c
@@ -73,7 +73,7 @@ ulong handle_trap(ulong mcause, ulong epc, struct pt_regs *regs)
else if ((is_int) && ((mcause & MCAUSE_CAUSE) == IRQ_M_TIMER))
timer_interrupt(0); /* handle_m_timer_interrupt
*/
else
- _exit_trap(mcause, epc, regs);
+ _exit_trap(mcause & MCAUSE_CAUSE, epc, regs);

The exception codes differ between traps caused by an interrupt (MSB
set) and those that are not. Besides software interrupts, the
handle_trap already checks for all possible machine-mode interrupts.

For the M-mode software interrupts, it will fall into the _exit_trap()
branch which is wrong. Do you mean we just leave it for now until we
support the SBI in the future?

Regards,
Bin

Auer, Lukas

2018-12-03 22:36:39 UTC

Hi Bin,

Post by Bin Meng
Hi Lukas,
On Thu, Nov 15, 2018 at 6:59 AM Auer, Lukas

Post by Auer, Lukas
Hi Bin,

Post by Bin Meng
The most significant bit in mcause register should be masked to
form the exception code for _exit_trap().
---
arch/riscv/lib/interrupts.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/riscv/lib/interrupts.c
b/arch/riscv/lib/interrupts.c
index c568706..5e09196 100644
--- a/arch/riscv/lib/interrupts.c
+++ b/arch/riscv/lib/interrupts.c
@@ -73,7 +73,7 @@ ulong handle_trap(ulong mcause, ulong epc,
struct
pt_regs *regs)
else if ((is_int) && ((mcause & MCAUSE_CAUSE) ==
IRQ_M_TIMER))
timer_interrupt(0); /* handle_m_timer_interrupt
*/
else
- _exit_trap(mcause, epc, regs);
+ _exit_trap(mcause & MCAUSE_CAUSE, epc, regs);

The exception codes differ between traps caused by an interrupt (MSB
set) and those that are not. Besides software interrupts, the
handle_trap already checks for all possible machine-mode
interrupts.

For the M-mode software interrupts, it will fall into the
_exit_trap()
branch which is wrong. Do you mean we just leave it for now until we
support the SBI in the future?

You are right. It is probably best to add a software_interrupt
function, similar to those for external and timer interrupts.

I also just noticed that I should have included more information, when
an undefined exception code is encountered in _exit_trap. I will send a
patch to fix this tomorrow.

Thanks,
Lukas

Post by Bin Meng
Regards,
Bin

Bin Meng

2018-11-13 08:22:07 UTC

Allow U-Boot to run on hart 0 only, and suspend other harts.

With this change, '-smp n' works on QEMU RISC-V board.

Signed-off-by: Bin Meng <***@gmail.com>

---

arch/riscv/cpu/start.S | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
index 9858058..fcb0466 100644
--- a/arch/riscv/cpu/start.S
+++ b/arch/riscv/cpu/start.S
@@ -46,6 +46,10 @@ _start:
/* mask all interrupts */
csrw mie, zero

+ csrr t0, mhartid
+ beqz t0, call_board_init_f
+1: j 1b
+
/*
* Set stackpointer in internal/ex RAM to call board_init_f
*/

--
2.7.4

Auer, Lukas

2018-11-14 23:05:57 UTC

Hi Bin,

Post by Bin Meng
Allow U-Boot to run on hart 0 only, and suspend other harts.
With this change, '-smp n' works on QEMU RISC-V board.
---
arch/riscv/cpu/start.S | 4 ++++
1 file changed, 4 insertions(+)

Reviewed-by: Lukas Auer <***@aisec.fraunhofer.de>

I'll try to send my patch series with multi-hart support soon, so I
hope we won't need this patch for long :)

Post by Bin Meng
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
index 9858058..fcb0466 100644
--- a/arch/riscv/cpu/start.S
+++ b/arch/riscv/cpu/start.S
/* mask all interrupts */
csrw mie, zero
+ csrr t0, mhartid
+ beqz t0, call_board_init_f
+1: j 1b
+

To suspend the other harts, you can also add a WFI instruction before
the jump instruction.

Thanks,
Lukas

Post by Bin Meng
/*
* Set stackpointer in internal/ex RAM to call board_init_f
*/

Bin Meng

2018-11-13 08:22:00 UTC

Implement arch_cpu_init() to do some basic architecture level cpu
initialization, like FPU enable, etc.

Signed-off-by: Bin Meng <***@gmail.com>
---

arch/riscv/cpu/cpu.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)

diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c
index d9f820c..4e508cf 100644
--- a/arch/riscv/cpu/cpu.c
+++ b/arch/riscv/cpu/cpu.c
@@ -5,6 +5,7 @@

#include <common.h>
#include <asm/csr.h>
+#include <asm/encoding.h>

/*
* prior_stage_fdt_address must be stored in the data section since it is used
@@ -53,3 +54,23 @@ int print_cpuinfo(void)

return 0;
}
+
+int arch_cpu_init(void)
+{
+ /* Enable FPU */
+ if (supports_extension('d') || supports_extension('f')) {
+ csr_write(mstatus, MSTATUS_FS);
+ csr_write(fcsr, 0);
+ }
+
+ /* Enable user/supervisor use of perf counters */
+ if (supports_extension('s'))
+ csr_write(scounteren, -1);
+ csr_write(mcounteren, -1);
+
+ /* Disable paging */
+ if (supports_extension('s'))
+ csr_write(sptbr, 0);
+
+ return 0;
+}

--
2.7.4

Auer, Lukas

2018-11-15 23:10:42 UTC

Hi Bin,

Post by Bin Meng
Implement arch_cpu_init() to do some basic architecture level cpu
initialization, like FPU enable, etc.
---
arch/riscv/cpu/cpu.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c
index d9f820c..4e508cf 100644
--- a/arch/riscv/cpu/cpu.c
+++ b/arch/riscv/cpu/cpu.c
@@ -5,6 +5,7 @@
#include <common.h>
#include <asm/csr.h>
+#include <asm/encoding.h>
/*
* prior_stage_fdt_address must be stored in the data section since it is used
@@ -53,3 +54,23 @@ int print_cpuinfo(void)
return 0;
}
+
+int arch_cpu_init(void)
+{
+ /* Enable FPU */
+ if (supports_extension('d') || supports_extension('f')) {
+ csr_write(mstatus, MSTATUS_FS);

This should use csr_set(), so that we don't overwrite the other mstatus
entries.

Post by Bin Meng
+ csr_write(fcsr, 0);

BBL also clears the floating point registers before clearing fcsr.
Coreboot does neither of these, I am not sure if they are required.

Post by Bin Meng
+ }
+
+ /* Enable user/supervisor use of perf counters */
+ if (supports_extension('s'))
+ csr_write(scounteren, -1);
+ csr_write(mcounteren, -1);

Should we really enable all counters, and for both user and supervisor
code? I would tend to enable only the ones we need. How about only
enabling the cycle, time, and instret counters (the rest are not
currently used in the kernel) and only for supervisor code (so in
mcounteren)?

Post by Bin Meng
+
+ /* Disable paging */
+ if (supports_extension('s'))
+ csr_write(sptbr, 0);

sptbr has been renamed to satp.

Thanks,
Lukas

Post by Bin Meng
+
+ return 0;
+}

Bin Meng

2018-11-30 09:48:19 UTC

Hi Lukas,

On Fri, Nov 16, 2018 at 7:10 AM Auer, Lukas

Post by Auer, Lukas
Hi Bin,

Post by Bin Meng
Implement arch_cpu_init() to do some basic architecture level cpu
initialization, like FPU enable, etc.
---
arch/riscv/cpu/cpu.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c
index d9f820c..4e508cf 100644
--- a/arch/riscv/cpu/cpu.c
+++ b/arch/riscv/cpu/cpu.c
@@ -5,6 +5,7 @@
#include <common.h>
#include <asm/csr.h>
+#include <asm/encoding.h>
/*
* prior_stage_fdt_address must be stored in the data section since it is used
@@ -53,3 +54,23 @@ int print_cpuinfo(void)
return 0;
}
+
+int arch_cpu_init(void)
+{
+ /* Enable FPU */
+ if (supports_extension('d') || supports_extension('f')) {
+ csr_write(mstatus, MSTATUS_FS);

This should use csr_set(), so that we don't overwrite the other mstatus
entries.

Ah, yes!

Post by Auer, Lukas

Post by Bin Meng
+ csr_write(fcsr, 0);

BBL also clears the floating point registers before clearing fcsr.
Coreboot does neither of these, I am not sure if they are required.

It's not required. I believe this is just for sanitizing these registers.

Post by Auer, Lukas

Post by Bin Meng
+ }
+
+ /* Enable user/supervisor use of perf counters */
+ if (supports_extension('s'))
+ csr_write(scounteren, -1);
+ csr_write(mcounteren, -1);

Should we really enable all counters, and for both user and supervisor
code? I would tend to enable only the ones we need. How about only
enabling the cycle, time, and instret counters (the rest are not
currently used in the kernel) and only for supervisor code (so in
mcounteren)?

Yes, not all of them are used by current kernel, but they might be
used in the future. To enable all of them is not a bad idea, unless
turning it on brings some consequences.

Post by Auer, Lukas

Post by Bin Meng
+
+ /* Disable paging */
+ if (supports_extension('s'))
+ csr_write(sptbr, 0);

sptbr has been renamed to satp.

Good catch! Will fix in v2.

Regards,
Bin

Auer, Lukas

2018-12-03 22:22:28 UTC

Hi Bin,

Post by Bin Meng
Hi Lukas,
On Fri, Nov 16, 2018 at 7:10 AM Auer, Lukas

Post by Auer, Lukas
Hi Bin,

Post by Bin Meng
Implement arch_cpu_init() to do some basic architecture level cpu
initialization, like FPU enable, etc.
---
arch/riscv/cpu/cpu.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c
index d9f820c..4e508cf 100644
--- a/arch/riscv/cpu/cpu.c
+++ b/arch/riscv/cpu/cpu.c
@@ -5,6 +5,7 @@
#include <common.h>
#include <asm/csr.h>
+#include <asm/encoding.h>
/*
* prior_stage_fdt_address must be stored in the data section
since
it is used
@@ -53,3 +54,23 @@ int print_cpuinfo(void)
return 0;
}
+
+int arch_cpu_init(void)
+{
+ /* Enable FPU */
+ if (supports_extension('d') || supports_extension('f')) {
+ csr_write(mstatus, MSTATUS_FS);

This should use csr_set(), so that we don't overwrite the other mstatus
entries.

Ah, yes!

Post by Auer, Lukas

Post by Bin Meng
+ csr_write(fcsr, 0);

BBL also clears the floating point registers before clearing fcsr.
Coreboot does neither of these, I am not sure if they are required.

It's not required. I believe this is just for sanitizing these
registers.

Ok, makes sense.

Post by Bin Meng

Post by Auer, Lukas

Post by Bin Meng
+ }
+
+ /* Enable user/supervisor use of perf counters */
+ if (supports_extension('s'))
+ csr_write(scounteren, -1);
+ csr_write(mcounteren, -1);

Should we really enable all counters, and for both user and
supervisor
code? I would tend to enable only the ones we need. How about only
enabling the cycle, time, and instret counters (the rest are not
currently used in the kernel) and only for supervisor code (so in
mcounteren)?

Yes, not all of them are used by current kernel, but they might be
used in the future. To enable all of them is not a bad idea, unless
turning it on brings some consequences.

Hardware performance counters can be used as part of side-channel
attacks. It is therefore a good idea to disable them by default. Here,
I would restrict them to accesses from the supervisor mode. If needed,
software running in supervisor mode can still enable access for the
user mode.

Thanks,
Lukas

Post by Bin Meng

Post by Auer, Lukas

Post by Bin Meng
+
+ /* Disable paging */
+ if (supports_extension('s'))
+ csr_write(sptbr, 0);

sptbr has been renamed to satp.

Good catch! Will fix in v2.
Regards,
Bin

Anup Patel

2018-12-03 07:58:54 UTC

Post by Bin Meng
This adds DM drivers to support RISC-V CPU and timer.
The U-Boot RISC-V SBI support is still working in progress.
Some patches in this series like adding CSR numbers, exception
numbers, are prerequisites for the SBI implementation, but it
does no harm to include them as part of this series.
http://patchwork.ozlabs.org/project/uboot/list/?series=74999
This series is available at u-boot-x86/riscv-working for testing.
dm: cpu: Add timebase frequency to the platdata
riscv: qemu: Create a simple-bus driver for the soc node
cpu: Add a RISC-V CPU driver
riscv: Add a SYSCON driver for Core Local Interruptor
timer: Add driver for RISC-V privileged architecture defined timer
riscv: kconfig: Allow platform to specify Kconfig options
riscv: Enlarge the default SYS_MALLOC_F_LEN
riscv: qemu: Probe cpus during boot
riscv: Add CSR numbers
riscv: Add exception codes for xcause register
riscv: Do some basic architecture level cpu initialization
riscv: Move trap handler codes to mtrap.S
riscv: Fix context restore before returning from trap handler
riscv: Return to previous privilege level after trap handling
riscv: Adjust the _exit_trap() position to come before handle_trap()
riscv: Pass correct exception code to _exit_trap()
riscv: Refactor handle_trap() a little for future extension
riscv: Allow U-Boot to run on hart 0 only

I see that this series tries to setup infrastructure for
implementing SBI runtime servies.

You might want to consider OpenSBI library, will be
eventually available.
(Refer, https://linuxplumbersconf.org/event/2/contributions/196/attachments/127/159/Atish_SBI.pdf)

If every bootloader starts implementing SBI runtime
services then there will be lot of fragmentation and SOC
vendors will have to support SBI in variety of bootloaders.

Regards,
Anup

Bin Meng

2018-12-03 08:04:36 UTC

Hi Anup,

Post by Anup Patel

Post by Bin Meng
This adds DM drivers to support RISC-V CPU and timer.
The U-Boot RISC-V SBI support is still working in progress.
Some patches in this series like adding CSR numbers, exception
numbers, are prerequisites for the SBI implementation, but it
does no harm to include them as part of this series.
http://patchwork.ozlabs.org/project/uboot/list/?series=74999
This series is available at u-boot-x86/riscv-working for testing.
dm: cpu: Add timebase frequency to the platdata
riscv: qemu: Create a simple-bus driver for the soc node
cpu: Add a RISC-V CPU driver
riscv: Add a SYSCON driver for Core Local Interruptor
timer: Add driver for RISC-V privileged architecture defined timer
riscv: kconfig: Allow platform to specify Kconfig options
riscv: Enlarge the default SYS_MALLOC_F_LEN
riscv: qemu: Probe cpus during boot
riscv: Add CSR numbers
riscv: Add exception codes for xcause register
riscv: Do some basic architecture level cpu initialization
riscv: Move trap handler codes to mtrap.S
riscv: Fix context restore before returning from trap handler
riscv: Return to previous privilege level after trap handling
riscv: Adjust the _exit_trap() position to come before handle_trap()
riscv: Pass correct exception code to _exit_trap()
riscv: Refactor handle_trap() a little for future extension
riscv: Allow U-Boot to run on hart 0 only

I see that this series tries to setup infrastructure for
implementing SBI runtime servies.
You might want to consider OpenSBI library, will be
eventually available.
(Refer, https://linuxplumbersconf.org/event/2/contributions/196/attachments/127/159/Atish_SBI.pdf)
If every bootloader starts implementing SBI runtime
services then there will be lot of fragmentation and SOC
vendors will have to support SBI in variety of bootloaders.

Agreed. In fact Atish contacted me offline some time ago about
contributing in OpenSBI. Definitely I think that's the right approach,
so that I am not sending SBI implementation of my own for U-Boot.

Regards,
Bin

Anup Patel

2018-12-06 03:37:13 UTC

Hi Bin,

Post by Bin Meng
This adds DM drivers to support RISC-V CPU and timer.
The U-Boot RISC-V SBI support is still working in progress.
Some patches in this series like adding CSR numbers, exception
numbers, are prerequisites for the SBI implementation, but it
does no harm to include them as part of this series.
http://patchwork.ozlabs.org/project/uboot/list/?series=74999
This series is available at u-boot-x86/riscv-working for testing.
dm: cpu: Add timebase frequency to the platdata
riscv: qemu: Create a simple-bus driver for the soc node
cpu: Add a RISC-V CPU driver
riscv: Add a SYSCON driver for Core Local Interruptor
timer: Add driver for RISC-V privileged architecture defined timer
riscv: kconfig: Allow platform to specify Kconfig options
riscv: Enlarge the default SYS_MALLOC_F_LEN
riscv: qemu: Probe cpus during boot
riscv: Add CSR numbers
riscv: Add exception codes for xcause register
riscv: Do some basic architecture level cpu initialization
riscv: Move trap handler codes to mtrap.S
riscv: Fix context restore before returning from trap handler
riscv: Return to previous privilege level after trap handling
riscv: Adjust the _exit_trap() position to come before handle_trap()
riscv: Pass correct exception code to _exit_trap()
riscv: Refactor handle_trap() a little for future extension
riscv: Allow U-Boot to run on hart 0 only
riscv: add Kconfig entries for the code model

I guess you had posted this series before S-mode changes.

For your v2, please do try your patches on S-mode too.

Thanks,
Anup

Bin Meng

2018-12-06 08:43:34 UTC

Hi Anup,

Post by Auer, Lukas
Hi Bin,

Post by Bin Meng
This adds DM drivers to support RISC-V CPU and timer.
The U-Boot RISC-V SBI support is still working in progress.
Some patches in this series like adding CSR numbers, exception
numbers, are prerequisites for the SBI implementation, but it
does no harm to include them as part of this series.
http://patchwork.ozlabs.org/project/uboot/list/?series=74999
This series is available at u-boot-x86/riscv-working for testing.
dm: cpu: Add timebase frequency to the platdata
riscv: qemu: Create a simple-bus driver for the soc node
cpu: Add a RISC-V CPU driver
riscv: Add a SYSCON driver for Core Local Interruptor
timer: Add driver for RISC-V privileged architecture defined timer
riscv: kconfig: Allow platform to specify Kconfig options
riscv: Enlarge the default SYS_MALLOC_F_LEN
riscv: qemu: Probe cpus during boot
riscv: Add CSR numbers
riscv: Add exception codes for xcause register
riscv: Do some basic architecture level cpu initialization
riscv: Move trap handler codes to mtrap.S
riscv: Fix context restore before returning from trap handler
riscv: Return to previous privilege level after trap handling
riscv: Adjust the _exit_trap() position to come before handle_trap()
riscv: Pass correct exception code to _exit_trap()
riscv: Refactor handle_trap() a little for future extension
riscv: Allow U-Boot to run on hart 0 only
riscv: add Kconfig entries for the code model

I guess you had posted this series before S-mode changes.

Yes.

Post by Auer, Lukas
For your v2, please do try your patches on S-mode too.

Sure.

Regards,
Bin

61 Replies
17 Views
Permalink to this page
Disable enhanced parsing

Thread Navigation

Bin Meng 2018-11-13 08:21:48 UTC

Bin Meng 2018-11-13 08:21:50 UTC

Simon Glass 2018-11-13 20:01:59 UTC

Bin Meng 2018-11-14 01:14:09 UTC

Simon Glass 2018-11-15 19:21:13 UTC

Auer, Lukas 2018-11-14 21:17:40 UTC

Bin Meng 2018-11-13 08:21:49 UTC

Bin Meng 2018-11-13 08:21:53 UTC

Auer, Lukas 2018-11-13 14:45:10 UTC

Bin Meng 2018-11-14 01:48:51 UTC

Rick Chen 2018-11-14 08:02:13 UTC

Auer, Lukas 2018-11-14 10:33:02 UTC

Bin Meng 2018-12-05 09:59:34 UTC

Auer, Lukas 2018-12-05 23:11:24 UTC

Bin Meng 2018-12-06 10:07:40 UTC

Auer, Lukas 2018-12-06 12:30:55 UTC

Bin Meng 2018-12-07 14:00:02 UTC

Philipp Tomsich 2018-12-06 14:33:42 UTC

Bin Meng 2018-12-07 14:01:16 UTC

Bin Meng 2018-11-13 08:21:59 UTC

Bin Meng 2018-11-13 08:21:52 UTC

Auer, Lukas 2018-11-14 21:57:20 UTC

Bin Meng 2018-12-07 13:59:44 UTC

Auer, Lukas 2018-12-11 00:03:36 UTC

Bin Meng 2018-11-13 08:22:01 UTC

Auer, Lukas 2018-11-14 22:44:53 UTC

Bin Meng 2018-11-13 08:21:51 UTC

Auer, Lukas 2018-11-14 21:26:26 UTC

Bin Meng 2018-11-30 09:47:58 UTC

Bin Meng 2018-11-13 08:22:02 UTC

Auer, Lukas 2018-11-14 22:46:46 UTC

Bin Meng 2018-11-13 08:21:58 UTC

Auer, Lukas 2018-11-14 22:26:24 UTC

Bin Meng 2018-11-30 09:48:12 UTC

Bin Meng 2018-11-13 08:21:56 UTC

Auer, Lukas 2018-11-14 22:11:31 UTC

Bin Meng 2018-11-13 08:22:06 UTC

Auer, Lukas 2018-11-14 23:01:16 UTC

Bin Meng 2018-11-13 08:21:54 UTC

Bin Meng 2018-11-13 08:21:55 UTC

Auer, Lukas 2018-11-14 22:05:21 UTC

Bin Meng 2018-11-13 08:21:57 UTC

Auer, Lukas 2018-11-14 22:21:51 UTC

Bin Meng 2018-11-30 09:48:08 UTC

Bin Meng 2018-11-13 08:22:04 UTC

Auer, Lukas 2018-11-14 22:50:05 UTC

Bin Meng 2018-11-13 08:22:03 UTC

Auer, Lukas 2018-11-14 22:49:29 UTC

Bin Meng 2018-11-13 08:22:05 UTC

Auer, Lukas 2018-11-14 22:58:59 UTC

Bin Meng 2018-11-30 09:56:32 UTC

Auer, Lukas 2018-12-03 22:36:39 UTC

Bin Meng 2018-11-13 08:22:07 UTC

Auer, Lukas 2018-11-14 23:05:57 UTC

Bin Meng 2018-11-13 08:22:00 UTC

Auer, Lukas 2018-11-15 23:10:42 UTC

Bin Meng 2018-11-30 09:48:19 UTC

Auer, Lukas 2018-12-03 22:22:28 UTC

Anup Patel 2018-12-03 07:58:54 UTC

Bin Meng 2018-12-03 08:04:36 UTC

Anup Patel 2018-12-06 03:37:13 UTC

Bin Meng 2018-12-06 08:43:34 UTC

about - legalese

Loading...