KVM: x86: SEV: Treat C-bit as legal GPA bit regardless of vCPU mode

Rename cr3_lm_rsvd_bits to reserved_gpa_bits, and use it for all GPA
legality checks.  AMD's APM states:

  If the C-bit is an address bit, this bit is masked from the guest
  physical address when it is translated through the nested page tables.

Thus, any access that can conceivably be run through NPT should ignore
the C-bit when checking for validity.

For features that KVM emulates in software, e.g. MTRRs, there is no
clear direction in the APM for how the C-bit should be handled.  For
such cases, follow the SME behavior inasmuch as possible, since SEV is
is essentially a VM-specific variant of SME.  For SME, the APM states:

  In this case the upper physical address bits are treated as reserved
  when the feature is enabled except where otherwise indicated.

Collecting the various relavant SME snippets in the APM and cross-
referencing the omissions with Linux kernel code, this leaves MTTRs and
APIC_BASE as the only flows that KVM emulates that should _not_ ignore
the C-bit.

Note, this means the reserved bit checks in the page tables are
technically broken.  This will be remedied in a future patch.

Although the page table checks are technically broken, in practice, it's
all but guaranteed to be irrelevant.  NPT is required for SEV, i.e.
shadowing page tables isn't needed in the common case.  Theoretically,
the checks could be in play for nested NPT, but it's extremely unlikely
that anyone is running nested VMs on SEV, as doing so would require L1
to expose sensitive data to L0, e.g. the entire VMCB.  And if anyone is
running nested VMs, L0 can't read the guest's encrypted memory, i.e. L1
would need to put its NPT in shared memory, in which case the C-bit will
never be set.  Or, L1 could use shadow paging, but again, if L0 needs to
read page tables, e.g. to load PDPTRs, the memory can't be encrypted if
L1 has any expectation of L0 doing the right thing.

Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
Message-Id: <20210204000117.3303214-8-seanjc@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index b37afd856bab2bd686442a6ec143ed46081ea02a..717940e97f662fe81ff4cca16937a67d478f55b8 100644 (file)
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -660,7 +660,7 @@ struct kvm_vcpu_arch {
        int cpuid_nent;
        struct kvm_cpuid_entry2 *cpuid_entries;
 
-       unsigned long cr3_lm_rsvd_bits;
+       u64 reserved_gpa_bits;
        int maxphyaddr;
        int max_tdp_level;
 

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index ff16734e6b6a9b7dd989f6f7dd5b87baf77105c9..8d75576edf9c613195c05f9d604fc93cc0c40151 100644 (file)
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -179,7 +179,7 @@ static void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
        vcpu->arch.cr4_guest_rsvd_bits =
            __cr4_reserved_bits(guest_cpuid_has, vcpu);
 
-       vcpu->arch.cr3_lm_rsvd_bits = rsvd_bits(cpuid_maxphyaddr(vcpu), 63);
+       vcpu->arch.reserved_gpa_bits = rsvd_bits(cpuid_maxphyaddr(vcpu), 63);
 
        /* Invoke the vendor callback only after the above state is updated. */
        static_call(kvm_x86_vcpu_after_set_cpuid)(vcpu);

diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
index a9d55ab51e3c407d27a91c5484765299fdaae2a2..f673f45bdf528da6c897cb17f9bd0cd49093bbff 100644 (file)
--- a/arch/x86/kvm/cpuid.h
+++ b/arch/x86/kvm/cpuid.h
@@ -38,7 +38,7 @@ static inline int cpuid_maxphyaddr(struct kvm_vcpu *vcpu)
 
 static inline bool kvm_vcpu_is_legal_gpa(struct kvm_vcpu *vcpu, gpa_t gpa)
 {
-       return !(gpa >> cpuid_maxphyaddr(vcpu));
+       return !(gpa & vcpu->arch.reserved_gpa_bits);
 }
 
 static inline bool kvm_vcpu_is_illegal_gpa(struct kvm_vcpu *vcpu, gpa_t gpa)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 1055b7d0012b1f9572edb97923c317697e166ad2..cc91738ab445cf8fd7d759458ba33d21eba03727 100644 (file)
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -248,7 +248,7 @@ static bool nested_vmcb_checks(struct vcpu_svm *svm, struct vmcb *vmcb12)
        if (vmcb12_lma) {
                if (!(vmcb12->save.cr4 & X86_CR4_PAE) ||
                    !(vmcb12->save.cr0 & X86_CR0_PE) ||
-                   (vmcb12->save.cr3 & vcpu->arch.cr3_lm_rsvd_bits))
+                   kvm_vcpu_is_illegal_gpa(vcpu, vmcb12->save.cr3))
                        return false;
        }
        if (!kvm_is_valid_cr4(&svm->vcpu, vmcb12->save.cr4))

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index fdea0994e94faac7a778ea88e6cb7e07d8508394..4141caea857ae62cd97d8ca878c39a0e9cf78e59 100644 (file)
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -4058,7 +4058,7 @@ static void svm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
        if (sev_guest(vcpu->kvm)) {
                best = kvm_find_cpuid_entry(vcpu, 0x8000001F, 0);
                if (best)
-                       vcpu->arch.cr3_lm_rsvd_bits &= ~(1UL << (best->ebx & 0x3f));
+                       vcpu->arch.reserved_gpa_bits &= ~(1UL << (best->ebx & 0x3f));
        }
 
        if (!kvm_vcpu_apicv_active(vcpu))

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 10414a78b951d075067a54faf7c1db5d05984bbf..077e8b9c9e11049e04467ee76493b54047d7735f 100644 (file)
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1074,8 +1074,7 @@ int kvm_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3)
                return 0;
        }
 
-       if (is_long_mode(vcpu) &&
-           (cr3 & vcpu->arch.cr3_lm_rsvd_bits))
+       if (is_long_mode(vcpu) && kvm_vcpu_is_illegal_gpa(vcpu, cr3))
                return 1;
        else if (is_pae_paging(vcpu) &&
                 !load_pdptrs(vcpu, vcpu->arch.walk_mmu, cr3))
@@ -9701,7 +9700,7 @@ static bool kvm_is_valid_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
                 */
                if (!(sregs->cr4 & X86_CR4_PAE) || !(sregs->efer & EFER_LMA))
                        return false;
-               if (sregs->cr3 & vcpu->arch.cr3_lm_rsvd_bits)
+               if (kvm_vcpu_is_illegal_gpa(vcpu, sregs->cr3))
                        return false;
        } else {
                /*
@@ -10080,7 +10079,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
        fx_init(vcpu);
 
        vcpu->arch.maxphyaddr = cpuid_query_maxphyaddr(vcpu);
-       vcpu->arch.cr3_lm_rsvd_bits = rsvd_bits(cpuid_maxphyaddr(vcpu), 63);
+       vcpu->arch.reserved_gpa_bits = rsvd_bits(cpuid_maxphyaddr(vcpu), 63);
 
        vcpu->arch.pat = MSR_IA32_CR_PAT_DEFAULT;