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

Tue Jun 8 19:33:11 UTC 2021

(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.

> 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>.)

> 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).

> 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).

> 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.)

> 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?

We have not experienced performance problems due to this kind of IOMMU
protocol usage, when booting SEV guests.

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

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#76234): https://edk2.groups.io/g/devel/message/76234
Mute This Topic: https://groups.io/mt/83283616/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-