KVM: SEV: Read start/end indices of PSC requests exactly once per #VMGEXIT

Rework Page State Change (PSC) handling to read the guest-provided start
and end indices exactly once, at the beginning of the request.  Re-reading
the indices is "fine", _if_ the guest is well-behaved.  KVM _should_ be
safe against concurrent guest modification of the indices, but there is
zero reason to introduce unnecessary risk.

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Reviewed-by: Michael Roth <michael.roth@amd.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
Message-ID: <20260501202250.2115252-14-seanjc@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-ID: <20260529183549.1104619-14-pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 9f6543cebedf55d3737020ff1278a3b24432e83a..4ebe0d449789883e28b85806dda6bb102dd8e7da 100644 (file)
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3841,7 +3841,7 @@ struct psc_buffer {
        struct psc_entry entries[];
 } __packed;
 
-static int snp_begin_psc(struct vcpu_svm *svm);
+static int snp_do_psc(struct vcpu_svm *svm);
 
 static void snp_complete_psc(struct vcpu_svm *svm, u64 psc_ret)
 {
@@ -3874,6 +3874,7 @@ static void __snp_complete_one_psc(struct vcpu_svm *svm)
                guest_psc->entries[idx].cur_page = entry.pagesize ? 512 : 1;
        }
 
+       sev_es->psc.cur_idx = idx;
        guest_psc->hdr.cur_entry = idx;
 }
 
@@ -3889,37 +3890,19 @@ static int snp_complete_one_psc(struct kvm_vcpu *vcpu)
        __snp_complete_one_psc(svm);
 
        /* Handle the next range (if any). */
-       return snp_begin_psc(svm);
+       return snp_do_psc(svm);
 }
 
-static int snp_begin_psc(struct vcpu_svm *svm)
+static int snp_do_psc(struct vcpu_svm *svm)
 {
        struct vcpu_sev_es_state *sev_es = &svm->sev_es;
        struct psc_buffer *guest_psc = sev_es->ghcb_sa;
        struct kvm_vcpu *vcpu = &svm->vcpu;
        struct psc_entry entry_start;
-       u16 idx, idx_start, idx_end, max_nr_entries;
        int npages;
        bool huge;
        u64 gfn;
-
-       if (!user_exit_on_hypercall(vcpu->kvm, KVM_HC_MAP_GPA_RANGE)) {
-               snp_complete_psc(svm, VMGEXIT_PSC_ERROR_GENERIC);
-               return 1;
-       }
-
-       /*
-        * GHCB v2 requires the scratch area to reside within the GHCB itself,
-        * and PSC requests are only supported for GHCB v2+.  Thus it should be
-        * impossible to exceed the max PSC entry count (which is derived from
-        * the size of the shared GHCB buffer).
-        */
-       max_nr_entries = (sev_es->ghcb_sa_len - sizeof(struct psc_hdr)) /
-                        sizeof(struct psc_entry);
-       if (WARN_ON_ONCE(max_nr_entries > VMGEXIT_PSC_MAX_COUNT)) {
-               snp_complete_psc(svm, VMGEXIT_PSC_ERROR_GENERIC);
-               return 1;
-       }
+       u16 idx;
 
 next_range:
        /* There should be no other PSCs in-flight at this point. */
@@ -3928,21 +3911,8 @@ next_range:
                return 1;
        }
 
-       /*
-        * The PSC descriptor buffer can be modified by a misbehaved guest after
-        * validation, so take care to only use validated copies of values used
-        * for things like array indexing.
-        */
-       idx_start = READ_ONCE(guest_psc->hdr.cur_entry);
-       idx_end = READ_ONCE(guest_psc->hdr.end_entry);
-
-       if (idx_end >= max_nr_entries) {
-               snp_complete_psc(svm, VMGEXIT_PSC_ERROR_INVALID_HDR);
-               return 1;
-       }
-
        /* Find the start of the next range which needs processing. */
-       for (idx = idx_start; idx <= idx_end; idx++) {
+       for (idx = sev_es->psc.cur_idx; idx <= sev_es->psc.end_idx; idx++) {
                entry_start = READ_ONCE(guest_psc->entries[idx]);
 
                gfn = entry_start.gfn;
@@ -3979,7 +3949,7 @@ next_range:
                guest_psc->hdr.cur_entry++;
        }
 
-       if (idx > idx_end) {
+       if (idx > sev_es->psc.end_idx) {
                /* Nothing more to process. */
                snp_complete_psc(svm, 0);
                return 1;
@@ -3994,7 +3964,7 @@ next_range:
         * ranges/operations and can be combined into a single
         * KVM_HC_MAP_GPA_RANGE exit.
         */
-       while (++idx <= idx_end) {
+       while (++idx <= sev_es->psc.end_idx) {
                struct psc_entry entry = READ_ONCE(guest_psc->entries[idx]);
 
                if (entry.operation != entry_start.operation ||
@@ -4044,6 +4014,46 @@ next_range:
        BUG();
 }
 
+static int snp_begin_psc(struct vcpu_svm *svm)
+{
+       struct vcpu_sev_es_state *sev_es = &svm->sev_es;
+       struct psc_buffer *guest_psc = sev_es->ghcb_sa;
+       u16 max_nr_entries;
+
+       if (!user_exit_on_hypercall(svm->vcpu.kvm, KVM_HC_MAP_GPA_RANGE)) {
+               snp_complete_psc(svm, VMGEXIT_PSC_ERROR_GENERIC);
+               return 1;
+       }
+
+       /*
+        * GHCB v2 requires the scratch area to reside within the GHCB itself,
+        * and PSC requests are only supported for GHCB v2+.  Thus it should be
+        * impossible to exceed the max PSC entry count (which is derived from
+        * the size of the shared GHCB buffer).
+        */
+       max_nr_entries = (sev_es->ghcb_sa_len - sizeof(struct psc_hdr)) /
+                        sizeof(struct psc_entry);
+       if (WARN_ON_ONCE(max_nr_entries > VMGEXIT_PSC_MAX_COUNT)) {
+               snp_complete_psc(svm, VMGEXIT_PSC_ERROR_GENERIC);
+               return 1;
+       }
+
+       /*
+        * The PSC descriptor buffer can be modified by a misbehaved guest after
+        * validation, so take care to only use validated copies of values used
+        * for things like array indexing.
+        */
+       sev_es->psc.cur_idx = READ_ONCE(guest_psc->hdr.cur_entry);
+       sev_es->psc.end_idx = READ_ONCE(guest_psc->hdr.end_entry);
+
+       if (sev_es->psc.end_idx >= max_nr_entries) {
+               snp_complete_psc(svm, VMGEXIT_PSC_ERROR_INVALID_HDR);
+               return 1;
+       }
+
+       return snp_do_psc(svm);
+}
+
 /*
  * Invoked as part of svm_vcpu_reset() processing of an init event.
  */

diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 06192bc9c1072f3e9a9e8c7a1ba5cf7e439dffdb..5137416be593d748d8daca5881283e9c666755ce 100644 (file)
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -259,6 +259,7 @@ struct vcpu_sev_es_state {
        /* SNP Page-State-Change buffer entries currently being processed */
        struct {
                u16 cur_idx;
+               u16 end_idx;
                u16 batch_size;
                bool is_2m;
        } psc;