Re: [edk2-rfc] [edk2-devel] RFC: design review for TDVF in OVMF


Min Xu
 

On 06/09/2021 3:33 AM, Laszlo wrote:
(Min Xu got dropped from the CC list for some reason, at *some* point in this
sub-thread! Not sure when. Re-adding him.)

Commenting on excerpts:

On 06/08/21 18:01, James Bottomley wrote:

On TdMailbox and TdHob, we already have two SEV pages in the MEMFD and
since TDX and SEV is an either/or, could we simply not rename both
pages and use them for either boot depending on what CPU type is
detected, so we only have two MEMFD pages, not four?
Great idea, in my opinion.
Agree, it's a good idea.

On your slide 13 Question: "Open: How will the QEMU find the metadata
location?" can't you just use the mechanism for SEV that's already
upstream in both QEMU and OVMF?
I think I made the same comment, in different words. (Point (12) at
<https://listman.redhat.com/archives/edk2-devel-archive/2021-
June/msg00143.html>.)
So my understanding to this solution is that:
1) GUID-ed structure chain is started from a fixed GPA in ResetVector.
2) Append a TDX-specific GUID-ed structure in the chain
3) Qemu search the GUID-ed chain from the fixed GPA and find the TDX-specific
GUID-ed structure based on TDX-specific GUID.
Is the expected process for QEMU?


On slide 19, the mucking with the reset vector really worries me
because we don't have that much space to play with. Given that you're
starting in 32 bit mode and can thus enter anywhere in the lower 4GB,
why not simply use a different and TDX specific entry point?
What's more, we should use a dedicated ResetVector (through a DSC+FDF
dedicated solely to TDX).
If TDVF has a separate DSC/FDF, then this is not a problem. Let's wait for a
conclusion to the *one binary* solution.
Thanks for your suggestion.

On all the Tcg2 changes: what about installing a vTPM driver that
simply translates to your MSRs? That way we can use all the standard
TCG code as is?
I believe I made the same comment in point (20) (see URL above).
I will answer this comments in my later response. Thanks.

Slide 41: IOMMU operation.
That's more like slides 40 and 42, no?


The implication is that you only transition to unencrypted memory for
DMA during the actual operation,
Yes, this is the idea behind EDKII_IOMMU_PROTOCOL, which
OvmfPkg/IoMmuDxe implements (for SEV only, currently).


so do I have it correct that the guest writes DMA to encrypted memory,
then the iommu marks the region as unencrypted and transforms the
memory to be in the clear and then transforms it back after the DMA
operation completes?
Effectively, yes. (Your summary corresponds to a BusMasterRead
operation.)
In TDX there are the concepts of Private-Memory and Shared-Memory.
By default the memory are private. Before the DMA operation, the private
memory should be converted to Shared-Memory by setting the S-Bit. Then
VMM and Guest can do the DMA operation.
(The difference is that in TDX even the Shared-Memory is encrypted with
a shared-key between VMM and Guest. So we call it Shared-Memory)
So all the changes in IOMMU by TDX is just to customize the page
decryption / encryption primitives in the driver , and the general logic will not
be touched.

Given that SEV operates quite happily with always in the clear DMA
buffers,
I don't understand this comment -- is it a statement about SEV as a technology,
or about OvmfPkg/IoMmuDxe?

Specifically in the context of OvmfPkg/IoMmuDxe, there is no "always-in-the-
clear DMA".

EDKII_IOMMU_PROTOCOL was designed to fit cleanly into the Map(), Unmap(),
AllocateBuffer(), FreeBuffer() terminology of the UEFI standard
EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL and EFI_PCI_IO_PROTOCOL. As far as I
can tell, the original use case for EDKII_IOMMU_PROTOCOL was VT-d on bare
metal, but the protocol proved a good match for SEV too.

VIRTIO_DEVICE_PROTOCOL has similar member functions
(AllocateSharedPages, FreeSharedPages, MapSharedBuffer,
UnmapSharedBuffer).

As long as a PCI device driver (or virtio driver) uses these member functions
judiciously, only "BusMasterCommonBuffer" operations will be backed by long-
term plaintext (decrypted) pages. One-shot read and write transactions will be
backed by plaintext (decrypted) pages only as long as necessary.

The transitions you outline already happen in any plain SEV guest that uses PCI
DMA or virtio.


this seems to have the potential to be a performance problem, but what
security does it gain?
As I mentioned above, in TDX DMA operation even the Shared-Memory is encrypted
by a shared-key between VMM and Guest. It is not *plain-text* outside VMM&Guest.

We have not experienced performance problems due to this kind of IOMMU
protocol usage, when booting SEV guests.
In our test, we don't see the performance problem when booting TDX guests.
We do have the performance issue in "Accept Pages". But we have several solutions
to resolve this performance. See below link.
https://software.intel.com/content/dam/develop/external/us/en/documents/tdx-virtual-firmware-design-guide-rev-1.pdf
Section 7.8 Optimization Consideration.

The basic goal was to keep everything as tightly encrypted as possible (as
permitted by the individual PCI or Virtio driver, through its conservative usage
of BusMasterCommonBuffer operations).

I won't claim that it has zero performance impact, but we should remember
the purpose that firmware serves (namely, "booting an operating system").
Really -- I don't recall any performance issues. This applies to such virtio
devices & drivers too that aren't "bootable", such as virtio-gpu-pci
(VirtioGpuDxe) and virtio-rng-pci (VirtioRngDxe).

(

If you enable verbose logs, OvmfPkg/IoMmuDxe does produce an immense
amount of messages (with the express purpose of a human reading
through them, and matching up decryption and re-encryption actions --
I've done it, likely with some ad-hoc scripts). *This* does slow down
the boot considerably (if you actually enable the QEMU debug console),
but for a different reason: producing debug logs through the QEMU
debug console (IO Port) is very-very costly in a SEV guest. Not just
because an IO port trap may be more expensive in a SEV guest, but
because SEV does not support REP OUTSB, so every debug character
written traps separately, as opposed to every line written. See
the following commits:

- b6d11d7c4678 ("MdePkg: BaseIoLibIntrinsic (IoLib class) library",
2017-04-13),

- 97353a9c914d ("OvmfPkg: Update dsc to use IoLib from
BaseIoLibIntrinsicSev.inf", 2017-07-10),

- 98a4d04e8fda ("MdePkg/BaseIoLibIntrinsic: fix SEV (=unrolled)
variants of IoWriteFifoXX()", 2017-09-11),

- c09d9571300a ("OvmfPkg: save on I/O port accesses when the debug
port is not in use", 2017-11-17).

)

From my perspective, I find the changes proposed for OvmfPkg/IoMmuDxe to
be among the least intrusive of the whole slide deck (after Min Xu confirmed
that the intent was really only to customize the page decryption / encryption
primitives in the driver, and to leave the general logic untouched).

That's not to say that I'm unhappy about this topic being raised. To the
contrary!

Thanks
Laszlo
Thanks
Min

Join devel@edk2.groups.io to automatically receive all group messages.