From: Igor Skalkin <igor.skalkin@opensynergy.com>
To: Enrico Granata <egranata@google.com>
Cc: virtio-dev@lists.oasis-open.org,
virtio-comment@lists.oasis-open.org, luiz.dentz@gmail.com,
mst@redhat.com, marcel@holtmann.org, johan.hedberg@gmail.com,
jasowang@redhat.com, Hao Chen <chenhaosjtuacm@google.com>,
Sal Savage <salsavage@google.com>
Subject: Re: [virtio-dev] [PATCH 1/1] RFC: virtio-bt: add virtio BT device specification
Date: Fri, 6 Oct 2023 18:17:17 +0200 [thread overview]
Message-ID: <0c514678-d71f-4d73-bb43-b30fde320a27@opensynergy.com> (raw)
In-Reply-To: <CAPR809u5AehjXzjn4QcsVUybL=pW9mWGDJHXfcXRZCXvTcEiLw@mail.gmail.com>
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
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
prev parent reply other threads:[~2023-10-06 16:17 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-10 6:26 [virtio-dev] [PATCH 0/1] Virtio-bt specification draft Igor Skalkin
2023-03-10 6:26 ` [virtio-dev] [PATCH 1/1] RFC: virtio-bt: add virtio BT device specification Igor Skalkin
2023-03-15 15:55 ` [virtio-dev] Re: [virtio-comment] " Cornelia Huck
2023-03-15 16:08 ` Michael S. Tsirkin
2023-03-15 16:20 ` Cornelia Huck
2023-03-15 16:26 ` Michael S. Tsirkin
2023-03-21 23:24 ` [virtio-dev] " Enrico Granata
2023-10-06 16:17 ` Igor Skalkin [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0c514678-d71f-4d73-bb43-b30fde320a27@opensynergy.com \
--to=igor.skalkin@opensynergy.com \
--cc=chenhaosjtuacm@google.com \
--cc=egranata@google.com \
--cc=jasowang@redhat.com \
--cc=johan.hedberg@gmail.com \
--cc=luiz.dentz@gmail.com \
--cc=marcel@holtmann.org \
--cc=mst@redhat.com \
--cc=salsavage@google.com \
--cc=virtio-comment@lists.oasis-open.org \
--cc=virtio-dev@lists.oasis-open.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).