RE: [PATCH v8 13/18] hw/usb/redirect-server: Announce the exported device

[Jamin Lin <jamin_lin@xxxxxxxxxxxxxx>]

Hi Marc-André Lureau

> Subject: Re: [PATCH v8 13/18] hw/usb/redirect-server: Announce the exported
> device
> 
> > Send device_connect when a device is attached and the host has greeted
> > us, device_disconnect when it goes away, and handle the bus reset the
> > host sends before enumerating.
> >
> > device_connect carries no identity. The peer asks the device itself
> > with GET_DESCRIPTOR as soon as it enumerates, so anything we put there
> > would be made up. The speed is real: it comes from the device on our port.
> >
> > Signed-off-by: Jamin Lin <jamin_lin@xxxxxxxxxxxxxx>
> > Message-ID: <20261102092201.1729245-14-jamin_lin@xxxxxxxxxxxxxx>
> > ---
> >  hw/usb/redirect-server.c | 150
> ++++++++++++++++++++++++++++++++++++++-
> >  hw/usb/trace-events      |   7 ++
> >  2 files changed, 156 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/usb/redirect-server.c b/hw/usb/redirect-server.c index
> > f78351bde919..e207f5a7eb56 100644
> > --- a/hw/usb/redirect-server.c
> > +++ b/hw/usb/redirect-server.c
> > @@ -56,6 +56,7 @@
> >  #include "qemu/main-loop.h"
> >  #include "qemu/module.h"
> >  #include "migration/vmstate.h"
> > +#include "qemu/cutils.h"
> >  #include "hw/core/sysbus.h"
> >  #include "hw/usb/usb.h"
> >  #include "chardev/char-fe.h"
> > @@ -85,17 +86,105 @@ struct USBRedirServer {
> >      /* usbredir over the chardev */
> >      struct usbredirparser *parser;
> >      QEMUBH *chardev_close_bh;
> > +    QEMUBH *announce_bh;
> >      const uint8_t *read_buf;
> >      int read_buf_size;
> >      bool in_write;
> >      guint watch;
> >      bool host_connected;
> > +    bool device_announced;
> >  };
> >
> > +/*
> > + * The device is whatever USBDevice the user plugged into our port
> > +with
> > + * "-device <device>,bus=<id>.0". NULL until then.
> > + */
> > +static USBDevice *usbredir_server_device(USBRedirServer *s) {
> > +    return s->port.dev;
> > +}
> > +
> > +/*
> > + * Device announcement
> > + */
> > +
> > +static uint8_t usbredir_server_speed(USBDevice *device) {
> > +    switch (device->speed) {
> > +    case USB_SPEED_LOW:
> > +        return usb_redir_speed_low;
> > +    case USB_SPEED_FULL:
> > +        return usb_redir_speed_full;
> > +    default:
> > +        return usb_redir_speed_high;
> > +    }
> > +}
> > +
> > +static void usbredir_server_announce_device(void *opaque) {
> > +    USBRedirServer *s = opaque;
> > +    USBDevice *device = usbredir_server_device(s);
> > +    struct usb_redir_interface_info_header iface_info = {
> > +        .interface_count = 0,
> > +    };
> > +    struct usb_redir_device_connect_header conn = {};
> > +
> > +    if (!s->host_connected || s->device_announced ||
> > +        !device || !device->attached) {
> > +        return;
> > +    }
> > +    s->device_announced = true;
> > +    conn.speed = usbredir_server_speed(device);
> > +
> > +    /*
> > +     * The device has to be in DEFAULT state before it can answer
> > +     * anything, so reset it here.
> > +     */
> > +    usb_device_reset(device);
> > +
> > +    /* Send this before device_connect. The peer needs it to accept us. */
> > +    usbredirparser_send_interface_info(s->parser, &iface_info);
> 
> The peer's usb-redir client evaluates its "filter" property against
> device_connect and this interface_info. With interface_count == 0 and the
> all-zero connect header above, any non-empty filter (vendor/product, class or
> interface rule) rejects the exported device. Fill interface_info from the 
> saved
> configuration descriptor (interface number, class/subclass/ protocol) and
> device_connect from the local USBDevice?
> 

Thanks for the review and suggestion.

I will rework the series to read the descriptors from the device instead of 
hard code 0. The host has no device
until device_connect arrives, so it cannot ask for anything itself. The server 
read the descriptors in the 
announce path before it sends device_connect.

I will add a patch for this and the patch order will be changed.

Thanks,
Jamin
> --
> Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>