lkml.org 
[lkml]   [2022]   [Oct]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH Part2 v6 41/49] KVM: SVM: Add support to handle the RMP nested page fault
    From
    On 10/12/2022 5:53 PM, Alper Gun wrote:
    > On Mon, Oct 10, 2022 at 7:32 PM Kalra, Ashish <ashish.kalra@amd.com> wrote:
    >>
    >> Hello Alper,
    >>
    >> On 10/10/2022 5:03 PM, Alper Gun wrote:
    >>> On Mon, Jun 20, 2022 at 4:13 PM Ashish Kalra <Ashish.Kalra@amd.com> wrote:
    >>>>
    >>>> From: Brijesh Singh <brijesh.singh@amd.com>
    >>>>
    >>>> When SEV-SNP is enabled in the guest, the hardware places restrictions on
    >>>> all memory accesses based on the contents of the RMP table. When hardware
    >>>> encounters RMP check failure caused by the guest memory access it raises
    >>>> the #NPF. The error code contains additional information on the access
    >>>> type. See the APM volume 2 for additional information.
    >>>>
    >>>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
    >>>> ---
    >>>> arch/x86/kvm/svm/sev.c | 76 ++++++++++++++++++++++++++++++++++++++++++
    >>>> arch/x86/kvm/svm/svm.c | 14 +++++---
    >>>> 2 files changed, 86 insertions(+), 4 deletions(-)
    >>>>
    >>>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
    >>>> index 4ed90331bca0..7fc0fad87054 100644
    >>>> --- a/arch/x86/kvm/svm/sev.c
    >>>> +++ b/arch/x86/kvm/svm/sev.c
    >>>> @@ -4009,3 +4009,79 @@ void sev_post_unmap_gfn(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn)
    >>>>
    >>>> spin_unlock(&sev->psc_lock);
    >>>> }
    >>>> +
    >>>> +void handle_rmp_page_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
    >>>> +{
    >>>> + int rmp_level, npt_level, rc, assigned;
    >>>> + struct kvm *kvm = vcpu->kvm;
    >>>> + gfn_t gfn = gpa_to_gfn(gpa);
    >>>> + bool need_psc = false;
    >>>> + enum psc_op psc_op;
    >>>> + kvm_pfn_t pfn;
    >>>> + bool private;
    >>>> +
    >>>> + write_lock(&kvm->mmu_lock);
    >>>> +
    >>>> + if (unlikely(!kvm_mmu_get_tdp_walk(vcpu, gpa, &pfn, &npt_level)))
    >>>> + goto unlock;
    >>>> +
    >>>> + assigned = snp_lookup_rmpentry(pfn, &rmp_level);
    >>>> + if (unlikely(assigned < 0))
    >>>> + goto unlock;
    >>>> +
    >>>> + private = !!(error_code & PFERR_GUEST_ENC_MASK);
    >>>> +
    >>>> + /*
    >>>> + * If the fault was due to size mismatch, or NPT and RMP page level's
    >>>> + * are not in sync, then use PSMASH to split the RMP entry into 4K.
    >>>> + */
    >>>> + if ((error_code & PFERR_GUEST_SIZEM_MASK) ||
    >>>> + (npt_level == PG_LEVEL_4K && rmp_level == PG_LEVEL_2M && private)) {
    >>>> + rc = snp_rmptable_psmash(kvm, pfn);
    >>>
    >>>
    >>> Regarding this case:
    >>> RMP level is 4K
    >>> Page table level is 2M
    >>>
    >>> Does this also cause a page fault with size mismatch? If so, we
    >>> shouldn't try psmash because the rmp entry is already 4K.
    >>>
    >>> I see these errors in our tests and I think it may be happening
    >>> because rmp size is already 4K.
    >>>
    >>> [ 1848.752952] psmash failed, gpa 0x191560000 pfn 0x536cd60 rc 7
    >>> [ 2922.879635] psmash failed, gpa 0x102830000 pfn 0x37c8230 rc 7
    >>> [ 3010.983090] psmash failed, gpa 0x104220000 pfn 0x6cf1e20 rc 7
    >>> [ 3170.792050] psmash failed, gpa 0x108a80000 pfn 0x20e0080 rc 7
    >>> [ 3345.955147] psmash failed, gpa 0x11b480000 pfn 0x1545e480 rc 7
    >>>
    >>> Shouldn't we use AND instead of OR in the if statement?
    >>>
    >>
    >> I believe this we can't do, looking at the typical usage case below :
    >>
    >> [ 37.243969] #VMEXIT (NPF) - SIZEM, err 0xc80000005 npt_level 2,
    >> rmp_level 2, private 1
    >> [ 37.243973] trying psmash gpa 0x7f790000 pfn 0x1f5d90
    >>
    >> This is typically the case with #VMEXIT(NPF) with SIZEM error code, when
    >> the guest tries to do PVALIDATE on 4K GHCB pages, in this case both the
    >> RMP table and NPT will be optimally setup to 2M hugepage as can be seen.
    >>
    >> Is it possible to investigate in more depth, when is the this case being
    >> observed:
    >
    > Yes, I added more logs and I can see that these errors happen when RMP
    > level is 4K and NPT level is 2M.
    > psmash fails as expected. I think it is just a log, there is no real
    > issue but the best is not trying psmash if rmp level is 4K.
    >

    Now, the SIZEM bit is only set when PVALIDATE or RMPADJUST fails due to
    guest attempting to validate a 4K page that is backed by a 2MB RMP
    entry, which is not the case here as RMP level is 4K.

    Also, this does not fall into the second case for the same reason.

    #NPF will happen during Guest page table walk if RMP checks fail
    for 2M nested page and RMP.SubPage_Count !=0 OR
    RMP.PageSize != Nested table page size, but then that shouldn't have
    the SIZEM fault bit set.

    This raises concern about some existing race condition, it probably
    can race with
    snp_handle_page_state_change()->snp_make_page_shared()->snp_rmptable_psmash(),
    but that code path seems to be protected from this nested RMP #PF
    handler as they both acquire the kvm->mmu_lock.

    So, this still needs more investigation.

    Can you share what kind of tests are you running to reproduce this
    issue ?

    Thanks,
    Ashish

    >> RMP level is 4K
    >> Page table level is 2M
    >> We shouldn't try psmash because the rmp entry is already 4K.
    >>
    >> Thanks,
    >> Ashish
    >>
    >>> if ((error_code & PFERR_GUEST_SIZEM_MASK) && ...
    >>>

    \
     
     \ /
      Last update: 2022-10-13 17:02    [W:6.003 / U:0.444 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site