Fast forward UEFI with the latest from closed source 02-26-2026#47
Open
maheeraeron wants to merge 22 commits intomicrosoft:mainfrom
Open
Fast forward UEFI with the latest from closed source 02-26-2026#47maheeraeron wants to merge 22 commits intomicrosoft:mainfrom
maheeraeron wants to merge 22 commits intomicrosoft:mainfrom
Conversation
Delete MsvmPkg/CpuDxe/Ia32/CpuAsm.nasm. We do not use it. ---- #### AI description (iteration 1) #### PR Classification This pull request is a code cleanup effort that removes outdated 32-bit support. #### PR Summary The PR deletes legacy 32-bit assembly code to streamline the codebase in alignment with the cleanup work item. - `/MsvmPkg/CpuDxe/Ia32/CpuAsm.nasm` was removed. <!-- GitOpsUserAgent=GitOps.Apps.Server.pullrequestcopilot --> Related work items: #60275790
Clang: Remove unused statics. The following are static and not used. Clang warngs. Remove them. AziHsmDumpSqe EncodeApiRevisionResponse DecodeApiRevisionCommandResponse DecodeRequestHeader EncodeResponseHeader ReadAqaReg ReadAsqReg ReadAcqReg CreateTopMenu IsPostReadyToBoot NetvscReceive::counter `git show | findstr /b /c:+` shows no added lines, only deletes, a _sorta_ measure of a review and proof that the description is correct. This is extracted from larger work mostly by John Starks, partly by me (in this case, really, all John). Related work items: #57987809, #60275790
Clang: Fix `__FUNCTION__`.
In Visual C++, `__FUNCTION__` is a concatenatable string literal.
In Clang, `__FUNCTION__` is a non-concatenatable string array like standard `__func__`.
```
MsBootPolicyLib.c:48: error: expected ')'
DEBUG((DEBUG_INFO,__FUNCTION__ " Checking if the following device path is permitted to boot:\n"));
^
```
etc. Use `%a` to format it, as a separate parameter.
John Starks started this and did most or all of this -- porting to Clang.
Related work items: #57987809
Clang: Port __declspecs. `DECLSPEC_ALIGN` - `__declspec(align())` or `__attribute__((aligned))`, or perhaps `_Alignas` `DECLSPEC_CACHEALIGN` - `DECLSPEC_ALIGN (64)` or `DECLSPEC_ALIGN (128)`, to match Windows (not necessary an actual cache line size, or what is recommended, but, match Windows). (ARM64 cache lines are guaranteed to be 128 or smaller, so 128 separates caches lines, guaranteed, perhaps by a lot, but C++ uses 256 (or 128) for similar purposes). `DECLSPEC_DEPRECATED` (remove, it is not used) Related work items: #54710354, #57987809, #60275790
Cleanup ExcludeMainFvFromMeasurementLib.c. - detab (one of the best things you can do to any codebase!) - deduplicate semicolons - 4 spaces instead of 2 - space before paren - add static on kinda short extern identifier Related work items: #57987809, #60275790
- Use uppercase form always. - Two parameters (second can be empty string). - Sometimes, rarely, requires brac.es. Related work items: #57987809, #60275790
bdapi.c: error: enumeration value 'BdNone' not handled in switch VideoGraphicsOutputBlt: Switch cover enum. When switching on an enum, clang warns if you do not case each member. This PR makes no judgement as to if this was a bug or what the should do. It lists the member, quiescing the warning, and maintains the old behavior as before, which is to fall out of the switch without doing anything (or going to default if there was one, not sure if Clang would warn on that or not, probably not.) ---- This pull request is a code cleanup that ensures all enum values are handled in switch statements to meet Clang's strict enum coverage requirements. The changes update switch statements to include missing enum cases, addressing Clang compatibility and cleanup work items. - `MsvmPkg/Library/BdLib/bdapi.c`: Adds a `BdNone` case and removes an unnecessary newline for proper enum handling. - `MsvmPkg/VideoDxe/VideoDxe.c`: Introduces an `EfiGraphicsOutputBltOperationMax` case and eliminates extraneous whitespace in the switch statement. <!-- GitOpsUserAgent=GitOps.Apps.Server.pullrequestcopilot --> Related work items: #57987809, #60275790
Clang: Initialize some local variables.
Imho we should initialize all locals variables:
To in {} in C++11.
{0] in C99
{} in C23 or C26
But in this case, just a few that clang warned about.
HvHypercallLib.c variable 'msrIndex' is used uninitialized whenever switch default is taken.
etc.
----
#### AI description (iteration 1)
#### PR Classification
This pull request performs a code cleanup by explicitly initializing local variables to support Clang builds.
#### PR Summary
This PR ensures that critical local variables are initialized to 0, addressing Clang's stricter compilation requirements as part of the work for building hyperv.uefi AARCH64 with Clang.
- `MsvmPkg/PlatformPei/IgvmConfig.c`: Initializes `svsmBase` and `svsmSize` to 0.
- `MsvmPkg/Library/HvHypercallLib/HvHypercallLib.c`: Initializes `msrIndex` to 0.
<!-- GitOpsUserAgent=GitOps.Apps.Server.pullrequestcopilot -->
Related work items: #57987809
Clang warns for this. Missed in earlier PR splitting. Related work items: #60275790, #57987809
KdDebugLib.c: Cast PCHAR8 to PUINT8. ULONG* and UINT32* are not the same, cast. netio.c: error: incompatible pointer types passing 'PUINT32' (aka 'unsigned int *') to parameter of type 'PULONG' (aka 'unsigned long *') Cast string constants to PUCHAR. Change CrashDumpBuffer from CHAR8 to UINT8. CrashDumpAgent.c error: passing 'CHAR8 [32768]' to parameter of type 'UINT8 *' AziHsmBKS3.c: cast CHAR8* to BYTE* Fix PlatformConsoleEventCallback signature, void* vs. non-void*. FrontPage\PlatformConsole.c: error: incompatible pointer types passing 'BOOLEAN (void *, const EFI_EVENT_DESCRIPTOR *, const BOOTEVENT_DEVICE_ENTRY *)' (aka 'unsigned char (void *, const EFI_EVENT_DESCRIPTOR *, const BOOTEVENT_DEVICE_ENTRY *)') to parameter of type 'EFI_EVENTLOG_ENUMERATE_CALLBACK' (aka 'unsigned char (*)(void *, const EFI_EVENT_DESCRIPTOR *, const void *)') BootDeviceEventEnumerate(PlatformConsoleEventCallback, &eventCount); Related work items: #57987809, #60275790
i.e. Do not use nice C language extension, that resembles a C++ base class. Related work items: #57987809, #60275790
…singly to UINT64. ReadNoFence returns INT32. In RingBuffer we are comparing that to a UINT32. The code casts the result to UINT64. This is confusing. Does it sign extend or not? i.e. There is both a change in size and signedness. In which order? The change in size is not required. Cast to UINT32 instead. Related work items: #57987809, #60275790
…d delay TPM PH lock to ReadyToBoot This PR accomplishes the following: - Driver Entry no longer generates Platform hierarchy secret and we avoid TPM null sealing. (This removes the perf issue of TPM operation in driver Entry) - Tcg2PlatformDxe is overriden to MsvmTcg2PlatformDxe, which allows our platform to delay the TPM platform hierarchy sealing to ReadyToBoot. This has security implications for third party drivers or option ROMs in the future though, but is necessary to unblock AziHsm GA - Removes AziHsmEnabled and its PCD Now, VMs that do not use manticore will never face a increased boot regression; DriverEntry for AziHsm is ~0 ms. The work is now done in DriverBindingStart, which only runs if AziHsm device is present. To test, we built an MSVM.fd with serial and consumed it with OpenHCL. We then used OpenHCL to boot a VM with manticore enabled on a manticore-equipped host. We verified that the AziHsm DriverBinding logic succeeds from the informational logs, confirming that the behavior is the same. Here are logs confirming so: [azihsm_tpm_seal_change_log.txt](https://microsoft.visualstudio.com/8d47e068-03c8-4cdc-aa9b-fc6929290322/_apis/git/repositories/1ad2e189-11bd-479f-8f25-a739cc641dfa/pullRequests/14855961/attachments/azihsm_tpm_seal_change_log.txt) This text document (convert it to .md) shows the timing for DriverEntry and DriverBindingStart with the fix [DriverEntryAndBindingTimingWithFix.txt](https://microsoft.visualstudio.com/8d47e068-03c8-4cdc-aa9b-fc6929290322/_apis/git/repositories/1ad2e189-11bd-479f-8f25-a739cc641dfa/pullRequests/14855961/attachments/DriverEntryAndBindingTimingWithFix.txt) Compare this to without the fix [DriverEntryAndBindingTimingWithoutFix.txt](https://microsoft.visualstudio.com/8d47e068-03c8-4cdc-aa9b-fc6929290322/_apis/git/repositories/1ad2e189-11bd-479f-8f25-a739cc641dfa/pullRequests/14855961/attachments/DriverEntryAndBindingTimingWithoutFix.txt) ---- #### AI description (iteration 1) #### PR Classification This pull request deprecates the TPM null sealing logic in the AziHsm driver and cleans up associated sensitive data handling. #### PR Summary The changes remove TPM null sealing operations and event-based sensitive data cleanup from the AziHsm driver while streamlining key derivation to use the TPM platform secret directly, and introduce a new module for TPM platform hierarchy configuration. - **`AziHsmDxe.c`**: Removed TPM null sealing logic, along with ReadyToBoot/UnableToBoot event callbacks and the sensitive data cleanup routines. - **AziHsm key derivation workflow**: Updated in `AziHsmPerformBks3SealingWorkflow` to bypass unsealing via the TPM null hierarchy. - **`MsvmTcg2PlatformDxe.c` & `MsvmTcg2PlatformDxe.inf`**: Added a new platform-specific module to configure and lock the TPM Platform Hierarchy at ReadyToBoot. - **Configuration files**: Updated in files like `BiosInterface.h`, DSC, FDF, DEC, and PlatformPei to remove references to AziHsmEnabled and align with the new TPM platform hierarchy setup. Related work...
…e for Linux L1VH SINT discov... Revert "Merged PR 14617102: Add opt-in ACPI device for Linux L1VH SINT discovery on ARM64" This reverts commit 374c9111ccc6cdf8352b44e1142b430eac8a880e. Related work items: #60730240
It was a bad idea with Visual C++ and should be avoided even if not switching compilers. It leads to silent bad code. HvHypercallLib.c implicit declaration of function '__assume' is invalid in C99. Related work items: #57987809, #60275790
Clang: Fix case of includes (uppercase, lowercase), to match file system. BiosDeviceLibCore.c non-portable path to file '<Library/BaseLib.h>'; specified path differs in case from file name on disk [-Werror,-Wnonportable-include-path] Snp.h: Fix case of include NetvscDxe.h. Fix case of include nvspprotocol.h. Correct case of includ VmbusPacketFormat.h Fix case of include BaseLib.h. Fix include path of of cpu.h. Bd.h: error: 'cpu.h' file not found Fix include ntstatus.h. Actually include the intended smaller one usually, larger one sometimes. This should be further cleaned up, to avoid the case insensitive name collision, VmbusNtStatus and KdNtStatus, or just combine them, now that KdNtStatus is reasonably small. If you get the wrong one, unsurprising: bdapi.c use of undeclared identifier 'STATUS_SUCCESS' bdapi.c use of undeclared identifier 'STATUS_INVALID_PARAMETER' ---- Code cleanup to fix include case mismatches for clang compliance. This pull request corrects header file include cases and updates architecture-specific conditional includes to match the filesystem, ensuring builds are clang compliant. It also supports linked work items addressing nested virtualization and miscellaneous clang fixes. - In `MsvmPkg/Library/BdLib/Bd.h`, the single include of "cpu.h" is replaced with conditional includes for architecture-specific headers and commented standard library includes are replaced with an include for "ntstatus.h". - In `MsvmPkg/NetvscDxe` (across both header and source files), include statements are updated to fix the case for `PrintLib` and `nvspprotocol.h`. - In `MsvmPkg/StorvscDxe/StorchannelDxe.c`, the include is corrected from "Industrystandard/Scsi.h" to "IndustryStandard/Scsi.h". - In modules like `EmclDxe/Init.c`, `BiosDeviceLibCore.c`, and `VariableDxe.c`, includes have been updated from "Baselib.h" to "BaseLib.h". - In `SynthKeyDxe/SynthKeyChannel.h` and `VideoDxe/VideoDxe.h`, header naming is fixed from "VmBusPacketFormat.h" to "VmbusPacketFormat.h". <!-- GitOpsUserAgent=GitOps.Apps.Server.pullrequestcopilot --> Related work items: #41399366, #54710354, #60275790
```
Emcl.h:191:5: error: declaration does not declare anything [-Werror,-Wmissing-declarations]
EFI_EMCL_PROTOCOL;
^~~~~~~~~~~~~~~~~
```
Name it Base, use it.
Use it instead of casting also.
C++ would help here. :)
Related work items: #54710354, #60275790
…e scopes. Add braces so there are scopes around local variables. And/or, to avoid goto (switch) around initialization, since C99 allows locals not at scope start. Braces should suffice in place of blank lines, and sometimes there is blank before break, sometimes not. Converge on not. Related work items: #57987809
```
DeviceBootManagerLib.c:493:25: error: incompatible pointer types
passing 'EFI_DEVICE_PATH_PROTOCOL **' to parameter of type 'void **' [-Werror,-Wincompatible-pointer-types]
DevicePath
^~~~~~~~~~
```
etc.
Related work items: #57987809
stdint.h is appropriate for UINT64_C. Really it is appropriate for UINT8, 16, 32, 64 (uint8_t, uint16_t, uint32_t, uint64_t), if C99 can be depended on instead of C89. (Even with C89, it is likely provided by gcc/clang.) And it provides UINT32_MAX, UINT64_MAX. Which clashes with ours. We could undef, or just use them. i.e. Extremely mild cleanup via reuse of std. While UEFI avoids std in general, I think for a very very small number of things, it is OK. (ie. stdint.h, stddef.h, limits.h, stdarg.h) Related work items: #57987809
Related work items: #57987809, #60275790
```
PlatformThemeLib.c:31 error: pasting formed
'mMsUiFontPackageHdr_Selawik_Regular_8pt_Fixed)', an invalid preprocessing token
FONT_DECL (FixedFont, Selawik_Regular_8pt_Fixed)
^
```
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
3 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.
Fast forwards UEFI with changes regarding fixes for the upcoming clang toolchain move + AziHsmDxe fixes