Re: [PATCH Part2 v6 39/49] KVM: SVM: Introduce ops for the post gfn map and unmap

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

 



On 11/17/2022 2:18 PM, Peter Gonda wrote:
On Wed, Aug 17, 2022 at 9:47 PM Alper Gun <alpergun@xxxxxxxxxx> wrote:

On Mon, Jun 20, 2022 at 4:12 PM Ashish Kalra <Ashish.Kalra@xxxxxxx> wrote:

From: Brijesh Singh <brijesh.singh@xxxxxxx>

When SEV-SNP is enabled in the guest VM, the guest memory pages can
either be a private or shared. A write from the hypervisor goes through
the RMP checks. If hardware sees that hypervisor is attempting to write
to a guest private page, then it triggers an RMP violation #PF.

To avoid the RMP violation with GHCB pages, added new post_{map,unmap}_gfn
functions to verify if its safe to map GHCB pages.  Uses a spinlock to
protect against the page state change for existing mapped pages.

Need to add generic post_{map,unmap}_gfn() ops that can be used to verify
that its safe to map a given guest page in the hypervisor.

This patch will need to be revisited later after consensus is reached on
how to manage guest private memory as probably UPM private memslots will
be able to handle this page state change more gracefully.

Signed-off-by: Brijesh Singh <brijesh.singh@xxxxxxx>
Signed-off by: Ashish Kalra <ashish.kalra@xxxxxxx>
---
  arch/x86/include/asm/kvm-x86-ops.h |  1 +
  arch/x86/include/asm/kvm_host.h    |  3 ++
  arch/x86/kvm/svm/sev.c             | 48 ++++++++++++++++++++++++++++--
  arch/x86/kvm/svm/svm.c             |  3 ++
  arch/x86/kvm/svm/svm.h             | 11 +++++++
  5 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
index e0068e702692..2dd2bc0cf4c3 100644
--- a/arch/x86/include/asm/kvm-x86-ops.h
+++ b/arch/x86/include/asm/kvm-x86-ops.h
@@ -130,6 +130,7 @@ KVM_X86_OP(vcpu_deliver_sipi_vector)
  KVM_X86_OP_OPTIONAL_RET0(vcpu_get_apicv_inhibit_reasons);
  KVM_X86_OP(alloc_apic_backing_page)
  KVM_X86_OP_OPTIONAL(rmp_page_level_adjust)
+KVM_X86_OP(update_protected_guest_state)

  #undef KVM_X86_OP
  #undef KVM_X86_OP_OPTIONAL
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 49b217dc8d7e..8abc0e724f5c 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1522,7 +1522,10 @@ struct kvm_x86_ops {
         unsigned long (*vcpu_get_apicv_inhibit_reasons)(struct kvm_vcpu *vcpu);

         void *(*alloc_apic_backing_page)(struct kvm_vcpu *vcpu);
+
         void (*rmp_page_level_adjust)(struct kvm *kvm, kvm_pfn_t pfn, int *level);
+
+       int (*update_protected_guest_state)(struct kvm_vcpu *vcpu);
  };

  struct kvm_x86_nested_ops {
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index cb2d1bbb862b..4ed90331bca0 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -341,6 +341,7 @@ static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp)
                 if (ret)
                         goto e_free;

+               spin_lock_init(&sev->psc_lock);
                 ret = sev_snp_init(&argp->error);
         } else {
                 ret = sev_platform_init(&argp->error);
@@ -2828,19 +2829,28 @@ static inline int svm_map_ghcb(struct vcpu_svm *svm, struct kvm_host_map *map)
  {
         struct vmcb_control_area *control = &svm->vmcb->control;
         u64 gfn = gpa_to_gfn(control->ghcb_gpa);
+       struct kvm_vcpu *vcpu = &svm->vcpu;

-       if (kvm_vcpu_map(&svm->vcpu, gfn, map)) {
+       if (kvm_vcpu_map(vcpu, gfn, map)) {
                 /* Unable to map GHCB from guest */
                 pr_err("error mapping GHCB GFN [%#llx] from guest\n", gfn);
                 return -EFAULT;
         }

+       if (sev_post_map_gfn(vcpu->kvm, map->gfn, map->pfn)) {
+               kvm_vcpu_unmap(vcpu, map, false);
+               return -EBUSY;
+       }
+
         return 0;
  }

  static inline void svm_unmap_ghcb(struct vcpu_svm *svm, struct kvm_host_map *map)
  {
-       kvm_vcpu_unmap(&svm->vcpu, map, true);
+       struct kvm_vcpu *vcpu = &svm->vcpu;
+
+       kvm_vcpu_unmap(vcpu, map, true);
+       sev_post_unmap_gfn(vcpu->kvm, map->gfn, map->pfn);
  }

  static void dump_ghcb(struct vcpu_svm *svm)
@@ -3383,6 +3393,8 @@ static int __snp_handle_page_state_change(struct kvm_vcpu *vcpu, enum psc_op op,
                                 return PSC_UNDEF_ERR;
                 }

+               spin_lock(&sev->psc_lock);
+
                 write_lock(&kvm->mmu_lock);

                 rc = kvm_mmu_get_tdp_walk(vcpu, gpa, &pfn, &npt_level);
@@ -3417,6 +3429,8 @@ static int __snp_handle_page_state_change(struct kvm_vcpu *vcpu, enum psc_op op,

                 write_unlock(&kvm->mmu_lock);

+               spin_unlock(&sev->psc_lock);

There is a corner case where the psc_lock is not released. If
kvm_mmu_get_tdp_walk fails, the lock will be kept and will cause soft
lockup.


This is also already fixed for v7.

Thanks,
Ashish



[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]
  Powered by Linux