[PATCH v8 5/8] sandbox: Report host default-filename in native mode

Simon Glass sjg at chromium.org
Wed Oct 30 08:44:30 CET 2024


Hi Tom,

On Tue, 29 Oct 2024 at 17:40, Tom Rini <trini at konsulko.com> wrote:
>
> On Tue, Oct 29, 2024 at 04:45:54PM +0100, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Mon, 28 Oct 2024 at 20:16, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > >
> > > On 10/25/24 11:56, Ilias Apalodimas wrote:
> > > > Hi Simon,
> > > >
> > > >
> > > > On Tue, 22 Oct 2024 at 15:00, Simon Glass <sjg at chromium.org> wrote:
> > > >>
> > > >> When the --native flag is given, pretend to be running the host
> > > >> architecture rather than sandbox.
> > > >>
> > > >> Add an 'efidebug filename' command to report it.
> > > >
> > > > Heinrich does this allow you to continue your testing in native archs?
> > > >
> > > >>
> > > >> Signed-off-by: Simon Glass <sjg at chromium.org>
> > > >> ---
> > > >>
> > > >> Changes in v8:
> > > >> - Add new patch to report host default-filename in native mode
> > > >>
> > > >>   boot/bootmeth_efi.c | 25 ++------------------
> > > >>   boot/efi_fname.c    | 57 +++++++++++++++++++++++++++++++++------------
> > > >>   cmd/efidebug.c      | 25 ++++++++++++++++++++
> > > >>   include/efi.h       | 25 ++++++++++++++++++++
> > > >>   4 files changed, 94 insertions(+), 38 deletions(-)
> > > >>
> > > >> diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
> > > >> index 371b36d550b..f836aa655f5 100644
> > > >> --- a/boot/bootmeth_efi.c
> > > >> +++ b/boot/bootmeth_efi.c
> > > >> @@ -25,32 +25,11 @@
> > > >>
> > > >>   #define EFI_DIRNAME    "/EFI/BOOT/"
> > > >>
> > > >> -static int get_efi_pxe_arch(void)
> > > >> -{
> > > >> -       /* http://www.iana.org/assignments/dhcpv6-parameters/dhcpv6-parameters.xml */
> > > >> -       if (IS_ENABLED(CONFIG_ARM64))
> > > >> -               return 0xb;
> > > >> -       else if (IS_ENABLED(CONFIG_ARM))
> > > >> -               return 0xa;
> > > >> -       else if (IS_ENABLED(CONFIG_X86_64))
> > > >> -               return 0x6;
> > > >> -       else if (IS_ENABLED(CONFIG_X86))
> > > >> -               return 0x7;
> > > >> -       else if (IS_ENABLED(CONFIG_ARCH_RV32I))
> > > >> -               return 0x19;
> > > >> -       else if (IS_ENABLED(CONFIG_ARCH_RV64I))
> > > >> -               return 0x1b;
> > > >> -       else if (IS_ENABLED(CONFIG_SANDBOX))
> > > >> -               return 0;       /* not used */
> > > >> -
> > > >> -       return -EINVAL;
> > > >> -}
> > > >> -
> > > >>   static int get_efi_pxe_vci(char *str, int max_len)
> > > >>   {
> > > >>          int ret;
> > > >>
> > > >> -       ret = get_efi_pxe_arch();
> > > >> +       ret = efi_get_pxe_arch();
> > > >>          if (ret < 0)
> > > >>                  return ret;
> > > >>
> > > >> @@ -239,7 +218,7 @@ static int distro_efi_read_bootflow_net(struct bootflow *bflow)
> > > >>          ret = get_efi_pxe_vci(str, sizeof(str));
> > > >>          if (ret)
> > > >>                  return log_msg_ret("vci", ret);
> > > >> -       ret = get_efi_pxe_arch();
> > > >> +       ret = efi_get_pxe_arch();
> > > >>          if (ret < 0)
> > > >>                  return log_msg_ret("arc", ret);
> > > >>          arch = ret;
> > > >> diff --git a/boot/efi_fname.c b/boot/efi_fname.c
> > > >> index a6b11383bba..790f9e2fa36 100644
> > > >> --- a/boot/efi_fname.c
> > > >> +++ b/boot/efi_fname.c
> > > >> @@ -9,29 +9,34 @@
> > > >>    */
> > > >>
> > > >>   #include <efi.h>
> > > >> +#include <errno.h>
> > > >>   #include <host_arch.h>
> > > >>
> > > >> -#ifdef CONFIG_SANDBOX
> > > >> -
> > > >>   #if HOST_ARCH == HOST_ARCH_X86_64
> > > >> -#define BOOTEFI_NAME "BOOTX64.EFI"
> > > >> +#define HOST_BOOTEFI_NAME "BOOTX64.EFI"
> > > >> +#define HOST_PXE_ARCH 0x6
> > > >>   #elif HOST_ARCH == HOST_ARCH_X86
> > > >> -#define BOOTEFI_NAME "BOOTIA32.EFI"
> > > >> +#define HOST_BOOTEFI_NAME "BOOTIA32.EFI"
> > > >> +#define HOST_PXE_ARCH 0x7
> > > >>   #elif HOST_ARCH == HOST_ARCH_AARCH64
> > > >> -#define BOOTEFI_NAME "BOOTAA64.EFI"
> > > >> +#define HOST_BOOTEFI_NAME "BOOTAA64.EFI"
> > > >> +#define HOST_PXE_ARCH 0xb
> > > >>   #elif HOST_ARCH == HOST_ARCH_ARM
> > > >> -#define BOOTEFI_NAME "BOOTARM.EFI"
> > > >> +#define HOST_BOOTEFI_NAME "BOOTARM.EFI"
> > > >> +#define HOST_PXE_ARCH 0xa
> > > >>   #elif HOST_ARCH == HOST_ARCH_RISCV32
> > > >> -#define BOOTEFI_NAME "BOOTRISCV32.EFI"
> > > >> +#define HOST_BOOTEFI_NAME "BOOTRISCV32.EFI"
> > > >> +#define HOST_PXE_ARCH 0x19
> > > >>   #elif HOST_ARCH == HOST_ARCH_RISCV64
> > > >> -#define BOOTEFI_NAME "BOOTRISCV64.EFI"
> > > >> +#define HOST_BOOTEFI_NAME "BOOTRISCV64.EFI"
> > > >> +#define HOST_PXE_ARCH 0x1b
> > > >>   #else
> > > >> -#error Unsupported UEFI architecture
> > > >> +#error Unsupported Host architecture
> > > >>   #endif
> > > >>
> > > >> -#else
> > > >> -
> > > >> -#if defined(CONFIG_ARM64)
> > > >> +#if defined(CONFIG_SANDBOX)
> > > >> +#define BOOTEFI_NAME "BOOTSBOX.EFI"
> > > >> +#elif defined(CONFIG_ARM64)
> > > >>   #define BOOTEFI_NAME "BOOTAA64.EFI"
> > > >>   #elif defined(CONFIG_ARM)
> > > >>   #define BOOTEFI_NAME "BOOTARM.EFI"
> > > >> @@ -47,9 +52,31 @@
> > > >>   #error Unsupported UEFI architecture
> > >  >>   #endif>>
> > > >> -#endif
> > > >> -
> > > >>   const char *efi_get_basename(void)
> > > >>   {
> > > >> -       return BOOTEFI_NAME;
> > > >> +       return efi_use_host_arch() ? HOST_BOOTEFI_NAME : BOOTEFI_NAME;
> > > >> +}
> > > >> +
> > > >> +int efi_get_pxe_arch(void)
> > > >> +{
> > > >> +       if (efi_use_host_arch())
> > > >> +               return HOST_PXE_ARCH;
> > > >> +
> > > >> +       /* http://www.iana.org/assignments/dhcpv6-parameters/dhcpv6-parameters.xml */
> > > >> +       if (IS_ENABLED(CONFIG_ARM64))
> > > >> +               return 0xb;
> > > >> +       else if (IS_ENABLED(CONFIG_ARM))
> > > >> +               return 0xa;
> > > >> +       else if (IS_ENABLED(CONFIG_X86_64))
> > > >> +               return 0x6;
> > > >> +       else if (IS_ENABLED(CONFIG_X86))
> > > >> +               return 0x7;
> > > >> +       else if (IS_ENABLED(CONFIG_ARCH_RV32I))
> > > >> +               return 0x19;
> > > >> +       else if (IS_ENABLED(CONFIG_ARCH_RV64I))
> > > >> +               return 0x1b;
> > > >> +       else if (IS_ENABLED(CONFIG_SANDBOX))
> > >
> > > Please, add a warning here:
> > >
> > >       log_warn("You are using the sandbox in non-compliant mode\n");
> > >
> > > The riscv64 sandbox can only run riscv64 binaries
> > > The arm64 sandbox can only run arm64 binary.
> > >
> > > We cannot expect a DHCP server to do the expected with the value 0.
> > >
> > > So why should we introduce this?
> > >
> > > >> +               return 0;       /* not used */
> > >
> > > The following comment would fit better:
> > >
> > >      /* This breaks PXE booting on the sandbox.*/
> >
> > This line is already in the code. I am just moving it to a new file.
> > PXE booting is not supported on sandbox. Neither is EFI booting, until
> > this series is applied.
>
> It sounds like EFI booting in sandbox is working and being tested and
> used (as a testing mechanism) right now. And the work and questions
> being raised here are around having your changes not break that existing
> use.

Well it would be, although at present that flow does not work, due to
a problem either with EDK2 or QEMU. It may be fixed at some point.

With this patch, the -N flag is needed to use this 'native' flow. The
flag is not needed for CI, which is what I consider to be the normal
'sandbox' case.

Regards,
Simon


More information about the U-Boot mailing list