[PATCH 04/16] event: add event to notify lmb memory map changes

[Heinrich Schuchardt]

On 26.09.24 14:58, Ilias Apalodimas wrote:
> Hi Heinrich,
>
> On Sat, 14 Sept 2024 at 18:08, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>
>> On 9/5/24 10:27, Sughosh Ganu wrote:
>>> Add an event which would be used for notifying changes in the
>>> LMB modules' memory map. This is to be used for having a
>>> synchronous view of the memory that is currently in use, and that is
>>> available for allocations.
>>
>> The synchronous view problem only exists because we are duplicating
>> data. Store the EFI memory type in LMB and the problem vanishes.
>
> We could, but I wonder if that would be enough.
> Apart from the EFI memory types, we plan on fixing the EFI memory
> attributes. I'll go read the spec again and take look at EDK2 but
> IIRC, you can't correlate memory types to memory attributes (at least
> not for all of them).
>
> If that's true we'll also need the attributes in LMB + perhaps logic
> for 64kb page size OSes and 4KB page firmware, which might be a bit
> too much. In that case, I think the sync is fine, but I really don't
> see the point of making an event out of it. Perhaps we could just
> update the efi memory map directly from LMB.

Yes, memory attributes should be stored in LMB too if UEFI is enabled.

I would suggest:

* GetMemoryMap() calls into LMB to generate the map on the fly.
* The EFI_MEMORY_ATTRIBUTES_TABLE is generated from LMB information.
* The EFI_MEMORY_ATTRIBUTE_PROTOCOL calls into LMB.

Best regards

Heinrich

>
> Thanks
> /Ilias
>>
>> The event is only needed to notify EFI_EVENT_GROUP_MEMORY_MAP_CHANGE.
>>
>> Best regards
>>
>> Heinrich
>>
>>>
>>> Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
>>> ---
>>>    common/event.c  |  2 ++
>>>    include/event.h | 14 ++++++++++++++
>>>    2 files changed, 16 insertions(+)
>>>
>>> diff --git a/common/event.c b/common/event.c
>>> index dda569d447..fc8002603c 100644
>>> --- a/common/event.c
>>> +++ b/common/event.c
>>> @@ -48,6 +48,8 @@ const char *const type_name[] = {
>>>
>>>        /* main loop events */
>>>        "main_loop",
>>> +
>>> +     "lmb_map_update",
>>>    };
>>>
>>>    _Static_assert(ARRAY_SIZE(type_name) == EVT_COUNT, "event type_name size");
>>> diff --git a/include/event.h b/include/event.h
>>> index fb353ad623..fce7e96170 100644
>>> --- a/include/event.h
>>> +++ b/include/event.h
>>> @@ -153,6 +153,14 @@ enum event_t {
>>>         */
>>>        EVT_MAIN_LOOP,
>>>
>>> +     /**
>>> +      * @EVT_LMB_MAP_UPDATE:
>>> +      * This event is triggered on an update to the LMB reserved memory
>>> +      * region. This can be used to notify about any LMB memory allocation
>>> +      * or freeing of memory having occurred.
>>> +      */
>>> +     EVT_LMB_MAP_UPDATE,
>>> +
>>>        /**
>>>         * @EVT_COUNT:
>>>         * This constants holds the maximum event number + 1 and is used when
>>> @@ -203,6 +211,12 @@ union event_data {
>>>                oftree tree;
>>>                struct bootm_headers *images;
>>>        } ft_fixup;
>>> +
>>> +     struct event_lmb_map_update {
>>> +             u64 base;
>>> +             u64 size;
>>> +             u8 op;
>>> +     } lmb_map;
>>>    };
>>>
>>>    /**
>>