lkml.org 
[lkml]   [2024]   [Mar]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 07/16] KVM: x86: Move synthetic PFERR_* sanity checks to SVM's #NPF handler
From


On 28/02/2024 3:41 pm, Sean Christopherson wrote:
> Move the sanity check that hardware never sets bits that collide with KVM-
> define synthetic bits from kvm_mmu_page_fault() to npf_interception(),
> i.e. make the sanity check #NPF specific. The legacy #PF path already
> WARNs if _any_ of bits 63:32 are set, and the error code that comes from
> VMX's EPT Violatation and Misconfig is 100% synthesized (KVM morphs VMX's
> EXIT_QUALIFICATION into error code flags).
>
> Add a compile-time assert in the legacy #PF handler to make sure that KVM-
> define flags are covered by its existing sanity check on the upper bits.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kvm/mmu/mmu.c | 12 +++---------
> arch/x86/kvm/svm/svm.c | 9 +++++++++
> 2 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 5d892bd59c97..bd342ebd0809 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -4561,6 +4561,9 @@ int kvm_handle_page_fault(struct kvm_vcpu *vcpu, u64 error_code,
> if (WARN_ON_ONCE(error_code >> 32))
> error_code = lower_32_bits(error_code);
>
> + /* Ensure the above sanity check also covers KVM-defined flags. */
> + BUILD_BUG_ON(lower_32_bits(PFERR_SYNTHETIC_MASK));
> +

Could you explain why adding this BUILD_BUG_ON() here, but not ...

> vcpu->arch.l1tf_flush_l1d = true;
> if (!flags) {
> trace_kvm_page_fault(vcpu, fault_address, error_code);
> @@ -5845,15 +5848,6 @@ int noinline kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa, u64 err
> int r, emulation_type = EMULTYPE_PF;
> bool direct = vcpu->arch.mmu->root_role.direct;
>
> - /*
> - * WARN if hardware generates a fault with an error code that collides
> - * with KVM-defined sythentic flags. Clear the flags and continue on,
> - * i.e. don't terminate the VM, as KVM can't possibly be relying on a
> - * flag that KVM doesn't know about.
> - */
> - if (WARN_ON_ONCE(error_code & PFERR_SYNTHETIC_MASK))
> - error_code &= ~PFERR_SYNTHETIC_MASK;
> -
> if (WARN_ON_ONCE(!VALID_PAGE(vcpu->arch.mmu->root.hpa)))
> return RET_PF_RETRY;
>
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index e90b429c84f1..199c4dd8d214 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -2055,6 +2055,15 @@ static int npf_interception(struct kvm_vcpu *vcpu)
> u64 fault_address = svm->vmcb->control.exit_info_2;
> u64 error_code = svm->vmcb->control.exit_info_1;
>
> + /*
> + * WARN if hardware generates a fault with an error code that collides
> + * with KVM-defined sythentic flags. Clear the flags and continue on,
> + * i.e. don't terminate the VM, as KVM can't possibly be relying on a
> + * flag that KVM doesn't know about.
> + */
> + if (WARN_ON_ONCE(error_code & PFERR_SYNTHETIC_MASK))
> + error_code &= ~PFERR_SYNTHETIC_MASK;
> +
> trace_kvm_page_fault(vcpu, fault_address, error_code);
> return kvm_mmu_page_fault(vcpu, fault_address, error_code,
> static_cpu_has(X86_FEATURE_DECODEASSISTS) ?

.. in npf_interception() or some common place like in
kvm_mmu_page_fault()?

Otherwise,

Reviewed-by: Kai Huang <kai.huang@intel.com>

\
 
 \ /
  Last update: 2024-02-29 23:20    [W:1.257 / U:0.716 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site