[PATCH 02/16] lmb: add a flag to allow suppressing memory map change notification

[Sughosh Ganu]

On Thu, 19 Sept 2024 at 19:42, Simon Glass <sjg at chromium.org> wrote:
>
> Hi,
>
> On Tue, 17 Sept 2024 at 13:55, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> >
> > On Sat, 14 Sept 2024 at 20:14, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > >
> > > On 9/5/24 10:27, Sughosh Ganu wrote:
> > > > Add a flag LMB_NONOTIFY that can be passed to the LMB API's for
> > > > reserving memory. This will then result in no notification being sent
> > > > from the LMB module for the changes to the LMB's memory map.
> > >
> > > You seem to be using this in patch 3 and 7.
> > >
> > > Please, describe in this patch why you want to be able to suppress
> > > notification.
> >
> > Will add the reasoning behind this flag in the commit message.
> >
> > >
> > > In the EFI context we should use LMB notification to notify the
> > > EFI_EVENT_GROUP_MEMORY_MAP_CHANGE event.
> > >
> > > See chapter 7.1.2 EFI_BOOT_SERVICES.CreateEventEx() in the UEFI
> > > specification.
> >
> > So, do you want me to use the EFI event signaling mechanism for this
> > purpose ? Is my understanding correct ? If so, this will mean that we
> > have an event notification specifically for EFI, and there might be
> > one needed for any other consumers of this event. Currently there
> > aren't any other consumers of the LMB memory map change event other
> > than EFI, but using the U-Boot's event notification mechanism means
> > that the same notification mechanism can be used if there were any
> > additional consumers of this event in the future. In that case, we
> > would have two separate event notifications, one for EFI, and one for
> > non-EFI consumers.
>
> As I have previously said, none of this is necessary.
>
> Essentially all of the EFI setup that is done in U-Boot can be delayed
> until we are actually starting an EFI app. The current approach of
> keeping parallel EFI tables everywhere is causing much confusion.
>
> For EFI, it should be enough to read the lmb tables at the end and add
> whatever parallel tables are needed to boot the app. We should not
> need to keep things in sync through the life of U-Boot, since:
>
> 1. EFI pool-allocations should use malloc() until the app starts
> 2. EFI page allocations should not be allowed until the app starts
>
> This whole area needs a healthy dose of 'keep it simple'.

Except, the current implementation proposed by the patch *is* actually
much more simpler than syncing the maps when the app starts. Because
there is another scenario when the map needs to be synced, when the
user issues a 'efidebug memmap' command, which dumps the efi memory
map.

Syncing the map from lmb means that the efi memory map first has to be
generated afresh. And that is much more complicated than the method
incorporated in the patch series. Apart from the ram/conventional
memory type, we also need to include other memory types that might
have been added to the memory map. So, this would mean, create a new
memory map, add entries for conventional memory as have been obtained
from lmb followed by other entries, and then discard the old memory
map. And this has to be carried out every time a need for a memory map
update is needed.

-sughosh

>
> Regards,
> Simon