[PATCH v8 5/8] sandbox: Report host default-filename in native mode
Tom Rini
trini at konsulko.com
Wed Oct 30 16:04:03 CET 2024
On Wed, Oct 30, 2024 at 08:44:30AM +0100, Simon Glass wrote:
> 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.
Since it sounds like Heinrich is testing shim+grub binaries currently,
I'm not sure what you mean here.
> 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.
Well, unfortunately your code has escaped in to the wild and is being
used by other people now. For your still unclear what we need it for use
case, you should be invoking the magic logic you need, not the other way
around.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20241030/2167e293/attachment.sig>
More information about the U-Boot
mailing list