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@xxxxxxxxxxxxx>
> ---
>  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@xxxxxxxxxxxxx>

>  		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,