Re: [PATCH v3 0/5] Qemu SEV-ES guest support

[Tom Lendacky]

On 9/21/20 6:48 AM, Dr. David Alan Gilbert wrote:
> * Tom Lendacky (thomas.lendacky@amd.com) wrote:
>> On 9/18/20 5:00 AM, Dr. David Alan Gilbert wrote:
>>> * Tom Lendacky (thomas.lendacky@amd.com) wrote:
>>>> On 9/17/20 12:28 PM, Dr. David Alan Gilbert wrote:
>>>>> * Tom Lendacky (thomas.lendacky@amd.com) wrote:
>>>>>> From: Tom Lendacky <thomas.lendacky@amd.com>
>>>>>>
>>>>>> This patch series provides support for launching an SEV-ES guest.
>>>>>>
>>>>>> Secure Encrypted Virtualization - Encrypted State (SEV-ES) expands on the
>>>>>> SEV support to protect the guest register state from the hypervisor. See
>>>>>> "AMD64 Architecture Programmer's Manual Volume 2: System Programming",
>>>>>> section "15.35 Encrypted State (SEV-ES)" [1].
>>>>>>
>>>>>> In order to allow a hypervisor to perform functions on behalf of a guest,
>>>>>> there is architectural support for notifying a guest's operating system
>>>>>> when certain types of VMEXITs are about to occur. This allows the guest 
>>>>>> to
>>>>>> selectively share information with the hypervisor to satisfy the 
>>>>>> requested
>>>>>> function. The notification is performed using a new exception, the VMM
>>>>>> Communication exception (#VC). The information is shared through the
>>>>>> Guest-Hypervisor Communication Block (GHCB) using the VMGEXIT 
>>>>>> instruction.
>>>>>> The GHCB format and the protocol for using it is documented in "SEV-ES
>>>>>> Guest-Hypervisor Communication Block Standardization" [2].
>>>>>>
>>>>>> The main areas of the Qemu code that are updated to support SEV-ES are
>>>>>> around the SEV guest launch process and AP booting in order to support
>>>>>> booting multiple vCPUs.
>>>>>>
>>>>>> There are no new command line switches required. Instead, the desire for
>>>>>> SEV-ES is presented using the SEV policy object. Bit 2 of the SEV policy
>>>>>> object indicates that SEV-ES is required.
>>>>>>
>>>>>> The SEV launch process is updated in two ways. The first is that a the
>>>>>> KVM_SEV_ES_INIT ioctl is used to initialize the guest instead of the
>>>>>> standard KVM_SEV_INIT ioctl. The second is that before the SEV launch
>>>>>> measurement is calculated, the LAUNCH_UPDATE_VMSA SEV API is invoked for
>>>>>> each vCPU that Qemu has created. Once the LAUNCH_UPDATE_VMSA API has been
>>>>>> invoked, no direct changes to the guest register state can be made.
>>>>>>
>>>>>> AP booting poses some interesting challenges. The INIT-SIPI-SIPI sequence
>>>>>> is typically used to boot the APs. However, the hypervisor is not allowed
>>>>>> to update the guest registers. For the APs, the reset vector must be 
>>>>>> known
>>>>>> in advance. An OVMF method to provide a known reset vector address exists
>>>>>> by providing an SEV information block, identified by UUID, near the end 
>>>>>> of
>>>>>> the firmware [3]. OVMF will program the jump to the actual reset vector 
>>>>>> in
>>>>>> this area of memory. Since the memory location is known in advance, an AP
>>>>>> can be created with the known reset vector address as its starting CS:IP.
>>>>>> The GHCB document [2] talks about how SMP booting under SEV-ES is
>>>>>> performed. SEV-ES also requires the use of the in-kernel irqchip support
>>>>>> in order to minimize the changes required to Qemu to support AP booting.
>>>>>
>>>>> Some random thoughts:
>>>>>     a) Is there something that explicitly disallows SMM?
>>>>
>>>> There isn't currently. Is there a way to know early on that SMM is enabled?
>>>> Could I just call x86_machine_is_smm_enabled() to check that?
>>>>
>>>>>     b) I think all the interfaces you're using are already defined in
>>>>> Linux header files - even if the code to implement them isn't actually
>>>>> upstream in the kernel yet (the launch_update in particular) - we
>>>>> normally wait for the kernel interface to be accepted before taking the
>>>>> QEMU patches, but if the constants are in the headers already I'm not
>>>>> sure what the rule is.
>>>>
>>>> Correct, everything was already present from a Linux header perspective.
>>>>
>>>>>     c) What happens if QEMU reads the register values from the state if
>>>>> the guest is paused - does it just see junk?  I'm just wondering if you
>>>>> need to add checks in places it might try to.
>>>>
>>>> I thought about what to do about calls to read the registers once the guest
>>>> state has become encrypted. I think it would take a lot of changes to make
>>>> Qemu "protected state aware" for what I see as little gain. Qemu is likely
>>>> to see a lot of zeroes or actual register values from the GHCB protocol for
>>>> previous VMGEXITs that took place.
>>>
>>> Yep, that's fair enough - I was curious if we'll hit anything
>>> accidentally still reading it.
>>>
>>> How does SEV-ES interact with the 'NODBG' flag of the guest policy - if
>>> that's 0, and 'debugging of the guest' is allowed, what can you actually
>>> do?
>>
>> The SEV-ES KVM patches will disallow debugging of the guest, or at least
>> setting breakpoints using the debug registers. Gdb can still break in, but
>> you wont get anything reasonable with register dumps and memory dumps.
>>
>> The NODBG policy bit enables or disables the DBG_DECRYPT and DBG_ENCRYPT
>> APIs. So if the guest has allowed debugging, memory dumps could be done
>> using those APIs (for encrypted pages). Registers are a different story
>> because you simply can't update from the hypervisor side under SEV-ES.
>>
>> Under SEV you could do actual debugging if the support was developed and in
>> place.
> 
> Thanks for the explanation - it might be interesting to wire the
> DBG_DECRYPT support up to dump/dump.c for doing full guest memory dumps
> - if the policy has it enabled.

Someone on our team is looking at hooking in those APIs for displaying /
dumping memory. I'll forward this on to them.

Thanks,
Tom

> 
> Dave
> 
>> Thanks,
>> Tom
>>
>>>
>>> Dave
>>>
>>>> Thanks,
>>>> Tom
>>>>
>>>>>
>>>>> Dave
>>>>>
>>>>>> [1] 
>>>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fsystem%2Ffiles%2FTechDocs%2F24593.pdf&amp;data=02%7C01%7Cthomas.lendacky%40amd.com%7C8287b3e99fdc41c682fe08d85e244ee0%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637362858215616687&amp;sdata=d9n0rRa2AvVS1dmKyBoCF%2BapDhV6UUc8mfGpWD2%2FxMU%3D&amp;reserved=0
>>>>>> [2] 
>>>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdeveloper.amd.com%2Fwp-content%2Fresources%2F56421.pdf&amp;data=02%7C01%7Cthomas.lendacky%40amd.com%7C8287b3e99fdc41c682fe08d85e244ee0%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637362858215616687&amp;sdata=84imjHi1kXbEf2omr1sS%2Brl7gKYeGFGKk%2BeTavyluN4%3D&amp;reserved=0
>>>>>> [3] 30937f2f98c4 ("OvmfPkg: Use the SEV-ES work area for the SEV-ES AP 
>>>>>> reset vector")
>>>>>>       
>>>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Fedk2%2Fcommit%2F30937f2f98c42496f2f143fe8374ae7f7e684847&amp;data=02%7C01%7Cthomas.lendacky%40amd.com%7C8287b3e99fdc41c682fe08d85e244ee0%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637362858215616687&amp;sdata=BTWlq%2BqgpjRtUrbEZE1wasyXxs3YFmfKioEvzvWlsYc%3D&amp;reserved=0
>>>>>>
>>>>>> ---
>>>>>>
>>>>>> These patches are based on commit:
>>>>>> d0ed6a69d3 ("Update version for v5.1.0 release")
>>>>>>
>>>>>> (I tried basing on the latest Qemu commit, but I was having build issues
>>>>>> that level)
>>>>>>
>>>>>> A version of the tree can be found at:
>>>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FAMDESE%2Fqemu%2Ftree%2Fsev-es-v11&amp;data=02%7C01%7Cthomas.lendacky%40amd.com%7C8287b3e99fdc41c682fe08d85e244ee0%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637362858215626678&amp;sdata=1o51C3GUrPwodt4yP6ZlkORcrtSfxLUytJtC66OSjSQ%3D&amp;reserved=0
>>>>>>
>>>>>> Changes since v2:
>>>>>> - Add in-kernel irqchip requirement for SEV-ES guests
>>>>>>
>>>>>> Changes since v1:
>>>>>> - Fixed checkpatch.pl errors/warnings
>>>>>>
>>>>>> Tom Lendacky (5):
>>>>>>     sev/i386: Add initial support for SEV-ES
>>>>>>     sev/i386: Require in-kernel irqchip support for SEV-ES guests
>>>>>>     sev/i386: Allow AP booting under SEV-ES
>>>>>>     sev/i386: Don't allow a system reset under an SEV-ES guest
>>>>>>     sev/i386: Enable an SEV-ES guest based on SEV policy
>>>>>>
>>>>>>    accel/kvm/kvm-all.c       |  73 ++++++++++++++++++++++++++
>>>>>>    accel/stubs/kvm-stub.c    |   5 ++
>>>>>>    hw/i386/pc_sysfw.c        |  10 +++-
>>>>>>    include/sysemu/cpus.h     |   2 +
>>>>>>    include/sysemu/hw_accel.h |   5 ++
>>>>>>    include/sysemu/kvm.h      |  18 +++++++
>>>>>>    include/sysemu/sev.h      |   3 ++
>>>>>>    softmmu/cpus.c            |   5 ++
>>>>>>    softmmu/vl.c              |   5 +-
>>>>>>    target/i386/cpu.c         |   1 +
>>>>>>    target/i386/kvm.c         |   2 +
>>>>>>    target/i386/sev-stub.c    |   5 ++
>>>>>>    target/i386/sev.c         | 105 +++++++++++++++++++++++++++++++++++++-
>>>>>>    target/i386/sev_i386.h    |   1 +
>>>>>>    14 files changed, 236 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> -- 
>>>>>> 2.28.0
>>>>>>
>>>>
>>