Re: [PATCH v10 12/16] KVM: x86: Introduce new KVM_FEATURE_SEV_LIVE_MIGRATION feature & Custom MSR.

[Steve Rutherford <srutherford@google.com>]

Hi Ashish,

On Wed, Feb 10, 2021 at 12:37 PM Ashish Kalra <ashish.kalra@amd.com> wrote:
>
> Hello Steve,
>
> We can remove the implicit enabling of this live migration feature
> from svm_vcpu_after_set_cpuid() callback invoked afer KVM_SET_CPUID2
> ioctl, and let this feature flag be controlled by the userspace
> VMM/qemu.
>
> Userspace can set this feature flag explicitly by calling the
> KVM_SET_CPUID2 ioctl and enable this feature whenever it is ready to
> do so.
>
> I have tested this as part of Qemu code :
>
> int kvm_arch_init_vcpu(CPUState *cs)
> {
> ...
> ...
>         c->function = KVM_CPUID_FEATURES | kvm_base;
>         c->eax = env->features[FEAT_KVM];
>         c->eax |= (1 << KVM_FEATURE_SEV_LIVE_MIGRATION);
> ...
> ...
>
>     r = kvm_vcpu_ioctl(cs, KVM_SET_CPUID2, &cpuid_data);
> ...
>
> Let me know if this addresses your concerns.
Removing implicit enablement is one part of the equation.
The other two are:
1) Host userspace being able to ask the kernel if it supports SEV Live Migration
2) Host userspace being able to disable access to the MSR/hypercall

Feature flagging for paravirt features is pretty complicated, since
you need all three parties to negotiate (host userspace/host
kernel/guest), and every single one has veto power. In the end, the
feature should only be available to the guest if every single party
says yes.

For an example of how to handle 1), the new feature flag could be
checked when asking the kernel which cpuid bits it supports by adding
it to the list of features that the kernel mentions in
KVM_GET_SUPPORTED_CPUID.

For example (in KVM's arch/x86/kvm/cpuid.c):
case KVM_CPUID_FEATURES:
==========
entry->eax = (1 << KVM_FEATURE_CLOCKSOURCE) |
    (1 << KVM_FEATURE_NOP_IO_DELAY) |
...
    (1 << KVM_FEATURE_PV_SCHED_YIELD) |
+  (1 << KVM_FEATURE_ASYNC_PF_INT) |
-   (1 << KVM_FEATURE_ASYNC_PF_INT);
+  (1 << KVM_FEATURE_SEV_LIVE_MIGRATION);
==========

Without this, userspace has to infer if the kernel it is on supports that flag.

For an example of how to handle 2), in the new msr handler, KVM should
throw a GP `if (!guest_pv_has(vcpu, KVM_FEATURE_SEV_LIVE_MIGRATION))`
(it can do this by returning th. The issue here is "what if the guest
ignores CPUID and calls the MSR/hypercall anyway". This is a less
important issue as it requires the guest to be malicious, but still
worth resolving. Additionally, the hypercall itself should check if
the MSR has been toggled by the guest.

Thanks,
Steve