Re: [PATCH v3 RESEND 4/5] RISC-V: KVM: Split huge pages during KVM_CLEAR_DIRTY_LOG

[sashiko-bot@kernel.org]

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

New issues:
- [High] Eager huge page splitting during KVM_CLEAR_DIRTY_LOG silently fails for most ranges due to incorrect address alignment in mmu_split_huge_pages().

Pre-existing issues:
- [High] Write-protecting guest page table entries (SPTEs) clobbers concurrent hardware Access/Dirty bit updates.
--

commit 00cb697f813eca012a08732046a12ddb66d6af56
Author: Wang Yechao <wang.yechao255@zte.com.cn>

RISC-V: KVM: Split huge pages during KVM_CLEAR_DIRTY_LOG

This commit splits huge pages on the range specified by KVM_CLEAR_DIRTY_LOG.
It also avoids splitting when enabling dirty logging if
KVM_DIRTY_LOG_INITIALLY_SET is set.

> diff --git a/arch/riscv/kvm/mmu.c b/arch/riscv/kvm/mmu.c
> index bc3bad67d507b..cbda927dd24e3 100644
> --- a/arch/riscv/kvm/mmu.c
> +++ b/arch/riscv/kvm/mmu.c
> @@ -164,6 +164,9 @@ void kvm_arch_mmu_enable_log_dirty_pt_masked(struct kvm *kvm,
>  	kvm_riscv_gstage_init(&gstage, kvm);
>  
>  	kvm_riscv_gstage_wp_range(&gstage, start, end);

[Severity: High]
This is a pre-existing issue, but does calling kvm_riscv_gstage_wp_range()
clobber concurrent hardware Access/Dirty bit updates?

Looking at arch/riscv/kvm/gstage.c, kvm_riscv_gstage_op_pte() uses a
non-atomic read-modify-write when write-protecting:

kvm_riscv_gstage_op_pte() {
    ...
    else if (op == GSTAGE_OP_WP)
        set_pte(ptep, __pte(pte_val(ptep_get(ptep)) & ~_PAGE_WRITE));
    ...
}

If the hardware page-table walker sets the Accessed or Dirty bit between the
read and the write, it seems the bit is permanently lost.

> +
> +	if (kvm_dirty_log_manual_protect_and_init_set(kvm))
> +		mmu_split_huge_pages(&gstage, start, end);

[Severity: High]
Does mmu_split_huge_pages() fail to split overlapping huge pages for small
or unaligned ranges?

Looking at mmu_split_huge_pages() in arch/riscv/kvm/mmu.c, it initializes
the address using ALIGN():

    phys_addr_t addr = ALIGN(start, PMD_SIZE);

If KVM_CLEAR_DIRTY_LOG passes an unaligned start address (e.g., clearing a
small mask), ALIGN() rounds the start address up. This could cause addr to
be greater than or equal to end, resulting in the while (addr < end) loop
terminating immediately and skipping the huge page that overlaps the
beginning of the target range. Should this use ALIGN_DOWN() instead?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624160054463wcDvJaMoydSggcNOWgcfB@zte.com.cn?part=4