On Wed, Feb 2, 2022 at 5:02 PM David Matlack <dmatlack@xxxxxxxxxx> wrote:
>
> Currently make_huge_page_split_spte() assumes execute permissions can be
> granted to any 4K SPTE when splitting huge pages. This is true for the
> TDP MMU but is not necessarily true for the shadow MMU. Huge pages
> mapped by the shadow MMU may be shadowing huge pages that the guest has
> disallowed execute permissions.
>
> No functional change intended.
>
Reviewed-by: Ben Gardon <bgardon@xxxxxxxxxx>
> Signed-off-by: David Matlack <dmatlack@xxxxxxxxxx>
> ---
> arch/x86/kvm/mmu/spte.c | 5 +++--
> arch/x86/kvm/mmu/spte.h | 3 ++-
> arch/x86/kvm/mmu/tdp_mmu.c | 2 +-
> 3 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/spte.c b/arch/x86/kvm/mmu/spte.c
> index 20cf9e0d45dd..7cba5cffc240 100644
> --- a/arch/x86/kvm/mmu/spte.c
> +++ b/arch/x86/kvm/mmu/spte.c
> @@ -215,7 +215,8 @@ static u64 make_spte_executable(u64 spte)
> * This is used during huge page splitting to build the SPTEs that make up the
> * new page table.
> */
> -u64 make_huge_page_split_spte(u64 huge_spte, int huge_level, int index)
> +u64 make_huge_page_split_spte(u64 huge_spte, int huge_level, int index,
> + unsigned int access)
> {
> u64 child_spte;
> int child_level;
> @@ -243,7 +244,7 @@ u64 make_huge_page_split_spte(u64 huge_spte, int huge_level, int index)
> * When splitting to a 4K page, mark the page executable as the
> * NX hugepage mitigation no longer applies.
> */
> - if (is_nx_huge_page_enabled())
> + if (is_nx_huge_page_enabled() && (access & ACC_EXEC_MASK))
> child_spte = make_spte_executable(child_spte);
> }
>
> diff --git a/arch/x86/kvm/mmu/spte.h b/arch/x86/kvm/mmu/spte.h
> index 73f12615416f..c7ccdd5c440d 100644
> --- a/arch/x86/kvm/mmu/spte.h
> +++ b/arch/x86/kvm/mmu/spte.h
> @@ -415,7 +415,8 @@ bool make_spte(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
> unsigned int pte_access, gfn_t gfn, kvm_pfn_t pfn,
> u64 old_spte, bool prefetch, bool can_unsync,
> bool host_writable, u64 *new_spte);
> -u64 make_huge_page_split_spte(u64 huge_spte, int huge_level, int index);
> +u64 make_huge_page_split_spte(u64 huge_spte, int huge_level, int index,
> + unsigned int access);
> u64 make_nonleaf_spte(u64 *child_pt, bool ad_disabled);
> u64 make_mmio_spte(struct kvm_vcpu *vcpu, u64 gfn, unsigned int access);
> u64 mark_spte_for_access_track(u64 spte);
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index 34c451f1eac9..02bfbc1bebbe 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -1310,7 +1310,7 @@ static int tdp_mmu_split_huge_page(struct kvm *kvm, struct tdp_iter *iter,
> * not been linked in yet and thus is not reachable from any other CPU.
> */
> for (i = 0; i < PT64_ENT_PER_PAGE; i++)
> - sp->spt[i] = make_huge_page_split_spte(huge_spte, level, i);
> + sp->spt[i] = make_huge_page_split_spte(huge_spte, level, i, ACC_ALL);
>
> /*
> * Replace the huge spte with a pointer to the populated lower level
> --
> 2.35.0.rc2.247.g8bbb082509-goog
>
