codecov-commenter
commented
2 months ago Codecov Report
Attention: Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.
Please upload report for BASE (
release/202405@63fc132). Learn more about missing BASE report.
| Files | Patch % | Lines |
|---|---|---|
| MdeModulePkg/Core/Dxe/Mem/Page.c | 0.00% | 5 Missing :warning: |
Additional details and impacted files
```diff @@ Coverage Diff @@ ## release/202405 #1095 +/- ## ================================================= Coverage ? 0.68% ================================================= Files ? 643 Lines ? 219670 Branches ? 1356 ================================================= Hits ? 1502 Misses ? 218121 Partials ? 47 ``` | [Flag](https://app.codecov.io/gh/microsoft/mu_basecore/pull/1095/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=microsoft) | Coverage Δ | | |---|---|---| | [MdeModulePkg](https://app.codecov.io/gh/microsoft/mu_basecore/pull/1095/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=microsoft) | `0.68% <0.00%> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=microsoft#carryforward-flags-in-the-pull-request-comment) to find out more.:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Description
This reverts commit 76f8060e3a61b52b5f874953a90d1cfaae5d9979.
This commit was from an earlier era of Project Mu where we were not as strict with memory protections. Now that we are more strict, these are important prints (and failures if alignment of a RUNTIME entry is wrong). This was confirmed by booting on Q35 and analyzing the few prints that showed up.
The first was that Rust code was not being linked as we expected, that is fixed here for pure Rust repos: https://github.com/microsoft/mu_devops/pull/356 and here for mu_basecore based repos: https://github.com/microsoft/mu_basecore/pull/1098. This may get fixed in a different way that is more broad. The in-depth issue was that /BASE:0 was not getting set for the Rust linker. It is failing to allocate pages when loading the image. The reason is that the ImageBase field in the PE/COFF optional header is set, so we try to load at that address: https://github.com/microsoft/mu_basecore/blob/45e4430db9d4a27a25201f51f91705fd7c4a0374/MdeModulePkg/Core/Dxe/Image/Image.c#L730-L748 (ImageAddress gets populated from ImageBase: https://github.com/microsoft/mu_basecore/blob/45e4430db9d4a27a25201f51f91705fd7c4a0374/MdePkg/Library/BasePeCoffLib/BasePeCoff.c#L619-L629).
Per the PE/COFF spec, this is only intended for use by Windows and I don't think we should really be honoring it, but probably things are relying on it:
UefiHidDxe and HelloWorldRustDxe are the only EFI binaries in the mu_tiano_platforms build that set ImageBase. For everything else it is 0, so we don't enter that codepath (what happens in that codepath is the ImageBase provided, 0x140000000 for both modules, is where we attempt to load the modules to, which isn't in our memory tree (or one of those two is already loaded there) and so we fail).Setting /BASE:0 causes MSVC to set the ImageBase to 0 (in ElfConvert we always set the ImageBase to 0: https://github.com/microsoft/mu_basecore/blob/45e4430db9d4a27a25201f51f91705fd7c4a0374/BaseTools/Source/C/GenFw/Elf64Convert.c#L1180).
The other set of failures was from gDS->SetMemorySpaceCapabilities() trying to set attributes in gMemoryMap on GCD memory types that don't exist there and an edk2 PR has been put up to fix that: https://github.com/tianocore/edk2/pull/6062.
There are no other failures seen and when these failures do occur, we want to debug them. Closes #1091.
For each item, place an "x" in between
[and]if true. Example:[x]. (you can also check items in the GitHub UI)How This Was Tested
Tested on Q35.
Integration Instructions
N/A.