mbox series

[v5,0/6] RISC-V DT related fixes for reserved memory & UEFI

Message ID 20200406204453.231945-1-atish.patra@wdc.com
Headers show
Series RISC-V DT related fixes for reserved memory & UEFI | expand

Message

Atish Patra April 6, 2020, 8:44 p.m. UTC
This series adds few DT related fixes required for Linux EFI stub to work
on RISC-V.

Patch 1 adds the boot hartid property under /chosen node. The related
discussion can be found here.

https://patchwork.ozlabs.org/patch/1233664/
https://lists.denx.de/pipermail/u-boot/2020-March/402085.html

Patch 2 fixes a generic issue in fdtdec related to reserved memory node.

Patch 3,4,5 provide one of the option to update reserved-memory node for next
stage. It depends on master OpenSBI branch.

The other options are SBI extension and trap/emulate on PMP csr access.
The detaild discussion can be found here.
https://github.com/riscv/riscv-sbi-doc/pull/37

Patch 1 & 2 can be applied independently from 3 and 4. I want to keep all
the patches together to provide a holistic view of changes required for
RISC-V UEFI.

Changes v4->v5:
1. Added comments for new functions.

Changes v3->v4:
1. Dropped generic efi fix patch as it is already merged.
2. Moved all the fdt fixups to a common file.
3. Addressed few nit comments.

Changes from v2->v3:
1. Update the DT meant for OS if it is different from the one used by U-Boot
2. Use different FDT api to obtain "reg" address & size to honor the cell count.

Changes from v1->v2:
1. Fix the issue if chosen node is not present.

Changes from previous version:
1. Renamed the DT node property to "boot-hartid" from "efi-boot-hartid".
2. Changed the property type to u32 instead of u64 for RV32 compatibility.

Atish Patra (6):
riscv: Add boot hartid to Device tree
fdtdec: Fix boundary check
riscv: Provide a mechanism to fix DT for reserved memory
riscv: Setup reserved-memory node for FU540
riscv: Copy the reserved-memory nodes to final DT
riscv: Move all fdt fixups together

arch/riscv/cpu/start.S                |   1 +
arch/riscv/include/asm/global_data.h  |   1 +
arch/riscv/include/asm/u-boot-riscv.h |   2 +
arch/riscv/lib/Makefile               |   1 +
arch/riscv/lib/asm-offsets.c          |   1 +
arch/riscv/lib/bootm.c                |   5 -
arch/riscv/lib/fdt_fixup.c            | 150 ++++++++++++++++++++++++++
configs/sifive_fu540_defconfig        |   1 +
lib/fdtdec.c                          |   3 +-
9 files changed, 159 insertions(+), 6 deletions(-)
create mode 100644 arch/riscv/lib/fdt_fixup.c

--
2.25.1

Comments

Ard Biesheuvel April 6, 2020, 9:01 p.m. UTC | #1
On Mon, 6 Apr 2020 at 22:45, Atish Patra <atish.patra at wdc.com> wrote:
>
> This series adds few DT related fixes required for Linux EFI stub to work
> on RISC-V.
>

I'm not sure how this is supposed to work, since DT reserved memory
regions are not used by EFI. If you want to reserve memory on a UEFI
system, you have to reserve it in the UEFI memory map from firmware.
The DT reserved-memory node is taken into account too late, the
/memreserve/ entries are ignored entirely.


> Patch 1 adds the boot hartid property under /chosen node. The related
> discussion can be found here.
>
> https://patchwork.ozlabs.org/patch/1233664/
> https://lists.denx.de/pipermail/u-boot/2020-March/402085.html
>
> Patch 2 fixes a generic issue in fdtdec related to reserved memory node.
>
> Patch 3,4,5 provide one of the option to update reserved-memory node for next
> stage. It depends on master OpenSBI branch.
>
> The other options are SBI extension and trap/emulate on PMP csr access.
> The detaild discussion can be found here.
> https://github.com/riscv/riscv-sbi-doc/pull/37
>
> Patch 1 & 2 can be applied independently from 3 and 4. I want to keep all
> the patches together to provide a holistic view of changes required for
> RISC-V UEFI.
>
> Changes v4->v5:
> 1. Added comments for new functions.
>
> Changes v3->v4:
> 1. Dropped generic efi fix patch as it is already merged.
> 2. Moved all the fdt fixups to a common file.
> 3. Addressed few nit comments.
>
> Changes from v2->v3:
> 1. Update the DT meant for OS if it is different from the one used by U-Boot
> 2. Use different FDT api to obtain "reg" address & size to honor the cell count.
>
> Changes from v1->v2:
> 1. Fix the issue if chosen node is not present.
>
> Changes from previous version:
> 1. Renamed the DT node property to "boot-hartid" from "efi-boot-hartid".
> 2. Changed the property type to u32 instead of u64 for RV32 compatibility.
>
> Atish Patra (6):
> riscv: Add boot hartid to Device tree
> fdtdec: Fix boundary check
> riscv: Provide a mechanism to fix DT for reserved memory
> riscv: Setup reserved-memory node for FU540
> riscv: Copy the reserved-memory nodes to final DT
> riscv: Move all fdt fixups together
>
> arch/riscv/cpu/start.S                |   1 +
> arch/riscv/include/asm/global_data.h  |   1 +
> arch/riscv/include/asm/u-boot-riscv.h |   2 +
> arch/riscv/lib/Makefile               |   1 +
> arch/riscv/lib/asm-offsets.c          |   1 +
> arch/riscv/lib/bootm.c                |   5 -
> arch/riscv/lib/fdt_fixup.c            | 150 ++++++++++++++++++++++++++
> configs/sifive_fu540_defconfig        |   1 +
> lib/fdtdec.c                          |   3 +-
> 9 files changed, 159 insertions(+), 6 deletions(-)
> create mode 100644 arch/riscv/lib/fdt_fixup.c
>
> --
> 2.25.1
>
Heinrich Schuchardt April 7, 2020, 6:41 a.m. UTC | #2
On 4/6/20 11:01 PM, Ard Biesheuvel wrote:
> On Mon, 6 Apr 2020 at 22:45, Atish Patra <atish.patra at wdc.com> wrote:
>>
>> This series adds few DT related fixes required for Linux EFI stub to work
>> on RISC-V.
>>
>
> I'm not sure how this is supposed to work, since DT reserved memory
> regions are not used by EFI. If you want to reserve memory on a UEFI
> system, you have to reserve it in the UEFI memory map from firmware.
> The DT reserved-memory node is taken into account too late, the
> /memreserve/ entries are ignored entirely.

Hello Ard,

thanks for reviewing.

What do you mean by "The DT reserved-memory node is taken into account
too late"?

Cf. commit 7be64b885a36 ("cmd: bootefi: Parse reserved-memory node from DT")

Best regards

Heinrich

>
>
>> Patch 1 adds the boot hartid property under /chosen node. The related
>> discussion can be found here.
>>
>> https://patchwork.ozlabs.org/patch/1233664/
>> https://lists.denx.de/pipermail/u-boot/2020-March/402085.html
>>
>> Patch 2 fixes a generic issue in fdtdec related to reserved memory node.
>>
>> Patch 3,4,5 provide one of the option to update reserved-memory node for next
>> stage. It depends on master OpenSBI branch.
>>
>> The other options are SBI extension and trap/emulate on PMP csr access.
>> The detaild discussion can be found here.
>> https://github.com/riscv/riscv-sbi-doc/pull/37
>>
>> Patch 1 & 2 can be applied independently from 3 and 4. I want to keep all
>> the patches together to provide a holistic view of changes required for
>> RISC-V UEFI.
>>
>> Changes v4->v5:
>> 1. Added comments for new functions.
>>
>> Changes v3->v4:
>> 1. Dropped generic efi fix patch as it is already merged.
>> 2. Moved all the fdt fixups to a common file.
>> 3. Addressed few nit comments.
>>
>> Changes from v2->v3:
>> 1. Update the DT meant for OS if it is different from the one used by U-Boot
>> 2. Use different FDT api to obtain "reg" address & size to honor the cell count.
>>
>> Changes from v1->v2:
>> 1. Fix the issue if chosen node is not present.
>>
>> Changes from previous version:
>> 1. Renamed the DT node property to "boot-hartid" from "efi-boot-hartid".
>> 2. Changed the property type to u32 instead of u64 for RV32 compatibility.
>>
>> Atish Patra (6):
>> riscv: Add boot hartid to Device tree
>> fdtdec: Fix boundary check
>> riscv: Provide a mechanism to fix DT for reserved memory
>> riscv: Setup reserved-memory node for FU540
>> riscv: Copy the reserved-memory nodes to final DT
>> riscv: Move all fdt fixups together
>>
>> arch/riscv/cpu/start.S                |   1 +
>> arch/riscv/include/asm/global_data.h  |   1 +
>> arch/riscv/include/asm/u-boot-riscv.h |   2 +
>> arch/riscv/lib/Makefile               |   1 +
>> arch/riscv/lib/asm-offsets.c          |   1 +
>> arch/riscv/lib/bootm.c                |   5 -
>> arch/riscv/lib/fdt_fixup.c            | 150 ++++++++++++++++++++++++++
>> configs/sifive_fu540_defconfig        |   1 +
>> lib/fdtdec.c                          |   3 +-
>> 9 files changed, 159 insertions(+), 6 deletions(-)
>> create mode 100644 arch/riscv/lib/fdt_fixup.c
>>
>> --
>> 2.25.1
>>
Ard Biesheuvel April 7, 2020, 6:51 a.m. UTC | #3
On Tue, 7 Apr 2020 at 08:46, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 4/6/20 11:01 PM, Ard Biesheuvel wrote:
> > On Mon, 6 Apr 2020 at 22:45, Atish Patra <atish.patra at wdc.com> wrote:
> >>
> >> This series adds few DT related fixes required for Linux EFI stub to work
> >> on RISC-V.
> >>
> >
> > I'm not sure how this is supposed to work, since DT reserved memory
> > regions are not used by EFI. If you want to reserve memory on a UEFI
> > system, you have to reserve it in the UEFI memory map from firmware.
> > The DT reserved-memory node is taken into account too late, the
> > /memreserve/ entries are ignored entirely.
>
> Hello Ard,
>
> thanks for reviewing.
>
> What do you mean by "The DT reserved-memory node is taken into account
> too late"?
>
> Cf. commit 7be64b885a36 ("cmd: bootefi: Parse reserved-memory node from DT")
>

What I mean is that the EFI stub in Linux uses memory allocation
functions, expecting the firmware to ensure that those allocations do
not conflict with memory descriptions and reservations in DT. So if
the firmware wants to express this redundantly, by passing
reservations in both reserved-memory and in the EFI memory map, that
is probably fine.

Also, I must sheepishly admit that I only realize now that this patch
set is against u-boot not Linux :-)

So if fixed reserved-memory regions are only being used to seed the
reserved regions in the EFI memory map, you can safely ignore me.
Apologies for the noise.
Atish Patra April 7, 2020, 5:35 p.m. UTC | #4
On Mon, Apr 6, 2020 at 11:51 PM Ard Biesheuvel
<ard.biesheuvel at linaro.org> wrote:
>
> On Tue, 7 Apr 2020 at 08:46, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> >
> > On 4/6/20 11:01 PM, Ard Biesheuvel wrote:
> > > On Mon, 6 Apr 2020 at 22:45, Atish Patra <atish.patra at wdc.com> wrote:
> > >>
> > >> This series adds few DT related fixes required for Linux EFI stub to work
> > >> on RISC-V.
> > >>
> > >
> > > I'm not sure how this is supposed to work, since DT reserved memory
> > > regions are not used by EFI. If you want to reserve memory on a UEFI
> > > system, you have to reserve it in the UEFI memory map from firmware.
> > > The DT reserved-memory node is taken into account too late, the
> > > /memreserve/ entries are ignored entirely.
> >
> > Hello Ard,
> >
> > thanks for reviewing.
> >
> > What do you mean by "The DT reserved-memory node is taken into account
> > too late"?
> >
> > Cf. commit 7be64b885a36 ("cmd: bootefi: Parse reserved-memory node from DT")
> >
>
> What I mean is that the EFI stub in Linux uses memory allocation
> functions, expecting the firmware to ensure that those allocations do
> not conflict with memory descriptions and reservations in DT. So if
> the firmware wants to express this redundantly, by passing
> reservations in both reserved-memory and in the EFI memory map, that
> is probably fine.
>
> Also, I must sheepishly admit that I only realize now that this patch
> set is against u-boot not Linux :-)
>
:)

> So if fixed reserved-memory regions are only being used to seed the
> reserved regions in the EFI memory map, you can safely ignore me.

Yeah. That's the purpose. Having reserved memory nodes in the final DT
used by linux
also ensures that proper Linux adds a reserved memory block or removes
it from memblock
entries depending on "no-map" property.

> Apologies for the noise.
Atish Patra April 13, 2020, 10:02 p.m. UTC | #5
On Tue, Apr 7, 2020 at 10:35 AM Atish Patra <atishp at atishpatra.org> wrote:
>
> On Mon, Apr 6, 2020 at 11:51 PM Ard Biesheuvel
> <ard.biesheuvel at linaro.org> wrote:
> >
> > On Tue, 7 Apr 2020 at 08:46, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > >
> > > On 4/6/20 11:01 PM, Ard Biesheuvel wrote:
> > > > On Mon, 6 Apr 2020 at 22:45, Atish Patra <atish.patra at wdc.com> wrote:
> > > >>
> > > >> This series adds few DT related fixes required for Linux EFI stub to work
> > > >> on RISC-V.
> > > >>
> > > >
> > > > I'm not sure how this is supposed to work, since DT reserved memory
> > > > regions are not used by EFI. If you want to reserve memory on a UEFI
> > > > system, you have to reserve it in the UEFI memory map from firmware.
> > > > The DT reserved-memory node is taken into account too late, the
> > > > /memreserve/ entries are ignored entirely.
> > >
> > > Hello Ard,
> > >
> > > thanks for reviewing.
> > >
> > > What do you mean by "The DT reserved-memory node is taken into account
> > > too late"?
> > >
> > > Cf. commit 7be64b885a36 ("cmd: bootefi: Parse reserved-memory node from DT")
> > >
> >
> > What I mean is that the EFI stub in Linux uses memory allocation
> > functions, expecting the firmware to ensure that those allocations do
> > not conflict with memory descriptions and reservations in DT. So if
> > the firmware wants to express this redundantly, by passing
> > reservations in both reserved-memory and in the EFI memory map, that
> > is probably fine.
> >
> > Also, I must sheepishly admit that I only realize now that this patch
> > set is against u-boot not Linux :-)
> >
> :)
>
> > So if fixed reserved-memory regions are only being used to seed the
> > reserved regions in the EFI memory map, you can safely ignore me.
>
> Yeah. That's the purpose. Having reserved memory nodes in the final DT
> used by linux
> also ensures that proper Linux adds a reserved memory block or removes
> it from memblock
> entries depending on "no-map" property.
>
> > Apologies for the noise.
>
>
>
> --
> Regards,
> Atish

Any other comments on the series ? It would be great if this series
could be merged before
v2020.07 release.
Bin Meng April 13, 2020, 10:41 p.m. UTC | #6
Hi Atish,

On Tue, Apr 14, 2020 at 6:02 AM Atish Patra <atishp at atishpatra.org> wrote:
>
> On Tue, Apr 7, 2020 at 10:35 AM Atish Patra <atishp at atishpatra.org> wrote:
> >
> > On Mon, Apr 6, 2020 at 11:51 PM Ard Biesheuvel
> > <ard.biesheuvel at linaro.org> wrote:
> > >
> > > On Tue, 7 Apr 2020 at 08:46, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > > >
> > > > On 4/6/20 11:01 PM, Ard Biesheuvel wrote:
> > > > > On Mon, 6 Apr 2020 at 22:45, Atish Patra <atish.patra at wdc.com> wrote:
> > > > >>
> > > > >> This series adds few DT related fixes required for Linux EFI stub to work
> > > > >> on RISC-V.
> > > > >>
> > > > >
> > > > > I'm not sure how this is supposed to work, since DT reserved memory
> > > > > regions are not used by EFI. If you want to reserve memory on a UEFI
> > > > > system, you have to reserve it in the UEFI memory map from firmware.
> > > > > The DT reserved-memory node is taken into account too late, the
> > > > > /memreserve/ entries are ignored entirely.
> > > >
> > > > Hello Ard,
> > > >
> > > > thanks for reviewing.
> > > >
> > > > What do you mean by "The DT reserved-memory node is taken into account
> > > > too late"?
> > > >
> > > > Cf. commit 7be64b885a36 ("cmd: bootefi: Parse reserved-memory node from DT")
> > > >
> > >
> > > What I mean is that the EFI stub in Linux uses memory allocation
> > > functions, expecting the firmware to ensure that those allocations do
> > > not conflict with memory descriptions and reservations in DT. So if
> > > the firmware wants to express this redundantly, by passing
> > > reservations in both reserved-memory and in the EFI memory map, that
> > > is probably fine.
> > >
> > > Also, I must sheepishly admit that I only realize now that this patch
> > > set is against u-boot not Linux :-)
> > >
> > :)
> >
> > > So if fixed reserved-memory regions are only being used to seed the
> > > reserved regions in the EFI memory map, you can safely ignore me.
> >
> > Yeah. That's the purpose. Having reserved memory nodes in the final DT
> > used by linux
> > also ensures that proper Linux adds a reserved memory block or removes
> > it from memblock
> > entries depending on "no-map" property.
> >
> > > Apologies for the noise.
> >
> >
> >
> > --
> > Regards,
> > Atish
>
> Any other comments on the series ? It would be great if this series
> could be merged before
> v2020.07 release.

I hope so if no one objects the proposed solution here in U-Boot vs.
the PMP SBI extension. I need have another look at the latest version
of patches though.

Regards,
Bin
Heinrich Schuchardt April 13, 2020, 11:04 p.m. UTC | #7
On 4/6/20 10:44 PM, Atish Patra wrote:
> Linux booting protocol mandates that register "a0" contains the hartid.
> However, U-boot can not pass the hartid via a0 during via standard UEFI
> protocol. DT nodes are commonly used to pass such information to the OS.
>
> 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 at wdc.com>
> Reviewed-by: Rick Chen <rick at andestech.com>
> ---
>  arch/riscv/lib/bootm.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c
> index fad16901c5f2..87cadad5016d 100644
> --- a/arch/riscv/lib/bootm.c
> +++ b/arch/riscv/lib/bootm.c
> @@ -28,6 +28,28 @@ __weak void board_quiesce_devices(void)
>
>  int arch_fixup_fdt(void *blob)
>  {
> +#ifdef CONFIG_EFI_LOADER
> +	int err;
> +	u32 size;
> +	int chosen_offset;
> +
> +	size = fdt_totalsize(blob);
> +	err  = fdt_open_into(blob, blob, size + 32);
> +	if (err < 0) {
> +		printf("Device Tree can't be expanded to accommodate new node");
> +		return err;
> +	}
> +	chosen_offset = fdt_path_offset(blob, "/chosen");
> +	if (chosen_offset < 0) {
> +		err = fdt_add_subnode(blob, 0, "chosen");
> +		if (err < 0) {
> +			printf("chosen node can not be added\n");
> +			return err;
> +		}
> +	}
> +	/* Overwrite the boot-hartid as U-Boot is the last stage BL */
> +	fdt_setprop_u32(blob, chosen_offset, "boot-hartid", gd->arch.boot_hart);
> +#endif
>  	return 0;
>  }
>
>

I have tested this on qemu-riscv64_defconfig by comparing the device
tree before and after running helloworld.efi:

=> fdt addr $fdtcontroladdr
=> fdt print /chosen
chosen {
        bootargs = "";
        stdout-path = "/uart at 10000000";
};
...
=> dhcp $kernel_addr_r helloworld.efi
=> bootefi $kernel_addr_r
...
=> fdt addr 0x87F00000
=> fdt print /chosen
chosen {
        riscv,kernel-end = <0x00000000 0x00000000>;
        riscv,kernel-start = <0x00000000 0x00000000>;
        boot-hartid = <0x00000000>;
        bootargs = "";
        stdout-path = "/uart at 10000000";
};

The entry for boot-hardid seems to be ok.

But the riscv,kernel-end and riscv,kernel-start values are just some
dummy values introduced in:

board/emulation/qemu-riscv/qemu-riscv.c:84
commit 897206c5cc5c ("riscv: qemu: clear kernel-start/-end in device
tree as workaround for BBL)"

@Lukas:
Why are these values set to zero and not deleted (using fdt_delprop())
from the device tree? I cannot see that we need them when booting via UEFI.

Tested-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
Bin Meng April 13, 2020, 11:08 p.m. UTC | #8
Hi Heinrich,

On Tue, Apr 14, 2020 at 7:05 AM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 4/6/20 10:44 PM, Atish Patra wrote:
> > Linux booting protocol mandates that register "a0" contains the hartid.
> > However, U-boot can not pass the hartid via a0 during via standard UEFI
> > protocol. DT nodes are commonly used to pass such information to the OS.
> >
> > 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 at wdc.com>
> > Reviewed-by: Rick Chen <rick at andestech.com>
> > ---
> >  arch/riscv/lib/bootm.c | 22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> >
> > diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c
> > index fad16901c5f2..87cadad5016d 100644
> > --- a/arch/riscv/lib/bootm.c
> > +++ b/arch/riscv/lib/bootm.c
> > @@ -28,6 +28,28 @@ __weak void board_quiesce_devices(void)
> >
> >  int arch_fixup_fdt(void *blob)
> >  {
> > +#ifdef CONFIG_EFI_LOADER
> > +     int err;
> > +     u32 size;
> > +     int chosen_offset;
> > +
> > +     size = fdt_totalsize(blob);
> > +     err  = fdt_open_into(blob, blob, size + 32);
> > +     if (err < 0) {
> > +             printf("Device Tree can't be expanded to accommodate new node");
> > +             return err;
> > +     }
> > +     chosen_offset = fdt_path_offset(blob, "/chosen");
> > +     if (chosen_offset < 0) {
> > +             err = fdt_add_subnode(blob, 0, "chosen");
> > +             if (err < 0) {
> > +                     printf("chosen node can not be added\n");
> > +                     return err;
> > +             }
> > +     }
> > +     /* Overwrite the boot-hartid as U-Boot is the last stage BL */
> > +     fdt_setprop_u32(blob, chosen_offset, "boot-hartid", gd->arch.boot_hart);
> > +#endif
> >       return 0;
> >  }
> >
> >
>
> I have tested this on qemu-riscv64_defconfig by comparing the device
> tree before and after running helloworld.efi:
>
> => fdt addr $fdtcontroladdr
> => fdt print /chosen
> chosen {
>         bootargs = "";
>         stdout-path = "/uart at 10000000";
> };
> ...
> => dhcp $kernel_addr_r helloworld.efi
> => bootefi $kernel_addr_r
> ...
> => fdt addr 0x87F00000
> => fdt print /chosen
> chosen {
>         riscv,kernel-end = <0x00000000 0x00000000>;
>         riscv,kernel-start = <0x00000000 0x00000000>;
>         boot-hartid = <0x00000000>;
>         bootargs = "";
>         stdout-path = "/uart at 10000000";
> };
>
> The entry for boot-hardid seems to be ok.
>
> But the riscv,kernel-end and riscv,kernel-start values are just some
> dummy values introduced in:
>
> board/emulation/qemu-riscv/qemu-riscv.c:84
> commit 897206c5cc5c ("riscv: qemu: clear kernel-start/-end in device
> tree as workaround for BBL)"
>
> @Lukas:
> Why are these values set to zero and not deleted (using fdt_delprop())
> from the device tree? I cannot see that we need them when booting via UEFI.
>

This should be removed as BBL is legacy and we only support working
with OpenSBI.

> Tested-by: Heinrich Schuchardt <xypron.glpk at gmx.de>

Regards,
Bin
Lukas Auer April 14, 2020, 10:56 a.m. UTC | #9
Hi Heinrich, hi Bin,

On Tue, 2020-04-14 at 07:08 +0800, Bin Meng wrote:
> Hi Heinrich,
> 
> On Tue, Apr 14, 2020 at 7:05 AM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > On 4/6/20 10:44 PM, Atish Patra wrote:
> > > Linux booting protocol mandates that register "a0" contains the hartid.
> > > However, U-boot can not pass the hartid via a0 during via standard UEFI
> > > protocol. DT nodes are commonly used to pass such information to the OS.
> > > 
> > > 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 at wdc.com>
> > > Reviewed-by: Rick Chen <rick at andestech.com>
> > > ---
> > >  arch/riscv/lib/bootm.c | 22 ++++++++++++++++++++++
> > >  1 file changed, 22 insertions(+)
> > > 
> > > diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c
> > > index fad16901c5f2..87cadad5016d 100644
> > > --- a/arch/riscv/lib/bootm.c
> > > +++ b/arch/riscv/lib/bootm.c
> > > @@ -28,6 +28,28 @@ __weak void board_quiesce_devices(void)
> > > 
> > >  int arch_fixup_fdt(void *blob)
> > >  {
> > > +#ifdef CONFIG_EFI_LOADER
> > > +     int err;
> > > +     u32 size;
> > > +     int chosen_offset;
> > > +
> > > +     size = fdt_totalsize(blob);
> > > +     err  = fdt_open_into(blob, blob, size + 32);
> > > +     if (err < 0) {
> > > +             printf("Device Tree can't be expanded to accommodate new node");
> > > +             return err;
> > > +     }
> > > +     chosen_offset = fdt_path_offset(blob, "/chosen");
> > > +     if (chosen_offset < 0) {
> > > +             err = fdt_add_subnode(blob, 0, "chosen");
> > > +             if (err < 0) {
> > > +                     printf("chosen node can not be added\n");
> > > +                     return err;
> > > +             }
> > > +     }
> > > +     /* Overwrite the boot-hartid as U-Boot is the last stage BL */
> > > +     fdt_setprop_u32(blob, chosen_offset, "boot-hartid", gd->arch.boot_hart);
> > > +#endif
> > >       return 0;
> > >  }
> > > 
> > > 
> > 
> > I have tested this on qemu-riscv64_defconfig by comparing the device
> > tree before and after running helloworld.efi:
> > 
> > => fdt addr $fdtcontroladdr
> > => fdt print /chosen
> > chosen {
> >         bootargs = "";
> >         stdout-path = "/uart at 10000000";
> > };
> > ...
> > => dhcp $kernel_addr_r helloworld.efi
> > => bootefi $kernel_addr_r
> > ...
> > => fdt addr 0x87F00000
> > => fdt print /chosen
> > chosen {
> >         riscv,kernel-end = <0x00000000 0x00000000>;
> >         riscv,kernel-start = <0x00000000 0x00000000>;
> >         boot-hartid = <0x00000000>;
> >         bootargs = "";
> >         stdout-path = "/uart at 10000000";
> > };
> > 
> > The entry for boot-hardid seems to be ok.
> > 
> > But the riscv,kernel-end and riscv,kernel-start values are just some
> > dummy values introduced in:
> > 
> > board/emulation/qemu-riscv/qemu-riscv.c:84
> > commit 897206c5cc5c ("riscv: qemu: clear kernel-start/-end in device
> > tree as workaround for BBL)"
> > 
> > @Lukas:
> > Why are these values set to zero and not deleted (using fdt_delprop())
> > from the device tree? I cannot see that we need them when booting via UEFI.
> > 
> 

That was a workaround for BBL, which required them to be set if I
remember correctly.

> This should be removed as BBL is legacy and we only support working
> with OpenSBI.
> 

Absolutely correct, this is not required anymore since we are using
OpenSBI now. I will send a patch later today reverting the workaround.

Regards,
Lukas

> > Tested-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> 
> Regards,
> Bin
Atish Patra April 14, 2020, 11:18 p.m. UTC | #10
On Mon, Apr 13, 2020 at 3:42 PM Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Atish,
>
> On Tue, Apr 14, 2020 at 6:02 AM Atish Patra <atishp at atishpatra.org> wrote:
> >
> > On Tue, Apr 7, 2020 at 10:35 AM Atish Patra <atishp at atishpatra.org> wrote:
> > >
> > > On Mon, Apr 6, 2020 at 11:51 PM Ard Biesheuvel
> > > <ard.biesheuvel at linaro.org> wrote:
> > > >
> > > > On Tue, 7 Apr 2020 at 08:46, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > > > >
> > > > > On 4/6/20 11:01 PM, Ard Biesheuvel wrote:
> > > > > > On Mon, 6 Apr 2020 at 22:45, Atish Patra <atish.patra at wdc.com> wrote:
> > > > > >>
> > > > > >> This series adds few DT related fixes required for Linux EFI stub to work
> > > > > >> on RISC-V.
> > > > > >>
> > > > > >
> > > > > > I'm not sure how this is supposed to work, since DT reserved memory
> > > > > > regions are not used by EFI. If you want to reserve memory on a UEFI
> > > > > > system, you have to reserve it in the UEFI memory map from firmware.
> > > > > > The DT reserved-memory node is taken into account too late, the
> > > > > > /memreserve/ entries are ignored entirely.
> > > > >
> > > > > Hello Ard,
> > > > >
> > > > > thanks for reviewing.
> > > > >
> > > > > What do you mean by "The DT reserved-memory node is taken into account
> > > > > too late"?
> > > > >
> > > > > Cf. commit 7be64b885a36 ("cmd: bootefi: Parse reserved-memory node from DT")
> > > > >
> > > >
> > > > What I mean is that the EFI stub in Linux uses memory allocation
> > > > functions, expecting the firmware to ensure that those allocations do
> > > > not conflict with memory descriptions and reservations in DT. So if
> > > > the firmware wants to express this redundantly, by passing
> > > > reservations in both reserved-memory and in the EFI memory map, that
> > > > is probably fine.
> > > >
> > > > Also, I must sheepishly admit that I only realize now that this patch
> > > > set is against u-boot not Linux :-)
> > > >
> > > :)
> > >
> > > > So if fixed reserved-memory regions are only being used to seed the
> > > > reserved regions in the EFI memory map, you can safely ignore me.
> > >
> > > Yeah. That's the purpose. Having reserved memory nodes in the final DT
> > > used by linux
> > > also ensures that proper Linux adds a reserved memory block or removes
> > > it from memblock
> > > entries depending on "no-map" property.
> > >
> > > > Apologies for the noise.
> > >
> > >
> > >
> > > --
> > > Regards,
> > > Atish
> >
> > Any other comments on the series ? It would be great if this series
> > could be merged before
> > v2020.07 release.
>
> I hope so if no one objects the proposed solution here in U-Boot vs.
> the PMP SBI extension. I need have another look at the latest version
> of patches though.
>

Thanks. As far as I know, there is no opposition to the current
approach adopted in U-Boot.
I am hoping EFI stub series can be merged before 5.8. If this series
can go in v2020.07,
RISC-V will have all required support to boot via EFI from Linux
kernel v5.8 and U-Boot v2020.07.

> Regards,
> Bin
Rick Chen April 17, 2020, 12:51 a.m. UTC | #11
<rick at andestech.com> ? 2020?4?17? ?? ??8:39???
>
>
>
> -----Original Message-----
> From: Atish Patra [mailto:atishp at atishpatra.org]
> Sent: Wednesday, April 15, 2020 7:18 AM
> To: Bin Meng
> Cc: Ard Biesheuvel; Heinrich Schuchardt; U-Boot Mailing List; Anup Patel; Lukas Auer; Alexander Graf; Rick Jian-Zhi Chen(???); Palmer Dabbelt
> Subject: Re: [PATCH v5 0/6] RISC-V DT related fixes for reserved memory & UEFI
>
> On Mon, Apr 13, 2020 at 3:42 PM Bin Meng <bmeng.cn at gmail.com> wrote:
> >
> > Hi Atish,
> >
> > On Tue, Apr 14, 2020 at 6:02 AM Atish Patra <atishp at atishpatra.org> wrote:
> > >
> > > On Tue, Apr 7, 2020 at 10:35 AM Atish Patra <atishp at atishpatra.org> wrote:
> > > >
> > > > On Mon, Apr 6, 2020 at 11:51 PM Ard Biesheuvel
> > > > <ard.biesheuvel at linaro.org> wrote:
> > > > >
> > > > > On Tue, 7 Apr 2020 at 08:46, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > > > > >
> > > > > > On 4/6/20 11:01 PM, Ard Biesheuvel wrote:
> > > > > > > On Mon, 6 Apr 2020 at 22:45, Atish Patra <atish.patra at wdc.com> wrote:
> > > > > > >>
> > > > > > >> This series adds few DT related fixes required for Linux
> > > > > > >> EFI stub to work on RISC-V.
> > > > > > >>
> > > > > > >
> > > > > > > I'm not sure how this is supposed to work, since DT reserved
> > > > > > > memory regions are not used by EFI. If you want to reserve
> > > > > > > memory on a UEFI system, you have to reserve it in the UEFI memory map from firmware.
> > > > > > > The DT reserved-memory node is taken into account too late,
> > > > > > > the /memreserve/ entries are ignored entirely.
> > > > > >
> > > > > > Hello Ard,
> > > > > >
> > > > > > thanks for reviewing.
> > > > > >
> > > > > > What do you mean by "The DT reserved-memory node is taken into
> > > > > > account too late"?
> > > > > >
> > > > > > Cf. commit 7be64b885a36 ("cmd: bootefi: Parse reserved-memory
> > > > > > node from DT")
> > > > > >
> > > > >
> > > > > What I mean is that the EFI stub in Linux uses memory allocation
> > > > > functions, expecting the firmware to ensure that those
> > > > > allocations do not conflict with memory descriptions and
> > > > > reservations in DT. So if the firmware wants to express this
> > > > > redundantly, by passing reservations in both reserved-memory and
> > > > > in the EFI memory map, that is probably fine.
> > > > >
> > > > > Also, I must sheepishly admit that I only realize now that this
> > > > > patch set is against u-boot not Linux :-)
> > > > >
> > > > :)
> > > >
> > > > > So if fixed reserved-memory regions are only being used to seed
> > > > > the reserved regions in the EFI memory map, you can safely ignore me.
> > > >
> > > > Yeah. That's the purpose. Having reserved memory nodes in the
> > > > final DT used by linux also ensures that proper Linux adds a
> > > > reserved memory block or removes it from memblock entries
> > > > depending on "no-map" property.
> > > >
> > > > > Apologies for the noise.
> > > >
> > > >
> > > >
> > > > --
> > > > Regards,
> > > > Atish
> > >
> > > Any other comments on the series ? It would be great if this series
> > > could be merged before
> > > v2020.07 release.
> >
> > I hope so if no one objects the proposed solution here in U-Boot vs.
> > the PMP SBI extension. I need have another look at the latest version
> > of patches though.
> >
>
> Thanks. As far as I know, there is no opposition to the current approach adopted in U-Boot.
> I am hoping EFI stub series can be merged before 5.8. If this series can go in v2020.07, RISC-V will have all required support to boot via EFI from Linux kernel v5.8 and U-Boot v2020.07.

OK!
I will pull and send a PR ASAP.

Thanks,
Rick
Bin Meng April 17, 2020, 1:10 a.m. UTC | #12
Hi Rick,

On Fri, Apr 17, 2020 at 8:51 AM Rick Chen <rickchen36 at gmail.com> wrote:
>
> <rick at andestech.com> ? 2020?4?17? ?? ??8:39???
> >
> >
> >
> > -----Original Message-----
> > From: Atish Patra [mailto:atishp at atishpatra.org]
> > Sent: Wednesday, April 15, 2020 7:18 AM
> > To: Bin Meng
> > Cc: Ard Biesheuvel; Heinrich Schuchardt; U-Boot Mailing List; Anup Patel; Lukas Auer; Alexander Graf; Rick Jian-Zhi Chen(???); Palmer Dabbelt
> > Subject: Re: [PATCH v5 0/6] RISC-V DT related fixes for reserved memory & UEFI
> >
> > On Mon, Apr 13, 2020 at 3:42 PM Bin Meng <bmeng.cn at gmail.com> wrote:
> > >
> > > Hi Atish,
> > >
> > > On Tue, Apr 14, 2020 at 6:02 AM Atish Patra <atishp at atishpatra.org> wrote:
> > > >
> > > > On Tue, Apr 7, 2020 at 10:35 AM Atish Patra <atishp at atishpatra.org> wrote:
> > > > >
> > > > > On Mon, Apr 6, 2020 at 11:51 PM Ard Biesheuvel
> > > > > <ard.biesheuvel at linaro.org> wrote:
> > > > > >
> > > > > > On Tue, 7 Apr 2020 at 08:46, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > > > > > >
> > > > > > > On 4/6/20 11:01 PM, Ard Biesheuvel wrote:
> > > > > > > > On Mon, 6 Apr 2020 at 22:45, Atish Patra <atish.patra at wdc.com> wrote:
> > > > > > > >>
> > > > > > > >> This series adds few DT related fixes required for Linux
> > > > > > > >> EFI stub to work on RISC-V.
> > > > > > > >>
> > > > > > > >
> > > > > > > > I'm not sure how this is supposed to work, since DT reserved
> > > > > > > > memory regions are not used by EFI. If you want to reserve
> > > > > > > > memory on a UEFI system, you have to reserve it in the UEFI memory map from firmware.
> > > > > > > > The DT reserved-memory node is taken into account too late,
> > > > > > > > the /memreserve/ entries are ignored entirely.
> > > > > > >
> > > > > > > Hello Ard,
> > > > > > >
> > > > > > > thanks for reviewing.
> > > > > > >
> > > > > > > What do you mean by "The DT reserved-memory node is taken into
> > > > > > > account too late"?
> > > > > > >
> > > > > > > Cf. commit 7be64b885a36 ("cmd: bootefi: Parse reserved-memory
> > > > > > > node from DT")
> > > > > > >
> > > > > >
> > > > > > What I mean is that the EFI stub in Linux uses memory allocation
> > > > > > functions, expecting the firmware to ensure that those
> > > > > > allocations do not conflict with memory descriptions and
> > > > > > reservations in DT. So if the firmware wants to express this
> > > > > > redundantly, by passing reservations in both reserved-memory and
> > > > > > in the EFI memory map, that is probably fine.
> > > > > >
> > > > > > Also, I must sheepishly admit that I only realize now that this
> > > > > > patch set is against u-boot not Linux :-)
> > > > > >
> > > > > :)
> > > > >
> > > > > > So if fixed reserved-memory regions are only being used to seed
> > > > > > the reserved regions in the EFI memory map, you can safely ignore me.
> > > > >
> > > > > Yeah. That's the purpose. Having reserved memory nodes in the
> > > > > final DT used by linux also ensures that proper Linux adds a
> > > > > reserved memory block or removes it from memblock entries
> > > > > depending on "no-map" property.
> > > > >
> > > > > > Apologies for the noise.
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Regards,
> > > > > Atish
> > > >
> > > > Any other comments on the series ? It would be great if this series
> > > > could be merged before
> > > > v2020.07 release.
> > >
> > > I hope so if no one objects the proposed solution here in U-Boot vs.
> > > the PMP SBI extension. I need have another look at the latest version
> > > of patches though.
> > >
> >
> > Thanks. As far as I know, there is no opposition to the current approach adopted in U-Boot.
> > I am hoping EFI stub series can be merged before 5.8. If this series can go in v2020.07, RISC-V will have all required support to boot via EFI from Linux kernel v5.8 and U-Boot v2020.07.
>
> OK!
> I will pull and send a PR ASAP.

I will need have a look and test today. Please wait for some time.

Regards,
Bin
Rick Chen April 17, 2020, 1:12 a.m. UTC | #13
Hi Bin

> Hi Rick,
>
> On Fri, Apr 17, 2020 at 8:51 AM Rick Chen <rickchen36 at gmail.com> wrote:
> >
> > <rick at andestech.com> ? 2020?4?17? ?? ??8:39???
> > >
> > >
> > >
> > > -----Original Message-----
> > > From: Atish Patra [mailto:atishp at atishpatra.org]
> > > Sent: Wednesday, April 15, 2020 7:18 AM
> > > To: Bin Meng
> > > Cc: Ard Biesheuvel; Heinrich Schuchardt; U-Boot Mailing List; Anup Patel; Lukas Auer; Alexander Graf; Rick Jian-Zhi Chen(???); Palmer Dabbelt
> > > Subject: Re: [PATCH v5 0/6] RISC-V DT related fixes for reserved memory & UEFI
> > >
> > > On Mon, Apr 13, 2020 at 3:42 PM Bin Meng <bmeng.cn at gmail.com> wrote:
> > > >
> > > > Hi Atish,
> > > >
> > > > On Tue, Apr 14, 2020 at 6:02 AM Atish Patra <atishp at atishpatra.org> wrote:
> > > > >
> > > > > On Tue, Apr 7, 2020 at 10:35 AM Atish Patra <atishp at atishpatra.org> wrote:
> > > > > >
> > > > > > On Mon, Apr 6, 2020 at 11:51 PM Ard Biesheuvel
> > > > > > <ard.biesheuvel at linaro.org> wrote:
> > > > > > >
> > > > > > > On Tue, 7 Apr 2020 at 08:46, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > > > > > > >
> > > > > > > > On 4/6/20 11:01 PM, Ard Biesheuvel wrote:
> > > > > > > > > On Mon, 6 Apr 2020 at 22:45, Atish Patra <atish.patra at wdc.com> wrote:
> > > > > > > > >>
> > > > > > > > >> This series adds few DT related fixes required for Linux
> > > > > > > > >> EFI stub to work on RISC-V.
> > > > > > > > >>
> > > > > > > > >
> > > > > > > > > I'm not sure how this is supposed to work, since DT reserved
> > > > > > > > > memory regions are not used by EFI. If you want to reserve
> > > > > > > > > memory on a UEFI system, you have to reserve it in the UEFI memory map from firmware.
> > > > > > > > > The DT reserved-memory node is taken into account too late,
> > > > > > > > > the /memreserve/ entries are ignored entirely.
> > > > > > > >
> > > > > > > > Hello Ard,
> > > > > > > >
> > > > > > > > thanks for reviewing.
> > > > > > > >
> > > > > > > > What do you mean by "The DT reserved-memory node is taken into
> > > > > > > > account too late"?
> > > > > > > >
> > > > > > > > Cf. commit 7be64b885a36 ("cmd: bootefi: Parse reserved-memory
> > > > > > > > node from DT")
> > > > > > > >
> > > > > > >
> > > > > > > What I mean is that the EFI stub in Linux uses memory allocation
> > > > > > > functions, expecting the firmware to ensure that those
> > > > > > > allocations do not conflict with memory descriptions and
> > > > > > > reservations in DT. So if the firmware wants to express this
> > > > > > > redundantly, by passing reservations in both reserved-memory and
> > > > > > > in the EFI memory map, that is probably fine.
> > > > > > >
> > > > > > > Also, I must sheepishly admit that I only realize now that this
> > > > > > > patch set is against u-boot not Linux :-)
> > > > > > >
> > > > > > :)
> > > > > >
> > > > > > > So if fixed reserved-memory regions are only being used to seed
> > > > > > > the reserved regions in the EFI memory map, you can safely ignore me.
> > > > > >
> > > > > > Yeah. That's the purpose. Having reserved memory nodes in the
> > > > > > final DT used by linux also ensures that proper Linux adds a
> > > > > > reserved memory block or removes it from memblock entries
> > > > > > depending on "no-map" property.
> > > > > >
> > > > > > > Apologies for the noise.
> > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Regards,
> > > > > > Atish
> > > > >
> > > > > Any other comments on the series ? It would be great if this series
> > > > > could be merged before
> > > > > v2020.07 release.
> > > >
> > > > I hope so if no one objects the proposed solution here in U-Boot vs.
> > > > the PMP SBI extension. I need have another look at the latest version
> > > > of patches though.
> > > >
> > >
> > > Thanks. As far as I know, there is no opposition to the current approach adopted in U-Boot.
> > > I am hoping EFI stub series can be merged before 5.8. If this series can go in v2020.07, RISC-V will have all required support to boot via EFI from Linux kernel v5.8 and U-Boot v2020.07.
> >
> > OK!
> > I will pull and send a PR ASAP.
>
> I will need have a look and test today. Please wait for some time.
>

OK
No problem :)

Thanks,
Rick

> Regards,
> Bin
Bin Meng April 17, 2020, 2:12 a.m. UTC | #14
Hi Rick,

On Fri, Apr 17, 2020 at 9:12 AM Rick Chen <rickchen36 at gmail.com> wrote:
>
> Hi Bin
>
> > Hi Rick,
> >
> > On Fri, Apr 17, 2020 at 8:51 AM Rick Chen <rickchen36 at gmail.com> wrote:
> > >
> > > <rick at andestech.com> ? 2020?4?17? ?? ??8:39???
> > > >
> > > >
> > > >
> > > > -----Original Message-----
> > > > From: Atish Patra [mailto:atishp at atishpatra.org]
> > > > Sent: Wednesday, April 15, 2020 7:18 AM
> > > > To: Bin Meng
> > > > Cc: Ard Biesheuvel; Heinrich Schuchardt; U-Boot Mailing List; Anup Patel; Lukas Auer; Alexander Graf; Rick Jian-Zhi Chen(???); Palmer Dabbelt
> > > > Subject: Re: [PATCH v5 0/6] RISC-V DT related fixes for reserved memory & UEFI
> > > >
> > > > On Mon, Apr 13, 2020 at 3:42 PM Bin Meng <bmeng.cn at gmail.com> wrote:
> > > > >
> > > > > Hi Atish,
> > > > >
> > > > > On Tue, Apr 14, 2020 at 6:02 AM Atish Patra <atishp at atishpatra.org> wrote:
> > > > > >
> > > > > > On Tue, Apr 7, 2020 at 10:35 AM Atish Patra <atishp at atishpatra.org> wrote:
> > > > > > >
> > > > > > > On Mon, Apr 6, 2020 at 11:51 PM Ard Biesheuvel
> > > > > > > <ard.biesheuvel at linaro.org> wrote:
> > > > > > > >
> > > > > > > > On Tue, 7 Apr 2020 at 08:46, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > > > > > > > >
> > > > > > > > > On 4/6/20 11:01 PM, Ard Biesheuvel wrote:
> > > > > > > > > > On Mon, 6 Apr 2020 at 22:45, Atish Patra <atish.patra at wdc.com> wrote:
> > > > > > > > > >>
> > > > > > > > > >> This series adds few DT related fixes required for Linux
> > > > > > > > > >> EFI stub to work on RISC-V.
> > > > > > > > > >>
> > > > > > > > > >
> > > > > > > > > > I'm not sure how this is supposed to work, since DT reserved
> > > > > > > > > > memory regions are not used by EFI. If you want to reserve
> > > > > > > > > > memory on a UEFI system, you have to reserve it in the UEFI memory map from firmware.
> > > > > > > > > > The DT reserved-memory node is taken into account too late,
> > > > > > > > > > the /memreserve/ entries are ignored entirely.
> > > > > > > > >
> > > > > > > > > Hello Ard,
> > > > > > > > >
> > > > > > > > > thanks for reviewing.
> > > > > > > > >
> > > > > > > > > What do you mean by "The DT reserved-memory node is taken into
> > > > > > > > > account too late"?
> > > > > > > > >
> > > > > > > > > Cf. commit 7be64b885a36 ("cmd: bootefi: Parse reserved-memory
> > > > > > > > > node from DT")
> > > > > > > > >
> > > > > > > >
> > > > > > > > What I mean is that the EFI stub in Linux uses memory allocation
> > > > > > > > functions, expecting the firmware to ensure that those
> > > > > > > > allocations do not conflict with memory descriptions and
> > > > > > > > reservations in DT. So if the firmware wants to express this
> > > > > > > > redundantly, by passing reservations in both reserved-memory and
> > > > > > > > in the EFI memory map, that is probably fine.
> > > > > > > >
> > > > > > > > Also, I must sheepishly admit that I only realize now that this
> > > > > > > > patch set is against u-boot not Linux :-)
> > > > > > > >
> > > > > > > :)
> > > > > > >
> > > > > > > > So if fixed reserved-memory regions are only being used to seed
> > > > > > > > the reserved regions in the EFI memory map, you can safely ignore me.
> > > > > > >
> > > > > > > Yeah. That's the purpose. Having reserved memory nodes in the
> > > > > > > final DT used by linux also ensures that proper Linux adds a
> > > > > > > reserved memory block or removes it from memblock entries
> > > > > > > depending on "no-map" property.
> > > > > > >
> > > > > > > > Apologies for the noise.
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > Regards,
> > > > > > > Atish
> > > > > >
> > > > > > Any other comments on the series ? It would be great if this series
> > > > > > could be merged before
> > > > > > v2020.07 release.
> > > > >
> > > > > I hope so if no one objects the proposed solution here in U-Boot vs.
> > > > > the PMP SBI extension. I need have another look at the latest version
> > > > > of patches though.
> > > > >
> > > >
> > > > Thanks. As far as I know, there is no opposition to the current approach adopted in U-Boot.
> > > > I am hoping EFI stub series can be merged before 5.8. If this series can go in v2020.07, RISC-V will have all required support to boot via EFI from Linux kernel v5.8 and U-Boot v2020.07.
> > >
> > > OK!
> > > I will pull and send a PR ASAP.
> >
> > I will need have a look and test today. Please wait for some time.
> >
>
> OK
> No problem :)

Do you know what happened to this series?

I only see patch 3, 5, 6 showing up on the patchwork. Are other
patches already applied somewhere?

Regards,
Bin
Bin Meng April 17, 2020, 2:14 a.m. UTC | #15
Correct Palmer's email address

On Fri, Apr 17, 2020 at 10:12 AM Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Rick,
>
> On Fri, Apr 17, 2020 at 9:12 AM Rick Chen <rickchen36 at gmail.com> wrote:
> >
> > Hi Bin
> >
> > > Hi Rick,
> > >
> > > On Fri, Apr 17, 2020 at 8:51 AM Rick Chen <rickchen36 at gmail.com> wrote:
> > > >
> > > > <rick at andestech.com> ? 2020?4?17? ?? ??8:39???
> > > > >
> > > > >
> > > > >
> > > > > -----Original Message-----
> > > > > From: Atish Patra [mailto:atishp at atishpatra.org]
> > > > > Sent: Wednesday, April 15, 2020 7:18 AM
> > > > > To: Bin Meng
> > > > > Cc: Ard Biesheuvel; Heinrich Schuchardt; U-Boot Mailing List; Anup Patel; Lukas Auer; Alexander Graf; Rick Jian-Zhi Chen(???); Palmer Dabbelt
> > > > > Subject: Re: [PATCH v5 0/6] RISC-V DT related fixes for reserved memory & UEFI
> > > > >
> > > > > On Mon, Apr 13, 2020 at 3:42 PM Bin Meng <bmeng.cn at gmail.com> wrote:
> > > > > >
> > > > > > Hi Atish,
> > > > > >
> > > > > > On Tue, Apr 14, 2020 at 6:02 AM Atish Patra <atishp at atishpatra.org> wrote:
> > > > > > >
> > > > > > > On Tue, Apr 7, 2020 at 10:35 AM Atish Patra <atishp at atishpatra.org> wrote:
> > > > > > > >
> > > > > > > > On Mon, Apr 6, 2020 at 11:51 PM Ard Biesheuvel
> > > > > > > > <ard.biesheuvel at linaro.org> wrote:
> > > > > > > > >
> > > > > > > > > On Tue, 7 Apr 2020 at 08:46, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > > > > > > > > >
> > > > > > > > > > On 4/6/20 11:01 PM, Ard Biesheuvel wrote:
> > > > > > > > > > > On Mon, 6 Apr 2020 at 22:45, Atish Patra <atish.patra at wdc.com> wrote:
> > > > > > > > > > >>
> > > > > > > > > > >> This series adds few DT related fixes required for Linux
> > > > > > > > > > >> EFI stub to work on RISC-V.
> > > > > > > > > > >>
> > > > > > > > > > >
> > > > > > > > > > > I'm not sure how this is supposed to work, since DT reserved
> > > > > > > > > > > memory regions are not used by EFI. If you want to reserve
> > > > > > > > > > > memory on a UEFI system, you have to reserve it in the UEFI memory map from firmware.
> > > > > > > > > > > The DT reserved-memory node is taken into account too late,
> > > > > > > > > > > the /memreserve/ entries are ignored entirely.
> > > > > > > > > >
> > > > > > > > > > Hello Ard,
> > > > > > > > > >
> > > > > > > > > > thanks for reviewing.
> > > > > > > > > >
> > > > > > > > > > What do you mean by "The DT reserved-memory node is taken into
> > > > > > > > > > account too late"?
> > > > > > > > > >
> > > > > > > > > > Cf. commit 7be64b885a36 ("cmd: bootefi: Parse reserved-memory
> > > > > > > > > > node from DT")
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > What I mean is that the EFI stub in Linux uses memory allocation
> > > > > > > > > functions, expecting the firmware to ensure that those
> > > > > > > > > allocations do not conflict with memory descriptions and
> > > > > > > > > reservations in DT. So if the firmware wants to express this
> > > > > > > > > redundantly, by passing reservations in both reserved-memory and
> > > > > > > > > in the EFI memory map, that is probably fine.
> > > > > > > > >
> > > > > > > > > Also, I must sheepishly admit that I only realize now that this
> > > > > > > > > patch set is against u-boot not Linux :-)
> > > > > > > > >
> > > > > > > > :)
> > > > > > > >
> > > > > > > > > So if fixed reserved-memory regions are only being used to seed
> > > > > > > > > the reserved regions in the EFI memory map, you can safely ignore me.
> > > > > > > >
> > > > > > > > Yeah. That's the purpose. Having reserved memory nodes in the
> > > > > > > > final DT used by linux also ensures that proper Linux adds a
> > > > > > > > reserved memory block or removes it from memblock entries
> > > > > > > > depending on "no-map" property.
> > > > > > > >
> > > > > > > > > Apologies for the noise.
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > --
> > > > > > > > Regards,
> > > > > > > > Atish
> > > > > > >
> > > > > > > Any other comments on the series ? It would be great if this series
> > > > > > > could be merged before
> > > > > > > v2020.07 release.
> > > > > >
> > > > > > I hope so if no one objects the proposed solution here in U-Boot vs.
> > > > > > the PMP SBI extension. I need have another look at the latest version
> > > > > > of patches though.
> > > > > >
> > > > >
> > > > > Thanks. As far as I know, there is no opposition to the current approach adopted in U-Boot.
> > > > > I am hoping EFI stub series can be merged before 5.8. If this series can go in v2020.07, RISC-V will have all required support to boot via EFI from Linux kernel v5.8 and U-Boot v2020.07.
> > > >
> > > > OK!
> > > > I will pull and send a PR ASAP.
> > >
> > > I will need have a look and test today. Please wait for some time.
> > >
> >
> > OK
> > No problem :)
>
> Do you know what happened to this series?
>
> I only see patch 3, 5, 6 showing up on the patchwork. Are other
> patches already applied somewhere?

I am referring to http://patchwork.ozlabs.org/project/uboot/list/?series=168858

Regards,
Bin
Bin Meng April 17, 2020, 2:26 a.m. UTC | #16
Hi Atish,

On Fri, Apr 17, 2020 at 10:14 AM Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Correct Palmer's email address
>
> On Fri, Apr 17, 2020 at 10:12 AM Bin Meng <bmeng.cn at gmail.com> wrote:
> >
> > Hi Rick,
> >
> > On Fri, Apr 17, 2020 at 9:12 AM Rick Chen <rickchen36 at gmail.com> wrote:
> > >
> > > Hi Bin
> > >
> > > > Hi Rick,
> > > >
> > > > On Fri, Apr 17, 2020 at 8:51 AM Rick Chen <rickchen36 at gmail.com> wrote:
> > > > >
> > > > > <rick at andestech.com> ? 2020?4?17? ?? ??8:39???
> > > > > >
> > > > > >
> > > > > >
> > > > > > -----Original Message-----
> > > > > > From: Atish Patra [mailto:atishp at atishpatra.org]
> > > > > > Sent: Wednesday, April 15, 2020 7:18 AM
> > > > > > To: Bin Meng
> > > > > > Cc: Ard Biesheuvel; Heinrich Schuchardt; U-Boot Mailing List; Anup Patel; Lukas Auer; Alexander Graf; Rick Jian-Zhi Chen(???); Palmer Dabbelt
> > > > > > Subject: Re: [PATCH v5 0/6] RISC-V DT related fixes for reserved memory & UEFI
> > > > > >
> > > > > > On Mon, Apr 13, 2020 at 3:42 PM Bin Meng <bmeng.cn at gmail.com> wrote:
> > > > > > >
> > > > > > > Hi Atish,
> > > > > > >
> > > > > > > On Tue, Apr 14, 2020 at 6:02 AM Atish Patra <atishp at atishpatra.org> wrote:
> > > > > > > >
> > > > > > > > On Tue, Apr 7, 2020 at 10:35 AM Atish Patra <atishp at atishpatra.org> wrote:
> > > > > > > > >
> > > > > > > > > On Mon, Apr 6, 2020 at 11:51 PM Ard Biesheuvel
> > > > > > > > > <ard.biesheuvel at linaro.org> wrote:
> > > > > > > > > >
> > > > > > > > > > On Tue, 7 Apr 2020 at 08:46, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > > > > > > > > > >
> > > > > > > > > > > On 4/6/20 11:01 PM, Ard Biesheuvel wrote:
> > > > > > > > > > > > On Mon, 6 Apr 2020 at 22:45, Atish Patra <atish.patra at wdc.com> wrote:
> > > > > > > > > > > >>
> > > > > > > > > > > >> This series adds few DT related fixes required for Linux
> > > > > > > > > > > >> EFI stub to work on RISC-V.
> > > > > > > > > > > >>
> > > > > > > > > > > >
> > > > > > > > > > > > I'm not sure how this is supposed to work, since DT reserved
> > > > > > > > > > > > memory regions are not used by EFI. If you want to reserve
> > > > > > > > > > > > memory on a UEFI system, you have to reserve it in the UEFI memory map from firmware.
> > > > > > > > > > > > The DT reserved-memory node is taken into account too late,
> > > > > > > > > > > > the /memreserve/ entries are ignored entirely.
> > > > > > > > > > >
> > > > > > > > > > > Hello Ard,
> > > > > > > > > > >
> > > > > > > > > > > thanks for reviewing.
> > > > > > > > > > >
> > > > > > > > > > > What do you mean by "The DT reserved-memory node is taken into
> > > > > > > > > > > account too late"?
> > > > > > > > > > >
> > > > > > > > > > > Cf. commit 7be64b885a36 ("cmd: bootefi: Parse reserved-memory
> > > > > > > > > > > node from DT")
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > What I mean is that the EFI stub in Linux uses memory allocation
> > > > > > > > > > functions, expecting the firmware to ensure that those
> > > > > > > > > > allocations do not conflict with memory descriptions and
> > > > > > > > > > reservations in DT. So if the firmware wants to express this
> > > > > > > > > > redundantly, by passing reservations in both reserved-memory and
> > > > > > > > > > in the EFI memory map, that is probably fine.
> > > > > > > > > >
> > > > > > > > > > Also, I must sheepishly admit that I only realize now that this
> > > > > > > > > > patch set is against u-boot not Linux :-)
> > > > > > > > > >
> > > > > > > > > :)
> > > > > > > > >
> > > > > > > > > > So if fixed reserved-memory regions are only being used to seed
> > > > > > > > > > the reserved regions in the EFI memory map, you can safely ignore me.
> > > > > > > > >
> > > > > > > > > Yeah. That's the purpose. Having reserved memory nodes in the
> > > > > > > > > final DT used by linux also ensures that proper Linux adds a
> > > > > > > > > reserved memory block or removes it from memblock entries
> > > > > > > > > depending on "no-map" property.
> > > > > > > > >
> > > > > > > > > > Apologies for the noise.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > --
> > > > > > > > > Regards,
> > > > > > > > > Atish
> > > > > > > >
> > > > > > > > Any other comments on the series ? It would be great if this series
> > > > > > > > could be merged before
> > > > > > > > v2020.07 release.
> > > > > > >
> > > > > > > I hope so if no one objects the proposed solution here in U-Boot vs.
> > > > > > > the PMP SBI extension. I need have another look at the latest version
> > > > > > > of patches though.
> > > > > > >
> > > > > >
> > > > > > Thanks. As far as I know, there is no opposition to the current approach adopted in U-Boot.
> > > > > > I am hoping EFI stub series can be merged before 5.8. If this series can go in v2020.07, RISC-V will have all required support to boot via EFI from Linux kernel v5.8 and U-Boot v2020.07.
> > > > >
> > > > > OK!
> > > > > I will pull and send a PR ASAP.
> > > >
> > > > I will need have a look and test today. Please wait for some time.
> > > >
> > >
> > > OK
> > > No problem :)
> >
> > Do you know what happened to this series?
> >
> > I only see patch 3, 5, 6 showing up on the patchwork. Are other
> > patches already applied somewhere?
>
> I am referring to http://patchwork.ozlabs.org/project/uboot/list/?series=168858

I checked on patchwork, and the mailing list archive. It looks to me
that the other patches did not arrive on the mailing list and both
patchwork and the archive did not see them.

Could you please resend the v5 of this series?

Regards,
Bin
Atish Patra April 18, 2020, 6:13 a.m. UTC | #17
On Thu, Apr 16, 2020 at 7:27 PM Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Atish,
>
> On Fri, Apr 17, 2020 at 10:14 AM Bin Meng <bmeng.cn at gmail.com> wrote:
> >
> > Correct Palmer's email address
> >
> > On Fri, Apr 17, 2020 at 10:12 AM Bin Meng <bmeng.cn at gmail.com> wrote:
> > >
> > > Hi Rick,
> > >
> > > On Fri, Apr 17, 2020 at 9:12 AM Rick Chen <rickchen36 at gmail.com> wrote:
> > > >
> > > > Hi Bin
> > > >
> > > > > Hi Rick,
> > > > >
> > > > > On Fri, Apr 17, 2020 at 8:51 AM Rick Chen <rickchen36 at gmail.com> wrote:
> > > > > >
> > > > > > <rick at andestech.com> ? 2020?4?17? ?? ??8:39???
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Atish Patra [mailto:atishp at atishpatra.org]
> > > > > > > Sent: Wednesday, April 15, 2020 7:18 AM
> > > > > > > To: Bin Meng
> > > > > > > Cc: Ard Biesheuvel; Heinrich Schuchardt; U-Boot Mailing List; Anup Patel; Lukas Auer; Alexander Graf; Rick Jian-Zhi Chen(???); Palmer Dabbelt
> > > > > > > Subject: Re: [PATCH v5 0/6] RISC-V DT related fixes for reserved memory & UEFI
> > > > > > >
> > > > > > > On Mon, Apr 13, 2020 at 3:42 PM Bin Meng <bmeng.cn at gmail.com> wrote:
> > > > > > > >
> > > > > > > > Hi Atish,
> > > > > > > >
> > > > > > > > On Tue, Apr 14, 2020 at 6:02 AM Atish Patra <atishp at atishpatra.org> wrote:
> > > > > > > > >
> > > > > > > > > On Tue, Apr 7, 2020 at 10:35 AM Atish Patra <atishp at atishpatra.org> wrote:
> > > > > > > > > >
> > > > > > > > > > On Mon, Apr 6, 2020 at 11:51 PM Ard Biesheuvel
> > > > > > > > > > <ard.biesheuvel at linaro.org> wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Tue, 7 Apr 2020 at 08:46, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > On 4/6/20 11:01 PM, Ard Biesheuvel wrote:
> > > > > > > > > > > > > On Mon, 6 Apr 2020 at 22:45, Atish Patra <atish.patra at wdc.com> wrote:
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> This series adds few DT related fixes required for Linux
> > > > > > > > > > > > >> EFI stub to work on RISC-V.
> > > > > > > > > > > > >>
> > > > > > > > > > > > >
> > > > > > > > > > > > > I'm not sure how this is supposed to work, since DT reserved
> > > > > > > > > > > > > memory regions are not used by EFI. If you want to reserve
> > > > > > > > > > > > > memory on a UEFI system, you have to reserve it in the UEFI memory map from firmware.
> > > > > > > > > > > > > The DT reserved-memory node is taken into account too late,
> > > > > > > > > > > > > the /memreserve/ entries are ignored entirely.
> > > > > > > > > > > >
> > > > > > > > > > > > Hello Ard,
> > > > > > > > > > > >
> > > > > > > > > > > > thanks for reviewing.
> > > > > > > > > > > >
> > > > > > > > > > > > What do you mean by "The DT reserved-memory node is taken into
> > > > > > > > > > > > account too late"?
> > > > > > > > > > > >
> > > > > > > > > > > > Cf. commit 7be64b885a36 ("cmd: bootefi: Parse reserved-memory
> > > > > > > > > > > > node from DT")
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > What I mean is that the EFI stub in Linux uses memory allocation
> > > > > > > > > > > functions, expecting the firmware to ensure that those
> > > > > > > > > > > allocations do not conflict with memory descriptions and
> > > > > > > > > > > reservations in DT. So if the firmware wants to express this
> > > > > > > > > > > redundantly, by passing reservations in both reserved-memory and
> > > > > > > > > > > in the EFI memory map, that is probably fine.
> > > > > > > > > > >
> > > > > > > > > > > Also, I must sheepishly admit that I only realize now that this
> > > > > > > > > > > patch set is against u-boot not Linux :-)
> > > > > > > > > > >
> > > > > > > > > > :)
> > > > > > > > > >
> > > > > > > > > > > So if fixed reserved-memory regions are only being used to seed
> > > > > > > > > > > the reserved regions in the EFI memory map, you can safely ignore me.
> > > > > > > > > >
> > > > > > > > > > Yeah. That's the purpose. Having reserved memory nodes in the
> > > > > > > > > > final DT used by linux also ensures that proper Linux adds a
> > > > > > > > > > reserved memory block or removes it from memblock entries
> > > > > > > > > > depending on "no-map" property.
> > > > > > > > > >
> > > > > > > > > > > Apologies for the noise.
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > --
> > > > > > > > > > Regards,
> > > > > > > > > > Atish
> > > > > > > > >
> > > > > > > > > Any other comments on the series ? It would be great if this series
> > > > > > > > > could be merged before
> > > > > > > > > v2020.07 release.
> > > > > > > >
> > > > > > > > I hope so if no one objects the proposed solution here in U-Boot vs.
> > > > > > > > the PMP SBI extension. I need have another look at the latest version
> > > > > > > > of patches though.
> > > > > > > >
> > > > > > >
> > > > > > > Thanks. As far as I know, there is no opposition to the current approach adopted in U-Boot.
> > > > > > > I am hoping EFI stub series can be merged before 5.8. If this series can go in v2020.07, RISC-V will have all required support to boot via EFI from Linux kernel v5.8 and U-Boot v2020.07.
> > > > > >
> > > > > > OK!
> > > > > > I will pull and send a PR ASAP.
> > > > >
> > > > > I will need have a look and test today. Please wait for some time.
> > > > >
> > > >
> > > > OK
> > > > No problem :)
> > >
> > > Do you know what happened to this series?
> > >
> > > I only see patch 3, 5, 6 showing up on the patchwork. Are other
> > > patches already applied somewhere?
> >
> > I am referring to http://patchwork.ozlabs.org/project/uboot/list/?series=168858
>
> I checked on patchwork, and the mailing list archive. It looks to me
> that the other patches did not arrive on the mailing list and both
> patchwork and the archive did not see them.
>

Strange. Not sure how did it happened.
> Could you please resend the v5 of this series?
>

Done. I have resent the series. Thanks for taking a look.

> Regards,
> Bin