All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/1] Add boot hartid to a Device tree
@ 2020-02-24 22:19 Atish Patra
  2020-02-24 22:19 ` [RFC PATCH 1/1] riscv: Add boot hartid to " Atish Patra
  2020-02-24 23:08 ` [RFC PATCH 0/1] Add boot hartid to a " Heinrich Schuchardt
  0 siblings, 2 replies; 13+ messages in thread
From: Atish Patra @ 2020-02-24 22:19 UTC (permalink / raw)
  To: u-boot

The RISC-V booting protocol requires the hart id to be present in "a0"
register. This is not a problem for bootm/booti commands as they directly
jump to Linux kernel. However, bootefi jumps to a EFI boot stub code in
Linux kernel which acts a loader and jumps to real Linux after terminating
the boot services. This boot stub code has to be aware of the boot hart id
so that it can set it in "a0" before jumping to Linux kernel. Currently,
UEFI protocol doesn't have any mechanism to pass the boot hart id to an
EFI executable. We should keep it this way as this is a RISC-V specific
requirement rather than a UEFI requirement. Out of the all possible options,
device tree seemed to be the best choice to do this job.
The detailed discussion can be found in the following thread. 

https://patchwork.ozlabs.org/patch/1233664/

This patch updates the device tree in arch_fixup_fdt() which is common for
all booting commands. As a result, the DT modification doesn't require any
efi related arch specific functions and all DT related modifications are
contained at one place. However, the hart id node will be available for
Linux even if the kernel is booted using bootm command.

If that is not acceptable, we can always move the code to an efi specific
function.

Atish Patra (1):
riscv: Add boot hartid to Device tree

arch/riscv/lib/bootm.c | 13 +++++++++++++
1 file changed, 13 insertions(+)

--
2.24.0

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC PATCH 1/1] riscv: Add boot hartid to Device tree
  2020-02-24 22:19 [RFC PATCH 0/1] Add boot hartid to a Device tree Atish Patra
@ 2020-02-24 22:19 ` Atish Patra
  2020-02-24 23:44   ` Ard Biesheuvel
  2020-02-24 23:08 ` [RFC PATCH 0/1] Add boot hartid to a " Heinrich Schuchardt
  1 sibling, 1 reply; 13+ messages in thread
From: Atish Patra @ 2020-02-24 22:19 UTC (permalink / raw)
  To: u-boot

Linux booting protocol mandates that register "a0" contains the hartid.
However, U-boot can not pass the hartid via a0 during EFI boot without
breaking the UEFI specification.

Add a DT node under chosen node to indicate the boot hartid. EFI stub
in Linux kernel will parse this node and pass it to the real kernel
in "a0" before jumping to it.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 arch/riscv/lib/bootm.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c
index fad16901c5f2..b84cc2db2016 100644
--- a/arch/riscv/lib/bootm.c
+++ b/arch/riscv/lib/bootm.c
@@ -28,6 +28,19 @@ __weak void board_quiesce_devices(void)
 
 int arch_fixup_fdt(void *blob)
 {
+	u32 size;
+	int chosen_offset, err;
+
+	size = fdt_totalsize(blob);
+	err  = fdt_open_into(blob, blob, size + 32);
+	if (err < 0) {
+		printf("Device Tree can't be expanded to accmodate new node");
+		return -1;
+	}
+	chosen_offset = fdt_path_offset(blob, "/chosen");
+	fdt_setprop_u64(blob, chosen_offset, "efi-boot-hartid",
+			   gd->arch.boot_hart);
+
 	return 0;
 }
 
-- 
2.24.0

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC PATCH 0/1] Add boot hartid to a Device tree
  2020-02-24 22:19 [RFC PATCH 0/1] Add boot hartid to a Device tree Atish Patra
  2020-02-24 22:19 ` [RFC PATCH 1/1] riscv: Add boot hartid to " Atish Patra
@ 2020-02-24 23:08 ` Heinrich Schuchardt
  2020-02-24 23:35   ` Ard Biesheuvel
  1 sibling, 1 reply; 13+ messages in thread
From: Heinrich Schuchardt @ 2020-02-24 23:08 UTC (permalink / raw)
  To: u-boot

On 2/24/20 11:19 PM, Atish Patra wrote:
> The RISC-V booting protocol requires the hart id to be present in "a0"
> register. This is not a problem for bootm/booti commands as they directly
> jump to Linux kernel. However, bootefi jumps to a EFI boot stub code in
> Linux kernel which acts a loader and jumps to real Linux after terminating
> the boot services. This boot stub code has to be aware of the boot hart id
> so that it can set it in "a0" before jumping to Linux kernel. Currently,
> UEFI protocol doesn't have any mechanism to pass the boot hart id to an
> EFI executable. We should keep it this way as this is a RISC-V specific
> requirement rather than a UEFI requirement. Out of the all possible options,
> device tree seemed to be the best choice to do this job.
> The detailed discussion can be found in the following thread.
>
> https://patchwork.ozlabs.org/patch/1233664/

The above mentioned patch is obsoleted by the new suggestion.

>
> This patch updates the device tree in arch_fixup_fdt() which is common for
> all booting commands. As a result, the DT modification doesn't require any
> efi related arch specific functions and all DT related modifications are
> contained at one place. However, the hart id node will be available for
> Linux even if the kernel is booted using bootm command.
>
> If that is not acceptable, we can always move the code to an efi specific
> function.

Does a related Linux patch already exist?
How about EDK2?

I guess boot loaders like GRUB would not have to care about the extra
property?

Best regards

Heinrich

>
> Atish Patra (1):
> riscv: Add boot hartid to Device tree
>
> arch/riscv/lib/bootm.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> --
> 2.24.0
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC PATCH 0/1] Add boot hartid to a Device tree
  2020-02-24 23:08 ` [RFC PATCH 0/1] Add boot hartid to a " Heinrich Schuchardt
@ 2020-02-24 23:35   ` Ard Biesheuvel
  2020-02-24 23:52     ` Atish Patra
  0 siblings, 1 reply; 13+ messages in thread
From: Ard Biesheuvel @ 2020-02-24 23:35 UTC (permalink / raw)
  To: u-boot

On Tue, 25 Feb 2020 at 00:22, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 2/24/20 11:19 PM, Atish Patra wrote:
> > The RISC-V booting protocol requires the hart id to be present in "a0"
> > register. This is not a problem for bootm/booti commands as they directly
> > jump to Linux kernel. However, bootefi jumps to a EFI boot stub code in
> > Linux kernel which acts a loader and jumps to real Linux after terminating
> > the boot services. This boot stub code has to be aware of the boot hart id
> > so that it can set it in "a0" before jumping to Linux kernel. Currently,
> > UEFI protocol doesn't have any mechanism to pass the boot hart id to an
> > EFI executable. We should keep it this way as this is a RISC-V specific
> > requirement rather than a UEFI requirement. Out of the all possible options,
> > device tree seemed to be the best choice to do this job.
> > The detailed discussion can be found in the following thread.
> >
> > https://patchwork.ozlabs.org/patch/1233664/
>
> The above mentioned patch is obsoleted by the new suggestion.
>
> >
> > This patch updates the device tree in arch_fixup_fdt() which is common for
> > all booting commands. As a result, the DT modification doesn't require any
> > efi related arch specific functions and all DT related modifications are
> > contained at one place. However, the hart id node will be available for
> > Linux even if the kernel is booted using bootm command.
> >
> > If that is not acceptable, we can always move the code to an efi specific
> > function.
>
> Does a related Linux patch already exist?
> How about EDK2?
>

RISC-V is not supported at all yet in EDK2.

> I guess boot loaders like GRUB would not have to care about the extra
> property?
>

Yes, that is basically the point.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC PATCH 1/1] riscv: Add boot hartid to Device tree
  2020-02-24 22:19 ` [RFC PATCH 1/1] riscv: Add boot hartid to " Atish Patra
@ 2020-02-24 23:44   ` Ard Biesheuvel
  0 siblings, 0 replies; 13+ messages in thread
From: Ard Biesheuvel @ 2020-02-24 23:44 UTC (permalink / raw)
  To: u-boot

On Mon, 24 Feb 2020 at 23:20, Atish Patra <atish.patra@wdc.com> wrote:
>
> Linux booting protocol mandates that register "a0" contains the hartid.
> However, U-boot can not pass the hartid via a0 during EFI boot without
> breaking the UEFI specification.
>

It is not about breaking or violating the UEFI specification. It is
about the firmware using a conduit that is already being used to
describe the hardware to the OS to pass an extra piece of information
that the OS needs.

> Add a DT node under chosen node to indicate the boot hartid. EFI stub
> in Linux kernel will parse this node and pass it to the real kernel
> in "a0" before jumping to it.
>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
>  arch/riscv/lib/bootm.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c
> index fad16901c5f2..b84cc2db2016 100644
> --- a/arch/riscv/lib/bootm.c
> +++ b/arch/riscv/lib/bootm.c
> @@ -28,6 +28,19 @@ __weak void board_quiesce_devices(void)
>
>  int arch_fixup_fdt(void *blob)
>  {
> +       u32 size;
> +       int chosen_offset, err;
> +
> +       size = fdt_totalsize(blob);
> +       err  = fdt_open_into(blob, blob, size + 32);
> +       if (err < 0) {
> +               printf("Device Tree can't be expanded to accmodate new node");

'accommodate'

> +               return -1;
> +       }
> +       chosen_offset = fdt_path_offset(blob, "/chosen");
> +       fdt_setprop_u64(blob, chosen_offset, "efi-boot-hartid",

I assume that boot hartid does not change value when you boot via
UEFI, so this should simply be /chosen/boot-hartid

> +                          gd->arch.boot_hart);
> +
>         return 0;
>  }
>
> --
> 2.24.0
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC PATCH 0/1] Add boot hartid to a Device tree
  2020-02-24 23:35   ` Ard Biesheuvel
@ 2020-02-24 23:52     ` Atish Patra
  2020-02-25  8:28       ` Chang, Abner
  0 siblings, 1 reply; 13+ messages in thread
From: Atish Patra @ 2020-02-24 23:52 UTC (permalink / raw)
  To: u-boot

On Mon, Feb 24, 2020 at 3:35 PM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
>
> On Tue, 25 Feb 2020 at 00:22, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >
> > On 2/24/20 11:19 PM, Atish Patra wrote:
> > > The RISC-V booting protocol requires the hart id to be present in "a0"
> > > register. This is not a problem for bootm/booti commands as they directly
> > > jump to Linux kernel. However, bootefi jumps to a EFI boot stub code in
> > > Linux kernel which acts a loader and jumps to real Linux after terminating
> > > the boot services. This boot stub code has to be aware of the boot hart id
> > > so that it can set it in "a0" before jumping to Linux kernel. Currently,
> > > UEFI protocol doesn't have any mechanism to pass the boot hart id to an
> > > EFI executable. We should keep it this way as this is a RISC-V specific
> > > requirement rather than a UEFI requirement. Out of the all possible options,
> > > device tree seemed to be the best choice to do this job.
> > > The detailed discussion can be found in the following thread.
> > >
> > > https://patchwork.ozlabs.org/patch/1233664/
> >
> > The above mentioned patch is obsoleted by the new suggestion.
> >

Thanks for pointing that out to avoid confusion.

> > >
> > > This patch updates the device tree in arch_fixup_fdt() which is common for
> > > all booting commands. As a result, the DT modification doesn't require any
> > > efi related arch specific functions and all DT related modifications are
> > > contained at one place. However, the hart id node will be available for
> > > Linux even if the kernel is booted using bootm command.
> > >
> > > If that is not acceptable, we can always move the code to an efi specific
> > > function.
> >
> > Does a related Linux patch already exist?

Yes. But in my local tree ;). It will be included in RISC-V EFI stub
support series which I am planning to post in a couple of days.

> > How about EDK2?
> >
>
> RISC-V is not supported at all yet in EDK2.
>

The EDK2 patches are out there and reviewed. I guess it will be
available in mainline EDK2 pretty soon.
Abner agreed that similar patch can be added to EDK2 as well in the
previous thread.

> > I guess boot loaders like GRUB would not have to care about the extra
> > property?
> >
>
> Yes, that is basically the point.



-- 
Regards,
Atish

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC PATCH 0/1] Add boot hartid to a Device tree
  2020-02-24 23:52     ` Atish Patra
@ 2020-02-25  8:28       ` Chang, Abner
  2020-02-25  8:48         ` Ard Biesheuvel
  0 siblings, 1 reply; 13+ messages in thread
From: Chang, Abner @ 2020-02-25  8:28 UTC (permalink / raw)
  To: u-boot



> -----Original Message-----
> From: Atish Patra [mailto:atishp at atishpatra.org]
> Sent: Tuesday, February 25, 2020 7:52 AM
> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>; Atish Patra
> <atish.patra@wdc.com>; U-Boot Mailing List <u-boot@lists.denx.de>;
> Alexander Graf <agraf@csgraf.de>; Anup Patel <anup.patel@wdc.com>; Bin
> Meng <bmeng.cn@gmail.com>; Joe Hershberger
> <joe.hershberger@ni.com>; Loic Pallardy <loic.pallardy@st.com>; Lukas Auer
> <lukas.auer@aisec.fraunhofer.de>; Marek Beh?n <marek.behun@nic.cz>;
> Marek Vasut <marek.vasut@gmail.com>; Patrick Delaunay
> <patrick.delaunay@st.com>; Peng Fan <peng.fan@nxp.com>; Philippe
> Reynes <philippe.reynes@softathome.com>; Simon Glass
> <sjg@chromium.org>; Simon Goldschmidt
> <simon.k.r.goldschmidt@gmail.com>; Stefano Babic <sbabic@denx.de>;
> Thierry Reding <treding@nvidia.com>; Tom Rini <trini@konsulko.com>;
> leif at nuviainc.com; Chang, Abner (HPS SW/FW Technologist)
> <abner.chang@hpe.com>; Schaefer, Daniel (DualStudy)
> <daniel.schaefer@hpe.com>
> Subject: Re: [RFC PATCH 0/1] Add boot hartid to a Device tree
> 
> On Mon, Feb 24, 2020 at 3:35 PM Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
> >
> > On Tue, 25 Feb 2020 at 00:22, Heinrich Schuchardt <xypron.glpk@gmx.de>
> wrote:
> > >
> > > On 2/24/20 11:19 PM, Atish Patra wrote:
> > > > The RISC-V booting protocol requires the hart id to be present in "a0"
> > > > register. This is not a problem for bootm/booti commands as they
> > > > directly jump to Linux kernel. However, bootefi jumps to a EFI
> > > > boot stub code in Linux kernel which acts a loader and jumps to
> > > > real Linux after terminating the boot services. This boot stub
> > > > code has to be aware of the boot hart id so that it can set it in
> > > > "a0" before jumping to Linux kernel. Currently, UEFI protocol
> > > > doesn't have any mechanism to pass the boot hart id to an EFI
> > > > executable. We should keep it this way as this is a RISC-V
> > > > specific requirement rather than a UEFI requirement. Out of the all
> possible options, device tree seemed to be the best choice to do this job.
> > > > The detailed discussion can be found in the following thread.
> > > >
> > > > INVALID URI REMOVED
> > > >
> abs.org_patch_1233664_&d=DwIBaQ&c=C5b8zRQO1miGmBeVZ2LFWg&r=_S
> N6FZB
> > > >
> N4Vgi4Ulkskz6qU3NYRO03nHp9P7Z5q59A3E&m=J8GY_HS3fV_cJH9duXP739y
> 0hDK
> > > > 3qfHGNx2Dpcf-UBY&s=iVpRlpTOME_A-
> O5STNbXXawkW24gxy2yi56Q8AtZ2bI&e=
> > >
> > > The above mentioned patch is obsoleted by the new suggestion.
> > >
> 
> Thanks for pointing that out to avoid confusion.
> 
> > > >
> > > > This patch updates the device tree in arch_fixup_fdt() which is
> > > > common for all booting commands. As a result, the DT modification
> > > > doesn't require any efi related arch specific functions and all DT
> > > > related modifications are contained at one place. However, the
> > > > hart id node will be available for Linux even if the kernel is booted using
> bootm command.
> > > >
> > > > If that is not acceptable, we can always move the code to an efi
> > > > specific function.
> > >
> > > Does a related Linux patch already exist?
> 
> Yes. But in my local tree ;). It will be included in RISC-V EFI stub support series
> which I am planning to post in a couple of days.
> 
> > > How about EDK2?
> > >
> >
> > RISC-V is not supported at all yet in EDK2.
> >
> 
> The EDK2 patches are out there and reviewed. I guess it will be available in
> mainline EDK2 pretty soon.
Yes, currently we are working on edk2 CI testing for RISCV64 arch.  We hope edk2 RISC-V port could be in mainstream in Mar.

> Abner agreed that similar patch can be added to EDK2 as well in the previous
> thread.
Yes, this will be another TODO for edk2 to add similar DT changes if we want to boot system from edk2 firmware to EFI Stub and Linux kernel. We do not have that so far.

> 
> > > I guess boot loaders like GRUB would not have to care about the
> > > extra property?
> > >
> >
> > Yes, that is basically the point.
> 
> 
> 
> --
> Regards,
> Atish

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC PATCH 0/1] Add boot hartid to a Device tree
  2020-02-25  8:28       ` Chang, Abner
@ 2020-02-25  8:48         ` Ard Biesheuvel
  2020-02-25  8:59           ` Chang, Abner
  0 siblings, 1 reply; 13+ messages in thread
From: Ard Biesheuvel @ 2020-02-25  8:48 UTC (permalink / raw)
  To: u-boot

On Tue, 25 Feb 2020 at 09:28, Chang, Abner (HPS SW/FW Technologist)
<abner.chang@hpe.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Atish Patra [mailto:atishp at atishpatra.org]

<snip header soup>

> >
> > On Mon, Feb 24, 2020 at 3:35 PM Ard Biesheuvel
> > <ard.biesheuvel@linaro.org> wrote:
> > >
> > > On Tue, 25 Feb 2020 at 00:22, Heinrich Schuchardt <xypron.glpk@gmx.de>
> > wrote:
> > > >
> > > > On 2/24/20 11:19 PM, Atish Patra wrote:
> > > > > The RISC-V booting protocol requires the hart id to be present in "a0"
> > > > > register. This is not a problem for bootm/booti commands as they
> > > > > directly jump to Linux kernel. However, bootefi jumps to a EFI
> > > > > boot stub code in Linux kernel which acts a loader and jumps to
> > > > > real Linux after terminating the boot services. This boot stub
> > > > > code has to be aware of the boot hart id so that it can set it in
> > > > > "a0" before jumping to Linux kernel. Currently, UEFI protocol
> > > > > doesn't have any mechanism to pass the boot hart id to an EFI
> > > > > executable. We should keep it this way as this is a RISC-V
> > > > > specific requirement rather than a UEFI requirement. Out of the all
> > possible options, device tree seemed to be the best choice to do this job.
> > > > > The detailed discussion can be found in the following thread.
> > > > >
> > > > > INVALID URI REMOVED
> > > > >
> > abs.org_patch_1233664_&d=DwIBaQ&c=C5b8zRQO1miGmBeVZ2LFWg&r=_S
> > N6FZB
> > > > >
> > N4Vgi4Ulkskz6qU3NYRO03nHp9P7Z5q59A3E&m=J8GY_HS3fV_cJH9duXP739y
> > 0hDK
> > > > > 3qfHGNx2Dpcf-UBY&s=iVpRlpTOME_A-
> > O5STNbXXawkW24gxy2yi56Q8AtZ2bI&e=
> > > >
> > > > The above mentioned patch is obsoleted by the new suggestion.
> > > >
> >
> > Thanks for pointing that out to avoid confusion.
> >
> > > > >
> > > > > This patch updates the device tree in arch_fixup_fdt() which is
> > > > > common for all booting commands. As a result, the DT modification
> > > > > doesn't require any efi related arch specific functions and all DT
> > > > > related modifications are contained at one place. However, the
> > > > > hart id node will be available for Linux even if the kernel is booted using
> > bootm command.
> > > > >
> > > > > If that is not acceptable, we can always move the code to an efi
> > > > > specific function.
> > > >
> > > > Does a related Linux patch already exist?
> >
> > Yes. But in my local tree ;). It will be included in RISC-V EFI stub support series
> > which I am planning to post in a couple of days.
> >
> > > > How about EDK2?
> > > >
> > >
> > > RISC-V is not supported at all yet in EDK2.
> > >
> >
> > The EDK2 patches are out there and reviewed. I guess it will be available in
> > mainline EDK2 pretty soon.
> Yes, currently we are working on edk2 CI testing for RISCV64 arch.  We hope edk2 RISC-V port could be in mainstream in Mar.
>

Excellent! Is this core support? Or do you have a platform implemented
as well that can be upstreamed?

> > Abner agreed that similar patch can be added to EDK2 as well in the previous
> > thread.
> Yes, this will be another TODO for edk2 to add similar DT changes if we want to boot system from edk2 firmware to EFI Stub and Linux kernel. We do not have that so far.
>
> >
> > > > I guess boot loaders like GRUB would not have to care about the
> > > > extra property?
> > > >
> > >
> > > Yes, that is basically the point.
> >
> >
> >
> > --
> > Regards,
> > Atish

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC PATCH 0/1] Add boot hartid to a Device tree
  2020-02-25  8:48         ` Ard Biesheuvel
@ 2020-02-25  8:59           ` Chang, Abner
  2020-02-25  9:07             ` Ard Biesheuvel
  0 siblings, 1 reply; 13+ messages in thread
From: Chang, Abner @ 2020-02-25  8:59 UTC (permalink / raw)
  To: u-boot



> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel at linaro.org]
> Sent: Tuesday, February 25, 2020 4:48 PM
> To: Chang, Abner (HPS SW/FW Technologist) <abner.chang@hpe.com>
> Cc: Atish Patra <atishp@atishpatra.org>; Heinrich Schuchardt
> <xypron.glpk@gmx.de>; Atish Patra <atish.patra@wdc.com>; U-Boot Mailing
> List <u-boot@lists.denx.de>; Alexander Graf <agraf@csgraf.de>; Anup Patel
> <anup.patel@wdc.com>; Bin Meng <bmeng.cn@gmail.com>; Joe
> Hershberger <joe.hershberger@ni.com>; Loic Pallardy
> <loic.pallardy@st.com>; Lukas Auer <lukas.auer@aisec.fraunhofer.de>;
> Marek Beh?n <marek.behun@nic.cz>; Marek Vasut
> <marek.vasut@gmail.com>; Patrick Delaunay <patrick.delaunay@st.com>;
> Peng Fan <peng.fan@nxp.com>; Philippe Reynes
> <philippe.reynes@softathome.com>; Simon Glass <sjg@chromium.org>;
> Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>; Stefano Babic
> <sbabic@denx.de>; Thierry Reding <treding@nvidia.com>; Tom Rini
> <trini@konsulko.com>; leif at nuviainc.com; Schaefer, Daniel (DualStudy)
> <daniel.schaefer@hpe.com>
> Subject: Re: [RFC PATCH 0/1] Add boot hartid to a Device tree
> 
> On Tue, 25 Feb 2020 at 09:28, Chang, Abner (HPS SW/FW Technologist)
> <abner.chang@hpe.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Atish Patra [mailto:atishp at atishpatra.org]
> 
> <snip header soup>
> 
> > >
> > > On Mon, Feb 24, 2020 at 3:35 PM Ard Biesheuvel
> > > <ard.biesheuvel@linaro.org> wrote:
> > > >
> > > > On Tue, 25 Feb 2020 at 00:22, Heinrich Schuchardt
> > > > <xypron.glpk@gmx.de>
> > > wrote:
> > > > >
> > > > > On 2/24/20 11:19 PM, Atish Patra wrote:
> > > > > > The RISC-V booting protocol requires the hart id to be present in
> "a0"
> > > > > > register. This is not a problem for bootm/booti commands as
> > > > > > they directly jump to Linux kernel. However, bootefi jumps to
> > > > > > a EFI boot stub code in Linux kernel which acts a loader and
> > > > > > jumps to real Linux after terminating the boot services. This
> > > > > > boot stub code has to be aware of the boot hart id so that it
> > > > > > can set it in "a0" before jumping to Linux kernel. Currently,
> > > > > > UEFI protocol doesn't have any mechanism to pass the boot hart
> > > > > > id to an EFI executable. We should keep it this way as this is
> > > > > > a RISC-V specific requirement rather than a UEFI requirement.
> > > > > > Out of the all
> > > possible options, device tree seemed to be the best choice to do this job.
> > > > > > The detailed discussion can be found in the following thread.
> > > > > >
> > > > > > INVALID URI REMOVED
> > > > > >
> > >
> abs.org_patch_1233664_&d=DwIBaQ&c=C5b8zRQO1miGmBeVZ2LFWg&r=_S
> > > N6FZB
> > > > > >
> > >
> N4Vgi4Ulkskz6qU3NYRO03nHp9P7Z5q59A3E&m=J8GY_HS3fV_cJH9duXP739y
> > > 0hDK
> > > > > > 3qfHGNx2Dpcf-UBY&s=iVpRlpTOME_A-
> > > O5STNbXXawkW24gxy2yi56Q8AtZ2bI&e=
> > > > >
> > > > > The above mentioned patch is obsoleted by the new suggestion.
> > > > >
> > >
> > > Thanks for pointing that out to avoid confusion.
> > >
> > > > > >
> > > > > > This patch updates the device tree in arch_fixup_fdt() which
> > > > > > is common for all booting commands. As a result, the DT
> > > > > > modification doesn't require any efi related arch specific
> > > > > > functions and all DT related modifications are contained at
> > > > > > one place. However, the hart id node will be available for
> > > > > > Linux even if the kernel is booted using
> > > bootm command.
> > > > > >
> > > > > > If that is not acceptable, we can always move the code to an
> > > > > > efi specific function.
> > > > >
> > > > > Does a related Linux patch already exist?
> > >
> > > Yes. But in my local tree ;). It will be included in RISC-V EFI stub
> > > support series which I am planning to post in a couple of days.
> > >
> > > > > How about EDK2?
> > > > >
> > > >
> > > > RISC-V is not supported at all yet in EDK2.
> > > >
> > >
> > > The EDK2 patches are out there and reviewed. I guess it will be
> > > available in mainline EDK2 pretty soon.
> > Yes, currently we are working on edk2 CI testing for RISCV64 arch.  We
> hope edk2 RISC-V port could be in mainstream in Mar.
> >
> 
> Excellent! Is this core support? Or do you have a platform implemented as
> well that can be upstreamed?
Yes we do have platform implementations to be upstreamed, below is the latest status of RISC-V edk2 port. We will have to update status later because we just merged OpenSBI tag 0.6 to edk2 RISC-V.
https://github.com/riscv/riscv-uefi-edk2-docs

> 
> > > Abner agreed that similar patch can be added to EDK2 as well in the
> > > previous thread.
> > Yes, this will be another TODO for edk2 to add similar DT changes if we
> want to boot system from edk2 firmware to EFI Stub and Linux kernel. We do
> not have that so far.
> >
> > >
> > > > > I guess boot loaders like GRUB would not have to care about the
> > > > > extra property?
> > > > >
> > > >
> > > > Yes, that is basically the point.
> > >
> > >
> > >
> > > --
> > > Regards,
> > > Atish

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC PATCH 0/1] Add boot hartid to a Device tree
  2020-02-25  8:59           ` Chang, Abner
@ 2020-02-25  9:07             ` Ard Biesheuvel
  2020-03-05  3:22               ` Schaefer, Daniel
  0 siblings, 1 reply; 13+ messages in thread
From: Ard Biesheuvel @ 2020-02-25  9:07 UTC (permalink / raw)
  To: u-boot

On Tue, 25 Feb 2020 at 09:59, Chang, Abner (HPS SW/FW Technologist)
<abner.chang@hpe.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Ard Biesheuvel [mailto:ard.biesheuvel at linaro.org]
> > Sent: Tuesday, February 25, 2020 4:48 PM
> > To: Chang, Abner (HPS SW/FW Technologist) <abner.chang@hpe.com>
> > Cc: Atish Patra <atishp@atishpatra.org>; Heinrich Schuchardt
> > <xypron.glpk@gmx.de>; Atish Patra <atish.patra@wdc.com>; U-Boot Mailing
> > List <u-boot@lists.denx.de>; Alexander Graf <agraf@csgraf.de>; Anup Patel
> > <anup.patel@wdc.com>; Bin Meng <bmeng.cn@gmail.com>; Joe
> > Hershberger <joe.hershberger@ni.com>; Loic Pallardy
> > <loic.pallardy@st.com>; Lukas Auer <lukas.auer@aisec.fraunhofer.de>;
> > Marek Beh?n <marek.behun@nic.cz>; Marek Vasut
> > <marek.vasut@gmail.com>; Patrick Delaunay <patrick.delaunay@st.com>;
> > Peng Fan <peng.fan@nxp.com>; Philippe Reynes
> > <philippe.reynes@softathome.com>; Simon Glass <sjg@chromium.org>;
> > Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>; Stefano Babic
> > <sbabic@denx.de>; Thierry Reding <treding@nvidia.com>; Tom Rini
> > <trini@konsulko.com>; leif at nuviainc.com; Schaefer, Daniel (DualStudy)
> > <daniel.schaefer@hpe.com>
> > Subject: Re: [RFC PATCH 0/1] Add boot hartid to a Device tree
> >
> > On Tue, 25 Feb 2020 at 09:28, Chang, Abner (HPS SW/FW Technologist)
> > <abner.chang@hpe.com> wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Atish Patra [mailto:atishp at atishpatra.org]
> >
> > <snip header soup>
> >
> > > >
> > > > On Mon, Feb 24, 2020 at 3:35 PM Ard Biesheuvel
> > > > <ard.biesheuvel@linaro.org> wrote:
> > > > >
> > > > > On Tue, 25 Feb 2020 at 00:22, Heinrich Schuchardt
> > > > > <xypron.glpk@gmx.de>
> > > > wrote:
> > > > > >
> > > > > > On 2/24/20 11:19 PM, Atish Patra wrote:
> > > > > > > The RISC-V booting protocol requires the hart id to be present in
> > "a0"
> > > > > > > register. This is not a problem for bootm/booti commands as
> > > > > > > they directly jump to Linux kernel. However, bootefi jumps to
> > > > > > > a EFI boot stub code in Linux kernel which acts a loader and
> > > > > > > jumps to real Linux after terminating the boot services. This
> > > > > > > boot stub code has to be aware of the boot hart id so that it
> > > > > > > can set it in "a0" before jumping to Linux kernel. Currently,
> > > > > > > UEFI protocol doesn't have any mechanism to pass the boot hart
> > > > > > > id to an EFI executable. We should keep it this way as this is
> > > > > > > a RISC-V specific requirement rather than a UEFI requirement.
> > > > > > > Out of the all
> > > > possible options, device tree seemed to be the best choice to do this job.
> > > > > > > The detailed discussion can be found in the following thread.
> > > > > > >
> > > > > > > INVALID URI REMOVED
> > > > > > >
> > > >
> > abs.org_patch_1233664_&d=DwIBaQ&c=C5b8zRQO1miGmBeVZ2LFWg&r=_S
> > > > N6FZB
> > > > > > >
> > > >
> > N4Vgi4Ulkskz6qU3NYRO03nHp9P7Z5q59A3E&m=J8GY_HS3fV_cJH9duXP739y
> > > > 0hDK
> > > > > > > 3qfHGNx2Dpcf-UBY&s=iVpRlpTOME_A-
> > > > O5STNbXXawkW24gxy2yi56Q8AtZ2bI&e=
> > > > > >
> > > > > > The above mentioned patch is obsoleted by the new suggestion.
> > > > > >
> > > >
> > > > Thanks for pointing that out to avoid confusion.
> > > >
> > > > > > >
> > > > > > > This patch updates the device tree in arch_fixup_fdt() which
> > > > > > > is common for all booting commands. As a result, the DT
> > > > > > > modification doesn't require any efi related arch specific
> > > > > > > functions and all DT related modifications are contained at
> > > > > > > one place. However, the hart id node will be available for
> > > > > > > Linux even if the kernel is booted using
> > > > bootm command.
> > > > > > >
> > > > > > > If that is not acceptable, we can always move the code to an
> > > > > > > efi specific function.
> > > > > >
> > > > > > Does a related Linux patch already exist?
> > > >
> > > > Yes. But in my local tree ;). It will be included in RISC-V EFI stub
> > > > support series which I am planning to post in a couple of days.
> > > >
> > > > > > How about EDK2?
> > > > > >
> > > > >
> > > > > RISC-V is not supported at all yet in EDK2.
> > > > >
> > > >
> > > > The EDK2 patches are out there and reviewed. I guess it will be
> > > > available in mainline EDK2 pretty soon.
> > > Yes, currently we are working on edk2 CI testing for RISCV64 arch.  We
> > hope edk2 RISC-V port could be in mainstream in Mar.
> > >
> >
> > Excellent! Is this core support? Or do you have a platform implemented as
> > well that can be upstreamed?
> Yes we do have platform implementations to be upstreamed, below is the latest status of RISC-V edk2 port. We will have to update status later because we just merged OpenSBI tag 0.6 to edk2 RISC-V.
> https://github.com/riscv/riscv-uefi-edk2-docs
>

Good to know! I saw some patches going by on the mailing list, but it
is hard to derive the current state of affairs from that.

I'm glad to see you did not make the same mistake we made on ARM and
omit the PEI phase entirely.

What I did notice is the use of APRIORI PEI and APRIORI DXE sections
in your platform descriptions. I recommend you try to avoid that, as
it is a maintenance burden going forward: instead, please use dummy
protocols and NULL library class resolutions if you need to make
generic components depend on platform specific protocols. Also, please
document this - the APRIORI section does not explain *why* you have to
circumvent the ordinary dependency tree based module dispatch.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC PATCH 0/1] Add boot hartid to a Device tree
  2020-02-25  9:07             ` Ard Biesheuvel
@ 2020-03-05  3:22               ` Schaefer, Daniel
  2020-03-05  7:35                 ` Ard Biesheuvel
  2020-03-06  0:34                 ` Atish Patra
  0 siblings, 2 replies; 13+ messages in thread
From: Schaefer, Daniel @ 2020-03-05  3:22 UTC (permalink / raw)
  To: u-boot

Hi,

I have started to implement the corresponding changes in EDK2: https://github.com/changab/edk2-staging-riscv/compare/5f63e9249751ccb9302514455b9a1a7038f34547...RISC-V-DT-fixup
What happens is: The DTB is embedded in the FW image and passed to sbi_init in SEC phase. We initialize OpenSBI as early as possible and because it also wants to modify the device tree, I have to pass it the DTB as next_arg1.
Then it's passed to PEI in the firmware context and further to DXE via a HOB. In DXE the boot-hartid is inserted and it's installed into the EFI system config table from where the EFISTUB reads it.

Unfortunately I don't get any console output after the EFISTUB calls ExitBootServices, so my patch is still WIP.
Atish, which platform file in OpenSBI did you use to test your changes? platform/qemu/virt/platform.c or platform/sifive/fu540/platform.c ?
Maybe the failure is unrelated to the new patches - we've never booted Linux from EDKII yet.


I'm a bit skeptical whether DT is the best way to pass the boothartid to the kernel. Ard has argued that it's good because it's independent from UEFI, but the proper kernel doesn't read the hartid from there - it gets it from a0. Only the EFISTUB reads the hartid from the device tree. Therefore the solution we need is EFI specific anyways, right?
One of the design goals is that we don't want to force bootloaders, which load the EFISTUB (e.g. UEFI Shell, grub chainloading, systemd-boot), to change.

If we let the firmware embed the hartid in the DT, the user cannot swap out the DT later for their custom one. They need to use the one given by the firmware.
Of course we could add commands and config to bootloaders to load and fixup (embed hartid) the device tree... but, as mentioned earlier, we want to avoid that.
Additionally from EDKII side we're also planning to run later stages, including the bootloader, in S-Mode, where they wouldn't even have access to mhartid and thus couldn't fixup the DT.

If instead the firmware writes the hartid into the EFI system config table, the EFISTUB can read it from there, just like it does the device tree already. Then bootloaders can put a user supplied DT in the config table, too, like they always have.

What do you all think - does that make sense?

- Daniel
On 2/25/20 10:07 AM, Ard Biesheuvel wrote:

On Tue, 25 Feb 2020 at 09:59, Chang, Abner (HPS SW/FW Technologist)
<abner.chang@hpe.com><mailto:abner.chang@hpe.com> wrote:


-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel at linaro.org]
Sent: Tuesday, February 25, 2020 4:48 PM
To: Chang, Abner (HPS SW/FW Technologist) <abner.chang@hpe.com><mailto:abner.chang@hpe.com>
Cc: Atish Patra <atishp@atishpatra.org><mailto:atishp@atishpatra.org>; Heinrich Schuchardt
<xypron.glpk@gmx.de><mailto:xypron.glpk@gmx.de>; Atish Patra <atish.patra@wdc.com><mailto:atish.patra@wdc.com>; U-Boot Mailing
List <u-boot@lists.denx.de><mailto:u-boot@lists.denx.de>; Alexander Graf <agraf@csgraf.de><mailto:agraf@csgraf.de>; Anup Patel
<anup.patel@wdc.com><mailto:anup.patel@wdc.com>; Bin Meng <bmeng.cn@gmail.com><mailto:bmeng.cn@gmail.com>; Joe
Hershberger <joe.hershberger@ni.com><mailto:joe.hershberger@ni.com>; Loic Pallardy
<loic.pallardy@st.com><mailto:loic.pallardy@st.com>; Lukas Auer <lukas.auer@aisec.fraunhofer.de><mailto:lukas.auer@aisec.fraunhofer.de>;
Marek Beh?n <marek.behun@nic.cz><mailto:marek.behun@nic.cz>; Marek Vasut
<marek.vasut@gmail.com><mailto:marek.vasut@gmail.com>; Patrick Delaunay <patrick.delaunay@st.com><mailto:patrick.delaunay@st.com>;
Peng Fan <peng.fan@nxp.com><mailto:peng.fan@nxp.com>; Philippe Reynes
<philippe.reynes@softathome.com><mailto:philippe.reynes@softathome.com>; Simon Glass <sjg@chromium.org><mailto:sjg@chromium.org>;
Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com><mailto:simon.k.r.goldschmidt@gmail.com>; Stefano Babic
<sbabic@denx.de><mailto:sbabic@denx.de>; Thierry Reding <treding@nvidia.com><mailto:treding@nvidia.com>; Tom Rini
<trini@konsulko.com><mailto:trini@konsulko.com>; leif at nuviainc.com<mailto:leif@nuviainc.com>; Schaefer, Daniel (DualStudy)
<daniel.schaefer@hpe.com><mailto:daniel.schaefer@hpe.com>
Subject: Re: [RFC PATCH 0/1] Add boot hartid to a Device tree

On Tue, 25 Feb 2020 at 09:28, Chang, Abner (HPS SW/FW Technologist)
<abner.chang@hpe.com><mailto:abner.chang@hpe.com> wrote:


-----Original Message-----
From: Atish Patra [mailto:atishp at atishpatra.org]


<snip header soup>



On Mon, Feb 24, 2020 at 3:35 PM Ard Biesheuvel
<ard.biesheuvel@linaro.org><mailto:ard.biesheuvel@linaro.org> wrote:


On Tue, 25 Feb 2020 at 00:22, Heinrich Schuchardt
<xypron.glpk@gmx.de><mailto:xypron.glpk@gmx.de>


wrote:


On 2/24/20 11:19 PM, Atish Patra wrote:


The RISC-V booting protocol requires the hart id to be present in


"a0"


register. This is not a problem for bootm/booti commands as
they directly jump to Linux kernel. However, bootefi jumps to
a EFI boot stub code in Linux kernel which acts a loader and
jumps to real Linux after terminating the boot services. This
boot stub code has to be aware of the boot hart id so that it
can set it in "a0" before jumping to Linux kernel. Currently,
UEFI protocol doesn't have any mechanism to pass the boot hart
id to an EFI executable. We should keep it this way as this is
a RISC-V specific requirement rather than a UEFI requirement.
Out of the all


possible options, device tree seemed to be the best choice to do this job.


The detailed discussion can be found in the following thread.

INVALID URI REMOVED



abs.org_patch_1233664_&d=DwIBaQ&c=C5b8zRQO1miGmBeVZ2LFWg&r=_S


N6FZB


N4Vgi4Ulkskz6qU3NYRO03nHp9P7Z5q59A3E&m=J8GY_HS3fV_cJH9duXP739y


0hDK


3qfHGNx2Dpcf-UBY&s=iVpRlpTOME_A-


O5STNbXXawkW24gxy2yi56Q8AtZ2bI&e=


The above mentioned patch is obsoleted by the new suggestion.



Thanks for pointing that out to avoid confusion.



This patch updates the device tree in arch_fixup_fdt() which
is common for all booting commands. As a result, the DT
modification doesn't require any efi related arch specific
functions and all DT related modifications are contained at
one place. However, the hart id node will be available for
Linux even if the kernel is booted using


bootm command.


If that is not acceptable, we can always move the code to an
efi specific function.


Does a related Linux patch already exist?


Yes. But in my local tree ;). It will be included in RISC-V EFI stub
support series which I am planning to post in a couple of days.



How about EDK2?



RISC-V is not supported at all yet in EDK2.



The EDK2 patches are out there and reviewed. I guess it will be
available in mainline EDK2 pretty soon.


Yes, currently we are working on edk2 CI testing for RISCV64 arch.  We


hope edk2 RISC-V port could be in mainstream in Mar.


Excellent! Is this core support? Or do you have a platform implemented as
well that can be upstreamed?


Yes we do have platform implementations to be upstreamed, below is the latest status of RISC-V edk2 port. We will have to update status later because we just merged OpenSBI tag 0.6 to edk2 RISC-V.
https://github.com/riscv/riscv-uefi-edk2-docs



Good to know! I saw some patches going by on the mailing list, but it
is hard to derive the current state of affairs from that.

I'm glad to see you did not make the same mistake we made on ARM and
omit the PEI phase entirely.

What I did notice is the use of APRIORI PEI and APRIORI DXE sections
in your platform descriptions. I recommend you try to avoid that, as
it is a maintenance burden going forward: instead, please use dummy
protocols and NULL library class resolutions if you need to make
generic components depend on platform specific protocols. Also, please
document this - the APRIORI section does not explain *why* you have to
circumvent the ordinary dependency tree based module dispatch.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC PATCH 0/1] Add boot hartid to a Device tree
  2020-03-05  3:22               ` Schaefer, Daniel
@ 2020-03-05  7:35                 ` Ard Biesheuvel
  2020-03-06  0:34                 ` Atish Patra
  1 sibling, 0 replies; 13+ messages in thread
From: Ard Biesheuvel @ 2020-03-05  7:35 UTC (permalink / raw)
  To: u-boot

On Thu, 5 Mar 2020 at 04:28, Schaefer, Daniel (DualStudy)
<daniel.schaefer@hpe.com> wrote:
>
> Hi,
>
> I have started to implement the corresponding changes in EDK2: https://github.com/changab/edk2-staging-riscv/compare/5f63e9249751ccb9302514455b9a1a7038f34547...RISC-V-DT-fixup
> What happens is: The DTB is embedded in the FW image and passed to sbi_init in SEC phase. We initialize OpenSBI as early as possible and because it also wants to modify the device tree, I have to pass it the DTB as next_arg1.
> Then it's passed to PEI in the firmware context and further to DXE via a HOB. In DXE the boot-hartid is inserted and it's installed into the EFI system config table from where the EFISTUB reads it.
>
> Unfortunately I don't get any console output after the EFISTUB calls ExitBootServices, so my patch is still WIP.
> Atish, which platform file in OpenSBI did you use to test your changes? platform/qemu/virt/platform.c or platform/sifive/fu540/platform.c ?
> Maybe the failure is unrelated to the new patches - we've never booted Linux from EDKII yet.
>
>
> I'm a bit skeptical whether DT is the best way to pass the boothartid to the kernel. Ard has argued that it's good because it's independent from UEFI, but the proper kernel doesn't read the hartid from there - it gets it from a0. Only the EFISTUB reads the hartid from the device tree. Therefore the solution we need is EFI specific anyways, right?
> One of the design goals is that we don't want to force bootloaders, which load the EFISTUB (e.g. UEFI Shell, grub chainloading, systemd-boot), to change.
>
> If we let the firmware embed the hartid in the DT, the user cannot swap out the DT later for their custom one. They need to use the one given by the firmware.
> Of course we could add commands and config to bootloaders to load and fixup (embed hartid) the device tree... but, as mentioned earlier, we want to avoid that.
> Additionally from EDKII side we're also planning to run later stages, including the bootloader, in S-Mode, where they wouldn't even have access to mhartid and thus couldn't fixup the DT.
>
> If instead the firmware writes the hartid into the EFI system config table, the EFISTUB can read it from there, just like it does the device tree already. Then bootloaders can put a user supplied DT in the config table, too, like they always have.
>
> What do you all think - does that make sense?
>

It doesn't really matter from the EFI stub pov if the information
comes from the DT or from another config table. But the idea that
RISC-V is 'special', and that you have to pass *two* pieces of
information around instead of just one like on every other platform is
something that you should really think about carefully - there may be
other places where it will bite you down the road.

Also, the idea that a DT is a file that you ship with the kernel is
giving a *lot* of grief in the ARM world. The DT describes the
hardware, not the software, and so it is the responsibility of the
platform to provide it. If the platform gives you some way to drop in
another version of the DT in one way or the other, that is absolutely
fine, but in this case, the platform should know how to update
/chosen/boot-hartid (which it already can, in any case). But don't
make it part of the boot protocol, it really doesn't belong there.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC PATCH 0/1] Add boot hartid to a Device tree
  2020-03-05  3:22               ` Schaefer, Daniel
  2020-03-05  7:35                 ` Ard Biesheuvel
@ 2020-03-06  0:34                 ` Atish Patra
  1 sibling, 0 replies; 13+ messages in thread
From: Atish Patra @ 2020-03-06  0:34 UTC (permalink / raw)
  To: u-boot

On Wed, Mar 4, 2020 at 7:22 PM Schaefer, Daniel (DualStudy)
<daniel.schaefer@hpe.com> wrote:
>
> Hi,
>
> I have started to implement the corresponding changes in EDK2: https://github.com/changab/edk2-staging-riscv/compare/5f63e9249751ccb9302514455b9a1a7038f34547...RISC-V-DT-fixup
> What happens is: The DTB is embedded in the FW image and passed to sbi_init in SEC phase. We initialize OpenSBI as early as possible and because it also wants to modify the device tree, I have to pass it the DTB as next_arg1.
> Then it's passed to PEI in the firmware context and further to DXE via a HOB. In DXE the boot-hartid is inserted and it's installed into the EFI system config table from where the EFISTUB reads it.
>
> Unfortunately I don't get any console output after the EFISTUB calls ExitBootServices, so my patch is still WIP.
> Atish, which platform file in OpenSBI did you use to test your changes? platform/qemu/virt/platform.c or platform/sifive/fu540/platform.c ?

Both. I have verified bootefi boot on Qemu and Unleashed.

> Maybe the failure is unrelated to the new patches - we've never booted Linux from EDKII yet.
>

I have never tried EDKII patches as well. I will give it a try. You
can add a quick hack can be added in OpenSBI to add the chosen node
easily.
By doing that, you ensure that EDK2 is unchanged and try my EFI stub
patches. I might have missed something in EFI stub as well :).

>
> I'm a bit skeptical whether DT is the best way to pass the boothartid to the kernel. Ard has argued that it's good because it's independent from UEFI, but the proper kernel doesn't read the hartid from there - it gets it from a0. Only the EFISTUB reads the hartid from the device tree. Therefore the solution we need is EFI specific anyways, right?

yes.

> One of the design goals is that we don't want to force bootloaders, which load the EFISTUB (e.g. UEFI Shell, grub chainloading, systemd-boot), to change.
>

I don't know how systemd-boot works but grub doesn't need to modify
DT. The stage loading the grub must have already made that
modification.

> If we let the firmware embed the hartid in the DT, the user cannot swap out the DT later for their custom one. They need to use the one given by the firmware.
> Of course we could add commands and config to bootloaders to load and fixup (embed hartid) the device tree... but, as mentioned earlier, we want to avoid that.
> Additionally from EDKII side we're also planning to run later stages, including the bootloader, in S-Mode, where they wouldn't even have access to mhartid and thus couldn't fixup the DT.
>
> If instead the firmware writes the hartid into the EFI system config table, the EFISTUB can read it from there, just like it does the device tree already. Then bootloaders can put a user supplied DT in the config table, too, like they always have.
>
> What do you all think - does that make sense?
>
> - Daniel
> On 2/25/20 10:07 AM, Ard Biesheuvel wrote:
>
> On Tue, 25 Feb 2020 at 09:59, Chang, Abner (HPS SW/FW Technologist)
> <abner.chang@hpe.com> wrote:
>
> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel at linaro.org]
> Sent: Tuesday, February 25, 2020 4:48 PM
> To: Chang, Abner (HPS SW/FW Technologist) <abner.chang@hpe.com>
> Cc: Atish Patra <atishp@atishpatra.org>; Heinrich Schuchardt
> <xypron.glpk@gmx.de>; Atish Patra <atish.patra@wdc.com>; U-Boot Mailing
> List <u-boot@lists.denx.de>; Alexander Graf <agraf@csgraf.de>; Anup Patel
> <anup.patel@wdc.com>; Bin Meng <bmeng.cn@gmail.com>; Joe
> Hershberger <joe.hershberger@ni.com>; Loic Pallardy
> <loic.pallardy@st.com>; Lukas Auer <lukas.auer@aisec.fraunhofer.de>;
> Marek Beh?n <marek.behun@nic.cz>; Marek Vasut
> <marek.vasut@gmail.com>; Patrick Delaunay <patrick.delaunay@st.com>;
> Peng Fan <peng.fan@nxp.com>; Philippe Reynes
> <philippe.reynes@softathome.com>; Simon Glass <sjg@chromium.org>;
> Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>; Stefano Babic
> <sbabic@denx.de>; Thierry Reding <treding@nvidia.com>; Tom Rini
> <trini@konsulko.com>; leif at nuviainc.com; Schaefer, Daniel (DualStudy)
> <daniel.schaefer@hpe.com>
> Subject: Re: [RFC PATCH 0/1] Add boot hartid to a Device tree
>
> On Tue, 25 Feb 2020 at 09:28, Chang, Abner (HPS SW/FW Technologist)
> <abner.chang@hpe.com> wrote:
>
> -----Original Message-----
> From: Atish Patra [mailto:atishp at atishpatra.org]
>
> <snip header soup>
>
> On Mon, Feb 24, 2020 at 3:35 PM Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
>
> On Tue, 25 Feb 2020 at 00:22, Heinrich Schuchardt
> <xypron.glpk@gmx.de>
>
> wrote:
>
> On 2/24/20 11:19 PM, Atish Patra wrote:
>
> The RISC-V booting protocol requires the hart id to be present in
>
> "a0"
>
> register. This is not a problem for bootm/booti commands as
> they directly jump to Linux kernel. However, bootefi jumps to
> a EFI boot stub code in Linux kernel which acts a loader and
> jumps to real Linux after terminating the boot services. This
> boot stub code has to be aware of the boot hart id so that it
> can set it in "a0" before jumping to Linux kernel. Currently,
> UEFI protocol doesn't have any mechanism to pass the boot hart
> id to an EFI executable. We should keep it this way as this is
> a RISC-V specific requirement rather than a UEFI requirement.
> Out of the all
>
> possible options, device tree seemed to be the best choice to do this job.
>
> The detailed discussion can be found in the following thread.
>
> INVALID URI REMOVED
>
> abs.org_patch_1233664_&d=DwIBaQ&c=C5b8zRQO1miGmBeVZ2LFWg&r=_S
>
> N6FZB
>
> N4Vgi4Ulkskz6qU3NYRO03nHp9P7Z5q59A3E&m=J8GY_HS3fV_cJH9duXP739y
>
> 0hDK
>
> 3qfHGNx2Dpcf-UBY&s=iVpRlpTOME_A-
>
> O5STNbXXawkW24gxy2yi56Q8AtZ2bI&e=
>
> The above mentioned patch is obsoleted by the new suggestion.
>
> Thanks for pointing that out to avoid confusion.
>
> This patch updates the device tree in arch_fixup_fdt() which
> is common for all booting commands. As a result, the DT
> modification doesn't require any efi related arch specific
> functions and all DT related modifications are contained at
> one place. However, the hart id node will be available for
> Linux even if the kernel is booted using
>
> bootm command.
>
> If that is not acceptable, we can always move the code to an
> efi specific function.
>
> Does a related Linux patch already exist?
>
> Yes. But in my local tree ;). It will be included in RISC-V EFI stub
> support series which I am planning to post in a couple of days.
>
> How about EDK2?
>
> RISC-V is not supported at all yet in EDK2.
>
> The EDK2 patches are out there and reviewed. I guess it will be
> available in mainline EDK2 pretty soon.
>
> Yes, currently we are working on edk2 CI testing for RISCV64 arch.  We
>
> hope edk2 RISC-V port could be in mainstream in Mar.
>
> Excellent! Is this core support? Or do you have a platform implemented as
> well that can be upstreamed?
>
> Yes we do have platform implementations to be upstreamed, below is the latest status of RISC-V edk2 port. We will have to update status later because we just merged OpenSBI tag 0.6 to edk2 RISC-V.
> https://github.com/riscv/riscv-uefi-edk2-docs
>
> Good to know! I saw some patches going by on the mailing list, but it
> is hard to derive the current state of affairs from that.
>
> I'm glad to see you did not make the same mistake we made on ARM and
> omit the PEI phase entirely.
>
> What I did notice is the use of APRIORI PEI and APRIORI DXE sections
> in your platform descriptions. I recommend you try to avoid that, as
> it is a maintenance burden going forward: instead, please use dummy
> protocols and NULL library class resolutions if you need to make
> generic components depend on platform specific protocols. Also, please
> document this - the APRIORI section does not explain *why* you have to
> circumvent the ordinary dependency tree based module dispatch.



-- 
Regards,
Atish

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-03-06  0:34 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-24 22:19 [RFC PATCH 0/1] Add boot hartid to a Device tree Atish Patra
2020-02-24 22:19 ` [RFC PATCH 1/1] riscv: Add boot hartid to " Atish Patra
2020-02-24 23:44   ` Ard Biesheuvel
2020-02-24 23:08 ` [RFC PATCH 0/1] Add boot hartid to a " Heinrich Schuchardt
2020-02-24 23:35   ` Ard Biesheuvel
2020-02-24 23:52     ` Atish Patra
2020-02-25  8:28       ` Chang, Abner
2020-02-25  8:48         ` Ard Biesheuvel
2020-02-25  8:59           ` Chang, Abner
2020-02-25  9:07             ` Ard Biesheuvel
2020-03-05  3:22               ` Schaefer, Daniel
2020-03-05  7:35                 ` Ard Biesheuvel
2020-03-06  0:34                 ` Atish Patra

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.