[PATCH v1 00/11] KVM: s390: And then... even more fixes again

[PATCH v1 00/11] KVM: s390: And then... even more fixes again

[Claudio Imbrenda]

Another round of mostly unrelated misc fixes.

Mostly small and/or unlikely issues, but they needed to be addressed.

Claudio Imbrenda (11):
  KVM: s390: Properly handle NULL pointer in dat_cond_set_storage_key()
  KVM: s390: Use srcu in kvm_arch_vcpu_unlocked_ioctl()
  KVM: s390: Fix get_all_floating_irqs()
  KVM: s390: Fix dirty marking in adapter_indicators_set*()
  KVM: s390: Fix pgste_get_trylock_multiple()
  KVM: s390: Introduce extended topup for struct kvm_s390_mmu_cache
  KVM: s390: Move all code into kvm_arch_prepare_memory_region()
  KVM: s390: Fix IRQ injection with SIGP Stop and Store Status
  KVM: s390: Fix kvm_s390_clear_pv_state()
  KVM: s390: Fix potential tiny kernel stack leak
  KVM: s390: Fix _gaccess_shadow_fault()

 arch/s390/kvm/dat.c       |  24 ++--
 arch/s390/kvm/dat.h       |  25 +++-
 arch/s390/kvm/gaccess.c   |  10 ++
 arch/s390/kvm/interrupt.c | 245 ++++++++++++++++++++------------------
 arch/s390/kvm/kvm-s390.c  | 126 ++++++++++----------
 arch/s390/kvm/pv.c        |   4 +
 6 files changed, 239 insertions(+), 195 deletions(-)

-- 
2.55.0

[PATCH v1 01/11] KVM: s390: Properly handle NULL pointer in dat_cond_set_storage_key()

[Claudio Imbrenda]

Some callers pass NULL as oldkey. Calling page_cond_set_storage_key()
will cause that NULL pointer to get dereferenced.

Fix by checking for NULL and assigning the pointer to a dummy local
variable to avoid crashes.

Fixes: 8e03e8316eb2 ("KVM: s390: KVM page table management functions: storage keys")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 arch/s390/kvm/dat.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/s390/kvm/dat.c b/arch/s390/kvm/dat.c
index 3f2d6e8902d7..165c704fcf29 100644
--- a/arch/s390/kvm/dat.c
+++ b/arch/s390/kvm/dat.c
@@ -722,9 +722,12 @@ int dat_cond_set_storage_key(struct kvm_s390_mmu_cache *mmc, union asce asce, gf
 	if (rc)
 		return rc;
 
-	if (!ptep)
+	if (!ptep) {
+		if (!oldkey)
+			oldkey = &prev;
 		return page_cond_set_storage_key(large_crste_to_phys(*crstep, gfn), skey, oldkey,
 						 nq, mr, mc);
+	}
 
 	old = pgste_get_lock(ptep);
 	pgste = old;
-- 
2.55.0

Re: [PATCH v1 01/11] KVM: s390: Properly handle NULL pointer in dat_cond_set_storage_key()

[Christian Borntraeger]

Am 11.08.26 um 17:56 schrieb Claudio Imbrenda:
> Some callers pass NULL as oldkey. Calling page_cond_set_storage_key()
> will cause that NULL pointer to get dereferenced.
> 
> Fix by checking for NULL and assigning the pointer to a dummy local
> variable to avoid crashes.
> 
> Fixes: 8e03e8316eb2 ("KVM: s390: KVM page table management functions: storage keys")
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

the whole thing looks a bit complicated, but as as fix I cannot see anything better

Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>



> ---
>   arch/s390/kvm/dat.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/s390/kvm/dat.c b/arch/s390/kvm/dat.c
> index 3f2d6e8902d7..165c704fcf29 100644
> --- a/arch/s390/kvm/dat.c
> +++ b/arch/s390/kvm/dat.c
> @@ -722,9 +722,12 @@ int dat_cond_set_storage_key(struct kvm_s390_mmu_cache *mmc, union asce asce, gf
>   	if (rc)
>   		return rc;
>   
> -	if (!ptep)
> +	if (!ptep) {
> +		if (!oldkey)
> +			oldkey = &prev;
>   		return page_cond_set_storage_key(large_crste_to_phys(*crstep, gfn), skey, oldkey,
>   						 nq, mr, mc);
> +	}
>   
>   	old = pgste_get_lock(ptep);
>   	pgste = old;

Re: [PATCH v1 01/11] KVM: s390: Properly handle NULL pointer in dat_cond_set_storage_key()

[Christoph Schlameuss]

On Tue Aug 11, 2026 at 5:56 PM CEST, Claudio Imbrenda wrote:
> Some callers pass NULL as oldkey. Calling page_cond_set_storage_key()
> will cause that NULL pointer to get dereferenced.
>
> Fix by checking for NULL and assigning the pointer to a dummy local
> variable to avoid crashes.
>
> Fixes: 8e03e8316eb2 ("KVM: s390: KVM page table management functions: storage keys")
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

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

> ---
>  arch/s390/kvm/dat.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/s390/kvm/dat.c b/arch/s390/kvm/dat.c
> index 3f2d6e8902d7..165c704fcf29 100644
> --- a/arch/s390/kvm/dat.c
> +++ b/arch/s390/kvm/dat.c
> @@ -722,9 +722,12 @@ int dat_cond_set_storage_key(struct kvm_s390_mmu_cache *mmc, union asce asce, gf
>  	if (rc)
>  		return rc;
>  
> -	if (!ptep)
> +	if (!ptep) {
> +		if (!oldkey)
> +			oldkey = &prev;
>  		return page_cond_set_storage_key(large_crste_to_phys(*crstep, gfn), skey, oldkey,
>  						 nq, mr, mc);
> +	}
>  
>  	old = pgste_get_lock(ptep);
>  	pgste = old;

[PATCH v1 02/11] KVM: s390: Use srcu in kvm_arch_vcpu_unlocked_ioctl()

[Claudio Imbrenda]

kvm_arch_vcpu_unlocked_ioctl() is called without further locks held, but
kvm_s390_inject_vcpu(), which is called from there, needs either the
kvm->srcu or the slots lock.

Fix by taking the kvm->srcu in kvm_arch_vcpu_unlocked_ioctl().

Fixes: ba5c1e9b6cee ("KVM: s390: interrupt subsystem, cpu timer, waitpsw")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 arch/s390/kvm/kvm-s390.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 518a69c55e85..02c5428ba239 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -5444,6 +5444,8 @@ long kvm_arch_vcpu_unlocked_ioctl(struct file *filp, unsigned int ioctl,
 	void __user *argp = (void __user *)arg;
 	int rc;
 
+	guard(srcu)(&vcpu->kvm->srcu);
+
 	switch (ioctl) {
 	case KVM_S390_IRQ: {
 		struct kvm_s390_irq s390irq;
-- 
2.55.0

Re: [PATCH v1 02/11] KVM: s390: Use srcu in kvm_arch_vcpu_unlocked_ioctl()

[Christian Borntraeger]

Am 11.08.26 um 17:56 schrieb Claudio Imbrenda:
> kvm_arch_vcpu_unlocked_ioctl() is called without further locks held, but
> kvm_s390_inject_vcpu(), which is called from there, needs either the
> kvm->srcu or the slots lock.
> 
> Fix by taking the kvm->srcu in kvm_arch_vcpu_unlocked_ioctl().
> 
> Fixes: ba5c1e9b6cee ("KVM: s390: interrupt subsystem, cpu timer, waitpsw")
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
>   arch/s390/kvm/kvm-s390.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 518a69c55e85..02c5428ba239 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -5444,6 +5444,8 @@ long kvm_arch_vcpu_unlocked_ioctl(struct file *filp, unsigned int ioctl,
>   	void __user *argp = (void __user *)arg;
>   	int rc;
>   
> +	guard(srcu)(&vcpu->kvm->srcu);
> +

not a bug, but just having srcu around both inject would avoid having holding srcu on the
argument usercopy. This is not forbidden but might just take a while when paging.

  scoped_guard(srcu)(&vcpu->kvm->srcu)
                       rc = kvm_s390_inject_vcpu(vcpu, &s390irq);

in any way, no objection to your variant and a real fix.

Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>


>   	switch (ioctl) {
>   	case KVM_S390_IRQ: {
>   		struct kvm_s390_irq s390irq;

Re: [PATCH v1 02/11] KVM: s390: Use srcu in kvm_arch_vcpu_unlocked_ioctl()

[Claudio Imbrenda]

On Tue, 11 Aug 2026 19:26:42 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> Am 11.08.26 um 17:56 schrieb Claudio Imbrenda:
> > kvm_arch_vcpu_unlocked_ioctl() is called without further locks held, but
> > kvm_s390_inject_vcpu(), which is called from there, needs either the
> > kvm->srcu or the slots lock.
> > 
> > Fix by taking the kvm->srcu in kvm_arch_vcpu_unlocked_ioctl().
> > 
> > Fixes: ba5c1e9b6cee ("KVM: s390: interrupt subsystem, cpu timer, waitpsw")
> > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> > ---
> >   arch/s390/kvm/kvm-s390.c | 2 ++
> >   1 file changed, 2 insertions(+)
> > 
> > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> > index 518a69c55e85..02c5428ba239 100644
> > --- a/arch/s390/kvm/kvm-s390.c
> > +++ b/arch/s390/kvm/kvm-s390.c
> > @@ -5444,6 +5444,8 @@ long kvm_arch_vcpu_unlocked_ioctl(struct file *filp, unsigned int ioctl,
> >   	void __user *argp = (void __user *)arg;
> >   	int rc;
> >   
> > +	guard(srcu)(&vcpu->kvm->srcu);
> > +  
> 
> not a bug, but just having srcu around both inject would avoid having holding srcu on the
> argument usercopy. This is not forbidden but might just take a while when paging.
> 
>   scoped_guard(srcu)(&vcpu->kvm->srcu)
>                        rc = kvm_s390_inject_vcpu(vcpu, &s390irq);
> 
> in any way, no objection to your variant and a real fix.
> 
> Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>

I had not considered the user copy; I will definitely respin with the
scoped_guard()

> 
> 
> >   	switch (ioctl) {
> >   	case KVM_S390_IRQ: {
> >   		struct kvm_s390_irq s390irq;  
> 

[PATCH v1 03/11] KVM: s390: Fix get_all_floating_irqs()

[Claudio Imbrenda]

When attempting to report all pending floating interrupt to userspace,
the GISA IPM bits are atomically tested and cleared, and the
corresponding interrupt description is written in the output buffer. If
the output buffer is too small, an error is returned to userspace, but
the GISA IPM bits are now lost.

Fix by moving the GISA test at the end of the function, and keeping
track of which bits have been cleared. In case of error, set the bits
again, so they are not lost.

Fixes: 24160af6cb28 ("KVM: s390: add GISA interrupts to FLIC ioctl interface")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 arch/s390/kvm/interrupt.c | 93 ++++++++++++++++++---------------------
 1 file changed, 44 insertions(+), 49 deletions(-)

diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 6b3f97a7513b..30963e05e0e6 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -2211,15 +2211,14 @@ void kvm_s390_clear_float_irqs(struct kvm *kvm)
 static int get_all_floating_irqs(struct kvm *kvm, u8 __user *usrbuf, u64 len)
 {
 	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
+	struct kvm_s390_irq *buf __free(kvfree) = NULL;
 	struct kvm_s390_interrupt_info *inti;
 	struct kvm_s390_float_interrupt *fi;
-	struct kvm_s390_irq *buf;
 	struct kvm_s390_irq *irq;
+	unsigned int tmp = 0;
 	int max_irqs;
-	int ret = 0;
 	int n = 0;
 	int i;
-	unsigned long flags;
 
 	if (len > KVM_S390_FLIC_MAX_BUFFER || len == 0)
 		return -EINVAL;
@@ -2235,14 +2234,48 @@ static int get_all_floating_irqs(struct kvm *kvm, u8 __user *usrbuf, u64 len)
 
 	max_irqs = len / sizeof(struct kvm_s390_irq);
 
+	fi = &kvm->arch.float_int;
+	scoped_guard(spinlock_irqsave, &fi->lock) {
+		for (i = 0; i < FIRQ_LIST_COUNT; i++) {
+			list_for_each_entry(inti, &fi->lists[i], list) {
+				/* signal userspace to try again */
+				if (n == max_irqs)
+					return -ENOMEM;
+				inti_to_irq(inti, &buf[n]);
+				n++;
+			}
+		}
+		if (test_bit(IRQ_PEND_EXT_SERVICE, &fi->pending_irqs) ||
+		    test_bit(IRQ_PEND_EXT_SERVICE_EV, &fi->pending_irqs)) {
+			/* signal userspace to try again */
+			if (n == max_irqs)
+				return -ENOMEM;
+			irq = (struct kvm_s390_irq *)&buf[n];
+			irq->type = KVM_S390_INT_SERVICE;
+			irq->u.ext = fi->srv_signal;
+			n++;
+		}
+		if (test_bit(IRQ_PEND_MCHK_REP, &fi->pending_irqs)) {
+			/* signal userspace to try again */
+			if (n == max_irqs)
+				return -ENOMEM;
+			irq = (struct kvm_s390_irq *)&buf[n];
+			irq->type = KVM_S390_MCHK;
+			irq->u.mchk = fi->mchk;
+			n++;
+		}
+	}
 	if (gi->origin && gisa_get_ipm(gi->origin)) {
 		for (i = 0; i <= MAX_ISC; i++) {
 			if (n == max_irqs) {
+				/* restore removed bits if returning failure */
+				__atomic_or(tmp, (void *)&gi->origin->ipm);
 				/* signal userspace to try again */
-				ret = -ENOMEM;
-				goto out_nolock;
+				return -ENOMEM;
 			}
 			if (gisa_tac_ipm_gisc(gi->origin, i)) {
+				/* set aside the bits we cleared */
+				tmp |= 1 << (31 - i);
 				irq = (struct kvm_s390_irq *) &buf[n];
 				irq->type = KVM_S390_INT_IO(1, 0, 0, 0);
 				irq->u.io.io_int_word = isc_to_int_word(i);
@@ -2250,53 +2283,15 @@ static int get_all_floating_irqs(struct kvm *kvm, u8 __user *usrbuf, u64 len)
 			}
 		}
 	}
-	fi = &kvm->arch.float_int;
-	spin_lock_irqsave(&fi->lock, flags);
-	for (i = 0; i < FIRQ_LIST_COUNT; i++) {
-		list_for_each_entry(inti, &fi->lists[i], list) {
-			if (n == max_irqs) {
-				/* signal userspace to try again */
-				ret = -ENOMEM;
-				goto out;
-			}
-			inti_to_irq(inti, &buf[n]);
-			n++;
-		}
-	}
-	if (test_bit(IRQ_PEND_EXT_SERVICE, &fi->pending_irqs) ||
-	    test_bit(IRQ_PEND_EXT_SERVICE_EV, &fi->pending_irqs)) {
-		if (n == max_irqs) {
-			/* signal userspace to try again */
-			ret = -ENOMEM;
-			goto out;
-		}
-		irq = (struct kvm_s390_irq *) &buf[n];
-		irq->type = KVM_S390_INT_SERVICE;
-		irq->u.ext = fi->srv_signal;
-		n++;
-	}
-	if (test_bit(IRQ_PEND_MCHK_REP, &fi->pending_irqs)) {
-		if (n == max_irqs) {
-				/* signal userspace to try again */
-				ret = -ENOMEM;
-				goto out;
-		}
-		irq = (struct kvm_s390_irq *) &buf[n];
-		irq->type = KVM_S390_MCHK;
-		irq->u.mchk = fi->mchk;
-		n++;
-}
 
-out:
-	spin_unlock_irqrestore(&fi->lock, flags);
-out_nolock:
-	if (!ret && n > 0) {
-		if (copy_to_user(usrbuf, buf, sizeof(struct kvm_s390_irq) * n))
-			ret = -EFAULT;
+	if (n > 0 && copy_to_user(usrbuf, buf, sizeof(struct kvm_s390_irq) * n)) {
+		/* restore removed bits if returning failure */
+		if (tmp)
+			__atomic_or(tmp, (void *)&gi->origin->ipm);
+		return -EFAULT;
 	}
-	vfree(buf);
 
-	return ret < 0 ? ret : n;
+	return n;
 }
 
 static int flic_ais_mode_get_all(struct kvm *kvm, struct kvm_device_attr *attr)
-- 
2.55.0

Re: [PATCH v1 03/11] KVM: s390: Fix get_all_floating_irqs()

[Christian Borntraeger]

Am 11.08.26 um 17:56 schrieb Claudio Imbrenda:
> When attempting to report all pending floating interrupt to userspace,
> the GISA IPM bits are atomically tested and cleared, and the
> corresponding interrupt description is written in the output buffer. If
> the output buffer is too small, an error is returned to userspace, but
> the GISA IPM bits are now lost.
> 
> Fix by moving the GISA test at the end of the function, and keeping
> track of which bits have been cleared. In case of error, set the bits
> again, so they are not lost.
> 
> Fixes: 24160af6cb28 ("KVM: s390: add GISA interrupts to FLIC ioctl interface")
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

this looks too complicated for a fix. Now what is the semantic of this?
This is used for migration purposes, and the doc says:

Documentation/virt/kvm/devices/s390_flic.rst

   KVM_DEV_FLIC_GET_ALL_IRQS
     Copies all floating interrupts into a buffer provided by userspace.
[...]
     All interrupts remain pending, i.e. are not deleted from the list of
     currently pending interrupts.
[...]

So even the success case is wrong. Why not simply add a new helper that
reads the GISA without clearing the bits?

static inline int gisa_test_ipm_gisc(struct kvm_s390_gisa *gisa, u32 gisc)
{
       return test_bit_inv(IPM_BIT_OFFSET + gisc, (unsigned long *) gisa);
}




> ---
>   arch/s390/kvm/interrupt.c | 93 ++++++++++++++++++---------------------
>   1 file changed, 44 insertions(+), 49 deletions(-)
> 
> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index 6b3f97a7513b..30963e05e0e6 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -2211,15 +2211,14 @@ void kvm_s390_clear_float_irqs(struct kvm *kvm)
>   static int get_all_floating_irqs(struct kvm *kvm, u8 __user *usrbuf, u64 len)
>   {
>   	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
> +	struct kvm_s390_irq *buf __free(kvfree) = NULL;
>   	struct kvm_s390_interrupt_info *inti;
>   	struct kvm_s390_float_interrupt *fi;
> -	struct kvm_s390_irq *buf;
>   	struct kvm_s390_irq *irq;
> +	unsigned int tmp = 0;
>   	int max_irqs;
> -	int ret = 0;
>   	int n = 0;
>   	int i;
> -	unsigned long flags;
>   
>   	if (len > KVM_S390_FLIC_MAX_BUFFER || len == 0)
>   		return -EINVAL;
> @@ -2235,14 +2234,48 @@ static int get_all_floating_irqs(struct kvm *kvm, u8 __user *usrbuf, u64 len)
>   
>   	max_irqs = len / sizeof(struct kvm_s390_irq);
>   
> +	fi = &kvm->arch.float_int;
> +	scoped_guard(spinlock_irqsave, &fi->lock) {
> +		for (i = 0; i < FIRQ_LIST_COUNT; i++) {
> +			list_for_each_entry(inti, &fi->lists[i], list) {
> +				/* signal userspace to try again */
> +				if (n == max_irqs)
> +					return -ENOMEM;
> +				inti_to_irq(inti, &buf[n]);
> +				n++;
> +			}
> +		}
> +		if (test_bit(IRQ_PEND_EXT_SERVICE, &fi->pending_irqs) ||
> +		    test_bit(IRQ_PEND_EXT_SERVICE_EV, &fi->pending_irqs)) {
> +			/* signal userspace to try again */
> +			if (n == max_irqs)
> +				return -ENOMEM;
> +			irq = (struct kvm_s390_irq *)&buf[n];
> +			irq->type = KVM_S390_INT_SERVICE;
> +			irq->u.ext = fi->srv_signal;
> +			n++;
> +		}
> +		if (test_bit(IRQ_PEND_MCHK_REP, &fi->pending_irqs)) {
> +			/* signal userspace to try again */
> +			if (n == max_irqs)
> +				return -ENOMEM;
> +			irq = (struct kvm_s390_irq *)&buf[n];
> +			irq->type = KVM_S390_MCHK;
> +			irq->u.mchk = fi->mchk;
> +			n++;
> +		}
> +	}
>   	if (gi->origin && gisa_get_ipm(gi->origin)) {
>   		for (i = 0; i <= MAX_ISC; i++) {
>   			if (n == max_irqs) {
> +				/* restore removed bits if returning failure */
> +				__atomic_or(tmp, (void *)&gi->origin->ipm);
>   				/* signal userspace to try again */
> -				ret = -ENOMEM;
> -				goto out_nolock;
> +				return -ENOMEM;
>   			}
>   			if (gisa_tac_ipm_gisc(gi->origin, i)) {
> +				/* set aside the bits we cleared */
> +				tmp |= 1 << (31 - i);
>   				irq = (struct kvm_s390_irq *) &buf[n];
>   				irq->type = KVM_S390_INT_IO(1, 0, 0, 0);
>   				irq->u.io.io_int_word = isc_to_int_word(i);
> @@ -2250,53 +2283,15 @@ static int get_all_floating_irqs(struct kvm *kvm, u8 __user *usrbuf, u64 len)
>   			}
>   		}
>   	}
> -	fi = &kvm->arch.float_int;
> -	spin_lock_irqsave(&fi->lock, flags);
> -	for (i = 0; i < FIRQ_LIST_COUNT; i++) {
> -		list_for_each_entry(inti, &fi->lists[i], list) {
> -			if (n == max_irqs) {
> -				/* signal userspace to try again */
> -				ret = -ENOMEM;
> -				goto out;
> -			}
> -			inti_to_irq(inti, &buf[n]);
> -			n++;
> -		}
> -	}
> -	if (test_bit(IRQ_PEND_EXT_SERVICE, &fi->pending_irqs) ||
> -	    test_bit(IRQ_PEND_EXT_SERVICE_EV, &fi->pending_irqs)) {
> -		if (n == max_irqs) {
> -			/* signal userspace to try again */
> -			ret = -ENOMEM;
> -			goto out;
> -		}
> -		irq = (struct kvm_s390_irq *) &buf[n];
> -		irq->type = KVM_S390_INT_SERVICE;
> -		irq->u.ext = fi->srv_signal;
> -		n++;
> -	}
> -	if (test_bit(IRQ_PEND_MCHK_REP, &fi->pending_irqs)) {
> -		if (n == max_irqs) {
> -				/* signal userspace to try again */
> -				ret = -ENOMEM;
> -				goto out;
> -		}
> -		irq = (struct kvm_s390_irq *) &buf[n];
> -		irq->type = KVM_S390_MCHK;
> -		irq->u.mchk = fi->mchk;
> -		n++;
> -}
>   
> -out:
> -	spin_unlock_irqrestore(&fi->lock, flags);
> -out_nolock:
> -	if (!ret && n > 0) {
> -		if (copy_to_user(usrbuf, buf, sizeof(struct kvm_s390_irq) * n))
> -			ret = -EFAULT;
> +	if (n > 0 && copy_to_user(usrbuf, buf, sizeof(struct kvm_s390_irq) * n)) {
> +		/* restore removed bits if returning failure */
> +		if (tmp)
> +			__atomic_or(tmp, (void *)&gi->origin->ipm);
> +		return -EFAULT;
>   	}
> -	vfree(buf);
>   
> -	return ret < 0 ? ret : n;
> +	return n;
>   }
>   
>   static int flic_ais_mode_get_all(struct kvm *kvm, struct kvm_device_attr *attr)

Re: [PATCH v1 03/11] KVM: s390: Fix get_all_floating_irqs()

[Claudio Imbrenda]

On Wed, 12 Aug 2026 09:11:11 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> Am 11.08.26 um 17:56 schrieb Claudio Imbrenda:
> > When attempting to report all pending floating interrupt to userspace,
> > the GISA IPM bits are atomically tested and cleared, and the
> > corresponding interrupt description is written in the output buffer. If
> > the output buffer is too small, an error is returned to userspace, but
> > the GISA IPM bits are now lost.
> > 
> > Fix by moving the GISA test at the end of the function, and keeping
> > track of which bits have been cleared. In case of error, set the bits
> > again, so they are not lost.
> > 
> > Fixes: 24160af6cb28 ("KVM: s390: add GISA interrupts to FLIC ioctl interface")
> > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>  
> 
> this looks too complicated for a fix. Now what is the semantic of this?
> This is used for migration purposes, and the doc says:

that's exactly what I did after I read the documentation more carefully

> 
> Documentation/virt/kvm/devices/s390_flic.rst
> 
>    KVM_DEV_FLIC_GET_ALL_IRQS
>      Copies all floating interrupts into a buffer provided by userspace.
> [...]
>      All interrupts remain pending, i.e. are not deleted from the list of
>      currently pending interrupts.
> [...]
> 
> So even the success case is wrong. Why not simply add a new helper that
> reads the GISA without clearing the bits?
> 
> static inline int gisa_test_ipm_gisc(struct kvm_s390_gisa *gisa, u32 gisc)
> {
>        return test_bit_inv(IPM_BIT_OFFSET + gisc, (unsigned long *) gisa);
> }
> 
> 
> 
> 
> > ---
> >   arch/s390/kvm/interrupt.c | 93 ++++++++++++++++++---------------------
> >   1 file changed, 44 insertions(+), 49 deletions(-)
> > 
> > diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> > index 6b3f97a7513b..30963e05e0e6 100644
> > --- a/arch/s390/kvm/interrupt.c
> > +++ b/arch/s390/kvm/interrupt.c
> > @@ -2211,15 +2211,14 @@ void kvm_s390_clear_float_irqs(struct kvm *kvm)
> >   static int get_all_floating_irqs(struct kvm *kvm, u8 __user *usrbuf, u64 len)
> >   {
> >   	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
> > +	struct kvm_s390_irq *buf __free(kvfree) = NULL;
> >   	struct kvm_s390_interrupt_info *inti;
> >   	struct kvm_s390_float_interrupt *fi;
> > -	struct kvm_s390_irq *buf;
> >   	struct kvm_s390_irq *irq;
> > +	unsigned int tmp = 0;
> >   	int max_irqs;
> > -	int ret = 0;
> >   	int n = 0;
> >   	int i;
> > -	unsigned long flags;
> >   
> >   	if (len > KVM_S390_FLIC_MAX_BUFFER || len == 0)
> >   		return -EINVAL;
> > @@ -2235,14 +2234,48 @@ static int get_all_floating_irqs(struct kvm *kvm, u8 __user *usrbuf, u64 len)
> >   
> >   	max_irqs = len / sizeof(struct kvm_s390_irq);
> >   
> > +	fi = &kvm->arch.float_int;
> > +	scoped_guard(spinlock_irqsave, &fi->lock) {
> > +		for (i = 0; i < FIRQ_LIST_COUNT; i++) {
> > +			list_for_each_entry(inti, &fi->lists[i], list) {
> > +				/* signal userspace to try again */
> > +				if (n == max_irqs)
> > +					return -ENOMEM;
> > +				inti_to_irq(inti, &buf[n]);
> > +				n++;
> > +			}
> > +		}
> > +		if (test_bit(IRQ_PEND_EXT_SERVICE, &fi->pending_irqs) ||
> > +		    test_bit(IRQ_PEND_EXT_SERVICE_EV, &fi->pending_irqs)) {
> > +			/* signal userspace to try again */
> > +			if (n == max_irqs)
> > +				return -ENOMEM;
> > +			irq = (struct kvm_s390_irq *)&buf[n];
> > +			irq->type = KVM_S390_INT_SERVICE;
> > +			irq->u.ext = fi->srv_signal;
> > +			n++;
> > +		}
> > +		if (test_bit(IRQ_PEND_MCHK_REP, &fi->pending_irqs)) {
> > +			/* signal userspace to try again */
> > +			if (n == max_irqs)
> > +				return -ENOMEM;
> > +			irq = (struct kvm_s390_irq *)&buf[n];
> > +			irq->type = KVM_S390_MCHK;
> > +			irq->u.mchk = fi->mchk;
> > +			n++;
> > +		}
> > +	}
> >   	if (gi->origin && gisa_get_ipm(gi->origin)) {
> >   		for (i = 0; i <= MAX_ISC; i++) {
> >   			if (n == max_irqs) {
> > +				/* restore removed bits if returning failure */
> > +				__atomic_or(tmp, (void *)&gi->origin->ipm);
> >   				/* signal userspace to try again */
> > -				ret = -ENOMEM;
> > -				goto out_nolock;
> > +				return -ENOMEM;
> >   			}
> >   			if (gisa_tac_ipm_gisc(gi->origin, i)) {
> > +				/* set aside the bits we cleared */
> > +				tmp |= 1 << (31 - i);
> >   				irq = (struct kvm_s390_irq *) &buf[n];
> >   				irq->type = KVM_S390_INT_IO(1, 0, 0, 0);
> >   				irq->u.io.io_int_word = isc_to_int_word(i);
> > @@ -2250,53 +2283,15 @@ static int get_all_floating_irqs(struct kvm *kvm, u8 __user *usrbuf, u64 len)
> >   			}
> >   		}
> >   	}
> > -	fi = &kvm->arch.float_int;
> > -	spin_lock_irqsave(&fi->lock, flags);
> > -	for (i = 0; i < FIRQ_LIST_COUNT; i++) {
> > -		list_for_each_entry(inti, &fi->lists[i], list) {
> > -			if (n == max_irqs) {
> > -				/* signal userspace to try again */
> > -				ret = -ENOMEM;
> > -				goto out;
> > -			}
> > -			inti_to_irq(inti, &buf[n]);
> > -			n++;
> > -		}
> > -	}
> > -	if (test_bit(IRQ_PEND_EXT_SERVICE, &fi->pending_irqs) ||
> > -	    test_bit(IRQ_PEND_EXT_SERVICE_EV, &fi->pending_irqs)) {
> > -		if (n == max_irqs) {
> > -			/* signal userspace to try again */
> > -			ret = -ENOMEM;
> > -			goto out;
> > -		}
> > -		irq = (struct kvm_s390_irq *) &buf[n];
> > -		irq->type = KVM_S390_INT_SERVICE;
> > -		irq->u.ext = fi->srv_signal;
> > -		n++;
> > -	}
> > -	if (test_bit(IRQ_PEND_MCHK_REP, &fi->pending_irqs)) {
> > -		if (n == max_irqs) {
> > -				/* signal userspace to try again */
> > -				ret = -ENOMEM;
> > -				goto out;
> > -		}
> > -		irq = (struct kvm_s390_irq *) &buf[n];
> > -		irq->type = KVM_S390_MCHK;
> > -		irq->u.mchk = fi->mchk;
> > -		n++;
> > -}
> >   
> > -out:
> > -	spin_unlock_irqrestore(&fi->lock, flags);
> > -out_nolock:
> > -	if (!ret && n > 0) {
> > -		if (copy_to_user(usrbuf, buf, sizeof(struct kvm_s390_irq) * n))
> > -			ret = -EFAULT;
> > +	if (n > 0 && copy_to_user(usrbuf, buf, sizeof(struct kvm_s390_irq) * n)) {
> > +		/* restore removed bits if returning failure */
> > +		if (tmp)
> > +			__atomic_or(tmp, (void *)&gi->origin->ipm);
> > +		return -EFAULT;
> >   	}
> > -	vfree(buf);
> >   
> > -	return ret < 0 ? ret : n;
> > +	return n;
> >   }
> >   
> >   static int flic_ais_mode_get_all(struct kvm *kvm, struct kvm_device_attr *attr)  
> 

[PATCH v1 04/11] KVM: s390: Fix dirty marking in adapter_indicators_set*()

[Claudio Imbrenda]

When the indicator and/or summary bits are set in the guest, the
accessed page was only marked dirty if the access was performed using
the slow path; accesses through the new kvm_arch_set_irq_inatomic fast
inject path would not mark the page as dirty.

Fix by adding/moving the missing calls to set_page_dirty_lock() and
mark_page_dirty().

Opportunistically reorder the local variables to be in reverse
Christmas tree oder.

Fixes: 1e95e3bc6b05 ("KVM: s390: Enable adapter_indicators_set to use mapped pages")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 arch/s390/kvm/interrupt.c | 84 +++++++++++++++++++++++----------------
 1 file changed, 49 insertions(+), 35 deletions(-)

diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 30963e05e0e6..400ce7195b87 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -2943,12 +2943,14 @@ static int adapter_indicators_set(struct kvm *kvm,
 				  struct s390_io_adapter *adapter,
 				  struct kvm_s390_adapter_int *adapter_int)
 {
-	unsigned long bit;
-	int summary_set, idx;
 	struct s390_map_info *ind_info, *summary_info;
-	void *map;
 	struct page *ind_page, *summary_page;
 	unsigned long flags;
+	unsigned long bit;
+	int summary_set;
+	void *map;
+
+	guard(srcu)(&kvm->srcu);
 
 	ind_page = NULL;
 
@@ -2959,21 +2961,20 @@ static int adapter_indicators_set(struct kvm *kvm,
 		ind_page = pin_map_page(kvm, adapter_int->ind_addr, 0);
 		if (!ind_page)
 			return -1;
-		idx = srcu_read_lock(&kvm->srcu);
 		map = page_address(ind_page);
 		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_gaddr >> PAGE_SHIFT);
 		set_page_dirty_lock(ind_page);
-		srcu_read_unlock(&kvm->srcu, idx);
 		unpin_user_page(ind_page);
 	} else {
 		map = page_address(ind_info->page);
 		bit = get_ind_bit(ind_info->addr, adapter_int->ind_offset, adapter->swap);
 		set_bit(bit, map);
 		spin_unlock_irqrestore(&adapter->maps_lock, flags);
+		set_page_dirty_lock(ind_info->page);
 	}
+	mark_page_dirty(kvm, gpa_to_gfn(adapter_int->ind_gaddr));
 
 	spin_lock_irqsave(&adapter->maps_lock, flags);
 	summary_info = get_map_info(adapter, adapter_int->summary_addr);
@@ -2982,14 +2983,11 @@ static int adapter_indicators_set(struct kvm *kvm,
 		summary_page = pin_map_page(kvm, adapter_int->summary_addr, 0);
 		if (WARN_ON_ONCE(!summary_page))
 			return -1;
-		idx = srcu_read_lock(&kvm->srcu);
 		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_gaddr >> PAGE_SHIFT);
 		set_page_dirty_lock(summary_page);
-		srcu_read_unlock(&kvm->srcu, idx);
 		unpin_user_page(summary_page);
 	} else {
 		map = page_address(summary_info->page);
@@ -2997,7 +2995,9 @@ static int adapter_indicators_set(struct kvm *kvm,
 				  adapter->swap);
 		summary_set = test_and_set_bit(bit, map);
 		spin_unlock_irqrestore(&adapter->maps_lock, flags);
+		set_page_dirty_lock(summary_info->page);
 	}
+	mark_page_dirty(kvm, gpa_to_gfn(adapter_int->summary_gaddr));
 
 	return summary_set ? 0 : 1;
 }
@@ -3007,37 +3007,51 @@ static int adapter_indicators_set_fast(struct kvm *kvm,
 				       struct kvm_s390_adapter_int *adapter_int,
 				       int setbit)
 {
-	unsigned long bit;
-	int summary_set;
 	struct s390_map_info *ind_info, *summary_info;
+	int summary_set = -1;
+	unsigned long bit;
 	void *map;
 
-	spin_lock(&adapter->maps_lock);
-	ind_info = get_map_info(adapter, adapter_int->ind_addr);
-	if (!ind_info) {
-		spin_unlock(&adapter->maps_lock);
-		return -EWOULDBLOCK;
+	guard(srcu)(&kvm->srcu);
+
+	scoped_guard(spinlock, &adapter->maps_lock) {
+		ind_info = get_map_info(adapter, adapter_int->ind_addr);
+		if (!ind_info)
+			return -EWOULDBLOCK;
+
+		map = page_address(ind_info->page);
+		bit = get_ind_bit(ind_info->addr, adapter_int->ind_offset, adapter->swap);
+		if (setbit)
+			set_bit(bit, map);
+
+		summary_info = get_map_info(adapter, adapter_int->summary_addr);
+		if (!summary_info)
+			goto out;
+
+		map = page_address(summary_info->page);
+		bit = get_ind_bit(summary_info->addr, adapter_int->summary_offset,
+				  adapter->swap);
+		/* If setbit then set summary bit. Else if falling back to the slow path */
+		/* with setbit==0 then clear the summary bit so the slow path re-injects */
+		if (setbit)
+			summary_set = test_and_set_bit(bit, map);
+		else
+			summary_set = test_and_clear_bit(bit, map);
 	}
-	map = page_address(ind_info->page);
-	bit = get_ind_bit(ind_info->addr, adapter_int->ind_offset, adapter->swap);
-	if (setbit)
-		set_bit(bit, map);
-	summary_info = get_map_info(adapter, adapter_int->summary_addr);
-	if (!summary_info) {
-		spin_unlock(&adapter->maps_lock);
-		return -EWOULDBLOCK;
+
+out:
+	if (setbit) {
+		mark_page_dirty(kvm, gpa_to_gfn(adapter_int->ind_gaddr));
+		set_page_dirty_lock(ind_info->page);
 	}
-	map = page_address(summary_info->page);
-	bit = get_ind_bit(summary_info->addr, adapter_int->summary_offset,
-			  adapter->swap);
-	/* If setbit then set summary bit. Else if falling back to the slow path */
-	/* with setbit==0 then clear the summary bit so the slow path re-injects */
-	if (setbit)
-		summary_set = test_and_set_bit(bit, map);
-	else
-		summary_set = test_and_clear_bit(bit, map);
-	spin_unlock(&adapter->maps_lock);
-	return summary_set ? 0 : 1;
+
+	if (summary_set >= 0) {
+		mark_page_dirty(kvm, gpa_to_gfn(adapter_int->summary_gaddr));
+		set_page_dirty_lock(summary_info->page);
+		return !summary_set;
+	}
+
+	return -EWOULDBLOCK;
 }
 
 /*
-- 
2.55.0

[PATCH v1 05/11] KVM: s390: Fix pgste_get_trylock_multiple()

[Claudio Imbrenda]

In case of failure, pgste_get_trylock_multiple() will attempt to unlock
the locked PGSTEs based on whether the PCL is set. In some
circumstances this can lead to unlocking PGSTEs that were locked by
other threads.

Fix by unlocking the amount of PGSTEs that were actually locked,
ignoring the PCL bit in the array.

Fixes: 94fd9b16cc67 ("KVM: s390: KVM page table management functions: lifecycle management")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 arch/s390/kvm/dat.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/s390/kvm/dat.c b/arch/s390/kvm/dat.c
index 165c704fcf29..f4dd6f783417 100644
--- a/arch/s390/kvm/dat.c
+++ b/arch/s390/kvm/dat.c
@@ -923,11 +923,8 @@ static void pgste_set_unlock_multiple(union pte *first, int n, union pgste *pgst
 {
 	int i;
 
-	for (i = 0; i < n; i++) {
-		if (!pgstes[i].pcl)
-			break;
+	for (i = 0; i < n; i++)
 		pgste_set_unlock(first + i, pgstes[i]);
-	}
 }
 
 static bool pgste_get_trylock_multiple(union pte *first, int n, union pgste *pgstes)
@@ -940,7 +937,7 @@ static bool pgste_get_trylock_multiple(union pte *first, int n, union pgste *pgs
 	}
 	if (i == n)
 		return true;
-	pgste_set_unlock_multiple(first, n, pgstes);
+	pgste_set_unlock_multiple(first, i, pgstes);
 	return false;
 }
 
-- 
2.55.0

Re: [PATCH v1 05/11] KVM: s390: Fix pgste_get_trylock_multiple()

[Christian Borntraeger]

Am 11.08.26 um 17:56 schrieb Claudio Imbrenda:
> In case of failure, pgste_get_trylock_multiple() will attempt to unlock
> the locked PGSTEs based on whether the PCL is set. In some
> circumstances this can lead to unlocking PGSTEs that were locked by
> other threads.
> 
> Fix by unlocking the amount of PGSTEs that were actually locked,
> ignoring the PCL bit in the array.
> 
> Fixes: 94fd9b16cc67 ("KVM: s390: KVM page table management functions: lifecycle management")
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>


Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>

> ---
>   arch/s390/kvm/dat.c | 7 ++-----
>   1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/s390/kvm/dat.c b/arch/s390/kvm/dat.c
> index 165c704fcf29..f4dd6f783417 100644
> --- a/arch/s390/kvm/dat.c
> +++ b/arch/s390/kvm/dat.c
> @@ -923,11 +923,8 @@ static void pgste_set_unlock_multiple(union pte *first, int n, union pgste *pgst
>   {
>   	int i;
>   
> -	for (i = 0; i < n; i++) {
> -		if (!pgstes[i].pcl)
> -			break;
> +	for (i = 0; i < n; i++)
>   		pgste_set_unlock(first + i, pgstes[i]);
> -	}
>   }
>   
>   static bool pgste_get_trylock_multiple(union pte *first, int n, union pgste *pgstes)
> @@ -940,7 +937,7 @@ static bool pgste_get_trylock_multiple(union pte *first, int n, union pgste *pgs
>   	}
>   	if (i == n)
>   		return true;
> -	pgste_set_unlock_multiple(first, n, pgstes);
> +	pgste_set_unlock_multiple(first, i, pgstes);
>   	return false;
>   }
>   

[PATCH v1 06/11] KVM: s390: Introduce extended topup for struct kvm_s390_mmu_cache

[Claudio Imbrenda]

In most cases the mmu caches should not blow up in size unnecessarily,
since there is one for each vCPU, as it can add up to a quite big
amount of memory. In some cases, though, it is necessary to have a
particularly large mmu cache, to guarantee that specific operations can
complete without needing to refill the cache.

This patch refactors the existing kvm_s390_mmu_cache_topup() function
to allow for an extended top-up. Two wrappers are also provided, one
with the old name and old behaviour, and one that performs the extended
top-up.

Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 arch/s390/kvm/dat.c | 11 +++++++----
 arch/s390/kvm/dat.h | 25 +++++++++++++++++++++----
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/arch/s390/kvm/dat.c b/arch/s390/kvm/dat.c
index f4dd6f783417..47ff8af1b22f 100644
--- a/arch/s390/kvm/dat.c
+++ b/arch/s390/kvm/dat.c
@@ -28,23 +28,26 @@
 #include <asm/tlb.h>
 #include "dat.h"
 
-int kvm_s390_mmu_cache_topup(struct kvm_s390_mmu_cache *mc)
+int _kvm_s390_mmu_cache_topup(struct kvm_s390_mmu_cache *mc, bool extended)
 {
+	size_t n_crsts = extended ? KVM_S390_MMU_CACHE_N_CRSTS_MAX : KVM_S390_MMU_CACHE_N_CRSTS;
+	size_t n_pts = extended ? KVM_S390_MMU_CACHE_N_PTS_MAX : KVM_S390_MMU_CACHE_N_PTS;
+	size_t n_rmaps = extended ? KVM_S390_MMU_CACHE_N_RMAPS_MAX : KVM_S390_MMU_CACHE_N_RMAPS;
 	void *o;
 
-	for ( ; mc->n_crsts < KVM_S390_MMU_CACHE_N_CRSTS; mc->n_crsts++) {
+	for ( ; mc->n_crsts < n_crsts; mc->n_crsts++) {
 		o = (void *)__get_free_pages(GFP_KERNEL_ACCOUNT | __GFP_COMP, CRST_ALLOC_ORDER);
 		if (!o)
 			return -ENOMEM;
 		mc->crsts[mc->n_crsts] = o;
 	}
-	for ( ; mc->n_pts < KVM_S390_MMU_CACHE_N_PTS; mc->n_pts++) {
+	for ( ; mc->n_pts < n_pts; mc->n_pts++) {
 		o = (void *)__get_free_page(GFP_KERNEL_ACCOUNT);
 		if (!o)
 			return -ENOMEM;
 		mc->pts[mc->n_pts] = o;
 	}
-	for ( ; mc->n_rmaps < KVM_S390_MMU_CACHE_N_RMAPS; mc->n_rmaps++) {
+	for ( ; mc->n_rmaps < n_rmaps; mc->n_rmaps++) {
 		o = kzalloc_obj(struct vsie_rmap, GFP_KERNEL_ACCOUNT);
 		if (!o)
 			return -ENOMEM;
diff --git a/arch/s390/kvm/dat.h b/arch/s390/kvm/dat.h
index 141ee7b9f019..461302f05e50 100644
--- a/arch/s390/kvm/dat.h
+++ b/arch/s390/kvm/dat.h
@@ -479,13 +479,20 @@ struct vsie_rmap {
 
 static_assert(sizeof(struct vsie_rmap) == 2 * sizeof(long));
 
+/* Used when performing normal top-ups */
 #define KVM_S390_MMU_CACHE_N_CRSTS	6
 #define KVM_S390_MMU_CACHE_N_PTS	2
 #define KVM_S390_MMU_CACHE_N_RMAPS	16
+
+/* Used to perform special extended top-ups */
+#define KVM_S390_MMU_CACHE_N_CRSTS_MAX	32
+#define KVM_S390_MMU_CACHE_N_PTS_MAX	8
+#define KVM_S390_MMU_CACHE_N_RMAPS_MAX	16
+
 struct kvm_s390_mmu_cache {
-	void *crsts[KVM_S390_MMU_CACHE_N_CRSTS];
-	void *pts[KVM_S390_MMU_CACHE_N_PTS];
-	void *rmaps[KVM_S390_MMU_CACHE_N_RMAPS];
+	void *crsts[KVM_S390_MMU_CACHE_N_CRSTS_MAX];
+	void *pts[KVM_S390_MMU_CACHE_N_PTS_MAX];
+	void *rmaps[KVM_S390_MMU_CACHE_N_RMAPS_MAX];
 	short int n_crsts;
 	short int n_pts;
 	short int n_rmaps;
@@ -555,7 +562,17 @@ int dat_get_cmma(union asce asce, gfn_t *start, unsigned int *count, u8 *values,
 int dat_set_cmma_bits(struct kvm_s390_mmu_cache *mc, union asce asce, gfn_t gfn,
 		      unsigned long count, unsigned long mask, const uint8_t *bits);
 
-int kvm_s390_mmu_cache_topup(struct kvm_s390_mmu_cache *mc);
+int _kvm_s390_mmu_cache_topup(struct kvm_s390_mmu_cache *mc, bool extended);
+
+static inline int kvm_s390_mmu_cache_topup(struct kvm_s390_mmu_cache *mc)
+{
+	return _kvm_s390_mmu_cache_topup(mc, false);
+}
+
+static inline int kvm_s390_mmu_cache_extended_topup(struct kvm_s390_mmu_cache *mc)
+{
+	return _kvm_s390_mmu_cache_topup(mc, true);
+}
 
 #define GFP_KVM_S390_MMU_CACHE (GFP_ATOMIC | __GFP_ACCOUNT | __GFP_NOWARN)
 
-- 
2.55.0

[PATCH v1 07/11] KVM: s390: Move all code into kvm_arch_prepare_memory_region()

[Claudio Imbrenda]

Move all code from kvm_arch_commit_memory_region() into
kvm_arch_prepare_memory_region(). This allows the function to fail
gracefully if needed. The previous behaviour was to print a warning and
continue execution with page tables incosistent with the memslots.

The new kvm_s390_mmu_cache_extended_topup() function is used to
guarantee that the necessary DAT table updates can be performed
successfully.

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

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 02c5428ba239..c4196f9af05d 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -5766,11 +5766,28 @@ bool kvm_arch_irqchip_in_kernel(struct kvm *kvm)
 }
 
 /* Section: memory related */
+static long cmma_d_count_pte(union pte *ptep, gfn_t gfn, gfn_t next, struct dat_walk *walk)
+{
+	union pgste pgste;
+
+	pgste = pgste_get_lock(ptep);
+	if (pgste.cmma_d) {
+		pgste.cmma_d = 0;
+		atomic64_dec(walk->priv);
+	}
+	pgste_set_unlock(ptep, pgste);
+	return 0;
+}
+
 int kvm_arch_prepare_memory_region(struct kvm *kvm,
 				   const struct kvm_memory_slot *old,
 				   struct kvm_memory_slot *new,
 				   enum kvm_mr_change change)
 {
+	const struct dat_walk_ops ops = { .pte_entry = cmma_d_count_pte, };
+	struct kvm_s390_mmu_cache *mc __free(kvm_s390_mmu_cache) = NULL;
+	int rc = 0;
+
 	if (kvm_is_ucontrol(kvm) && new && new->id < KVM_USER_MEM_SLOTS)
 		return -EINVAL;
 
@@ -5785,6 +5802,10 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,
 		 * and munmap() stuff in this slot after doing this call at any
 		 * time.
 		 */
+		if (change != KVM_MR_MOVE && change != KVM_MR_CREATE) {
+			WARN(1, "Unknown KVM MR CHANGE: %d\n", change);
+			return -EINVAL;
+		}
 		if (new->userspace_addr & ~PAGE_MASK)
 			return -EINVAL;
 		if ((new->base_gfn + new->npages) * PAGE_SIZE > kvm->arch.mem_limit)
@@ -5793,57 +5814,27 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,
 			return -EINVAL;
 	}
 
-	if (!kvm->arch.migration_mode)
-		return 0;
-
-	/*
-	 * Turn off migration mode when:
-	 * - userspace creates a new memslot with dirty logging off,
-	 * - userspace modifies an existing memslot (MOVE or FLAGS_ONLY) and
-	 *   dirty logging is turned off.
-	 * Migration mode expects dirty page logging being enabled to store
-	 * its dirty bitmap.
-	 */
-	if (change != KVM_MR_DELETE &&
-	    !(new->flags & KVM_MEM_LOG_DIRTY_PAGES))
-		WARN(kvm_s390_vm_stop_migration(kvm),
-		     "Failed to stop migration mode");
-
-	return 0;
-}
-
-static long cmma_d_count_pte(union pte *ptep, gfn_t gfn, gfn_t next, struct dat_walk *walk)
-{
-	union pgste pgste;
-
-	pgste = pgste_get_lock(ptep);
-	if (pgste.cmma_d) {
-		pgste.cmma_d = 0;
-		atomic64_dec(walk->priv);
+	if (kvm->arch.migration_mode) {
+		/*
+		 * Turn off migration mode when:
+		 * - userspace creates a new memslot with dirty logging off,
+		 * - userspace modifies an existing memslot (MOVE or FLAGS_ONLY)
+		 *   and dirty logging is turned off.
+		 * Migration mode expects dirty page logging being enabled to
+		 * store its dirty bitmap.
+		 */
+		if (change != KVM_MR_DELETE &&
+		    !(new->flags & KVM_MEM_LOG_DIRTY_PAGES))
+			WARN(kvm_s390_vm_stop_migration(kvm),
+			     "Failed to stop migration mode");
 	}
-	pgste_set_unlock(ptep, pgste);
-	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)
-{
-	const struct dat_walk_ops ops = { .pte_entry = cmma_d_count_pte, };
-	struct kvm_s390_mmu_cache *mc __free(kvm_s390_mmu_cache) = NULL;
-	int rc = 0;
-
-	guard(mutex)(&kvm->slots_arch_lock);
 
 	if (change == KVM_MR_FLAGS_ONLY)
-		return;
+		return 0;
 
 	mc = kvm_s390_new_mmu_cache();
-	if (!mc) {
-		rc = -ENOMEM;
-		goto out;
-	}
+	if (!mc || kvm_s390_mmu_cache_extended_topup(mc))
+		return -ENOMEM;
 
 	scoped_guard(write_lock, &kvm->mmu_lock) {
 		if (kvm->arch.migration_mode && kvm->arch.use_cmma && old) {
@@ -5852,28 +5843,31 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
 					    &kvm->arch.cmma_dirty_pages);
 		}
 
-		switch (change) {
-		case KVM_MR_DELETE:
+		if (change == KVM_MR_DELETE || change == KVM_MR_MOVE)
 			rc = dat_delete_slot(mc, kvm->arch.gmap->asce, old->base_gfn, old->npages);
-			break;
-		case KVM_MR_MOVE:
-			rc = dat_delete_slot(mc, kvm->arch.gmap->asce, old->base_gfn, old->npages);
-			if (rc)
-				break;
-			fallthrough;
-		case KVM_MR_CREATE:
+		if (!rc && (change == KVM_MR_MOVE || change == KVM_MR_CREATE))
 			rc = dat_create_slot(mc, kvm->arch.gmap->asce, new->base_gfn, new->npages);
-			break;
-		case KVM_MR_FLAGS_ONLY:
-			break;
-		default:
-			WARN(1, "Unknown KVM MR CHANGE: %d\n", change);
-		}
 	}
-out:
-	if (rc)
-		pr_warn("failed to commit memory region\n");
-	return;
+
+	/*
+	 * dat_{create,delete}_slot() can only fail in two cases:
+	 * - Internal consistency error in the gmap DAT tables
+	 * - Out of memory while allocating DAT tables
+	 * The first case should not be possible in general, and is a symptom of
+	 * a serious KVM bug; the second should also be impossible because the
+	 * mmu cache has been filled to the brim, and has more than enough
+	 * capacity to handle the worst cases. Hence it is safe to put this
+	 * KVM_BUG_ON() here, as it should not be triggerable unless some
+	 * serious bug has occurred somewhere else.
+	 */
+	return KVM_BUG_ON(rc, kvm);
+}
+
+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)
+{
 }
 
 /**
-- 
2.55.0

Re: [PATCH v1 07/11] KVM: s390: Move all code into kvm_arch_prepare_memory_region()

[Christian Borntraeger]

Am 11.08.26 um 17:56 schrieb Claudio Imbrenda:
> Move all code from kvm_arch_commit_memory_region() into
> kvm_arch_prepare_memory_region(). This allows the function to fail
> gracefully if needed. The previous behaviour was to print a warning and
> continue execution with page tables incosistent with the memslots.
> 
> The new kvm_s390_mmu_cache_extended_topup() function is used to
> guarantee that the necessary DAT table updates can be performed
> successfully.
> 
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> Fixes: e38c884df921 ("KVM: s390: Switch to new gmap")

Uhh, this feels pretty large for stable. sashiko founnd the  KVM_BUG_ON return code
but this is hard tor review.


Another think.
kvm_s390_mmu_cache_extended_topup() asks for up to 32 order-2 allocations,
roughly 512 KiB. Even for a delete?

[PATCH v1 08/11] KVM: s390: Fix IRQ injection with SIGP Stop and Store Status

[Claudio Imbrenda]

When __inject_sigp_stop() is called for a Stop and Store Status
operation, if the vCPU is running, the interrupt is marked as pending
and the status is stored by the thread performing the KVM_RUN IOCTL.

If the vCPU is already stopped, the status is stored immediately.

Storing the status means writing into userspace, which might fault, and
__inject_sigp_stop() is called from do_inject_vcpu() which in turn is
always called holding a spinlock, which is obviously an issue.

Fix this by returning -EWOULDBLOCK from __inject_sigp_stop(), and
adding a bool flag to indicate whether a store status is needed. The
callers of do_inject_vcpu() are modified to pass the pointer to the
bool flag; whenever a Store Status operation is needed, the callers can
now perform it outside the spinlock.

Opportunistically refactor kvm_s390_set_irq_state() to use
scoped_guard() and __free().

Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 arch/s390/kvm/interrupt.c | 68 +++++++++++++++++++++------------------
 1 file changed, 36 insertions(+), 32 deletions(-)

diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 400ce7195b87..d38b9da0d09e 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -1550,23 +1550,21 @@ static int __inject_set_prefix(struct kvm_vcpu *vcpu, struct kvm_s390_irq *irq)
 }
 
 #define KVM_S390_STOP_SUPP_FLAGS (KVM_S390_STOP_FLAG_STORE_STATUS)
-static int __inject_sigp_stop(struct kvm_vcpu *vcpu, struct kvm_s390_irq *irq)
+static int __inject_sigp_stop(struct kvm_vcpu *vcpu, struct kvm_s390_irq *irq, bool *storestatus)
 {
 	struct kvm_s390_local_interrupt *li = &vcpu->arch.local_int;
 	struct kvm_s390_stop_info *stop = &li->irq.stop;
-	int rc = 0;
 
 	vcpu->stat.inject_stop_signal++;
 	trace_kvm_s390_inject_vcpu(vcpu->vcpu_id, KVM_S390_SIGP_STOP, 0, 0);
 
 	if (irq->u.stop.flags & ~KVM_S390_STOP_SUPP_FLAGS)
 		return -EINVAL;
-
 	if (is_vcpu_stopped(vcpu)) {
-		if (irq->u.stop.flags & KVM_S390_STOP_FLAG_STORE_STATUS)
-			rc = kvm_s390_store_status_unloaded(vcpu,
-						KVM_S390_STORE_STATUS_NOADDR);
-		return rc;
+		if (!(irq->u.stop.flags & KVM_S390_STOP_FLAG_STORE_STATUS))
+			return 0;
+		*storestatus = true;
+		return -EWOULDBLOCK;
 	}
 
 	if (test_and_set_bit(IRQ_PEND_SIGP_STOP, &li->pending_irqs))
@@ -2102,7 +2100,7 @@ void kvm_s390_clear_stop_irq(struct kvm_vcpu *vcpu)
 	spin_unlock(&li->lock);
 }
 
-static int do_inject_vcpu(struct kvm_vcpu *vcpu, struct kvm_s390_irq *irq)
+static int do_inject_vcpu(struct kvm_vcpu *vcpu, struct kvm_s390_irq *irq, bool *storestatus)
 {
 	int rc;
 
@@ -2114,7 +2112,7 @@ static int do_inject_vcpu(struct kvm_vcpu *vcpu, struct kvm_s390_irq *irq)
 		rc = __inject_set_prefix(vcpu, irq);
 		break;
 	case KVM_S390_SIGP_STOP:
-		rc = __inject_sigp_stop(vcpu, irq);
+		rc = __inject_sigp_stop(vcpu, irq, storestatus);
 		break;
 	case KVM_S390_RESTART:
 		rc = __inject_sigp_restart(vcpu);
@@ -2150,11 +2148,16 @@ static int do_inject_vcpu(struct kvm_vcpu *vcpu, struct kvm_s390_irq *irq)
 int kvm_s390_inject_vcpu(struct kvm_vcpu *vcpu, struct kvm_s390_irq *irq)
 {
 	struct kvm_s390_local_interrupt *li = &vcpu->arch.local_int;
+	bool storestatus = false;
 	int rc;
 
 	spin_lock(&li->lock);
-	rc = do_inject_vcpu(vcpu, irq);
+	rc = do_inject_vcpu(vcpu, irq, &storestatus);
 	spin_unlock(&li->lock);
+
+	if (rc == -EWOULDBLOCK && storestatus)
+		rc = kvm_s390_store_status_unloaded(vcpu, KVM_S390_STORE_STATUS_NOADDR);
+
 	if (!rc)
 		kvm_s390_vcpu_wakeup(vcpu);
 	return rc;
@@ -3178,7 +3181,8 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e, struct kvm *kvm,
 int kvm_s390_set_irq_state(struct kvm_vcpu *vcpu, void __user *irqstate, int len)
 {
 	struct kvm_s390_local_interrupt *li = &vcpu->arch.local_int;
-	struct kvm_s390_irq *buf;
+	struct kvm_s390_irq *buf __free(kvfree) = NULL;
+	bool tmp, storestatus = false;
 	int r = 0;
 	int n;
 
@@ -3186,31 +3190,31 @@ int kvm_s390_set_irq_state(struct kvm_vcpu *vcpu, void __user *irqstate, int len
 	if (!buf)
 		return -ENOMEM;
 
-	if (copy_from_user((void *) buf, irqstate, len)) {
-		r = -EFAULT;
-		goto out_free;
-	}
+	if (copy_from_user((void *)buf, irqstate, len))
+		return -EFAULT;
 
-	/*
-	 * Don't allow setting the interrupt state
-	 * when there are already interrupts pending
-	 */
-	spin_lock(&li->lock);
-	if (li->pending_irqs) {
-		r = -EBUSY;
-		goto out_unlock;
-	}
+	scoped_guard(spinlock, &li->lock) {
+		/*
+		 * Don't allow setting the interrupt state
+		 * when there are already interrupts pending
+		 */
+		if (li->pending_irqs)
+			return -EBUSY;
 
-	for (n = 0; n < len / sizeof(*buf); n++) {
-		r = do_inject_vcpu(vcpu, &buf[n]);
-		if (r)
-			break;
+		for (n = 0; n < len / sizeof(*buf); n++) {
+			tmp = false;
+			r = do_inject_vcpu(vcpu, &buf[n], &tmp);
+			if (r == -EWOULDBLOCK && tmp) {
+				storestatus = true;
+				continue;
+			}
+			if (r)
+				break;
+		}
 	}
 
-out_unlock:
-	spin_unlock(&li->lock);
-out_free:
-	vfree(buf);
+	if (storestatus)
+		r = kvm_s390_store_status_unloaded(vcpu, KVM_S390_STORE_STATUS_NOADDR);
 
 	return r;
 }
-- 
2.55.0

[PATCH v1 09/11] KVM: s390: Fix kvm_s390_clear_pv_state()

[Claudio Imbrenda]

kvm_s390_clear_pv_state() needs to also clear the dumping flag, to
allow the protected VM to be started again (as non-protected, with all
protected state safely destroyed) after a forced reboot while a
protected dump was ongoing and not completed.

Fixes: e40df9efd68a ("KVM: s390: pv: clear the state without memset")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 arch/s390/kvm/pv.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c
index b02e0159d3cd..98a9a57f71b9 100644
--- a/arch/s390/kvm/pv.c
+++ b/arch/s390/kvm/pv.c
@@ -242,6 +242,10 @@ static void kvm_s390_clear_pv_state(struct kvm *kvm)
 	kvm->arch.pv.guest_len = 0;
 	kvm->arch.pv.stor_base = 0;
 	kvm->arch.pv.stor_var = NULL;
+	if (kvm->arch.pv.dumping) {
+		kvm_s390_vcpu_unblock_all(kvm);
+		kvm->arch.pv.dumping = false;
+	}
 }
 
 static void kvm_s390_pv_dispose_cpu(struct kvm_vcpu *vcpu, bool free_stor_base)
-- 
2.55.0

Re: [PATCH v1 09/11] KVM: s390: Fix kvm_s390_clear_pv_state()

[Christian Borntraeger]

Am 11.08.26 um 17:56 schrieb Claudio Imbrenda:
> kvm_s390_clear_pv_state() needs to also clear the dumping flag, to
> allow the protected VM to be started again (as non-protected, with all
> protected state safely destroyed) after a forced reboot while a
> protected dump was ongoing and not completed.
> 
> Fixes: e40df9efd68a ("KVM: s390: pv: clear the state without memset")
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>


Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>


> ---
>   arch/s390/kvm/pv.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c
> index b02e0159d3cd..98a9a57f71b9 100644
> --- a/arch/s390/kvm/pv.c
> +++ b/arch/s390/kvm/pv.c
> @@ -242,6 +242,10 @@ static void kvm_s390_clear_pv_state(struct kvm *kvm)
>   	kvm->arch.pv.guest_len = 0;
>   	kvm->arch.pv.stor_base = 0;
>   	kvm->arch.pv.stor_var = NULL;
> +	if (kvm->arch.pv.dumping) {
> +		kvm_s390_vcpu_unblock_all(kvm);
> +		kvm->arch.pv.dumping = false;
> +	}
>   }
>   
>   static void kvm_s390_pv_dispose_cpu(struct kvm_vcpu *vcpu, bool free_stor_base)

Re: [PATCH v1 09/11] KVM: s390: Fix kvm_s390_clear_pv_state()

[Christian Borntraeger]

Am 11.08.26 um 17:56 schrieb Claudio Imbrenda:
> kvm_s390_clear_pv_state() needs to also clear the dumping flag, to
> allow the protected VM to be started again (as non-protected, with all
> protected state safely destroyed) after a forced reboot while a
> protected dump was ongoing and not completed.
> 
> Fixes: e40df9efd68a ("KVM: s390: pv: clear the state without memset")
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>

> ---
>   arch/s390/kvm/pv.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c
> index b02e0159d3cd..98a9a57f71b9 100644
> --- a/arch/s390/kvm/pv.c
> +++ b/arch/s390/kvm/pv.c
> @@ -242,6 +242,10 @@ static void kvm_s390_clear_pv_state(struct kvm *kvm)
>   	kvm->arch.pv.guest_len = 0;
>   	kvm->arch.pv.stor_base = 0;
>   	kvm->arch.pv.stor_var = NULL;
> +	if (kvm->arch.pv.dumping) {
> +		kvm_s390_vcpu_unblock_all(kvm);
> +		kvm->arch.pv.dumping = false;
> +	}
>   }
>   
>   static void kvm_s390_pv_dispose_cpu(struct kvm_vcpu *vcpu, bool free_stor_base)

[PATCH v1 10/11] KVM: s390: Fix potential tiny kernel stack leak

[Claudio Imbrenda]

In some circumstances, one bit of kernel stack could have been leaked
from dat_cond_set_storage_key().

Fix by clearing prev before use.

Fixes: 8e03e8316eb2 ("KVM: s390: KVM page table management functions: storage keys")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 arch/s390/kvm/dat.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/s390/kvm/dat.c b/arch/s390/kvm/dat.c
index 47ff8af1b22f..e2a74eb4d631 100644
--- a/arch/s390/kvm/dat.c
+++ b/arch/s390/kvm/dat.c
@@ -740,6 +740,7 @@ int dat_cond_set_storage_key(struct kvm_s390_mmu_cache *mmc, union asce asce, gf
 	pgste.fp = skey.fp;
 	pgste.gc = skey.c;
 	pgste.gr = skey.r;
+	prev.skey = 0;
 
 	if (!ptep->h.i) {
 		rc = page_cond_set_storage_key(pte_origin(*ptep), skey, &prev, nq, mr, mc);
-- 
2.55.0

Re: [PATCH v1 10/11] KVM: s390: Fix potential tiny kernel stack leak

[Christian Borntraeger]

Am 11.08.26 um 17:56 schrieb Claudio Imbrenda:
> In some circumstances, one bit of kernel stack could have been leaked
> from dat_cond_set_storage_key().
> 
> Fix by clearing prev before use.
> 
> Fixes: 8e03e8316eb2 ("KVM: s390: KVM page table management functions: storage keys")
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>


> ---
>   arch/s390/kvm/dat.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/arch/s390/kvm/dat.c b/arch/s390/kvm/dat.c
> index 47ff8af1b22f..e2a74eb4d631 100644
> --- a/arch/s390/kvm/dat.c
> +++ b/arch/s390/kvm/dat.c
> @@ -740,6 +740,7 @@ int dat_cond_set_storage_key(struct kvm_s390_mmu_cache *mmc, union asce asce, gf
>   	pgste.fp = skey.fp;
>   	pgste.gc = skey.c;
>   	pgste.gr = skey.r;
> +	prev.skey = 0;
>   
>   	if (!ptep->h.i) {
>   		rc = page_cond_set_storage_key(pte_origin(*ptep), skey, &prev, nq, mr, mc);

in the declaration
-       union skey prev;
+       union skey prev = {};

or
  prev.zero=0 in the else

would have worked as well, but this way also works.

Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>

[PATCH v1 11/11] KVM: s390: Fix _gaccess_shadow_fault()

[Claudio Imbrenda]

In some circumstances, it is possible that the page of nested guest
memory that is being shadowed is not present at all in the parent guest
gmap. dat_entry_walk() will not find any leaf entry and return with
-ENOENT, which will erroneously be propagated all the way to userspace.

Fix by manually calling gmap_link() on the memory of the nested guest
that is being shadowed if the mapping was not already present.

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

diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
index 36102b2727fb..f3a889988cdb 100644
--- a/arch/s390/kvm/gaccess.c
+++ b/arch/s390/kvm/gaccess.c
@@ -1507,6 +1507,7 @@ static int _do_shadow_crste(struct kvm_s390_mmu_cache *mc, struct gmap *sg, gpa_
 static int _gaccess_do_shadow(struct kvm_s390_mmu_cache *mc, struct gmap *sg,
 			      unsigned long saddr, struct pgtwalk *w)
 {
+	struct kvm_memory_slot *slot;
 	struct guest_fault *entries;
 	int flags, i, hl, gl, l, rc;
 	union crste *table, *host;
@@ -1551,8 +1552,17 @@ static int _gaccess_do_shadow(struct kvm_s390_mmu_cache *mc, struct gmap *sg,
 			return -EAGAIN;
 	}
 
+retry:
 	rc = dat_entry_walk(NULL, entries[LEVEL_MEM].gfn, sg->parent->asce, DAT_WALK_LEAF,
 			    TABLE_TYPE_PAGE_TABLE, &host, &ptep_h);
+	if (rc == -ENOENT) {
+		slot = gfn_to_memslot(sg->kvm, entries[LEVEL_MEM].gfn);
+		if (!slot)
+			return PGM_ADDRESSING;
+		rc = gmap_link(mc, sg->parent, entries + LEVEL_MEM, slot);
+		if (!rc)
+			goto retry;
+	}
 	if (rc)
 		return rc;
 
-- 
2.55.0