Re: [PATCH v6 06/12] efi: Use the same filename for all sandbox builds

[Heinrich Schuchardt]

Am 18. Oktober 2024 17:02:47 MESZ schrieb Simon Glass <sjg at chromium.org>:
>Hi Heinrich,
>
>On Thu, 17 Oct 2024 at 21:45, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>
>>
>>
>> Am 18. Oktober 2024 05:05:08 MESZ schrieb Simon Glass <sjg at chromium.org>:
>> >Hi Tom,
>> >
>> >On Thu, 17 Oct 2024 at 18:17, Tom Rini <trini at konsulko.com> wrote:
>> >>
>> >> On Thu, Oct 17, 2024 at 05:23:19PM -0600, Simon Glass wrote:
>> >> > Hi Heinrich,
>> >> >
>> >> > On Mon, 30 Sept 2024 at 17:23, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>> >> > >
>> >> > > On 26.09.24 23:59, 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.
>> >> > > >
>> >> > > > Drop the confusing use of host architecture and just let sandbox be
>> >> > > > sandbox.
>> >> > >
>> >> > > As I already wrote in
>> >> > > https://lore.kernel.org/u-boot/ae1cf1fa-766e-46a0-8ef9-2c2c7af73d9e@gmx.de/
>> >> > > this patch should not be merged.
>> >> > >
>> >> > > bootsbx.efi does not exist in the UEFI specification.
>> >> >
>> >> > If that is of concern I can get it added. Let me know.
>> >> >
>> >> > >
>> >> > > Without this patch I can test that shim work and grub are correctly
>> >> > > loaded from a distro image. This patch makes the sandbox misbehave.
>> >> >
>> >> > Why don't you do that with QEMU?
>> >> >
>> >> > If we want sandbox to do this, I could add a way for sandbox to select
>> >> > its architecture. But this patch is correct, sorry. It is basically a
>> >> > revert of your patch:
>> >> >
>> >> > 3a0654ecd0d efi_loader: correctly identify binary name
>> >> >
>> >> > Let me know if you would like a selection mechanism and I'll see what I can do.
>> >>
>> >> I'm confused. It sounds like Heinrich is using sandbox to test on
>> >> various hosts, how U-Boot behaves, without relying on "just test in
>> >> QEMU". I don't see how this disconnects from your usual request on how
>> >> to use sandbox for further testing. What is breaking for you, Simon, by
>> >> us saying that the host instruction set determines what we load and run
>> >> (and so, is portable whereas saying "sandbox" is not, between x86 hosts,
>> >> arm64 hosts and riscv64 hosts).
>> >
>> >This:
>> >
>> >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.
>>
>> As I wrote before:
>>
>> The UEFI specification prescribes the naming of the default boot binary. Deviating from the specification should be avoided.
>
>As I said before, I'm happy to get this added to the spec, if it
>helps. It is a real use case.

Do you really think that the UEFI organization should care about your test preference? This is just absurd.

As tests should run on all devices including but not restricted to the sandbox we have to support different names for the default file anyway.

This patch has no virtue in this context.

Best regards

Heinrich

>
>>
>> Anyway you need different images per host architecture. Writing a single switch statement to derive the default EFI binary name from the OS architecture does not look like an undue burden.
>
>Yes, it is easy, but it is taking sandbox in a strange direction. It
>becomes a hybrid of a host architecture and a U-Boot architecture,
>which is then going to lead to all sorts of strange conversations. At
>present it is pretty clear. So I want to head this off at the pass.
>
>I did not get a chance to review the original patch, of which this is
>essentially a revert. It has actually bitten me in a few areas.
>
>>
>> The sandbox should remain able to start GRUB from a distro image for the host architecture.
>>
>> The sandbox is not a fake architecture but can be considered a valid testbed for host native EFI binaries including GRUB, the kernel stub, the stubs of UKI images, and the EFI SCT.
>
>I have to say that I wrote sandbox and I know what it is for. It
>wasn't designed to run binaries built outside of U-Boot, or anything
>of the sort. That said, as I mentioned, I should be able to make
>something work here.
>
>To help me come up with a solution, can you please send your workflow
>/ commands for the testing you do? Also, what architectures do you do
>this with?
>
>My approach to U-Boot is that code needs to land, one way or another,
>perhaps with some changes for the use case that is thrown up, or
>perhaps done another way. But please try to consider this approach to
>Open Source too.
>
>Regards,
>Simon
>
>
>>
>> Best regards
>>
>> Heinrich
>>
>>
>> >
>> >The idea of using sandbox to run native EFI apps is new to me, but it
>> >sounds great, if it will get us more tests in this area.
>> >
>> >But sandbox is sandbox. It is not (x86, arm, riscv). It is a fake
>> >architecture that we use in U-Boot. I've offered to create a way to
>> >get the behaviour that Heinrich wants, so let's see what he says.
>
>Regards,
>SImon