[PATCH v2 0/1] virtio_bt: Fix alignment in configuration struct

[PATCH v2 0/1] virtio_bt: Fix alignment in configuration struct

[Igor Skalkin]

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 has non-aligned 16bit
fields.

This patch adds 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

Changes in v2:
  The first version of this patch just changed the configuration
  structure in uapi/linux/virtio_bt.h
  This can not be done, because it will break the userspace, so the
  second version offers a less radical approach - it introduces a new
  feature bit and a new configuration structure that both the device
  and the driver will use if this bit is negotiated.

Igor Skalkin (1):
  virtio_bt: Fix alignment in configuration struct

 drivers/bluetooth/virtio_bt.c  | 16 +++++++++++++---
 include/uapi/linux/virtio_bt.h |  8 ++++++++
 2 files changed, 21 insertions(+), 3 deletions(-)

-- 
2.37.2

[PATCH v2 1/1] virtio_bt: Fix alignment in configuration struct

[Igor Skalkin]

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,
 };
 
 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

RE: virtio_bt: Fix alignment in configuration struct

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2236 bytes --]

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=686388

---Test result---

Test Summary:
CheckPatch                    FAIL      1.21 seconds
GitLint                       PASS      0.50 seconds
SubjectPrefix                 FAIL      0.33 seconds
BuildKernel                   PASS      43.15 seconds
BuildKernel32                 PASS      35.30 seconds
Incremental Build with patchesPASS      57.19 seconds
TestRunner: Setup             PASS      670.92 seconds
TestRunner: l2cap-tester      PASS      19.69 seconds
TestRunner: iso-tester        PASS      20.41 seconds
TestRunner: bnep-tester       PASS      7.48 seconds
TestRunner: mgmt-tester       PASS      125.55 seconds
TestRunner: rfcomm-tester     PASS      11.91 seconds
TestRunner: sco-tester        PASS      11.23 seconds
TestRunner: ioctl-tester      PASS      13.98 seconds
TestRunner: mesh-tester       PASS      10.32 seconds
TestRunner: smp-tester        PASS      12.00 seconds
TestRunner: userchan-tester   PASS      8.77 seconds

Details
##############################
Test: CheckPatch - FAIL - 1.21 seconds
Run checkpatch.pl script with rule in .checkpatch.conf
[v2,1/1] virtio_bt: Fix alignment in configuration struct\WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Igor Skalkin <igor375@gmail.com>' != 'Signed-off-by: Igor Skalkin <Igor.Skalkin@opensynergy.com>'

total: 0 errors, 1 warnings, 52 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/13010854.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: SubjectPrefix - FAIL - 0.33 seconds
Check subject contains "Bluetooth" prefix
"Bluetooth: " is not specified in the subject



---
Regards,
Linux Bluetooth

Re: virtio_bt: Fix alignment in configuration struct

[Luiz Augusto von Dentz]

Hi Igor,

On Tue, Oct 18, 2022 at 11:33 AM <bluez.test.bot@gmail.com> wrote:
>
> 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=686388
>
> ---Test result---
>
> Test Summary:
> CheckPatch                    FAIL      1.21 seconds
> GitLint                       PASS      0.50 seconds
> SubjectPrefix                 FAIL      0.33 seconds
> BuildKernel                   PASS      43.15 seconds
> BuildKernel32                 PASS      35.30 seconds
> Incremental Build with patchesPASS      57.19 seconds
> TestRunner: Setup             PASS      670.92 seconds
> TestRunner: l2cap-tester      PASS      19.69 seconds
> TestRunner: iso-tester        PASS      20.41 seconds
> TestRunner: bnep-tester       PASS      7.48 seconds
> TestRunner: mgmt-tester       PASS      125.55 seconds
> TestRunner: rfcomm-tester     PASS      11.91 seconds
> TestRunner: sco-tester        PASS      11.23 seconds
> TestRunner: ioctl-tester      PASS      13.98 seconds
> TestRunner: mesh-tester       PASS      10.32 seconds
> TestRunner: smp-tester        PASS      12.00 seconds
> TestRunner: userchan-tester   PASS      8.77 seconds
>
> Details
> ##############################
> Test: CheckPatch - FAIL - 1.21 seconds
> Run checkpatch.pl script with rule in .checkpatch.conf
> [v2,1/1] virtio_bt: Fix alignment in configuration struct\WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Igor Skalkin <igor375@gmail.com>' != 'Signed-off-by: Igor Skalkin <Igor.Skalkin@opensynergy.com>'

You probably need to configure your .gitconfig to match your Signed-off-by.

>
> total: 0 errors, 1 warnings, 52 lines checked
>
> NOTE: For some of the reported defects, checkpatch may be able to
>       mechanically convert to the typical style using --fix or --fix-inplace.
>
> /github/workspace/src/13010854.patch has style problems, please review.
>
> NOTE: Ignored message types: UNKNOWN_COMMIT_ID
>
> NOTE: If any of the errors are false positives, please report
>       them to the maintainer, see CHECKPATCH in MAINTAINERS.
>
>
> ##############################
> Test: SubjectPrefix - FAIL - 0.33 seconds
> Check subject contains "Bluetooth" prefix
> "Bluetooth: " is not specified in the subject
>
>
>
> ---
> Regards,
> Linux Bluetooth
>


-- 
Luiz Augusto von Dentz

RE: virtio_bt: Fix alignment in configuration struct

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 674 bytes --]

This is an automated email and please do not reply to this email.

Dear Submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
While preparing the CI tests, the patches you submitted couldn't be applied to the current HEAD of the repository.

----- Output -----
error: patch failed: drivers/bluetooth/virtio_bt.c:306
error: drivers/bluetooth/virtio_bt.c: patch does not apply
error: patch failed: include/uapi/linux/virtio_bt.h:9
error: include/uapi/linux/virtio_bt.h: patch does not apply
hint: Use 'git am --show-current-patch' to see the failed patch


Please resolve the issue and submit the patches again.


---
Regards,
Linux Bluetooth

Re: virtio_bt: Fix alignment in configuration struct

[Luiz Augusto von Dentz]

Hi Igor,

On Tue, Oct 18, 2022 at 12:58 PM <bluez.test.bot@gmail.com> wrote:
>
> This is an automated email and please do not reply to this email.
>
> Dear Submitter,
>
> Thank you for submitting the patches to the linux bluetooth mailing list.
> While preparing the CI tests, the patches you submitted couldn't be applied to the current HEAD of the repository.
>
> ----- Output -----
> error: patch failed: drivers/bluetooth/virtio_bt.c:306
> error: drivers/bluetooth/virtio_bt.c: patch does not apply
> error: patch failed: include/uapi/linux/virtio_bt.h:9
> error: include/uapi/linux/virtio_bt.h: patch does not apply
> hint: Use 'git am --show-current-patch' to see the failed patch
>
>
> Please resolve the issue and submit the patches again.

This is very likely due to your mail server changing the format of the
patch, you can use gmail to send the email like you did previously,
that doesn't affect the commit author which is what needs to match the
Signed-off-by.

>
> ---
> Regards,
> Linux Bluetooth
>


-- 
Luiz Augusto von Dentz

Re: [PATCH v2 0/1] virtio_bt: Fix alignment in configuration struct

[patchwork-bot+bluetooth]

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Tue, 18 Oct 2022 19:30:49 +0200 you 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 has non-aligned 16bit
> fields.
> 
> [...]

Here is the summary with links:
  - [v2,1/1] virtio_bt: Fix alignment in configuration struct
    https://git.kernel.org/bluetooth/bluetooth-next/c/57dc0d471d27

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html