Re: [PATCH 1/2] KVM: SEV: Return appropriate error codes if SEV-ES scratch setup fails

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/9/21 4:23 PM, Sean Christopherson wrote:
Return appropriate error codes if setting up the GHCB scratch area for an
SEV-ES guest fails.  In particular, returning -EINVAL instead of -ENOMEM
when allocating the kernel buffer could be confusing as userspace would
likely suspect a guest issue.

Based on previous feedback and to implement the changes to the GHCB specification, I'm planning on submitting a patch that will return an error code back to the guest, instead of terminating the guest, if the scratch area fails to be setup properly. So you could hold off on this patch if you want.

Thanks,
Tom


Fixes: 8f423a80d299 ("KVM: SVM: Support MMIO for an SEV-ES guest")
Cc: Tom Lendacky <thomas.lendacky@xxxxxxx>
Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
---
  arch/x86/kvm/svm/sev.c | 31 ++++++++++++++++++-------------
  1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 3e2769855e51..ea8069c9b5cb 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2299,7 +2299,7 @@ void pre_sev_run(struct vcpu_svm *svm, int cpu)
  }
#define GHCB_SCRATCH_AREA_LIMIT (16ULL * PAGE_SIZE)
-static bool setup_vmgexit_scratch(struct vcpu_svm *svm, bool sync, u64 len)
+static int setup_vmgexit_scratch(struct vcpu_svm *svm, bool sync, u64 len)
  {
  	struct vmcb_control_area *control = &svm->vmcb->control;
  	struct ghcb *ghcb = svm->ghcb;
@@ -2310,14 +2310,14 @@ static bool setup_vmgexit_scratch(struct vcpu_svm *svm, bool sync, u64 len)
  	scratch_gpa_beg = ghcb_get_sw_scratch(ghcb);
  	if (!scratch_gpa_beg) {
  		pr_err("vmgexit: scratch gpa not provided\n");
-		return false;
+		return -EINVAL;
  	}
scratch_gpa_end = scratch_gpa_beg + len;
  	if (scratch_gpa_end < scratch_gpa_beg) {
  		pr_err("vmgexit: scratch length (%#llx) not valid for scratch address (%#llx)\n",
  		       len, scratch_gpa_beg);
-		return false;
+		return -EINVAL;
  	}
if ((scratch_gpa_beg & PAGE_MASK) == control->ghcb_gpa) {
@@ -2335,7 +2335,7 @@ static bool setup_vmgexit_scratch(struct vcpu_svm *svm, bool sync, u64 len)
  		    scratch_gpa_end > ghcb_scratch_end) {
  			pr_err("vmgexit: scratch area is outside of GHCB shared buffer area (%#llx - %#llx)\n",
  			       scratch_gpa_beg, scratch_gpa_end);
-			return false;
+			return -EINVAL;
  		}
scratch_va = (void *)svm->ghcb;
@@ -2348,18 +2348,18 @@ static bool setup_vmgexit_scratch(struct vcpu_svm *svm, bool sync, u64 len)
  		if (len > GHCB_SCRATCH_AREA_LIMIT) {
  			pr_err("vmgexit: scratch area exceeds KVM limits (%#llx requested, %#llx limit)\n",
  			       len, GHCB_SCRATCH_AREA_LIMIT);
-			return false;
+			return -EINVAL;
  		}
  		scratch_va = kzalloc(len, GFP_KERNEL_ACCOUNT);
  		if (!scratch_va)
-			return false;
+			return -ENOMEM;
if (kvm_read_guest(svm->vcpu.kvm, scratch_gpa_beg, scratch_va, len)) {
  			/* Unable to copy scratch area from guest */
  			pr_err("vmgexit: kvm_read_guest for scratch area failed\n");
kfree(scratch_va);
-			return false;
+			return -EFAULT;
  		}
/*
@@ -2375,7 +2375,7 @@ static bool setup_vmgexit_scratch(struct vcpu_svm *svm, bool sync, u64 len)
  	svm->ghcb_sa = scratch_va;
  	svm->ghcb_sa_len = len;
- return true;
+	return 0;
  }
static void set_ghcb_msr_bits(struct vcpu_svm *svm, u64 value, u64 mask,
@@ -2514,10 +2514,10 @@ int sev_handle_vmgexit(struct kvm_vcpu *vcpu)
  	ghcb_set_sw_exit_info_1(ghcb, 0);
  	ghcb_set_sw_exit_info_2(ghcb, 0);
- ret = -EINVAL;
  	switch (exit_code) {
  	case SVM_VMGEXIT_MMIO_READ:
-		if (!setup_vmgexit_scratch(svm, true, control->exit_info_2))
+		ret = setup_vmgexit_scratch(svm, true, control->exit_info_2);
+		if (ret)
  			break;
ret = kvm_sev_es_mmio_read(vcpu,
@@ -2526,7 +2526,8 @@ int sev_handle_vmgexit(struct kvm_vcpu *vcpu)
  					   svm->ghcb_sa);
  		break;
  	case SVM_VMGEXIT_MMIO_WRITE:
-		if (!setup_vmgexit_scratch(svm, false, control->exit_info_2))
+		ret = setup_vmgexit_scratch(svm, false, control->exit_info_2);
+		if (ret)
  			break;
ret = kvm_sev_es_mmio_write(vcpu,
@@ -2569,6 +2570,7 @@ int sev_handle_vmgexit(struct kvm_vcpu *vcpu)
  		vcpu_unimpl(vcpu,
  			    "vmgexit: unsupported event - exit_info_1=%#llx, exit_info_2=%#llx\n",
  			    control->exit_info_1, control->exit_info_2);
+		ret = -EINVAL;
  		break;
  	default:
  		ret = svm_invoke_exit_handler(vcpu, exit_code);
@@ -2579,8 +2581,11 @@ int sev_handle_vmgexit(struct kvm_vcpu *vcpu)
int sev_es_string_io(struct vcpu_svm *svm, int size, unsigned int port, int in)
  {
-	if (!setup_vmgexit_scratch(svm, in, svm->vmcb->control.exit_info_2))
-		return -EINVAL;
+	int r;
+
+	r = setup_vmgexit_scratch(svm, in, svm->vmcb->control.exit_info_2);
+	if (r)
+		return r;
return kvm_sev_es_string_io(&svm->vcpu, size, port,
  				    svm->ghcb_sa, svm->ghcb_sa_len / size, in);




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux