Re: [Qemu-devel] libvirt/QEMU/SEV interaction

[Brijesh Singh]

Hi Laszlo,


On 10/01/2017 04:56 AM, Laszlo Ersek wrote:
> On 10/01/17 11:17, Laszlo Ersek wrote:
>
> > (3) Implement SEV encryption for pflash. A pflash chip can be in one of
> > two modes: (a) it reads and executes as ROM, or (b) it behaves like a
> > programmable (r/w) device with MMIO registers. Switching between both
> > modes is explicit (see
> > "OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c"); perhaps some SEV
> > controls could be hooked in there.
>
> Hmmm wait a second, we *already* execute code from pflash (namely SEC
> code from OVMF_CODE.fd), and instruction fetch always entails decryption.
>
> Furthermore, in SEC, we decompress PEI and DXE FVs from pflash
> (OVMF_CODE.fd) to RAM -- the decompression code runs in long mode, plus
> you modified the OVMF/X64 reset vector to set the C-bit in all PTEs,
> covering the low 4GB of guest RAM.
>
> This tells me that at least read and execute from pflash work with SEV
> decryption already. Is this expected?


Yes, this is expected. During the machine_init, qemu creates two pflash units.
The pflash unit#0 contains OVMF_CODE.fd and unit #1 points to OVMF_VARS.fd.
As part of machine creation, we encrypt the unit#0 using KVM_SEV_LAUNCH_UPDATE
command. The unit#0 is mapped at the x86 reset vector. On vm_start, guest
fetches the code from the unit#0. Since the code was already encrypted hence
guest was able to read and execute the initial bootstrap code just fine.

When OVMF is build with OvmfPkgX64.dsc, SetCr3ForPageTables64 builds the
initial page table. When SEV is active, we add the C-bit pte mask [1] -- it
will ensures that when decompression happen then data get written as
encrypted in the guest RAM.

[1] 
https://github.com/tianocore/edk2/commit/e60af8a1ebb15bfcbf2ecc4afb6cf35084c847aa


When OVMF is build with OvmfPkgIa32X64.dsc, the SEC phase runs in 32-bit mode
hence SEV hardware forces the C-bit to 1.  This will ensure that decompression
routines writes the data as encrypted.



> However, in PlatformPei, we don't report the varstore pflash range via
> any resource descriptor HOB (the varstore chip is at [0xffc00000,
> 0xFFC84000) in the 4MB build).  Consequently, AmdSevDxe clears the C-bit
> under the "NonExistent" branch:
>
> AmdSevDxe:SetMemoryEncDec: Cr3Base=0x0 Physical=0xFEF00000
> Length=0x1100000 Mode=Decrypt CacheFlush=0
>
> Subsequently, "OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c"
> adds the pflash range as EFI_MEMORY_UC | EFI_MEMORY_RUNTIME. The C-bit
> remains clear, AIUI.
>
> OK, I'm totally confused now. Looks like SEV decryption already works
> for pflash (from SEC's example), and transparently at that (???), but we
> fail to use it in QemuFlashFvbServicesRuntimeDxe?


AmdSevDxe clears the C-bit of pflash range in PlatformPei because
QemuFlashFvbServicesRuntimeDxe switches the pflash out from ROMD mode; i.e
it become MMIO. The MMIO regions must be mapped as unencrypted.

SEV hardware engine uses a random encryption key on each run. Since
OVMF_VAR.fd is used for storing the UEFI persistent variable hence we
will not be able to store the data encrypted using the SEV engine. Any
persistent storage should be protected using a disk encryption technologies
(like luks, dm-crypt etc).

Regarding optionROM comments, I will propose something on edk-devel
list. I am thinking at least we should start by adding some kind of bug_on or
warning when a SEV guest BIOS is attempting to copy the option ROM.
I am hoping that once we have secure boot working then we should be able to
handle the signed option ROM, grub.efi or kernel.efi etc.


-Brijesh