Re: [PATCH 4/5] usb: allow max 8192 bytes for desc
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 >>> --- >>> 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é
Re: [PATCH 4/5] usb: allow max 8192 bytes for desc
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 > --- > hw/usb/desc.c | 15 --- > hw/usb/desc.h | 1 + > 2 files changed, 9 insertions(+), 7 deletions(-) Reviewed-by: Daniel P. Berrangé 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 :|
Re: Re: [PATCH 4/5] usb: allow max 8192 bytes for desc
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
---
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
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
---
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_LEN8192
#define USB_DESC_FLAG_SUPER (1 << 1)
/* little helpers */
[PATCH 4/5] usb: allow max 8192 bytes for desc
A device of USB video class usually uses larger desc structure, so
use larger buffer to avoid failure.
Signed-off-by: zhenwei pi
---
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_LEN8192
#define USB_DESC_FLAG_SUPER (1 << 1)
/* little helpers */
--
2.25.1
