On Thu, Mar 28, 2019 at 5:40 PM Singh, Brijesh <brijesh.singh@xxxxxxx> wrote:
>
>
>
> On 3/28/19 3:55 PM, Lendacky, Thomas wrote:
> > On 3/28/19 2:57 PM, Nathaniel McCallum wrote:
> >> During the launch of an SEV-enabled guest, a measurement is taken of
> >> all plaintext pages (KVM_SEV_LAUNCH_UPDATE_DATA) and, in SEV-ES, VCPU
> >> state (KVM_SEV_LAUNCH_UPDATE_VMSA) injected into the guest. This
> >
> > I'm just going to address the SEV-ES/ LAUNCH_UPDATE_VMSA portion of this.
> > And, just to be clear, there has been no SEV-ES support submitted yet, so
> > the KVM_SEV_LAUNCH_UPDATE_VMSA doesn't exist yet.
> >
> >> measurement becomes part of the chain of trust reported to the guest
> >> owner.
> >>
> >> Currently, the kernel passes these commands somewhat directly to the
> >> firmware for measurement. Likewise, the firmware calculates the
> >> measurement of the data in the order it is received. This is fragile.
> >> It means that the entire burden for reproducing the measurement falls
> >> on the hypervisor. In turn this means that slight, even unintentional,
> >> changes of the ordering by the hypervisor can result in different
> >> measurements on different hypervisor versions.
> >>
> >> There is also a secondary problem that, even though it controls CPU
> >> state, the KVM_SEV_LAUNCH_UPDATE_VMSA is called on the KVM virtual
> >> machine file descriptor. In order to support multiple vCPUs, you have> to call it once for each vCPU - in the order the vCPUs are assigned to
> >> the guest. Hypervisors typically call this vCPU initialization code in
> >> an independent thread, making this ordering somewhat cumbersome (QEMU
> >> does this today).
> >>
> >> In response to these problems, I'd like to propose the following:
> >>
> >> 1. Calls to KVM_SEV_LAUNCH_UPDATE_VMSA are moved from the VM fd to the vCPU fd.
> >
> > Given #2 below, these calls aren't needed.
> >
> >>
> >> 2. All calls to KVM_SEV_LAUNCH_UPDATE_DATA and
> >> KVM_SEV_LAUNCH_UPDATE_VMSA are batched by the kernel. When
> >> KVM_SEV_LAUNCH_MEASURE is called, the kernel forwards all batched
> >> calls in a well defined order to the firmware. First, it would send
> >> all LAUNCH_UPDATE_DATA commands (in guest address order?). Second, it
> >> would send all LAUNCH_UPDATE_VMSA commands in vCPU # order.
> >
>
>
> The KVM_SEV_LAUNCH_UPDATE_DATA accepts the hva and not gpa. All of the
> PSP command works with the system physical address. So currently, caller
> passes hva in the command and KVM driver find its corresponding spa
> hence relying on address order passed to the KVM_SEV_LAUNCH_UPDATE_DATA
> may lead us to wrong path.
>
> IMO, instead of pushing the batching in kernel, we can have userspace
> batch all the calls and invoke the FW when its ready to encrypt
> the data and get the measurement?
>
> If we are batching the calls then we also need to consider the
> DBG_{ENCRYPT, DECRYPT} command. What should happen if caller issues
> the commands in the below order:
>
> - LAUNCH_UPDATE_DATA
> - LAUNCH_UPDATE_DATA
> - DBG_DECRYPT
> - LAUNCH_UPDATE_DATA
> ....
> ...
> - DBG_ENCRYPT
> ...
> ..
> LAUNCH_MEASURE
> LAUNCH_FINISH
I've changed my mind about KVM_SEV_LAUNCH_UPDATE_DATA. All the
problems above are solvable. However, there is another one that isn't:
we don't have a consistent sorting parameter.
If we use GPA, the hypervisor controls that - so we can't get a
consistent measurement. Both HVA and HPA don't have any relation to VM
contents. In short, batching adds cost but, because we don't have a
valid sorting parameter, it provides no value.
Therefore, I'm in favor of the existing code.
However, I'm still in favor of the kernel handling LAUNCH_UPDATE_VMSA
entirely internally.
> > Since *ALL* vCPUs have to be measured for an SEV-ES guest, the kernel /
> > hypervisor would just call LAUNCH_UPDATE_VMSA for every vCPU in vCPU#
> > order when required (this is not done for an SEV guest). There would be
> > no need to even have a KVM_SEV_LAUNCH_UPDATE_VMSA.
> >
> > Thanks,
> > Tom
> >
> >>
> >> This ensures that, so long as the contents of the VM are the same, the
> >> measurement is the same across all hypervisors and hypervisor
> >> versions.
> >>
