Fast forward UEFI from closed source.#52
Closed
jaykrell wants to merge 28 commits intomicrosoft:mainfrom
Closed
Conversation
jaykrell
changed the title
jaykrell
force-pushed
the
mu_327_jay2
branch
2 times, most recently
from
c28d321 to
48339ac
Compare
This PR adds the AIP protocol to SnpDxe. Previously, we took a 202502 submodule update, which modifies `DxeNetLib`'s `NetLibDetectMediaWaitTimeout()` function to poll instead of returning EFI_NO_MEDIA when the AIP protocol is absent. This causes Gen2 VMs with an unplugged NIC to hang in boot for a while. By implementing the AIP protocol, whenever our NIC is unplugged, we can immediately reflect `MediaState = EFI_NO_MEDIA` to achieve what upstream code used to do. ---- #### AI description (iteration 1) #### PR Classification This PR introduces a new feature by adding support for the Adapter Information Protocol (AIP) to the SnpDxe driver, enhancing media state reporting to address a UEFI regression. #### PR Summary The changes integrate the AIP protocol into the NetvscDxe driver to quickly assess network media status and streamline protocol management. - **`MsvmPkg/NetvscDxe/AdapterInformation.c`**: New file implementing AIP functions (GetInformation, SetInformation, GetSupportedTypes) for media state reporting. - **`MsvmPkg/NetvscDxe/Snp.h`**: Added AIP function declarations and a macro to retrieve the SNP driver from the AIP instance. - **`MsvmPkg/NetvscDxe/Snp.c`**: Modified to install/uninstall the AIP protocol alongside the SNP protocol and initialize its handlers. - **`MsvmPkg/NetvscDxe/NetvscDxe.inf`**: Updated to produce the AIP protocol and include the new file. <!-- GitOpsUserAgent=GitOps.Apps.Server.pullrequestcopilot --> Related work items: #61421898
Fix debug build for AziHsm
This PR fixes what was missed in this PR: https://dev.azure.com/microsoft/OS/_git/hyperv.uefi/pullrequest/10689820?path=/MsvmPkg/MsvmPkgX64.fdf&version=GBofficial/main&line=33&lineEnd=34&lineStartColumn=1&lineEndColumn=1&type=2&lineStyle=plain&_a=files&iteration=11&base=0 That is, while we split the DXE FV apart from Main FV, we forgot to skip measuring it. This results in 38 new measurements going to PCR2. This PR adds three things: - Ensure that ExcludeFvsFromMeasurementLib includes DXE FV. But again, by default we don't want to exclude measuring this. We want to make sure MainFV and DxeFV are in PCR0, so that we don't get any drivers from _our_ platform into PCR2 - Issue a `PeiServicesInstallFvInfoPpi` to make `Tcg2Pei` aware of the DXE FV. This was the key thing missing - Add the SHA384 library to ARM64 + remove SHA1 - Rename the Exclude PCD Attached are raw boot measurements for arm64 and amd64 to prove that this fix works: [raw-boot-measurement-amd64.txt](https://microsoft.visualstudio.com/8d47e068-03c8-4cdc-aa9b-fc6929290322/_apis/git/repositories/1ad2e189-11bd-479f-8f25-a739cc641dfa/pullRequests/15013511/attachments/raw-boot-measurement-amd64.txt) [raw-boot-measurement-arm64.txt](https://microsoft.visualstudio.com/8d47e068-03c8-4cdc-aa9b-fc6929290322/_apis/git/repositories/1ad2e189-11bd-479f-8f25-a739cc641dfa/pullRequests/15013511/attachments/raw-boot-measurement-arm64.txt) Related work items: #61409673
Otherwise fails to link to memcpy.
We have identical BdTripleFault and TripleFault. They are the same. We do not need both. Delete BdTripleFault and keep TripleFault. Use mnemonic instead of db for ud2. Change the declaration to EFIAPI. EFIAPI is important for assembly functions when using gcc, where the default calling convention is not EFIAPI, but we shall only have EFIAPI for assembly functions. EFIAPI is also needed for protocols, if we interoperate with binaries from other builds, such as RuntimeServices called by OS. Otherwise, arguably, EFIAPI can vary, i.e. in a consistent build. Tested via disassembly both before and one after. Related work items: #57987809
I will add unwind data later. It has always been missing. Related work items: #57987809
…cpy. (AZIHSM_DDI_API_REV) Also remove toplevel const in parameters. It has a meaning, it can be useful, people do it on purpose. But usually it is an accident. Related work items: #57987809
Gcc tends to warn, if an inline function "maybe" not inlinable. We can fix this by making ALWAYS_INLINE always empty for gcc. Or pragma off the warning. But, in this PR, just..well, I went back and forth. The best approach, really is to move the code out of .h to .c. Let the compiler decide. We do use LTCG. inline is no longer useful in non-template code, assuming single build variant. But that is an unreadable diff. AI does not assist. Maybe do it anyway. It has the advantage of being C89. Another approach is to #if the inline code and include it only once, without inline. In this PR, use "statiic inline" in .h, and nothing in .c. Related work items: #57987809
Use ZeroMem instead.
```
#define INIT {...}
typedef struct A { ... } A;
A a = (A)INIT;
```
is nice, but kinda not allowed in EFI.
Instead:
```
#define INIT {...}
typedef struct A { ... } A;
static A s = INIT;
A a;
CopyMem(&a, &s, sizeof(a));
```
or `ASSIGN_STRUCT(&a, &s)`
In this case, if we instantiate more variations, we could avoid the copy.
It requires a bit of a trick though, with designated initializers, in order to not break the `INIT` abstraction.
Like this:
```
typedef struct WrapA { A a; } WrapA;
static WrapA s {.a = INIT, .a.foo = 1};
```
The copying in this case is pretty small. I think it is OK.
Related work items: #57987809
…emset. Compilers are prone to output calls to memset and memcpy. Gcc is a little more prone, but Clang is a "problem" too. (Not clearly a problem. Compiler writers are good candidates for writing these functions.) Therefore they have been provided. Use them. It is easy to eliminate most of the calls, but difficult to eliminate them all. This presents the dilema then to eliminate some or not. In some cases, the copy or set is only made explicit, not removed. And other times I noticed excessive copying, easiiy removed. Given that presently upstream AMD64 is broken, and upstream AMD64 community does not like using this library by default, the present approach is a hybrid. Eliminate what we can. Use the library when we can. Hope that covers everything. Which it does seem to. Also submit upstream patches. Related work items: #57987809
Related work items: #57987809
…dLogger During SEC phase, the Advanced Logger writes a small pointer structure to the address specified by this PCD. The original address (0xFA000000) sits in the MMIO gap, which is not backed by real RAM. On normal VMs this works because the hypervisor/paravisor maps the access. On hardware-isolated VMs without a paravisor (TDX or SNP "NoManagementVtl"), guest memory pages must be explicitly accepted before use, and SEC has no ability to do that. Writing to an unaccepted address causes a fatal fault (#VE on TDX, #VC on SNP) that SEC cannot handle, crashing the VM at boot. Setting this to 0 makes the logger skip the write in SEC. PEI Core detects the 0 and allocates its own log buffer from accepted RAM, so logging still works from PEI onward. NOTE: I still need to test this on a regular test machine to ensure EfiDiagnostics is not broken ---- #### AI description (iteration 1) #### PR Classification Bug fix aimed at preventing SEC phase memory faults on NoManagementVtl CVMs. #### PR Summary This pull request disables the SEC pre-memory logger by setting `PcdAdvancedLoggerBase` to 0, ensuring that logging skips the problematic MMIO region during SEC and uses accepted RAM in PEI. - `MsvmPkg/MsvmPkgX64.dsc`: Updated the Advanced Logger configuration by changing `PcdAdvancedLoggerBase` from 0xFA000000 to 0 with detailed comments explaining the rationale. <!-- GitOpsUserAgent=GitOps.Apps.Server.pullrequestcopilot --> Related work items: #53782343
Related work items: #57987809
Light handed alternative to !15020076
We forked around 2016 or earlier.
In 2019 and 2020 mErrorCodeFlag got two more bits.
Gcc/clang fail to link due to the duplicate symbols.
Remove ours in C and link to upstream/fork.
Update ours in assembly.
Here is the history of the two bits:
Ours:
```
commit ee188cdbb20c13f2361192fa1f07192eea8835ef
Author: Larry Cleeton <lcleeton@ntdev.microsoft.com>
Date: Wed Jan 13 12:50:27 2016 -0800
checkpoint - tries to load a boot loader!
```
Upstream:
```
commit 5277540e37e88a1a69f9517c4ad895051b4b3ed3
Author: Tom Lendacky <thomas.lendacky@amd.com>
Date: Wed Aug 12 15:21:36 2020 -0500
UefiCpuPkg/CpuExceptionHandler: Add base support for the #VC exception
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
Add base support to handle #VC exceptions. Update the common exception
-CONST UINT32 mErrorCodeFlag = 0x00227d00;
+CONST UINT32 mErrorCodeFlag = 0x20227d00;
+ "Reserved",
+ "Reserved",
+ "Reserved",
+ "Reserved",
+ "Reserved",
+ "Reserved",
+ "Reserved",
+ "#VC - VMM Communication"
```
```
commit 0d25074cbcc272532d6ca5a47974ac5b31f4b6ec
Author: Jiewen Yao <jiewen.yao@intel.com>
Date: Fri Feb 22 21:30:35 2019 +0800
UefiCpuPkg/ExceptionLib: Add CET support.
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1521
Add information dump for Control Protection exception.
-CONST UINT32 mErrorCodeFlag = 0x00027d00;
+CONST UINT32 mErrorCodeFlag = 0x00227d00;
- "#VE - Virtualization"
+ "#VE - Virtualization",
+ "#CP - Control Protection"
};
```
Related work items: #57987809
…chitecture tag. Related work items: #57987809
- Gcc LTO checks function signatures. Fix them. - Add/remove stubs. Related work items: #57987809
Make the lld-link flags not the gcc flags. Set gcc flags. Note that gcc flags are sometimes filealign:64K, rendering it, not great. We should use windows targeting/cygwin/mingwin tools instead, so that there is no ELF=>PE conversion, and sectioAlign and filAalign can be unequal. Related work items: #57987809
Upcoming module update breaks this code. Make it more resililent. Remove 33 lines, add 11, no loss of meaning or clarity, except we no longer list all the zeroed member data. Related work items: #57987809
Also for Msvc in MsKdDebugPkg. Related work items: #57987809
- Standard. - Shorter. - I saw other PRs doing it. - A few cases deliberately deferred, since they will not compile (still using __FUNCTION__ as concatenable string literal, instead of non-concatenable char array; stuff not in clang build?) ---- This pull request is a code cleanup that standardizes debug logging by replacing all instances of `__FUNCTION__` with `__func__`. The pull request updates the logging macros across the codebase to use `__func__`, ensuring more consistent and portable debug messages without changing the underlying functionality. - `MsvmPkg/AcpiPlatformDxe/Dsdt.c`, `Tcg2PreInitLibPei.c`, and similar files now use `__func__` in their DEBUG statements. - Modules under `AziHsmDxe` have been modified to uniformly report errors and status using `__func__`. - `VmbusDxe` and `VmbusRoot.c` files have updated logging for memory operations and protocol communications. - Various libraries (e.g., `MsvmRngLib`, `Tpm2DeviceLib`) and unit test files also reflect the change for improved log consistency. <!-- GitOpsUserAgent=GitOps.Apps.Server.pullrequestcopilot --> Related work items: #57987809
See microsoft/mu_basecore#1712 Related work items: #57987809
It is an old optimization, that we never use. Remove it. Related work items: #57987809
It used by a lot of the Masm, and derivation might be used by Nasm. You really cannot write decent ABI-following code with Masm without it. Though the ABI in UEFI is a little unclear wrt unwind, and Nasm does not really "work". Related work items: #57987809
Convert macamd64.inc to macamd64.nasm to reduce porting work. ---- #### AI description (iteration 4) #### PR Classification This pull request adds a new feature by introducing a NASM portability layer to support Clang builds. #### PR Summary The changes introduce a comprehensive assembly macro file that provides AMD64 macros to ease the transition from MASM to NASM, aligning with the Clang build requirements for hyperv.uefi AARCH64. - `MsvmPkg/Include/macamd64.nasm`: Newly added file defining macros for register pushes, stack allocation, register and xmm saving, and function prologue/epilogue management. <!-- GitOpsUserAgent=GitOps.Apps.Server.pullrequestcopilot --> Related work items: #57987809
…KS3.c). `char big[99]= "small"` is `memcpy` and/or `memset`. In this case, the copy is not needed. (In other cases we often add CopyMem/ZeroMem calls as part of the fix). Related work items: #57987809
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.