Re: [PATCH] KVM: SEV: Return INVALID_INPUT on SNP req/resp buffer access failure

[Sean Christopherson <seanjc@google.com>]

On Tue, Sep 15, 2026, Jacky Li wrote:
> On Tue, Sep 15, 2026 at 8:21 AM Tom Lendacky <thomas.lendacky@amd.com> wrote:
> >
> > On 9/15/26 09:31, Sean Christopherson wrote:
> > > What if we keep this one as -EIO (or better, change it to -EFAULT in a separate
> > > patch?), but add a comment explaning why KVM needs to exit to userspace in this
> > > particular case?  That would be a good compromise; if the guest is attempting to
> > > access non-existent memory, the initial READ will fail, i.e. we still get most
> > > of the behavior Jacky wants.  The only fatal case would be where either userspace
> > > really did screw up, or the guest managed to find read-only memory (though I
> > > would probably argue that's likely also a userspace bug?).
> >
> > That sounds good to me.
> 
> Thinking through the sequence number and VMM bug concerns, I'd still argue
> that returning INVALID_INPUT on kvm_write_guest() failure is preferable to
> exiting to userspace:
> 
> 1. req_gpa and resp_gpa are two independent guest inputs, so a guest can pass
>    a valid req_gpa and a resp_gpa that isn't backed by any memslot to sail
>    through the initial read and fail only on the write.  Exiting to userspace
>    on write failure therefore still lets a guest kill the VM at will.

Drat, I missed that.

> 2. Comparing the two outcomes after a kvm_write_guest() failure:
> 
>    - Case 1 (exit to userspace with -EFAULT/-EIO): unrecoverable.  The PSP's
>      response is lost, and userspace can't rewind the PSP's sequence number,
>      so retrying KVM_RUN would just fail that check.  There is nothing
>      userspace can do other than kill the VM.
> 
>    - Case 2 (return INVALID_INPUT to the guest): the guest disables the VMPCK
>      and moves on to the next one, as Tom mentioned.  The worst case, once all
>      four VMPCKs are exhausted, is that the guest can no longer issue guest
>      requests, but the VM keeps running.
> 
>    Neither is ideal, but Case 2 seems to be better: a localized loss of
>    functionality instead of the entire VM dying over a single bad request.
>    It also leaves the decision to the guest, which can carry on with whatever
>    doesn't depend on attestation, wind down cleanly, and shut itself down on
>    its own terms instead of being killed abruptly.

The problem is, I don't see how the guest can know when it needs to discard the
VMPCK (post-request failures) versus when the VMPCK is still "fine" (pre-request
errors). 

> 3. More generally, when KVM can't tell a guest error apart from a VMM bug at a
>    hypercall boundary, reporting the error to the guest seems to be the safer
>    default: failing the guest request degrades one service while keeping the
>    VM alive and debuggable, whereas exiting to userspace turns a single failed
>    request into a dead VM, and hands an untrusted guest a way to terminate it.

Well, ideally KVM never has to make a choice.

>    And in the context of SNP the guest doesn't trust the VMM anyway, so a VMM
>    that fails to deliver a response is indistinguishable from one that refuses
>    to, and the guest already deals with that by disabling the VMPCK on a
>    VMGEXIT error.  Letting a host problem bleed into the guest is arguably
>    "fine" here, since the guest is designed to cope with it regardless.

FWIW, I'm not terribly concerned about bleeding host issues into the guest, I'm
more concerned about ending up with deferred fatalities and a mess of an "ABI"
between the guest and KVM with respect to handling failures.

What if we "tickle" the resp_gpa before doing the request?  Similar to how CPUs
probe bytes early in XSAVE/XRSTOR to avoid having to unwind later on.  In this
case, the response is restricted to a single page, so we only need to tickle a
single byte.  E.g.

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 2da61843a6f2..b2e74a407c2b 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -4207,6 +4207,7 @@ static int snp_handle_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t resp_
 	struct kvm *kvm = svm->vcpu.kvm;
 	struct kvm_sev_info *sev = to_kvm_sev_info(kvm);
 	sev_ret_code fw_err = 0;
+	u8 tickle = 0;
 	int ret;
 
 	if (!is_sev_snp_guest(&svm->vcpu))
@@ -4219,6 +4220,11 @@ static int snp_handle_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t resp_
 		return 1;
 	}
 
+	if (kvm_write_guest(kvm, resp_gpa, &tickle, sizeof(tickle))) {
+		svm_vmgexit_bad_input(svm, GHCB_ERR_INVALID_INPUT);
+		return 1;
+	}
+
 	data.gctx_paddr = __psp_pa(sev->snp_context);
 	data.req_paddr = __psp_pa(sev->guest_req_buf);
 	data.res_paddr = __psp_pa(sev->guest_resp_buf);
@@ -4232,10 +4238,9 @@ static int snp_handle_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t resp_
 	if (ret && !fw_err)
 		return ret;
 
-	if (kvm_write_guest(kvm, resp_gpa, sev->guest_resp_buf, PAGE_SIZE)) {
-		svm_vmgexit_bad_input(svm, GHCB_ERR_INVALID_INPUT);
-		return 1;
-	}
+	/* Comment goes here. */
+	if (kvm_write_guest(kvm, resp_gpa, sev->guest_resp_buf, PAGE_SIZE))
+		return -EIO;
 
 	/* No action is requested *from KVM* if there was a firmware error. */
 	svm_vmgexit_no_action(svm, SNP_GUEST_ERR(0, fw_err));