Re: [edk2-devel] Memory Attribute for depex section

["Nhi Pham via groups.io" <nhi=os.amperecomputing.com@groups.io>]

Hi Laszlo,

On 1/16/2024 2:00 AM, Laszlo Ersek wrote:
> On 1/15/24 15:04, Ard Biesheuvel wrote:
>> On Mon, 15 Jan 2024 at 14:07, Nhi Pham <nhi@os.amperecomputing.com> wrote:
>>>
>>> On 1/12/2024 4:45 PM, Laszlo Ersek wrote:
>>>> (Independently: I think that's a valid thing to do for *SMM* drivers,
>>>> because the entry point functions of those drivers are permitted to use
>>>> both SMM and DXE/UEFI protocols. But whether the same is valid for the
>>>> *standalone* MM drivers -- that looks questionable. Standalone MM
>>>> drivers should not depend on UEFI/DXE protocols ever, IIUC.)
>>>>
>>>>> 3) The issue is patching the grammar in place, why can’t we just make a
>>>>> copy for the dispatcher grammer, and operate on the copy. Maybe via a
>>>>> copy on 1st update strategy?
>>>>
>>>> Yes, copying the depex to the heap, and patching it there, was Nhi's #1
>>>> fix proposal. I think that could be made work. But I'm not sure if the
>>>> perf savings are worth the additional complexity. The heap allocation
>>>> (where the writeable depex would exist) would have to be permanently
>>>> associated with the loaded PE image -- because the dispatcher might need
>>>> to reevaluate the depex across multiple rounds of dispatching. So that's
>>>> a new field in some image-related structure, it also needs to be freed
>>>> upon unload (?), what if the memory allocation fails during depex eval
>>>> (just consider the depex to eval to FALSE?), etc. Doable, but hairy; not
>>>> sure if the perf is worth that effort.
>>>>
>>>
>>> Thanks so much, Laszlo for your valuable insights.
>>>
>>> The approach #1 works for me. I will do further check for your concerns
>>> above.
>>>
>>> I'm trying your suggested patch and investigating the performance being
>>> discussed here.
>>>
>>
>> Not sure what approach #1 means,
> 
> (copying the depex to the heap, and maintaining it there, so that it can
> be patched)

Thanks!

> 
>> but I'd prefer to just remove this
>> optimization from standalone MM, given that not only a) it shouldn't
>> have to deal with a large number of protocol GUIDs, but also b) the
>> driver dispatch is much more straight-forward. (Typically, StMM
>> drivers can be dispatched in the order they appear in the firmware
>> volume, in which case each DEPEX is evaluated only once anyway)
> 
> Sounds like a promising basis for removing the optimization indeed!
> 

Your patch suggested earlier works for me. And I don't see significant 
performance reduction compared with keeping optimization.

I don't have strong reason on removing the optimization, but I think it 
would be simply good for now. Could you post your patch to edk2-devel 
for review and merge?

Thanks,
Nhi


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113986): https://edk2.groups.io/g/devel/message/113986
Mute This Topic: https://groups.io/mt/103594587/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-