Hi Heinrich, On Thu, 12 Sept 2024 at 08:08, Heinrich Schuchardt <xypron.g...@gmx.de> wrote: > > On 02.09.24 03:18, Simon Glass wrote: > > Sandbox is not a real architecture, but within U-Boot it is real enough. > > We should not need to pretend it is x86 or ARM anywhere in the code. > > > > Also we want to be able to locate the sandbox app using a single > > filename, 'bootsbox.efi', to avoid needing tests to produce different > > files on each host architecture. > > The changed include is not meant to set anything for the sandbox app > which you run on top of an UEFI firmware. The sandbox app probably > should be called depending on the architecture sandboxaa64.efi, > sandobxriscv64.efi, etc. > > This include is defining which file the EFI boot manager loads by > default. This value must conform to the EFI specification. When I have > built the sandbox on riscv64 I expect that it will load bootriscv64.efi > from a distro installer image.
Perhaps we can support something like that later, but sandbox is mostly for testing and development, so I don't see a lot of benefit. It just makes things confusing. As I mentioned in the commit message, sandbox is an architecture within U-Boot (see arch/sandbox). In the early day we started adding host pass-through features but they didn't end up being very useful. If you want to boot a riscv64 image, you should do that on suitable board. Of course the actual code in the bootsbox.efi file will match the host, but that is actually the point of sandbox! So I don't agree and I believe that the original change which split these out is wrong. Ilias requested that I drop the Fixes tag, but the offending commit is here: 3a0654ecd0d efi_loader: correctly identify binary name I am not sure what this has to do with reproducible builds and the commit was silent on that point. Regards, SImon > > Best regards > > Heinrich > > > > > Drop the confusing use of host architecture and just let sandbox be > > sandbox. > > > > Signed-off-by: Simon Glass <s...@chromium.org> > > Reviewed-by: Ilias Apalodimas <ilias.apalodi...@linaro.org> > > --- > > > > Changes in v5: > > - Drop the Fixes tag > > > > Changes in v3: > > - Put back the Linaro copyright accidentally removed > > > > include/efi_default_filename.h | 24 ++---------------------- > > 1 file changed, 2 insertions(+), 22 deletions(-) > > > > diff --git a/include/efi_default_filename.h b/include/efi_default_filename.h > > index 77932984b55..06ca8735002 100644 > > --- a/include/efi_default_filename.h > > +++ b/include/efi_default_filename.h > > @@ -16,26 +16,8 @@ > > #undef BOOTEFI_NAME > > > > #ifdef CONFIG_SANDBOX > > - > > -#if HOST_ARCH == HOST_ARCH_X86_64 > > -#define BOOTEFI_NAME "BOOTX64.EFI" > > -#elif HOST_ARCH == HOST_ARCH_X86 > > -#define BOOTEFI_NAME "BOOTIA32.EFI" > > -#elif HOST_ARCH == HOST_ARCH_AARCH64 > > -#define BOOTEFI_NAME "BOOTAA64.EFI" > > -#elif HOST_ARCH == HOST_ARCH_ARM > > -#define BOOTEFI_NAME "BOOTARM.EFI" > > -#elif HOST_ARCH == HOST_ARCH_RISCV32 > > -#define BOOTEFI_NAME "BOOTRISCV32.EFI" > > -#elif HOST_ARCH == HOST_ARCH_RISCV64 > > -#define BOOTEFI_NAME "BOOTRISCV64.EFI" > > -#else > > -#error Unsupported UEFI architecture > > -#endif > > - > > -#else > > - > > -#if defined(CONFIG_ARM64) > > +#define BOOTEFI_NAME "BOOTSBOX.EFI" > > +#elif defined(CONFIG_ARM64) > > #define BOOTEFI_NAME "BOOTAA64.EFI" > > #elif defined(CONFIG_ARM) > > #define BOOTEFI_NAME "BOOTARM.EFI" > > @@ -52,5 +34,3 @@ > > #endif > > > > #endif > > - > > -#endif >
