Re: [PATCH V2 1/1] OvmfPkg/OvmfPkgX64: Adjust load sequence of TdxDxe and AmdSevDxe driver

["Yao, Jiewen" <jiewen.yao@intel.com>]

Thanks Min.

Would you please add some comment in the APRIORI section, to explain why the order is in this way?
It also reminds people that the order should not be changed.

With comment added, reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>




> -----Original Message-----
> From: Xu, Min M <min.m.xu@intel.com>
> Sent: Wednesday, April 20, 2022 2:57 PM
> To: devel@edk2.groups.io
> Cc: Xu, Min M <min.m.xu@intel.com>; Brijesh Singh <brijesh.singh@amd.com>;
> Aktas, Erdem <erdemaktas@google.com>; James Bottomley
> <jejb@linux.ibm.com>; Yao, Jiewen <jiewen.yao@intel.com>; Tom Lendacky
> <thomas.lendacky@amd.com>
> Subject: [PATCH V2 1/1] OvmfPkg/OvmfPkgX64: Adjust load sequence of TdxDxe
> and AmdSevDxe driver
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3904
> 
> TdxDxe driver is introduced for Intel TDX feature. Unfortunately, this
> driver also breaks boot process in SEV-ES guest. The root cause is in
> the PciLib which is imported by TdxDxe driver.
> 
> In a SEV-ES guest the AmdSevDxe driver performs a
> MemEncryptSevClearMmioPageEncMask() call against the
> PcdPciExpressBaseAddress range to mark it shared/unencrypted. However,
> the TdxDxe driver is loaded before the AmdSevDxe driver, and the PciLib
> in TdxDxe is DxePciLibI440FxQ35 which will access the
> PcdPciExpressBaseAddress range. Since the range has not been marked
> shared/unencrypted, the #VC handler terminates the guest for trying to
> do MMIO to an encrypted region.
> 
> Adjusting the load sequence of TdxDxe and AmdSevDxe can fix the issue.
> 
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> SEV-Tested-by: Tom Lendacky <thomas.lendacky@amd.com>
> TDX-Tested-by: Min Xu <min.m.xu@intel.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> ---
>  OvmfPkg/OvmfPkgX64.fdf | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
> index 71df28705ea8..1f9d0b3f5fcc 100644
> --- a/OvmfPkg/OvmfPkgX64.fdf
> +++ b/OvmfPkg/OvmfPkgX64.fdf
> @@ -214,8 +214,8 @@ READ_LOCK_STATUS   = TRUE
>  APRIORI DXE {
>    INF  MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
>    INF  MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
> -  INF  OvmfPkg/TdxDxe/TdxDxe.inf
>    INF  OvmfPkg/AmdSevDxe/AmdSevDxe.inf
> +  INF  OvmfPkg/TdxDxe/TdxDxe.inf
>  !if $(SMM_REQUIRE) == FALSE
>    INF  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf
>  !endif
> --
> 2.29.2.windows.2