MsvmPkg: unify UEFI parameter configuration handoff#93
Conversation
|
So if I am understanding correctly, this change makes it so that in isolated contexts, we dodge the UefiConfigInfo path and instead rely on some GetIgvmConfigInfo instead? |
|
|
||
| //DEBUG((DEBUG_VERBOSE, ">>> %a\n", __func__)); | ||
| ZeroMem(mNvramCommandDescriptor, sizeof(NVRAM_COMMAND_DESCRIPTOR)); | ||
| ZeroMem((VOID*)mNvramCommandDescriptor, sizeof(NVRAM_COMMAND_DESCRIPTOR)); |
There was a problem hiding this comment.
nit: are all of these VOID* additions and EFIAPI done to dodge an actual build error or warnings now?
If these are just for the sake of making intellisense happy, I would prefer if we can move these to a separate PR to make reviewing this easier + maintain a slim history per commit (we cherry-pick these to closed source)
There was a problem hiding this comment.
Removed these parts of changes.
| // Isolated guests retain the existing IGVM defaults above. | ||
| // | ||
| configFlags.Flags.SerialControllersEnabled = 1; | ||
| configFlags.Flags.HibernateEnabled = 1; |
There was a problem hiding this comment.
I used Codex to analyze and generate the default setting here. Codex derived them from the legacy direct-UEFI blob builder plus the Petri test configuration
968358e to
52addae
Compare
No change on isolated context, still call GetIgvmConfigInfo. The problem is there could be non-isolated IGVM, and we should use GetIgvmConfigInfo as well rather than call the legacy UefiConfigInfo. This PR is trying to unify this, all IGVM cases call GetIgvmConfigInfo, no matter isolated or not. @maheeraeron |
Firmware should consume the loader-provided parameter-area schema without parsing or otherwise depending on the IGVM container format. Publish the optional parameter-area address received by SEC through a new parameter-config PPI. X64 receives the address through R12, while AArch64 defines x3 as the corresponding optional SEC input for a future loader producer. Platform PEI evaluates the hardware-isolated-without-a-paravisor condition once and resolves the effective parameter header before parsing. Isolated launches start with the historical fixed address for compatibility with TDX and older images, and a non-NULL pointer supplied by SEC overrides that value. Non-isolated launches require an explicit pointer to select parameter parsing; otherwise they retain the legacy configuration path. Pass the resolved non-NULL header and evaluated isolation state to GetIgvmConfigInfo. The parser validates its required pointer, does not repeat the isolation query, and applies the appropriate firmware defaults. Non-isolated parameter launches retain the former OpenVMM defaults for serial COM1, hibernation, memory protection, MTRRs, and preassigned PCI resources, while isolated guests retain their existing defaults. This lets OpenVMM use the parameter area already described by a UEFI IGVM instead of constructing a second legacy UEFI configuration blob. Direct UEFI launches and isolated images without an explicit pointer remain compatible. Tested with RELEASE CLANGPDB firmware builds for X64 and AARCH64. The focused X64 OpenVMM boot_uefi_custom_igvm execution test passed (1 passed, 313 skipped; nextest run 29ddd7ec-c4ab-41aa-b2c1-0c56f79cb385).
52addae to
0e21b41
Compare
|
I thought it might be helpful to the review by providing some additional background to give more context for the problem the PR is trying to address. We are currently enabling AArch64 IGVM support in OpenVMM, using the existing x64 IGVM implementation as a reference. Our IGVM payload is UEFI. During this work, we discovered that a UEFI IGVM did not boot even on x64. OpenVMM currently assumes that every IGVM represents an OpenHCL/VTL2 isolation image. However, IGVM is a general-purpose container format and can carry other payloads as well, including the UEFI test image we created. To address this, we first updated OpenVMM to distinguish UEFI IGVMs from OpenHCL IGVMs and initialize the required UEFI chipset and host services, while preserving the existing behavior for OpenHCL images. The next question was how to pass configuration information to UEFI. One option was to reconstruct the legacy OpenVMM UEFI configuration blob at its fixed address. However, since UEFI is already packaged within an IGVM, the build process naturally generates an IGVM parameter area containing all of the required configuration. The proposed design (under suggestion from Chris) therefore uses the IGVM parameter area directly: This approach avoids maintaining two separate configuration representations and establishes a clear ownership boundary: OpenVMM is responsible for processing the IGVM container, while the firmware is responsible for parsing the parameter-area ABI. The x64 execution test passes successfully using firmware built with changes of this PR + new OpenVMM of change at PR 3726, and the firmware also builds successfully for AArch64 (execution test pending future OpenVMM Aarch64 IGVM loader enablement) |
We are currently enabling AArch64 IGVM support in OpenVMM, using the
existing x64 IGVM implementation as a reference. Our IGVM payload is UEFI.
During this work, we discovered that a UEFI IGVM did not boot even on
x64. OpenVMM currently assumes that every IGVM represents an
OpenHCL/VTL2 isolation image. However, IGVM is a general-purpose
container format and can carry other payloads as well, including the
UEFI test image we created.
To address this, we first updated OpenVMM to distinguish UEFI IGVMs from
OpenHCL IGVMs and initialize the required UEFI chipset and host
services, while preserving the existing behavior for OpenHCL images.
The next question was how to pass configuration information to UEFI.
One option was to reconstruct the legacy OpenVMM UEFI configuration
blob at its fixed address. However, since UEFI is already packaged
within an IGVM, the build process naturally generates an IGVM parameter
area containing all of the required configuration.
The proposed design (following Chris's suggestion) therefore uses the
IGVM parameter area directly:
parse the IGVM parameter area.
PEI falls back to locating the parameter area at its historical fixed
address, preserving backward compatibility.
This approach avoids maintaining two separate configuration
representations and establishes a clear ownership boundary: OpenVMM is
responsible for processing the IGVM container, while the firmware is
responsible for parsing the parameter-area ABI.
The x64 execution test passes successfully using firmware built with the
changes in this PR together with the OpenVMM changes in
microsoft/openvmm#3726. The firmware also builds
successfully for AArch64. Execution testing is pending future OpenVMM
AArch64 IGVM loader enablement.