[PATCH 0/2] sandbox: provide /chosen/boot-hartid property

[PATCH 0/2] sandbox: provide /chosen/boot-hartid property

[Heinrich Schuchardt]

For booting via UEFI on RISC-V the devicetree must provide the
/chosen/boot-hartid property.

Fix /cpus node of the sandbox test devicetree to provide a matching cpu.

Heinrich Schuchardt (2):
  sandbox: correct cpu nodes
  sandbox: provide /chosen/boot-hartid property

 arch/sandbox/dts/test.dts    | 14 +++++++++++---
 arch/sandbox/lib/Makefile    |  2 +-
 arch/sandbox/lib/fdt_fixup.c | 25 +++++++++++++++++++++++++
 drivers/cpu/cpu_sandbox.c    |  2 +-
 test/dm/cpu.c                |  2 +-
 test/dm/timer.c              |  6 +++---
 6 files changed, 42 insertions(+), 9 deletions(-)
 create mode 100644 arch/sandbox/lib/fdt_fixup.c

--
2.30.2

[PATCH 1/2] sandbox: correct cpu nodes

[Heinrich Schuchardt]

The cpu nodes in arch/sandbox/dts/test.dts should conform to the devicetree
specification:

* property device_type must be set to "cpu"
* the reg property must be provided
* the cpu nodes must have an address

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 arch/sandbox/dts/test.dts | 14 +++++++++++---
 drivers/cpu/cpu_sandbox.c |  2 +-
 test/dm/cpu.c             |  2 +-
 test/dm/timer.c           |  6 +++---
 4 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 962bdbe556..9046fcf558 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -805,19 +805,27 @@
 	};

 	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
 		timebase-frequency = <2000000>;
-		cpu-test1 {
+		cpu1: cpu@1 {
+			device_type = "cpu";
+			reg = <0x1>;
 			timebase-frequency = <3000000>;
 			compatible = "sandbox,cpu_sandbox";
 			u-boot,dm-pre-reloc;
 		};

-		cpu-test2 {
+		cpu2: cpu@2 {
+			device_type = "cpu";
+			reg = <0x2>;
 			compatible = "sandbox,cpu_sandbox";
 			u-boot,dm-pre-reloc;
 		};

-		cpu-test3 {
+		cpu3: cpu@3 {
+			device_type = "cpu";
+			reg = <0x3>;
 			compatible = "sandbox,cpu_sandbox";
 			u-boot,dm-pre-reloc;
 		};
diff --git a/drivers/cpu/cpu_sandbox.c b/drivers/cpu/cpu_sandbox.c
index fe6772ba5a..2e871fe313 100644
--- a/drivers/cpu/cpu_sandbox.c
+++ b/drivers/cpu/cpu_sandbox.c
@@ -38,7 +38,7 @@ static int cpu_sandbox_get_vendor(const struct udevice *dev, char *buf,
 	return 0;
 }

-static const char *cpu_current = "cpu-test1";
+static const char *cpu_current = "cpu@1";

 void cpu_sandbox_set_current(const char *name)
 {
diff --git a/test/dm/cpu.c b/test/dm/cpu.c
index ed12cafee2..d7e596ee39 100644
--- a/test/dm/cpu.c
+++ b/test/dm/cpu.c
@@ -27,7 +27,7 @@ static int dm_test_cpu(struct unit_test_state *uts)
 	     uclass_find_next_device(&dev))
 		ut_assert(dev_get_flags(dev) & DM_FLAG_ACTIVATED);

-	ut_assertok(uclass_get_device_by_name(UCLASS_CPU, "cpu-test1", &dev));
+	ut_assertok(uclass_get_device_by_name(UCLASS_CPU, "cpu@1", &dev));
 	ut_asserteq_ptr(cpu_get_current_dev(), dev);
 	ut_asserteq(cpu_is_current(dev), 1);

diff --git a/test/dm/timer.c b/test/dm/timer.c
index 70043b9eee..9f94d47692 100644
--- a/test/dm/timer.c
+++ b/test/dm/timer.c
@@ -33,16 +33,16 @@ static int dm_test_timer_timebase_fallback(struct unit_test_state *uts)
 {
 	struct udevice *dev;

-	cpu_sandbox_set_current("cpu-test1");
+	cpu_sandbox_set_current("cpu@1");
 	ut_assertok(uclass_get_device_by_name(UCLASS_TIMER, "timer@1", &dev));
 	ut_asserteq(3000000, timer_get_rate(dev));
 	ut_assertok(device_remove(dev, DM_REMOVE_NORMAL));

-	cpu_sandbox_set_current("cpu-test2");
+	cpu_sandbox_set_current("cpu@2");
 	ut_assertok(uclass_get_device_by_name(UCLASS_TIMER, "timer@1", &dev));
 	ut_asserteq(2000000, timer_get_rate(dev));

-	cpu_sandbox_set_current("cpu-test1");
+	cpu_sandbox_set_current("cpu@1");

 	return 0;
 }
--
2.30.2

[PATCH 2/2] sandbox: provide /chosen/boot-hartid property

[Heinrich Schuchardt]

On RISC-V the sandbox must provide the /chosen/boot-hartid in the
devicetree.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 arch/sandbox/lib/Makefile    |  2 +-
 arch/sandbox/lib/fdt_fixup.c | 25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)
 create mode 100644 arch/sandbox/lib/fdt_fixup.c

diff --git a/arch/sandbox/lib/Makefile b/arch/sandbox/lib/Makefile
index b4ff717e78..a2bc5a7ee6 100644
--- a/arch/sandbox/lib/Makefile
+++ b/arch/sandbox/lib/Makefile
@@ -5,7 +5,7 @@
 # (C) Copyright 2002-2006
 # Wolfgang Denk, DENX Software Engineering, wd@denx.de.

-obj-y	+= interrupts.o sections.o
+obj-y	+= fdt_fixup.o interrupts.o sections.o
 obj-$(CONFIG_PCI)	+= pci_io.o
 obj-$(CONFIG_CMD_BOOTM) += bootm.o
 obj-$(CONFIG_CMD_BOOTZ) += bootm.o
diff --git a/arch/sandbox/lib/fdt_fixup.c b/arch/sandbox/lib/fdt_fixup.c
new file mode 100644
index 0000000000..a646f2059c
--- /dev/null
+++ b/arch/sandbox/lib/fdt_fixup.c
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#define LOG_CATEGORY LOGC_ARCH
+
+#include <common.h>
+#include <fdt_support.h>
+#include <log.h>
+
+#if defined(__riscv)
+int arch_fixup_fdt(void *blob)
+{
+	int ret;
+
+	ret = fdt_find_or_add_subnode(blob, 0, "chosen");;
+	if (ret < 0)
+		goto err;
+	ret = fdt_setprop_u32(blob, ret, "boot-hartid", 1);
+	if (ret < 0)
+		goto err;
+	return 0;
+err:
+	log_err("Setting /chosen/boot-hartid failed: %s\n", fdt_strerror(ret));
+	return ret;
+}
+#endif
--
2.30.2

Re: [PATCH 1/2] sandbox: correct cpu nodes

[Simon Glass]

On Sat, 28 Aug 2021 at 03:42, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> The cpu nodes in arch/sandbox/dts/test.dts should conform to the devicetree
> specification:
>
> * property device_type must be set to "cpu"
> * the reg property must be provided
> * the cpu nodes must have an address
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  arch/sandbox/dts/test.dts | 14 +++++++++++---
>  drivers/cpu/cpu_sandbox.c |  2 +-
>  test/dm/cpu.c             |  2 +-
>  test/dm/timer.c           |  6 +++---
>  4 files changed, 16 insertions(+), 8 deletions(-)

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

Re: [PATCH 1/2] sandbox: correct cpu nodes

[Simon Glass]

On Sat, 28 Aug 2021 at 03:42, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> The cpu nodes in arch/sandbox/dts/test.dts should conform to the devicetree
> specification:
>
> * property device_type must be set to "cpu"
> * the reg property must be provided
> * the cpu nodes must have an address
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  arch/sandbox/dts/test.dts | 14 +++++++++++---
>  drivers/cpu/cpu_sandbox.c |  2 +-
>  test/dm/cpu.c             |  2 +-
>  test/dm/timer.c           |  6 +++---
>  4 files changed, 16 insertions(+), 8 deletions(-)

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

Applied to u-boot-dm/next, thanks!

Re: [PATCH 2/2] sandbox: provide /chosen/boot-hartid property

[Simon Glass]

Hi Heinrich,

On Sat, 28 Aug 2021 at 03:42, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On RISC-V the sandbox must provide the /chosen/boot-hartid in the
> devicetree.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  arch/sandbox/lib/Makefile    |  2 +-
>  arch/sandbox/lib/fdt_fixup.c | 25 +++++++++++++++++++++++++
>  2 files changed, 26 insertions(+), 1 deletion(-)
>  create mode 100644 arch/sandbox/lib/fdt_fixup.c

Can you add a bit more detail here? This is for the sandbox build, so
we don't actually get to boot the kernel. So who is actually using
this data?

Regards,
Simon


>
> diff --git a/arch/sandbox/lib/Makefile b/arch/sandbox/lib/Makefile
> index b4ff717e78..a2bc5a7ee6 100644
> --- a/arch/sandbox/lib/Makefile
> +++ b/arch/sandbox/lib/Makefile
> @@ -5,7 +5,7 @@
>  # (C) Copyright 2002-2006
>  # Wolfgang Denk, DENX Software Engineering, wd@denx.de.
>
> -obj-y  += interrupts.o sections.o
> +obj-y  += fdt_fixup.o interrupts.o sections.o
>  obj-$(CONFIG_PCI)      += pci_io.o
>  obj-$(CONFIG_CMD_BOOTM) += bootm.o
>  obj-$(CONFIG_CMD_BOOTZ) += bootm.o
> diff --git a/arch/sandbox/lib/fdt_fixup.c b/arch/sandbox/lib/fdt_fixup.c
> new file mode 100644
> index 0000000000..a646f2059c
> --- /dev/null
> +++ b/arch/sandbox/lib/fdt_fixup.c
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +#define LOG_CATEGORY LOGC_ARCH
> +
> +#include <common.h>
> +#include <fdt_support.h>
> +#include <log.h>
> +
> +#if defined(__riscv)
> +int arch_fixup_fdt(void *blob)
> +{
> +       int ret;
> +
> +       ret = fdt_find_or_add_subnode(blob, 0, "chosen");;
> +       if (ret < 0)
> +               goto err;
> +       ret = fdt_setprop_u32(blob, ret, "boot-hartid", 1);
> +       if (ret < 0)
> +               goto err;
> +       return 0;
> +err:
> +       log_err("Setting /chosen/boot-hartid failed: %s\n", fdt_strerror(ret));
> +       return ret;
> +}
> +#endif
> --
> 2.30.2
>

Re: [PATCH 2/2] sandbox: provide /chosen/boot-hartid property

[Heinrich Schuchardt]

On 10/20/21 00:54, Simon Glass wrote:
> Hi Heinrich,
>
> On Sat, 28 Aug 2021 at 03:42, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>> On RISC-V the sandbox must provide the /chosen/boot-hartid in the
>> devicetree.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>>   arch/sandbox/lib/Makefile    |  2 +-
>>   arch/sandbox/lib/fdt_fixup.c | 25 +++++++++++++++++++++++++
>>   2 files changed, 26 insertions(+), 1 deletion(-)
>>   create mode 100644 arch/sandbox/lib/fdt_fixup.c
>
> Can you add a bit more detail here? This is for the sandbox build, so
> we don't actually get to boot the kernel. So who is actually using
> this data?

The U-Boot sandbox can be built on RISC-V. It is very convenient to
debug the Linux EFI stub and its interaction with other UEFI software
like GRUB.

The Linux EFI stub reads the /chosen/boot-hartid property in function
get_boot_hartid_from_fdt(), drivers/firmware/efi/libstub/riscv-stub.c.
It will stop booting if /chosen/boot-hartid is not found.

Of course the kernel will fail when trying to set up virtual memory. But
that is after most of the interesting stuff in the EFI stub have
occurred. Linux can run in nommu mode. So maybe you could even run Linux
on the sandbox.

Best regards

Heinrich

>
> Regards,
> Simon
>
>
>>
>> diff --git a/arch/sandbox/lib/Makefile b/arch/sandbox/lib/Makefile
>> index b4ff717e78..a2bc5a7ee6 100644
>> --- a/arch/sandbox/lib/Makefile
>> +++ b/arch/sandbox/lib/Makefile
>> @@ -5,7 +5,7 @@
>>   # (C) Copyright 2002-2006
>>   # Wolfgang Denk, DENX Software Engineering, wd@denx.de.
>>
>> -obj-y  += interrupts.o sections.o
>> +obj-y  += fdt_fixup.o interrupts.o sections.o
>>   obj-$(CONFIG_PCI)      += pci_io.o
>>   obj-$(CONFIG_CMD_BOOTM) += bootm.o
>>   obj-$(CONFIG_CMD_BOOTZ) += bootm.o
>> diff --git a/arch/sandbox/lib/fdt_fixup.c b/arch/sandbox/lib/fdt_fixup.c
>> new file mode 100644
>> index 0000000000..a646f2059c
>> --- /dev/null
>> +++ b/arch/sandbox/lib/fdt_fixup.c
>> @@ -0,0 +1,25 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +
>> +#define LOG_CATEGORY LOGC_ARCH
>> +
>> +#include <common.h>
>> +#include <fdt_support.h>
>> +#include <log.h>
>> +
>> +#if defined(__riscv)
>> +int arch_fixup_fdt(void *blob)
>> +{
>> +       int ret;
>> +
>> +       ret = fdt_find_or_add_subnode(blob, 0, "chosen");;
>> +       if (ret < 0)
>> +               goto err;
>> +       ret = fdt_setprop_u32(blob, ret, "boot-hartid", 1);
>> +       if (ret < 0)
>> +               goto err;
>> +       return 0;
>> +err:
>> +       log_err("Setting /chosen/boot-hartid failed: %s\n", fdt_strerror(ret));
>> +       return ret;
>> +}
>> +#endif
>> --
>> 2.30.2
>>

Re: [PATCH 2/2] sandbox: provide /chosen/boot-hartid property

[Simon Glass]

On 10/20/21 00:54, Simon Glass wrote:
> Hi Heinrich,
>
> On Sat, 28 Aug 2021 at 03:42, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>> On RISC-V the sandbox must provide the /chosen/boot-hartid in the
>> devicetree.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>>   arch/sandbox/lib/Makefile    |  2 +-
>>   arch/sandbox/lib/fdt_fixup.c | 25 +++++++++++++++++++++++++
>>   2 files changed, 26 insertions(+), 1 deletion(-)
>>   create mode 100644 arch/sandbox/lib/fdt_fixup.c
>
> Can you add a bit more detail here? This is for the sandbox build, so
> we don't actually get to boot the kernel. So who is actually using
> this data?

The U-Boot sandbox can be built on RISC-V. It is very convenient to
debug the Linux EFI stub and its interaction with other UEFI software
like GRUB.

The Linux EFI stub reads the /chosen/boot-hartid property in function
get_boot_hartid_from_fdt(), drivers/firmware/efi/libstub/riscv-stub.c.
It will stop booting if /chosen/boot-hartid is not found.

Of course the kernel will fail when trying to set up virtual memory. But
that is after most of the interesting stuff in the EFI stub have
occurred. Linux can run in nommu mode. So maybe you could even run Linux
on the sandbox.

Best regards

Heinrich

>
> Regards,
> Simon
>
>
>>
Applied to u-boot-dm, thanks!