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

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

[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.

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

[U-Boot] [PATCH 01/19] riscv: add Kconfig entries for the code model

[Bin Meng]

From: Lukas Auer <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 <lukas.auer@aisec.fraunhofer.de>
[bmeng: adjust to make medlow the default code model for U-Boot]
Signed-off-by: Bin Meng <bmeng.cn@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

[U-Boot] [PATCH 02/19] dm: cpu: Add timebase frequency to the platdata

[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.

Signed-off-by: Bin Meng <bmeng.cn@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

[U-Boot] [PATCH 03/19] riscv: qemu: Create a simple-bus driver for the soc node

[Bin Meng]

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

Signed-off-by: Bin Meng <bmeng.cn@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

[U-Boot] [PATCH 04/19] cpu: Add a RISC-V CPU driver

[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.

Signed-off-by: Bin Meng <bmeng.cn@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 <bmeng.cn@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

[U-Boot] [PATCH 05/19] riscv: Add a SYSCON driver for Core Local Interruptor

[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.

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

 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+ */
+/*
+ * Copyright (C) 2018, Bin Meng <bmeng.cn@gmail.com>
+ */
+
+#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+
+/*
+ * Copyright (C) 2018, Bin Meng <bmeng.cn@gmail.com>
+ *
+ * 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,
+	.of_match	= clint_ids,
+	.probe		= clint_probe,
+	.flags		= DM_FLAG_PRE_RELOC,
+};
-- 
2.7.4

[U-Boot] [PATCH 06/19] timer: Add driver for RISC-V privileged architecture defined timer

[Bin Meng]

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 <bmeng.cn@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 <bmeng.cn@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

[U-Boot] [PATCH 07/19] riscv: kconfig: Allow platform to specify Kconfig options

[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,
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 <bmeng.cn@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 <bmeng.cn@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

[U-Boot] [PATCH 08/19] riscv: Enlarge the default SYS_MALLOC_F_LEN

[Bin Meng]

Increase the heap size for the pre-relocation stage, so that CPU
driver can be loaded.

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

 arch/riscv/Kconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 4292ffd..f020060 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -87,4 +87,7 @@ config RISCV_CLINT
 	  The CLINT block holds memory-mapped control and status registers
 	  associated with software and timer interrupts.
 
+config SYS_MALLOC_F_LEN
+	default 0x1000
+
 endmenu
-- 
2.7.4

[U-Boot] [PATCH 09/19] riscv: qemu: Probe cpus during boot

[Bin Meng]

This calls cpu_probe_all() to probe all available cpus.

Signed-off-by: Bin Meng <bmeng.cn@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

[U-Boot] [PATCH 10/19] riscv: Add CSR numbers

[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.

Signed-off-by: Bin Meng <bmeng.cn@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

[U-Boot] [PATCH 11/19] riscv: Add exception codes for xcause register

[Bin Meng]

This adds all exception codes in encoding.h.

Signed-off-by: Bin Meng <bmeng.cn@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

[U-Boot] [PATCH 12/19] riscv: Do some basic architecture level cpu initialization

[Bin Meng]

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

Signed-off-by: Bin Meng <bmeng.cn@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

[U-Boot] [PATCH 13/19] riscv: Move trap handler codes to mtrap.S

[Bin Meng]

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 <bmeng.cn@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 <Padmarao.Begari@microsemi.com>
+ *
+ * Copyright (C) 2017 Andes Technology Corporation
+ * Rick Chen, Andes Technology Corporation <rick@andestech.com>
+ *
+ * Copyright (C) 2018, Bin Meng <bmeng.cn@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

[U-Boot] [PATCH 14/19] riscv: Fix context restore before returning from trap handler

[Bin Meng]

sp cannot be loaded before restoring other registers.

Signed-off-by: Bin Meng <bmeng.cn@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

[U-Boot] [PATCH 15/19] riscv: Return to previous privilege level after trap handling

[Bin Meng]

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

Signed-off-by: Bin Meng <bmeng.cn@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

[U-Boot] [PATCH 16/19] riscv: Adjust the _exit_trap() position to come before handle_trap()

[Bin Meng]

With this change, we can avoid a forward declaration.

Signed-off-by: Bin Meng <bmeng.cn@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

[U-Boot] [PATCH 17/19] riscv: Pass correct exception code to _exit_trap()

[Bin Meng]

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

Signed-off-by: Bin Meng <bmeng.cn@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

[U-Boot] [PATCH 18/19] riscv: Refactor handle_trap() a little for future extension

[Bin Meng]

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

Signed-off-by: Bin Meng <bmeng.cn@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

[U-Boot] [PATCH 19/19] riscv: Allow U-Boot to run on hart 0 only

[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.

Signed-off-by: Bin Meng <bmeng.cn@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

[U-Boot] [PATCH 05/19] riscv: Add a SYSCON driver for Core Local Interruptor

[Auer, Lukas]

Hi Bin,

On Tue, 2018-11-13 at 00:21 -0800, Bin Meng wrote:
> 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.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---

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

[U-Boot] [PATCH 02/19] dm: cpu: Add timebase frequency to the platdata

[Simon Glass]

On 13 November 2018 at 00:21, Bin Meng <bmeng.cn@gmail.com> wrote:
> 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 <bmeng.cn@gmail.com>
> ---
>
>  include/cpu.h | 3 +++
>  1 file changed, 3 insertions(+)

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

Could we have comments for the struct members?

[U-Boot] [PATCH 02/19] dm: cpu: Add timebase frequency to the platdata

[Bin Meng]

Hi Simon,

On Wed, Nov 14, 2018 at 4:02 AM Simon Glass <sjg@chromium.org> wrote:
>
> On 13 November 2018 at 00:21, Bin Meng <bmeng.cn@gmail.com> wrote:
> > 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 <bmeng.cn@gmail.com>
> > ---
> >
> >  include/cpu.h | 3 +++
> >  1 file changed, 3 insertions(+)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> 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

[U-Boot] [PATCH 05/19] riscv: Add a SYSCON driver for Core Local Interruptor

[Bin Meng]

Hi Lukas,

On Tue, Nov 13, 2018 at 10:45 PM Auer, Lukas
<lukas.auer@aisec.fraunhofer.de> wrote:
>
> Hi Bin,
>
> On Tue, 2018-11-13 at 00:21 -0800, Bin Meng wrote:
> > 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.
> >
> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > ---
>
> 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?

> 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.

> 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

[U-Boot] [PATCH 05/19] riscv: Add a SYSCON driver for Core Local Interruptor

[Rick Chen]

<rick@andestech.com> 於 2018年11月14日 週三 下午3:35寫道：
>
> > Hi Lukas,
> >
> > On Tue, Nov 13, 2018 at 10:45 PM Auer, Lukas <lukas.auer@aisec.fraunhofer.de>
> > wrote:
> > >
> > > Hi Bin,
> > >
> > > On Tue, 2018-11-13 at 00:21 -0800, Bin Meng wrote:
> > > > 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.
> > > >
> > > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > > > ---
> > >
> > > 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-controller at 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
 plmt0 at 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

> > > 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.
> >
> > > 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

[U-Boot] [PATCH 05/19] riscv: Add a SYSCON driver for Core Local Interruptor

[Auer, Lukas]

Hi Bin,

On Wed, 2018-11-14 at 09:48 +0800, Bin Meng wrote:
> Hi Lukas,
> 
> On Tue, Nov 13, 2018 at 10:45 PM Auer, Lukas
> <lukas.auer@aisec.fraunhofer.de> wrote:
> > 
> > Hi Bin,
> > 
> > On Tue, 2018-11-13 at 00:21 -0800, Bin Meng wrote:
> > > 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.
> > > 
> > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > > ---
> > 
> > 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?
> 
> > 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.
> 

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

> > 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

[U-Boot] [PATCH 02/19] dm: cpu: Add timebase frequency to the platdata

[Auer, Lukas]

Hi Bin,

On Tue, 2018-11-13 at 00:21 -0800, Bin Meng wrote:
> 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 <bmeng.cn@gmail.com>
> ---
> 
>  include/cpu.h | 3 +++
>  1 file changed, 3 insertions(+)
> 

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

> 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)

nit: Hz

Thanks,
Lukas

>   *
>   * 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 */

[U-Boot] [PATCH 03/19] riscv: qemu: Create a simple-bus driver for the soc node

[Auer, Lukas]

Hi Bin,

On Tue, 2018-11-13 at 00:21 -0800, Bin Meng wrote:
> To enumerate devices on the /soc/ node, create a "simple-bus"
> driver to match "riscv-virtio-soc".
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
> 
>  arch/riscv/cpu/qemu/cpu.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 

Reviewed-by: Lukas Auer <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.

> 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

[U-Boot] [PATCH 04/19] cpu: Add a RISC-V CPU driver

[Auer, Lukas]

Hi Bin,

On Tue, 2018-11-13 at 00:21 -0800, Bin Meng wrote:
> 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 <bmeng.cn@gmail.com>
> ---
> 

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

>  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 <bmeng.cn@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);
> +

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?

> +	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);

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

> +	}
> +
> +	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,
> +};

[U-Boot] [PATCH 07/19] riscv: kconfig: Allow platform to specify Kconfig options

[Auer, Lukas]

Hi Bin,

On Tue, 2018-11-13 at 00:21 -0800, Bin Meng wrote:
> 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

> 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 <bmeng.cn@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
> 

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

[U-Boot] [PATCH 08/19] riscv: Enlarge the default SYS_MALLOC_F_LEN

[Auer, Lukas]

On Tue, 2018-11-13 at 00:21 -0800, Bin Meng wrote:
> Increase the heap size for the pre-relocation stage, so that CPU
> driver can be loaded.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
> 
>  arch/riscv/Kconfig | 3 +++
>  1 file changed, 3 insertions(+)
> 

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

[U-Boot] [PATCH 09/19] riscv: qemu: Probe cpus during boot

[Auer, Lukas]

Hi Bin,

On Tue, 2018-11-13 at 00:21 -0800, Bin Meng wrote:
> This calls cpu_probe_all() to probe all available cpus.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
> 
>  arch/riscv/cpu/qemu/Kconfig |  1 +
>  arch/riscv/cpu/qemu/cpu.c   | 14 ++++++++++++++
>  2 files changed, 15 insertions(+)
> 

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

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

> 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

> +
> +	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] [PATCH 10/19] riscv: Add CSR numbers

[Auer, Lukas]

Hi Bin,

On Tue, 2018-11-13 at 00:21 -0800, Bin Meng wrote:
> 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 <bmeng.cn@gmail.com>
> ---
> 
>  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

> 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 */

[U-Boot] [PATCH 13/19] riscv: Move trap handler codes to mtrap.S

[Auer, Lukas]

On Tue, 2018-11-13 at 00:22 -0800, Bin Meng wrote:
> 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 <bmeng.cn@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
> 

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

[U-Boot] [PATCH 14/19] riscv: Fix context restore before returning from trap handler

[Auer, Lukas]

On Tue, 2018-11-13 at 00:22 -0800, Bin Meng wrote:
> sp cannot be loaded before restoring other registers.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
> 
>  arch/riscv/cpu/mtrap.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

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

Good catch!

> 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

[U-Boot] [PATCH 15/19] riscv: Return to previous privilege level after trap handling

[Auer, Lukas]

On Tue, 2018-11-13 at 00:22 -0800, Bin Meng wrote:
> At present the trap handler returns to M-mode only. Change to
> returning to previous privilege level instead.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
> 
>  arch/riscv/cpu/mtrap.S | 3 ---
>  1 file changed, 3 deletions(-)
> 

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

[U-Boot] [PATCH 16/19] riscv: Adjust the _exit_trap() position to come before handle_trap()

[Auer, Lukas]

On Tue, 2018-11-13 at 00:22 -0800, Bin Meng wrote:
> With this change, we can avoid a forward declaration.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
> 
>  arch/riscv/lib/interrupts.c | 62 ++++++++++++++++++++++-------------
> ----------
>  1 file changed, 30 insertions(+), 32 deletions(-)
> 

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

[U-Boot] [PATCH 17/19] riscv: Pass correct exception code to _exit_trap()

[Auer, Lukas]

Hi Bin,

On Tue, 2018-11-13 at 00:22 -0800, Bin Meng wrote:
> The most significant bit in mcause register should be masked to
> form the exception code for _exit_trap().
> 
> Signed-off-by: Bin Meng <bmeng.cn@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);

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

>  
>  	return epc;
>  }

[U-Boot] [PATCH 18/19] riscv: Refactor handle_trap() a little for future extension

[Auer, Lukas]

Hi Bin,

On Tue, 2018-11-13 at 00:22 -0800, Bin Meng wrote:
> Use a variable 'code' to store the exception code to simplify the
> codes in handle_trap().
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
> 
>  arch/riscv/lib/interrupts.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 

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

> 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

> +	}
>  
>  	return epc;
>  }

[U-Boot] [PATCH 19/19] riscv: Allow U-Boot to run on hart 0 only

[Auer, Lukas]

Hi Bin,

On Tue, 2018-11-13 at 00:22 -0800, Bin Meng wrote:
> 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 <bmeng.cn@gmail.com>
> 
> ---
> 
>  arch/riscv/cpu/start.S | 4 ++++
>  1 file changed, 4 insertions(+)
> 

Reviewed-by: Lukas Auer <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 :)

> 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
> +

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

Thanks,
Lukas

>  /*
>   * Set stackpointer in internal/ex RAM to call board_init_f
>   */

[U-Boot] [PATCH 02/19] dm: cpu: Add timebase frequency to the platdata

[Simon Glass]

Hi Bin,

On 13 November 2018 at 17:14, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Simon,
>
> On Wed, Nov 14, 2018 at 4:02 AM Simon Glass <sjg@chromium.org> wrote:
>>
>> On 13 November 2018 at 00:21, Bin Meng <bmeng.cn@gmail.com> wrote:
>> > 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 <bmeng.cn@gmail.com>
>> > ---
>> >
>> >  include/cpu.h | 3 +++
>> >  1 file changed, 3 insertions(+)
>>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>
>> 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

[U-Boot] [PATCH 12/19] riscv: Do some basic architecture level cpu initialization

[Auer, Lukas]

Hi Bin,

On Tue, 2018-11-13 at 00:22 -0800, Bin Meng wrote:
> Implement arch_cpu_init() to do some basic architecture level cpu
> initialization, like FPU enable, etc.
> 
> Signed-off-by: Bin Meng <bmeng.cn@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);

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

> +		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.

> +	}
> +
> +	/* 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)?

> +
> +	/* Disable paging */
> +	if (supports_extension('s'))
> +		csr_write(sptbr, 0);

sptbr has been renamed to satp.

Thanks,
Lukas

> +
> +	return 0;
> +}

[U-Boot] [PATCH 03/19] riscv: qemu: Create a simple-bus driver for the soc node

[Bin Meng]

Hi Lukas,

On Thu, Nov 15, 2018 at 5:26 AM Auer, Lukas
<lukas.auer@aisec.fraunhofer.de> wrote:
>
> Hi Bin,
>
> On Tue, 2018-11-13 at 00:21 -0800, Bin Meng wrote:
> > To enumerate devices on the /soc/ node, create a "simple-bus"
> > driver to match "riscv-virtio-soc".
> >
> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > ---
> >
> >  arch/riscv/cpu/qemu/cpu.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
>
> Reviewed-by: Lukas Auer <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.
>

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

> > 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

[U-Boot] [PATCH 09/19] riscv: qemu: Probe cpus during boot

[Bin Meng]

Hi Lukas,

On Thu, Nov 15, 2018 at 6:22 AM Auer, Lukas
<lukas.auer@aisec.fraunhofer.de> wrote:
>
> Hi Bin,
>
> On Tue, 2018-11-13 at 00:21 -0800, Bin Meng wrote:
> > This calls cpu_probe_all() to probe all available cpus.
> >
> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > ---
> >
> >  arch/riscv/cpu/qemu/Kconfig |  1 +
> >  arch/riscv/cpu/qemu/cpu.c   | 14 ++++++++++++++
> >  2 files changed, 15 insertions(+)
> >
>
> Reviewed-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de>
>
> This could also go into the generic cpu/cpu.c, what do you think?
>

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

> > 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

[U-Boot] [PATCH 10/19] riscv: Add CSR numbers

[Bin Meng]

Hi Lukas,

On Thu, Nov 15, 2018 at 6:26 AM Auer, Lukas
<lukas.auer@aisec.fraunhofer.de> wrote:
>
> Hi Bin,
>
> On Tue, 2018-11-13 at 00:21 -0800, Bin Meng wrote:
> > 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 <bmeng.cn@gmail.com>
> > ---
> >
> >  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

[U-Boot] [PATCH 12/19] riscv: Do some basic architecture level cpu initialization

[Bin Meng]

Hi Lukas,

On Fri, Nov 16, 2018 at 7:10 AM Auer, Lukas
<lukas.auer@aisec.fraunhofer.de> wrote:
>
> Hi Bin,
>
> On Tue, 2018-11-13 at 00:22 -0800, Bin Meng wrote:
> > Implement arch_cpu_init() to do some basic architecture level cpu
> > initialization, like FPU enable, etc.
> >
> > Signed-off-by: Bin Meng <bmeng.cn@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);
>
> This should use csr_set(), so that we don't overwrite the other mstatus
> entries.
>

Ah, yes!

> > +             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.

> > +     }
> > +
> > +     /* 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.

> > +
> > +     /* Disable paging */
> > +     if (supports_extension('s'))
> > +             csr_write(sptbr, 0);
>
> sptbr has been renamed to satp.
>

Good catch! Will fix in v2.

Regards,
Bin

[U-Boot] [PATCH 17/19] riscv: Pass correct exception code to _exit_trap()

[Bin Meng]

Hi Lukas,

On Thu, Nov 15, 2018 at 6:59 AM Auer, Lukas
<lukas.auer@aisec.fraunhofer.de> wrote:
>
> Hi Bin,
>
> On Tue, 2018-11-13 at 00:22 -0800, Bin Meng wrote:
> > The most significant bit in mcause register should be masked to
> > form the exception code for _exit_trap().
> >
> > Signed-off-by: Bin Meng <bmeng.cn@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);
>
> 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

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

[Anup Patel]

On Tue, Nov 13, 2018 at 1:47 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> 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
>

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

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

[Bin Meng]

Hi Anup,

On Mon, Dec 3, 2018 at 3:59 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Tue, Nov 13, 2018 at 1:47 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > 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
> >
>
> 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

[U-Boot] [PATCH 12/19] riscv: Do some basic architecture level cpu initialization

[Auer, Lukas]

Hi Bin,

On Fri, 2018-11-30 at 17:48 +0800, Bin Meng wrote:
> Hi Lukas,
> 
> On Fri, Nov 16, 2018 at 7:10 AM Auer, Lukas
> <lukas.auer@aisec.fraunhofer.de> wrote:
> > 
> > Hi Bin,
> > 
> > On Tue, 2018-11-13 at 00:22 -0800, Bin Meng wrote:
> > > Implement arch_cpu_init() to do some basic architecture level cpu
> > > initialization, like FPU enable, etc.
> > > 
> > > Signed-off-by: Bin Meng <bmeng.cn@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);
> > 
> > This should use csr_set(), so that we don't overwrite the other
> > mstatus
> > entries.
> > 
> 
> Ah, yes!
> 
> > > +             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.

> > > +     }
> > > +
> > > +     /* 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

> > > +
> > > +     /* Disable paging */
> > > +     if (supports_extension('s'))
> > > +             csr_write(sptbr, 0);
> > 
> > sptbr has been renamed to satp.
> > 
> 
> Good catch! Will fix in v2.
> 
> Regards,
> Bin

[U-Boot] [PATCH 17/19] riscv: Pass correct exception code to _exit_trap()

[Auer, Lukas]

Hi Bin,

On Fri, 2018-11-30 at 17:56 +0800, Bin Meng wrote:
> Hi Lukas,
> 
> On Thu, Nov 15, 2018 at 6:59 AM Auer, Lukas
> <lukas.auer@aisec.fraunhofer.de> wrote:
> > 
> > Hi Bin,
> > 
> > On Tue, 2018-11-13 at 00:22 -0800, Bin Meng wrote:
> > > The most significant bit in mcause register should be masked to
> > > form the exception code for _exit_trap().
> > > 
> > > Signed-off-by: Bin Meng <bmeng.cn@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);
> > 
> > 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

> Regards,
> Bin

[U-Boot] [PATCH 05/19] riscv: Add a SYSCON driver for Core Local Interruptor

[Bin Meng]

Hi Lukas,

On Wed, Nov 14, 2018 at 6:33 PM Auer, Lukas
<lukas.auer@aisec.fraunhofer.de> wrote:
>
> Hi Bin,
>
> On Wed, 2018-11-14 at 09:48 +0800, Bin Meng wrote:
> > Hi Lukas,
> >
> > On Tue, Nov 13, 2018 at 10:45 PM Auer, Lukas
> > <lukas.auer@aisec.fraunhofer.de> wrote:
> > >
> > > Hi Bin,
> > >
> > > On Tue, 2018-11-13 at 00:21 -0800, Bin Meng wrote:
> > > > 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.
> > > >
> > > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > > > ---
> > >
> > > 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?
> >
> > > 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.
> >
>
> 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

[U-Boot] [PATCH 05/19] riscv: Add a SYSCON driver for Core Local Interruptor

[Auer, Lukas]

Hi Bin,

On Wed, 2018-12-05 at 17:59 +0800, Bin Meng wrote:
> Hi Lukas,
> 
> On Wed, Nov 14, 2018 at 6:33 PM Auer, Lukas
> <lukas.auer@aisec.fraunhofer.de> wrote:
> > 
> > Hi Bin,
> > 
> > On Wed, 2018-11-14 at 09:48 +0800, Bin Meng wrote:
> > > Hi Lukas,
> > > 
> > > On Tue, Nov 13, 2018 at 10:45 PM Auer, Lukas
> > > <lukas.auer@aisec.fraunhofer.de> wrote:
> > > > 
> > > > Hi Bin,
> > > > 
> > > > On Tue, 2018-11-13 at 00:21 -0800, Bin Meng wrote:
> > > > > 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.
> > > > > 
> > > > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > > > > ---
> > > > 
> > > > 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?
> > > 
> > > > 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.
> > > 
> > 
> > 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
> 

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 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

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

[Anup Patel]

Hi Bin,

On Tue, Nov 13, 2018 at 1:47 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> 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
>

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

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

[Bin Meng]

Hi Anup,

On Thu, Dec 6, 2018 at 11:37 AM Anup Patel <anup@brainfault.org> wrote:
>
> Hi Bin,
>
> On Tue, Nov 13, 2018 at 1:47 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > 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
> >
>
> I guess you had posted this series before S-mode changes.
>

Yes.

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

Sure.

Regards,
Bin

[U-Boot] [PATCH 05/19] riscv: Add a SYSCON driver for Core Local Interruptor

[Bin Meng]

Hi Lukas,

On Thu, Dec 6, 2018 at 7:11 AM Auer, Lukas
<lukas.auer@aisec.fraunhofer.de> wrote:
>
> Hi Bin,
>
> On Wed, 2018-12-05 at 17:59 +0800, Bin Meng wrote:
> > Hi Lukas,
> >
> > On Wed, Nov 14, 2018 at 6:33 PM Auer, Lukas
> > <lukas.auer@aisec.fraunhofer.de> wrote:
> > >
> > > Hi Bin,
> > >
> > > On Wed, 2018-11-14 at 09:48 +0800, Bin Meng wrote:
> > > > Hi Lukas,
> > > >
> > > > On Tue, Nov 13, 2018 at 10:45 PM Auer, Lukas
> > > > <lukas.auer@aisec.fraunhofer.de> wrote:
> > > > >
> > > > > Hi Bin,
> > > > >
> > > > > On Tue, 2018-11-13 at 00:21 -0800, Bin Meng wrote:
> > > > > > 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.
> > > > > >
> > > > > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > > > > > ---
> > > > >
> > > > > 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?
> > > >
> > > > > 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.
> > > >
> > >
> > > 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
> >
>
> 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 ...

> >  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.

> 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

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

[U-Boot] [PATCH 05/19] riscv: Add a SYSCON driver for Core Local Interruptor

[Auer, Lukas]

Hi Bin,

On Thu, 2018-12-06 at 18:07 +0800, Bin Meng wrote:
> Hi Lukas,
> 
> On Thu, Dec 6, 2018 at 7:11 AM Auer, Lukas
> <lukas.auer@aisec.fraunhofer.de> wrote:
> > 
> > Hi Bin,
> > 
> > On Wed, 2018-12-05 at 17:59 +0800, Bin Meng wrote:
> > > Hi Lukas,
> > > 
> > > On Wed, Nov 14, 2018 at 6:33 PM Auer, Lukas
> > > <lukas.auer@aisec.fraunhofer.de> wrote:
> > > > 
> > > > Hi Bin,
> > > > 
> > > > On Wed, 2018-11-14 at 09:48 +0800, Bin Meng wrote:
> > > > > Hi Lukas,
> > > > > 
> > > > > On Tue, Nov 13, 2018 at 10:45 PM Auer, Lukas
> > > > > <lukas.auer@aisec.fraunhofer.de> wrote:
> > > > > > 
> > > > > > Hi Bin,
> > > > > > 
> > > > > > On Tue, 2018-11-13 at 00:21 -0800, Bin Meng wrote:
> > > > > > > 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.
> > > > > > > 
> > > > > > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > > > > > > ---
> > > > > > 
> > > > > > 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?
> > > > > 
> > > > > > 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.
> > > > > 
> > > > 
> > > > 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
> > > 
> > 
> > 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 ...
> 
> > >  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?

> > 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
> 
> 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

[U-Boot] [PATCH 05/19] riscv: Add a SYSCON driver for Core Local Interruptor

[Philipp Tomsich]

> On 13.11.2018, at 09:21, Bin Meng <bmeng.cn@gmail.com> wrote:
> 
> 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.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
> 
> 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+ */
> +/*
> + * Copyright (C) 2018, Bin Meng <bmeng.cn@gmail.com>
> + */
> +
> +#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+
> +/*
> + * Copyright (C) 2018, Bin Meng <bmeng.cn@gmail.com>
> + *
> + * 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?

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

[U-Boot] [PATCH 04/19] cpu: Add a RISC-V CPU driver

[Bin Meng]

Hi Lukas,

On Thu, Nov 15, 2018 at 5:57 AM Auer, Lukas
<lukas.auer@aisec.fraunhofer.de> wrote:
>
> Hi Bin,
>
> On Tue, 2018-11-13 at 00:21 -0800, Bin Meng wrote:
> > 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 <bmeng.cn@gmail.com>
> > ---
> >
>
> Since we have the CPU driver, we could also enable CMD_CPU.
>

Agreed. Will do in v2.

> >  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 <bmeng.cn@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);
> > +
>
> 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.

> > +     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);
>
> You don't need the timer variable here, you can just pass NULL.
>

Yes, smarter!

> 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

[U-Boot] [PATCH 05/19] riscv: Add a SYSCON driver for Core Local Interruptor

[Bin Meng]

Hi Lukas,

On Thu, Dec 6, 2018 at 8:30 PM Auer, Lukas
<lukas.auer@aisec.fraunhofer.de> wrote:
>
> Hi Bin,
>
> On Thu, 2018-12-06 at 18:07 +0800, Bin Meng wrote:
> > Hi Lukas,
> >
> > On Thu, Dec 6, 2018 at 7:11 AM Auer, Lukas
> > <lukas.auer@aisec.fraunhofer.de> wrote:
> > >
> > > Hi Bin,
> > >
> > > On Wed, 2018-12-05 at 17:59 +0800, Bin Meng wrote:
> > > > Hi Lukas,
> > > >
> > > > On Wed, Nov 14, 2018 at 6:33 PM Auer, Lukas
> > > > <lukas.auer@aisec.fraunhofer.de> wrote:
> > > > >
> > > > > Hi Bin,
> > > > >
> > > > > On Wed, 2018-11-14 at 09:48 +0800, Bin Meng wrote:
> > > > > > Hi Lukas,
> > > > > >
> > > > > > On Tue, Nov 13, 2018 at 10:45 PM Auer, Lukas
> > > > > > <lukas.auer@aisec.fraunhofer.de> wrote:
> > > > > > >
> > > > > > > Hi Bin,
> > > > > > >
> > > > > > > On Tue, 2018-11-13 at 00:21 -0800, Bin Meng wrote:
> > > > > > > > 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.
> > > > > > > >
> > > > > > > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > > > > > > > ---
> > > > > > >
> > > > > > > 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?
> > > > > >
> > > > > > > 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.
> > > > > >
> > > > >
> > > > > 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
> > > >
> > >
> > > 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 ...
> >
> > > >  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.

> > > 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
> >
> > 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

[U-Boot] [PATCH 05/19] riscv: Add a SYSCON driver for Core Local Interruptor

[Bin Meng]

Hi Philipp,

On Thu, Dec 6, 2018 at 10:33 PM Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
>
>
>
> > On 13.11.2018, at 09:21, Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > 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.
> >
> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > ---
> >
> > 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+ */
> > +/*
> > + * Copyright (C) 2018, Bin Meng <bmeng.cn@gmail.com>
> > + */
> > +
> > +#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+
> > +/*
> > + * Copyright (C) 2018, Bin Meng <bmeng.cn@gmail.com>
> > + *
> > + * 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

[U-Boot] [PATCH 04/19] cpu: Add a RISC-V CPU driver

[Auer, Lukas]

Hi Bin,

On Fri, 2018-12-07 at 21:59 +0800, Bin Meng wrote:
> Hi Lukas,
> 
> On Thu, Nov 15, 2018 at 5:57 AM Auer, Lukas
> <lukas.auer@aisec.fraunhofer.de> wrote:
> > 
> > Hi Bin,
> > 
> > On Tue, 2018-11-13 at 00:21 -0800, Bin Meng wrote:
> > > 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 <bmeng.cn@gmail.com>
> > > ---
> > > 
> > 
> > Since we have the CPU driver, we could also enable CMD_CPU.
> > 
> 
> Agreed. Will do in v2.
> 
> > >  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 <bmeng.cn@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);
> > > +
> > 
> > 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

> > > +     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);
> > 
> > You don't need the timer variable here, you can just pass NULL.
> > 
> 
> Yes, smarter!
> 
> > 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