Kernel KVM virtualization development
 help / color / mirror / Atom feed
* [PATCH v2 0/5] KVM: s390: A few misc gmap fixes.
@ 2026-06-10 16:52 Claudio Imbrenda
  2026-06-10 16:52 ` [PATCH v2 1/5] KVM: s390: Silence potential warnings in _gmap_crstep_xchg_atomic() Claudio Imbrenda
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Claudio Imbrenda @ 2026-06-10 16:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: stable, kvm, linux-s390, borntraeger, frankja, david, seiden, nrb,
	schlameuss, gra

A few more minor gmap fixes.

v1->v2:
* Improve suppression of __must_check, by using a pointless if instead
  of casting to (void)
* Fix allocation size for struct vsie_rmap in kvm_s390_mmu_cache_topup()
* Add missing radix_tree_preload() in _gaccess_shadow_fault()
 
Claudio Imbrenda (5):
  KVM: s390: Silence potential warnings in _gmap_crstep_xchg_atomic()
  KVM: s390: Fix unlikely race in try_get_locked_pte()
  KVM: s390: vsie: Fix allocation of struct vsie_rmap
  KVM: s390: vsie: Add missing radix_tree_preload() in
    _gaccess_shadow_fault()
  KVM: s390: vsie: Use mmu cache to allocate rmap

 arch/s390/kvm/dat.c         |  2 +-
 arch/s390/kvm/gaccess.c     | 65 ++++++++++++++++++++++---------------
 arch/s390/kvm/gmap.c        |  7 ++--
 arch/s390/kvm/gmap.h        | 14 ++++++--
 arch/s390/mm/gmap_helpers.c |  6 ++--
 5 files changed, 58 insertions(+), 36 deletions(-)

-- 
2.54.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/5] KVM: s390: Silence potential warnings in _gmap_crstep_xchg_atomic()
  2026-06-10 16:52 [PATCH v2 0/5] KVM: s390: A few misc gmap fixes Claudio Imbrenda
@ 2026-06-10 16:52 ` Claudio Imbrenda
  2026-06-10 16:52 ` [PATCH v2 2/5] KVM: s390: Fix unlikely race in try_get_locked_pte() Claudio Imbrenda
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Claudio Imbrenda @ 2026-06-10 16:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: stable, kvm, linux-s390, borntraeger, frankja, david, seiden, nrb,
	schlameuss, gra

While dat_crstep_xchg_atomic() is marked as __must_check, in this
particular case the return value should be ignored.

Silence potential compiler warnings with a pointless check, and add a
comment to explain the situation.

Fixes: d1adc098ce08 ("KVM: s390: Fix _gmap_crstep_xchg_atomic()")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
CC: stable@vger.kernel.org # 7.1
---
 arch/s390/kvm/gmap.h | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/s390/kvm/gmap.h b/arch/s390/kvm/gmap.h
index 5374f21aaf8d..20881e3ce9d8 100644
--- a/arch/s390/kvm/gmap.h
+++ b/arch/s390/kvm/gmap.h
@@ -279,7 +279,16 @@ static inline bool __must_check _gmap_crstep_xchg_atomic(struct gmap *gmap, unio
 			gmap_handle_vsie_unshadow_event(gmap, gfn);
 		else
 			_gmap_handle_vsie_unshadow_event(gmap, gfn);
-		dat_crstep_xchg_atomic(crstep, oldcrste, newcrste, gfn, gmap->asce);
+		if (!dat_crstep_xchg_atomic(crstep, oldcrste, newcrste, gfn, gmap->asce))
+			return false;
+		/*
+		 * Return false even if the swap was successful, as it only
+		 * indicates that the best effort clearing of the vsie_notif
+		 * bit was successful. The caller will have to try again
+		 * regardless, since the desired value has not been set.
+		 * This pointless check is needed to silence a potential
+		 * __must_check warning.
+		 */
 		return false;
 	}
 	if (!oldcrste.s.fc1.d && newcrste.s.fc1.d && !newcrste.s.fc1.s)
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/5] KVM: s390: Fix unlikely race in try_get_locked_pte()
  2026-06-10 16:52 [PATCH v2 0/5] KVM: s390: A few misc gmap fixes Claudio Imbrenda
  2026-06-10 16:52 ` [PATCH v2 1/5] KVM: s390: Silence potential warnings in _gmap_crstep_xchg_atomic() Claudio Imbrenda
@ 2026-06-10 16:52 ` Claudio Imbrenda
  2026-06-10 16:52 ` [PATCH v2 3/5] KVM: s390: vsie: Fix allocation of struct vsie_rmap Claudio Imbrenda
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Claudio Imbrenda @ 2026-06-10 16:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: stable, kvm, linux-s390, borntraeger, frankja, david, seiden, nrb,
	schlameuss, gra

Fix an unlikely race in try_get_locked_pte(), which could have happened
if puds or pmds get unmapped between the p?dp_get() and p?d_offset()
functions.

Fixes: 89fa757931dc ("KVM: s390: Avoid potentially sleeping while atomic when zapping pages")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
CC: stable@vger.kernel.org # 7.1
---
 arch/s390/mm/gmap_helpers.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/s390/mm/gmap_helpers.c b/arch/s390/mm/gmap_helpers.c
index 1cfe4724fbe2..ee3f37af8aee 100644
--- a/arch/s390/mm/gmap_helpers.c
+++ b/arch/s390/mm/gmap_helpers.c
@@ -51,15 +51,15 @@ pte_t *try_get_locked_pte(struct mm_struct *mm, unsigned long vmaddr, spinlock_t
 	pgd = pgdp_get(pgdp);
 	if (pgd_none(pgd) || !pgd_present(pgd))
 		return NULL;
-	p4dp = p4d_offset(pgdp, vmaddr);
+	p4dp = p4d_offset_lockless(pgdp, pgd, vmaddr);
 	p4d = p4dp_get(p4dp);
 	if (p4d_none(p4d) || !p4d_present(p4d))
 		return NULL;
-	pudp = pud_offset(p4dp, vmaddr);
+	pudp = pud_offset_lockless(p4dp, p4d, vmaddr);
 	pud = pudp_get(pudp);
 	if (pud_none(pud) || pud_leaf(pud) || !pud_present(pud))
 		return NULL;
-	pmdp = pmd_offset(pudp, vmaddr);
+	pmdp = pmd_offset_lockless(pudp, pud, vmaddr);
 	pmd = pmdp_get_lockless(pmdp);
 	if (pmd_none(pmd) || pmd_leaf(pmd) || !pmd_present(pmd))
 		return NULL;
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 3/5] KVM: s390: vsie: Fix allocation of struct vsie_rmap
  2026-06-10 16:52 [PATCH v2 0/5] KVM: s390: A few misc gmap fixes Claudio Imbrenda
  2026-06-10 16:52 ` [PATCH v2 1/5] KVM: s390: Silence potential warnings in _gmap_crstep_xchg_atomic() Claudio Imbrenda
  2026-06-10 16:52 ` [PATCH v2 2/5] KVM: s390: Fix unlikely race in try_get_locked_pte() Claudio Imbrenda
@ 2026-06-10 16:52 ` Claudio Imbrenda
  2026-06-10 17:07   ` sashiko-bot
  2026-06-10 16:52 ` [PATCH v2 4/5] KVM: s390: vsie: Add missing radix_tree_preload() in _gaccess_shadow_fault() Claudio Imbrenda
  2026-06-10 16:52 ` [PATCH v2 5/5] KVM: s390: vsie: Use mmu cache to allocate rmap Claudio Imbrenda
  4 siblings, 1 reply; 9+ messages in thread
From: Claudio Imbrenda @ 2026-06-10 16:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: stable, kvm, linux-s390, borntraeger, frankja, david, seiden, nrb,
	schlameuss, gra

The allocation size for struct vsie_rmap in kvm_s390_mmu_cache_topup()
was wrong due to a copy-paste error.

Fix it by using the correct size.

Fixes: 12f2f61a9e1a ("KVM: s390: KVM page table management functions: allocation")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
CC: stable@vger.kernel.org # 7.1
---
 arch/s390/kvm/dat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/s390/kvm/dat.c b/arch/s390/kvm/dat.c
index 4a41c0247ffa..b3931e3592b5 100644
--- a/arch/s390/kvm/dat.c
+++ b/arch/s390/kvm/dat.c
@@ -45,7 +45,7 @@ int kvm_s390_mmu_cache_topup(struct kvm_s390_mmu_cache *mc)
 		mc->pts[mc->n_pts] = o;
 	}
 	for ( ; mc->n_rmaps < KVM_S390_MMU_CACHE_N_RMAPS; mc->n_rmaps++) {
-		o = kzalloc_obj(*mc->rmaps[0], GFP_KERNEL_ACCOUNT);
+		o = kzalloc(sizeof(struct vsie_rmap), GFP_KERNEL_ACCOUNT);
 		if (!o)
 			return -ENOMEM;
 		mc->rmaps[mc->n_rmaps] = o;
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 4/5] KVM: s390: vsie: Add missing radix_tree_preload() in _gaccess_shadow_fault()
  2026-06-10 16:52 [PATCH v2 0/5] KVM: s390: A few misc gmap fixes Claudio Imbrenda
                   ` (2 preceding siblings ...)
  2026-06-10 16:52 ` [PATCH v2 3/5] KVM: s390: vsie: Fix allocation of struct vsie_rmap Claudio Imbrenda
@ 2026-06-10 16:52 ` Claudio Imbrenda
  2026-06-10 17:06   ` sashiko-bot
  2026-06-10 16:52 ` [PATCH v2 5/5] KVM: s390: vsie: Use mmu cache to allocate rmap Claudio Imbrenda
  4 siblings, 1 reply; 9+ messages in thread
From: Claudio Imbrenda @ 2026-06-10 16:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: stable, kvm, linux-s390, borntraeger, frankja, david, seiden, nrb,
	schlameuss, gra

Add missing radix_tree_preload() in _gaccess_shadow_fault() to
guarantee forward progress. The core of _gaccess_shadow_fault() has
been split into ___gaccess_shadow_fault() in order to simplify locking.

Fixes: e38c884df921 ("KVM: s390: Switch to new gmap")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
CC: stable@vger.kernel.org # 7.1
---
 arch/s390/kvm/gaccess.c | 49 +++++++++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 19 deletions(-)

diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
index 20e28b183c1a..c072b6872bf8 100644
--- a/arch/s390/kvm/gaccess.c
+++ b/arch/s390/kvm/gaccess.c
@@ -1582,35 +1582,46 @@ static int _gaccess_do_shadow(struct kvm_s390_mmu_cache *mc, struct gmap *sg,
 	return _do_shadow_crste(sg, saddr, host, table, entries + LEVEL_MEM, w->p);
 }
 
+static inline int ___gaccess_shadow_fault(struct kvm_vcpu *vcpu, struct gmap *sg, gpa_t saddr,
+					  unsigned long seq, struct pgtwalk *walk)
+{
+	struct gmap *parent;
+	int rc;
+
+	if (kvm_s390_array_needs_retry_safe(vcpu->kvm, seq, walk->raw_entries))
+		return -EAGAIN;
+	parent = READ_ONCE(sg->parent);
+	if (!parent)
+		return -EAGAIN;
+	scoped_guard(spinlock, &parent->children_lock) {
+		if (READ_ONCE(sg->parent) != parent)
+			return -EAGAIN;
+		sg->invalidated = false;
+		rc = _gaccess_do_shadow(vcpu->arch.mc, sg, saddr, walk);
+	}
+	if (!rc)
+		kvm_s390_release_faultin_array(vcpu->kvm, walk->raw_entries, false);
+	return rc;
+}
+
 static inline int _gaccess_shadow_fault(struct kvm_vcpu *vcpu, struct gmap *sg, gpa_t saddr,
 					unsigned long seq, struct pgtwalk *walk)
 {
-	struct gmap *parent;
 	int rc;
 
 	if (kvm_s390_array_needs_retry_unsafe(vcpu->kvm, seq, walk->raw_entries))
 		return -EAGAIN;
-again:
 	rc = kvm_s390_mmu_cache_topup(vcpu->arch.mc);
 	if (rc)
 		return rc;
-	scoped_guard(read_lock, &vcpu->kvm->mmu_lock) {
-		if (kvm_s390_array_needs_retry_safe(vcpu->kvm, seq, walk->raw_entries))
-			return -EAGAIN;
-		parent = READ_ONCE(sg->parent);
-		if (!parent)
-			return -EAGAIN;
-		scoped_guard(spinlock, &parent->children_lock) {
-			if (READ_ONCE(sg->parent) != parent)
-				return -EAGAIN;
-			sg->invalidated = false;
-			rc = _gaccess_do_shadow(vcpu->arch.mc, sg, saddr, walk);
-		}
-		if (rc == -ENOMEM)
-			goto again;
-		if (!rc)
-			kvm_s390_release_faultin_array(vcpu->kvm, walk->raw_entries, false);
-	}
+
+	do {
+		radix_tree_preload(GFP_KERNEL);
+		scoped_guard(read_lock, &vcpu->kvm->mmu_lock)
+			rc = ___gaccess_shadow_fault(vcpu, sg, saddr, seq, walk);
+		radix_tree_preload_end();
+	} while (rc == -ENOMEM);
+
 	return rc;
 }
 
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 5/5] KVM: s390: vsie: Use mmu cache to allocate rmap
  2026-06-10 16:52 [PATCH v2 0/5] KVM: s390: A few misc gmap fixes Claudio Imbrenda
                   ` (3 preceding siblings ...)
  2026-06-10 16:52 ` [PATCH v2 4/5] KVM: s390: vsie: Add missing radix_tree_preload() in _gaccess_shadow_fault() Claudio Imbrenda
@ 2026-06-10 16:52 ` Claudio Imbrenda
  2026-06-10 17:10   ` sashiko-bot
  4 siblings, 1 reply; 9+ messages in thread
From: Claudio Imbrenda @ 2026-06-10 16:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: stable, kvm, linux-s390, borntraeger, frankja, david, seiden, nrb,
	schlameuss, gra

Use kvm_s390_mmu_cache_alloc_rmap() to allocate the rmap in
gmap_insert_rmap(), instead of a normal kzalloc_obj() with GFP_ATOMIC.

This guarantees forward progress.

Fixes: a2c17f9270cc ("KVM: s390: New gmap code")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
CC: stable@vger.kernel.org # 7.1
---
 arch/s390/kvm/gaccess.c | 16 ++++++++--------
 arch/s390/kvm/gmap.c    |  7 ++++---
 arch/s390/kvm/gmap.h    |  3 ++-
 3 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
index c072b6872bf8..cf4b2fdc2454 100644
--- a/arch/s390/kvm/gaccess.c
+++ b/arch/s390/kvm/gaccess.c
@@ -1419,8 +1419,8 @@ static int walk_guest_tables(struct gmap *sg, unsigned long saddr, struct pgtwal
 	return kvm_s390_get_guest_page(kvm, entries + LEVEL_MEM, table.pte.pfra, wr);
 }
 
-static int _do_shadow_pte(struct gmap *sg, gpa_t raddr, union pte *ptep_h, union pte *ptep,
-			  struct guest_fault *f, bool p)
+static int _do_shadow_pte(struct kvm_s390_mmu_cache *mc, struct gmap *sg, gpa_t raddr,
+			  union pte *ptep_h, union pte *ptep, struct guest_fault *f, bool p)
 {
 	union pgste pgste;
 	union pte newpte;
@@ -1430,7 +1430,7 @@ static int _do_shadow_pte(struct gmap *sg, gpa_t raddr, union pte *ptep_h, union
 	lockdep_assert_held(&sg->parent->children_lock);
 
 	scoped_guard(spinlock, &sg->host_to_rmap_lock)
-		rc = gmap_insert_rmap(sg, f->gfn, gpa_to_gfn(raddr), TABLE_TYPE_PAGE_TABLE);
+		rc = gmap_insert_rmap(mc, sg, f->gfn, gpa_to_gfn(raddr), TABLE_TYPE_PAGE_TABLE);
 	if (rc)
 		return rc;
 
@@ -1462,8 +1462,8 @@ static int _do_shadow_pte(struct gmap *sg, gpa_t raddr, union pte *ptep_h, union
 	return 0;
 }
 
-static int _do_shadow_crste(struct gmap *sg, gpa_t raddr, union crste *host, union crste *table,
-			    struct guest_fault *f, bool p)
+static int _do_shadow_crste(struct kvm_s390_mmu_cache *mc, struct gmap *sg, gpa_t raddr,
+			    union crste *host, union crste *table, struct guest_fault *f, bool p)
 {
 	union crste newcrste, oldcrste;
 	unsigned long mask;
@@ -1476,7 +1476,7 @@ static int _do_shadow_crste(struct gmap *sg, gpa_t raddr, union crste *host, uni
 	mask = is_pmd(*table) ? _SEGMENT_FR_MASK : _REGION3_FR_MASK;
 	r_gfn = gpa_to_gfn(raddr) & mask;
 	scoped_guard(spinlock, &sg->host_to_rmap_lock)
-		rc = gmap_insert_rmap(sg, f->gfn & mask, r_gfn, host->h.tt);
+		rc = gmap_insert_rmap(mc, sg, f->gfn & mask, r_gfn, host->h.tt);
 	if (rc)
 		return rc;
 
@@ -1578,8 +1578,8 @@ static int _gaccess_do_shadow(struct kvm_s390_mmu_cache *mc, struct gmap *sg,
 	if (KVM_BUG_ON(l > TABLE_TYPE_REGION3, sg->kvm))
 		return -EFAULT;
 	if (l == TABLE_TYPE_PAGE_TABLE)
-		return _do_shadow_pte(sg, saddr, ptep_h, ptep, entries + LEVEL_MEM, w->p);
-	return _do_shadow_crste(sg, saddr, host, table, entries + LEVEL_MEM, w->p);
+		return _do_shadow_pte(mc, sg, saddr, ptep_h, ptep, entries + LEVEL_MEM, w->p);
+	return _do_shadow_crste(mc, sg, saddr, host, table, entries + LEVEL_MEM, w->p);
 }
 
 static inline int ___gaccess_shadow_fault(struct kvm_vcpu *vcpu, struct gmap *sg, gpa_t saddr,
diff --git a/arch/s390/kvm/gmap.c b/arch/s390/kvm/gmap.c
index 52d55ddea8d4..1d289f8fa3b2 100644
--- a/arch/s390/kvm/gmap.c
+++ b/arch/s390/kvm/gmap.c
@@ -1000,7 +1000,8 @@ int gmap_pv_destroy_range(struct gmap *gmap, gfn_t start, gfn_t end, bool interr
 	return 0;
 }
 
-int gmap_insert_rmap(struct gmap *sg, gfn_t p_gfn, gfn_t r_gfn, int level)
+int gmap_insert_rmap(struct kvm_s390_mmu_cache *mc, struct gmap *sg, gfn_t p_gfn,
+		     gfn_t r_gfn, int level)
 {
 	struct vsie_rmap *rmap __free(kvfree) = NULL;
 	struct vsie_rmap *temp;
@@ -1010,7 +1011,7 @@ int gmap_insert_rmap(struct gmap *sg, gfn_t p_gfn, gfn_t r_gfn, int level)
 	KVM_BUG_ON(!is_shadow(sg), sg->kvm);
 	lockdep_assert_held(&sg->host_to_rmap_lock);
 
-	rmap = kzalloc_obj(*rmap, GFP_ATOMIC);
+	rmap = kvm_s390_mmu_cache_alloc_rmap(mc);
 	if (!rmap)
 		return -ENOMEM;
 
@@ -1057,7 +1058,7 @@ int gmap_protect_rmap(struct kvm_s390_mmu_cache *mc, struct gmap *sg, gfn_t p_gf
 	if (level <= TABLE_TYPE_REGION1) {
 		bitmask = -1UL << (8 + 11 * level);
 		scoped_guard(spinlock, &sg->host_to_rmap_lock)
-			rc = gmap_insert_rmap(sg, p_gfn, r_gfn & bitmask, level);
+			rc = gmap_insert_rmap(mc, sg, p_gfn, r_gfn & bitmask, level);
 	}
 	if (rc)
 		return rc;
diff --git a/arch/s390/kvm/gmap.h b/arch/s390/kvm/gmap.h
index 20881e3ce9d8..1c040472f56d 100644
--- a/arch/s390/kvm/gmap.h
+++ b/arch/s390/kvm/gmap.h
@@ -100,7 +100,8 @@ int gmap_ucas_map(struct gmap *gmap, gfn_t p_gfn, gfn_t c_gfn, unsigned long cou
 void gmap_ucas_unmap(struct gmap *gmap, gfn_t c_gfn, unsigned long count);
 int gmap_enable_skeys(struct gmap *gmap);
 int gmap_pv_destroy_range(struct gmap *gmap, gfn_t start, gfn_t end, bool interruptible);
-int gmap_insert_rmap(struct gmap *sg, gfn_t p_gfn, gfn_t r_gfn, int level);
+int gmap_insert_rmap(struct kvm_s390_mmu_cache *mc, struct gmap *sg, gfn_t p_gfn,
+		     gfn_t r_gfn, int level);
 int gmap_protect_rmap(struct kvm_s390_mmu_cache *mc, struct gmap *sg, gfn_t p_gfn, gfn_t r_gfn,
 		      kvm_pfn_t pfn, int level, bool wr);
 void gmap_set_cmma_all_dirty(struct gmap *gmap);
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 4/5] KVM: s390: vsie: Add missing radix_tree_preload() in _gaccess_shadow_fault()
  2026-06-10 16:52 ` [PATCH v2 4/5] KVM: s390: vsie: Add missing radix_tree_preload() in _gaccess_shadow_fault() Claudio Imbrenda
@ 2026-06-10 17:06   ` sashiko-bot
  0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-06-10 17:06 UTC (permalink / raw)
  To: Claudio Imbrenda
  Cc: Heiko Carstens, kvm, Alexander Gordeev, linux-s390, Vasily Gorbik

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Ignoring the return value of radix_tree_preload() causes an unbalanced unlock and preempt count underflow if preloading fails.
--

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

KVM: s390: vsie: Add missing radix_tree_preload() in _gaccess_shadow_fault()

This commit adds a missing radix_tree_preload() to _gaccess_shadow_fault()
to guarantee forward progress. It also splits the core of the function
into ___gaccess_shadow_fault() to simplify locking.

CC: stable@vger.kernel.org # 7.1

> diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
> index 20e28b183c1ac..c072b6872bf8b 100644
> --- a/arch/s390/kvm/gaccess.c
> +++ b/arch/s390/kvm/gaccess.c

[ ... ]

> @@ -1582,35 +1582,46 @@ static int _gaccess_do_shadow(struct kvm_s390_mmu_cache *mc, struct gmap *sg,

[ ... ]

>  static inline int _gaccess_shadow_fault(struct kvm_vcpu *vcpu, struct gmap *sg, gpa_t saddr,
>  					unsigned long seq, struct pgtwalk *walk)
>  {
>  	int rc;
>  
>  	if (kvm_s390_array_needs_retry_unsafe(vcpu->kvm, seq, walk->raw_entries))
>  		return -EAGAIN;
>  	rc = kvm_s390_mmu_cache_topup(vcpu->arch.mc);
>  	if (rc)
>  		return rc;
> +
> +	do {
> +		radix_tree_preload(GFP_KERNEL);
> +		scoped_guard(read_lock, &vcpu->kvm->mmu_lock)
> +			rc = ___gaccess_shadow_fault(vcpu, sg, saddr, seq, walk);
> +		radix_tree_preload_end();

[Severity: High]
Does this code handle radix_tree_preload() allocation failures correctly?

radix_tree_preload() can return -ENOMEM under memory pressure. When it fails,
it returns without disabling preemption or acquiring the local lock.

Since the return value is ignored here, radix_tree_preload_end() is
unconditionally called at the end of the loop iteration. Could this result
in an unbalanced unlock and preempt count underflow if the preload fails?

> +	} while (rc == -ENOMEM);
> +
>  	return rc;
>  }

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 3/5] KVM: s390: vsie: Fix allocation of struct vsie_rmap
  2026-06-10 16:52 ` [PATCH v2 3/5] KVM: s390: vsie: Fix allocation of struct vsie_rmap Claudio Imbrenda
@ 2026-06-10 17:07   ` sashiko-bot
  0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-06-10 17:07 UTC (permalink / raw)
  To: Claudio Imbrenda
  Cc: Vasily Gorbik, linux-s390, Heiko Carstens, kvm, Alexander Gordeev

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

Pre-existing issues:
- [High] Infinite loop on OOM in handle_sske() and handle_pfmf() due to ignored return value.
--

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

KVM: s390: vsie: Fix allocation of struct vsie_rmap

The allocation size for struct vsie_rmap in kvm_s390_mmu_cache_topup()
was wrong due to a copy-paste error. This patch fixes it by using the
correct size.

> diff --git a/arch/s390/kvm/dat.c b/arch/s390/kvm/dat.c
> index 4a41c0247ffa2..b3931e3592b52 100644
> --- a/arch/s390/kvm/dat.c
> +++ b/arch/s390/kvm/dat.c
> @@ -45,7 +45,7 @@ int kvm_s390_mmu_cache_topup(struct kvm_s390_mmu_cache *mc)
>  		mc->pts[mc->n_pts] = o;
>  	}
>  	for ( ; mc->n_rmaps < KVM_S390_MMU_CACHE_N_RMAPS; mc->n_rmaps++) {
> -		o = kzalloc_obj(*mc->rmaps[0], GFP_KERNEL_ACCOUNT);
> +		o = kzalloc(sizeof(struct vsie_rmap), GFP_KERNEL_ACCOUNT);
>  		if (!o)
>  			return -ENOMEM;
>  		mc->rmaps[mc->n_rmaps] = o;

[Severity: High]
This isn't a bug introduced by this patch, but since we return -ENOMEM
here on allocation failure, are the callers handling this correctly?

Looking at handle_sske() in arch/s390/kvm/priv.c:

	while (start != end) {
		scoped_guard(read_lock, &vcpu->kvm->mmu_lock) {
			rc = dat_cond_set_storage_key(...);
		}
		if (rc > 1)
			return kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
		if (rc == -ENOMEM) {
			kvm_s390_mmu_cache_topup(vcpu->arch.mc);
			continue;
		}
		if (rc < 0)
			return rc;
		start += PAGE_SIZE;
	}

If dat_cond_set_storage_key() fails with -ENOMEM, we call
kvm_s390_mmu_cache_topup() and then continue the loop. Since we ignore the
return value of kvm_s390_mmu_cache_topup(), if it also fails with -ENOMEM
(e.g., under severe memory pressure or with a pending SIGKILL), the loop
will restart with an empty cache.

Can this result in an endless retry loop causing a CPU soft-lockup?

The same pattern seems to exist in handle_pfmf() in the same file.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 5/5] KVM: s390: vsie: Use mmu cache to allocate rmap
  2026-06-10 16:52 ` [PATCH v2 5/5] KVM: s390: vsie: Use mmu cache to allocate rmap Claudio Imbrenda
@ 2026-06-10 17:10   ` sashiko-bot
  0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-06-10 17:10 UTC (permalink / raw)
  To: Claudio Imbrenda
  Cc: linux-s390, Heiko Carstens, Vasily Gorbik, Alexander Gordeev, kvm

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Infinite loop under memory pressure due to MMU cache depletion during shadow fault retries.
--

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

KVM: s390: vsie: Use mmu cache to allocate rmap

The patch modifies gmap_insert_rmap() to allocate rmaps using
kvm_s390_mmu_cache_alloc_rmap() rather than kzalloc_obj() with GFP_ATOMIC.
This aims to ensure forward progress by leveraging pre-allocated cache items.

> diff --git a/arch/s390/kvm/gmap.c b/arch/s390/kvm/gmap.c
> index 52d55ddea8d4f..1d289f8fa3b20 100644
> --- a/arch/s390/kvm/gmap.c
> +++ b/arch/s390/kvm/gmap.c
> @@ -1000,7 +1000,8 @@ int gmap_pv_destroy_range(struct gmap *gmap, gfn_t start, gfn_t end, bool interr
>  	return 0;
>  }
>  
> -int gmap_insert_rmap(struct gmap *sg, gfn_t p_gfn, gfn_t r_gfn, int level)
> +int gmap_insert_rmap(struct kvm_s390_mmu_cache *mc, struct gmap *sg, gfn_t p_gfn,
> +		     gfn_t r_gfn, int level)
>  {
>  	struct vsie_rmap *rmap __free(kvfree) = NULL;
>  	struct vsie_rmap *temp;
> @@ -1010,7 +1011,7 @@ int gmap_insert_rmap(struct gmap *sg, gfn_t p_gfn, gfn_t r_gfn, int level)
>  	KVM_BUG_ON(!is_shadow(sg), sg->kvm);
>  	lockdep_assert_held(&sg->host_to_rmap_lock);
>  
> -	rmap = kzalloc_obj(*rmap, GFP_ATOMIC);
> +	rmap = kvm_s390_mmu_cache_alloc_rmap(mc);

[Severity: High]
Does this code leak the pre-allocated cache items during retries?

If gmap_insert_rmap() finds a duplicate mapping, it returns 0 without returning
the allocated rmap to the MMU cache. Instead, the __free(kvfree) cleanup
attribute returns it to the generic slab allocator:

arch/s390/kvm/gmap.c:gmap_insert_rmap() {
	...
	rmap = kvm_s390_mmu_cache_alloc_rmap(mc);
	if (!rmap)
		return -ENOMEM;

	rmap->r_gfn = r_gfn;
	...
	if (slot) {
		rmap->next = radix_tree_deref_slot_protected(slot, &sg->host_to_rmap_lock);
		for (temp = rmap->next; temp; temp = temp->next) {
			if (temp->val == rmap->val)
				return 0; // rmap freed via __free(kvfree)
	...
}

In the retry loop inside _gaccess_shadow_fault(), memory allocation failures
trigger a retry:

	do {
		radix_tree_preload(GFP_KERNEL);
		scoped_guard(read_lock, &vcpu->kvm->mmu_lock)
			rc = ___gaccess_shadow_fault(vcpu, sg, saddr, seq, walk);
		radix_tree_preload_end();
	} while (rc == -ENOMEM);

The MMU cache is only topped up outside of this loop. Because the pre-allocated
cache items can be quickly depleted across partial retries and freed to the slab
allocator rather than the cache, kvm_s390_mmu_cache_alloc_rmap() will fall back
to kzalloc_obj() with GFP_ATOMIC.

Under memory pressure, this atomic allocation will consistently fail, returning
-ENOMEM. Since the loop never yields, reclaims memory, or re-tops the cache,
could this create a deterministic infinite loop, leading to a soft lockup?

>  	if (!rmap)
>  		return -ENOMEM;

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-06-10 17:10 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-10 16:52 [PATCH v2 0/5] KVM: s390: A few misc gmap fixes Claudio Imbrenda
2026-06-10 16:52 ` [PATCH v2 1/5] KVM: s390: Silence potential warnings in _gmap_crstep_xchg_atomic() Claudio Imbrenda
2026-06-10 16:52 ` [PATCH v2 2/5] KVM: s390: Fix unlikely race in try_get_locked_pte() Claudio Imbrenda
2026-06-10 16:52 ` [PATCH v2 3/5] KVM: s390: vsie: Fix allocation of struct vsie_rmap Claudio Imbrenda
2026-06-10 17:07   ` sashiko-bot
2026-06-10 16:52 ` [PATCH v2 4/5] KVM: s390: vsie: Add missing radix_tree_preload() in _gaccess_shadow_fault() Claudio Imbrenda
2026-06-10 17:06   ` sashiko-bot
2026-06-10 16:52 ` [PATCH v2 5/5] KVM: s390: vsie: Use mmu cache to allocate rmap Claudio Imbrenda
2026-06-10 17:10   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox