Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 25 May 2021 11:22:52 +0200
From:   Jean-Philippe Brucker <jean-philippe@linaro.org>
To:     Marcel Holtmann <marcel@holtmann.org>
Cc:     linux-bluetooth@vger.kernel.org
Subject: Re: [PATCH] Bluetooth: Add support for virtio transport driver
Message-ID: <YKzB7HKqjjetjd1M@myrica>
References: <20210406141258.258544-1-marcel@holtmann.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210406141258.258544-1-marcel@holtmann.org>
Precedence: bulk

Hi Marcel,

On Tue, Apr 06, 2021 at 04:12:58PM +0200, Marcel Holtmann wrote:
> diff --git a/include/uapi/linux/virtio_bt.h b/include/uapi/linux/virtio_bt.h
> new file mode 100644
> index 000000000000..0cedceaacf88
> --- /dev/null
> +++ b/include/uapi/linux/virtio_bt.h
> @@ -0,0 +1,31 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +
> +#ifndef _UAPI_LINUX_VIRTIO_BT_H
> +#define _UAPI_LINUX_VIRTIO_BT_H
> +
> +#include <linux/virtio_types.h>
> +
> +/* Feature bits */
> +#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 */
> +
> +enum virtio_bt_config_type {
> +	VIRTIO_BT_CONFIG_TYPE_PRIMARY	= 0,
> +	VIRTIO_BT_CONFIG_TYPE_AMP	= 1,
> +};
> +
> +enum virtio_bt_config_vendor {
> +	VIRTIO_BT_CONFIG_VENDOR_NONE	= 0,
> +	VIRTIO_BT_CONFIG_VENDOR_ZEPHYR	= 1,
> +	VIRTIO_BT_CONFIG_VENDOR_INTEL	= 2,
> +	VIRTIO_BT_CONFIG_VENDOR_REALTEK	= 3,
> +};
> +
> +struct virtio_bt_config {
> +	__u8  type;
> +	__u16 vendor;
> +	__u16 msft_opcode;
> +} __attribute__((packed));

Config fields should be naturally aligned, because the virtio spec says
(4.1.3.1 Driver Requirements: PCI Device Layout).

	For device configuration access, 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-bit and 64-bit wide
	fields.

> +
> +#endif /* _UAPI_LINUX_VIRTIO_BT_H */
> diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h
> index bc1c0621f5ed..b4f468e9441d 100644
> --- a/include/uapi/linux/virtio_ids.h
> +++ b/include/uapi/linux/virtio_ids.h
> @@ -53,6 +53,7 @@
>  #define VIRTIO_ID_MEM			24 /* virtio mem */
>  #define VIRTIO_ID_FS			26 /* virtio filesystem */
>  #define VIRTIO_ID_PMEM			27 /* virtio pmem */
> +#define VIRTIO_ID_BT			28 /* virtio bluetooth */

ID 28 is already reserved for virtio-rpmb:
https://github.com/oasis-tcs/virtio-spec/issues/58

To avoid major compatibility pains going forward I think this should be
changed to ID 40 before v5.13. It won't be easy to fix afterwards since
it's UAPI.

Probably a good idea to fix or revert virtio_bt.h as well before it's too
late. Patches to the virtio headers should Cc
virtualization@lists.linux-foundation.org and perhaps
virtio-dev@lists.oasis-open.org because they can provide guidance about
this.

Thanks,
Jean

>  #define VIRTIO_ID_MAC80211_HWSIM	29 /* virtio mac80211-hwsim */
>  
>  #endif /* _LINUX_VIRTIO_IDS_H */
> -- 
> 2.30.2
>