1. Patchew
  2. QEMU
  3. [PATCH 0/5] Introduce camera subsystem and USB video device
  4. View patch

[PATCH 4/5] usb: allow max 8192 bytes for desc

zhenwei pi posted 5 patches 4 years, 1 month ago
Maintainers: Markus Armbruster <armbru@redhat.com>, Michael Roth <michael.roth@amd.com>, Eric Blake <eblake@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>
[PATCH 4/5] usb: allow max 8192 bytes for desc
Posted by zhenwei pi 4 years, 1 month ago 
A device of USB video class usually uses larger desc structure, so
use larger buffer to avoid failure.

Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
 hw/usb/desc.c | 15 ++++++++-------
 hw/usb/desc.h |  1 +
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/hw/usb/desc.c b/hw/usb/desc.c
index 8b6eaea407..7f6cc2f99b 100644
--- a/hw/usb/desc.c
+++ b/hw/usb/desc.c
@@ -632,7 +632,8 @@ int usb_desc_get_descriptor(USBDevice *dev, USBPacket *p,
     bool msos = (dev->flags & (1 << USB_DEV_FLAG_MSOS_DESC_IN_USE));
     const USBDesc *desc = usb_device_get_usb_desc(dev);
     const USBDescDevice *other_dev;
-    uint8_t buf[256];
+    size_t buflen = USB_DESC_MAX_LEN;
+    g_autofree uint8_t *buf = g_malloc(buflen);
     uint8_t type = value >> 8;
     uint8_t index = value & 0xff;
     int flags, ret = -1;
@@ -650,36 +651,36 @@ int usb_desc_get_descriptor(USBDevice *dev, USBPacket *p,
 
     switch(type) {
     case USB_DT_DEVICE:
-        ret = usb_desc_device(&desc->id, dev->device, msos, buf, sizeof(buf));
+        ret = usb_desc_device(&desc->id, dev->device, msos, buf, buflen);
         trace_usb_desc_device(dev->addr, len, ret);
         break;
     case USB_DT_CONFIG:
         if (index < dev->device->bNumConfigurations) {
             ret = usb_desc_config(dev->device->confs + index, flags,
-                                  buf, sizeof(buf));
+                                  buf, buflen);
         }
         trace_usb_desc_config(dev->addr, index, len, ret);
         break;
     case USB_DT_STRING:
-        ret = usb_desc_string(dev, index, buf, sizeof(buf));
+        ret = usb_desc_string(dev, index, buf, buflen);
         trace_usb_desc_string(dev->addr, index, len, ret);
         break;
     case USB_DT_DEVICE_QUALIFIER:
         if (other_dev != NULL) {
-            ret = usb_desc_device_qualifier(other_dev, buf, sizeof(buf));
+            ret = usb_desc_device_qualifier(other_dev, buf, buflen);
         }
         trace_usb_desc_device_qualifier(dev->addr, len, ret);
         break;
     case USB_DT_OTHER_SPEED_CONFIG:
         if (other_dev != NULL && index < other_dev->bNumConfigurations) {
             ret = usb_desc_config(other_dev->confs + index, flags,
-                                  buf, sizeof(buf));
+                                  buf, buflen);
             buf[0x01] = USB_DT_OTHER_SPEED_CONFIG;
         }
         trace_usb_desc_other_speed_config(dev->addr, index, len, ret);
         break;
     case USB_DT_BOS:
-        ret = usb_desc_bos(desc, buf, sizeof(buf));
+        ret = usb_desc_bos(desc, buf, buflen);
         trace_usb_desc_bos(dev->addr, len, ret);
         break;
 
diff --git a/hw/usb/desc.h b/hw/usb/desc.h
index 3ac604ecfa..35babdeff6 100644
--- a/hw/usb/desc.h
+++ b/hw/usb/desc.h
@@ -199,6 +199,7 @@ struct USBDesc {
     const USBDescMSOS         *msos;
 };
 
+#define USB_DESC_MAX_LEN    8192
 #define USB_DESC_FLAG_SUPER (1 << 1)
 
 /* little helpers */
-- 
2.25.1
Re: [PATCH 4/5] usb: allow max 8192 bytes for desc
Posted by Philippe Mathieu-Daudé 4 years, 1 month ago 
On 27/12/21 15:27, zhenwei pi wrote:
> A device of USB video class usually uses larger desc structure, so
> use larger buffer to avoid failure.
> 
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> ---
>   hw/usb/desc.c | 15 ++++++++-------
>   hw/usb/desc.h |  1 +
>   2 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/usb/desc.c b/hw/usb/desc.c
> index 8b6eaea407..7f6cc2f99b 100644
> --- a/hw/usb/desc.c
> +++ b/hw/usb/desc.c
> @@ -632,7 +632,8 @@ int usb_desc_get_descriptor(USBDevice *dev, USBPacket *p,
>       bool msos = (dev->flags & (1 << USB_DEV_FLAG_MSOS_DESC_IN_USE));
>       const USBDesc *desc = usb_device_get_usb_desc(dev);
>       const USBDescDevice *other_dev;
> -    uint8_t buf[256];
> +    size_t buflen = USB_DESC_MAX_LEN;
> +    g_autofree uint8_t *buf = g_malloc(buflen);

Do we want to have a per-device desc_size (in USBDevice, default to
256, video devices set it to 8K)?

How "hot" is this path? Could we keep 8K on the stack?

> diff --git a/hw/usb/desc.h b/hw/usb/desc.h
> index 3ac604ecfa..35babdeff6 100644
> --- a/hw/usb/desc.h
> +++ b/hw/usb/desc.h
> @@ -199,6 +199,7 @@ struct USBDesc {
>       const USBDescMSOS         *msos;
>   };
>   
> +#define USB_DESC_MAX_LEN    8192
>   #define USB_DESC_FLAG_SUPER (1 << 1)
>   
>   /* little helpers */
Re: Re: [PATCH 4/5] usb: allow max 8192 bytes for desc
Posted by zhenwei pi 4 years, 1 month ago 
On 1/4/22 11:22 PM, Philippe Mathieu-Daudé wrote:
> On 27/12/21 15:27, zhenwei pi wrote:
>> A device of USB video class usually uses larger desc structure, so
>> use larger buffer to avoid failure.
>>
>> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
>> ---
>>   hw/usb/desc.c | 15 ++++++++-------
>>   hw/usb/desc.h |  1 +
>>   2 files changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/usb/desc.c b/hw/usb/desc.c
>> index 8b6eaea407..7f6cc2f99b 100644
>> --- a/hw/usb/desc.c
>> +++ b/hw/usb/desc.c
>> @@ -632,7 +632,8 @@ int usb_desc_get_descriptor(USBDevice *dev, 
>> USBPacket *p,
>>       bool msos = (dev->flags & (1 << USB_DEV_FLAG_MSOS_DESC_IN_USE));
>>       const USBDesc *desc = usb_device_get_usb_desc(dev);
>>       const USBDescDevice *other_dev;
>> -    uint8_t buf[256];
>> +    size_t buflen = USB_DESC_MAX_LEN;
>> +    g_autofree uint8_t *buf = g_malloc(buflen);
> 
> Do we want to have a per-device desc_size (in USBDevice, default to
> 256, video devices set it to 8K)?
> 
> How "hot" is this path? Could we keep 8K on the stack?
> 
It's an unlikely code path:
1, During guest startup, guest tries to probe device.
2, run 'lsusb' command in guest

Keeping 8K on the stack also seems OK.

>> diff --git a/hw/usb/desc.h b/hw/usb/desc.h
>> index 3ac604ecfa..35babdeff6 100644
>> --- a/hw/usb/desc.h
>> +++ b/hw/usb/desc.h
>> @@ -199,6 +199,7 @@ struct USBDesc {
>>       const USBDescMSOS         *msos;
>>   };
>> +#define USB_DESC_MAX_LEN    8192
>>   #define USB_DESC_FLAG_SUPER (1 << 1)
>>   /* little helpers */
> 

-- 
zhenwei pi
Re: [PATCH 4/5] usb: allow max 8192 bytes for desc
Posted by Philippe Mathieu-Daudé 4 years ago 
On 1/5/22 08:25, zhenwei pi wrote:
> 
> On 1/4/22 11:22 PM, Philippe Mathieu-Daudé wrote:
>> On 27/12/21 15:27, zhenwei pi wrote:
>>> A device of USB video class usually uses larger desc structure, so
>>> use larger buffer to avoid failure.
>>>
>>> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
>>> ---
>>>   hw/usb/desc.c | 15 ++++++++-------
>>>   hw/usb/desc.h |  1 +
>>>   2 files changed, 9 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/hw/usb/desc.c b/hw/usb/desc.c
>>> index 8b6eaea407..7f6cc2f99b 100644
>>> --- a/hw/usb/desc.c
>>> +++ b/hw/usb/desc.c
>>> @@ -632,7 +632,8 @@ int usb_desc_get_descriptor(USBDevice *dev,
>>> USBPacket *p,
>>>       bool msos = (dev->flags & (1 << USB_DEV_FLAG_MSOS_DESC_IN_USE));
>>>       const USBDesc *desc = usb_device_get_usb_desc(dev);
>>>       const USBDescDevice *other_dev;
>>> -    uint8_t buf[256];
>>> +    size_t buflen = USB_DESC_MAX_LEN;
>>> +    g_autofree uint8_t *buf = g_malloc(buflen);
>>
>> Do we want to have a per-device desc_size (in USBDevice, default to
>> 256, video devices set it to 8K)?
>>
>> How "hot" is this path? Could we keep 8K on the stack?
>>
> It's an unlikely code path:
> 1, During guest startup, guest tries to probe device.
> 2, run 'lsusb' command in guest

If you have to respin, do you mind adding this 3 lines
in the description? Anyhow:

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Re: [PATCH 4/5] usb: allow max 8192 bytes for desc
Posted by Daniel P. Berrangé 4 years ago 
On Mon, Dec 27, 2021 at 10:27:33PM +0800, zhenwei pi wrote:
> A device of USB video class usually uses larger desc structure, so
> use larger buffer to avoid failure.
> 
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> ---
>  hw/usb/desc.c | 15 ++++++++-------
>  hw/usb/desc.h |  1 +
>  2 files changed, 9 insertions(+), 7 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.