Re: [PATCH] KVM: s390: pci: Fix aisb calculation

[Niklas Schnelle <schnelle@linux.ibm.com>]

On Fri, 2026-04-24 at 15:39 -0400, Matthew Rosato wrote:
> The current implementation of aisb calculation will erroneously index
> via an unsigned long * as well as multiply by 8B for every 64-bits in
> the offset; only one or the other is required.  This throws off aisb
> calculations once the number of devices exceeds 64, and can result
> in out-of-bounds access as well as failure to indicate summary bits
> associated with those devices in guests.
> 
> Fix this by converting to a physical address before applying the
> offset, as is already done in arch/s390/pci/pci_irq.c.
> 
> Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
> ---
>  arch/s390/kvm/pci.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index 86d93e8dddae..338171f9371b 100644
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c
> @@ -166,7 +166,7 @@ static int kvm_zpci_set_airq(struct zpci_dev *zdev)
>  	fib.fmt0.noi = airq_iv_end(zdev->aibv);
>  	fib.fmt0.aibv = virt_to_phys(zdev->aibv->vector);
>  	fib.fmt0.aibvo = 0;
> -	fib.fmt0.aisb = virt_to_phys(aift->sbv->vector + (zdev->aisb / 64) * 8);
> +	fib.fmt0.aisb = virt_to_phys(aift->sbv->vector) + (zdev->aisb / 64) * 8;

As you state, one or the other would work, wouldn't it be easy to read
like below:

         fib.fmt0.aisb = virt_to_phys(aift->sbv->vector + (zdev->aisb / 64));

>  	fib.fmt0.aisbo = zdev->aisb & 63;
>  	fib.gd = zdev->gisa;
>  
> @@ -309,7 +309,7 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
>  
>  	/* 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.aisb = virt_to_phys(aift->sbv->vector) + (zdev->aisb / 64) * 8;

Same argument as above.

>  	fib->fmt0.isc = gisc;
>  
>  	/* Save some guest fib values in the host for later use */

Ouch, good catch! Was this kind of a follow up to the issue someone
found upstream with a wrong calculation in the gait? Or did you just
notice things break with more than 64 devices?

Thanks,
Niklas