Hi Sughosh, On Mon, 15 Jul 2024 at 10:39, Sughosh Ganu <sughosh.g...@linaro.org> wrote: > > hi Simon, > > On Sat, 13 Jul 2024 at 20:46, Simon Glass <s...@chromium.org> wrote: > > > > Hi Sughosh, > > > > On Thu, 4 Jul 2024 at 08:38, Sughosh Ganu <sughosh.g...@linaro.org> wrote: > > > > > > There are events that would be used to notify other interested modules > > > of any changes in available and occupied memory. This would happen > > > when a module allocates or reserves memory, or frees up memory. These > > > changes in memory map should be notified to other interested modules > > > so that the allocated memory does not get overwritten. Add an event > > > handler in the EFI memory module to update the EFI memory map > > > accordingly when such changes happen. As a consequence, any subsequent > > > memory request would honour the updated memory map and only available > > > memory would be allocated from. > > > > > > Signed-off-by: Sughosh Ganu <sughosh.g...@linaro.org> > > > --- > > > Changes since V1: > > > * Handle the addition of memory to the LMB memory map. > > > * Pass the overlap_only_ram parameter to the efi_add_memory_map_pg() > > > based on the type of operation. > > > > > > lib/efi_loader/Kconfig | 1 + > > > lib/efi_loader/efi_memory.c | 34 ++++++++++++++++++++++++++++++++++ > > > 2 files changed, 35 insertions(+) > > > > > > > This is getting complicated and I don't believe it is needed. > > > > EFI should not be allocating memory 'in free space' until it starts > > up. For the very few (if any) cases where it does, it can do an lmb > > allocation. > > EFI memory module is not allocating memory at all now. This patch is > adding an event handler for updating the EFI memory map, whenever the > LMB memory map changes. All the EFI allocations are now being routed > through the LMB API's.
OK > > > > > As to the lmb allocations themselves, EFI can simply call look through > > the lmb list and call efi_add_memory_map_pg() for each entry, when it > > is ready to boot. There is no need to do it earlier. > > So in this case, I believe that rather than adding code in multiple > places where the EFI memory module would have to get the LMB map and > then update it's own, I think it is easier to update the EFI memory > map as and when the LMB map gets updated. Else, we have a scenario > where the EFI memory map would have to be updated as part of the EFI > memory map dump function, as well as before the EFI boot. Any new code > that would be subsequently introduced that might have a similar > requirement would then be needed to keep this point in mind(to get the > memory map from LMB). That doesn't hold water in my eyes. I actually like the idea of the EFI memory map being set up before booting. It should be done in a single function called from one place, just before booting. Well, I suppose it could be called from the memory-map-dump function too. But it should be pretty simple...just add some pre-defined things and then add the lmb records. You can even write a unit test for it. > > This is not an OS file-system kind of an operation where performance > is critical, nor is this event(LMB memory map update) going to happen > very frequently. So I believe that it would be better to keep the EFI > memory map updated along with the LMB one. I really don't like that idea at all. One table is enough for use by U-Boot. The EFI one is needed for booting. Keeping them in sync as U-Boot is running is not necessary, just invites bugs and makes the whole thing harder to test. Regards, Simon
