Re: [PATCH v4 2/2] RISC-V: KVM: Recover gstage huge page mappings during disable-dirty-log

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Arithmetic subtraction of pfn_pte(..., 0) corrupts custom memory type bits (e.g., T-Head PMA) in make_huge_pte().
- [High] Missing congruent alignment check for host page size leads to misaligned guest superpages and guest crashes.
- [Medium] Flawed boundary checks and loop advancement logic prematurely skip valid memory ranges during huge page recovery.
--

commit 83685cea613e39b130969d813c4ab2e7f3ad286f
Author: Wang Yechao <wang.yechao255@zte.com.cn>

RISC-V: KVM: Recover gstage huge page mappings during disable-dirty-log

When dirty logging is enabled, the gstage mappings are split into
4K pages to track dirty pages. If the migration fails or is canceled,
this patch recovers the gstage huge page mappings when dirty logging is
disabled to keep the VM's performance consistent.

> diff --git a/arch/riscv/kvm/gstage.c b/arch/riscv/kvm/gstage.c
> index e5002cb9cbef1..b9414a62c3e04 100644
> --- a/arch/riscv/kvm/gstage.c
> +++ b/arch/riscv/kvm/gstage.c

[ ... ]

> @@ -369,6 +369,73 @@ bool kvm_riscv_gstage_split_huge(struct kvm_gstage *gstage,
>  	return need_flush;
>  }
>  
> +static inline unsigned long make_huge_pte(unsigned long child_pte, int index,
> +					  unsigned long child_page_size)
> +{
> +	unsigned long huge_pte = child_pte;
> +	unsigned long child_pfn_offset;
> +
> +	child_pfn_offset = index * (child_page_size / PAGE_SIZE);
> +	huge_pte -= pte_val(pfn_pte(child_pfn_offset, __pgprot(0)));

[Severity: High]
Does subtracting pfn_pte(..., 0) corrupt custom memory type bits on T-Head
CPUs?

On RISC-V platforms with T-Head extensions, pfn_pte() with __pgprot(0)
automatically applies custom memory attribute bits because the initial
protection value is 0. Subtracting this value inadvertently subtracts these
high memory bits from the PTE rather than just the shifted PFN.

Could this corrupt the huge PTE, potentially clearing required attribute bits
or underflowing into the PFN bits, resulting in an invalid guest mapping?

> +
> +	return huge_pte;
> +}

[ ... ]

> diff --git a/arch/riscv/kvm/mmu.c b/arch/riscv/kvm/mmu.c
> index 6035b5ec95039..8e76187abd89a 100644
> --- a/arch/riscv/kvm/mmu.c
> +++ b/arch/riscv/kvm/mmu.c

[ ... ]

> +static void mmu_recover_huge_pages(struct kvm *kvm, int slot)
> +{

[ ... ]

> +	while (addr < end) {
> +		/*
> +		 * If a very large memslot is mapped exclusively with
> +		 * 4KB host pages, or too many hugepages need to recover,
> +		 * release the kvm->mmu_lock to prevent starvation and
> +		 * lockup detector warnings.
> +		 */
> +		cond_resched_rwlock_write(&kvm->mmu_lock);
> +
> +		if (!kvm->arch.pgd)
> +			break;
> +
> +		hva = gfn_to_hva(kvm, addr >> PAGE_SHIFT);
> +		page_size = get_hva_mapping_size(kvm, hva);

[Severity: High]
Does the host mapping size need to be validated against the memslot alignment
before attempting recovery?

The loop fetches the host's mapping size but fails to validate it against
fault_supports_gstage_huge_mapping(..., page_size). If the host mapping is
1GB, KVM accepts this size without re-verifying that the memslot supports 1GB
mappings (for example, if GPA and HVA are congruently aligned for 2MB but not
for 1GB).

Because the size is not validated for congruent alignment, could subtracting
the offset in make_huge_pte() leave a base PFN that is misaligned for the
superpage size, producing an architecturally invalid PTE?

> +		if (page_size == PAGE_SIZE) {
> +			addr += page_size;
> +			continue;
> +		}
> +
> +		range_start = ALIGN_DOWN(addr, page_size);
> +		range_end = range_start + page_size;
> +
> +		/*
> +		 * Make sure the recover range [range_start, range_end)
> +		 * is within the slot range.
> +		 */
> +		if (range_start < start || range_end > end) {
> +			addr = range_end;

[Severity: Medium]
If the host mapping size is 1GB but the memslot does not start on a 1GB
boundary, range_start < start evaluates to true. Does advancing addr
unconditionally to range_end entirely skip over valid memory that could have
been recovered with smaller huge pages?

> +			continue;
> +		}
> +
> +		out_size = mmu_recover_huge_pages_range(&gstage, page_size,
> +							range_start, range_end);
> +
> +		if (out_size > page_size)
> +			page_size = out_size;
> +
> +		addr = range_start + page_size;

[Severity: Medium]
If out_size is larger than the original page_size (for instance, 1GB vs 2MB),
does adding it to range_start (which was aligned only to the smaller size)
result in a misaligned addr that skips memory in the next loop iteration?

> +	}
> +
> +	write_unlock(&kvm->mmu_lock);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260913070352.3626956-1-wang.yechao255@zte.com.cn?part=2