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 <n...@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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
