-
Notifications
You must be signed in to change notification settings - Fork 39
RFC: Runtime Service Module Support #877
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This RFC proposes a roadmap for support of runtime service modules in Patina.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| resources from the core as well as boot services for allocations and other DXE | ||
| phase calls. | ||
|
|
||
| However, as a stepping stone to replacement RuntimeDxe should be implementation of a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| However, as a stepping stone to replacement RuntimeDxe should be implementation of a | |
| However, as a stepping stone to replacing RuntimeDxe, Patina should implement a |
|
|
||
| ### Refactoring Relocation Logic (optional) | ||
|
|
||
| The logic necessary to relocate the PE is currently split across multiple modules |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| The logic necessary to relocate the PE is currently split across multiple modules | |
| The logic necessary to relocate PE images is currently split across multiple modules |
|
|
||
| ### Patina Runtime (new) | ||
|
|
||
| A new module compiled as a Rust driver in an EDKII environment, provides a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this has been discussed, but from my point of view, I'm not sure how much value this adds. I know we have the Rust code that was originally part of Patina that could be used, but, as evidenced it already had design issues and would have to be refactored into an EDKII Rust driver, which is a model we have decided has major drawbacks. From my point of view, I would just keep to using EDKII's RuntimeDxe until we have a comprehensive solution in Patina, whether it is the dynamic linking approach or removal approach. But I worry that we promote a model we don't want to promote and that we open ourselves up for bugs in a place where we want to remove support entirely. As we've noted, Rust in a single driver may only add minimal improvement (particularly a heavily protocol reliant driver such as this) and it adds risk by introducing a new implementation. My two cents would be to not incur this risk for RuntimeDxe, it doesn't feel like a valuable investment of resources. Certainly open to other opinions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I've also been having this internal debate. There are a few benefits of using a rust runtime DXE.
- Even if using the C-ABI to the core, it provides a foundation to write key services that could be migrated to a more robust solution later.
- Further reduces the C code towards a goal of having a pure Patina solution (at some point)
That being said, it is an investment and whether those benefits are worth the cost is debatable.
@makubacki should weigh in on this as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RuntimeDxe is broken out from the C DXE Core (DXE Foundation) for the same reason we're having to break it out here. It is essentially "core" code than can't exist in the DXE Core binary image. If our goal is to write "core" code in a memory safe language, that would extend to services implemented behind EFI_RUNTIME_ARCH_PROTOCOL.
The other issue is torn state with Patina DXE Core. The Patina DXE Core produces Boot Services which includes GetMemoryMap. The EFI_RUNTIME_ARCH_PROTOCOL interface is:
struct _EFI_RUNTIME_ARCH_PROTOCOL {
EFI_LIST_ENTRY ImageHead; ///< A list of type EFI_RUNTIME_IMAGE_ENTRY.
EFI_LIST_ENTRY EventHead; ///< A list of type EFI_RUNTIME_EVENT_ENTRY.
UINTN MemoryDescriptorSize; ///< Size of a memory descriptor that is returned by GetMemoryMap().
UINT32 MemoryDesciptorVersion; ///< Version of a memory descriptor that is returned by GetMemoryMap().
UINTN MemoryMapSize; ///< Size of the memory map in bytes contained in MemoryMapPhysical and MemoryMapVirtual.
EFI_MEMORY_DESCRIPTOR *MemoryMapPhysical; ///< Pointer to a runtime buffer that contains a copy of
///< the memory map returned via GetMemoryMap().
EFI_MEMORY_DESCRIPTOR *MemoryMapVirtual; ///< Pointer to MemoryMapPhysical that is updated to virtual mode after SetVirtualAddressMap().
BOOLEAN VirtualMode; ///< Boolean that is TRUE if SetVirtualAddressMap() has been called.
BOOLEAN AtRuntime; ///< Boolean that is TRUE if ExitBootServices () has been called.
};Now, we have an interface produced in RuntimeDxe describing the properties of an interface implemented in the Patina DXE Core:
MemoryDescriptorSizeMemoryDesciptorVersionMemoryMapSize
Regardless of RuntimeDxe implementation details in edk2, it seems strange to leave this information open to an unknown EFI_RUNTIME_ARCH_PROTOCOL (of which might be RuntimeDxe).
That said, the runtime services logic is relatively minimal and I'd prefer to be written in a memory safe lanuage with full unit test coverage, the ability for integration level testing, and using the same underlying dependencies used by the Patina DXE Core for consistency.
We don't have a universal rule that separate .efi binaries are bad (at least while co-existing in today's PI platforms). When code can be built as a single binary and we can better leverage static safety mechanisms, we should do so, but I don't see it as extending beyond that. We simply can't do so here, right now. So, I don't see it in opposition to our stance for a single monolithic DXE Core binary, it's just the rationale doesn't apply here. Just like we'll need to build a separate monolithic MM Core binary becuse it doesn't apply there either.
All that said, for practical reasons, most people are going to use the well-known RuntimeDxe module and be fine. But, from a design perspective, I consider this an extension to the "DXE Foundation" and it is within the spirit of our work that the extension also have a Rust solution available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For what it's worth, we aren't actually filling any of the memory map fields today as they are not actually used. The only information we are providing is the image and event lists.
Overall, I agree that switching to a rust implementation of RuntimeDxe seems reasonable, and I think its fine to put as the point of record with this RFC, though I'm not concerned with this being accomplished in any immediate timeframe and the cost/reward seems limited.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
General thoughts on this (perhaps beyond this scope of the RFC): given that complete removal of runtime services (e.g. variable services/clock set, etc) is quite a way over the horizon, have we considered what our approach would be to enabling runtime services development in the monolithic component model in general?
I think for runtime you have a number of challenges that come to mind that are hard to do in the monolith: 1) you need to load some code into different pages (maybe in a completely separate space if memory map stability is important). I am not sure that clever tricks with the linker will get you far enough to do this well 2) You need to ensure that your runtime code doesn't call non-runtime code at runtime. This is hard to do in a monolithic compiled environment, because you have to be careful that the compiler doesn't choose to optimize that repeated "runtime" chunk of code by merging it with a "boot time" similar chunk of code. 3) runtime code essentially has to be no_alloc in addition to no_std since the allocators shutdown and the ownership of memory transitions to the OS at EBS.
Dynamic approaches (e.g. rust dynlib) would require loaders; as far as I can tell with some cursory research, these rely on underlying OS loading mechanisms which wouldn't exist in the patina context unless we built equivalents. If we have to build our own loaders, I'd submit that our existing PE/COFF loader + protocol data base are already sufficient to meet this need - which basically means building a driver separately from Patina which collapses to the model we have today.
Alternatively, you could just hoist the entire Patina monolith into runtime code pages so that everything in it is "runtime code". I think significant architecture work would need to be done to enforce a strong separation between runtime components and boot services (to include allocation, for example - no Vec expansions at runtime, even if they are allocated in runtime pages). It seems really challenging to me to enforce this separation in the monolith.
I think the monolith presents some really significant challenges to the runtime model, and IMHO, the best solution we have now is to leverage the existing "build it as a separate driver" model we have (and that is essentially what is promoted by this RFC). So maybe the answer, even in the long term to the question of "how do you enable this in the monolith" is "you don't'".
For many of the reasons you mentioned, my thought has been not to consider integration the current definition of Runtime Services into a non-RT binary and not to invest it significant infrastructure for a "extensible runtime environment". |
| core filled to re-relocate the images, virtualizing their execution, and call all | ||
| events registered to allow drivers/libraries to virtualize any stored pointers. | ||
|
|
||
| ## Alternatives |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may have missed it, but what is the current proposed solution versus listing of alternatives?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll make the plan more clear in the summary
Description
This RFC proposes a roadmap for support of runtime service modules in Patina.
How This Was Tested
N/A
Integration Instructions
N/A