Re: [PATCH v4 4/7] KVM: s390: Improve unmapping of large ranges

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] KVM: s390: unit mismatch comparing gfn against _REGION3_SIZE in gmap_unmap_gfn_range_fast()
--

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

KVM: s390: Improve unmapping of large ranges

This commit speeds up unmapping of guest address ranges larger than 1M by
detaching PMD entries, clearing the detached PTEs, and reattaching the PMDs
instead of invalidating each leaf PTE individually.

> diff --git a/arch/s390/kvm/gmap/gmap.c b/arch/s390/kvm/gmap/gmap.c
> index 225fdae3d57ca..04f3ab03a041a 100644
> --- a/arch/s390/kvm/gmap/gmap.c
> +++ b/arch/s390/kvm/gmap/gmap.c
[ ... ]
> @@ -462,6 +453,129 @@ bool gmap_unmap_gfn_range(struct gmap *gmap, struct kvm_memory_slot *slot, gfn_t
[ ... ]
> +static bool gmap_unmap_gfn_range_fast(struct gmap *gmap, gfn_t start, gfn_t end)
> +{
> +	const struct dat_walk_ops ops = {
> +		.pud_entry = _gmap_unmap_pud_fast,
> +		.pmd_entry = _gmap_unmap_pmd_fast,
> +	};
> +	gfn_t aligned_start = ALIGN_DOWN(start, _PAGE_ENTRIES);
> +	gfn_t aligned_end = ALIGN(end, _PAGE_ENTRIES);
> +	bool failed;
> +
> +	lockdep_assert_held_write(&gmap->kvm->mmu_lock);
> +
> +	/* Prefixes are below 2G; if the start is above 2G, nothing to do */
> +	if (aligned_start < _REGION3_SIZE)
> +		gmap_unmap_prefix(gmap, aligned_start, aligned_end);

[Severity: Medium]
In gmap_unmap_gfn_range_fast(), aligned_start is a gfn_t (in 4K page frames),
whereas _REGION3_SIZE is 2G in bytes (1UL << _REGION3_SHIFT, or 0x80000000).

Comparing aligned_start against _REGION3_SIZE checks whether the guest address
is below 8T rather than 2G, causing gmap_unmap_prefix() to walk all vCPUs
while holding kvm->mmu_lock in write mode for any fast-path unmap starting
between 2G and 8T.

Elsewhere in arch/s390/kvm/gmap/gmap.c, gmap_limit_to_type() shifts
_REGION3_SIZE when comparing against a gfn_t:

	if (limit <= _REGION3_SIZE >> PAGE_SHIFT)
		return TABLE_TYPE_SEGMENT;

And arch/s390/kvm/gmap/dat.h defines the region-3 page count directly:

#define _PAGES_PER_SEGMENT	_PAGE_ENTRIES
#define _PAGES_PER_REGION3	(_PAGES_PER_SEGMENT * _CRST_ENTRIES)

Should this check compare aligned_start against _PAGES_PER_REGION3 (or
_REGION3_SIZE >> PAGE_SHIFT) instead of _REGION3_SIZE?

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