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

["Laszlo Ersek" <lersek@redhat.com>]

On 1/12/24 03:44, Andrew (EFI) Fish wrote:
> Sorry need some more time to digest this…. First thoughts.
> 
> 1) The actual performance issue we hit was the explosion
> of CoreValidateHandle() calls as the number of protocols got large for
> some diags. The newer handles tended to be at the end of the list if I
> remember correctly. 
>   a) It looks like CoreValidateHandle() is the only
> place  *gHandleList* was walked, as the handle info is crossed
> referenced in the protocol database. So that is why we changed that.
> 2) If the issue at hand is MM why not just drop the optimizations?

Yes, that's the first (and easy) idea. But I guess it needs some
measurements (no perf regression). It would be nice to know whether
Intel (?) measured any serious perf gains when they originally
implemented (in the 2000s?) the optimization.

>   b) If we have so many MM protocols and handles that seems like its own
> problem? 

I would agree; however, IIRC, the depex evaluator for the MM drivers
also searches the UEFI (DXE) protocol database, not just the MM protocol
database.

(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!
Laszlo



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