OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.

 


Help: OASIS Mailing Lists Help | MarkMail Help

virtio-comment message

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]


Subject: Re: [virtio-dev] [PATCH 1/1] RFC: virtio-bt: add virtio BT device specification


Hello all, sorry for a delay.

Almost all of you questions relate to the structure of the configuration space, in which I simple described the current virtio-bt driver implementation in the Linux kernel.

Brief history of this draft specification:
  - Suddenly the virtio-bt Linux kernel driver was merged in the mainline.
- We started using it in our projects - for this we wrote a host virtio Bluetooth device. - This device could not be merged because the structure of the configuration space is not aligned, which violates the virtio specification (and our virtio MMIO middle layer asserts such accesses). - The corrected version of the configuration with the corresponding feature bit was also merged into the mainline.
  - But fixing the configuration requires fixing the specification.
  - There is no specification - that means someone need to write it.
- Well, for the first draft version I think it is possible to simply describe the existing Linux implementation and see what can be done with all this.
  - we are here.

The main question is whether there are already existing devices that use this configuration structure. I do not know, may be the authors of the Linux kernel driver know the answer to this question, and they are in this message CC.

About the use of the second version of the configuration by new devices - can, should or must they use it. It seems to me that they must, since the old one violates the virtio specification.
[q]
4.2.2.2 Driver Requirements: MMIO Device Register Layout
...
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.
[/q]

On 3/22/23 00:24, Enrico Granata wrote:
On Thu, Mar 9, 2023 at 11:27âPM Igor Skalkin
<Igor.Skalkin@opensynergy.com> wrote:

This PR is aimed as review for comments(RFC) purpose.

* Initial draft version.

Signed-off-by: Igor Skalkin <Igor.Skalkin@opensynergy.com>
---
  conformance.tex                        |  12 ++-
  content.tex                            |   1 +
  device-types/bt/description.tex        | 112 +++++++++++++++++++++++++
  device-types/bt/device-conformance.tex |   8 ++
  device-types/bt/driver-conformance.tex |   8 ++
  5 files changed, 137 insertions(+), 4 deletions(-)
  create mode 100644 device-types/bt/description.tex
  create mode 100644 device-types/bt/device-conformance.tex
  create mode 100644 device-types/bt/driver-conformance.tex

diff --git a/conformance.tex b/conformance.tex
index 01ccd69..a69eb47 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -32,8 +32,9 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
  \ref{sec:Conformance / Driver Conformance / Memory Driver Conformance},
  \ref{sec:Conformance / Driver Conformance / I2C Adapter Driver Conformance},
  \ref{sec:Conformance / Driver Conformance / SCMI Driver Conformance},
-\ref{sec:Conformance / Driver Conformance / GPIO Driver Conformance} or
-\ref{sec:Conformance / Driver Conformance / PMEM Driver Conformance}.
+\ref{sec:Conformance / Driver Conformance / GPIO Driver Conformance},
+\ref{sec:Conformance / Driver Conformance / PMEM Driver Conformance} or
+\ref{sec:Conformance / Driver Conformance / BT Driver Conformance}

      \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
    \end{itemize}
@@ -59,8 +60,9 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
  \ref{sec:Conformance / Device Conformance / Memory Device Conformance},
  \ref{sec:Conformance / Device Conformance / I2C Adapter Device Conformance},
  \ref{sec:Conformance / Device Conformance / SCMI Device Conformance},
-\ref{sec:Conformance / Device Conformance / GPIO Device Conformance} or
-\ref{sec:Conformance / Device Conformance / PMEM Device Conformance}.
+\ref{sec:Conformance / Device Conformance / GPIO Device Conformance},
+\ref{sec:Conformance / Device Conformance / PMEM Device Conformance} or
+\ref{sec:Conformance / Device Conformance / BT Device Conformance}

      \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
    \end{itemize}
@@ -152,6 +154,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
  \input{device-types/scmi/driver-conformance.tex}
  \input{device-types/gpio/driver-conformance.tex}
  \input{device-types/pmem/driver-conformance.tex}
+\input{device-types/bt/driver-conformance.tex}

  \conformance{\section}{Device Conformance}\label{sec:Conformance / Device Conformance}

@@ -238,6 +241,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
  \input{device-types/scmi/device-conformance.tex}
  \input{device-types/gpio/device-conformance.tex}
  \input{device-types/pmem/device-conformance.tex}
+\input{device-types/bt/device-conformance.tex}

  \conformance{\section}{Legacy Interface: Transitional Device and Transitional Driver Conformance}\label{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}
  A conformant implementation MUST be either transitional or
diff --git a/content.tex b/content.tex
index 0c7cdf8..3723ec8 100644
--- a/content.tex
+++ b/content.tex
@@ -3022,6 +3022,7 @@ \chapter{Device Types}\label{sec:Device Types}
  \input{device-types/scmi/description.tex}
  \input{device-types/gpio/description.tex}
  \input{device-types/pmem/description.tex}
+\input{device-types/bt/description.tex}

  \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}

diff --git a/device-types/bt/description.tex b/device-types/bt/description.tex
new file mode 100644
index 0000000..3ce265d
--- /dev/null
+++ b/device-types/bt/description.tex
@@ -0,0 +1,112 @@
+\section{BT Device}\label{sec:Device Types / BT Device}
+
+The virtio-bt device provides an HCI (Host Control Interface) over VirtIO
+link between the guest HCI device and the host HCI backend.
+Also, the device can inform the guest driver which vendor-specific command
+set it supports.
+Host Control Interface is described in Bluetooth Core Specification:
+\newline\url{https://www.bluetooth.com/specifications/specs/core-specification-5-4/}\\

Are you giving any thought to out of band communication (e.g. A2DP and HFP)?

A2DP works on top of HCI, with HFP everything is more complicated.
There is already a working project with the transmission of additional commands to the host to control the switching of audio streams on host. But, it was necessary, because BT chip on board in this project do not support SCO over HCI, but has hardware connection to PCM. In common case (BT USB dongle, for example) SCO data should be transmitted via HCI connection. A project with the transmission of SCO data via HCI will be started soon, that is, work is underway but has not yet reached the specification phase.
+
+\subsection{Device ID}\label{sec:Device Types / BT Device / Device ID}
+
+40
+
+\subsection{Virtqueues}\label{sec:Device Types / BT Device / Virtqueues}
+
+\begin{description}
+\item[0] transmitq
+\item[1] receiveq
+\end{description}
+
+\subsection{Feature bits}\label{sec:Device Types / BT Device / Feature bits}
+
+\begin{description}
+\item[VIRTIO_BT_F_VND_HCI (0)]  Indicates vendor command support.
+\item[VIRTIO_BT_F_MSFT_EXT (1)] Indicates MSFT vendor support.
+\item[VIRTIO_BT_F_AOSP_EXT (2)] Indicates AOSP vendor support.
+\item[VIRTIO_BT_F_CONFIG_V2 (3)] The device uses the second version of the
+configuration space structure.
+\end{description}
+

How are these different from the vendor fields discussed elsewhere in the spec?
Is it the idea that any vendor could support AOSP or MSFT specific
command sets, and so you need two separate flags?

If so, maybe calling them "guest OS support" instead of "vendor" might
be a better choice to avoid confusion
Also, we should probably document somewhere what commands those
extensions entail

Thank you, will be fixed in next version.

+\devicenormative{\subsubsection}{Feature bits}{Device Types / BT Device / Feature bits}
+
+The device MUST require the driver to accept the VIRTIO_BT_F_CONFIG_V2 feature
+bit, i.e. not set FEATURES_OK without it, and use the second version
+(struct virtio_bt_config_v2) of the configuration layout, because the
+first one (struct virtio_bt_config) is unaligned, which violates the
+specification.
+
+The device should offer VIRTIO_BT_F_MSFT_EXT or VIRTIO_BT_F_AOSP_EXT feature bit
+if it supports correspondingly MSFT or AOSP extension command sets. In case of
+VIRTIO_BT_F_MSFT_EXT, device should also set configuration \field{msft_opcode}.
+
+The device should offer VIRTIO_BT_F_VND_HCI feature bit and set \field{vendor}
+to the VIRTIO_BT_CONFIG_VENDOR_ZEPHYR, VIRTIO_BT_CONFIG_VENDOR_INTEL or
+VIRTIO_BT_CONFIG_VENDOR_REALTEK, if it supports corresponding vendor extensions.
+
+\drivernormative{\subsubsection}{Feature bits}{Device Types / BT Device / Feature bits}
+
+The driver MUST accept VIRTIO_BT_F_CONFIG_V2 feature bit if offered by the device.
+
+The driver SHOULD accept any of the  VIRTIO_BT_F_VND_HCI, VIRTIO_BT_F_MSFT_EXT
+or VIRTIO_BT_F_AOSP_EXT feature bits if offered by the device.
+
+\subsection{Device configuration layout}\label{sec:Device Types / BT Device / Device configuration layout}
+
+
+The first version:
+\begin{lstlisting}
+struct virtio_bt_config {
+       u8 type;
+       le16 vendor;
+       le16 msft_opcode;
+} __attribute__((packed));
+\end{lstlisting}
+
+is deprecated, new devices must use the second one:
+\begin{lstlisting}
+struct virtio_bt_config_v2 {
+       u8 type;
+       u8 alignment;
+       le16 vendor;
+       le16 msft_opcode;
+};
+\end{lstlisting}
+
+\devicenormative{\subsubsection}{Device configuration layout}{Device Types / BT Device / Device configuration layout}
+The device MUST NOT present a value different than
+\begin{lstlisting}
+       VIRTIO_BT_CONFIG_TYPE_PRIMARY = 0,
+       VIRTIO_BT_CONFIG_TYPE_AMP = 1,
+\end{lstlisting}
+in \field{type}.

What do these values mean?

Bluetooth controllers can be one of two types - HCI_PRIMARY or HCI_AMP (Alternate MAC/PHY (AMP) secondary controllers). The description can be found in the Bluetooth Core Specification up to version 4.2; starting from version 4.3, HCI_AMP was removed from the specification due to the fact that no one uses it. Also, this field is not used at all in the current version of the Linux kernel driver. So maybe, this field should be marked unused in the specification.
+
+The values 1..3 of the \field{vendor} are already reserved for vendor extensions listed below:
+\begin{lstlisting}
+       VIRTIO_BT_CONFIG_VENDOR_NONE = 0
+       VIRTIO_BT_CONFIG_VENDOR_ZEPHYR = 1
+       VIRTIO_BT_CONFIG_VENDOR_INTEL = 2
+       VIRTIO_BT_CONFIG_VENDOR_REALTEK = 3
+\end{lstlisting}
+

How would any other vendor add their extensions to this list?
For example, say I start my own Bluetooth adapter company tomorrow,
how do I add VENDOR_ENRICO to the list?

Please, do not do that))
Sorry, I actually don't know the answer to this question.
But there is one idea, it is necessary to consult the driver author:

differences between specific vendors include:
a) During setup, the command "Read build info/version" with vendor specific opcode is executed and does nothing with the result.
b) The set_bdaddr() function is replaced using a non-standard opcode.
c) The vendor-specific set of the HCI_QUIRK_* bits in the hdev->quirks field is set.

So, to make the configuration more flexible, it is possible to remove the "vendor" field and add some new configuration fields:

Ð) opcode for set_bdaddr command (if non-standard)
b) opcode for "Read Build Information"/"Read Version"
c) HCI_QUIRK_* bit mask which necessary to be set to hdev->quirks.

+If value of the \field{vendor} is not VIRTIO_BT_CONFIG_VENDOR_NONE, device MUST
+offer VIRTIO_BT_F_VND_HCI feature bit.
+
+\drivernormative{\subsubsection}{Driver configuration layout}{Device Types / BT Device / Driver configuration layout}
+All configuration fields are read-only for the driver.
+
+\subsection{Device initialization}\label{sec:Device Types / BT Device / Device initialization}
+
+The virtqueues are initialized.
+
+\subsection{Device operations}\label{sec:Device Types / BT Device / Device operations}
+
+The driver SHOULD populate the receive queue with at least one buffer of at
+least 258 bytes to contain 1 byte "packet type" and HCI event packet (2 bytes
+of HCI event packet header and up to 255 bytes payload).
+Synchronous and asynchronous data packets that are longer than the provided
+buffer will be fragmented.
+
+The driver sends to the transmit queue all (command and data) packets, received
+from the guest HCI device, and transfers to the guest HCI device all (event and
+data) HCI packets, received from the receive queue.
diff --git a/device-types/bt/device-conformance.tex b/device-types/bt/device-conformance.tex
new file mode 100644
index 0000000..7b73f4a
--- /dev/null
+++ b/device-types/bt/device-conformance.tex
@@ -0,0 +1,8 @@
+\conformance{\subsection}{Bluetooth Device Conformance}\label{sec:Conformance / Device Conformance / Bluetooth Device Conformance}
+
+A BT device MUST conform to the following normative statements:
+
+\begin{itemize}
+\item \ref{devicenormative:Device Types / BT Device / Feature bits}
+\item \ref{devicenormative:Device Types / BT Device / Device configuration layout}
+\end{itemize}
diff --git a/device-types/bt/driver-conformance.tex b/device-types/bt/driver-conformance.tex
new file mode 100644
index 0000000..2264a79
--- /dev/null
+++ b/device-types/bt/driver-conformance.tex
@@ -0,0 +1,8 @@
+\conformance{\subsection}{BT Driver Conformance}\label{sec:Conformance / Driver Conformance / BT Driver Conformance}
+
+A BT driver MUST conform to the following normative statements:
+
+\begin{itemize}
+\item \ref{drivernormative:Device Types / BT Device / Feature bits}
+\item \ref{drivernormative:Device Types / BT Device / Driver configuration layout}
+\end{itemize}
--
2.37.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org



[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]