Re: [PATCH 5.15.y v2 3/8] KVM: x86/mmu: Derive shadow MMU page role from parent

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Direct shadow pages can be allocated with `gpte_is_8_bytes = false`, violating KVM MMU invariants.
--

commit 0b01cee575924096aea0e7bca6d1f776d55aa386
Author: Paolo Bonzini <pbonzini@redhat.com>

KVM: x86/mmu: Derive shadow MMU page role from parent

This patch refactors how shadow page roles are computed, deriving most
information from the parent shadow page rather than calculating it from
scratch. This eliminates the dependency on the vCPU root role when
allocating shadow page tables.

> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
[ ... ]
> @@ -2171,6 +2153,54 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
> +static union kvm_mmu_page_role kvm_mmu_child_role(u64 *sptep, bool direct, unsigned int access)
> +{
> +	struct kvm_mmu_page *parent_sp = sptep_to_sp(sptep);
> +	union kvm_mmu_page_role role;
> +
> +	role = parent_sp->role;
> +	role.level--;
> +	role.access = access;
> +	role.direct = direct;
[ ... ]
> +	if (!role.gpte_is_8_bytes) {
> +		WARN_ON_ONCE(role.level != PG_LEVEL_4K);
> +		role.quadrant = (sptep - parent_sp->spt) % 2;
> +	}
> +
> +	return role;
> +}

[Severity: High]
Will this result in direct pages inheriting an incorrect quadrant and byte size?

When breaking down a huge page for a 32-bit non-PAE guest, FNAME(fetch) calls
kvm_mmu_get_child_sp() with direct set to true. kvm_mmu_child_role() derives
the role from the indirect parent, leaving gpte_is_8_bytes as false. It then
incorrectly calculates a quadrant for the direct page. Does this violate the
core MMU invariant that direct pages must always use 8-byte PTE semantics?

[ ... ]
> @@ -3408,13 +3437,18 @@ static int mmu_check_root(struct kvm_vcpu *vcpu, gfn_t root_gfn)
> -static hpa_t mmu_alloc_root(struct kvm_vcpu *vcpu, gfn_t gfn, gva_t gva,
> +static hpa_t mmu_alloc_root(struct kvm_vcpu *vcpu, gfn_t gfn, int quadrant,
>  			    u8 level)
>  {
> -	bool direct = vcpu->arch.mmu->mmu_role.base.direct;
> +	union kvm_mmu_page_role role = vcpu->arch.mmu->mmu_role.base;
>  	struct kvm_mmu_page *sp;
>  
> -	sp = kvm_mmu_get_page(vcpu, gfn, gva, level, direct, ACC_ALL);
> +	role.level = level;
> +
> +	if (!role.gpte_is_8_bytes)
> +		role.quadrant = quadrant;
> +
> +	sp = kvm_mmu_get_page(vcpu, gfn, role);

[Severity: High]
Could this cause issues for non-paging guests?

For non-paging guests where TDP is disabled, the base role has direct
set to true but gpte_is_8_bytes set to false. This role is passed
unmodified to mmu_alloc_root(), which seems like it might propagate the
inconsistent state (direct with 4-byte PTEs) to the root allocation.
If a later check asserts that all direct pages use 8-byte PTEs, would
this trigger a host warning or panic on boot?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626174620.1819772-1-pbonzini@redhat.com?part=3