Re: [PATCH v2 3/6] virtio: 9p: Add 9P transport driver

[Kuan-Wei Chiu <visitorckw@gmail.com>]

Hi Simon,

On Fri, Sep 11, 2026 at 05:43:39AM -0600, Simon Glass wrote:
> Hi Kuan-Wei,
> 
> On 2026-08-29T19:21:13, Kuan-Wei Chiu <visitorckw@gmail.com> wrote:
> > virtio: 9p: Add 9P transport driver
> >
> > Add virtio transport driver for the 9P filesystem.
> >
> > This driver binds to the virtio-9p device exposed by the hypervisor and
> > registers as a DM UCLASS_9P device. It implements the transmit and
> > receive routines.
> >
> > Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
> >
> > drivers/virtio/Kconfig         |   7 +++
> >  drivers/virtio/Makefile        |   1 +
> >  drivers/virtio/virtio-uclass.c |   1 +
> >  drivers/virtio/virtio_9p.c     | 119 +++++++++++++++++++++++++++++++++++++++++
> >  include/virtio.h               |  12 ++++-
> >  5 files changed, 139 insertions(+), 1 deletion(-)
> 
> > diff --git a/drivers/virtio/virtio_9p.c b/drivers/virtio/virtio_9p.c
> > @@ -0,0 +1,119 @@
> > +static int virtio_9p_remove(struct udevice *dev)
> > +{
> > +     return virtio_reset(dev);
> > +}
> > +
> > +U_BOOT_DRIVER(virtio_9p) = {
> > +     .name       = VIRTIO_9P_DRV_NAME,
> > +     .id         = UCLASS_9P,
> > +     .ops        = &virtio_9p_ops,
> > +     .bind       = virtio_9p_bind,
> > +     .probe      = virtio_9p_probe,
> > +     .remove     = virtio_9p_remove,
> 
> Sorry, I gave you the wrong function name last time. Other virtio
> drivers set .remove = virtio_remove, which calls virtio_del_vqs()
> before virtio_reset(). Going straight to virtio_reset() here leaks the
> vq allocated in probe(). Please drop the wrapper and use virtio_remove
> directly.

I checked the tree and it seems virtio_remove() does not exist in
U-Boot. Existing drivers like virtio_blk, virtio_net, and virtio_rng
all set .remove = virtio_reset directly. To avoid leaking the vq on
removal, I will keep virtio_9p_remove() and call virtio_del_vqs(dev)
before virtio_reset(dev) in v3.

> 
> > diff --git a/drivers/virtio/virtio_9p.c b/drivers/virtio/virtio_9p.c
> > @@ -0,0 +1,119 @@
> > +     start = get_timer(0);
> > +     while (!virtqueue_get_buf(priv->vq, &len)) {
> > +             if (get_timer(start) > VIRTIO_9P_TIMEOUT_MS)
> > +                     return -ETIMEDOUT;
> > +     }
> > +
> > +     if (len < (int)sizeof(struct p9_header))
> > +             return -EIO;
> > +
> > +     return 0;
> > +}
> 
> Looks good, It might be worth returning len to the caller so the
> client can cross-check it against the size field in the reply header,
> but up to you.
> 
Ack. Will do in v3.

Regards,
Kuan-Wei