| Message ID | 20250411080431.207579-3-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | usb/msd: Permit relaxed ordering of IN packets | expand |
On 11/4/25 10:04, Nicholas Piggin wrote: > These structures are hardware interfaces, ensure the layout is > correct. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > hw/usb/dev-storage.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
On 11/4/25 10:04, Nicholas Piggin wrote: > These structures are hardware interfaces, ensure the layout is > correct. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > hw/usb/dev-storage.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c > index 2d7306b0572..87c22476f6b 100644 > --- a/hw/usb/dev-storage.c > +++ b/hw/usb/dev-storage.c > @@ -27,7 +27,7 @@ > #define MassStorageReset 0xff > #define GetMaxLun 0xfe > > -struct usb_msd_cbw { > +struct QEMU_PACKED usb_msd_cbw { > uint32_t sig; > uint32_t tag; > uint32_t data_len; > @@ -636,6 +636,9 @@ static const TypeInfo usb_storage_dev_type_info = { > > static void usb_msd_register_types(void) > { > + qemu_build_assert(sizeof(struct usb_msd_cbw) == 31); > + qemu_build_assert(sizeof(struct usb_msd_csw) == 13); Can we add definitions for these 13/31 magic values? Then we can use them in try_get_valid_cbw(). > + > type_register_static(&usb_storage_dev_type_info); > } >
On 11/4/25 12:21, Philippe Mathieu-Daudé wrote: > On 11/4/25 10:04, Nicholas Piggin wrote: >> These structures are hardware interfaces, ensure the layout is >> correct. >> >> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> >> --- >> hw/usb/dev-storage.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c >> index 2d7306b0572..87c22476f6b 100644 >> --- a/hw/usb/dev-storage.c >> +++ b/hw/usb/dev-storage.c >> @@ -27,7 +27,7 @@ >> #define MassStorageReset 0xff >> #define GetMaxLun 0xfe >> -struct usb_msd_cbw { >> +struct QEMU_PACKED usb_msd_cbw { >> uint32_t sig; >> uint32_t tag; >> uint32_t data_len; >> @@ -636,6 +636,9 @@ static const TypeInfo usb_storage_dev_type_info = { >> static void usb_msd_register_types(void) >> { >> + qemu_build_assert(sizeof(struct usb_msd_cbw) == 31); >> + qemu_build_assert(sizeof(struct usb_msd_csw) == 13); > > Can we add definitions for these 13/31 magic values? Then > we can use them in try_get_valid_cbw(). Maybe USB_MSD_CBW/CSW_MIN_SIZE? > >> + >> type_register_static(&usb_storage_dev_type_info); >> } >
On Fri Apr 11, 2025 at 8:21 PM AEST, Philippe Mathieu-Daudé wrote: > On 11/4/25 10:04, Nicholas Piggin wrote: >> These structures are hardware interfaces, ensure the layout is >> correct. >> >> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> >> --- >> hw/usb/dev-storage.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c >> index 2d7306b0572..87c22476f6b 100644 >> --- a/hw/usb/dev-storage.c >> +++ b/hw/usb/dev-storage.c >> @@ -27,7 +27,7 @@ >> #define MassStorageReset 0xff >> #define GetMaxLun 0xfe >> >> -struct usb_msd_cbw { >> +struct QEMU_PACKED usb_msd_cbw { >> uint32_t sig; >> uint32_t tag; >> uint32_t data_len; >> @@ -636,6 +636,9 @@ static const TypeInfo usb_storage_dev_type_info = { >> >> static void usb_msd_register_types(void) >> { >> + qemu_build_assert(sizeof(struct usb_msd_cbw) == 31); >> + qemu_build_assert(sizeof(struct usb_msd_csw) == 13); > > Can we add definitions for these 13/31 magic values? Then > we can use them in try_get_valid_cbw(). Good idea, I've done something to improve them. Thanks, Nick
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c index 2d7306b0572..87c22476f6b 100644 --- a/hw/usb/dev-storage.c +++ b/hw/usb/dev-storage.c @@ -27,7 +27,7 @@ #define MassStorageReset 0xff #define GetMaxLun 0xfe -struct usb_msd_cbw { +struct QEMU_PACKED usb_msd_cbw { uint32_t sig; uint32_t tag; uint32_t data_len; @@ -636,6 +636,9 @@ static const TypeInfo usb_storage_dev_type_info = { static void usb_msd_register_types(void) { + qemu_build_assert(sizeof(struct usb_msd_cbw) == 31); + qemu_build_assert(sizeof(struct usb_msd_csw) == 13); + type_register_static(&usb_storage_dev_type_info); }
These structures are hardware interfaces, ensure the layout is correct. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- hw/usb/dev-storage.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)