Re: [PATCH v1 03/11] KVM: s390: Fix get_all_floating_irqs()
From: Claudio Imbrenda
Date: Wed Aug 12 2026 - 05:20:24 EST
On Wed, 12 Aug 2026 09:11:11 +0200
Christian Borntraeger <borntraeger@xxxxxxxxxx> 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@xxxxxxxxxxxxx>
>
> 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)
>