Re: [PATCH V12 42/47] OvmfPkg: Add TdxDxe driver

["Lendacky, Thomas" <thomas.lendacky@amd.com>]

On 3/29/22 18:46, Min Xu wrote:
> RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
> 
> TdxDxe driver is dispatched early in DXE, due to being list in APRIORI.
> This module is responsible for below features:
>   - Sets max logical cpus based on TDINFO
>   - Sets PCI PCDs based on resource hobs
>   - Set shared bit in MMIO region
>   - Relocate Td mailbox and set its address in MADT table.
> 
> 1. Set shared bit in MMIO region
> 
> Qemu allows a ROM device to set to ROMD mode (default) or MMIO mode.
> When it is in ROMD mode, the device is mapped to guest memory and
> satisfies read access directly.
> 
> In EDK2 Option ROM is treated as MMIO region. So Tdx guest access
> Option ROM via TDVMCALL(MMIO). But as explained above, since Qemu set
> the Option ROM to ROMD mode, the call of TDVMCALL(MMIO) always return
> INVALID_OPERAND. Tdvf then falls back to direct access. This requires
> to set the shared bit to corresponding PageTable entry. Otherwise it
> triggers GP fault.
> 
> TdxDxe's entry point is the right place to set the shared bit in MMIO
> region because Option ROM has not been discoverd yet.
> 
> 2. Relocate Td mailbox and set the new address in MADT Mutiprocessor
> Wakeup Table.
> 
> In TDX the guest firmware is designed to publish a multiprocessor-wakeup
> structure to let the guest-bootstrap processor wake up guest-application
> processors with a mailbox. The mailbox is memory that the guest firmware
> can reserve so each guest virtual processor can have the guest OS send
> a message to them. The address of the mailbox is recorded in the MADT
> table. See [ACPI].
> 
> TdxDxe registers for protocol notification
> (gQemuAcpiTableNotifyProtocolGuid) to call the AlterAcpiTable(), in
> which MADT table is altered by the above Mailbox address. The protocol
> will be installed in AcpiPlatformDxe when the MADT table provided by
> Qemu is ready. This is to maintain the simplicity of the AcpiPlatformDxe.
> 
> AlterAcpiTable is the registered function which traverses the ACPI
> table list to find the original MADT from Qemu. After the new MADT is
> configured and installed, the original one will be uninstalled.
> 
> [ACPI] https://uefi.org/specs/ACPI/6.4/05_ACPI_Software_Programming_Model
> /ACPI_Software_Programming_Model.html#multiprocessor-wakeup-structure
> 
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> 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>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>

Unfortunately, this driver also breaks SEV-ES. I bypassed the TDX code in
the SEC library, but then hit an issue because this driver is loaded
before the AmdSevDxe driver. 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 it appears
the dependencies result in an MMIO being performed to an address in 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.

Here is the error information:

Loading driver at 0x0003F1A5000 EntryPoint=0x0003F1A6BF2 TdxDxe.efi
InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 3F1AFA98
ProtectUefiImageCommon - 0x3F1AFB40
   - 0x000000003F1A5000 - 0x0000000000004300
MMIO using encrypted memory: B00F8040
!!!! X64 Exception Type - 0D(#GP - General Protection)  CPU Apic ID - 00000000 !!!!
ExceptionData - 0000000000000000
RIP  - 000000003F1A6E86, CS  - 0000000000000018, RFLAGS - 0000000000010002
RAX  - 00000000B0000000, RCX - 000000003F4EAD18, RDX - 000000003F4EAD01
RBX  - 0000000000000001, RSP - 000000003FBA5B60, RBP - 0000000000000000
RSI  - 0000000000000000, RDI - 000000003F1AFB18
R8   - 00000000B00F8040, R9  - 00000000000029C0, R10 - 0000000000000000
R11  - 0000000000000000, R12 - 0000000000000000, R13 - 0000000000000000
R14  - 000000003FBCC6D0, R15 - 000000003FBC43E6
DS   - 0000000000000008, ES  - 0000000000000008, FS  - 0000000000000008
GS   - 0000000000000008, SS  - 0000000000000008
CR0  - 0000000080010033, CR2 - 0000000000000000, CR3 - 000000003F801000
CR4  - 0000000000000668, CR8 - 0000000000000000
DR0  - 0000000000000000, DR1 - 0000000000000000, DR2 - 0000000000000000
DR3  - 0000000000000000, DR6 - 0000000000000000, DR7 - 0000000000000000
GDTR - 000000003FBFE000 0000000000000027, LDTR - 0000000000000000
IDTR - 000000003BF95D70 000000000000021F,   TR - 0000000000000000
FXSAVE_STATE - 000000003FBA57C0
!!!! Find image based on IP(0x3F1A6E86) /root/kernels/ovmf-build-X64/Build/OvmfX64/DEBUG_GCC5/X64/OvmfPkg/TdxDxe/TdxDxe/DEBUG/TdxDxe.dll (ImageBase=000000003F1A5000, EntryPoint=000000003F1A6BF2) !!!!

Not sure what the best way forward is on this.

Thanks,
Tom

> ---