Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 16 May 2022 20:45:00 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     Lai Jiangshan <jiangshanlai@gmail.com>
Cc:     linux-kernel@vger.kernel.org,
        Lai Jiangshan <jiangshan.ljs@antgroup.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Jim Mattson <jmattson@google.com>,
        Joerg Roedel <joro@8bytes.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>, x86@kernel.org,
        "H. Peter Anvin" <hpa@zytor.com>,
        Marcelo Tosatti <mtosatti@redhat.com>,
        Avi Kivity <avi@redhat.com>, kvm@vger.kernel.org
Subject: Re: [PATCH] kvm: x86/svm/nested: Cache PDPTEs for nested NPT in PAE
 paging mode
Message-ID: <YoK3zEVj+DuIBEs7@google.com>
References: <20220415103414.86555-1-jiangshanlai@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220415103414.86555-1-jiangshanlai@gmail.com>
Precedence: bulk

On Fri, Apr 15, 2022, Lai Jiangshan wrote:
> From: Lai Jiangshan <jiangshan.ljs@antgroup.com>
> 
> When NPT enabled L1 is PAE paging, vcpu->arch.mmu->get_pdptrs() which
> is nested_svm_get_tdp_pdptr() reads the guest NPT's PDPTE from memroy
> unconditionally for each call.
> 
> The guest PAE root page is not write-protected.
> 
> The mmu->get_pdptrs() in FNAME(walk_addr_generic) might get different
> values every time or it is different from the return value of
> mmu->get_pdptrs() in mmu_alloc_shadow_roots().
> 
> And it will cause FNAME(fetch) installs the spte in a wrong sp
> or links a sp to a wrong parent since FNAME(gpte_changed) can't
> check these kind of changes.
> 
> Cache the PDPTEs and the problem is resolved.  The guest is responsible
> to info the host if its PAE root page is updated which will cause
> nested vmexit and the host updates the cache when next nested run.

Hmm, no, the guest is responsible for invalidating translations that can be
cached in the TLB, but the guest is not responsible for a full reload of PDPTEs.
Per the APM, the PDPTEs can be cached like regular PTEs:

  Under SVM, however, when the processor is in guest mode with PAE enabled, the
  guest PDPT entries are not cached or validated at this point, but instead are
  loaded and checked on demand in the normal course of address translation, just
  like page directory and page table entries. Any reserved bit violations ared
  etected at the point of use, and result in a page-fault (#PF) exception rather
  than a general-protection (#GP) exception.

So if L1 modifies a PDPTE from !PRESENT (or RESERVED) to PRESENT (and valid), then
any active L2 vCPUs should recognize the new PDPTE without a nested VM-Exit because
the old entry can't have been cached in the TLB.

In practice, snapshotting at nested VMRUN would likely work, but architecturally
it's wrong and could cause problems if L1+L2 are engange in paravirt shenanigans,
e.g. async #PF comes to mind.

I believe the correct way to fix this is to write-protect nNPT PDPTEs like all other
shadow pages, which shouldn't be too awful to do as part of your series to route
PDPTEs through kvm_mmu_get_page().