Re: [PATCH v1 6/8] KVM: s390: cmma: Fix dirty tracking when removing memslot

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Double decrement and underflow of cmma_dirty_pages counter when recreating unaligned memslots.
--

commit cfdf82bf92088bc4e4a719ccbb0ad71b6708ed76
Author: Claudio Imbrenda <imbrenda@linux.ibm.com>

KVM: s390: cmma: Fix dirty tracking when removing memslot

This commit fixes the dirty tracking counter when a memslot is removed.
It introduces a dat_walk_ops callback to appropriately decrement the
cmma-dirty counter for any cmma-dirty pages being removed.

> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 512c81eee0684..2ac3a9ac46988 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c

[ ... ]

> @@ -5792,11 +5792,19 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,
>  	return 0;
>  }
>  
> +static long cmma_d_count_pte(union pte *ptep, gfn_t gfn, gfn_t next, struct dat_walk *walk)
> +{
> +	if (pgste_of(ptep)->cmma_d)
> +		atomic64_dec(walk->priv);

[Severity: High]
Does this leave the cmma_d bit set in the PGSTE, which could lead to a double
decrement later?

If a memslot that isn't fully 1MB-aligned is removed or moved,
dat_delete_slot() replaces the PTEs with hole tokens but preserves the PGSTEs.
When userspace creates a new memslot over the same GFN range, dat_create_slot()
replaces the hole tokens with empty PTEs, leaving the stale cmma_d == 1 bits
intact.

During migration, could dat_get_cmma() visit these newly recreated empty PTEs,
see cmma_d == 1, clear the bit, and decrement the counter a second time?

This would cause the cmma_dirty_pages counter to underflow. Since this value is
returned to userspace as an unsigned 64-bit integer, an underflow could cause
QEMU's live migration loop to never converge.

Should we clear the cmma_d bit here after decrementing the counter?

> +	return 0;
> +}
> +
>  void kvm_arch_commit_memory_region(struct kvm *kvm,
>  				struct kvm_memory_slot *old,
>  				const struct kvm_memory_slot *new,
>  				enum kvm_mr_change change)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702152406.204782-1-imbrenda@linux.ibm.com?part=6