| Message ID | 20221024134033.30142-2-Igor.Skalkin@opensynergy.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v4,1/1] virtio_bt: Fix alignment in configuration struct | expand |
Hi Igor, On Mon, Oct 24, 2022 at 6:41 AM Igor Skalkin <Igor.Skalkin@opensynergy.com> wrote: > > The current version of the configuration structure has unaligned > 16-bit fields, but according to the specification [1], access to > the configuration space must be aligned. > > Add a second, aligned version of the configuration structure > and a new feature bit indicating that this version is being used. > > [1] https://docs.oasis-open.org/virtio/virtio/v1.1/virtio-v1.1.pdf > > Signed-off-by: Igor Skalkin <Igor.Skalkin@opensynergy.com> > --- > drivers/bluetooth/virtio_bt.c | 16 +++++++++++++--- > include/uapi/linux/virtio_bt.h | 8 ++++++++ > 2 files changed, 21 insertions(+), 3 deletions(-) > > diff --git a/drivers/bluetooth/virtio_bt.c b/drivers/bluetooth/virtio_bt.c > index 67c21263f9e0..35f8041722c8 100644 > --- a/drivers/bluetooth/virtio_bt.c > +++ b/drivers/bluetooth/virtio_bt.c > @@ -306,7 +306,12 @@ static int virtbt_probe(struct virtio_device *vdev) > if (virtio_has_feature(vdev, VIRTIO_BT_F_VND_HCI)) { > __u16 vendor; > > - virtio_cread(vdev, struct virtio_bt_config, vendor, &vendor); > + if (virtio_has_feature(vdev, VIRTIO_BT_F_CONFIG_V2)) > + virtio_cread(vdev, struct virtio_bt_config_v2, > + vendor, &vendor); > + else > + virtio_cread(vdev, struct virtio_bt_config, > + vendor, &vendor); > > switch (vendor) { > case VIRTIO_BT_CONFIG_VENDOR_ZEPHYR: > @@ -339,8 +344,12 @@ static int virtbt_probe(struct virtio_device *vdev) > if (virtio_has_feature(vdev, VIRTIO_BT_F_MSFT_EXT)) { > __u16 msft_opcode; > > - virtio_cread(vdev, struct virtio_bt_config, > - msft_opcode, &msft_opcode); > + if (virtio_has_feature(vdev, VIRTIO_BT_F_CONFIG_V2)) > + virtio_cread(vdev, struct virtio_bt_config_v2, > + msft_opcode, &msft_opcode); > + else > + virtio_cread(vdev, struct virtio_bt_config, > + msft_opcode, &msft_opcode); > > hci_set_msft_opcode(hdev, msft_opcode); > } > @@ -387,6 +396,7 @@ static const unsigned int virtbt_features[] = { > VIRTIO_BT_F_VND_HCI, > VIRTIO_BT_F_MSFT_EXT, > VIRTIO_BT_F_AOSP_EXT, > + VIRTIO_BT_F_CONFIG_V2, > }; So this introduces a new flag which must be checked when attempting to config, right? But is this backward compatible? What happens if for some reason the userspace doesn't use the new struct are we able to detect that? > static struct virtio_driver virtbt_driver = { > diff --git a/include/uapi/linux/virtio_bt.h b/include/uapi/linux/virtio_bt.h > index a7bd48daa9a9..af798f4c9680 100644 > --- a/include/uapi/linux/virtio_bt.h > +++ b/include/uapi/linux/virtio_bt.h > @@ -9,6 +9,7 @@ > #define VIRTIO_BT_F_VND_HCI 0 /* Indicates vendor command support */ > #define VIRTIO_BT_F_MSFT_EXT 1 /* Indicates MSFT vendor support */ > #define VIRTIO_BT_F_AOSP_EXT 2 /* Indicates AOSP vendor support */ > +#define VIRTIO_BT_F_CONFIG_V2 3 /* Use second version configuration */ > > enum virtio_bt_config_type { > VIRTIO_BT_CONFIG_TYPE_PRIMARY = 0, > @@ -28,4 +29,11 @@ struct virtio_bt_config { > __u16 msft_opcode; > } __attribute__((packed)); > > +struct virtio_bt_config_v2 { > + __u8 type; > + __u8 alignment; > + __u16 vendor; > + __u16 msft_opcode; > +}; > + > #endif /* _UAPI_LINUX_VIRTIO_BT_H */ > -- > 2.37.2 >
This is automated email and please do not reply to this email! Dear submitter, Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series: PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=688397 ---Test result--- Test Summary: CheckPatch PASS 1.80 seconds GitLint PASS 1.02 seconds SubjectPrefix FAIL 0.90 seconds BuildKernel PASS 34.59 seconds BuildKernel32 PASS 31.04 seconds Incremental Build with patchesPASS 45.75 seconds TestRunner: Setup PASS 515.95 seconds TestRunner: l2cap-tester PASS 17.43 seconds TestRunner: iso-tester PASS 16.58 seconds TestRunner: bnep-tester PASS 6.65 seconds TestRunner: mgmt-tester PASS 106.36 seconds TestRunner: rfcomm-tester PASS 10.56 seconds TestRunner: sco-tester PASS 10.04 seconds TestRunner: ioctl-tester PASS 11.22 seconds TestRunner: mesh-tester PASS 8.12 seconds TestRunner: smp-tester PASS 9.89 seconds TestRunner: userchan-tester PASS 6.90 seconds Details ############################## Test: SubjectPrefix - FAIL - 0.90 seconds Check subject contains "Bluetooth" prefix "Bluetooth: " is not specified in the subject --- Regards, Linux Bluetooth
Hello: This patch was applied to bluetooth/bluetooth-next.git (master) by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>: On Mon, 24 Oct 2022 15:40:33 +0200 you wrote: > The current version of the configuration structure has unaligned > 16-bit fields, but according to the specification [1], access to > the configuration space must be aligned. > > Add a second, aligned version of the configuration structure > and a new feature bit indicating that this version is being used. > > [...] Here is the summary with links: - [v4,1/1] virtio_bt: Fix alignment in configuration struct https://git.kernel.org/bluetooth/bluetooth-next/c/57dc0d471d27 You are awesome, thank you!
diff --git a/drivers/bluetooth/virtio_bt.c b/drivers/bluetooth/virtio_bt.c index 67c21263f9e0..35f8041722c8 100644 --- a/drivers/bluetooth/virtio_bt.c +++ b/drivers/bluetooth/virtio_bt.c @@ -306,7 +306,12 @@ static int virtbt_probe(struct virtio_device *vdev) if (virtio_has_feature(vdev, VIRTIO_BT_F_VND_HCI)) { __u16 vendor; - virtio_cread(vdev, struct virtio_bt_config, vendor, &vendor); + if (virtio_has_feature(vdev, VIRTIO_BT_F_CONFIG_V2)) + virtio_cread(vdev, struct virtio_bt_config_v2, + vendor, &vendor); + else + virtio_cread(vdev, struct virtio_bt_config, + vendor, &vendor); switch (vendor) { case VIRTIO_BT_CONFIG_VENDOR_ZEPHYR: @@ -339,8 +344,12 @@ static int virtbt_probe(struct virtio_device *vdev) if (virtio_has_feature(vdev, VIRTIO_BT_F_MSFT_EXT)) { __u16 msft_opcode; - virtio_cread(vdev, struct virtio_bt_config, - msft_opcode, &msft_opcode); + if (virtio_has_feature(vdev, VIRTIO_BT_F_CONFIG_V2)) + virtio_cread(vdev, struct virtio_bt_config_v2, + msft_opcode, &msft_opcode); + else + virtio_cread(vdev, struct virtio_bt_config, + msft_opcode, &msft_opcode); hci_set_msft_opcode(hdev, msft_opcode); } @@ -387,6 +396,7 @@ static const unsigned int virtbt_features[] = { VIRTIO_BT_F_VND_HCI, VIRTIO_BT_F_MSFT_EXT, VIRTIO_BT_F_AOSP_EXT, + VIRTIO_BT_F_CONFIG_V2, }; static struct virtio_driver virtbt_driver = { diff --git a/include/uapi/linux/virtio_bt.h b/include/uapi/linux/virtio_bt.h index a7bd48daa9a9..af798f4c9680 100644 --- a/include/uapi/linux/virtio_bt.h +++ b/include/uapi/linux/virtio_bt.h @@ -9,6 +9,7 @@ #define VIRTIO_BT_F_VND_HCI 0 /* Indicates vendor command support */ #define VIRTIO_BT_F_MSFT_EXT 1 /* Indicates MSFT vendor support */ #define VIRTIO_BT_F_AOSP_EXT 2 /* Indicates AOSP vendor support */ +#define VIRTIO_BT_F_CONFIG_V2 3 /* Use second version configuration */ enum virtio_bt_config_type { VIRTIO_BT_CONFIG_TYPE_PRIMARY = 0, @@ -28,4 +29,11 @@ struct virtio_bt_config { __u16 msft_opcode; } __attribute__((packed)); +struct virtio_bt_config_v2 { + __u8 type; + __u8 alignment; + __u16 vendor; + __u16 msft_opcode; +}; + #endif /* _UAPI_LINUX_VIRTIO_BT_H */
The current version of the configuration structure has unaligned 16-bit fields, but according to the specification [1], access to the configuration space must be aligned. Add a second, aligned version of the configuration structure and a new feature bit indicating that this version is being used. [1] https://docs.oasis-open.org/virtio/virtio/v1.1/virtio-v1.1.pdf Signed-off-by: Igor Skalkin <Igor.Skalkin@opensynergy.com> --- drivers/bluetooth/virtio_bt.c | 16 +++++++++++++--- include/uapi/linux/virtio_bt.h | 8 ++++++++ 2 files changed, 21 insertions(+), 3 deletions(-)