Re: [PATCH v13 08/12] KVM: X86: Introduce KVM_HC_PAGE_ENC_STATUS hypercall

Paolo Bonzini
2021-04-20T11:10:47Z

On 15/04/21 17:57, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
>
> This hypercall is used by the SEV guest to notify a change in the page
> encryption status to the hypervisor. The hypercall should be invoked
> only when the encryption attribute is changed from encrypted -> decrypted
> and vice versa. By default all guest pages are considered encrypted.
>
> The hypercall exits to userspace to manage the guest shared regions and
> integrate with the userspace VMM's migration code.

I think this should be exposed to userspace as a capability, rather than
as a CPUID bit. Userspace then can enable the capability and set the
CPUID bit if it wants.

The reason is that userspace could pass KVM_GET_SUPPORTED_CPUID to
KVM_SET_CPUID2 and the hypercall then would break the guest.

Paolo

> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: x86@kernel.org
> Cc: kvm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Reviewed-by: Steve Rutherford <srutherford@google.com>
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> Co-developed-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> Documentation/virt/kvm/hypercalls.rst | 15 ++++++++++++++
> arch/x86/include/asm/kvm_host.h | 2 ++
> arch/x86/kvm/svm/sev.c | 1 +
> arch/x86/kvm/x86.c | 29 +++++++++++++++++++++++++++
> include/uapi/linux/kvm_para.h | 1 +
> 5 files changed, 48 insertions(+)
>
> diff --git a/Documentation/virt/kvm/hypercalls.rst b/Documentation/virt/kvm/hypercalls.rst
> index ed4fddd364ea..7aff0cebab7c 100644
> --- a/Documentation/virt/kvm/hypercalls.rst
> +++ b/Documentation/virt/kvm/hypercalls.rst
> @@ -169,3 +169,18 @@ a0: destination APIC ID
>
> :Usage example: When sending a call-function IPI-many to vCPUs, yield if
> any of the IPI target vCPUs was preempted.
> +
> +
> +8. KVM_HC_PAGE_ENC_STATUS
> +-------------------------
> +:Architecture: x86
> +:Status: active
> +:Purpose: Notify the encryption status changes in guest page table (SEV guest)
> +
> +a0: the guest physical address of the start page
> +a1: the number of pages
> +a2: encryption attribute
> +
> + Where:
> + * 1: Encryption attribute is set
> + * 0: Encryption attribute is cleared
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 3768819693e5..42eb0fe3df5d 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1050,6 +1050,8 @@ struct kvm_arch {
>
> bool bus_lock_detection_enabled;
>
> + bool page_enc_hc_enable;
> +
> /* Deflect RDMSR and WRMSR to user space when they trigger a #GP */
> u32 user_space_msr_mask;
> struct kvm_x86_msr_filter __rcu *msr_filter;
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index c9795a22e502..5184a0c0131a 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -197,6 +197,7 @@ static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp)
> sev->active = true;
> sev->asid = asid;
> INIT_LIST_HEAD(&sev->regions_list);
> + kvm->arch.page_enc_hc_enable = true;
>
> return 0;
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index f7d12fca397b..e8986478b653 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -8208,6 +8208,13 @@ static void kvm_sched_yield(struct kvm *kvm, unsigned long dest_id)
> kvm_vcpu_yield_to(target);
> }
>
> +static int complete_hypercall_exit(struct kvm_vcpu *vcpu)
> +{
> + kvm_rax_write(vcpu, vcpu->run->hypercall.ret);
> + ++vcpu->stat.hypercalls;
> + return kvm_skip_emulated_instruction(vcpu);
> +}
> +
> int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
> {
> unsigned long nr, a0, a1, a2, a3, ret;
> @@ -8273,6 +8280,28 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
> kvm_sched_yield(vcpu->kvm, a0);
> ret = 0;
> break;
> + case KVM_HC_PAGE_ENC_STATUS: {
> + u64 gpa = a0, npages = a1, enc = a2;
> +
> + ret = -KVM_ENOSYS;
> + if (!vcpu->kvm->arch.page_enc_hc_enable)
> + break;
> +
> + if (!PAGE_ALIGNED(gpa) || !npages ||
> + gpa_to_gfn(gpa) + npages <= gpa_to_gfn(gpa)) {
> + ret = -EINVAL;
> + break;
> + }
> +
> + vcpu->run->exit_reason = KVM_EXIT_HYPERCALL;
> + vcpu->run->hypercall.nr = KVM_HC_PAGE_ENC_STATUS;
> + vcpu->run->hypercall.args[0] = gpa;
> + vcpu->run->hypercall.args[1] = npages;
> + vcpu->run->hypercall.args[2] = enc;
> + vcpu->run->hypercall.longmode = op_64_bit;
> + vcpu->arch.complete_userspace_io = complete_hypercall_exit;
> + return 0;
> + }
> default:
> ret = -KVM_ENOSYS;
> break;
> diff --git a/include/uapi/linux/kvm_para.h b/include/uapi/linux/kvm_para.h
> index 8b86609849b9..847b83b75dc8 100644
> --- a/include/uapi/linux/kvm_para.h
> +++ b/include/uapi/linux/kvm_para.h
> @@ -29,6 +29,7 @@
> #define KVM_HC_CLOCK_PAIRING 9
> #define KVM_HC_SEND_IPI 10
> #define KVM_HC_SCHED_YIELD 11
> +#define KVM_HC_PAGE_ENC_STATUS 12
>
> /*
> * hypercalls use architecture specific
>