[PATCH v1 0/3] KVM: s390: Three small fixes

[PATCH v1 0/3] KVM: s390: Three small fixes

[Claudio Imbrenda]

This is a follow-up bugfix series for the previous series
titled "KVM: s390: gmap rewrite, the real deal"

* Fix a small long standing issue when marking as dirty guest pages that
  contain the interrupt indicator bits and summary bits.
* Fix two newly introduced race conditions that can be triggered with
  nested virtualization.

To be applied on top of kvms390/next:
commit f7ab71f178d5 ("KVM: s390: Add explicit padding to struct kvm_s390_keyop")

Claudio Imbrenda (3):
  KVM: s390: Use guest address to mark guest page dirty
  KVM: s390: vsie: Fix race in walk_guest_tables()
  KVM: s390: vsie: Fix race in acquire_gmap_shadow()

 arch/s390/kvm/gaccess.c   |  3 +++
 arch/s390/kvm/gmap.c      | 15 ++++++++++++---
 arch/s390/kvm/interrupt.c |  6 ++++--
 arch/s390/kvm/vsie.c      |  6 +++++-
 include/linux/kvm_host.h  |  2 ++
 5 files changed, 26 insertions(+), 6 deletions(-)

-- 
2.52.0

[PATCH v1 1/3] KVM: s390: Use guest address to mark guest page dirty

[Claudio Imbrenda]

Stop using the userspace address to mark the guest page dirty.
mark_page_dirty() expects a guest frame number, but was being passed a
host virtual frame number. When slot == NULL, mark_page_dirty_in_slot()
does nothing and does not complain.

This means that in some circumstances the dirtiness of the guest page
might have been lost.

Fix by adding two fields in struct kvm_s390_adapter_int to keep the
guest addressses, and use those for mark_page_dirty().

Fixes: f65470661f36 ("KVM: s390/interrupt: do not pin adapter interrupt pages")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 arch/s390/kvm/interrupt.c | 6 ++++--
 include/linux/kvm_host.h  | 2 ++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index f55eca9aa638..1c2bb5cd7e12 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -2768,13 +2768,13 @@ static int adapter_indicators_set(struct kvm *kvm,
 	bit = get_ind_bit(adapter_int->ind_addr,
 			  adapter_int->ind_offset, adapter->swap);
 	set_bit(bit, map);
-	mark_page_dirty(kvm, adapter_int->ind_addr >> PAGE_SHIFT);
+	mark_page_dirty(kvm, adapter_int->ind_gaddr >> PAGE_SHIFT);
 	set_page_dirty_lock(ind_page);
 	map = page_address(summary_page);
 	bit = get_ind_bit(adapter_int->summary_addr,
 			  adapter_int->summary_offset, adapter->swap);
 	summary_set = test_and_set_bit(bit, map);
-	mark_page_dirty(kvm, adapter_int->summary_addr >> PAGE_SHIFT);
+	mark_page_dirty(kvm, adapter_int->summary_gaddr >> PAGE_SHIFT);
 	set_page_dirty_lock(summary_page);
 	srcu_read_unlock(&kvm->srcu, idx);
 
@@ -2870,7 +2870,9 @@ int kvm_set_routing_entry(struct kvm *kvm,
 		if (kvm_is_error_hva(uaddr_s) || kvm_is_error_hva(uaddr_i))
 			return -EFAULT;
 		e->adapter.summary_addr = uaddr_s;
+		e->adapter.summary_gaddr = ue->u.adapter.summary_addr;
 		e->adapter.ind_addr = uaddr_i;
+		e->adapter.ind_gaddr = ue->u.adapter.ind_addr;
 		e->adapter.summary_offset = ue->u.adapter.summary_offset;
 		e->adapter.ind_offset = ue->u.adapter.ind_offset;
 		e->adapter.adapter_id = ue->u.adapter.adapter_id;
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index d93f75b05ae2..deb36007480d 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -645,7 +645,9 @@ static inline unsigned long *kvm_second_dirty_bitmap(struct kvm_memory_slot *mem
 
 struct kvm_s390_adapter_int {
 	u64 ind_addr;
+	u64 ind_gaddr;
 	u64 summary_addr;
+	u64 summary_gaddr;
 	u64 ind_offset;
 	u32 summary_offset;
 	u32 adapter_id;
-- 
2.52.0

[PATCH v1 2/3] KVM: s390: vsie: Fix race in walk_guest_tables()

[Claudio Imbrenda]

It is possible that walk_guest_tables() is called on a shadow gmap that
has been removed already, in which case its parent will be NULL.

In such case, return -EAGAIN and let the callers deal with it.

Fixes: e38c884df921 ("KVM: s390: Switch to new gmap")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 arch/s390/kvm/gaccess.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
index 67de47a81a87..4630b2a067ea 100644
--- a/arch/s390/kvm/gaccess.c
+++ b/arch/s390/kvm/gaccess.c
@@ -1287,7 +1287,10 @@ static int walk_guest_tables(struct gmap *sg, unsigned long saddr, struct pgtwal
 	union asce asce;
 	int rc;
 
+	if (!parent)
+		return -EAGAIN;
 	kvm = parent->kvm;
+	WARN_ON(!kvm);
 	asce = sg->guest_asce;
 	entries = get_entries(w);
 
-- 
2.52.0

[PATCH v1 3/3] KVM: s390: vsie: Fix race in acquire_gmap_shadow()

[Claudio Imbrenda]

The shadow gmap returned by gmap_create_shadow() could get dropped
before taking the gmap->children_lock. This meant that the shadow gmap
was sometimes being used while its reference count was 0.

Fix this by taking the additional reference inside gmap_create_shadow()
while still holding gmap->children_lock, instead of afterwards.

Fixes: e38c884df921 ("KVM: s390: Switch to new gmap")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 arch/s390/kvm/gmap.c | 15 ++++++++++++---
 arch/s390/kvm/vsie.c |  6 +++++-
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/arch/s390/kvm/gmap.c b/arch/s390/kvm/gmap.c
index da222962ef6d..26cd2b208b6f 100644
--- a/arch/s390/kvm/gmap.c
+++ b/arch/s390/kvm/gmap.c
@@ -1179,6 +1179,8 @@ static int gmap_protect_asce_top_level(struct kvm_s390_mmu_cache *mc, struct gma
  * The shadow table will be removed automatically on any change to the
  * PTE mapping for the source table.
  *
+ * The returned shadow gmap will be returned with one extra reference.
+ *
  * Return: A guest address space structure, ERR_PTR(-ENOMEM) if out of memory,
  * ERR_PTR(-EAGAIN) if the caller has to retry and ERR_PTR(-EFAULT) if the
  * parent gmap table could not be protected.
@@ -1189,10 +1191,13 @@ struct gmap *gmap_create_shadow(struct kvm_s390_mmu_cache *mc, struct gmap *pare
 	struct gmap *sg, *new;
 	int rc;
 
-	scoped_guard(spinlock, &parent->children_lock)
+	scoped_guard(spinlock, &parent->children_lock) {
 		sg = gmap_find_shadow(parent, asce, edat_level);
-	if (sg)
-		return sg;
+		if (sg) {
+			gmap_get(sg);
+			return sg;
+		}
+	}
 	/* Create a new shadow gmap. */
 	new = gmap_new(parent->kvm, asce.r ? 1UL << (64 - PAGE_SHIFT) : asce_end(asce));
 	if (!new)
@@ -1206,6 +1211,7 @@ struct gmap *gmap_create_shadow(struct kvm_s390_mmu_cache *mc, struct gmap *pare
 		sg = gmap_find_shadow(parent, asce, edat_level);
 		if (sg) {
 			gmap_put(new);
+			gmap_get(sg);
 			return sg;
 		}
 		if (asce.r) {
@@ -1219,16 +1225,19 @@ struct gmap *gmap_create_shadow(struct kvm_s390_mmu_cache *mc, struct gmap *pare
 			}
 			gmap_add_child(parent, new);
 			/* Nothing to protect, return right away. */
+			gmap_get(new);
 			return new;
 		}
 	}
 
+	gmap_get(new);
 	new->parent = parent;
 	/* Protect while inserting, protects against invalidation races. */
 	rc = gmap_protect_asce_top_level(mc, new);
 	if (rc) {
 		new->parent = NULL;
 		gmap_put(new);
+		gmap_put(new);
 		return ERR_PTR(rc);
 	}
 	return new;
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index faf8b01fa672..d0296491b2f7 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -1256,6 +1256,7 @@ static struct gmap *acquire_gmap_shadow(struct kvm_vcpu *vcpu, struct vsie_page
 			release_gmap_shadow(vsie_page);
 		}
 	}
+again:
 	gmap = gmap_create_shadow(vcpu->arch.mc, vcpu->kvm->arch.gmap, asce, edat);
 	if (IS_ERR(gmap))
 		return gmap;
@@ -1263,11 +1264,14 @@ static struct gmap *acquire_gmap_shadow(struct kvm_vcpu *vcpu, struct vsie_page
 		/* unlikely race condition, remove the previous shadow */
 		if (vsie_page->gmap_cache.gmap)
 			release_gmap_shadow(vsie_page);
+		if (!gmap->parent) {
+			gmap_put(gmap);
+			goto again;
+		}
 		vcpu->kvm->stat.gmap_shadow_create++;
 		list_add(&vsie_page->gmap_cache.list, &gmap->scb_users);
 		vsie_page->gmap_cache.gmap = gmap;
 		prefix_unmapped(vsie_page);
-		gmap_get(gmap);
 	}
 	return gmap;
 }
-- 
2.52.0

Re: [PATCH v1 1/3] KVM: s390: Use guest address to mark guest page dirty

[Janosch Frank]

On 2/6/26 15:35, Claudio Imbrenda wrote:
> Stop using the userspace address to mark the guest page dirty.
> mark_page_dirty() expects a guest frame number, but was being passed a
> host virtual frame number. When slot == NULL, mark_page_dirty_in_slot()
> does nothing and does not complain.
> 
> This means that in some circumstances the dirtiness of the guest page
> might have been lost.
> 
> Fix by adding two fields in struct kvm_s390_adapter_int to keep the
> guest addressses, and use those for mark_page_dirty().
> 
> Fixes: f65470661f36 ("KVM: s390/interrupt: do not pin adapter interrupt pages")
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

Ouff
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>


We had so many of these issues and I wonder if we should move away from 
making everything u64 and enforce type checks in some form.

> ---
>   arch/s390/kvm/interrupt.c | 6 ++++--
>   include/linux/kvm_host.h  | 2 ++
>   2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index f55eca9aa638..1c2bb5cd7e12 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -2768,13 +2768,13 @@ static int adapter_indicators_set(struct kvm *kvm,
>   	bit = get_ind_bit(adapter_int->ind_addr,
>   			  adapter_int->ind_offset, adapter->swap);
>   	set_bit(bit, map);
> -	mark_page_dirty(kvm, adapter_int->ind_addr >> PAGE_SHIFT);
> +	mark_page_dirty(kvm, adapter_int->ind_gaddr >> PAGE_SHIFT);
>   	set_page_dirty_lock(ind_page);
>   	map = page_address(summary_page);
>   	bit = get_ind_bit(adapter_int->summary_addr,
>   			  adapter_int->summary_offset, adapter->swap);
>   	summary_set = test_and_set_bit(bit, map);
> -	mark_page_dirty(kvm, adapter_int->summary_addr >> PAGE_SHIFT);
> +	mark_page_dirty(kvm, adapter_int->summary_gaddr >> PAGE_SHIFT);
>   	set_page_dirty_lock(summary_page);
>   	srcu_read_unlock(&kvm->srcu, idx);
>   
> @@ -2870,7 +2870,9 @@ int kvm_set_routing_entry(struct kvm *kvm,
>   		if (kvm_is_error_hva(uaddr_s) || kvm_is_error_hva(uaddr_i))
>   			return -EFAULT;
>   		e->adapter.summary_addr = uaddr_s;
> +		e->adapter.summary_gaddr = ue->u.adapter.summary_addr;
>   		e->adapter.ind_addr = uaddr_i;
> +		e->adapter.ind_gaddr = ue->u.adapter.ind_addr;
>   		e->adapter.summary_offset = ue->u.adapter.summary_offset;
>   		e->adapter.ind_offset = ue->u.adapter.ind_offset;
>   		e->adapter.adapter_id = ue->u.adapter.adapter_id;
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index d93f75b05ae2..deb36007480d 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -645,7 +645,9 @@ static inline unsigned long *kvm_second_dirty_bitmap(struct kvm_memory_slot *mem
>   
>   struct kvm_s390_adapter_int {
>   	u64 ind_addr;
> +	u64 ind_gaddr;
>   	u64 summary_addr;
> +	u64 summary_gaddr;
>   	u64 ind_offset;
>   	u32 summary_offset;
>   	u32 adapter_id;

Re: [PATCH v1 2/3] KVM: s390: vsie: Fix race in walk_guest_tables()

[Janosch Frank]

On 2/6/26 15:35, Claudio Imbrenda wrote:
> It is possible that walk_guest_tables() is called on a shadow gmap that
> has been removed already, in which case its parent will be NULL.
> 
> In such case, return -EAGAIN and let the callers deal with it.
> 
> Fixes: e38c884df921 ("KVM: s390: Switch to new gmap")
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

Acked-by: Janosch Frank <frankja@linux.ibm.com>

> ---
>   arch/s390/kvm/gaccess.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
> index 67de47a81a87..4630b2a067ea 100644
> --- a/arch/s390/kvm/gaccess.c
> +++ b/arch/s390/kvm/gaccess.c
> @@ -1287,7 +1287,10 @@ static int walk_guest_tables(struct gmap *sg, unsigned long saddr, struct pgtwal
>   	union asce asce;
>   	int rc;
>   
> +	if (!parent)
> +		return -EAGAIN;
>   	kvm = parent->kvm;
> +	WARN_ON(!kvm);
>   	asce = sg->guest_asce;
>   	entries = get_entries(w);
>   

Re: [PATCH v1 1/3] KVM: s390: Use guest address to mark guest page dirty

[Steffen Eiden]

On Fri, Feb 06, 2026 at 03:35:51PM +0100, Claudio Imbrenda wrote:
> Stop using the userspace address to mark the guest page dirty.
> mark_page_dirty() expects a guest frame number, but was being passed a
> host virtual frame number. When slot == NULL, mark_page_dirty_in_slot()
> does nothing and does not complain.
> 
> This means that in some circumstances the dirtiness of the guest page
> might have been lost.
> 
> Fix by adding two fields in struct kvm_s390_adapter_int to keep the
> guest addressses, and use those for mark_page_dirty().
> 
> Fixes: f65470661f36 ("KVM: s390/interrupt: do not pin adapter interrupt pages")
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Great catch!

Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>

...

Re: [PATCH v1 1/3] KVM: s390: Use guest address to mark guest page dirty

[Christoph Schlameuss]

On Fri Feb 6, 2026 at 3:35 PM CET, Claudio Imbrenda wrote:
> Stop using the userspace address to mark the guest page dirty.
> mark_page_dirty() expects a guest frame number, but was being passed a
> host virtual frame number. When slot == NULL, mark_page_dirty_in_slot()
> does nothing and does not complain.
>
> This means that in some circumstances the dirtiness of the guest page
> might have been lost.
>
> Fix by adding two fields in struct kvm_s390_adapter_int to keep the
> guest addressses, and use those for mark_page_dirty().
>
> Fixes: f65470661f36 ("KVM: s390/interrupt: do not pin adapter interrupt pages")
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

Reviewed-by: Christoph Schlameuss <schlameuss@linux.ibm.com>

> ---
>  arch/s390/kvm/interrupt.c | 6 ++++--
>  include/linux/kvm_host.h  | 2 ++
>  2 files changed, 6 insertions(+), 2 deletions(-)

Re: [PATCH v1 2/3] KVM: s390: vsie: Fix race in walk_guest_tables()

[Christoph Schlameuss]

On Fri Feb 6, 2026 at 3:35 PM CET, Claudio Imbrenda wrote:
> It is possible that walk_guest_tables() is called on a shadow gmap that
> has been removed already, in which case its parent will be NULL.
>
> In such case, return -EAGAIN and let the callers deal with it.
>
> Fixes: e38c884df921 ("KVM: s390: Switch to new gmap")
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

Reviewed-by: Christoph Schlameuss <schlameuss@linux.ibm.com>

> ---
>  arch/s390/kvm/gaccess.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
> index 67de47a81a87..4630b2a067ea 100644
> --- a/arch/s390/kvm/gaccess.c
> +++ b/arch/s390/kvm/gaccess.c
> @@ -1287,7 +1287,10 @@ static int walk_guest_tables(struct gmap *sg, unsigned long saddr, struct pgtwal
>  	union asce asce;
>  	int rc;
>  
> +	if (!parent)
> +		return -EAGAIN;
>  	kvm = parent->kvm;
> +	WARN_ON(!kvm);
>  	asce = sg->guest_asce;
>  	entries = get_entries(w);
>  

Re: [PATCH v1 3/3] KVM: s390: vsie: Fix race in acquire_gmap_shadow()

[Christoph Schlameuss]

On Fri Feb 6, 2026 at 3:35 PM CET, Claudio Imbrenda wrote:
> The shadow gmap returned by gmap_create_shadow() could get dropped
> before taking the gmap->children_lock. This meant that the shadow gmap
> was sometimes being used while its reference count was 0.
>
> Fix this by taking the additional reference inside gmap_create_shadow()
> while still holding gmap->children_lock, instead of afterwards.
>
> Fixes: e38c884df921 ("KVM: s390: Switch to new gmap")
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

Reviewed-by: Christoph Schlameuss <schlameuss@linux.ibm.com>

> ---
>  arch/s390/kvm/gmap.c | 15 ++++++++++++---
>  arch/s390/kvm/vsie.c |  6 +++++-
>  2 files changed, 17 insertions(+), 4 deletions(-)