Michael S. Tsirkin
2022-Aug-07 23:00 UTC
[PATCH] virtio_bt: Fix alignment in configuration struct
On Mon, Aug 08, 2022 at 12:11:52AM +0200, Igor Skalkin wrote:> According to specification [1], "For the device-specific configuration > space, the driver MUST use 8 bit wide accesses for 8 bit wide fields, > 16 bit wide and aligned accesses for 16 bit wide fields and 32 bit wide > and aligned accesses for 32 and 64 bit wide fields.". > > Current version of the configuration structure: > > struct virtio_bt_config { > __u8 type; > __u16 vendor; > __u16 msft_opcode; > } __attribute__((packed)); > > has both 16bit fields non-aligned. > > This commit fixes it. > > [1] https://docs.oasis-open.org/virtio/virtio/v1.1/virtio-v1.1.pdf > > Signed-off-by: Igor Skalkin <Igor.Skalkin at opensynergy.com>This is all true enough, but the problem is 1. changing uapi like this can't be done, will break userspace 2. the driver has more issues and no one seems to want to maintain it. I posted a patch "Bluetooth: virtio_bt: mark broken" and intend to merge it for this release.> --- > include/uapi/linux/virtio_bt.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/uapi/linux/virtio_bt.h b/include/uapi/linux/virtio_bt.h > index a7bd48daa9a9..adc03709cc4f 100644 > --- a/include/uapi/linux/virtio_bt.h > +++ b/include/uapi/linux/virtio_bt.h > @@ -23,9 +23,9 @@ enum virtio_bt_config_vendor { > }; > > struct virtio_bt_config { > - __u8 type; > __u16 vendor; > __u16 msft_opcode; > + __u8 type; > } __attribute__((packed)); > > #endif /* _UAPI_LINUX_VIRTIO_BT_H */ > -- > 2.34.1
Igor Skalkin
2022-Aug-08 12:04 UTC
[PATCH] virtio_bt: Fix alignment in configuration struct
On 8/8/22 01:00, Michael S. Tsirkin wrote:
On Mon, Aug 08, 2022 at 12:11:52AM +0200, Igor Skalkin wrote:
According to specification [1], "For the device-specific configuration
space, the driver MUST use 8 bit wide accesses for 8 bit wide fields,
16 bit wide and aligned accesses for 16 bit wide fields and 32 bit wide
and aligned accesses for 32 and 64 bit wide fields.".
Current version of the configuration structure:
struct virtio_bt_config {
__u8 type;
__u16 vendor;
__u16 msft_opcode;
} __attribute__((packed));
has both 16bit fields non-aligned.
This commit fixes it.
[1]
https://ddec1-0-en-ctp.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2fdocs.oasis%2dopen.org%2fvirtio%2fvirtio%2fv1.1%2fvirtio%2dv1.1.pdf&umid=d1786ace-e8ea-40e8-9665-96c0949174e5&auth=53c7c7de28b92dfd96e93d9dd61a23e634d2fbec-39b15885ceebe9fda9357320aec1ccbac416a470
Signed-off-by: Igor Skalkin <Igor.Skalkin at
opensynergy.com><mailto:Igor.Skalkin at opensynergy.com>
This is all true enough, but the problem is
1. changing uapi like this can't be done, will break userspace
2. the driver has more issues and no one seems to want to
maintain it.
I posted a patch "Bluetooth: virtio_bt: mark broken" and intend
to merge it for this release.
This is very sad. We already use this driver in our projects.
Our virtio bluetooth device has two backends - HCI_USER socket backend for one
platform and uart backend for the other, and works well (after applying your
"[PATCH] Bluetooth: virtio_bt: fix device remove") patch, so this
"device
removal" problem can probably be considered solved .
We could help with the rest of the problems you listed that can be solved
(specification, QEMU support).
And the only problem that is difficult to solve (because of the need to change
UAPI header files) is just this one with unaligned configuration fields.
At the moment, it does not reproduce, because without VIRTIO_BT_F_VND_HCI
(Indicates vendor command support) feature negotiated, the driver does not
read the non-aligned configuration fields.
So, what would you advise us to do? Continuing to use the "marked
broken"
driver, start writing a specification for a new from scratch, better one?
Or is there any way to bring this one back to life?
---
include/uapi/linux/virtio_bt.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/virtio_bt.h b/include/uapi/linux/virtio_bt.h
index a7bd48daa9a9..adc03709cc4f 100644
--- a/include/uapi/linux/virtio_bt.h
+++ b/include/uapi/linux/virtio_bt.h
@@ -23,9 +23,9 @@ enum virtio_bt_config_vendor {
};
struct virtio_bt_config {
- __u8 type;
__u16 vendor;
__u16 msft_opcode;
+ __u8 type;
} __attribute__((packed));
#endif /* _UAPI_LINUX_VIRTIO_BT_H */
--
2.34.1
--
Best regards,
Igor Skalkin
Software Engineer
OpenSynergy GmbH
Rotherstr. 20, 10245 Berlin
igor.skalkin at opensynergy.com<mailto:igor.skalkin at opensynergy.com>
www.opensynergy.com<http://www.opensynergy.com>
registered: Amtsgericht Charlottenburg, HRB 108616B
General Management: Rolf Morich, Stefaan Sonck Thiebaut
Please mind our privacy
notice<https://www.opensynergy.com/datenschutzerklaerung/privacy-notice-for-business-partners-pursuant-to-article-13-of-the-general-data-protection-regulation-gdpr/>
pursuant to Art. 13 GDPR. // Unsere Hinweise zum Datenschutz gem. Art. 13 DSGVO
finden Sie
hier.<https://www.opensynergy.com/de/datenschutzerklaerung/datenschutzhinweise-fuer-geschaeftspartner-gem-art-13-dsgvo/>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20220808/d9002c82/attachment-0001.html>