[PATCH v4 0/6] KVM s390x PCI fixes

[PATCH v4 0/6] KVM s390x PCI fixes

[Farhan Ali]

Hi,

This series attempts to fix some the pre-existing issues[1] found by sashiko.

[1] https://lore.kernel.org/all/20260624063447.85DF51F000E9@smtp.kernel.org/

Thanks
Farhan

ChangeLog
---------
v3: https://lore.kernel.org/all/20260720175819.1723-1-alifm@linux.ibm.com/
v3 -> v4
   - Add validation checks for AISB/AIBV spanning more than a page.
   - Reject multiple ioctl call for the same device, if adapter interrupt
   forwarding is already enabled for the device.
   - Rebase on 7.2-rc4.

v2: https://lore.kernel.org/all/20260716175241.1039-1-alifm@linux.ibm.com/
v2 -> v3
   - Remove overwriting guest FIB since we don't use it for
   re-issue (patch 4).

v1: https://lore.kernel.org/all/20260713172600.1284-1-alifm@linux.ibm.com/
v1 -> v2
   - Drop fix handling AISB/AIBV spanning multiple pages.
   - Fix memory accounting functions for the case when interrupt forwarding
   is enabled by one process but disabled by a different process (patch 1).


Farhan Ali (6):
  KVM: s390: pci: Reject adapter interrupt forwarding if already enabled
  KVM: s390: pci: Fix memory accounting for pinned/unpinned pages
  KVM: s390: pci: Fix missing error codes and memory unaccounting
  KVM: s390: pci: Fix NULL dereference on AIBV allocation failure
  KVM: s390: pci: Fix resource leak on IRQ registration failure
  KVM: s390: pci: Validate AIBV and AISB before pinning guest pages

 arch/s390/kvm/pci.c | 102 +++++++++++++++++++++++++++++++++++---------
 arch/s390/kvm/pci.h |   2 +
 2 files changed, 84 insertions(+), 20 deletions(-)

-- 
2.43.0

[PATCH v4 1/6] KVM: s390: pci: Reject adapter interrupt forwarding if already enabled

[Farhan Ali]

The MPCIFC instruction doesn't allow registering adapter interrupts without
first unregistering. So reject any request to enable interrupt forwarding
if its already enabled for the zPCI device. This also fixes overwriting and
thus leaking resources when the ioctl is called multiple times for the same
device.

Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
---
 arch/s390/kvm/pci.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
index 720bb58cabe2..d2a11cdf6941 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -237,6 +237,10 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
 	if (zdev->gisa == 0)
 		return -EINVAL;
 
+	/* AIF already enabled for the device */
+	if (zdev->kzdev->fib.fmt0.aibv != 0)
+		return -EINVAL;
+
 	kvm = zdev->kzdev->kvm;
 	msi_vecs = min_t(unsigned int, fib->fmt0.noi, zdev->max_msi);
 
-- 
2.43.0

Re: [PATCH v4 1/6] KVM: s390: pci: Reject adapter interrupt forwarding if already enabled

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Missing NULL check for the result of `airq_iv_create()` in `kvm_s390_pci_aif_enable`.
- [Critical] Missing page boundary check for guest-provided AIBV address allows an out-of-bounds write into host physical memory.
- [High] Reference count leak of `struct user_struct` in `account_mem` and `unaccount_mem`.
- [High] Memory un-accounting uses the current thread's credentials instead of the original ones, causing counter underflows.
- [High] Complete cleanup bypass when `kvm_zpci_set_airq()` fails in `kvm_s390_pci_aif_enable`.
- [High] Use-After-Free of `zdev` structure in `kvm_s390_pci_zpci_op()` due to lack of reference counting.
--

--- Patch [1]: [PATCH v4 1/6] KVM: s390: pci: Reject adapter interrupt forwarding if already enabled ---
commit 32372cae1fd4bf986fc6da1a214da1e85e7be6ad
Author: Farhan Ali <alifm@linux.ibm.com>
KVM: s390: pci: Reject adapter interrupt forwarding if already enabled

This commit prevents registering adapter interrupts via the MPCIFC instruction
if they are already registered. It avoids leaking resources by rejecting
requests to enable interrupt forwarding if it is already enabled for the zPCI
device.

> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c
[ ... ]
>  	list_for_each_entry(kzdev, &kvm->arch.kzdev_list, entry) {
>  		if (kzdev->zdev->fh == fh) {
>  			zdev = kzdev->zdev;
>  			break;
>  		}
>  	}
>  	spin_unlock(&kvm->arch.kzdev_list_lock);
>  	return zdev;

[Severity: High]
This isn't a bug introduced by this patch, but is the returned zdev pointer
vulnerable to a use-after-free?

The pointer is retrieved and returned without elevating the device's reference
count via zpci_zdev_get(). If a concurrent hot-unplug occurs, the PCI core
might remove and free the device.

If the KVM ioctl thread then executes mutex_lock(&zdev->kzdev_lock) in
kvm_s390_pci_zpci_op() on the freed memory, can it lead to memory corruption
or a host crash?

[ ... ]
>  	struct user_struct *user = get_uid(current_user());
[ ... ]
>  	return 0;

[Severity: High]
This is a pre-existing issue, but does account_mem() leak the user_struct
reference?

The get_uid(current_user()) call fetches the user and increments its reference
count. However, the function returns 0 without ever pairing this with a
free_uid(user) call on the success path.

Could an unprivileged guest repeatedly enable and disable AIF to leak kernel
memory continuously and eventually overflow the reference counter?

[ ... ]
>  	struct user_struct *user = get_uid(current_user());
[ ... ]
>  	if (current->mm)
>  		atomic64_sub(nr_pages, &current->mm->pinned_vm);

[Severity: High]
This isn't a bug introduced by this patch, but does unaccount_mem()
incorrectly rely on the current thread's credentials?

When kvm_s390_pci_unregister_kvm() is invoked by the PCI core during device
hot-unplug or VFIO unbind, it executes in a system worker thread, not the
original KVM vcpu thread. It then calls unaccount_mem().

Does unaccount_mem() use current_user() and current->mm of the worker thread
instead of the original ones? Could this corrupt the memory accounting limits
of random processes, cause locked_vm to underflow, and permanently leak the
locked memory quota of the original KVM process?

[ ... ]
> @@ -237,6 +237,10 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
>  	if (zdev->gisa == 0)
>  		return -EINVAL;
>  
> +	/* AIF already enabled for the device */
> +	if (zdev->kzdev->fib.fmt0.aibv != 0)
> +		return -EINVAL;
> +
>  	kvm = zdev->kzdev->kvm;
>  	msi_vecs = min_t(unsigned int, fib->fmt0.noi, zdev->max_msi);
[ ... ]
>  	npages = pin_user_pages_fast(hva, 1, FOLL_WRITE | FOLL_LONGTERM, pages);
[ ... ]
>  	gaddr = page_to_phys(aibv_page) + (fib->fmt0.aibv & ~PAGE_MASK);
>  	fib->fmt0.aibv = gaddr;

[Severity: Critical]
This is a pre-existing issue, but does this calculation allow an out-of-bounds
write into host physical memory?

Exactly one page is pinned, but the physical address is constructed by adding
an unaligned offset (fib->fmt0.aibv & ~PAGE_MASK). It doesn't appear to verify
if the required bit-vector size (msi_vecs bits) fits within the remaining
bounds of this single pinned page.

If a guest provides an address with a high intra-page offset, could the
hardware page-table walker or DMA overwrite the adjacent, unpinned physical
page and cause severe host memory corruption?

[ ... ]
>  	zdev->aibv = airq_iv_create(msi_vecs, AIRQ_IV_DATA |
>  				    AIRQ_IV_BITLOCK |
>  				    AIRQ_IV_GUESTVEC,
>  				    phys_to_virt(fib->fmt0.aibv));
>  
>  	spin_lock_irq(&aift->gait_lock);

[Severity: Critical]
This isn't a bug introduced by this patch, but does airq_iv_create() return a
pointer that is used without a NULL check?

If airq_iv_create() dynamically allocates memory and returns NULL on failure,
the pointer seems to be assigned to zdev->aibv and then dereferenced shortly
after in kvm_zpci_set_airq() via airq_iv_end().

Can an unprivileged guest trigger this path via KVM_S390_ZPCIOP_REG_AEN and
crash the host system if it is under memory pressure?

[ ... ]
>  	/* Issue the clp to setup the irq now */
>  	rc = kvm_zpci_set_airq(zdev);
>  	return rc;
>  
>  unlock:
>  	mutex_unlock(&aift->aift_lock);

[Severity: High]
This is a pre-existing issue, but does the function bypass the cleanup block
if kvm_zpci_set_airq() fails?

If the hardware rejects the configuration and kvm_zpci_set_airq() returns an
error, the function appears to return rc directly instead of jumping to the
cleanup labels (unlock:, unpin2:, out:).

Could this leave pinned pages, memory accounting, GAIT entries, and airq_iv
structures leaked? Since zdev->kzdev->fib.fmt0.aibv is left with a non-zero
value, does this persistently break the device for the guest?

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

Re: [PATCH v4 1/6] KVM: s390: pci: Reject adapter interrupt forwarding if already enabled

[Christian Borntraeger]

Am 22.07.26 um 19:06 schrieb Farhan Ali:
> The MPCIFC instruction doesn't allow registering adapter interrupts without
> first unregistering. So reject any request to enable interrupt forwarding
> if its already enabled for the zPCI device. This also fixes overwriting and
> thus leaking resources when the ioctl is called multiple times for the same
> device.
> 
> Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>

> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
> ---
>   arch/s390/kvm/pci.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index 720bb58cabe2..d2a11cdf6941 100644
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c
> @@ -237,6 +237,10 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
>   	if (zdev->gisa == 0)
>   		return -EINVAL;
>   
> +	/* AIF already enabled for the device */
> +	if (zdev->kzdev->fib.fmt0.aibv != 0)
> +		return -EINVAL;
> +
>   	kvm = zdev->kzdev->kvm;
>   	msi_vecs = min_t(unsigned int, fib->fmt0.noi, zdev->max_msi);
>   

Re: [PATCH v4 1/6] KVM: s390: pci: Reject adapter interrupt forwarding if already enabled

[Matthew Rosato]

On 7/22/26 1:06 PM, Farhan Ali wrote:
> The MPCIFC instruction doesn't allow registering adapter interrupts without
> first unregistering. So reject any request to enable interrupt forwarding
> if its already enabled for the zPCI device. This also fixes overwriting and
> thus leaking resources when the ioctl is called multiple times for the same
> device.
> 
> Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>

Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>

> ---
>  arch/s390/kvm/pci.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index 720bb58cabe2..d2a11cdf6941 100644
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c
> @@ -237,6 +237,10 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
>  	if (zdev->gisa == 0)
>  		return -EINVAL;
>  
> +	/* AIF already enabled for the device */
> +	if (zdev->kzdev->fib.fmt0.aibv != 0)
> +		return -EINVAL;
> +
>  	kvm = zdev->kzdev->kvm;
>  	msi_vecs = min_t(unsigned int, fib->fmt0.noi, zdev->max_msi);
>  

[PATCH v4 2/6] KVM: s390: pci: Fix memory accounting for pinned/unpinned pages

[Farhan Ali]

The account_mem() and unaccount_mem() functions call get_uid() which
increments the reference count of struct user_struct on every invocation.
But we don't decrement the count by calling free_uid(). It also
accounted/unaccounted the pages against the current->mm. But its possible
the unaccount_mem() can be called from a different process context than the
one that originally pinned the pages.

Let's fix this by storing the pinning process user_struct and mm_struct
when accounting for pinned pages, and subsequently free these resources
when the pages are unpinned.

Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
---
 arch/s390/kvm/pci.c | 38 ++++++++++++++++++++++++++++----------
 arch/s390/kvm/pci.h |  2 ++
 2 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
index d2a11cdf6941..1b3114c7cfbb 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -190,33 +190,51 @@ static int kvm_zpci_clear_airq(struct zpci_dev *zdev)
 	return cc ? -EIO : 0;
 }
 
-static inline void unaccount_mem(unsigned long nr_pages)
+static inline void unaccount_mem(struct kvm_zdev *kzdev, unsigned long nr_pages)
 {
-	struct user_struct *user = get_uid(current_user());
+	struct user_struct *user = kzdev->user_account;
+	struct mm_struct *mm_account = kzdev->mm_account;
 
-	if (user)
+	if (user) {
 		atomic_long_sub(nr_pages, &user->locked_vm);
-	if (current->mm)
-		atomic64_sub(nr_pages, &current->mm->pinned_vm);
+		free_uid(user);
+		kzdev->user_account = NULL;
+	}
+
+	if (mm_account) {
+		atomic64_sub(nr_pages, &mm_account->pinned_vm);
+		mmdrop(mm_account);
+		kzdev->mm_account = NULL;
+	}
 }
 
-static inline int account_mem(unsigned long nr_pages)
+static inline int account_mem(struct kvm_zdev *kzdev, unsigned long nr_pages)
 {
 	struct user_struct *user = get_uid(current_user());
 	unsigned long page_limit, cur_pages, new_pages;
+	int rc = 0;
 
 	page_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
 
 	cur_pages = atomic_long_read(&user->locked_vm);
 	do {
 		new_pages = cur_pages + nr_pages;
-		if (new_pages > page_limit)
-			return -ENOMEM;
+		if (new_pages > page_limit) {
+			rc =  -ENOMEM;
+			goto out;
+		}
 	} while (!atomic_long_try_cmpxchg(&user->locked_vm, &cur_pages, new_pages));
 
+	mmgrab(current->mm);
 	atomic64_add(nr_pages, &current->mm->pinned_vm);
+	kzdev->user_account = user;
+	kzdev->mm_account = current->mm;
 
 	return 0;
+
+out:
+	free_uid(user);
+	return rc;
 }
 
 static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
@@ -279,7 +297,7 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
 	}
 
 	/* Account for pinned pages, roll back on failure */
-	if (account_mem(pcount))
+	if (account_mem(zdev->kzdev, pcount))
 		goto unpin2;
 
 	/* AISB must be allocated before we can fill in GAITE */
@@ -400,7 +418,7 @@ static int kvm_s390_pci_aif_disable(struct zpci_dev *zdev, bool force)
 		pcount++;
 	}
 	if (pcount > 0)
-		unaccount_mem(pcount);
+		unaccount_mem(kzdev, pcount);
 out:
 	mutex_unlock(&aift->aift_lock);
 
diff --git a/arch/s390/kvm/pci.h b/arch/s390/kvm/pci.h
index ff0972dd5e71..544e6aa75e38 100644
--- a/arch/s390/kvm/pci.h
+++ b/arch/s390/kvm/pci.h
@@ -21,6 +21,8 @@ struct kvm_zdev {
 	struct zpci_dev *zdev;
 	struct kvm *kvm;
 	struct zpci_fib fib;
+	struct user_struct *user_account;
+	struct mm_struct *mm_account;
 	struct list_head entry;
 };
 
-- 
2.43.0

Re: [PATCH v4 2/6] KVM: s390: pci: Fix memory accounting for pinned/unpinned pages

[sashiko-bot]

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

Pre-existing issues:
- [High] Race condition between KVM_S390_ZPCI_OP ioctl and device hot-unplug leads to Use-After-Free on `zdev`.
--

--- Patch [2]: [PATCH v4 2/6] KVM: s390: pci: Fix memory accounting for pinned/unpinned pages ---
commit 2ef683f1261aa42729e17194a74f67aefe6f5d45
Author: Farhan Ali <alifm@linux.ibm.com>

KVM: s390: pci: Fix memory accounting for pinned/unpinned pages
    
The account_mem() and unaccount_mem() functions call get_uid() which
increments the reference count of struct user_struct on every invocation.
This commit fixes this by storing the pinning process user_struct and
mm_struct when accounting for pinned pages, and subsequently freeing these
resources when the pages are unpinned.

> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index d2a11cdf6941d..1b3114c7cfbb8 100644
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but can a race condition between
the KVM_S390_ZPCI_OP ioctl and device hot-unplug lead to a use-after-free on
the zdev structure?

Looking at get_zdev_from_kvm_by_fh() in arch/s390/kvm/pci.c:

			zdev = kzdev->zdev;
			break;
		}
	}
	spin_unlock(&kvm->arch.kzdev_list_lock);

	return zdev;

The zdev pointer is returned without taking a reference, leaving it
vulnerable to being freed once the list lock is dropped.

Then in kvm_s390_pci_zpci_op():

	zdev = get_zdev_from_kvm_by_fh(kvm, args->fh);
	if (!zdev)
		return -ENODEV;

	mutex_lock(&zdev->kzdev_lock);

The ioctl thread dereferences the potentially freed zdev structure by
attempting to acquire its lock, outside of any RCU protection.

If a device hot-unplug is initiated on the host, and QEMU closes the VFIO
file descriptor after the ioctl thread drops the list lock but before it
acquires kzdev_lock, the device can be unregistered and freed.

Because the ioctl thread cannot be in an RCU read-side critical section
while it prepares to sleep on a mutex, the RCU grace period can elapse.
This could allow the ioctl thread to resume and attempt to call mutex_lock()
on the freed zdev structure.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722170621.1686-1-alifm@linux.ibm.com?part=2

Re: [PATCH v4 2/6] KVM: s390: pci: Fix memory accounting for pinned/unpinned pages

[Matthew Rosato]

On 7/22/26 1:06 PM, Farhan Ali wrote:
> The account_mem() and unaccount_mem() functions call get_uid() which
> increments the reference count of struct user_struct on every invocation.
> But we don't decrement the count by calling free_uid(). It also
> accounted/unaccounted the pages against the current->mm. But its possible
> the unaccount_mem() can be called from a different process context than the
> one that originally pinned the pages.
> 
> Let's fix this by storing the pinning process user_struct and mm_struct
> when accounting for pinned pages, and subsequently free these resources
> when the pages are unpinned.
> 
> Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
> ---
>  arch/s390/kvm/pci.c | 38 ++++++++++++++++++++++++++++----------
>  arch/s390/kvm/pci.h |  2 ++
>  2 files changed, 30 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index d2a11cdf6941..1b3114c7cfbb 100644
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c
> @@ -190,33 +190,51 @@ static int kvm_zpci_clear_airq(struct zpci_dev *zdev)
>  	return cc ? -EIO : 0;
>  }
>  
> -static inline void unaccount_mem(unsigned long nr_pages)
> +static inline void unaccount_mem(struct kvm_zdev *kzdev, unsigned long nr_pages)
>  {
> -	struct user_struct *user = get_uid(current_user());
> +	struct user_struct *user = kzdev->user_account;
> +	struct mm_struct *mm_account = kzdev->mm_account;
>  
> -	if (user)
> +	if (user) {
>  		atomic_long_sub(nr_pages, &user->locked_vm);
> -	if (current->mm)
> -		atomic64_sub(nr_pages, &current->mm->pinned_vm);

Previous code handled the case where current->mm could be NULL...

> +		free_uid(user);
> +		kzdev->user_account = NULL;
> +	}
> +
> +	if (mm_account) {

... And you check for kzdev->mm_account being NULL here...

> +		atomic64_sub(nr_pages, &mm_account->pinned_vm);
> +		mmdrop(mm_account);
> +		kzdev->mm_account = NULL;
> +	}
>  }
>  
> -static inline int account_mem(unsigned long nr_pages)
> +static inline int account_mem(struct kvm_zdev *kzdev, unsigned long nr_pages)
>  {
>  	struct user_struct *user = get_uid(current_user());
>  	unsigned long page_limit, cur_pages, new_pages;
> +	int rc = 0;
>  
>  	page_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
>  
>  	cur_pages = atomic_long_read(&user->locked_vm);
>  	do {
>  		new_pages = cur_pages + nr_pages;
> -		if (new_pages > page_limit)
> -			return -ENOMEM;
> +		if (new_pages > page_limit) {
> +			rc =  -ENOMEM;
> +			goto out;
> +		}
>  	} while (!atomic_long_try_cmpxchg(&user->locked_vm, &cur_pages, new_pages));
>  
> +	mmgrab(current->mm);

... But here you do not check if current->mm is NULL.  I'm not sure if
it will happen in practice, but we did guard against it before this
patch.  Just add if (!current->mm) around this mmgrab?

>  	atomic64_add(nr_pages, &current->mm->pinned_vm);
> +	kzdev->user_account = user;
> +	kzdev->mm_account = current->mm;

Then if it IS NULL, we will stash a NULL into kzdev->mm_account here and
handle it above as before with the if (mm_account) check.

>  
>  	return 0;
> +
> +out:
> +	free_uid(user);
> +	return rc;
>  }
>  
>  static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
> @@ -279,7 +297,7 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
>  	}
>  
>  	/* Account for pinned pages, roll back on failure */
> -	if (account_mem(pcount))
> +	if (account_mem(zdev->kzdev, pcount))
>  		goto unpin2;
>  
>  	/* AISB must be allocated before we can fill in GAITE */
> @@ -400,7 +418,7 @@ static int kvm_s390_pci_aif_disable(struct zpci_dev *zdev, bool force)
>  		pcount++;
>  	}
>  	if (pcount > 0)
> -		unaccount_mem(pcount);
> +		unaccount_mem(kzdev, pcount);
>  out:
>  	mutex_unlock(&aift->aift_lock);
>  
> diff --git a/arch/s390/kvm/pci.h b/arch/s390/kvm/pci.h
> index ff0972dd5e71..544e6aa75e38 100644
> --- a/arch/s390/kvm/pci.h
> +++ b/arch/s390/kvm/pci.h
> @@ -21,6 +21,8 @@ struct kvm_zdev {
>  	struct zpci_dev *zdev;
>  	struct kvm *kvm;
>  	struct zpci_fib fib;
> +	struct user_struct *user_account;
> +	struct mm_struct *mm_account;
>  	struct list_head entry;
>  };
>  

Re: [PATCH v4 2/6] KVM: s390: pci: Fix memory accounting for pinned/unpinned pages

[Farhan Ali]

On 7/23/2026 7:12 AM, Matthew Rosato wrote:
> On 7/22/26 1:06 PM, Farhan Ali wrote:
>> The account_mem() and unaccount_mem() functions call get_uid() which
>> increments the reference count of struct user_struct on every invocation.
>> But we don't decrement the count by calling free_uid(). It also
>> accounted/unaccounted the pages against the current->mm. But its possible
>> the unaccount_mem() can be called from a different process context than the
>> one that originally pinned the pages.
>>
>> Let's fix this by storing the pinning process user_struct and mm_struct
>> when accounting for pinned pages, and subsequently free these resources
>> when the pages are unpinned.
>>
>> Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
>> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
>> ---
>>   arch/s390/kvm/pci.c | 38 ++++++++++++++++++++++++++++----------
>>   arch/s390/kvm/pci.h |  2 ++
>>   2 files changed, 30 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
>> index d2a11cdf6941..1b3114c7cfbb 100644
>> --- a/arch/s390/kvm/pci.c
>> +++ b/arch/s390/kvm/pci.c
>> @@ -190,33 +190,51 @@ static int kvm_zpci_clear_airq(struct zpci_dev *zdev)
>>   	return cc ? -EIO : 0;
>>   }
>>   
>> -static inline void unaccount_mem(unsigned long nr_pages)
>> +static inline void unaccount_mem(struct kvm_zdev *kzdev, unsigned long nr_pages)
>>   {
>> -	struct user_struct *user = get_uid(current_user());
>> +	struct user_struct *user = kzdev->user_account;
>> +	struct mm_struct *mm_account = kzdev->mm_account;
>>   
>> -	if (user)
>> +	if (user) {
>>   		atomic_long_sub(nr_pages, &user->locked_vm);
>> -	if (current->mm)
>> -		atomic64_sub(nr_pages, &current->mm->pinned_vm);
> Previous code handled the case where current->mm could be NULL...
>
>> +		free_uid(user);
>> +		kzdev->user_account = NULL;
>> +	}
>> +
>> +	if (mm_account) {
> ... And you check for kzdev->mm_account being NULL here...
>
>> +		atomic64_sub(nr_pages, &mm_account->pinned_vm);
>> +		mmdrop(mm_account);
>> +		kzdev->mm_account = NULL;
>> +	}
>>   }
>>   
>> -static inline int account_mem(unsigned long nr_pages)
>> +static inline int account_mem(struct kvm_zdev *kzdev, unsigned long nr_pages)
>>   {
>>   	struct user_struct *user = get_uid(current_user());
>>   	unsigned long page_limit, cur_pages, new_pages;
>> +	int rc = 0;
>>   
>>   	page_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
>>   
>>   	cur_pages = atomic_long_read(&user->locked_vm);
>>   	do {
>>   		new_pages = cur_pages + nr_pages;
>> -		if (new_pages > page_limit)
>> -			return -ENOMEM;
>> +		if (new_pages > page_limit) {
>> +			rc =  -ENOMEM;
>> +			goto out;
>> +		}
>>   	} while (!atomic_long_try_cmpxchg(&user->locked_vm, &cur_pages, new_pages));
>>   
>> +	mmgrab(current->mm);
> ... But here you do not check if current->mm is NULL.  I'm not sure if
> it will happen in practice, but we did guard against it before this
> patch.  Just add if (!current->mm) around this mmgrab?

But wouldn't the atomic64_add just below this also need to be in the 
check? FWIW looking at some of the other references to mmgrab, I didn't 
see explicit checks for the current->mm [1] [2]

[1] 
https://elixir.bootlin.com/linux/v7.2-rc4/source/io_uring/io_uring.c#L3047

[2] 
https://elixir.bootlin.com/linux/v7.2-rc4/source/drivers/vfio/vfio_iommu_type1.c#L1675

Thanks

Farhan


>
>>   	atomic64_add(nr_pages, &current->mm->pinned_vm);
>> +	kzdev->user_account = user;
>> +	kzdev->mm_account = current->mm;
> Then if it IS NULL, we will stash a NULL into kzdev->mm_account here and
> handle it above as before with the if (mm_account) check.
>
>>   
>>   	return 0;
>> +
>> +out:
>> +	free_uid(user);
>> +	return rc;
>>   }
>>   
>>   static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
>> @@ -279,7 +297,7 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
>>   	}
>>   
>>   	/* Account for pinned pages, roll back on failure */
>> -	if (account_mem(pcount))
>> +	if (account_mem(zdev->kzdev, pcount))
>>   		goto unpin2;
>>   
>>   	/* AISB must be allocated before we can fill in GAITE */
>> @@ -400,7 +418,7 @@ static int kvm_s390_pci_aif_disable(struct zpci_dev *zdev, bool force)
>>   		pcount++;
>>   	}
>>   	if (pcount > 0)
>> -		unaccount_mem(pcount);
>> +		unaccount_mem(kzdev, pcount);
>>   out:
>>   	mutex_unlock(&aift->aift_lock);
>>   
>> diff --git a/arch/s390/kvm/pci.h b/arch/s390/kvm/pci.h
>> index ff0972dd5e71..544e6aa75e38 100644
>> --- a/arch/s390/kvm/pci.h
>> +++ b/arch/s390/kvm/pci.h
>> @@ -21,6 +21,8 @@ struct kvm_zdev {
>>   	struct zpci_dev *zdev;
>>   	struct kvm *kvm;
>>   	struct zpci_fib fib;
>> +	struct user_struct *user_account;
>> +	struct mm_struct *mm_account;
>>   	struct list_head entry;
>>   };
>>   

Re: [PATCH v4 2/6] KVM: s390: pci: Fix memory accounting for pinned/unpinned pages

[Matthew Rosato]

On 7/23/26 1:08 PM, Farhan Ali wrote:
> 
> On 7/23/2026 7:12 AM, Matthew Rosato wrote:
>> On 7/22/26 1:06 PM, Farhan Ali wrote:
>>> The account_mem() and unaccount_mem() functions call get_uid() which
>>> increments the reference count of struct user_struct on every
>>> invocation.
>>> But we don't decrement the count by calling free_uid(). It also
>>> accounted/unaccounted the pages against the current->mm. But its
>>> possible
>>> the unaccount_mem() can be called from a different process context
>>> than the
>>> one that originally pinned the pages.
>>>
>>> Let's fix this by storing the pinning process user_struct and mm_struct
>>> when accounting for pinned pages, and subsequently free these resources
>>> when the pages are unpinned.
>>>
>>> Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/
>>> disabling interrupt forwarding")
>>> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
>>> ---
>>>   arch/s390/kvm/pci.c | 38 ++++++++++++++++++++++++++++----------
>>>   arch/s390/kvm/pci.h |  2 ++
>>>   2 files changed, 30 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
>>> index d2a11cdf6941..1b3114c7cfbb 100644
>>> --- a/arch/s390/kvm/pci.c
>>> +++ b/arch/s390/kvm/pci.c
>>> @@ -190,33 +190,51 @@ static int kvm_zpci_clear_airq(struct zpci_dev
>>> *zdev)
>>>       return cc ? -EIO : 0;
>>>   }
>>>   -static inline void unaccount_mem(unsigned long nr_pages)
>>> +static inline void unaccount_mem(struct kvm_zdev *kzdev, unsigned
>>> long nr_pages)
>>>   {
>>> -    struct user_struct *user = get_uid(current_user());
>>> +    struct user_struct *user = kzdev->user_account;
>>> +    struct mm_struct *mm_account = kzdev->mm_account;
>>>   -    if (user)
>>> +    if (user) {
>>>           atomic_long_sub(nr_pages, &user->locked_vm);
>>> -    if (current->mm)
>>> -        atomic64_sub(nr_pages, &current->mm->pinned_vm);
>> Previous code handled the case where current->mm could be NULL...
>>
>>> +        free_uid(user);
>>> +        kzdev->user_account = NULL;
>>> +    }
>>> +
>>> +    if (mm_account) {
>> ... And you check for kzdev->mm_account being NULL here...
>>
>>> +        atomic64_sub(nr_pages, &mm_account->pinned_vm);
>>> +        mmdrop(mm_account);
>>> +        kzdev->mm_account = NULL;
>>> +    }
>>>   }
>>>   -static inline int account_mem(unsigned long nr_pages)
>>> +static inline int account_mem(struct kvm_zdev *kzdev, unsigned long
>>> nr_pages)
>>>   {
>>>       struct user_struct *user = get_uid(current_user());
>>>       unsigned long page_limit, cur_pages, new_pages;
>>> +    int rc = 0;
>>>         page_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
>>>         cur_pages = atomic_long_read(&user->locked_vm);
>>>       do {
>>>           new_pages = cur_pages + nr_pages;
>>> -        if (new_pages > page_limit)
>>> -            return -ENOMEM;
>>> +        if (new_pages > page_limit) {
>>> +            rc =  -ENOMEM;
>>> +            goto out;
>>> +        }
>>>       } while (!atomic_long_try_cmpxchg(&user->locked_vm, &cur_pages,
>>> new_pages));
>>>   +    mmgrab(current->mm);
>> ... But here you do not check if current->mm is NULL.  I'm not sure if
>> it will happen in practice, but we did guard against it before this
>> patch.  Just add if (!current->mm) around this mmgrab?
> 
> But wouldn't the atomic64_add just below this also need to be in the
> check? FWIW looking at some of the other references to mmgrab, I didn't

Yep good point - my intent was to not mess with a structure pointing at
address 0, so you'd want to skip that too.

As for other references to mmgrab, I don't know, but
https://www.kernel.org/doc/Documentation/mm/active_mm.rst
Specifically says to check
if (!current->mm)
to ensure we have user context.

Now, here's the thing:  I believe in practice today all of our calls to
account_mem will have a user context because they are done in response
to an ioctl.  And it is likely that the examples you looked at are also
guaranteed to be in user context.

The problem would be if we were to ever call this accounting function
later from a kernel thread where current->mm was indeed NULL.  I suspect
it was either a review comment on the initial implementation or a case
of 'easy enough to protect against it'

> see explicit checks for the current->mm [1] [2]
> 
> [1] https://elixir.bootlin.com/linux/v7.2-rc4/source/io_uring/
> io_uring.c#L3047
> 
> [2] https://elixir.bootlin.com/linux/v7.2-rc4/source/drivers/vfio/
> vfio_iommu_type1.c#L1675
> 
> Thanks
> 
> Farhan
> 
> 
>>
>>>       atomic64_add(nr_pages, &current->mm->pinned_vm);
>>> +    kzdev->user_account = user;
>>> +    kzdev->mm_account = current->mm;
>> Then if it IS NULL, we will stash a NULL into kzdev->mm_account here and
>> handle it above as before with the if (mm_account) check.
>>
>>>         return 0;
>>> +
>>> +out:
>>> +    free_uid(user);
>>> +    return rc;
>>>   }
>>>     static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct
>>> zpci_fib *fib,
>>> @@ -279,7 +297,7 @@ static int kvm_s390_pci_aif_enable(struct
>>> zpci_dev *zdev, struct zpci_fib *fib,
>>>       }
>>>         /* Account for pinned pages, roll back on failure */
>>> -    if (account_mem(pcount))
>>> +    if (account_mem(zdev->kzdev, pcount))
>>>           goto unpin2;
>>>         /* AISB must be allocated before we can fill in GAITE */
>>> @@ -400,7 +418,7 @@ static int kvm_s390_pci_aif_disable(struct
>>> zpci_dev *zdev, bool force)
>>>           pcount++;
>>>       }
>>>       if (pcount > 0)
>>> -        unaccount_mem(pcount);
>>> +        unaccount_mem(kzdev, pcount);
>>>   out:
>>>       mutex_unlock(&aift->aift_lock);
>>>   diff --git a/arch/s390/kvm/pci.h b/arch/s390/kvm/pci.h
>>> index ff0972dd5e71..544e6aa75e38 100644
>>> --- a/arch/s390/kvm/pci.h
>>> +++ b/arch/s390/kvm/pci.h
>>> @@ -21,6 +21,8 @@ struct kvm_zdev {
>>>       struct zpci_dev *zdev;
>>>       struct kvm *kvm;
>>>       struct zpci_fib fib;
>>> +    struct user_struct *user_account;
>>> +    struct mm_struct *mm_account;
>>>       struct list_head entry;
>>>   };
>>>   

[PATCH v4 3/6] KVM: s390: pci: Fix missing error codes and memory unaccounting

[Farhan Ali]

In kvm_s390_pci_aif_enable() two error paths failed to set error code,
causing the function to return 0 on failure. It also failed to rollback
memory accounting on failure. Fix both by propagating error code on
failure and calling unaccount_mem() in the cleanup path.

Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
---
 arch/s390/kvm/pci.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
index 1b3114c7cfbb..33abc15aa685 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -297,14 +297,17 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
 	}
 
 	/* Account for pinned pages, roll back on failure */
-	if (account_mem(zdev->kzdev, pcount))
+	rc = account_mem(zdev->kzdev, pcount);
+	if (rc)
 		goto unpin2;
 
 	/* AISB must be allocated before we can fill in GAITE */
 	mutex_lock(&aift->aift_lock);
 	bit = airq_iv_alloc_bit(aift->sbv);
-	if (bit == -1UL)
+	if (bit == -1UL) {
+		rc = -ENOMEM;
 		goto unlock;
+	}
 	zdev->aisb = bit; /* store the summary bit number */
 	zdev->aibv = airq_iv_create(msi_vecs, AIRQ_IV_DATA |
 				    AIRQ_IV_BITLOCK |
@@ -348,6 +351,8 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
 	return rc;
 
 unlock:
+	if (pcount > 0)
+		unaccount_mem(zdev->kzdev, pcount);
 	mutex_unlock(&aift->aift_lock);
 unpin2:
 	if (fib->fmt0.sum == 1)
-- 
2.43.0

Re: [PATCH v4 3/6] KVM: s390: pci: Fix missing error codes and memory unaccounting

[sashiko-bot]

> In kvm_s390_pci_aif_enable() two error paths failed to set error code,
> causing the function to return 0 on failure. It also failed to rollback
> memory accounting on failure. Fix both by propagating error code on
> failure and calling unaccount_mem() in the cleanup path.
> 
> Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

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

Re: [PATCH v4 3/6] KVM: s390: pci: Fix missing error codes and memory unaccounting

[Christian Borntraeger]

Am 22.07.26 um 19:06 schrieb Farhan Ali:
> In kvm_s390_pci_aif_enable() two error paths failed to set error code,
> causing the function to return 0 on failure. It also failed to rollback
> memory accounting on failure. Fix both by propagating error code on
> failure and calling unaccount_mem() in the cleanup path.
> 
> Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>

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

> ---
>   arch/s390/kvm/pci.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index 1b3114c7cfbb..33abc15aa685 100644
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c
> @@ -297,14 +297,17 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
>   	}
>   
>   	/* Account for pinned pages, roll back on failure */
> -	if (account_mem(zdev->kzdev, pcount))
> +	rc = account_mem(zdev->kzdev, pcount);
> +	if (rc)
>   		goto unpin2;
>   
>   	/* AISB must be allocated before we can fill in GAITE */
>   	mutex_lock(&aift->aift_lock);
>   	bit = airq_iv_alloc_bit(aift->sbv);
> -	if (bit == -1UL)
> +	if (bit == -1UL) {
> +		rc = -ENOMEM;
>   		goto unlock;
> +	}
>   	zdev->aisb = bit; /* store the summary bit number */
>   	zdev->aibv = airq_iv_create(msi_vecs, AIRQ_IV_DATA |
>   				    AIRQ_IV_BITLOCK |
> @@ -348,6 +351,8 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
>   	return rc;
>   
>   unlock:
> +	if (pcount > 0)
> +		unaccount_mem(zdev->kzdev, pcount);
>   	mutex_unlock(&aift->aift_lock);
>   unpin2:
>   	if (fib->fmt0.sum == 1)

Re: [PATCH v4 3/6] KVM: s390: pci: Fix missing error codes and memory unaccounting

[Matthew Rosato]

On 7/22/26 1:06 PM, Farhan Ali wrote:
> In kvm_s390_pci_aif_enable() two error paths failed to set error code,

nit: set 'an' error code

> causing the function to return 0 on failure. It also failed to rollback
> memory accounting on failure. Fix both by propagating error code on

'an' error code

> failure and calling unaccount_mem() in the cleanup path.

With or without those changes:

Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>

> 
> Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
> ---
>  arch/s390/kvm/pci.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index 1b3114c7cfbb..33abc15aa685 100644
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c
> @@ -297,14 +297,17 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
>  	}
>  
>  	/* Account for pinned pages, roll back on failure */
> -	if (account_mem(zdev->kzdev, pcount))
> +	rc = account_mem(zdev->kzdev, pcount);
> +	if (rc)
>  		goto unpin2;
>  
>  	/* AISB must be allocated before we can fill in GAITE */
>  	mutex_lock(&aift->aift_lock);
>  	bit = airq_iv_alloc_bit(aift->sbv);
> -	if (bit == -1UL)
> +	if (bit == -1UL) {
> +		rc = -ENOMEM;
>  		goto unlock;
> +	}
>  	zdev->aisb = bit; /* store the summary bit number */
>  	zdev->aibv = airq_iv_create(msi_vecs, AIRQ_IV_DATA |
>  				    AIRQ_IV_BITLOCK |
> @@ -348,6 +351,8 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
>  	return rc;
>  
>  unlock:
> +	if (pcount > 0)
> +		unaccount_mem(zdev->kzdev, pcount);
>  	mutex_unlock(&aift->aift_lock);
>  unpin2:
>  	if (fib->fmt0.sum == 1)

[PATCH v4 4/6] KVM: s390: pci: Fix NULL dereference on AIBV allocation failure

[Farhan Ali]

The airq_iv_create() can return NULL on failure, but the return value was
never checked. If it fails, zdev->aibv will be NULL and fail when
derefenced in kvm_zpci_set_airq(). Add a NULL check and free the previously
allocated AISB bit and zdev->aisb on failure.

Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
---
 arch/s390/kvm/pci.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
index 33abc15aa685..231a4236fc3c 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -314,6 +314,11 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
 				    AIRQ_IV_GUESTVEC,
 				    phys_to_virt(fib->fmt0.aibv));
 
+	if (!zdev->aibv) {
+		rc = -ENOMEM;
+		goto free_aisb;
+	}
+
 	spin_lock_irq(&aift->gait_lock);
 	gaite = aift->gait + zdev->aisb;
 
@@ -350,6 +355,9 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
 	rc = kvm_zpci_set_airq(zdev);
 	return rc;
 
+free_aisb:
+	airq_iv_free_bit(aift->sbv, zdev->aisb);
+	zdev->aisb = 0;
 unlock:
 	if (pcount > 0)
 		unaccount_mem(zdev->kzdev, pcount);
-- 
2.43.0

Re: [PATCH v4 4/6] KVM: s390: pci: Fix NULL dereference on AIBV allocation failure

[sashiko-bot]

> The airq_iv_create() can return NULL on failure, but the return value was
> never checked. If it fails, zdev->aibv will be NULL and fail when
> derefenced in kvm_zpci_set_airq(). Add a NULL check and free the previously
> allocated AISB bit and zdev->aisb on failure.
> 
> Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

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

Re: [PATCH v4 4/6] KVM: s390: pci: Fix NULL dereference on AIBV allocation failure

[Christian Borntraeger]

Am 22.07.26 um 19:06 schrieb Farhan Ali:
> The airq_iv_create() can return NULL on failure, but the return value was
> never checked. If it fails, zdev->aibv will be NULL and fail when
> derefenced in kvm_zpci_set_airq(). Add a NULL check and free the previously
> allocated AISB bit and zdev->aisb on failure.
> 
> Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>

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


> ---
>   arch/s390/kvm/pci.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index 33abc15aa685..231a4236fc3c 100644
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c
> @@ -314,6 +314,11 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
>   				    AIRQ_IV_GUESTVEC,
>   				    phys_to_virt(fib->fmt0.aibv));
>   
> +	if (!zdev->aibv) {
> +		rc = -ENOMEM;
> +		goto free_aisb;
> +	}
> +
>   	spin_lock_irq(&aift->gait_lock);
>   	gaite = aift->gait + zdev->aisb;
>   
> @@ -350,6 +355,9 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
>   	rc = kvm_zpci_set_airq(zdev);
>   	return rc;
>   
> +free_aisb:
> +	airq_iv_free_bit(aift->sbv, zdev->aisb);
> +	zdev->aisb = 0;
>   unlock:
>   	if (pcount > 0)
>   		unaccount_mem(zdev->kzdev, pcount);

Re: [PATCH v4 4/6] KVM: s390: pci: Fix NULL dereference on AIBV allocation failure

[Matthew Rosato]

On 7/22/26 1:06 PM, Farhan Ali wrote:
> The airq_iv_create() can return NULL on failure, but the return value was
> never checked. If it fails, zdev->aibv will be NULL and fail when
> derefenced in kvm_zpci_set_airq(). Add a NULL check and free the previously

Nit: s/derefenced/dereferenced/


> allocated AISB bit and zdev->aisb on failure.
> 
> Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>

Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>

> ---
>  arch/s390/kvm/pci.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index 33abc15aa685..231a4236fc3c 100644
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c
> @@ -314,6 +314,11 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
>  				    AIRQ_IV_GUESTVEC,
>  				    phys_to_virt(fib->fmt0.aibv));
>  
> +	if (!zdev->aibv) {
> +		rc = -ENOMEM;
> +		goto free_aisb;
> +	}
> +
>  	spin_lock_irq(&aift->gait_lock);
>  	gaite = aift->gait + zdev->aisb;
>  
> @@ -350,6 +355,9 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
>  	rc = kvm_zpci_set_airq(zdev);
>  	return rc;
>  
> +free_aisb:
> +	airq_iv_free_bit(aift->sbv, zdev->aisb);
> +	zdev->aisb = 0;
>  unlock:
>  	if (pcount > 0)
>  		unaccount_mem(zdev->kzdev, pcount);

[PATCH v4 5/6] KVM: s390: pci: Fix resource leak on IRQ registration failure

[Farhan Ali]

Currently if kvm_zpci_set_airq() fails, kvm_s390_pci_aif_enable() returns
the error code but doesn't do any resource cleanup thus leaking resources.
Fix this by cleaning up all the resources such as the GAITE, AIBV, AISB and
unpinning any pinned pages.

Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
---
 arch/s390/kvm/pci.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
index 231a4236fc3c..d76b2c5484ac 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -341,19 +341,32 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
 	aift->kzdev[zdev->aisb] = zdev->kzdev;
 	spin_unlock_irq(&aift->gait_lock);
 
-	/* Update guest FIB for re-issue */
-	fib->fmt0.aisbo = zdev->aisb & 63;
-	fib->fmt0.aisb = virt_to_phys(aift->sbv->vector) + (zdev->aisb / 64) * 8;
-	fib->fmt0.isc = gisc;
-
 	/* Save some guest fib values in the host for later use */
-	zdev->kzdev->fib.fmt0.isc = fib->fmt0.isc;
+	zdev->kzdev->fib.fmt0.isc = gisc;
 	zdev->kzdev->fib.fmt0.aibv = fib->fmt0.aibv;
-	mutex_unlock(&aift->aift_lock);
 
 	/* Issue the clp to setup the irq now */
 	rc = kvm_zpci_set_airq(zdev);
-	return rc;
+	if (!rc) {
+		mutex_unlock(&aift->aift_lock);
+		return rc;
+	}
+
+	/* Start cleanup */
+	zdev->kzdev->fib.fmt0.isc = 0;
+	zdev->kzdev->fib.fmt0.aibv = 0;
+
+	spin_lock_irq(&aift->gait_lock);
+	gaite->count--;
+	gaite->aisb = 0;
+	gaite->gisc = 0;
+	gaite->aisbo = 0;
+	gaite->gisa = 0;
+	aift->kzdev[zdev->aisb] = NULL;
+	spin_unlock_irq(&aift->gait_lock);
+
+	airq_iv_release(zdev->aibv);
+	zdev->aibv = NULL;
 
 free_aisb:
 	airq_iv_free_bit(aift->sbv, zdev->aisb);
-- 
2.43.0

Re: [PATCH v4 5/6] KVM: s390: pci: Fix resource leak on IRQ registration failure

[sashiko-bot]

> Currently if kvm_zpci_set_airq() fails, kvm_s390_pci_aif_enable() returns
> the error code but doesn't do any resource cleanup thus leaking resources.
> Fix this by cleaning up all the resources such as the GAITE, AIBV, AISB and
> unpinning any pinned pages.
> 
> Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

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

Re: [PATCH v4 5/6] KVM: s390: pci: Fix resource leak on IRQ registration failure

[Matthew Rosato]

On 7/22/26 1:06 PM, Farhan Ali wrote:
> Currently if kvm_zpci_set_airq() fails, kvm_s390_pci_aif_enable() returns
> the error code but doesn't do any resource cleanup thus leaking resources.

Nits: s/the/an/  ....  and 'resource cleanup, thus leaking' (add a comma)

> Fix this by cleaning up all the resources such as the GAITE, AIBV, AISB and
> unpinning any pinned pages.
> 
> Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
> ---
>  arch/s390/kvm/pci.c | 29 +++++++++++++++++++++--------
>  1 file changed, 21 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index 231a4236fc3c..d76b2c5484ac 100644
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c
> @@ -341,19 +341,32 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
>  	aift->kzdev[zdev->aisb] = zdev->kzdev;
>  	spin_unlock_irq(&aift->gait_lock);
>  
> -	/* Update guest FIB for re-issue */
> -	fib->fmt0.aisbo = zdev->aisb & 63;
> -	fib->fmt0.aisb = virt_to_phys(aift->sbv->vector) + (zdev->aisb / 64) * 8;
> -	fib->fmt0.isc = gisc;
> -

OK, I had the same problem as Christian here.

This is dead code because we don't re-use the fib, we make a new one in
kvm_zpci_set_airq() and fill it with values from the kzdev.  Right?

I agree adding something to the commit message to that effect would be
good, even something like 'While at it, remove dead code that stored FIB
values that were never referenced.'

>  	/* Save some guest fib values in the host for later use */
> -	zdev->kzdev->fib.fmt0.isc = fib->fmt0.isc;
> +	zdev->kzdev->fib.fmt0.isc = gisc;
>  	zdev->kzdev->fib.fmt0.aibv = fib->fmt0.aibv;
> -	mutex_unlock(&aift->aift_lock);
>  
>  	/* Issue the clp to setup the irq now */
>  	rc = kvm_zpci_set_airq(zdev);
> -	return rc;
> +	if (!rc) {
> +		mutex_unlock(&aift->aift_lock);

This is subtle; we are now holding the aift_lock a bit longer now (over
the MPCIFC re-issue) which I don't think is strictly necessary since we
aren't messing with any of the zpci_aift fields during that timeframe,
but I think is fine to do.  Maybe also worth a mention in the commit
message that the lock is held a bit longer in order to handle the error
case vs drop/re-acquire.

For the code itself:

Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>

> +		return rc;
> +	}
> +
> +	/* Start cleanup */
> +	zdev->kzdev->fib.fmt0.isc = 0;
> +	zdev->kzdev->fib.fmt0.aibv = 0;
> +
> +	spin_lock_irq(&aift->gait_lock);
> +	gaite->count--;
> +	gaite->aisb = 0;
> +	gaite->gisc = 0;
> +	gaite->aisbo = 0;
> +	gaite->gisa = 0;
> +	aift->kzdev[zdev->aisb] = NULL;
> +	spin_unlock_irq(&aift->gait_lock);
> +
> +	airq_iv_release(zdev->aibv);
> +	zdev->aibv = NULL;
>  
>  free_aisb:
>  	airq_iv_free_bit(aift->sbv, zdev->aisb);

Re: [PATCH v4 5/6] KVM: s390: pci: Fix resource leak on IRQ registration failure

[Farhan Ali]

On 7/23/2026 8:19 AM, Matthew Rosato wrote:
> On 7/22/26 1:06 PM, Farhan Ali wrote:
>> Currently if kvm_zpci_set_airq() fails, kvm_s390_pci_aif_enable() returns
>> the error code but doesn't do any resource cleanup thus leaking resources.
> Nits: s/the/an/  ....  and 'resource cleanup, thus leaking' (add a comma)
>
>> Fix this by cleaning up all the resources such as the GAITE, AIBV, AISB and
>> unpinning any pinned pages.
>>
>> Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
>> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
>> ---
>>   arch/s390/kvm/pci.c | 29 +++++++++++++++++++++--------
>>   1 file changed, 21 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
>> index 231a4236fc3c..d76b2c5484ac 100644
>> --- a/arch/s390/kvm/pci.c
>> +++ b/arch/s390/kvm/pci.c
>> @@ -341,19 +341,32 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
>>   	aift->kzdev[zdev->aisb] = zdev->kzdev;
>>   	spin_unlock_irq(&aift->gait_lock);
>>   
>> -	/* Update guest FIB for re-issue */
>> -	fib->fmt0.aisbo = zdev->aisb & 63;
>> -	fib->fmt0.aisb = virt_to_phys(aift->sbv->vector) + (zdev->aisb / 64) * 8;
>> -	fib->fmt0.isc = gisc;
>> -
> OK, I had the same problem as Christian here.
>
> This is dead code because we don't re-use the fib, we make a new one in
> kvm_zpci_set_airq() and fill it with values from the kzdev.  Right?

Yes, this becomes unnecessary and actually overwriting the fib->fmt0.isc 
causes a reference count leak in the cleanup path as we use 
fib->fmt0.isc when calling kvm_s390_gisc_unregister().


>
> I agree adding something to the commit message to that effect would be
> good, even something like 'While at it, remove dead code that stored FIB
> values that were never referenced.'

Ack, will add that.


>
>>   	/* Save some guest fib values in the host for later use */
>> -	zdev->kzdev->fib.fmt0.isc = fib->fmt0.isc;
>> +	zdev->kzdev->fib.fmt0.isc = gisc;
>>   	zdev->kzdev->fib.fmt0.aibv = fib->fmt0.aibv;
>> -	mutex_unlock(&aift->aift_lock);
>>   
>>   	/* Issue the clp to setup the irq now */
>>   	rc = kvm_zpci_set_airq(zdev);
>> -	return rc;
>> +	if (!rc) {
>> +		mutex_unlock(&aift->aift_lock);
> This is subtle; we are now holding the aift_lock a bit longer now (over
> the MPCIFC re-issue) which I don't think is strictly necessary since we
> aren't messing with any of the zpci_aift fields during that timeframe,
> but I think is fine to do.  Maybe also worth a mention in the commit
> message that the lock is held a bit longer in order to handle the error
> case vs drop/re-acquire.

Yeah, will do. I agree, that its not strictly necessary just makes the 
code a little less messier having to drop and re-acquire. FWIW we also 
acquire the lock before doing a kvm_zpci_clear_airq() in the disable path.

Thanks

Farhan

>
> For the code itself:
>
> Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
>
>> +		return rc;
>> +	}
>> +
>> +	/* Start cleanup */
>> +	zdev->kzdev->fib.fmt0.isc = 0;
>> +	zdev->kzdev->fib.fmt0.aibv = 0;
>> +
>> +	spin_lock_irq(&aift->gait_lock);
>> +	gaite->count--;
>> +	gaite->aisb = 0;
>> +	gaite->gisc = 0;
>> +	gaite->aisbo = 0;
>> +	gaite->gisa = 0;
>> +	aift->kzdev[zdev->aisb] = NULL;
>> +	spin_unlock_irq(&aift->gait_lock);
>> +
>> +	airq_iv_release(zdev->aibv);
>> +	zdev->aibv = NULL;
>>   
>>   free_aisb:
>>   	airq_iv_free_bit(aift->sbv, zdev->aisb);

[PATCH v4 6/6] KVM: s390: pci: Validate AIBV and AISB before pinning guest pages

[Farhan Ali]

The AIBV holds one bit per MSI-X vector for a given function. The size of
the bit vector is derived from the NOI and the AIBVO. If the size of the
AIBV exceeds a single page boundary, then reject the request as we cannot
safely pin the guest AIBV.

Similarly reject the request if the AISB address is not 8-byte aligned as
the architecture requires doubleword alignment for the summary bit address.
Since the AISBO can address up to 64 bits, the size of the AISB can only be
8 bytes for the function. This also ensures the AISB doesn't exceed a
single page boundary.

Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
---
 arch/s390/kvm/pci.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
index d76b2c5484ac..e55e75b81b51 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -241,7 +241,7 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
 				   bool assist)
 {
 	struct page *pages[1], *aibv_page, *aisb_page = NULL;
-	unsigned int msi_vecs, idx;
+	unsigned int msi_vecs, idx, size;
 	struct zpci_gaite *gaite;
 	unsigned long hva, bit;
 	struct kvm *kvm;
@@ -268,6 +268,14 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
 		return gisc;
 
 	/* Replace AIBV address */
+	size = BITS_TO_LONGS(msi_vecs) * sizeof(unsigned long);
+	size += (fib->fmt0.aibvo / 8) + 1;
+	npages = DIV_ROUND_UP((fib->fmt0.aibv & ~PAGE_MASK) + size, PAGE_SIZE);
+	if (npages > 1) {
+		rc = -EINVAL;
+		goto out;
+	}
+
 	idx = srcu_read_lock(&kvm->srcu);
 	hva = gfn_to_hva(kvm, gpa_to_gfn((gpa_t)fib->fmt0.aibv));
 	npages = pin_user_pages_fast(hva, 1, FOLL_WRITE | FOLL_LONGTERM, pages);
@@ -283,6 +291,12 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
 
 	/* Pin the guest AISB if one was specified */
 	if (fib->fmt0.sum == 1) {
+
+		if (fib->fmt0.aisb & 0x7) {
+			rc = -EINVAL;
+			goto unpin1;
+		}
+
 		idx = srcu_read_lock(&kvm->srcu);
 		hva = gfn_to_hva(kvm, gpa_to_gfn((gpa_t)fib->fmt0.aisb));
 		npages = pin_user_pages_fast(hva, 1, FOLL_WRITE | FOLL_LONGTERM,
-- 
2.43.0

Re: [PATCH v4 6/6] KVM: s390: pci: Validate AIBV and AISB before pinning guest pages

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The AIBV size calculation systematically overestimates the required memory size, leading to false rejection of valid AIBV configurations located near page boundaries.
--

--- Patch [6]: [PATCH v4 6/6] KVM: s390: pci: Validate AIBV and AISB before pinning guest pages ---
commit 4b6ef516a235f953d1559a623689c91ffb836199
Author: Farhan Ali <alifm@linux.ibm.com>

KVM: s390: pci: Validate AIBV and AISB before pinning guest pages

This commit adds validation checks for the guest-provided AIBV and AISB 
addresses before pinning them. It ensures the AIBV does not exceed a single 
page boundary and that the AISB is aligned properly to 8 bytes.

> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index d76b2c5484aca..e55e75b81b516 100644
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c

[ ... ]

> @@ -268,6 +268,14 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
>  		return gisc;
>  
>  	/* Replace AIBV address */
> +	size = BITS_TO_LONGS(msi_vecs) * sizeof(unsigned long);
> +	size += (fib->fmt0.aibvo / 8) + 1;
> +	npages = DIV_ROUND_UP((fib->fmt0.aibv & ~PAGE_MASK) + size, PAGE_SIZE);
> +	if (npages > 1) {
> +		rc = -EINVAL;
> +		goto out;
> +	}
> +

[Severity: High]
Does this size calculation artificially inflate the required AIBV memory space 
and falsely reject valid guest configurations?

If a guest places a 1-MSI vector AIBV near the end of a physical page, such 
as at page offset 4088, the size formula calculates at least 9 bytes (8 
bytes from BITS_TO_LONGS(1) plus 1 arbitrary byte). 

This inflated size (4088 + 9 = 4097) crosses the page boundary, causing KVM 
to mistakenly reject a perfectly valid, 8-byte aligned memory placement with 
-EINVAL. Can this formula be adjusted to calculate the exact size required?

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

Re: [PATCH v4 6/6] KVM: s390: pci: Validate AIBV and AISB before pinning guest pages

[Farhan Ali]

On 7/23/2026 4:57 AM, Christian Borntraeger wrote:
>
>
> Am 22.07.26 um 19:26 schrieb sashiko-bot@kernel.org:
>> oes this size calculation artificially inflate the required AIBV 
>> memory space
>> and falsely reject valid guest configurations?
>>
>> If a guest places a 1-MSI vector AIBV near the end of a physical 
>> page, such
>> as at page offset 4088, the size formula calculates at least 9 bytes (8
>> bytes from BITS_TO_LONGS(1) plus 1 arbitrary byte).
>
> Farhan, I think this is indeed a problem.
> Would
> -    size += (fib->fmt0.aibvo / 8) + 1; │
> +    size += (fib->fmt0.aibvo / 8);
>
> be the correct solution? BITS_TO_LONGS already rounds up I think. 

Yes, I was looking at it already and sashiko is right here, was waiting 
for some more feedback before sending a new revision. And yeah we should 
just remove the extra + 1. Will fix it in v5.


Thanks

Farhan

Thanks

Re: [PATCH v4 6/6] KVM: s390: pci: Validate AIBV and AISB before pinning guest pages

[Matthew Rosato]

On 7/22/26 1:06 PM, Farhan Ali wrote:
> The AIBV holds one bit per MSI-X vector for a given function. The size of
> the bit vector is derived from the NOI and the AIBVO. If the size of the
> AIBV exceeds a single page boundary, then reject the request as we cannot
> safely pin the guest AIBV.
> 
> Similarly reject the request if the AISB address is not 8-byte aligned as
> the architecture requires doubleword alignment for the summary bit address.
> Since the AISBO can address up to 64 bits, the size of the AISB can only be
> 8 bytes for the function. This also ensures the AISB doesn't exceed a
> single page boundary.
> 
> Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
> ---
>  arch/s390/kvm/pci.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index d76b2c5484ac..e55e75b81b51 100644
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c
> @@ -241,7 +241,7 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
>  				   bool assist)
>  {
>  	struct page *pages[1], *aibv_page, *aisb_page = NULL;
> -	unsigned int msi_vecs, idx;
> +	unsigned int msi_vecs, idx, size;
>  	struct zpci_gaite *gaite;
>  	unsigned long hva, bit;
>  	struct kvm *kvm;
> @@ -268,6 +268,14 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
>  		return gisc;
>  
>  	/* Replace AIBV address */
> +	size = BITS_TO_LONGS(msi_vecs) * sizeof(unsigned long);
> +	size += (fib->fmt0.aibvo / 8) + 1;

Sashiko's comment here looks valid, remove the +1.

> +	npages = DIV_ROUND_UP((fib->fmt0.aibv & ~PAGE_MASK) + size, PAGE_SIZE);
> +	if (npages > 1) {
> +		rc = -EINVAL;
> +		goto out;
> +	}
> +

Nit: 1-liner comment above that the AIBV cannot span a page?

>  	idx = srcu_read_lock(&kvm->srcu);
>  	hva = gfn_to_hva(kvm, gpa_to_gfn((gpa_t)fib->fmt0.aibv));
>  	npages = pin_user_pages_fast(hva, 1, FOLL_WRITE | FOLL_LONGTERM, pages);
> @@ -283,6 +291,12 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
>  
>  	/* Pin the guest AISB if one was specified */
>  	if (fib->fmt0.sum == 1) {
> +

Nit: ^ unnecessary newline?

> +		if (fib->fmt0.aisb & 0x7) {
> +			rc = -EINVAL;
> +			goto unpin1;
> +		}
> +

Nit: 1-liner comment above that AISB must be dword-aligned?

With the +1 issue fixed and regardless if you take the nit suggestions
or not:

Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>

>  		idx = srcu_read_lock(&kvm->srcu);
>  		hva = gfn_to_hva(kvm, gpa_to_gfn((gpa_t)fib->fmt0.aisb));
>  		npages = pin_user_pages_fast(hva, 1, FOLL_WRITE | FOLL_LONGTERM,

Re: [PATCH v4 0/6] KVM s390x PCI fixes

[Matthew Rosato]

On 7/23/26 9:00 AM, Matthew Rosato wrote:
> 
>> I would take this series and do small fixups where necessary. Just tell
>> me if the change for patch 6 is fine. Or provide a v5.
>>
> 
> I am reviewing and testing this today, please give me a few more hours.
> 

Farhan, could you do a v5 with comments addressed?

I am willing to also tag patch 2 if you either make the change I asked
for there (or explain to me why it's not necessary).

I've also done a variety of testing, so I'd be happy to also give my
Tested-by to the series.  But would be good to do this on a v5.

>> I suggest to add cc stable to all patches.
> 
> Agree

Re: [PATCH v4 0/6] KVM s390x PCI fixes

[Farhan Ali]

On 7/23/2026 8:35 AM, Matthew Rosato wrote:
> On 7/23/26 9:00 AM, Matthew Rosato wrote:
>>> I would take this series and do small fixups where necessary. Just tell
>>> me if the change for patch 6 is fine. Or provide a v5.
>>>
>> I am reviewing and testing this today, please give me a few more hours.
>>
> Farhan, could you do a v5 with comments addressed?
>
> I am willing to also tag patch 2 if you either make the change I asked
> for there (or explain to me why it's not necessary).

Yes I can spin a v5, I think we have enough reviews to warrant a v5. 
Thanks all for reviewing.

Thanks

Farhan

>
> I've also done a variety of testing, so I'd be happy to also give my
> Tested-by to the series.  But would be good to do this on a v5.
>
>>> I suggest to add cc stable to all patches.
>> Agree