On Wed, Aug 26, 2026 at 2:47 PM Eugenio Perez Martin
<[email protected]> wrote:
>
> On Tue, Aug 18, 2026 at 11:15 PM Alexander Graf <[email protected]> wrote:
> >
> > Commit f7728002c1c7 ("virtio_ring: fix return code on DMA mapping
> > fails") moved virtqueue_add_split() and virtqueue_add_indirect_packed()
> > to -ENOMEM, because virtio_queue_rq() maps -EIO to BLK_STS_IOERR and
> > the request fails. We still return -EIO from virtqueue_add_packed(),
> > and virtqueue_add_packed_in_order() copied that when it was added later.
> >
> > Guests that bounce their I/O through swiotlb (SEV-SNP, TDX, s390 secure
> > execution) run the pool out with enough I/O in flight. On a split ring
> > virtio_queue_rq() reports BLK_STS_RESOURCE and the block layer requeues
> > the request. On a packed ring virtio_queue_rq() reports BLK_STS_IOERR
> > instead and the error reaches the filesystem.
> >
> > Return -ENOMEM from the packed unmap_release paths too. Both are reached
> > from a single goto on a failed mapping, which is where
> > vring_map_one_sg() already produces -ENOMEM.
> >
> > That way every ring layout reports the same errno, and the block layer
> > requeues the request instead of failing it.
> >
> > Fixes: f7728002c1c7 ("virtio_ring: fix return code on DMA mapping fails")
> > Fixes: f6a15d854986 ("virtio_ring: add in order support")
>
> Acked-by: Eugenio Pérez <[email protected]>
>

Even if I'd like to see this merged, I'm having second thoughts
because it introduces userland visible changes in some drivers. Are
them acceptable?

---
1. virtio_crypto (skcipher) — errno changes via AF_ALG socket

userspace: socket(AF_ALG, SOCK_SEQPACKET, 0)
           bind(fd, {ALG_SET_KEY, "cbc(aes)"}, ...)
           sendmsg(fd, plaintext, ...)     ← submits data
           recvmsg(fd, ciphertext, ...)    ← triggers encryption,
waits for result

The kernel path:
recvmsg()
  → skcipher_recvmsg()           [crypto/algif_skcipher.c:180]
      → _skcipher_recvmsg()      [crypto/algif_skcipher.c:84]
          → crypto_skcipher_encrypt(&areq->cra_u.skcipher_req)
              → virtio_crypto_skcipher_encrypt()
                  → crypto_transfer_skcipher_request_to_engine()
                  [enqueues req; engine kthread picks it up]
          → crypto_wait_req(err, &ctx->wait)
              waits on completion

In the crypto engine kthread:
crypto_pump_requests()           [crypto/crypto_engine.c:~100]
  → op->do_one_request(engine, async_req)
      = virtio_crypto_skcipher_crypt_req()  [virtio_crypto_skcipher_algs.c:530]
          → __virtio_crypto_skcipher_do_req()
              → virtqueue_add_sgs(data_vq->vq, ...)
                  → virtqueue_add_packed()   ← changed here

Error propagation — before commit:
virtqueue_add_packed()                returns -EIO
  __virtio_crypto_skcipher_do_req()   returns -EIO (err propagated
directly)
  virtio_crypto_skcipher_crypt_req()  returns -EIO
  crypto_engine.c:155:  crypto_request_complete(async_req, -EIO)
    → crypto_req_done(wait, -EIO)
    → wait->err = -EIO; complete(&wait->completion)
  crypto_wait_req() returns -EIO
  _skcipher_recvmsg() returns -EIO
  skcipher_recvmsg() returns -EIO
  userspace recvmsg() returns -1, errno = EIO (5)

Error propagation — after commit:
virtqueue_add_packed()                returns -ENOMEM
  ... same chain ...
  userspace recvmsg() returns -1, errno = ENOMEM (12)

---
2. spi-virtio — errno changes via spidev ioctl

A userspace SPI application using /dev/spidevX.Y:

userspace: ioctl(fd, SPI_IOC_MESSAGE(n), &xfer)
  → spidev_ioctl()           [drivers/spi/spidev.c]
      → spidev_message()
          → spi_sync(spi, &msg)
              → __spi_sync()       [drivers/spi/spi.c:4687]
                  → __spi_transfer_message_noqueue()
                      → spi_transfer_one_message()
[drivers/spi/spi.c:~1603]
                          → ctlr->transfer_one(ctlr, msg->spi, xfer)
                              = virtio_spi_transfer_one()
[drivers/spi/spi-virtio.c:148]
                                  → virtqueue_add_sgs(priv->vq, ...)
                                      → virtqueue_add_packed()   ←
changed here

Error propagation:

Inside virtio_spi_transfer_one() at line 251–253:
msg_done:
    if (ret)
        ctrl->cur_msg->status = ret;
    return ret;

In spi_transfer_one_message() at line 1637–1655:
ret = ctlr->transfer_one(ctlr, msg->spi, xfer);
if (ret < 0) {
    ...goto out;
}
// out: msg->status = ret  (line 1708)

Back in __spi_sync() at line 4723: return message->status;

The errno from virtqueue_add_packed() propagates all the way to the
ioctl() return value: before the commit, ioctl() returns -EIO; after,
-ENOMEM. Any spidev userspace code that checks errno would observe the
difference.
---

Claude also found another issue in gpio but the code has more
ramifications and I couldn't verify it:

3. gpio-virtio — errno changes via GPIO character device ioctl

A userspace GPIO application using the character device API (/dev/gpiochipX):

userspace: ioctl(fd, GPIO_V2_GET_LINE_VALUES_IOCTL, &data)
  → linereq_ioctl()          [drivers/gpio/gpiolib-cdev.c]
      → linereq_get_values()
          → gpiod_get_value_cansleep(line->desc)
              → gpio_chip_get_value()   [drivers/gpio/gpiolib.c:3412]
                  → gpiochip_get(gc, offset)  [drivers/gpio/gpiolib.c:3394]
                      → gc->get(gc, offset)
                          = virtio_gpio_get()   [drivers/gpio/gpio-virtio.c:194]
                              → virtio_gpio_req(vgpio, ...)
                                  → _virtio_gpio_req()
                                      →
virtqueue_add_sgs(vgpio->request_vq, ...)
                                          → virtqueue_add_packed()   ←
changed here

_virtio_gpio_req() at line 100–104:
ret = virtqueue_add_sgs(vgpio->request_vq, sgs, 1, 1, line, GFP_KERNEL);
if (ret) {
    ...
    goto out;
}
// out: return ret

Apart from that, Claude also detected a change in behavior worth
checking, regardless of whether this patch is applied.
drivers/scsi/virtio_scsi.c:virtscsi_queuecommand will complete the
command if the return of virtscsi_add_cmd is -EIO, but it will not do
it if ret is different than 0 and -EIO.

> > Assisted-by: Kiro:claude-opus-5 checkpatch sparse
> > Signed-off-by: Alexander Graf <[email protected]>
> > ---
> >  drivers/virtio/virtio_ring.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index ea8e774b6d8e..b7b03166a301 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -1810,7 +1810,7 @@ static inline int virtqueue_add_packed(struct 
> > vring_virtqueue *vq,
> >         }
> >
> >         END_USE(vq);
> > -       return -EIO;
> > +       return -ENOMEM;
> >  }
> >
> >  static inline int virtqueue_add_packed_in_order(struct vring_virtqueue *vq,
> > @@ -1966,7 +1966,7 @@ static inline int 
> > virtqueue_add_packed_in_order(struct vring_virtqueue *vq,
> >         }
> >
> >         END_USE(vq);
> > -       return -EIO;
> > +       return -ENOMEM;
> >  }
> >
> >  static bool virtqueue_kick_prepare_packed(struct vring_virtqueue *vq)
> >


Reply via email to