[PATCH v4 04/25] efi_loader: Add comments where incorrect addresses are used

[Simon Glass]

Hi Ilias,

On Tue, 3 Dec 2024 at 01:49, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> Hi Simon,
>
> On Tue, 3 Dec 2024 at 02:22, Simon Glass <sjg at chromium.org> wrote:
> >
> > Hi Tom,
> >
> > On Mon, 2 Dec 2024 at 13:18, Tom Rini <trini at konsulko.com> wrote:
> > >
> > > On Sun, Dec 01, 2024 at 08:24:23AM -0700, Simon Glass wrote:
> > >
> > > > Some functions are passing addresses instead of pointers to the
> > > > efi_add_memory_map() function. This confusion is understandable since
> > > > the function arguments indicate an address.
> > > >
> > > > Make a note of the 8 places where there are problems, which would break
> > > > usage in sandbox tests.
> > > >
> > > > Future work will resolve these problems.
> > > >
> > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > >
> > > Please just resolve these rather than introducing a patch to then fix
> > > them later. This is something that should have been fixup'd before
> > > posting. Thanks.
> >
> > That was deliberate, as I wanted people to see the problems. It will
> > save discussion on later patches where the problems are fixed, if we
> > can agree that these are actual problems. If people are happy to add
> > review tags to the later patches then I'm happy to redo it.
>
> I am pretty sure Heinrich has repeated this in the past. Why do we
> have to sprinkle around map_sysmem/unmap sysmem for sandbox?
> Polluting the entire u-boot to support a special platform is less than
> ideal. Why can't sandbox limit this internally and do whatever
> mappings it needs when it receives an address?

Tom, I suppose I have made my point.

Ilias, this is documented at [1] and has been the same for 10 years.

See for example the 'md' command, do_mem_md(), which shows how 'md 0'
is implemented in sandbox.

As I have mentioned before, the nice thing is that you can easily make
code work with sandbox just by converting casts from
address-to-pointer into map_sysmem().

Back to this series, if you look at efi_allocate_pages() you'll
currently see three calls (two map_to_sysmem() one map_sysmem(), but
now, with this series, there is none. It also makes it a lot easier to
understand what is going on, IMO.

Regards,
Simon

[1] https://docs.u-boot.org/en/latest/arch/sandbox/sandbox.html#memory-emulation