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 <[email protected]> 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 <[email protected]>
> >
> > 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
