Re: [PATCH v4 RESEND 2/5] KVM: riscv: Avoid THP adjustment for existing 4K G-stage leaves

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch's fix for an infinite page fault livelock is incomplete. It fails to detect a split THP when the specific faulting GPA lacks a valid 4K leaf, leading to an incorrect PMD alignment and a continuing infinite loop.
--

--- Patch [2]: [PATCH v4 RESEND 2/5] KVM: riscv: Avoid THP adjustment for existing 4K G-stage leaves ---
commit e6651e4debdd3eb4a9adeb5c1e519831130009d2
Author: Jinyu Tang <[email protected]>
KVM: riscv: Avoid THP adjustment for existing 4K G-stage leaves

This commit attempts to fix a page fault handling issue where
transparent_hugepage_adjust() rewrites the fault GPA to the PMD base,
causing the wrong 4K leaf to be updated. It checks if the original fault
GPA already has a 4K G-stage leaf, and if so, skips THP adjustment.

> diff --git a/arch/riscv/kvm/mmu.c b/arch/riscv/kvm/mmu.c
> index bfd6168ebe308..2fabcd4099919 100644
> --- a/arch/riscv/kvm/mmu.c
> +++ b/arch/riscv/kvm/mmu.c
[ ... ]
> @@ -500,10 +500,21 @@ static int get_hva_mapping_size(struct kvm *kvm,
>  
>  static unsigned long transparent_hugepage_adjust(struct kvm *kvm,
>  						 struct kvm_memory_slot *memslot,
> +						 struct kvm_gstage *gstage,
>  						 unsigned long hva,
>  						 kvm_pfn_t *hfnp, gpa_t *gpa)
>  {
>  	kvm_pfn_t hfn = *hfnp;
> +	u32 ptep_level;
> +	pte_t *ptep;
> +
> +	/*
> +	 * Keep the existing split G-stage leaf and update the original
> +	 * faulting 4K page in the vCPU fault path.
> +	 */
> +	if (kvm_riscv_gstage_get_leaf(gstage, *gpa, &ptep, &ptep_level) &&
> +	    !ptep_level)
> +		return PAGE_SIZE;

[Severity: High]
This is a pre-existing issue, but does the fix remain incomplete when the
specific faulting GPA lacks a valid 4K leaf?

If the guest accesses a 4K page within a THP-backed region during dirty
logging, it gets faulted in. If another page in that same split PMD directory
remains unmapped when dirty logging is disabled, what happens when the guest
eventually faults on the unmapped offset?

In that scenario, kvm_riscv_gstage_get_leaf() for the unmapped GPA returns
false. This allows transparent_hugepage_adjust() to fall through and align the
GPA to the PMD base.

When kvm_riscv_gstage_map_page() is subsequently called with the PMD-aligned
GPA and PMD_SIZE, it finds the existing 4K leaf at the PMD base. Because the
PFNs match, it updates the protection of the base leaf and returns success,
without mapping the originally faulting GPA or replacing the split table.

Can this result in an infinite page fault loop since the faulted page is
never actually mapped?

>  
>  	/*
>  	 * Make sure the adjustment is done only for THP pages. Also make

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2