All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/1] efi_loader: expose the device-tree file name
@ 2023-10-17 13:49 Heinrich Schuchardt
  2023-10-17 14:15 ` Ilias Apalodimas
  2023-10-18  3:33 ` Simon Glass
  0 siblings, 2 replies; 26+ messages in thread
From: Heinrich Schuchardt @ 2023-10-17 13:49 UTC (permalink / raw)
  To: Ilias Apalodimas; +Cc: u-boot, Heinrich Schuchardt

Forward and backward compatibility of Linux kernel device-trees is
sometimes missing. One solution approach is to load a kernel specific
device-tree. This can either be done via a U-Boot scripts (like the one
generated by Debian package flash-kernel or by a boot loader like GRUB.
The boot loader approach currently requires to know the device-tree name
before first boot which makes it unusable for generic images.

Expose the device-tree file name as EFI variable FdtFile.
This will allow bootloaders to load a kernel specific device-tree.

The variable will not be exposed on ACPI based systems or if the
environment variable fdtfile is not defined.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
v2:
	Use a unique GUID to enable future U-Boot independent
	standardization.
	Do not try to add the variable on ACPI based systems.
---
 include/efi_loader.h       |  5 +++++
 lib/efi_loader/efi_setup.c | 30 ++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index e24410505f..2faa1c191c 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -152,6 +152,11 @@ static inline efi_status_t efi_launch_capsules(void)
 	EFI_GUID(0x8108ac4e, 0x9f11, 0x4d59, \
 		 0x85, 0x0e, 0xe2, 0x1a, 0x52, 0x2c, 0x59, 0xb2)
 
+/* Vendor GUID for the FdtFile variable */
+#define VENDOR_FDTFILE_GUID \
+	EFI_GUID(d45dde69, 0x3bd6, 0x40e0, \
+		 0x90, 0xd5, 0x6b, 0x60, 0x6a, 0xa5, 0x77, 0x30)
+
 /* Use internal device tree when starting UEFI application */
 #define EFI_FDT_USE_INTERNAL NULL
 
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index e6de685e87..71bcde645b 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -17,6 +17,8 @@
 
 efi_status_t efi_obj_list_initialized = OBJ_LIST_NOT_INITIALIZED;
 
+efi_guid_t vendor_fdtfile_guid = VENDOR_FDTFILE_GUID;
+
 /*
  * Allow unaligned memory access.
  *
@@ -26,6 +28,27 @@ void __weak allow_unaligned(void)
 {
 }
 
+/**
+ * efi_init_fdtfile() - set EFI variable FdtFile
+ *
+ * Return:	status code
+ */
+static efi_status_t efi_init_fdtfile(void)
+{
+	char *val;
+
+	val = env_get("fdtfile");
+	if (!val)
+		return EFI_SUCCESS;
+
+	return efi_set_variable_int(u"FdtFile",
+				    &vendor_fdtfile_guid,
+				    EFI_VARIABLE_BOOTSERVICE_ACCESS |
+				    EFI_VARIABLE_RUNTIME_ACCESS |
+				    EFI_VARIABLE_READ_ONLY,
+				    strlen(val) + 1, val, false);
+}
+
 /**
  * efi_init_platform_lang() - define supported languages
  *
@@ -250,6 +273,13 @@ efi_status_t efi_init_obj_list(void)
 	if (ret != EFI_SUCCESS)
 		goto out;
 
+	/* Define EFI variable FdtFile */
+	if (!CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)) {
+		ret = efi_init_fdtfile();
+		if (ret != EFI_SUCCESS)
+			goto out;
+	}
+
 	/* Indicate supported features */
 	ret = efi_init_os_indications();
 	if (ret != EFI_SUCCESS)
-- 
2.40.1


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

* Re: [PATCH v2 1/1] efi_loader: expose the device-tree file name
  2023-10-17 13:49 [PATCH v2 1/1] efi_loader: expose the device-tree file name Heinrich Schuchardt
@ 2023-10-17 14:15 ` Ilias Apalodimas
  2023-10-18  3:33 ` Simon Glass
  1 sibling, 0 replies; 26+ messages in thread
From: Ilias Apalodimas @ 2023-10-17 14:15 UTC (permalink / raw)
  To: Heinrich Schuchardt; +Cc: u-boot

On Tue, 17 Oct 2023 at 16:50, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> Forward and backward compatibility of Linux kernel device-trees is
> sometimes missing. One solution approach is to load a kernel specific
> device-tree. This can either be done via a U-Boot scripts (like the one
> generated by Debian package flash-kernel or by a boot loader like GRUB.
> The boot loader approach currently requires to know the device-tree name
> before first boot which makes it unusable for generic images.
>
> Expose the device-tree file name as EFI variable FdtFile.
> This will allow bootloaders to load a kernel specific device-tree.
>
> The variable will not be exposed on ACPI based systems or if the
> environment variable fdtfile is not defined.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> v2:
>         Use a unique GUID to enable future U-Boot independent
>         standardization.
>         Do not try to add the variable on ACPI based systems.
> ---
>  include/efi_loader.h       |  5 +++++
>  lib/efi_loader/efi_setup.c | 30 ++++++++++++++++++++++++++++++
>  2 files changed, 35 insertions(+)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index e24410505f..2faa1c191c 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -152,6 +152,11 @@ static inline efi_status_t efi_launch_capsules(void)
>         EFI_GUID(0x8108ac4e, 0x9f11, 0x4d59, \
>                  0x85, 0x0e, 0xe2, 0x1a, 0x52, 0x2c, 0x59, 0xb2)
>
> +/* Vendor GUID for the FdtFile variable */
> +#define VENDOR_FDTFILE_GUID \
> +       EFI_GUID(d45dde69, 0x3bd6, 0x40e0, \
> +                0x90, 0xd5, 0x6b, 0x60, 0x6a, 0xa5, 0x77, 0x30)
> +
>  /* Use internal device tree when starting UEFI application */
>  #define EFI_FDT_USE_INTERNAL NULL
>
> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> index e6de685e87..71bcde645b 100644
> --- a/lib/efi_loader/efi_setup.c
> +++ b/lib/efi_loader/efi_setup.c
> @@ -17,6 +17,8 @@
>
>  efi_status_t efi_obj_list_initialized = OBJ_LIST_NOT_INITIALIZED;
>
> +efi_guid_t vendor_fdtfile_guid = VENDOR_FDTFILE_GUID;
> +
>  /*
>   * Allow unaligned memory access.
>   *
> @@ -26,6 +28,27 @@ void __weak allow_unaligned(void)
>  {
>  }
>
> +/**
> + * efi_init_fdtfile() - set EFI variable FdtFile
> + *
> + * Return:     status code
> + */
> +static efi_status_t efi_init_fdtfile(void)
> +{
> +       char *val;
> +
> +       val = env_get("fdtfile");
> +       if (!val)
> +               return EFI_SUCCESS;
> +
> +       return efi_set_variable_int(u"FdtFile",
> +                                   &vendor_fdtfile_guid,
> +                                   EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +                                   EFI_VARIABLE_RUNTIME_ACCESS |
> +                                   EFI_VARIABLE_READ_ONLY,
> +                                   strlen(val) + 1, val, false);
> +}
> +
>  /**
>   * efi_init_platform_lang() - define supported languages
>   *
> @@ -250,6 +273,13 @@ efi_status_t efi_init_obj_list(void)
>         if (ret != EFI_SUCCESS)
>                 goto out;
>
> +       /* Define EFI variable FdtFile */
> +       if (!CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)) {
> +               ret = efi_init_fdtfile();
> +               if (ret != EFI_SUCCESS)
> +                       goto out;
> +       }
> +
>         /* Indicate supported features */
>         ret = efi_init_os_indications();
>         if (ret != EFI_SUCCESS)
> --
> 2.40.1
>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

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

* Re: [PATCH v2 1/1] efi_loader: expose the device-tree file name
  2023-10-17 13:49 [PATCH v2 1/1] efi_loader: expose the device-tree file name Heinrich Schuchardt
  2023-10-17 14:15 ` Ilias Apalodimas
@ 2023-10-18  3:33 ` Simon Glass
  2023-10-18  8:15   ` Heinrich Schuchardt
  1 sibling, 1 reply; 26+ messages in thread
From: Simon Glass @ 2023-10-18  3:33 UTC (permalink / raw)
  To: Heinrich Schuchardt; +Cc: Ilias Apalodimas, u-boot

Hi Heinrich,

On Tue, 17 Oct 2023 at 07:50, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> Forward and backward compatibility of Linux kernel device-trees is
> sometimes missing. One solution approach is to load a kernel specific
> device-tree. This can either be done via a U-Boot scripts (like the one
> generated by Debian package flash-kernel or by a boot loader like GRUB.
> The boot loader approach currently requires to know the device-tree name
> before first boot which makes it unusable for generic images.
>
> Expose the device-tree file name as EFI variable FdtFile.
> This will allow bootloaders to load a kernel specific device-tree.
>
> The variable will not be exposed on ACPI based systems or if the
> environment variable fdtfile is not defined.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> v2:
>         Use a unique GUID to enable future U-Boot independent
>         standardization.
>         Do not try to add the variable on ACPI based systems.
> ---
>  include/efi_loader.h       |  5 +++++
>  lib/efi_loader/efi_setup.c | 30 ++++++++++++++++++++++++++++++
>  2 files changed, 35 insertions(+)

I was too slow to reply to v1.

Does grub load the DT? I was assuming that U-Boot would pass it on?
What is the interface between U-Boot and grub?

Regards,
Simon

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

* Re: [PATCH v2 1/1] efi_loader: expose the device-tree file name
  2023-10-18  3:33 ` Simon Glass
@ 2023-10-18  8:15   ` Heinrich Schuchardt
  2023-10-19 13:55     ` Simon Glass
  0 siblings, 1 reply; 26+ messages in thread
From: Heinrich Schuchardt @ 2023-10-18  8:15 UTC (permalink / raw)
  To: Simon Glass; +Cc: Ilias Apalodimas, u-boot

On 10/18/23 05:33, Simon Glass wrote:
> Hi Heinrich,
> 
> On Tue, 17 Oct 2023 at 07:50, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
>>
>> Forward and backward compatibility of Linux kernel device-trees is
>> sometimes missing. One solution approach is to load a kernel specific
>> device-tree. This can either be done via a U-Boot scripts (like the one
>> generated by Debian package flash-kernel or by a boot loader like GRUB.
>> The boot loader approach currently requires to know the device-tree name
>> before first boot which makes it unusable for generic images.
>>
>> Expose the device-tree file name as EFI variable FdtFile.
>> This will allow bootloaders to load a kernel specific device-tree.
>>
>> The variable will not be exposed on ACPI based systems or if the
>> environment variable fdtfile is not defined.
>>
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>> ---
>> v2:
>>          Use a unique GUID to enable future U-Boot independent
>>          standardization.
>>          Do not try to add the variable on ACPI based systems.
>> ---
>>   include/efi_loader.h       |  5 +++++
>>   lib/efi_loader/efi_setup.c | 30 ++++++++++++++++++++++++++++++
>>   2 files changed, 35 insertions(+)
> 
> I was too slow to reply to v1.
> 
> Does grub load the DT? I was assuming that U-Boot would pass it on?
> What is the interface between U-Boot and grub?

The device-tree built into U-Boot is often out of date and not usable to 
boot current Linux. A single device-tree can be loaded by U-Boot from 
file and passed on as EFI configuration table. This device-tree may not 
be compatible with all kernel versions exposed by GRUB.

GRUB provides a devicetree command. It is disabled if you use secure 
boot. At least in Debian and Ubuntu GRUB invokes the 
EFI_DT_FIXUP_PROTOCOL exposed by U-Boot to run U-Boot's device-tree 
fix-ups after loading a device-tree.

Vendor scripts for GRUB like Ubuntu's /etc/grub.d/10_linux add 
devicetree commands to the boot options in grub.cfg.

Best regards

Heinrich

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

* Re: [PATCH v2 1/1] efi_loader: expose the device-tree file name
  2023-10-18  8:15   ` Heinrich Schuchardt
@ 2023-10-19 13:55     ` Simon Glass
  2023-10-19 14:14       ` Mark Kettenis
  2023-10-19 16:09       ` Heinrich Schuchardt
  0 siblings, 2 replies; 26+ messages in thread
From: Simon Glass @ 2023-10-19 13:55 UTC (permalink / raw)
  To: Heinrich Schuchardt; +Cc: Ilias Apalodimas, u-boot

Hi Heinrich,

On Wed, 18 Oct 2023 at 02:15, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> On 10/18/23 05:33, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Tue, 17 Oct 2023 at 07:50, Heinrich Schuchardt
> > <heinrich.schuchardt@canonical.com> wrote:
> >>
> >> Forward and backward compatibility of Linux kernel device-trees is
> >> sometimes missing. One solution approach is to load a kernel specific
> >> device-tree. This can either be done via a U-Boot scripts (like the one
> >> generated by Debian package flash-kernel or by a boot loader like GRUB.
> >> The boot loader approach currently requires to know the device-tree name
> >> before first boot which makes it unusable for generic images.
> >>
> >> Expose the device-tree file name as EFI variable FdtFile.
> >> This will allow bootloaders to load a kernel specific device-tree.
> >>
> >> The variable will not be exposed on ACPI based systems or if the
> >> environment variable fdtfile is not defined.
> >>
> >> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> >> ---
> >> v2:
> >>          Use a unique GUID to enable future U-Boot independent
> >>          standardization.
> >>          Do not try to add the variable on ACPI based systems.
> >> ---
> >>   include/efi_loader.h       |  5 +++++
> >>   lib/efi_loader/efi_setup.c | 30 ++++++++++++++++++++++++++++++
> >>   2 files changed, 35 insertions(+)
> >
> > I was too slow to reply to v1.
> >
> > Does grub load the DT? I was assuming that U-Boot would pass it on?
> > What is the interface between U-Boot and grub?
>
> The device-tree built into U-Boot is often out of date and not usable to
> boot current Linux. A single device-tree can be loaded by U-Boot from
> file and passed on as EFI configuration table. This device-tree may not
> be compatible with all kernel versions exposed by GRUB.
>
> GRUB provides a devicetree command. It is disabled if you use secure
> boot. At least in Debian and Ubuntu GRUB invokes the
> EFI_DT_FIXUP_PROTOCOL exposed by U-Boot to run U-Boot's device-tree
> fix-ups after loading a device-tree.
>
> Vendor scripts for GRUB like Ubuntu's /etc/grub.d/10_linux add
> devicetree commands to the boot options in grub.cfg.

Thanks. I wonder if you could document this somewhere? It seems like
there are a lot of options and it is quite complicated.

Back to the question, I suppose you are expecting grub to load the DT
using this filename? But why doesn't U-Boot load it instead? It seems
very convoluted.

Also, can we test this interface?

Regards,
Simon

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

* Re: [PATCH v2 1/1] efi_loader: expose the device-tree file name
  2023-10-19 13:55     ` Simon Glass
@ 2023-10-19 14:14       ` Mark Kettenis
  2023-10-19 16:09       ` Heinrich Schuchardt
  1 sibling, 0 replies; 26+ messages in thread
From: Mark Kettenis @ 2023-10-19 14:14 UTC (permalink / raw)
  To: Simon Glass; +Cc: heinrich.schuchardt, ilias.apalodimas, u-boot

> From: Simon Glass <sjg@google.com>
> Date: Thu, 19 Oct 2023 07:55:49 -0600
> 
> Hi Heinrich,
> 
> On Wed, 18 Oct 2023 at 02:15, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
> >
> > On 10/18/23 05:33, Simon Glass wrote:
> > > Hi Heinrich,
> > >
> > > On Tue, 17 Oct 2023 at 07:50, Heinrich Schuchardt
> > > <heinrich.schuchardt@canonical.com> wrote:
> > >>
> > >> Forward and backward compatibility of Linux kernel device-trees is
> > >> sometimes missing. One solution approach is to load a kernel specific
> > >> device-tree. This can either be done via a U-Boot scripts (like the one
> > >> generated by Debian package flash-kernel or by a boot loader like GRUB.
> > >> The boot loader approach currently requires to know the device-tree name
> > >> before first boot which makes it unusable for generic images.
> > >>
> > >> Expose the device-tree file name as EFI variable FdtFile.
> > >> This will allow bootloaders to load a kernel specific device-tree.
> > >>
> > >> The variable will not be exposed on ACPI based systems or if the
> > >> environment variable fdtfile is not defined.
> > >>
> > >> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > >> ---
> > >> v2:
> > >>          Use a unique GUID to enable future U-Boot independent
> > >>          standardization.
> > >>          Do not try to add the variable on ACPI based systems.
> > >> ---
> > >>   include/efi_loader.h       |  5 +++++
> > >>   lib/efi_loader/efi_setup.c | 30 ++++++++++++++++++++++++++++++
> > >>   2 files changed, 35 insertions(+)
> > >
> > > I was too slow to reply to v1.
> > >
> > > Does grub load the DT? I was assuming that U-Boot would pass it on?
> > > What is the interface between U-Boot and grub?
> >
> > The device-tree built into U-Boot is often out of date and not usable to
> > boot current Linux. A single device-tree can be loaded by U-Boot from
> > file and passed on as EFI configuration table. This device-tree may not
> > be compatible with all kernel versions exposed by GRUB.
> >
> > GRUB provides a devicetree command. It is disabled if you use secure
> > boot. At least in Debian and Ubuntu GRUB invokes the
> > EFI_DT_FIXUP_PROTOCOL exposed by U-Boot to run U-Boot's device-tree
> > fix-ups after loading a device-tree.
> >
> > Vendor scripts for GRUB like Ubuntu's /etc/grub.d/10_linux add
> > devicetree commands to the boot options in grub.cfg.
> 
> Thanks. I wonder if you could document this somewhere? It seems like
> there are a lot of options and it is quite complicated.
> 
> Back to the question, I suppose you are expecting grub to load the DT
> using this filename? But why doesn't U-Boot load it instead? It seems
> very convoluted.

From an OpenBSD perspective:

* It allows us to load directly from an OpenBSD filesystem.  (And yes
  I really think you don't want to add support for all the different
  flavours of the UFS/FFS filesystem to U-Boot).

* It means we don't have to worry about breaking other OS installs on
  the same disk since we can avoid copying the device tree onto the
  (shared) EFI System Partition.

* It means that we can provide a uniform user interface that is
  independent on the UEFI implementation; the user doesn't need to
  care whether they are dealing with U-Boot, EDK2 or some other
  proprietary UEFI implementation.

Cheers,

Mark

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

* Re: [PATCH v2 1/1] efi_loader: expose the device-tree file name
  2023-10-19 13:55     ` Simon Glass
  2023-10-19 14:14       ` Mark Kettenis
@ 2023-10-19 16:09       ` Heinrich Schuchardt
  2023-10-20 13:21         ` Simon Glass
  1 sibling, 1 reply; 26+ messages in thread
From: Heinrich Schuchardt @ 2023-10-19 16:09 UTC (permalink / raw)
  To: Simon Glass; +Cc: Ilias Apalodimas, u-boot

On 19.10.23 15:55, Simon Glass wrote:
> Hi Heinrich,
> 
> On Wed, 18 Oct 2023 at 02:15, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
>>
>> On 10/18/23 05:33, Simon Glass wrote:
>>> Hi Heinrich,
>>>
>>> On Tue, 17 Oct 2023 at 07:50, Heinrich Schuchardt
>>> <heinrich.schuchardt@canonical.com> wrote:
>>>>
>>>> Forward and backward compatibility of Linux kernel device-trees is
>>>> sometimes missing. One solution approach is to load a kernel specific
>>>> device-tree. This can either be done via a U-Boot scripts (like the one
>>>> generated by Debian package flash-kernel or by a boot loader like GRUB.
>>>> The boot loader approach currently requires to know the device-tree name
>>>> before first boot which makes it unusable for generic images.
>>>>
>>>> Expose the device-tree file name as EFI variable FdtFile.
>>>> This will allow bootloaders to load a kernel specific device-tree.
>>>>
>>>> The variable will not be exposed on ACPI based systems or if the
>>>> environment variable fdtfile is not defined.
>>>>
>>>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>>>> ---
>>>> v2:
>>>>           Use a unique GUID to enable future U-Boot independent
>>>>           standardization.
>>>>           Do not try to add the variable on ACPI based systems.
>>>> ---
>>>>    include/efi_loader.h       |  5 +++++
>>>>    lib/efi_loader/efi_setup.c | 30 ++++++++++++++++++++++++++++++
>>>>    2 files changed, 35 insertions(+)
>>>
>>> I was too slow to reply to v1.
>>>
>>> Does grub load the DT? I was assuming that U-Boot would pass it on?
>>> What is the interface between U-Boot and grub?
>>
>> The device-tree built into U-Boot is often out of date and not usable to
>> boot current Linux. A single device-tree can be loaded by U-Boot from
>> file and passed on as EFI configuration table. This device-tree may not
>> be compatible with all kernel versions exposed by GRUB.
>>
>> GRUB provides a devicetree command. It is disabled if you use secure
>> boot. At least in Debian and Ubuntu GRUB invokes the
>> EFI_DT_FIXUP_PROTOCOL exposed by U-Boot to run U-Boot's device-tree
>> fix-ups after loading a device-tree.
>>
>> Vendor scripts for GRUB like Ubuntu's /etc/grub.d/10_linux add
>> devicetree commands to the boot options in grub.cfg.
> 
> Thanks. I wonder if you could document this somewhere? It seems like
> there are a lot of options and it is quite complicated.
> 
> Back to the question, I suppose you are expecting grub to load the DT
> using this filename? But why doesn't U-Boot load it instead? It seems
> very convoluted.

A separate file of this name exists for every kernel version installed. 
The loaded dtb must match the kernel. U-Boot does not know what kernel 
version will be chosen in GRUB. And for a generic image GRUB does not 
what board it is on.

> 
> Also, can we test this interface?

Neither the sandbox nor QEMU have environment variable fdtfile. And we 
don't create the EFI variable with ACPI as used on the sandbox.

Best regards

Heinrich

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

* Re: [PATCH v2 1/1] efi_loader: expose the device-tree file name
  2023-10-19 16:09       ` Heinrich Schuchardt
@ 2023-10-20 13:21         ` Simon Glass
  2023-10-20 13:55           ` Tom Rini
                             ` (2 more replies)
  0 siblings, 3 replies; 26+ messages in thread
From: Simon Glass @ 2023-10-20 13:21 UTC (permalink / raw)
  To: Heinrich Schuchardt; +Cc: Ilias Apalodimas, u-boot

+Doug Anderson

Hi Heinrich,

On Thu, 19 Oct 2023 at 09:09, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> On 19.10.23 15:55, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Wed, 18 Oct 2023 at 02:15, Heinrich Schuchardt
> > <heinrich.schuchardt@canonical.com> wrote:
> >>
> >> On 10/18/23 05:33, Simon Glass wrote:
> >>> Hi Heinrich,
> >>>
> >>> On Tue, 17 Oct 2023 at 07:50, Heinrich Schuchardt
> >>> <heinrich.schuchardt@canonical.com> wrote:
> >>>>
> >>>> Forward and backward compatibility of Linux kernel device-trees is
> >>>> sometimes missing. One solution approach is to load a kernel specific
> >>>> device-tree. This can either be done via a U-Boot scripts (like the one
> >>>> generated by Debian package flash-kernel or by a boot loader like GRUB.
> >>>> The boot loader approach currently requires to know the device-tree name
> >>>> before first boot which makes it unusable for generic images.
> >>>>
> >>>> Expose the device-tree file name as EFI variable FdtFile.
> >>>> This will allow bootloaders to load a kernel specific device-tree.
> >>>>
> >>>> The variable will not be exposed on ACPI based systems or if the
> >>>> environment variable fdtfile is not defined.
> >>>>
> >>>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> >>>> ---
> >>>> v2:
> >>>>           Use a unique GUID to enable future U-Boot independent
> >>>>           standardization.
> >>>>           Do not try to add the variable on ACPI based systems.
> >>>> ---
> >>>>    include/efi_loader.h       |  5 +++++
> >>>>    lib/efi_loader/efi_setup.c | 30 ++++++++++++++++++++++++++++++
> >>>>    2 files changed, 35 insertions(+)
> >>>
> >>> I was too slow to reply to v1.
> >>>
> >>> Does grub load the DT? I was assuming that U-Boot would pass it on?
> >>> What is the interface between U-Boot and grub?
> >>
> >> The device-tree built into U-Boot is often out of date and not usable to
> >> boot current Linux. A single device-tree can be loaded by U-Boot from
> >> file and passed on as EFI configuration table. This device-tree may not
> >> be compatible with all kernel versions exposed by GRUB.
> >>
> >> GRUB provides a devicetree command. It is disabled if you use secure
> >> boot. At least in Debian and Ubuntu GRUB invokes the
> >> EFI_DT_FIXUP_PROTOCOL exposed by U-Boot to run U-Boot's device-tree
> >> fix-ups after loading a device-tree.
> >>
> >> Vendor scripts for GRUB like Ubuntu's /etc/grub.d/10_linux add
> >> devicetree commands to the boot options in grub.cfg.
> >
> > Thanks. I wonder if you could document this somewhere? It seems like
> > there are a lot of options and it is quite complicated.
> >
> > Back to the question, I suppose you are expecting grub to load the DT
> > using this filename? But why doesn't U-Boot load it instead? It seems
> > very convoluted.
>
> A separate file of this name exists for every kernel version installed.
> The loaded dtb must match the kernel. U-Boot does not know what kernel
> version will be chosen in GRUB. And for a generic image GRUB does not
> what board it is on.
>
> >
> > Also, can we test this interface?
>
> Neither the sandbox nor QEMU have environment variable fdtfile. And we
> don't create the EFI variable with ACPI as used on the sandbox.

I worry that this is creating another interface that some poor sod is
going to have to deal with in the future. Is this part of the EFI
standard?

We should really be using the compatible string to choose the
devicetree. Why are we using filenames at all? What is the
relationship between the compatible string and the filename? Is there
a lookup table, or should we create one?

The correct way of doing this is implemented in U-Boot with
CONFIG_FIT_BEST_MATCH. Can we mirror something like that in grub, etc?

Regards,
Simon

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

* Re: [PATCH v2 1/1] efi_loader: expose the device-tree file name
  2023-10-20 13:21         ` Simon Glass
@ 2023-10-20 13:55           ` Tom Rini
  2023-10-20 15:40           ` Heinrich Schuchardt
  2023-10-20 21:19           ` Simon Glass
  2 siblings, 0 replies; 26+ messages in thread
From: Tom Rini @ 2023-10-20 13:55 UTC (permalink / raw)
  To: Simon Glass, Peter Robinson; +Cc: Heinrich Schuchardt, Ilias Apalodimas, u-boot

[-- Attachment #1: Type: text/plain, Size: 4556 bytes --]

On Fri, Oct 20, 2023 at 06:21:33AM -0700, Simon Glass wrote:
> +Doug Anderson
> 
> Hi Heinrich,
> 
> On Thu, 19 Oct 2023 at 09:09, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
> >
> > On 19.10.23 15:55, Simon Glass wrote:
> > > Hi Heinrich,
> > >
> > > On Wed, 18 Oct 2023 at 02:15, Heinrich Schuchardt
> > > <heinrich.schuchardt@canonical.com> wrote:
> > >>
> > >> On 10/18/23 05:33, Simon Glass wrote:
> > >>> Hi Heinrich,
> > >>>
> > >>> On Tue, 17 Oct 2023 at 07:50, Heinrich Schuchardt
> > >>> <heinrich.schuchardt@canonical.com> wrote:
> > >>>>
> > >>>> Forward and backward compatibility of Linux kernel device-trees is
> > >>>> sometimes missing. One solution approach is to load a kernel specific
> > >>>> device-tree. This can either be done via a U-Boot scripts (like the one
> > >>>> generated by Debian package flash-kernel or by a boot loader like GRUB.
> > >>>> The boot loader approach currently requires to know the device-tree name
> > >>>> before first boot which makes it unusable for generic images.
> > >>>>
> > >>>> Expose the device-tree file name as EFI variable FdtFile.
> > >>>> This will allow bootloaders to load a kernel specific device-tree.
> > >>>>
> > >>>> The variable will not be exposed on ACPI based systems or if the
> > >>>> environment variable fdtfile is not defined.
> > >>>>
> > >>>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > >>>> ---
> > >>>> v2:
> > >>>>           Use a unique GUID to enable future U-Boot independent
> > >>>>           standardization.
> > >>>>           Do not try to add the variable on ACPI based systems.
> > >>>> ---
> > >>>>    include/efi_loader.h       |  5 +++++
> > >>>>    lib/efi_loader/efi_setup.c | 30 ++++++++++++++++++++++++++++++
> > >>>>    2 files changed, 35 insertions(+)
> > >>>
> > >>> I was too slow to reply to v1.
> > >>>
> > >>> Does grub load the DT? I was assuming that U-Boot would pass it on?
> > >>> What is the interface between U-Boot and grub?
> > >>
> > >> The device-tree built into U-Boot is often out of date and not usable to
> > >> boot current Linux. A single device-tree can be loaded by U-Boot from
> > >> file and passed on as EFI configuration table. This device-tree may not
> > >> be compatible with all kernel versions exposed by GRUB.
> > >>
> > >> GRUB provides a devicetree command. It is disabled if you use secure
> > >> boot. At least in Debian and Ubuntu GRUB invokes the
> > >> EFI_DT_FIXUP_PROTOCOL exposed by U-Boot to run U-Boot's device-tree
> > >> fix-ups after loading a device-tree.
> > >>
> > >> Vendor scripts for GRUB like Ubuntu's /etc/grub.d/10_linux add
> > >> devicetree commands to the boot options in grub.cfg.
> > >
> > > Thanks. I wonder if you could document this somewhere? It seems like
> > > there are a lot of options and it is quite complicated.
> > >
> > > Back to the question, I suppose you are expecting grub to load the DT
> > > using this filename? But why doesn't U-Boot load it instead? It seems
> > > very convoluted.
> >
> > A separate file of this name exists for every kernel version installed.
> > The loaded dtb must match the kernel. U-Boot does not know what kernel
> > version will be chosen in GRUB. And for a generic image GRUB does not
> > what board it is on.
> >
> > >
> > > Also, can we test this interface?
> >
> > Neither the sandbox nor QEMU have environment variable fdtfile. And we
> > don't create the EFI variable with ACPI as used on the sandbox.
> 
> I worry that this is creating another interface that some poor sod is
> going to have to deal with in the future. Is this part of the EFI
> standard?
> 
> We should really be using the compatible string to choose the
> devicetree. Why are we using filenames at all? What is the
> relationship between the compatible string and the filename? Is there
> a lookup table, or should we create one?
> 
> The correct way of doing this is implemented in U-Boot with
> CONFIG_FIT_BEST_MATCH. Can we mirror something like that in grub, etc?

I'm adding Peter here as well because we spent a good chunk of time
yesterday digging in to issues that turn out to be, in short the patch
to make Fedora load the device tree that matches the kernel be loaded
wasn't in the actively used path anymore.

So we continue to be in the place where the hope is that in a few years,
platforms will ship with a device tree that the OS can use, but that
reality is generally not there yet.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

* Re: [PATCH v2 1/1] efi_loader: expose the device-tree file name
  2023-10-20 13:21         ` Simon Glass
  2023-10-20 13:55           ` Tom Rini
@ 2023-10-20 15:40           ` Heinrich Schuchardt
  2023-10-20 16:24             ` Tom Rini
  2023-10-20 21:19           ` Simon Glass
  2 siblings, 1 reply; 26+ messages in thread
From: Heinrich Schuchardt @ 2023-10-20 15:40 UTC (permalink / raw)
  To: Simon Glass; +Cc: Ilias Apalodimas, u-boot

On 20.10.23 15:21, Simon Glass wrote:
> +Doug Anderson
> 
> Hi Heinrich,
> 
> On Thu, 19 Oct 2023 at 09:09, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
>>
>> On 19.10.23 15:55, Simon Glass wrote:
>>> Hi Heinrich,
>>>
>>> On Wed, 18 Oct 2023 at 02:15, Heinrich Schuchardt
>>> <heinrich.schuchardt@canonical.com> wrote:
>>>>
>>>> On 10/18/23 05:33, Simon Glass wrote:
>>>>> Hi Heinrich,
>>>>>
>>>>> On Tue, 17 Oct 2023 at 07:50, Heinrich Schuchardt
>>>>> <heinrich.schuchardt@canonical.com> wrote:
>>>>>>
>>>>>> Forward and backward compatibility of Linux kernel device-trees is
>>>>>> sometimes missing. One solution approach is to load a kernel specific
>>>>>> device-tree. This can either be done via a U-Boot scripts (like the one
>>>>>> generated by Debian package flash-kernel or by a boot loader like GRUB.
>>>>>> The boot loader approach currently requires to know the device-tree name
>>>>>> before first boot which makes it unusable for generic images.
>>>>>>
>>>>>> Expose the device-tree file name as EFI variable FdtFile.
>>>>>> This will allow bootloaders to load a kernel specific device-tree.
>>>>>>
>>>>>> The variable will not be exposed on ACPI based systems or if the
>>>>>> environment variable fdtfile is not defined.
>>>>>>
>>>>>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>>>>>> ---
>>>>>> v2:
>>>>>>            Use a unique GUID to enable future U-Boot independent
>>>>>>            standardization.
>>>>>>            Do not try to add the variable on ACPI based systems.
>>>>>> ---
>>>>>>     include/efi_loader.h       |  5 +++++
>>>>>>     lib/efi_loader/efi_setup.c | 30 ++++++++++++++++++++++++++++++
>>>>>>     2 files changed, 35 insertions(+)
>>>>>
>>>>> I was too slow to reply to v1.
>>>>>
>>>>> Does grub load the DT? I was assuming that U-Boot would pass it on?
>>>>> What is the interface between U-Boot and grub?
>>>>
>>>> The device-tree built into U-Boot is often out of date and not usable to
>>>> boot current Linux. A single device-tree can be loaded by U-Boot from
>>>> file and passed on as EFI configuration table. This device-tree may not
>>>> be compatible with all kernel versions exposed by GRUB.
>>>>
>>>> GRUB provides a devicetree command. It is disabled if you use secure
>>>> boot. At least in Debian and Ubuntu GRUB invokes the
>>>> EFI_DT_FIXUP_PROTOCOL exposed by U-Boot to run U-Boot's device-tree
>>>> fix-ups after loading a device-tree.
>>>>
>>>> Vendor scripts for GRUB like Ubuntu's /etc/grub.d/10_linux add
>>>> devicetree commands to the boot options in grub.cfg.
>>>
>>> Thanks. I wonder if you could document this somewhere? It seems like
>>> there are a lot of options and it is quite complicated.
>>>
>>> Back to the question, I suppose you are expecting grub to load the DT
>>> using this filename? But why doesn't U-Boot load it instead? It seems
>>> very convoluted.
>>
>> A separate file of this name exists for every kernel version installed.
>> The loaded dtb must match the kernel. U-Boot does not know what kernel
>> version will be chosen in GRUB. And for a generic image GRUB does not
>> what board it is on.
>>
>>>
>>> Also, can we test this interface?
>>
>> Neither the sandbox nor QEMU have environment variable fdtfile. And we
>> don't create the EFI variable with ACPI as used on the sandbox.
> 
> I worry that this is creating another interface that some poor sod is
> going to have to deal with in the future. Is this part of the EFI
> standard?

No, the UEFI standard does not care much about device-trees. It only 
defines the GUID for the configuration table.

> 
> We should really be using the compatible string to choose the
> devicetree. Why are we using filenames at all? What is the
> relationship between the compatible string and the filename? Is there
> a lookup table, or should we create one?

There is no 1:1 relationship between compatible string and filename, 
e.g. the following arm64 device-trees use the same compatible string:

amd/amd-overdrive-rev-b0.dts
amd/amd-overdrive-rev-b1.dts

amlogic/meson-axg-jethome-jethub-j110-rev-2.dts
amlogic/meson-axg-jethome-jethub-j110-rev-3.dts

xilinx/zynqmp-zc1751-xm015-dc1.dts
xilinx/zynqmp-zc1751-xm015-dc1.dts
xilinx/zynqmp-zc1751-xm015-dc1.dts
xilinx/zynqmp-zc1751-xm015-dc1.dts

> 
> The correct way of doing this is implemented in U-Boot with
> CONFIG_FIT_BEST_MATCH. Can we mirror something like that in grub, etc?

Why should we write complicated code to find a *possibly* matching file 
if we already know the filename that needs to be loaded?

Best regards

Heinrich

> 
> Regards,
> Simon


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

* Re: [PATCH v2 1/1] efi_loader: expose the device-tree file name
  2023-10-20 15:40           ` Heinrich Schuchardt
@ 2023-10-20 16:24             ` Tom Rini
  2023-10-21 15:42               ` Simon Glass
  0 siblings, 1 reply; 26+ messages in thread
From: Tom Rini @ 2023-10-20 16:24 UTC (permalink / raw)
  To: Heinrich Schuchardt; +Cc: Simon Glass, Ilias Apalodimas, u-boot

[-- Attachment #1: Type: text/plain, Size: 6007 bytes --]

On Fri, Oct 20, 2023 at 05:40:03PM +0200, Heinrich Schuchardt wrote:
> On 20.10.23 15:21, Simon Glass wrote:
> > +Doug Anderson
> > 
> > Hi Heinrich,
> > 
> > On Thu, 19 Oct 2023 at 09:09, Heinrich Schuchardt
> > <heinrich.schuchardt@canonical.com> wrote:
> > > 
> > > On 19.10.23 15:55, Simon Glass wrote:
> > > > Hi Heinrich,
> > > > 
> > > > On Wed, 18 Oct 2023 at 02:15, Heinrich Schuchardt
> > > > <heinrich.schuchardt@canonical.com> wrote:
> > > > > 
> > > > > On 10/18/23 05:33, Simon Glass wrote:
> > > > > > Hi Heinrich,
> > > > > > 
> > > > > > On Tue, 17 Oct 2023 at 07:50, Heinrich Schuchardt
> > > > > > <heinrich.schuchardt@canonical.com> wrote:
> > > > > > > 
> > > > > > > Forward and backward compatibility of Linux kernel device-trees is
> > > > > > > sometimes missing. One solution approach is to load a kernel specific
> > > > > > > device-tree. This can either be done via a U-Boot scripts (like the one
> > > > > > > generated by Debian package flash-kernel or by a boot loader like GRUB.
> > > > > > > The boot loader approach currently requires to know the device-tree name
> > > > > > > before first boot which makes it unusable for generic images.
> > > > > > > 
> > > > > > > Expose the device-tree file name as EFI variable FdtFile.
> > > > > > > This will allow bootloaders to load a kernel specific device-tree.
> > > > > > > 
> > > > > > > The variable will not be exposed on ACPI based systems or if the
> > > > > > > environment variable fdtfile is not defined.
> > > > > > > 
> > > > > > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > > > > > > ---
> > > > > > > v2:
> > > > > > >            Use a unique GUID to enable future U-Boot independent
> > > > > > >            standardization.
> > > > > > >            Do not try to add the variable on ACPI based systems.
> > > > > > > ---
> > > > > > >     include/efi_loader.h       |  5 +++++
> > > > > > >     lib/efi_loader/efi_setup.c | 30 ++++++++++++++++++++++++++++++
> > > > > > >     2 files changed, 35 insertions(+)
> > > > > > 
> > > > > > I was too slow to reply to v1.
> > > > > > 
> > > > > > Does grub load the DT? I was assuming that U-Boot would pass it on?
> > > > > > What is the interface between U-Boot and grub?
> > > > > 
> > > > > The device-tree built into U-Boot is often out of date and not usable to
> > > > > boot current Linux. A single device-tree can be loaded by U-Boot from
> > > > > file and passed on as EFI configuration table. This device-tree may not
> > > > > be compatible with all kernel versions exposed by GRUB.
> > > > > 
> > > > > GRUB provides a devicetree command. It is disabled if you use secure
> > > > > boot. At least in Debian and Ubuntu GRUB invokes the
> > > > > EFI_DT_FIXUP_PROTOCOL exposed by U-Boot to run U-Boot's device-tree
> > > > > fix-ups after loading a device-tree.
> > > > > 
> > > > > Vendor scripts for GRUB like Ubuntu's /etc/grub.d/10_linux add
> > > > > devicetree commands to the boot options in grub.cfg.
> > > > 
> > > > Thanks. I wonder if you could document this somewhere? It seems like
> > > > there are a lot of options and it is quite complicated.
> > > > 
> > > > Back to the question, I suppose you are expecting grub to load the DT
> > > > using this filename? But why doesn't U-Boot load it instead? It seems
> > > > very convoluted.
> > > 
> > > A separate file of this name exists for every kernel version installed.
> > > The loaded dtb must match the kernel. U-Boot does not know what kernel
> > > version will be chosen in GRUB. And for a generic image GRUB does not
> > > what board it is on.
> > > 
> > > > 
> > > > Also, can we test this interface?
> > > 
> > > Neither the sandbox nor QEMU have environment variable fdtfile. And we
> > > don't create the EFI variable with ACPI as used on the sandbox.
> > 
> > I worry that this is creating another interface that some poor sod is
> > going to have to deal with in the future. Is this part of the EFI
> > standard?
> 
> No, the UEFI standard does not care much about device-trees. It only defines
> the GUID for the configuration table.
> 
> > 
> > We should really be using the compatible string to choose the
> > devicetree. Why are we using filenames at all? What is the
> > relationship between the compatible string and the filename? Is there
> > a lookup table, or should we create one?
> 
> There is no 1:1 relationship between compatible string and filename, e.g.
> the following arm64 device-trees use the same compatible string:
> 
> amd/amd-overdrive-rev-b0.dts
> amd/amd-overdrive-rev-b1.dts
> 
> amlogic/meson-axg-jethome-jethub-j110-rev-2.dts
> amlogic/meson-axg-jethome-jethub-j110-rev-3.dts
> 
> xilinx/zynqmp-zc1751-xm015-dc1.dts
> xilinx/zynqmp-zc1751-xm015-dc1.dts
> xilinx/zynqmp-zc1751-xm015-dc1.dts
> xilinx/zynqmp-zc1751-xm015-dc1.dts
> 
> > 
> > The correct way of doing this is implemented in U-Boot with
> > CONFIG_FIT_BEST_MATCH.
> 
> Why should we write complicated code to find a *possibly* matching file if
> we already know the filename that needs to be loaded?

I think that is the unfortunate key here. We can make a guess, or best
match, but it might not be right. And we need a reliable way to find and
use the correct tree. And the U-Boot portion of that may be "set the EFI
var if it's not already set" rather than "and now load it".

> > Can we mirror something like that in grub, etc?

I'm splitting this part out because no, having to have N projects write
the same bit of code, but with its own quirks and bugs doesn't sound
like the right direction. It's why today we have a few different sets of
logic to try and find / set the right device tree, so that whatever
follows after doesn't have to also do that, and get updated for every
new platform too.

And of course yes, ideally, boards would be manufactured with an up to
date and correct device tree on them.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

* Re: [PATCH v2 1/1] efi_loader: expose the device-tree file name
  2023-10-20 13:21         ` Simon Glass
  2023-10-20 13:55           ` Tom Rini
  2023-10-20 15:40           ` Heinrich Schuchardt
@ 2023-10-20 21:19           ` Simon Glass
  2 siblings, 0 replies; 26+ messages in thread
From: Simon Glass @ 2023-10-20 21:19 UTC (permalink / raw)
  To: Heinrich Schuchardt; +Cc: Ilias Apalodimas, U-Boot Mailing List, Doug Anderson

Trying again to add Doug as we chatted briefly about this yesterday.

On Fri, Oct 20, 2023, 07:21 Simon Glass <sjg@google.com> wrote:
>
> +Doug Anderson
>
> Hi Heinrich,
>
> On Thu, 19 Oct 2023 at 09:09, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
> >
> > On 19.10.23 15:55, Simon Glass wrote:
> > > Hi Heinrich,
> > >
> > > On Wed, 18 Oct 2023 at 02:15, Heinrich Schuchardt
> > > <heinrich.schuchardt@canonical.com> wrote:
> > >>
> > >> On 10/18/23 05:33, Simon Glass wrote:
> > >>> Hi Heinrich,
> > >>>
> > >>> On Tue, 17 Oct 2023 at 07:50, Heinrich Schuchardt
> > >>> <heinrich.schuchardt@canonical.com> wrote:
> > >>>>
> > >>>> Forward and backward compatibility of Linux kernel device-trees is
> > >>>> sometimes missing. One solution approach is to load a kernel specific
> > >>>> device-tree. This can either be done via a U-Boot scripts (like the one
> > >>>> generated by Debian package flash-kernel or by a boot loader like GRUB.
> > >>>> The boot loader approach currently requires to know the device-tree name
> > >>>> before first boot which makes it unusable for generic images.
> > >>>>
> > >>>> Expose the device-tree file name as EFI variable FdtFile.
> > >>>> This will allow bootloaders to load a kernel specific device-tree.
> > >>>>
> > >>>> The variable will not be exposed on ACPI based systems or if the
> > >>>> environment variable fdtfile is not defined.
> > >>>>
> > >>>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > >>>> ---
> > >>>> v2:
> > >>>>           Use a unique GUID to enable future U-Boot independent
> > >>>>           standardization.
> > >>>>           Do not try to add the variable on ACPI based systems.
> > >>>> ---
> > >>>>    include/efi_loader.h       |  5 +++++
> > >>>>    lib/efi_loader/efi_setup.c | 30 ++++++++++++++++++++++++++++++
> > >>>>    2 files changed, 35 insertions(+)
> > >>>
> > >>> I was too slow to reply to v1.
> > >>>
> > >>> Does grub load the DT? I was assuming that U-Boot would pass it on?
> > >>> What is the interface between U-Boot and grub?
> > >>
> > >> The device-tree built into U-Boot is often out of date and not usable to
> > >> boot current Linux. A single device-tree can be loaded by U-Boot from
> > >> file and passed on as EFI configuration table. This device-tree may not
> > >> be compatible with all kernel versions exposed by GRUB.
> > >>
> > >> GRUB provides a devicetree command. It is disabled if you use secure
> > >> boot. At least in Debian and Ubuntu GRUB invokes the
> > >> EFI_DT_FIXUP_PROTOCOL exposed by U-Boot to run U-Boot's device-tree
> > >> fix-ups after loading a device-tree.
> > >>
> > >> Vendor scripts for GRUB like Ubuntu's /etc/grub.d/10_linux add
> > >> devicetree commands to the boot options in grub.cfg.
> > >
> > > Thanks. I wonder if you could document this somewhere? It seems like
> > > there are a lot of options and it is quite complicated.
> > >
> > > Back to the question, I suppose you are expecting grub to load the DT
> > > using this filename? But why doesn't U-Boot load it instead? It seems
> > > very convoluted.
> >
> > A separate file of this name exists for every kernel version installed.
> > The loaded dtb must match the kernel. U-Boot does not know what kernel
> > version will be chosen in GRUB. And for a generic image GRUB does not
> > what board it is on.
> >
> > >
> > > Also, can we test this interface?
> >
> > Neither the sandbox nor QEMU have environment variable fdtfile. And we
> > don't create the EFI variable with ACPI as used on the sandbox.
>
> I worry that this is creating another interface that some poor sod is
> going to have to deal with in the future. Is this part of the EFI
> standard?
>
> We should really be using the compatible string to choose the
> devicetree. Why are we using filenames at all? What is the
> relationship between the compatible string and the filename? Is there
> a lookup table, or should we create one?
>
> The correct way of doing this is implemented in U-Boot with
> CONFIG_FIT_BEST_MATCH. Can we mirror something like that in grub, etc?
>
> Regards,
> Simon

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

* Re: [PATCH v2 1/1] efi_loader: expose the device-tree file name
  2023-10-20 16:24             ` Tom Rini
@ 2023-10-21 15:42               ` Simon Glass
  2023-10-22  4:53                 ` Heinrich Schuchardt
  0 siblings, 1 reply; 26+ messages in thread
From: Simon Glass @ 2023-10-21 15:42 UTC (permalink / raw)
  To: Tom Rini; +Cc: Heinrich Schuchardt, Ilias Apalodimas, u-boot, Doug Anderson

Hi Tom,

On Fri, 20 Oct 2023 at 09:24, Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Oct 20, 2023 at 05:40:03PM +0200, Heinrich Schuchardt wrote:
> > On 20.10.23 15:21, Simon Glass wrote:
> > > +Doug Anderson
> > >
> > > Hi Heinrich,
> > >
> > > On Thu, 19 Oct 2023 at 09:09, Heinrich Schuchardt
> > > <heinrich.schuchardt@canonical.com> wrote:
> > > >
> > > > On 19.10.23 15:55, Simon Glass wrote:
> > > > > Hi Heinrich,
> > > > >
> > > > > On Wed, 18 Oct 2023 at 02:15, Heinrich Schuchardt
> > > > > <heinrich.schuchardt@canonical.com> wrote:
> > > > > >
> > > > > > On 10/18/23 05:33, Simon Glass wrote:
> > > > > > > Hi Heinrich,
> > > > > > >
> > > > > > > On Tue, 17 Oct 2023 at 07:50, Heinrich Schuchardt
> > > > > > > <heinrich.schuchardt@canonical.com> wrote:
> > > > > > > >
> > > > > > > > Forward and backward compatibility of Linux kernel device-trees is
> > > > > > > > sometimes missing. One solution approach is to load a kernel specific
> > > > > > > > device-tree. This can either be done via a U-Boot scripts (like the one
> > > > > > > > generated by Debian package flash-kernel or by a boot loader like GRUB.
> > > > > > > > The boot loader approach currently requires to know the device-tree name
> > > > > > > > before first boot which makes it unusable for generic images.
> > > > > > > >
> > > > > > > > Expose the device-tree file name as EFI variable FdtFile.
> > > > > > > > This will allow bootloaders to load a kernel specific device-tree.
> > > > > > > >
> > > > > > > > The variable will not be exposed on ACPI based systems or if the
> > > > > > > > environment variable fdtfile is not defined.
> > > > > > > >
> > > > > > > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > > > > > > > ---
> > > > > > > > v2:
> > > > > > > >            Use a unique GUID to enable future U-Boot independent
> > > > > > > >            standardization.
> > > > > > > >            Do not try to add the variable on ACPI based systems.
> > > > > > > > ---
> > > > > > > >     include/efi_loader.h       |  5 +++++
> > > > > > > >     lib/efi_loader/efi_setup.c | 30 ++++++++++++++++++++++++++++++
> > > > > > > >     2 files changed, 35 insertions(+)
> > > > > > >
> > > > > > > I was too slow to reply to v1.
> > > > > > >
> > > > > > > Does grub load the DT? I was assuming that U-Boot would pass it on?
> > > > > > > What is the interface between U-Boot and grub?
> > > > > >
> > > > > > The device-tree built into U-Boot is often out of date and not usable to
> > > > > > boot current Linux. A single device-tree can be loaded by U-Boot from
> > > > > > file and passed on as EFI configuration table. This device-tree may not
> > > > > > be compatible with all kernel versions exposed by GRUB.
> > > > > >
> > > > > > GRUB provides a devicetree command. It is disabled if you use secure
> > > > > > boot. At least in Debian and Ubuntu GRUB invokes the
> > > > > > EFI_DT_FIXUP_PROTOCOL exposed by U-Boot to run U-Boot's device-tree
> > > > > > fix-ups after loading a device-tree.
> > > > > >
> > > > > > Vendor scripts for GRUB like Ubuntu's /etc/grub.d/10_linux add
> > > > > > devicetree commands to the boot options in grub.cfg.
> > > > >
> > > > > Thanks. I wonder if you could document this somewhere? It seems like
> > > > > there are a lot of options and it is quite complicated.
> > > > >
> > > > > Back to the question, I suppose you are expecting grub to load the DT
> > > > > using this filename? But why doesn't U-Boot load it instead? It seems
> > > > > very convoluted.
> > > >
> > > > A separate file of this name exists for every kernel version installed.
> > > > The loaded dtb must match the kernel. U-Boot does not know what kernel
> > > > version will be chosen in GRUB. And for a generic image GRUB does not
> > > > what board it is on.
> > > >
> > > > >
> > > > > Also, can we test this interface?
> > > >
> > > > Neither the sandbox nor QEMU have environment variable fdtfile. And we
> > > > don't create the EFI variable with ACPI as used on the sandbox.
> > >
> > > I worry that this is creating another interface that some poor sod is
> > > going to have to deal with in the future. Is this part of the EFI
> > > standard?
> >
> > No, the UEFI standard does not care much about device-trees. It only defines
> > the GUID for the configuration table.
> >
> > >
> > > We should really be using the compatible string to choose the
> > > devicetree. Why are we using filenames at all? What is the
> > > relationship between the compatible string and the filename? Is there
> > > a lookup table, or should we create one?
> >
> > There is no 1:1 relationship between compatible string and filename, e.g.
> > the following arm64 device-trees use the same compatible string:
> >
> > amd/amd-overdrive-rev-b0.dts
> > amd/amd-overdrive-rev-b1.dts
> >
> > amlogic/meson-axg-jethome-jethub-j110-rev-2.dts
> > amlogic/meson-axg-jethome-jethub-j110-rev-3.dts
> >
> > xilinx/zynqmp-zc1751-xm015-dc1.dts
> > xilinx/zynqmp-zc1751-xm015-dc1.dts
> > xilinx/zynqmp-zc1751-xm015-dc1.dts
> > xilinx/zynqmp-zc1751-xm015-dc1.dts
> >
> > >
> > > The correct way of doing this is implemented in U-Boot with
> > > CONFIG_FIT_BEST_MATCH.
> >
> > Why should we write complicated code to find a *possibly* matching file if
> > we already know the filename that needs to be loaded?
>
> I think that is the unfortunate key here. We can make a guess, or best
> match, but it might not be right. And we need a reliable way to find and
> use the correct tree. And the U-Boot portion of that may be "set the EFI
> var if it's not already set" rather than "and now load it".

That is not my understanding of how it works. The compatible string is
how Linux knows what the hardware is...if it doesn't match, then
things are going to go wrong. It is also how U-Boot works, e.g. with
FIT. I don't believe this is a 'guess'. The compatible string is used
programmatically and must be correct.

fdt_node_check_compatible() does most of the work...then you need to
check which FDT has the most specific match (i.e. latest in the string
list). That handles things like board revisions, variants, etc.

My concern is about adding a feature when there is already a defined
spec and mechanism for this to work. What happens when we load the
file and the compatible is wrong?

At best, I see the filename as a hint.

[Perhaps this is the wrong time to ask, but why are kernels +DT not
shipped in FIT on ARM?]

>
> > > Can we mirror something like that in grub, etc?
>
> I'm splitting this part out because no, having to have N projects write
> the same bit of code, but with its own quirks and bugs doesn't sound
> like the right direction. It's why today we have a few different sets of
> logic to try and find / set the right device tree, so that whatever
> follows after doesn't have to also do that, and get updated for every
> new platform too.
>
> And of course yes, ideally, boards would be manufactured with an up to
> date and correct device tree on them.

OK, well I perhaps have the wrong end of the stick here.

As I learn more about how distros boot I see a great need for
information about what is actually being booted. For example, the
current 'bootflow menu' mostly shows useless information when EFI is
used to boot, since it doesn't know what the things are. We have to
jump to grub (or whatever) to find out. Grub knows because the OS set
up a grub menu.

Really we need to see this standard boot thing to the end. Each OS
should provide information from /etc/lsb_release as well as info from
menu.grub (sorry can't remember the right name), so that we know what
can be booted. Having to jump to an OS-specific bootloader to even be
able to show a menu is a pretty poor show.

Anyway I think I understand why this variable is needed. Please add
some documentation on all this.

My main concern is whether this sort of thing is going to make it even
harder to boot in a simple, standard manner.

Re Heinrich's comment:

> There is no 1:1 relationship between compatible string and filename,
> e.g. the following arm64 device-trees use the same compatible string:

> > amd/amd-overdrive-rev-b0.dts
> > amd/amd-overdrive-rev-b1.dts

That just seems like a bug to me. The compatible should include the
board rev as it does for xlnx,zynqmp-zcu102-rev1.1 etc.

Regards,
Simon

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

* Re: [PATCH v2 1/1] efi_loader: expose the device-tree file name
  2023-10-21 15:42               ` Simon Glass
@ 2023-10-22  4:53                 ` Heinrich Schuchardt
  2023-10-23  7:08                   ` Simon Glass
  0 siblings, 1 reply; 26+ messages in thread
From: Heinrich Schuchardt @ 2023-10-22  4:53 UTC (permalink / raw)
  To: Simon Glass; +Cc: Ilias Apalodimas, u-boot, Doug Anderson, Tom Rini

On 10/21/23 17:42, Simon Glass wrote:
> Hi Tom,
> 
> On Fri, 20 Oct 2023 at 09:24, Tom Rini <trini@konsulko.com> wrote:
>>
>> On Fri, Oct 20, 2023 at 05:40:03PM +0200, Heinrich Schuchardt wrote:
>>> On 20.10.23 15:21, Simon Glass wrote:
>>>> +Doug Anderson
>>>>
>>>> Hi Heinrich,
>>>>
>>>> On Thu, 19 Oct 2023 at 09:09, Heinrich Schuchardt
>>>> <heinrich.schuchardt@canonical.com> wrote:
>>>>>
>>>>> On 19.10.23 15:55, Simon Glass wrote:
>>>>>> Hi Heinrich,
>>>>>>
>>>>>> On Wed, 18 Oct 2023 at 02:15, Heinrich Schuchardt
>>>>>> <heinrich.schuchardt@canonical.com> wrote:
>>>>>>>
>>>>>>> On 10/18/23 05:33, Simon Glass wrote:
>>>>>>>> Hi Heinrich,
>>>>>>>>
>>>>>>>> On Tue, 17 Oct 2023 at 07:50, Heinrich Schuchardt
>>>>>>>> <heinrich.schuchardt@canonical.com> wrote:
>>>>>>>>>
>>>>>>>>> Forward and backward compatibility of Linux kernel device-trees is
>>>>>>>>> sometimes missing. One solution approach is to load a kernel specific
>>>>>>>>> device-tree. This can either be done via a U-Boot scripts (like the one
>>>>>>>>> generated by Debian package flash-kernel or by a boot loader like GRUB.
>>>>>>>>> The boot loader approach currently requires to know the device-tree name
>>>>>>>>> before first boot which makes it unusable for generic images.
>>>>>>>>>
>>>>>>>>> Expose the device-tree file name as EFI variable FdtFile.
>>>>>>>>> This will allow bootloaders to load a kernel specific device-tree.
>>>>>>>>>
>>>>>>>>> The variable will not be exposed on ACPI based systems or if the
>>>>>>>>> environment variable fdtfile is not defined.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>>>>>>>>> ---
>>>>>>>>> v2:
>>>>>>>>>             Use a unique GUID to enable future U-Boot independent
>>>>>>>>>             standardization.
>>>>>>>>>             Do not try to add the variable on ACPI based systems.
>>>>>>>>> ---
>>>>>>>>>      include/efi_loader.h       |  5 +++++
>>>>>>>>>      lib/efi_loader/efi_setup.c | 30 ++++++++++++++++++++++++++++++
>>>>>>>>>      2 files changed, 35 insertions(+)
>>>>>>>>
>>>>>>>> I was too slow to reply to v1.
>>>>>>>>
>>>>>>>> Does grub load the DT? I was assuming that U-Boot would pass it on?
>>>>>>>> What is the interface between U-Boot and grub?
>>>>>>>
>>>>>>> The device-tree built into U-Boot is often out of date and not usable to
>>>>>>> boot current Linux. A single device-tree can be loaded by U-Boot from
>>>>>>> file and passed on as EFI configuration table. This device-tree may not
>>>>>>> be compatible with all kernel versions exposed by GRUB.
>>>>>>>
>>>>>>> GRUB provides a devicetree command. It is disabled if you use secure
>>>>>>> boot. At least in Debian and Ubuntu GRUB invokes the
>>>>>>> EFI_DT_FIXUP_PROTOCOL exposed by U-Boot to run U-Boot's device-tree
>>>>>>> fix-ups after loading a device-tree.
>>>>>>>
>>>>>>> Vendor scripts for GRUB like Ubuntu's /etc/grub.d/10_linux add
>>>>>>> devicetree commands to the boot options in grub.cfg.
>>>>>>
>>>>>> Thanks. I wonder if you could document this somewhere? It seems like
>>>>>> there are a lot of options and it is quite complicated.
>>>>>>
>>>>>> Back to the question, I suppose you are expecting grub to load the DT
>>>>>> using this filename? But why doesn't U-Boot load it instead? It seems
>>>>>> very convoluted.
>>>>>
>>>>> A separate file of this name exists for every kernel version installed.
>>>>> The loaded dtb must match the kernel. U-Boot does not know what kernel
>>>>> version will be chosen in GRUB. And for a generic image GRUB does not
>>>>> what board it is on.
>>>>>
>>>>>>
>>>>>> Also, can we test this interface?
>>>>>
>>>>> Neither the sandbox nor QEMU have environment variable fdtfile. And we
>>>>> don't create the EFI variable with ACPI as used on the sandbox.
>>>>
>>>> I worry that this is creating another interface that some poor sod is
>>>> going to have to deal with in the future. Is this part of the EFI
>>>> standard?
>>>
>>> No, the UEFI standard does not care much about device-trees. It only defines
>>> the GUID for the configuration table.
>>>
>>>>
>>>> We should really be using the compatible string to choose the
>>>> devicetree. Why are we using filenames at all? What is the
>>>> relationship between the compatible string and the filename? Is there
>>>> a lookup table, or should we create one?
>>>
>>> There is no 1:1 relationship between compatible string and filename, e.g.
>>> the following arm64 device-trees use the same compatible string:
>>>
>>> amd/amd-overdrive-rev-b0.dts
>>> amd/amd-overdrive-rev-b1.dts
>>>
>>> amlogic/meson-axg-jethome-jethub-j110-rev-2.dts
>>> amlogic/meson-axg-jethome-jethub-j110-rev-3.dts
>>>
>>> xilinx/zynqmp-zc1751-xm015-dc1.dts
>>> xilinx/zynqmp-zc1751-xm015-dc1.dts
>>> xilinx/zynqmp-zc1751-xm015-dc1.dts
>>> xilinx/zynqmp-zc1751-xm015-dc1.dts
>>>
>>>>
>>>> The correct way of doing this is implemented in U-Boot with
>>>> CONFIG_FIT_BEST_MATCH.
>>>
>>> Why should we write complicated code to find a *possibly* matching file if
>>> we already know the filename that needs to be loaded?
>>
>> I think that is the unfortunate key here. We can make a guess, or best
>> match, but it might not be right. And we need a reliable way to find and
>> use the correct tree. And the U-Boot portion of that may be "set the EFI
>> var if it's not already set" rather than "and now load it".
> 
> That is not my understanding of how it works. The compatible string is
> how Linux knows what the hardware is...if it doesn't match, then
> things are going to go wrong. It is also how U-Boot works, e.g. with
> FIT. I don't believe this is a 'guess'. The compatible string is used
> programmatically and must be correct.

It is not how U-Boot works. In distroboot U-Boot uses environment 
variable fdtfile to load the correct device-tree.

Your suggestion would mean that distroboot or GRUB would have to sift 
through literally hundreds of device-tree files which would considerably 
increase the boot time. The process might end up with multiple dtb files 
that match.

> 
> fdt_node_check_compatible() does most of the work...then you need to
> check which FDT has the most specific match (i.e. latest in the string
> list). That handles things like board revisions, variants, etc.
> 
> My concern is about adding a feature when there is already a defined
> spec and mechanism for this to work. What happens when we load the
> file and the compatible is wrong?
> 
> At best, I see the filename as a hint.
> 
> [Perhaps this is the wrong time to ask, but why are kernels +DT not
> shipped in FIT on ARM?]

FIT is U-Boot specific. For Linux distributions it is easier to use a 
firmware agnostic method of booting.

>>
>>>> Can we mirror something like that in grub, etc?
>>
>> I'm splitting this part out because no, having to have N projects write
>> the same bit of code, but with its own quirks and bugs doesn't sound
>> like the right direction. It's why today we have a few different sets of
>> logic to try and find / set the right device tree, so that whatever
>> follows after doesn't have to also do that, and get updated for every
>> new platform too.
>>
>> And of course yes, ideally, boards would be manufactured with an up to
>> date and correct device tree on them.
> 
> OK, well I perhaps have the wrong end of the stick here.
> 
> As I learn more about how distros boot I see a great need for
> information about what is actually being booted. For example, the
> current 'bootflow menu' mostly shows useless information when EFI is
> used to boot, since it doesn't know what the things are. We have to
> jump to grub (or whatever) to find out. Grub knows because the OS set
> up a grub menu.

The menu should show the EFI boot options. Then you can decide which of 
the multiple operating systems on your device you want to boot.

The bottleneck is secure boot. As Red Hat did not want Microsoft to sign 
every single kernel they came up with shim. Now we can only have one EFI 
boot option per operating system. It is only after shim that you can 
load one of multiple kernels.

> 
> Really we need to see this standard boot thing to the end. Each OS
> should provide information from /etc/lsb_release as well as info from
> menu.grub (sorry can't remember the right name), so that we know what

/boot/grub/grub.cfg

> can be booted. Having to jump to an OS-specific bootloader to even be
> able to show a menu is a pretty poor show.

With secure-boot you still would need to load the OS specific shim. 
Further you would need a means to communicate your decision to whatever 
program loads the kernel.

> 
> Anyway I think I understand why this variable is needed. Please add
> some documentation on all this.

Yes I should add a paragraph on the variable.

> 
> My main concern is whether this sort of thing is going to make it even
> harder to boot in a simple, standard manner.
> 
> Re Heinrich's comment:
> 
>> There is no 1:1 relationship between compatible string and filename,
>> e.g. the following arm64 device-trees use the same compatible string:
> 
>>> amd/amd-overdrive-rev-b0.dts
>>> amd/amd-overdrive-rev-b1.dts
> 
> That just seems like a bug to me. The compatible should include the
> board rev as it does for xlnx,zynqmp-zcu102-rev1.1 etc.

Yes, here it is a bug.

In theory you could have different device-trees for different device 
configurations (e.g. USB OTG vs USB host). In this case having the same 
compatible string would be correct.

Best regards

Heinrich

> 
> Regards,
> Simon

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

* Re: [PATCH v2 1/1] efi_loader: expose the device-tree file name
  2023-10-22  4:53                 ` Heinrich Schuchardt
@ 2023-10-23  7:08                   ` Simon Glass
  2023-10-23  8:10                     ` Heinrich Schuchardt
  2023-10-23 15:37                     ` Mark Kettenis
  0 siblings, 2 replies; 26+ messages in thread
From: Simon Glass @ 2023-10-23  7:08 UTC (permalink / raw)
  To: Heinrich Schuchardt; +Cc: Ilias Apalodimas, u-boot, Doug Anderson, Tom Rini

Hi Heinrich,

On Sat, 21 Oct 2023 at 21:53, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> On 10/21/23 17:42, Simon Glass wrote:
> > Hi Tom,
> >
> > On Fri, 20 Oct 2023 at 09:24, Tom Rini <trini@konsulko.com> wrote:
> >>
> >> On Fri, Oct 20, 2023 at 05:40:03PM +0200, Heinrich Schuchardt wrote:
> >>> On 20.10.23 15:21, Simon Glass wrote:
> >>>> +Doug Anderson
> >>>>
> >>>> Hi Heinrich,
> >>>>
> >>>> On Thu, 19 Oct 2023 at 09:09, Heinrich Schuchardt
> >>>> <heinrich.schuchardt@canonical.com> wrote:
> >>>>>
> >>>>> On 19.10.23 15:55, Simon Glass wrote:
> >>>>>> Hi Heinrich,
> >>>>>>
> >>>>>> On Wed, 18 Oct 2023 at 02:15, Heinrich Schuchardt
> >>>>>> <heinrich.schuchardt@canonical.com> wrote:
> >>>>>>>
> >>>>>>> On 10/18/23 05:33, Simon Glass wrote:
> >>>>>>>> Hi Heinrich,
> >>>>>>>>
> >>>>>>>> On Tue, 17 Oct 2023 at 07:50, Heinrich Schuchardt
> >>>>>>>> <heinrich.schuchardt@canonical.com> wrote:
> >>>>>>>>>
> >>>>>>>>> Forward and backward compatibility of Linux kernel device-trees is
> >>>>>>>>> sometimes missing. One solution approach is to load a kernel specific
> >>>>>>>>> device-tree. This can either be done via a U-Boot scripts (like the one
> >>>>>>>>> generated by Debian package flash-kernel or by a boot loader like GRUB.
> >>>>>>>>> The boot loader approach currently requires to know the device-tree name
> >>>>>>>>> before first boot which makes it unusable for generic images.
> >>>>>>>>>
> >>>>>>>>> Expose the device-tree file name as EFI variable FdtFile.
> >>>>>>>>> This will allow bootloaders to load a kernel specific device-tree.
> >>>>>>>>>
> >>>>>>>>> The variable will not be exposed on ACPI based systems or if the
> >>>>>>>>> environment variable fdtfile is not defined.
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> >>>>>>>>> ---
> >>>>>>>>> v2:
> >>>>>>>>>             Use a unique GUID to enable future U-Boot independent
> >>>>>>>>>             standardization.
> >>>>>>>>>             Do not try to add the variable on ACPI based systems.
> >>>>>>>>> ---
> >>>>>>>>>      include/efi_loader.h       |  5 +++++
> >>>>>>>>>      lib/efi_loader/efi_setup.c | 30 ++++++++++++++++++++++++++++++
> >>>>>>>>>      2 files changed, 35 insertions(+)
> >>>>>>>>
> >>>>>>>> I was too slow to reply to v1.
> >>>>>>>>
> >>>>>>>> Does grub load the DT? I was assuming that U-Boot would pass it on?
> >>>>>>>> What is the interface between U-Boot and grub?
> >>>>>>>
> >>>>>>> The device-tree built into U-Boot is often out of date and not usable to
> >>>>>>> boot current Linux. A single device-tree can be loaded by U-Boot from
> >>>>>>> file and passed on as EFI configuration table. This device-tree may not
> >>>>>>> be compatible with all kernel versions exposed by GRUB.
> >>>>>>>
> >>>>>>> GRUB provides a devicetree command. It is disabled if you use secure
> >>>>>>> boot. At least in Debian and Ubuntu GRUB invokes the
> >>>>>>> EFI_DT_FIXUP_PROTOCOL exposed by U-Boot to run U-Boot's device-tree
> >>>>>>> fix-ups after loading a device-tree.
> >>>>>>>
> >>>>>>> Vendor scripts for GRUB like Ubuntu's /etc/grub.d/10_linux add
> >>>>>>> devicetree commands to the boot options in grub.cfg.
> >>>>>>
> >>>>>> Thanks. I wonder if you could document this somewhere? It seems like
> >>>>>> there are a lot of options and it is quite complicated.
> >>>>>>
> >>>>>> Back to the question, I suppose you are expecting grub to load the DT
> >>>>>> using this filename? But why doesn't U-Boot load it instead? It seems
> >>>>>> very convoluted.
> >>>>>
> >>>>> A separate file of this name exists for every kernel version installed.
> >>>>> The loaded dtb must match the kernel. U-Boot does not know what kernel
> >>>>> version will be chosen in GRUB. And for a generic image GRUB does not
> >>>>> what board it is on.
> >>>>>
> >>>>>>
> >>>>>> Also, can we test this interface?
> >>>>>
> >>>>> Neither the sandbox nor QEMU have environment variable fdtfile. And we
> >>>>> don't create the EFI variable with ACPI as used on the sandbox.
> >>>>
> >>>> I worry that this is creating another interface that some poor sod is
> >>>> going to have to deal with in the future. Is this part of the EFI
> >>>> standard?
> >>>
> >>> No, the UEFI standard does not care much about device-trees. It only defines
> >>> the GUID for the configuration table.
> >>>
> >>>>
> >>>> We should really be using the compatible string to choose the
> >>>> devicetree. Why are we using filenames at all? What is the
> >>>> relationship between the compatible string and the filename? Is there
> >>>> a lookup table, or should we create one?
> >>>
> >>> There is no 1:1 relationship between compatible string and filename, e.g.
> >>> the following arm64 device-trees use the same compatible string:
> >>>
> >>> amd/amd-overdrive-rev-b0.dts
> >>> amd/amd-overdrive-rev-b1.dts
> >>>
> >>> amlogic/meson-axg-jethome-jethub-j110-rev-2.dts
> >>> amlogic/meson-axg-jethome-jethub-j110-rev-3.dts
> >>>
> >>> xilinx/zynqmp-zc1751-xm015-dc1.dts
> >>> xilinx/zynqmp-zc1751-xm015-dc1.dts
> >>> xilinx/zynqmp-zc1751-xm015-dc1.dts
> >>> xilinx/zynqmp-zc1751-xm015-dc1.dts
> >>>
> >>>>
> >>>> The correct way of doing this is implemented in U-Boot with
> >>>> CONFIG_FIT_BEST_MATCH.
> >>>
> >>> Why should we write complicated code to find a *possibly* matching file if
> >>> we already know the filename that needs to be loaded?
> >>
> >> I think that is the unfortunate key here. We can make a guess, or best
> >> match, but it might not be right. And we need a reliable way to find and
> >> use the correct tree. And the U-Boot portion of that may be "set the EFI
> >> var if it's not already set" rather than "and now load it".
> >
> > That is not my understanding of how it works. The compatible string is
> > how Linux knows what the hardware is...if it doesn't match, then
> > things are going to go wrong. It is also how U-Boot works, e.g. with
> > FIT. I don't believe this is a 'guess'. The compatible string is used
> > programmatically and must be correct.
>
> It is not how U-Boot works. In distroboot U-Boot uses environment
> variable fdtfile to load the correct device-tree.

Yes, that is my point.

>
> Your suggestion would mean that distroboot or GRUB would have to sift
> through literally hundreds of device-tree files which would considerably
> increase the boot time. The process might end up with multiple dtb files
> that match.

Have you tried it? Just for interest I tried it on the latest Fedora on rpi_4:

$ python scripts/make_fit.py -f test.fit /media/sda2/dtb
Fit size 6a0400 / 6.6 MB, 485 files, uncompressed 16.8 MB

On an rpi_3 with a terribly slow uSD card:
U-Boot> load mmc 0 10000 teste.fit
6910708 bytes read in 601 ms (11 MiB/s)

U-Boot> bootm start 10000
## Loading kernel from FIT Image at 00010000 ...
   Using 'conf-272' configuration
   Trying 'kernel' kernel subimage
   Verifying Hash Integrity ... OK
## Loading fdt from FIT Image at 00010000 ...
   Using 'conf-272' configuration
   Trying 'fdt-272' fdt subimage
   Verifying Hash Integrity ... OK
   Uncompressing Flat Device Tree
   Booting using the fdt blob at 0x3dbf8010
Working FDT set to 3dbf8010

Then I timed how long it takes to get the right FDT:
                 3,466  fdt_best

So that is 3.5ms. Re the load time, it is easy enough to use an
external FIT and just load the metadata part, which cuts the load time
down to 35ms. So the overall cost is about 40ms. It actually takes the
rpi4 longer than that to write the messages to the display (although
I'm not sure why).

There cannot be multiple DTs that match...unless you mean that there
might be different board revisions. But even then, the compatible
string is how they are disambiguated.

So this process is already defined and correct.

> >
> > fdt_node_check_compatible() does most of the work...then you need to
> > check which FDT has the most specific match (i.e. latest in the string
> > list). That handles things like board revisions, variants, etc.
> >
> > My concern is about adding a feature when there is already a defined
> > spec and mechanism for this to work. What happens when we load the
> > file and the compatible is wrong?
> >
> > At best, I see the filename as a hint.
> >
> > [Perhaps this is the wrong time to ask, but why are kernels +DT not
> > shipped in FIT on ARM?]
>
> FIT is U-Boot specific. For Linux distributions it is easier to use a
> firmware agnostic method of booting.

I'd like to suggest that distros use both. Then U-Boot can work as it
was designed and we can avoid these work-arounds.

FIT is actually implemented in various other bootloaders. In fact
perhaps grub is the only one that doesn't? I can't think of any
others.

>
> >>
> >>>> Can we mirror something like that in grub, etc?
> >>
> >> I'm splitting this part out because no, having to have N projects write
> >> the same bit of code, but with its own quirks and bugs doesn't sound
> >> like the right direction. It's why today we have a few different sets of
> >> logic to try and find / set the right device tree, so that whatever
> >> follows after doesn't have to also do that, and get updated for every
> >> new platform too.
> >>
> >> And of course yes, ideally, boards would be manufactured with an up to
> >> date and correct device tree on them.
> >
> > OK, well I perhaps have the wrong end of the stick here.
> >
> > As I learn more about how distros boot I see a great need for
> > information about what is actually being booted. For example, the
> > current 'bootflow menu' mostly shows useless information when EFI is
> > used to boot, since it doesn't know what the things are. We have to
> > jump to grub (or whatever) to find out. Grub knows because the OS set
> > up a grub menu.
>
> The menu should show the EFI boot options. Then you can decide which of
> the multiple operating systems on your device you want to boot.
>
> The bottleneck is secure boot. As Red Hat did not want Microsoft to sign
> every single kernel they came up with shim. Now we can only have one EFI
> boot option per operating system. It is only after shim that you can
> load one of multiple kernels.

OK, but I wonder if each .efi could have a description of at least who
wrote it? It would be very useful to tell if it is Ubuntu or Fedora
before booting it.

>
> >
> > Really we need to see this standard boot thing to the end. Each OS
> > should provide information from /etc/lsb_release as well as info from
> > menu.grub (sorry can't remember the right name), so that we know what
>
> /boot/grub/grub.cfg

That is actually a script in a grub format. I think we need something
more generic. Or are you suggesting that U-Boot should parse grub.cfg
?

>
> > can be booted. Having to jump to an OS-specific bootloader to even be
> > able to show a menu is a pretty poor show.
>
> With secure-boot you still would need to load the OS specific shim.
> Further you would need a means to communicate your decision to whatever
> program loads the kernel.

Yes that's right, similar to how you are communicating the FDT here, I suppose.

>
> >
> > Anyway I think I understand why this variable is needed. Please add
> > some documentation on all this.
>
> Yes I should add a paragraph on the variable.
>
> >
> > My main concern is whether this sort of thing is going to make it even
> > harder to boot in a simple, standard manner.
> >
> > Re Heinrich's comment:
> >
> >> There is no 1:1 relationship between compatible string and filename,
> >> e.g. the following arm64 device-trees use the same compatible string:
> >
> >>> amd/amd-overdrive-rev-b0.dts
> >>> amd/amd-overdrive-rev-b1.dts
> >
> > That just seems like a bug to me. The compatible should include the
> > board rev as it does for xlnx,zynqmp-zcu102-rev1.1 etc.
>
> Yes, here it is a bug.
>
> In theory you could have different device-trees for different device
> configurations (e.g. USB OTG vs USB host). In this case having the same
> compatible string would be correct.

That is the sort of thing that really should not be allowed. The FIT
mechanism keeps people honest.

Regards,
Simon

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

* Re: [PATCH v2 1/1] efi_loader: expose the device-tree file name
  2023-10-23  7:08                   ` Simon Glass
@ 2023-10-23  8:10                     ` Heinrich Schuchardt
  2023-10-23 15:37                     ` Mark Kettenis
  1 sibling, 0 replies; 26+ messages in thread
From: Heinrich Schuchardt @ 2023-10-23  8:10 UTC (permalink / raw)
  To: Simon Glass; +Cc: Ilias Apalodimas, u-boot, Doug Anderson, Tom Rini

On 10/23/23 09:08, Simon Glass wrote:
> Hi Heinrich,
> 
> On Sat, 21 Oct 2023 at 21:53, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
>>
>> On 10/21/23 17:42, Simon Glass wrote:
>>> Hi Tom,
>>>
>>> On Fri, 20 Oct 2023 at 09:24, Tom Rini <trini@konsulko.com> wrote:
>>>>
>>>> On Fri, Oct 20, 2023 at 05:40:03PM +0200, Heinrich Schuchardt wrote:
>>>>> On 20.10.23 15:21, Simon Glass wrote:
>>>>>> +Doug Anderson
>>>>>>
>>>>>> Hi Heinrich,
>>>>>>
>>>>>> On Thu, 19 Oct 2023 at 09:09, Heinrich Schuchardt
>>>>>> <heinrich.schuchardt@canonical.com> wrote:
>>>>>>>
>>>>>>> On 19.10.23 15:55, Simon Glass wrote:
>>>>>>>> Hi Heinrich,
>>>>>>>>
>>>>>>>> On Wed, 18 Oct 2023 at 02:15, Heinrich Schuchardt
>>>>>>>> <heinrich.schuchardt@canonical.com> wrote:
>>>>>>>>>
>>>>>>>>> On 10/18/23 05:33, Simon Glass wrote:
>>>>>>>>>> Hi Heinrich,
>>>>>>>>>>
>>>>>>>>>> On Tue, 17 Oct 2023 at 07:50, Heinrich Schuchardt
>>>>>>>>>> <heinrich.schuchardt@canonical.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>> Forward and backward compatibility of Linux kernel device-trees is
>>>>>>>>>>> sometimes missing. One solution approach is to load a kernel specific
>>>>>>>>>>> device-tree. This can either be done via a U-Boot scripts (like the one
>>>>>>>>>>> generated by Debian package flash-kernel or by a boot loader like GRUB.
>>>>>>>>>>> The boot loader approach currently requires to know the device-tree name
>>>>>>>>>>> before first boot which makes it unusable for generic images.
>>>>>>>>>>>
>>>>>>>>>>> Expose the device-tree file name as EFI variable FdtFile.
>>>>>>>>>>> This will allow bootloaders to load a kernel specific device-tree.
>>>>>>>>>>>
>>>>>>>>>>> The variable will not be exposed on ACPI based systems or if the
>>>>>>>>>>> environment variable fdtfile is not defined.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>>>>>>>>>>> ---
>>>>>>>>>>> v2:
>>>>>>>>>>>              Use a unique GUID to enable future U-Boot independent
>>>>>>>>>>>              standardization.
>>>>>>>>>>>              Do not try to add the variable on ACPI based systems.
>>>>>>>>>>> ---
>>>>>>>>>>>       include/efi_loader.h       |  5 +++++
>>>>>>>>>>>       lib/efi_loader/efi_setup.c | 30 ++++++++++++++++++++++++++++++
>>>>>>>>>>>       2 files changed, 35 insertions(+)
>>>>>>>>>>
>>>>>>>>>> I was too slow to reply to v1.
>>>>>>>>>>
>>>>>>>>>> Does grub load the DT? I was assuming that U-Boot would pass it on?
>>>>>>>>>> What is the interface between U-Boot and grub?
>>>>>>>>>
>>>>>>>>> The device-tree built into U-Boot is often out of date and not usable to
>>>>>>>>> boot current Linux. A single device-tree can be loaded by U-Boot from
>>>>>>>>> file and passed on as EFI configuration table. This device-tree may not
>>>>>>>>> be compatible with all kernel versions exposed by GRUB.
>>>>>>>>>
>>>>>>>>> GRUB provides a devicetree command. It is disabled if you use secure
>>>>>>>>> boot. At least in Debian and Ubuntu GRUB invokes the
>>>>>>>>> EFI_DT_FIXUP_PROTOCOL exposed by U-Boot to run U-Boot's device-tree
>>>>>>>>> fix-ups after loading a device-tree.
>>>>>>>>>
>>>>>>>>> Vendor scripts for GRUB like Ubuntu's /etc/grub.d/10_linux add
>>>>>>>>> devicetree commands to the boot options in grub.cfg.
>>>>>>>>
>>>>>>>> Thanks. I wonder if you could document this somewhere? It seems like
>>>>>>>> there are a lot of options and it is quite complicated.
>>>>>>>>
>>>>>>>> Back to the question, I suppose you are expecting grub to load the DT
>>>>>>>> using this filename? But why doesn't U-Boot load it instead? It seems
>>>>>>>> very convoluted.
>>>>>>>
>>>>>>> A separate file of this name exists for every kernel version installed.
>>>>>>> The loaded dtb must match the kernel. U-Boot does not know what kernel
>>>>>>> version will be chosen in GRUB. And for a generic image GRUB does not
>>>>>>> what board it is on.
>>>>>>>
>>>>>>>>
>>>>>>>> Also, can we test this interface?
>>>>>>>
>>>>>>> Neither the sandbox nor QEMU have environment variable fdtfile. And we
>>>>>>> don't create the EFI variable with ACPI as used on the sandbox.
>>>>>>
>>>>>> I worry that this is creating another interface that some poor sod is
>>>>>> going to have to deal with in the future. Is this part of the EFI
>>>>>> standard?
>>>>>
>>>>> No, the UEFI standard does not care much about device-trees. It only defines
>>>>> the GUID for the configuration table.
>>>>>
>>>>>>
>>>>>> We should really be using the compatible string to choose the
>>>>>> devicetree. Why are we using filenames at all? What is the
>>>>>> relationship between the compatible string and the filename? Is there
>>>>>> a lookup table, or should we create one?
>>>>>
>>>>> There is no 1:1 relationship between compatible string and filename, e.g.
>>>>> the following arm64 device-trees use the same compatible string:
>>>>>
>>>>> amd/amd-overdrive-rev-b0.dts
>>>>> amd/amd-overdrive-rev-b1.dts
>>>>>
>>>>> amlogic/meson-axg-jethome-jethub-j110-rev-2.dts
>>>>> amlogic/meson-axg-jethome-jethub-j110-rev-3.dts
>>>>>
>>>>> xilinx/zynqmp-zc1751-xm015-dc1.dts
>>>>> xilinx/zynqmp-zc1751-xm015-dc1.dts
>>>>> xilinx/zynqmp-zc1751-xm015-dc1.dts
>>>>> xilinx/zynqmp-zc1751-xm015-dc1.dts
>>>>>
>>>>>>
>>>>>> The correct way of doing this is implemented in U-Boot with
>>>>>> CONFIG_FIT_BEST_MATCH.
>>>>>
>>>>> Why should we write complicated code to find a *possibly* matching file if
>>>>> we already know the filename that needs to be loaded?
>>>>
>>>> I think that is the unfortunate key here. We can make a guess, or best
>>>> match, but it might not be right. And we need a reliable way to find and
>>>> use the correct tree. And the U-Boot portion of that may be "set the EFI
>>>> var if it's not already set" rather than "and now load it".
>>>
>>> That is not my understanding of how it works. The compatible string is
>>> how Linux knows what the hardware is...if it doesn't match, then
>>> things are going to go wrong. It is also how U-Boot works, e.g. with
>>> FIT. I don't believe this is a 'guess'. The compatible string is used
>>> programmatically and must be correct.
>>
>> It is not how U-Boot works. In distroboot U-Boot uses environment
>> variable fdtfile to load the correct device-tree.
> 
> Yes, that is my point.
> 
>>
>> Your suggestion would mean that distroboot or GRUB would have to sift
>> through literally hundreds of device-tree files which would considerably
>> increase the boot time. The process might end up with multiple dtb files
>> that match.
> 
> Have you tried it? Just for interest I tried it on the latest Fedora on rpi_4:
> 
> $ python scripts/make_fit.py -f test.fit /media/sda2/dtb
> Fit size 6a0400 / 6.6 MB, 485 files, uncompressed 16.8 MB
> 
> On an rpi_3 with a terribly slow uSD card:
> U-Boot> load mmc 0 10000 teste.fit
> 6910708 bytes read in 601 ms (11 MiB/s)
> 
> U-Boot> bootm start 10000
> ## Loading kernel from FIT Image at 00010000 ...
>     Using 'conf-272' configuration
>     Trying 'kernel' kernel subimage
>     Verifying Hash Integrity ... OK
> ## Loading fdt from FIT Image at 00010000 ...
>     Using 'conf-272' configuration
>     Trying 'fdt-272' fdt subimage
>     Verifying Hash Integrity ... OK
>     Uncompressing Flat Device Tree
>     Booting using the fdt blob at 0x3dbf8010
> Working FDT set to 3dbf8010
> 
> Then I timed how long it takes to get the right FDT:
>                   3,466  fdt_best
> 
> So that is 3.5ms. Re the load time, it is easy enough to use an
> external FIT and just load the metadata part, which cuts the load time
> down to 35ms. So the overall cost is about 40ms. It actually takes the
> rpi4 longer than that to write the messages to the display (although
> I'm not sure why).
> 
> There cannot be multiple DTs that match...unless you mean that there
> might be different board revisions. But even then, the compatible
> string is how they are disambiguated.
> 
> So this process is already defined and correct.
> 
>>>
>>> fdt_node_check_compatible() does most of the work...then you need to
>>> check which FDT has the most specific match (i.e. latest in the string
>>> list). That handles things like board revisions, variants, etc.
>>>
>>> My concern is about adding a feature when there is already a defined
>>> spec and mechanism for this to work. What happens when we load the
>>> file and the compatible is wrong?
>>>
>>> At best, I see the filename as a hint.
>>>
>>> [Perhaps this is the wrong time to ask, but why are kernels +DT not
>>> shipped in FIT on ARM?]
>>
>> FIT is U-Boot specific. For Linux distributions it is easier to use a
>> firmware agnostic method of booting.
> 
> I'd like to suggest that distros use both. Then U-Boot can work as it
> was designed and we can avoid these work-arounds.
> 
> FIT is actually implemented in various other bootloaders. In fact
> perhaps grub is the only one that doesn't? I can't think of any
> others.
> 
>>
>>>>
>>>>>> Can we mirror something like that in grub, etc?
>>>>
>>>> I'm splitting this part out because no, having to have N projects write
>>>> the same bit of code, but with its own quirks and bugs doesn't sound
>>>> like the right direction. It's why today we have a few different sets of
>>>> logic to try and find / set the right device tree, so that whatever
>>>> follows after doesn't have to also do that, and get updated for every
>>>> new platform too.
>>>>
>>>> And of course yes, ideally, boards would be manufactured with an up to
>>>> date and correct device tree on them.
>>>
>>> OK, well I perhaps have the wrong end of the stick here.
>>>
>>> As I learn more about how distros boot I see a great need for
>>> information about what is actually being booted. For example, the
>>> current 'bootflow menu' mostly shows useless information when EFI is
>>> used to boot, since it doesn't know what the things are. We have to
>>> jump to grub (or whatever) to find out. Grub knows because the OS set
>>> up a grub menu.
>>
>> The menu should show the EFI boot options. Then you can decide which of
>> the multiple operating systems on your device you want to boot.
>>
>> The bottleneck is secure boot. As Red Hat did not want Microsoft to sign
>> every single kernel they came up with shim. Now we can only have one EFI
>> boot option per operating system. It is only after shim that you can
>> load one of multiple kernels.
> 
> OK, but I wonder if each .efi could have a description of at least who
> wrote it? It would be very useful to tell if it is Ubuntu or Fedora
> before booting it.

You could link an extra section with meta-data into an EFI binary. But 
there is not established standard for this.

https://uapi-group.org/specifications/specs/boot_loader_specification/ 
describes providing the meta-data in a separate file per boot entry. The 
standard seems to reflect the ideas of the systemd developers.

Upstream GRUB makes no use of this specification. It only scans for 
available kernels in /boot/. See file 10_linux.

According to 
https://fedoraproject.org/wiki/Changes/BootLoaderSpecByDefault#Current_status 
Fedora adheres to this specification since release 30.

Debian and Ubuntu don't implement the specification.

> 
>>
>>>
>>> Really we need to see this standard boot thing to the end. Each OS
>>> should provide information from /etc/lsb_release as well as info from
>>> menu.grub (sorry can't remember the right name), so that we know what
>>
>> /boot/grub/grub.cfg
> 
> That is actually a script in a grub format. I think we need something
> more generic. Or are you suggesting that U-Boot should parse grub.cfg
> ?

No. grub.cfg is a programmatic script which may hand off to other 
scripts. It may create labels for menu items but as there may be 
multiple menu levels this is not reliable meta-data.

> 
>>
>>> can be booted. Having to jump to an OS-specific bootloader to even be
>>> able to show a menu is a pretty poor show.
>>
>> With secure-boot you still would need to load the OS specific shim.
>> Further you would need a means to communicate your decision to whatever
>> program loads the kernel.
> 
> Yes that's right, similar to how you are communicating the FDT here, I suppose.
> 
>>
>>>
>>> Anyway I think I understand why this variable is needed. Please add
>>> some documentation on all this.
>>
>> Yes I should add a paragraph on the variable.
>>
>>>
>>> My main concern is whether this sort of thing is going to make it even
>>> harder to boot in a simple, standard manner.
>>>
>>> Re Heinrich's comment:
>>>
>>>> There is no 1:1 relationship between compatible string and filename,
>>>> e.g. the following arm64 device-trees use the same compatible string:
>>>
>>>>> amd/amd-overdrive-rev-b0.dts
>>>>> amd/amd-overdrive-rev-b1.dts
>>>
>>> That just seems like a bug to me. The compatible should include the
>>> board rev as it does for xlnx,zynqmp-zcu102-rev1.1 etc.
>>
>> Yes, here it is a bug.
>>
>> In theory you could have different device-trees for different device
>> configurations (e.g. USB OTG vs USB host). In this case having the same
>> compatible string would be correct.
> 
> That is the sort of thing that really should not be allowed. The FIT
> mechanism keeps people honest.

The FIT mechanism has the concept of configurations. It does not impose 
any restrictions on the compatible string on root level.

Best regards

Heinrich

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

* Re: [PATCH v2 1/1] efi_loader: expose the device-tree file name
  2023-10-23  7:08                   ` Simon Glass
  2023-10-23  8:10                     ` Heinrich Schuchardt
@ 2023-10-23 15:37                     ` Mark Kettenis
  2023-10-23 16:34                       ` Tom Rini
  2023-12-31 14:25                       ` Peter Robinson
  1 sibling, 2 replies; 26+ messages in thread
From: Mark Kettenis @ 2023-10-23 15:37 UTC (permalink / raw)
  To: Simon Glass
  Cc: heinrich.schuchardt, ilias.apalodimas, u-boot, dianders, trini

> From: Simon Glass <sjg@google.com>
> Date: Mon, 23 Oct 2023 00:08:40 -0700
> 
> > > fdt_node_check_compatible() does most of the work...then you need to
> > > check which FDT has the most specific match (i.e. latest in the string
> > > list). That handles things like board revisions, variants, etc.
> > >
> > > My concern is about adding a feature when there is already a defined
> > > spec and mechanism for this to work. What happens when we load the
> > > file and the compatible is wrong?
> > >
> > > At best, I see the filename as a hint.
> > >
> > > [Perhaps this is the wrong time to ask, but why are kernels +DT not
> > > shipped in FIT on ARM?]
> >
> > FIT is U-Boot specific. For Linux distributions it is easier to use a
> > firmware agnostic method of booting.
> 
> I'd like to suggest that distros use both. Then U-Boot can work as it
> was designed and we can avoid these work-arounds.
> 
> FIT is actually implemented in various other bootloaders. In fact
> perhaps grub is the only one that doesn't? I can't think of any
> others.

Simon, please stop pushing this.  OpenBSD's bootloader does not
support FIT and we have no interest in supporting it.  Our users
expect to be able to just copy a new kernel in place and use it and
our OS upgrade procedure depends on this as well.  And this is
incompatble with FIT.  I've explained this about a dozen times to you
now.

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

* Re: [PATCH v2 1/1] efi_loader: expose the device-tree file name
  2023-10-23 15:37                     ` Mark Kettenis
@ 2023-10-23 16:34                       ` Tom Rini
  2023-10-23 17:05                         ` Mark Kettenis
  2023-12-31 14:25                       ` Peter Robinson
  1 sibling, 1 reply; 26+ messages in thread
From: Tom Rini @ 2023-10-23 16:34 UTC (permalink / raw)
  To: Mark Kettenis
  Cc: Simon Glass, heinrich.schuchardt, ilias.apalodimas, u-boot, dianders

[-- Attachment #1: Type: text/plain, Size: 1930 bytes --]

On Mon, Oct 23, 2023 at 05:37:34PM +0200, Mark Kettenis wrote:
> > From: Simon Glass <sjg@google.com>
> > Date: Mon, 23 Oct 2023 00:08:40 -0700
> > 
> > > > fdt_node_check_compatible() does most of the work...then you need to
> > > > check which FDT has the most specific match (i.e. latest in the string
> > > > list). That handles things like board revisions, variants, etc.
> > > >
> > > > My concern is about adding a feature when there is already a defined
> > > > spec and mechanism for this to work. What happens when we load the
> > > > file and the compatible is wrong?
> > > >
> > > > At best, I see the filename as a hint.
> > > >
> > > > [Perhaps this is the wrong time to ask, but why are kernels +DT not
> > > > shipped in FIT on ARM?]
> > >
> > > FIT is U-Boot specific. For Linux distributions it is easier to use a
> > > firmware agnostic method of booting.
> > 
> > I'd like to suggest that distros use both. Then U-Boot can work as it
> > was designed and we can avoid these work-arounds.
> > 
> > FIT is actually implemented in various other bootloaders. In fact
> > perhaps grub is the only one that doesn't? I can't think of any
> > others.
> 
> Simon, please stop pushing this.  OpenBSD's bootloader does not
> support FIT and we have no interest in supporting it.  Our users
> expect to be able to just copy a new kernel in place and use it and
> our OS upgrade procedure depends on this as well.  And this is
> incompatble with FIT.  I've explained this about a dozen times to you
> now.

In the context of this thread, genuinely, how will OpenBSD (and the rest
of the BSD families) operate? I agree U-Boot doesn't want to have to
know all of the UFSes, so that means the SCT will be populated either by
the DT passed to U-Boot, or the DT we were built with. Is it that since
the next stage is an EFI app, it will check that variable and use that
hint?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

* Re: [PATCH v2 1/1] efi_loader: expose the device-tree file name
  2023-10-23 16:34                       ` Tom Rini
@ 2023-10-23 17:05                         ` Mark Kettenis
  2023-11-03 19:17                           ` Simon Glass
  0 siblings, 1 reply; 26+ messages in thread
From: Mark Kettenis @ 2023-10-23 17:05 UTC (permalink / raw)
  To: Tom Rini; +Cc: sjg, heinrich.schuchardt, ilias.apalodimas, u-boot, dianders

> Date: Mon, 23 Oct 2023 12:34:55 -0400
> From: Tom Rini <trini@konsulko.com>
> 
> On Mon, Oct 23, 2023 at 05:37:34PM +0200, Mark Kettenis wrote:
> > > From: Simon Glass <sjg@google.com>
> > > Date: Mon, 23 Oct 2023 00:08:40 -0700
> > > 
> > > > > fdt_node_check_compatible() does most of the work...then you need to
> > > > > check which FDT has the most specific match (i.e. latest in the string
> > > > > list). That handles things like board revisions, variants, etc.
> > > > >
> > > > > My concern is about adding a feature when there is already a defined
> > > > > spec and mechanism for this to work. What happens when we load the
> > > > > file and the compatible is wrong?
> > > > >
> > > > > At best, I see the filename as a hint.
> > > > >
> > > > > [Perhaps this is the wrong time to ask, but why are kernels +DT not
> > > > > shipped in FIT on ARM?]
> > > >
> > > > FIT is U-Boot specific. For Linux distributions it is easier to use a
> > > > firmware agnostic method of booting.
> > > 
> > > I'd like to suggest that distros use both. Then U-Boot can work as it
> > > was designed and we can avoid these work-arounds.
> > > 
> > > FIT is actually implemented in various other bootloaders. In fact
> > > perhaps grub is the only one that doesn't? I can't think of any
> > > others.
> > 
> > Simon, please stop pushing this.  OpenBSD's bootloader does not
> > support FIT and we have no interest in supporting it.  Our users
> > expect to be able to just copy a new kernel in place and use it and
> > our OS upgrade procedure depends on this as well.  And this is
> > incompatble with FIT.  I've explained this about a dozen times to you
> > now.
> 
> In the context of this thread, genuinely, how will OpenBSD (and the rest
> of the BSD families) operate? I agree U-Boot doesn't want to have to
> know all of the UFSes, so that means the SCT will be populated either by
> the DT passed to U-Boot, or the DT we were built with. Is it that since
> the next stage is an EFI app, it will check that variable and use that
> hint?

Yes, that is exactly what I want to do.  Hopefully the DT that is
passed to U-Boot or that U-Boot was built with will be good enough in
most cases.  But when it isn't users can use the OpenBSD bootloader
(which is an EFI app) to load an updated DT.

I can't speak for the other BSDs, but I think both FreeBSD and NetBSD
have a very similar boot mechanism.

Cheers,

Mark

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

* Re: [PATCH v2 1/1] efi_loader: expose the device-tree file name
  2023-10-23 17:05                         ` Mark Kettenis
@ 2023-11-03 19:17                           ` Simon Glass
  2023-11-03 19:42                             ` Ilias Apalodimas
                                               ` (3 more replies)
  0 siblings, 4 replies; 26+ messages in thread
From: Simon Glass @ 2023-11-03 19:17 UTC (permalink / raw)
  To: Mark Kettenis
  Cc: Tom Rini, heinrich.schuchardt, ilias.apalodimas, u-boot, dianders

Hi,

On Mon, 23 Oct 2023 at 11:06, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>
> > Date: Mon, 23 Oct 2023 12:34:55 -0400
> > From: Tom Rini <trini@konsulko.com>
> >
> > On Mon, Oct 23, 2023 at 05:37:34PM +0200, Mark Kettenis wrote:
> > > > From: Simon Glass <sjg@google.com>
> > > > Date: Mon, 23 Oct 2023 00:08:40 -0700
> > > >
> > > > > > fdt_node_check_compatible() does most of the work...then you need to
> > > > > > check which FDT has the most specific match (i.e. latest in the string
> > > > > > list). That handles things like board revisions, variants, etc.
> > > > > >
> > > > > > My concern is about adding a feature when there is already a defined
> > > > > > spec and mechanism for this to work. What happens when we load the
> > > > > > file and the compatible is wrong?
> > > > > >
> > > > > > At best, I see the filename as a hint.
> > > > > >
> > > > > > [Perhaps this is the wrong time to ask, but why are kernels +DT not
> > > > > > shipped in FIT on ARM?]
> > > > >
> > > > > FIT is U-Boot specific. For Linux distributions it is easier to use a
> > > > > firmware agnostic method of booting.
> > > >
> > > > I'd like to suggest that distros use both. Then U-Boot can work as it
> > > > was designed and we can avoid these work-arounds.
> > > >
> > > > FIT is actually implemented in various other bootloaders. In fact
> > > > perhaps grub is the only one that doesn't? I can't think of any
> > > > others.
> > >
> > > Simon, please stop pushing this.  OpenBSD's bootloader does not
> > > support FIT and we have no interest in supporting it.  Our users
> > > expect to be able to just copy a new kernel in place and use it and
> > > our OS upgrade procedure depends on this as well.  And this is
> > > incompatble with FIT.  I've explained this about a dozen times to you
> > > now.
> >
> > In the context of this thread, genuinely, how will OpenBSD (and the rest
> > of the BSD families) operate? I agree U-Boot doesn't want to have to
> > know all of the UFSes, so that means the SCT will be populated either by
> > the DT passed to U-Boot, or the DT we were built with. Is it that since
> > the next stage is an EFI app, it will check that variable and use that
> > hint?
>
> Yes, that is exactly what I want to do.  Hopefully the DT that is
> passed to U-Boot or that U-Boot was built with will be good enough in
> most cases.  But when it isn't users can use the OpenBSD bootloader
> (which is an EFI app) to load an updated DT.
>
> I can't speak for the other BSDs, but I think both FreeBSD and NetBSD
> have a very similar boot mechanism.

I've been thinking about this patch a bit more, and I have grave misgivings.

I predict that if we take this, it will become an ABI from U-Boot and
we will not be able to drop it.

Here is what I suggest instead: provide a protocol for U-Boot to
provide the DT over EFI. Provide any information needed by U-Boot,
such as the directory containing the files.

We already have the ability to put a DT in the system table. We
already have a way to package DT into a FIT and allow U-Boot to select
the correct one.

With something like this it will be impossible for U-Boot to boot a
distro without using grub, etc. since it won't know what DT to use.
This information would be held in a script somewhere which no one can
figure out without executing it.

I think we need to carefully think about the design of this.

Regards,
Simon

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

* Re: [PATCH v2 1/1] efi_loader: expose the device-tree file name
  2023-11-03 19:17                           ` Simon Glass
@ 2023-11-03 19:42                             ` Ilias Apalodimas
  2023-11-03 20:00                             ` Ilias Apalodimas
                                               ` (2 subsequent siblings)
  3 siblings, 0 replies; 26+ messages in thread
From: Ilias Apalodimas @ 2023-11-03 19:42 UTC (permalink / raw)
  To: Simon Glass
  Cc: Mark Kettenis, Tom Rini, heinrich.schuchardt, u-boot, dianders

Hi Simon

On Fri, 3 Nov 2023 at 21:17, Simon Glass <sjg@google.com> wrote:
>
> Hi,
>
> On Mon, 23 Oct 2023 at 11:06, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> >
> > > Date: Mon, 23 Oct 2023 12:34:55 -0400
> > > From: Tom Rini <trini@konsulko.com>
> > >
> > > On Mon, Oct 23, 2023 at 05:37:34PM +0200, Mark Kettenis wrote:
> > > > > From: Simon Glass <sjg@google.com>
> > > > > Date: Mon, 23 Oct 2023 00:08:40 -0700
> > > > >
> > > > > > > fdt_node_check_compatible() does most of the work...then you need to
> > > > > > > check which FDT has the most specific match (i.e. latest in the string
> > > > > > > list). That handles things like board revisions, variants, etc.
> > > > > > >
> > > > > > > My concern is about adding a feature when there is already a defined
> > > > > > > spec and mechanism for this to work. What happens when we load the
> > > > > > > file and the compatible is wrong?
> > > > > > >
> > > > > > > At best, I see the filename as a hint.
> > > > > > >
> > > > > > > [Perhaps this is the wrong time to ask, but why are kernels +DT not
> > > > > > > shipped in FIT on ARM?]
> > > > > >
> > > > > > FIT is U-Boot specific. For Linux distributions it is easier to use a
> > > > > > firmware agnostic method of booting.
> > > > >
> > > > > I'd like to suggest that distros use both. Then U-Boot can work as it
> > > > > was designed and we can avoid these work-arounds.
> > > > >
> > > > > FIT is actually implemented in various other bootloaders. In fact
> > > > > perhaps grub is the only one that doesn't? I can't think of any
> > > > > others.
> > > >
> > > > Simon, please stop pushing this.  OpenBSD's bootloader does not
> > > > support FIT and we have no interest in supporting it.  Our users
> > > > expect to be able to just copy a new kernel in place and use it and
> > > > our OS upgrade procedure depends on this as well.  And this is
> > > > incompatble with FIT.  I've explained this about a dozen times to you
> > > > now.
> > >
> > > In the context of this thread, genuinely, how will OpenBSD (and the rest
> > > of the BSD families) operate? I agree U-Boot doesn't want to have to
> > > know all of the UFSes, so that means the SCT will be populated either by
> > > the DT passed to U-Boot, or the DT we were built with. Is it that since
> > > the next stage is an EFI app, it will check that variable and use that
> > > hint?
> >
> > Yes, that is exactly what I want to do.  Hopefully the DT that is
> > passed to U-Boot or that U-Boot was built with will be good enough in
> > most cases.  But when it isn't users can use the OpenBSD bootloader
> > (which is an EFI app) to load an updated DT.
> >
> > I can't speak for the other BSDs, but I think both FreeBSD and NetBSD
> > have a very similar boot mechanism.
>
> I've been thinking about this patch a bit more, and I have grave misgivings.
>
> I predict that if we take this, it will become an ABI from U-Boot and
> we will not be able to drop it.
>
> Here is what I suggest instead: provide a protocol for U-Boot to
> provide the DT over EFI. Provide any information needed by U-Boot,
> such as the directory containing the files.
>
> We already have the ability to put a DT in the system table. We
> already have a way to package DT into a FIT and allow U-Boot to select
> the correct one.
>
> With something like this it will be impossible for U-Boot to boot a
> distro without using grub, etc. since it won't know what DT to use.
> This information would be held in a script somewhere which no one can
> figure out without executing it.

That's not entirely correct.  If you want to boot an OS with EFI
without GRUB you will boot via the EFI bootmanager. We already have a
way of adding the initrd we need to use in EFI boot options. Adding
the DT in a similar fashion wouldn't be too hard.

Thanks
/Ilias
>
> I think we need to carefully think about the design of this.
>
> Regards,
> Simon

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

* Re: [PATCH v2 1/1] efi_loader: expose the device-tree file name
  2023-11-03 19:17                           ` Simon Glass
  2023-11-03 19:42                             ` Ilias Apalodimas
@ 2023-11-03 20:00                             ` Ilias Apalodimas
  2023-11-03 20:03                             ` Tom Rini
  2023-11-14 19:09                             ` Mark Kettenis
  3 siblings, 0 replies; 26+ messages in thread
From: Ilias Apalodimas @ 2023-11-03 20:00 UTC (permalink / raw)
  To: Heinrich Schuchardt
  Cc: Mark Kettenis, Tom Rini, u-boot, dianders, Simon Glass

Hi all,

On Fri, 3 Nov 2023 at 21:17, Simon Glass <sjg@google.com> wrote:
>
> Hi,
>
> On Mon, 23 Oct 2023 at 11:06, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> >
> > > Date: Mon, 23 Oct 2023 12:34:55 -0400
> > > From: Tom Rini <trini@konsulko.com>
> > >
> > > On Mon, Oct 23, 2023 at 05:37:34PM +0200, Mark Kettenis wrote:
> > > > > From: Simon Glass <sjg@google.com>
> > > > > Date: Mon, 23 Oct 2023 00:08:40 -0700
> > > > >
> > > > > > > fdt_node_check_compatible() does most of the work...then you need to
> > > > > > > check which FDT has the most specific match (i.e. latest in the string
> > > > > > > list). That handles things like board revisions, variants, etc.
> > > > > > >
> > > > > > > My concern is about adding a feature when there is already a defined
> > > > > > > spec and mechanism for this to work. What happens when we load the
> > > > > > > file and the compatible is wrong?
> > > > > > >
> > > > > > > At best, I see the filename as a hint.
> > > > > > >
> > > > > > > [Perhaps this is the wrong time to ask, but why are kernels +DT not
> > > > > > > shipped in FIT on ARM?]
> > > > > >
> > > > > > FIT is U-Boot specific. For Linux distributions it is easier to use a
> > > > > > firmware agnostic method of booting.
> > > > >
> > > > > I'd like to suggest that distros use both. Then U-Boot can work as it
> > > > > was designed and we can avoid these work-arounds.
> > > > >
> > > > > FIT is actually implemented in various other bootloaders. In fact
> > > > > perhaps grub is the only one that doesn't? I can't think of any
> > > > > others.
> > > >
> > > > Simon, please stop pushing this.  OpenBSD's bootloader does not
> > > > support FIT and we have no interest in supporting it.  Our users
> > > > expect to be able to just copy a new kernel in place and use it and
> > > > our OS upgrade procedure depends on this as well.  And this is
> > > > incompatble with FIT.  I've explained this about a dozen times to you
> > > > now.
> > >
> > > In the context of this thread, genuinely, how will OpenBSD (and the rest
> > > of the BSD families) operate? I agree U-Boot doesn't want to have to
> > > know all of the UFSes, so that means the SCT will be populated either by
> > > the DT passed to U-Boot, or the DT we were built with. Is it that since
> > > the next stage is an EFI app, it will check that variable and use that
> > > hint?
> >
> > Yes, that is exactly what I want to do.  Hopefully the DT that is
> > passed to U-Boot or that U-Boot was built with will be good enough in
> > most cases.  But when it isn't users can use the OpenBSD bootloader
> > (which is an EFI app) to load an updated DT.
> >
> > I can't speak for the other BSDs, but I think both FreeBSD and NetBSD
> > have a very similar boot mechanism.
>
> I've been thinking about this patch a bit more, and I have grave misgivings.
>
> I predict that if we take this, it will become an ABI from U-Boot and
> we will not be able to drop it.
>
> Here is what I suggest instead: provide a protocol for U-Boot to
> provide the DT over EFI. Provide any information needed by U-Boot,
> such as the directory containing the files.

Heinirch perhaps we can use LoadFile2 just like we use it for the
initrd? But expose a different GUID for the DT?

Thanks
/Ilias
>
> We already have the ability to put a DT in the system table. We
> already have a way to package DT into a FIT and allow U-Boot to select
> the correct one.
>
> With something like this it will be impossible for U-Boot to boot a
> distro without using grub, etc. since it won't know what DT to use.
> This information would be held in a script somewhere which no one can
> figure out without executing it.
>
> I think we need to carefully think about the design of this.
>
> Regards,
> Simon

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

* Re: [PATCH v2 1/1] efi_loader: expose the device-tree file name
  2023-11-03 19:17                           ` Simon Glass
  2023-11-03 19:42                             ` Ilias Apalodimas
  2023-11-03 20:00                             ` Ilias Apalodimas
@ 2023-11-03 20:03                             ` Tom Rini
  2023-11-14 19:09                             ` Mark Kettenis
  3 siblings, 0 replies; 26+ messages in thread
From: Tom Rini @ 2023-11-03 20:03 UTC (permalink / raw)
  To: Simon Glass
  Cc: Mark Kettenis, heinrich.schuchardt, ilias.apalodimas, u-boot,
	dianders, Rob Herring

[-- Attachment #1: Type: text/plain, Size: 4249 bytes --]

On Fri, Nov 03, 2023 at 01:17:18PM -0600, Simon Glass wrote:
> Hi,
> 
> On Mon, 23 Oct 2023 at 11:06, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> >
> > > Date: Mon, 23 Oct 2023 12:34:55 -0400
> > > From: Tom Rini <trini@konsulko.com>
> > >
> > > On Mon, Oct 23, 2023 at 05:37:34PM +0200, Mark Kettenis wrote:
> > > > > From: Simon Glass <sjg@google.com>
> > > > > Date: Mon, 23 Oct 2023 00:08:40 -0700
> > > > >
> > > > > > > fdt_node_check_compatible() does most of the work...then you need to
> > > > > > > check which FDT has the most specific match (i.e. latest in the string
> > > > > > > list). That handles things like board revisions, variants, etc.
> > > > > > >
> > > > > > > My concern is about adding a feature when there is already a defined
> > > > > > > spec and mechanism for this to work. What happens when we load the
> > > > > > > file and the compatible is wrong?
> > > > > > >
> > > > > > > At best, I see the filename as a hint.
> > > > > > >
> > > > > > > [Perhaps this is the wrong time to ask, but why are kernels +DT not
> > > > > > > shipped in FIT on ARM?]
> > > > > >
> > > > > > FIT is U-Boot specific. For Linux distributions it is easier to use a
> > > > > > firmware agnostic method of booting.
> > > > >
> > > > > I'd like to suggest that distros use both. Then U-Boot can work as it
> > > > > was designed and we can avoid these work-arounds.
> > > > >
> > > > > FIT is actually implemented in various other bootloaders. In fact
> > > > > perhaps grub is the only one that doesn't? I can't think of any
> > > > > others.
> > > >
> > > > Simon, please stop pushing this.  OpenBSD's bootloader does not
> > > > support FIT and we have no interest in supporting it.  Our users
> > > > expect to be able to just copy a new kernel in place and use it and
> > > > our OS upgrade procedure depends on this as well.  And this is
> > > > incompatble with FIT.  I've explained this about a dozen times to you
> > > > now.
> > >
> > > In the context of this thread, genuinely, how will OpenBSD (and the rest
> > > of the BSD families) operate? I agree U-Boot doesn't want to have to
> > > know all of the UFSes, so that means the SCT will be populated either by
> > > the DT passed to U-Boot, or the DT we were built with. Is it that since
> > > the next stage is an EFI app, it will check that variable and use that
> > > hint?
> >
> > Yes, that is exactly what I want to do.  Hopefully the DT that is
> > passed to U-Boot or that U-Boot was built with will be good enough in
> > most cases.  But when it isn't users can use the OpenBSD bootloader
> > (which is an EFI app) to load an updated DT.
> >
> > I can't speak for the other BSDs, but I think both FreeBSD and NetBSD
> > have a very similar boot mechanism.
> 
> I've been thinking about this patch a bit more, and I have grave misgivings.
> 
> I predict that if we take this, it will become an ABI from U-Boot and
> we will not be able to drop it.

To be clear, if the GUID in question (and that isn't quoted in this
part of the thread) is accepted and used as suggested, it's not an ABI
for U-Boot, it's an ABI for everyone, including the rest of device tree.

> Here is what I suggest instead: provide a protocol for U-Boot to
> provide the DT over EFI. Provide any information needed by U-Boot,
> such as the directory containing the files.
> 
> We already have the ability to put a DT in the system table. We
> already have a way to package DT into a FIT and allow U-Boot to select
> the correct one.
> 
> With something like this it will be impossible for U-Boot to boot a
> distro without using grub, etc. since it won't know what DT to use.

The existing system DT is the DT that should be used, as the long term
goal, not a loaded from storage DT. Until that point however, it's
already required to know what and where to load a device tree from, and
it not being at all uniform is where some of the current pain comes
from.

> This information would be held in a script somewhere which no one can
> figure out without executing it.
> 
> I think we need to carefully think about the design of this.

Yes, we do need to be careful and intentional here.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

* Re: [PATCH v2 1/1] efi_loader: expose the device-tree file name
  2023-11-03 19:17                           ` Simon Glass
                                               ` (2 preceding siblings ...)
  2023-11-03 20:03                             ` Tom Rini
@ 2023-11-14 19:09                             ` Mark Kettenis
  2023-11-14 23:20                               ` Tom Rini
  3 siblings, 1 reply; 26+ messages in thread
From: Mark Kettenis @ 2023-11-14 19:09 UTC (permalink / raw)
  To: Simon Glass
  Cc: trini, heinrich.schuchardt, ilias.apalodimas, u-boot, dianders

> From: Simon Glass <sjg@google.com>
> Date: Fri, 3 Nov 2023 13:17:18 -0600
> 
> Hi,
> 
> On Mon, 23 Oct 2023 at 11:06, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> >
> > > Date: Mon, 23 Oct 2023 12:34:55 -0400
> > > From: Tom Rini <trini@konsulko.com>
> > >
> > > On Mon, Oct 23, 2023 at 05:37:34PM +0200, Mark Kettenis wrote:
> > > > > From: Simon Glass <sjg@google.com>
> > > > > Date: Mon, 23 Oct 2023 00:08:40 -0700
> > > > >
> > > > > > > fdt_node_check_compatible() does most of the work...then you need to
> > > > > > > check which FDT has the most specific match (i.e. latest in the string
> > > > > > > list). That handles things like board revisions, variants, etc.
> > > > > > >
> > > > > > > My concern is about adding a feature when there is already a defined
> > > > > > > spec and mechanism for this to work. What happens when we load the
> > > > > > > file and the compatible is wrong?
> > > > > > >
> > > > > > > At best, I see the filename as a hint.
> > > > > > >
> > > > > > > [Perhaps this is the wrong time to ask, but why are kernels +DT not
> > > > > > > shipped in FIT on ARM?]
> > > > > >
> > > > > > FIT is U-Boot specific. For Linux distributions it is easier to use a
> > > > > > firmware agnostic method of booting.
> > > > >
> > > > > I'd like to suggest that distros use both. Then U-Boot can work as it
> > > > > was designed and we can avoid these work-arounds.
> > > > >
> > > > > FIT is actually implemented in various other bootloaders. In fact
> > > > > perhaps grub is the only one that doesn't? I can't think of any
> > > > > others.
> > > >
> > > > Simon, please stop pushing this.  OpenBSD's bootloader does not
> > > > support FIT and we have no interest in supporting it.  Our users
> > > > expect to be able to just copy a new kernel in place and use it and
> > > > our OS upgrade procedure depends on this as well.  And this is
> > > > incompatble with FIT.  I've explained this about a dozen times to you
> > > > now.
> > >
> > > In the context of this thread, genuinely, how will OpenBSD (and the rest
> > > of the BSD families) operate? I agree U-Boot doesn't want to have to
> > > know all of the UFSes, so that means the SCT will be populated either by
> > > the DT passed to U-Boot, or the DT we were built with. Is it that since
> > > the next stage is an EFI app, it will check that variable and use that
> > > hint?
> >
> > Yes, that is exactly what I want to do.  Hopefully the DT that is
> > passed to U-Boot or that U-Boot was built with will be good enough in
> > most cases.  But when it isn't users can use the OpenBSD bootloader
> > (which is an EFI app) to load an updated DT.
> >
> > I can't speak for the other BSDs, but I think both FreeBSD and NetBSD
> > have a very similar boot mechanism.
> 
> I've been thinking about this patch a bit more, and I have grave misgivings.
> 
> I predict that if we take this, it will become an ABI from U-Boot and
> we will not be able to drop it.

Why would we want to drop it?

> Here is what I suggest instead: provide a protocol for U-Boot to
> provide the DT over EFI. Provide any information needed by U-Boot,
> such as the directory containing the files.

So you want something more complicated than a simple EFI variable?  Why?

> We already have the ability to put a DT in the system table. We
> already have a way to package DT into a FIT and allow U-Boot to select
> the correct one.

Please stop pushing FIT for use cases where it doesn't make sense.

> With something like this it will be impossible for U-Boot to boot a
> distro without using grub, etc. since it won't know what DT to use.
> This information would be held in a script somewhere which no one can
> figure out without executing it.

Huh?  U-Boot already knows the name of name of the DT filename.  It is
what we currently use in the distroboot scripts.  You are free to use
a U-Boot specific bootflow based on that if you really want to.  But
many distros (or other OSes like OpenBSD) really don't want to
maintain support for multiple boot flows.

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

* Re: [PATCH v2 1/1] efi_loader: expose the device-tree file name
  2023-11-14 19:09                             ` Mark Kettenis
@ 2023-11-14 23:20                               ` Tom Rini
  0 siblings, 0 replies; 26+ messages in thread
From: Tom Rini @ 2023-11-14 23:20 UTC (permalink / raw)
  To: Mark Kettenis
  Cc: Simon Glass, heinrich.schuchardt, ilias.apalodimas, u-boot, dianders

[-- Attachment #1: Type: text/plain, Size: 7733 bytes --]

On Tue, Nov 14, 2023 at 08:09:52PM +0100, Mark Kettenis wrote:
> > From: Simon Glass <sjg@google.com>
> > Date: Fri, 3 Nov 2023 13:17:18 -0600
> > 
> > Hi,
> > 
> > On Mon, 23 Oct 2023 at 11:06, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> > >
> > > > Date: Mon, 23 Oct 2023 12:34:55 -0400
> > > > From: Tom Rini <trini@konsulko.com>
> > > >
> > > > On Mon, Oct 23, 2023 at 05:37:34PM +0200, Mark Kettenis wrote:
> > > > > > From: Simon Glass <sjg@google.com>
> > > > > > Date: Mon, 23 Oct 2023 00:08:40 -0700
> > > > > >
> > > > > > > > fdt_node_check_compatible() does most of the work...then you need to
> > > > > > > > check which FDT has the most specific match (i.e. latest in the string
> > > > > > > > list). That handles things like board revisions, variants, etc.
> > > > > > > >
> > > > > > > > My concern is about adding a feature when there is already a defined
> > > > > > > > spec and mechanism for this to work. What happens when we load the
> > > > > > > > file and the compatible is wrong?
> > > > > > > >
> > > > > > > > At best, I see the filename as a hint.
> > > > > > > >
> > > > > > > > [Perhaps this is the wrong time to ask, but why are kernels +DT not
> > > > > > > > shipped in FIT on ARM?]
> > > > > > >
> > > > > > > FIT is U-Boot specific. For Linux distributions it is easier to use a
> > > > > > > firmware agnostic method of booting.
> > > > > >
> > > > > > I'd like to suggest that distros use both. Then U-Boot can work as it
> > > > > > was designed and we can avoid these work-arounds.
> > > > > >
> > > > > > FIT is actually implemented in various other bootloaders. In fact
> > > > > > perhaps grub is the only one that doesn't? I can't think of any
> > > > > > others.
> > > > >
> > > > > Simon, please stop pushing this.  OpenBSD's bootloader does not
> > > > > support FIT and we have no interest in supporting it.  Our users
> > > > > expect to be able to just copy a new kernel in place and use it and
> > > > > our OS upgrade procedure depends on this as well.  And this is
> > > > > incompatble with FIT.  I've explained this about a dozen times to you
> > > > > now.
> > > >
> > > > In the context of this thread, genuinely, how will OpenBSD (and the rest
> > > > of the BSD families) operate? I agree U-Boot doesn't want to have to
> > > > know all of the UFSes, so that means the SCT will be populated either by
> > > > the DT passed to U-Boot, or the DT we were built with. Is it that since
> > > > the next stage is an EFI app, it will check that variable and use that
> > > > hint?
> > >
> > > Yes, that is exactly what I want to do.  Hopefully the DT that is
> > > passed to U-Boot or that U-Boot was built with will be good enough in
> > > most cases.  But when it isn't users can use the OpenBSD bootloader
> > > (which is an EFI app) to load an updated DT.
> > >
> > > I can't speak for the other BSDs, but I think both FreeBSD and NetBSD
> > > have a very similar boot mechanism.
> > 
> > I've been thinking about this patch a bit more, and I have grave misgivings.
> > 
> > I predict that if we take this, it will become an ABI from U-Boot and
> > we will not be able to drop it.
> 
> Why would we want to drop it?

Because filename isn't an obvious great idea to encode as ABI? It's not
formally one right now, even with the Linux Kernel installing the ARM=arm
dtb files in a single directory even after re-organizing the source
directory to match ARCH=arm64 with per-vendor directories.

> > Here is what I suggest instead: provide a protocol for U-Boot to
> > provide the DT over EFI. Provide any information needed by U-Boot,
> > such as the directory containing the files.
> 
> So you want something more complicated than a simple EFI variable?  Why?

I think part of this all came out of the issues Fedora was having where
they (a) need to override the DTB to ensure the kernel boots and (b)
oops, the logic they had been using wasn't being invoked (unexpected
Fedora-based change).

Some way to say "here is the device tree you should use" needs to come
from somewhere. It would be great if the running device tree was already
correct, but we don't know that it will be, and that's also still not a
U-Boot problem. One of the examples here was that the Pi firmware DTB
wasn't 100% correct. I forget what the exact fixup ended up being there.

So, some sort of hint for the board to say "here is the device tree you
should use". I know you earlier noted that you didn't think U-Boot would
want to know how to read N different UFS formats, and yes, true. Maybe
that just means that it needs to reside on some other filesystem format,
like Linux has to do? I don't know. How to best do that, I don't know.

> > We already have the ability to put a DT in the system table. We
> > already have a way to package DT into a FIT and allow U-Boot to select
> > the correct one.
> 
> Please stop pushing FIT for use cases where it doesn't make sense.

OK, but where is that? At least part of the "we don't want to use FIT"
camp instead has
https://uapi-group.org/specifications/specs/unified_kernel_image/ which
then leaves the details on how to select the right device tree up to
the implementation.  Which, sigh, that's the hard part. And at least as
I understand it part of the motivation is that "everyone" wants a single
file that can be used for UEFI secure boot and include all of the device
trees, so that they can be covered by the signature. Which is something
FIT images have had for forever (minus a "wrap it inside a .data section
so it can be included in a signature).

Life is a lot easier if we know the device's compatible string and then
if needed, load a newer device tree based on that compatible string.
Unfortunately, that too is not an ABI and there's a few examples where
that wouldn't work, and hasn't been fixed yet because so little relies
on the machine compatible string itself today.

> > With something like this it will be impossible for U-Boot to boot a
> > distro without using grub, etc. since it won't know what DT to use.
> > This information would be held in a script somewhere which no one can
> > figure out without executing it.
> 
> Huh?  U-Boot already knows the name of name of the DT filename.  It is
> what we currently use in the distroboot scripts.  You are free to use
> a U-Boot specific bootflow based on that if you really want to.  But
> many distros (or other OSes like OpenBSD) really don't want to
> maintain support for multiple boot flows.

U-Boot doesn't know the DT filename, we have assorted bits of
board-specific logic to either setenv at runtime, or we hard-code that
filename at build time. We do however "know" the machine compatible
string already because you need that for a valid device tree which you
need for U-Boot today. And yes, if and only if the device tree we get
from the prior stage (or build in) is correct, we don't need to do
anything else, and can pass that to the next stage. But it also doesn't
seem like today any Linux distribution does the same thing another Linux
distribution does (excepting "spins" and so on) when it comes to loading
the kernel and device tree. Not to pick on Fedora (really!) but part of
the issue was that the patch to change where device trees are loaded
from was applied, but then not in the boot command that was being used.

Maybe part of the issue is that enough of us don't know how it works and
haven't experimented. And since at least I learn best by investigating,
should I try pine64_plus_defconfig or rpi_arm64_defconfig (on a 3 B+) on
a fresh SD card to get a better feel for what's going on?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

* Re: [PATCH v2 1/1] efi_loader: expose the device-tree file name
  2023-10-23 15:37                     ` Mark Kettenis
  2023-10-23 16:34                       ` Tom Rini
@ 2023-12-31 14:25                       ` Peter Robinson
  1 sibling, 0 replies; 26+ messages in thread
From: Peter Robinson @ 2023-12-31 14:25 UTC (permalink / raw)
  To: Mark Kettenis
  Cc: Simon Glass, heinrich.schuchardt, ilias.apalodimas, u-boot,
	dianders, trini

On Mon, Oct 23, 2023 at 4:55 PM Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>
> > From: Simon Glass <sjg@google.com>
> > Date: Mon, 23 Oct 2023 00:08:40 -0700
> >
> > > > fdt_node_check_compatible() does most of the work...then you need to
> > > > check which FDT has the most specific match (i.e. latest in the string
> > > > list). That handles things like board revisions, variants, etc.
> > > >
> > > > My concern is about adding a feature when there is already a defined
> > > > spec and mechanism for this to work. What happens when we load the
> > > > file and the compatible is wrong?
> > > >
> > > > At best, I see the filename as a hint.
> > > >
> > > > [Perhaps this is the wrong time to ask, but why are kernels +DT not
> > > > shipped in FIT on ARM?]
> > >
> > > FIT is U-Boot specific. For Linux distributions it is easier to use a
> > > firmware agnostic method of booting.
> >
> > I'd like to suggest that distros use both. Then U-Boot can work as it
> > was designed and we can avoid these work-arounds.
> >
> > FIT is actually implemented in various other bootloaders. In fact
> > perhaps grub is the only one that doesn't? I can't think of any
> > others.
>
> Simon, please stop pushing this.  OpenBSD's bootloader does not
> support FIT and we have no interest in supporting it.  Our users
> expect to be able to just copy a new kernel in place and use it and
> our OS upgrade procedure depends on this as well.  And this is
> incompatble with FIT.  I've explained this about a dozen times to you
> now.

For reference Fedora and related ecosystems have no interest in
supporting FIT either, we have enough moving targets, we're not
supporting more, it's very much why we're focused on UEFI, it provides
one way, one kernel, for booting all various different devices we
actively support in main Fedora, it's very much the way the x86
ecosystem has gone as well as the aarch64 server ecosystem. Fedora has
no interest in using FIT in this context either.

Peter

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

end of thread, other threads:[~2023-12-31 14:25 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-17 13:49 [PATCH v2 1/1] efi_loader: expose the device-tree file name Heinrich Schuchardt
2023-10-17 14:15 ` Ilias Apalodimas
2023-10-18  3:33 ` Simon Glass
2023-10-18  8:15   ` Heinrich Schuchardt
2023-10-19 13:55     ` Simon Glass
2023-10-19 14:14       ` Mark Kettenis
2023-10-19 16:09       ` Heinrich Schuchardt
2023-10-20 13:21         ` Simon Glass
2023-10-20 13:55           ` Tom Rini
2023-10-20 15:40           ` Heinrich Schuchardt
2023-10-20 16:24             ` Tom Rini
2023-10-21 15:42               ` Simon Glass
2023-10-22  4:53                 ` Heinrich Schuchardt
2023-10-23  7:08                   ` Simon Glass
2023-10-23  8:10                     ` Heinrich Schuchardt
2023-10-23 15:37                     ` Mark Kettenis
2023-10-23 16:34                       ` Tom Rini
2023-10-23 17:05                         ` Mark Kettenis
2023-11-03 19:17                           ` Simon Glass
2023-11-03 19:42                             ` Ilias Apalodimas
2023-11-03 20:00                             ` Ilias Apalodimas
2023-11-03 20:03                             ` Tom Rini
2023-11-14 19:09                             ` Mark Kettenis
2023-11-14 23:20                               ` Tom Rini
2023-12-31 14:25                       ` Peter Robinson
2023-10-20 21:19           ` Simon Glass

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.