Hello,
there was a printout on my desk for some time to go through and address
the comments but the E-Mail I did indeed not answer yet. There should go
out in the next days a reworked specification and driver.
On 07.11.22 13:30, Cornelia Huck wrote:
On Thu, Aug 25 2022, Harald Mommer <harald.mommer@xxxxxxxxxxxxxxx> wrote:
virtio-can is a virtual CAN device. It provides a way to give access to
a CAN controller from a driver guest. The device is aimed to be used by
driver guests running a HLOS as well as by driver guests running a
typical RTOS as used in controller environments.
Note that I'm not familiar with CAN, so I'll comment mostly from a
virtio spec POV.
This is the reworked (2nd) version of the specification after having
gotten some feedback on the virtio lists and the Linux CAN mailing
lists. (The 1st now outdated version of the specification draft has been
sent out on 1-Apr-2021 to the virtio lists.) There is now also a virtio
CAN Linux driver which in the meantime has become good enough to be
shown.
There were a lot of changes, only mentioning the most important ones
omitting editorial changes.
- Classic CAN is indeed non-mandatory, so a feature bit is needed.
According to ISO all CAN controllers support classic CAN but it may be
disabled by configuration. So classic CAN is indeed a feature which
may not be available in some environments.
- Introduce a new feature flag VIRTIO_CAN_F_LATE_TX_ACK. While marking
as used after the actual transmission has been done on the CAN bus
this cannot be implemented reliably in all environments. SocketCAN is
affected at least under heavy load for TX and RX.
- RTR frames were requested on the Linux mailing list. They cannot be
supported when the underlying CAN driver backend is a 3rd party
AUTOSAR driver as AUTOSAR CAN does not support RTR frames. A feature
flag VIRTIO_CAN_F_RTR_FRAMES has been added to make support of RTR
frames an optional feature.
- Add le32 flags. There is now a reserved field in both RX and TX
messages which might serve to contain an AUTOSAR hardware object
handle or similar in a future version of the specification without
need to change the layout of the RX and TX message structures.
- Removal of priorities and config space. Priorities cannot be supported
using SocketCAN, the information delivered in the config space cannot
be determined using SocketCAN. Support of different priorities was
anyway too much for a first version of a specification. If priorities
are supported in a future version there will probably be only 2
different priorities, normal and high. In a future version of the
specification high priority messages may either be supported by using
a flag bit in the TX message but most probably the better solution
will be to add add a dedicated 2nd TX queue for high priority messages
in a review comment. But as already said priorities are not for now.
Please keep the change log separate from the description.
[Most people add a S-o-b, although we don't enforce DCO.]
I understand neither "S-o-b" nor "DCO". Neither Wikipedia nor Google are
helpful, nothing fits.
---
conformance.tex | 27 +++++-
content.tex | 1 +
introduction.tex | 2 +
virtio-can.tex | 229 +++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 255 insertions(+), 4 deletions(-)
create mode 100644 virtio-can.tex
(...)
diff --git a/virtio-can.tex b/virtio-can.tex
new file mode 100644
index 0000000..ef4de7b
--- /dev/null
+++ b/virtio-can.tex
@@ -0,0 +1,229 @@
+\section{CAN Device}\label{sec:Device Types / CAN Device}
+
+virtio-can is a virtio based CAN (Controller Area Network) controller.
+It is used to give a virtual machine access to a CAN bus. The CAN bus
+might either be a physical CAN bus or a virtual CAN bus between virtual
+machines or a combination of both.
+
+\subsection{Device ID}\label{sec:Device Types / CAN Device / Device ID}
+
+36
+
+\subsection{Virtqueues}\label{sec:Device Types / CAN Device / Virtqueues}
+
+\begin{description}
+\item[0] Txq
+\item[1] Rxq
+\item[2] Controlq
+\item[3] Indicationq
+\end{description}
+
+The \field{Txq} is used to send CAN packets to the CAN bus.
+
+The \field{Rxq} is used to receive CAN packets from the CAN bus.
+
+The \field{Controlq} is used to control the state of the CAN controller.
+
+The \field{Indicationq} is used to receive unsolicited indications of
+CAN controller state changes.
+
+\devicenormative{\subsubsection}{Feature bits}{Device Types / CAN Device / Feature bits}
We usually don't put the whole feature bit specification into a
normative section, especially as it applies to both device and
driver. A device normative section is for statements like "The device
MUST offer <feature> if <condition> applies", and a driver normative
section for things like "The driver MUST accept <feature> if offered".
Reworked.
+
+Actual CAN controllers support Extended CAN IDs with 29 bits (CAN~2.0B)
+as well as Standard CAN IDs with 11 bits (CAN~2.0A). The support of
+CAN~2.0B Extended CAN IDs is considered as mandatory for this
+specification.
+
+\begin{description}
+
+\item[VIRTIO_CAN_F_CAN_CLASSIC (0)]
+
+The device supports classic CAN frames with a maximum payload size of 8
+bytes.
+
+\item[VIRTIO_CAN_F_CAN_FD (1)]
+
+The device supports CAN FD frames with a maximum payload size of 64
+bytes.
+
+\item[VIRTIO_CAN_F_RTR_FRAMES (2)]
+
+The device supports RTR (remote transmission request) frames. RTR frames
+are only supported with classic CAN.
Are any combinations of those three feature bits valid (both to be
offered, and to be negotiated?) It looks like F_RTR_FRAMES would only
work with F_CAN_CLASSIC?
Without VIRTIO_CAN_F_CAN_CLASSIC no VIRTIO_CAN_F_RTR_FRAMES. Added.
+
+\item[VIRTIO_CAN_F_LATE_TX_ACK (3)]
+
+The virtio CAN device marks transmission requests from the \field{Txq}
+as used after the CAN message has been transmitted on the CAN bus.
+Without this feature flag negotiated the device is allowed to mark
"If this feature bit has not been negotiated, ..."
Replaced now not only here but also in a later chapter.
+transmission requests already as used when the CAN message has been
+scheduled for transmission but might not yet have been transmitted on
+the CAN bus.
+
+\end{description}
+
+\drivernormative{\subsubsection}{Device Initialization}{Device Types / CAN Device / Initialization}
+
+\begin{enumerate}
+
+\item Read the feature bits and negotiate with the device.
I think this step is redundant.
Removed.
+
+\item The driver MUST populate the \field{Rxq} with empty
+device-writeable buffers of at least the struct virtio_can_rx size to be
+ready for the reception of CAN messages.
virtio_can_rx is only defined further below, and, IIUC, depends on the
frames that are supported. Can this be expressed as a kind of absolute
value (depending on the negotiated features)?
Made a forward reference to the chapter where the structure is defined.
I could add here a a constant but nobody else is doing this in the
specification. 2 le16 + 2 le32 = 12 bytes + max payload length. (Will
become 16 bytes instead of 12 bytes in the next version.) With CAN
classic the payload may have a length of up to 8 bytes, with CAN FD the
payload may have a length of up to 64 bytes. The max payload length of
CAN classic and CAN FD are mentioned at different places already, for
CAN people the max. payload length are fundamental constants they know
anyway. All is given. 12+8 = X for CAN classic and 12+64 = Y for CAN FD.
+
+\item The driver MUST populate the \field{Indicationq} with empty
+device-writeable buffers of at least the struct virtio_can_event_ind size
+so that the CAN device is able to provide status change indications to the
+virtio CAN driver.
Is virtio_can_event_ind always supposed to be an le16 value? If yes,
it would probably be easier to specify that here.
The indication queue will be thrown out. An extra queue to indicate a
rare event which could be represented in a single bit in the config
space was considered overkill and the indication queue also caused
headache with certain race conditions we thought about.
+
+\end{enumerate}
+
+\subsection{Device Operation}\label{sec:Device Types / CAN Device / Device Operation}
+
+A device operation has an outcome which is described by one of the
+following values:
+
+\begin{lstlisting}
+#define VIRTIO_CAN_RESULT_OK 0u
+#define VIRTIO_CAN_RESULT_NOT_OK 1u
+\end{lstlisting}
+
+Other values are to be treated like VIRTIO_CAN_RESULT_NOT_OK.
+
+The type of a CAN message identifier is determined by \field{flags}. The
+3 most significant bits of
+\field{can_id} do not bear the information about the type of the CAN
+message identifier and are 0.
+
+\begin{lstlisting}
+#define VIRTIO_CAN_FLAGS_FD 0x4000U
+#define VIRTIO_CAN_FLAGS_EXTENDED 0x8000U
+#define VIRTIO_CAN_FLAGS_RTR 0x2000U
+\end{lstlisting}
You refer to the "flags" and "can_id" fields, but it is unclear in which
structure. Would this benefit from reordering?
Put the flags now close to struct virtio_can_tx_out and mentioned for
struct virtio_can_tx_in that the flags are the same. Should be more
clear now to what those definitions belong.
+
+\subsubsection{Controller Mode}\label{sec:Device Types / CAN Device / Device Operation / Controller Mode}
+
+The general format of a request in the \field{Controlq} is
+
+\begin{lstlisting}
+struct virtio_can_control_out {
+#define VIRTIO_CAN_SET_CTRL_MODE_START 0x0201u
+#define VIRTIO_CAN_SET_CTRL_MODE_STOP 0x0202u
+ le16 msg_type;
+};
+\end{lstlisting}
+
+To participate in bus communication the CAN controller is started by
+sending a VIRTIO_CAN_SET_CTRL_MODE_START control message, to stop
+participating in bus communication it is stopped by sending a
+VIRTIO_CAN_SET_CTRL_MODE_STOP control message. Both requests are
+confirmed by the result of the operation.
+
+\begin{lstlisting}
+struct virtio_can_control_in {
+ u8 result;
+};
+\end{lstlisting}
+
+If the transition succeeded the \field{result} is VIRTIO_CAN_RESULT_OK
+otherwise it is VIRTIO_CAN_RESULT_NOT_OK. The device marks the request
+used when the CAN controller has finalized the transition to the
+requested controller mode.
+
+On transition to the STOPPED state the device cancels all CAN messages
+already pending for transmission and marks them as used with
+\field{result} VIRTIO_CAN_RESULT_NOT_OK. In the STOPPED state the
+device marks messages received from the
+\field{Txq} as used with \field{result} VIRTIO_CAN_RESULT_NOT_OK without
+transmitting them to the CAN bus.
+
+Initially the CAN controller is in the STOPPED state.
Is that an internal state of the controller that can be changed from the
outside?
Can be changed from the outside, this is the control queue sending
VIRTIO_CAN_SET_CTRL_MODE_START to start the controller or
VIRTIO_CAN_SET_CTRL_MODE_STOP to stop the controller. And the controller
transitions from start to stop on certain error conditions on the bus
which are indicated by a bus off from the physical CAN controller hardware.
+
+Control queue messages are processed in order.
+
+\devicenormative{\subsubsection}{CAN Message Transmission}{Device Types / CAN Device / Device Operation / CAN Message Transmission}
I think this should be a normal subsection, as you describe not only
what the device needs to do, but also what the driver does. Maybe use
separate normative sections for MUST statements?
What I have now in the reworked section is that I start with an active
descriptive sentence "The driver transmits messages by placing outgoing
CAN messages in the Txq virtqueue" and subsequently talk about the
device. Could be ok now but I might be wrong.
+
+Messages are transmitted by placing outgoing CAN messages in the
+\field{Txq} virtqueue.
Who places the messages? The driver? Probably better to use active voice
here.
This is the descriptive sentence now in active voice explaining what the
message is and where it comes from.
+
+\begin{lstlisting}
+struct virtio_can_tx_out {
+#define VIRTIO_CAN_TX 0x0001u
+ le16 msg_type;
+ le16 reserved;
+ le32 flags;
+ le32 can_id;
+ u8 sdu[];
+};
+
+struct virtio_can_tx_in {
+ u8 result;
+};
+\end{lstlisting}
+
+The length of \field{sdu} is implicit in the length of the device
+read-only descriptors.
Not sure what this means -- does that mean that any length of the
desriptors is ok, as long as it fits the whole structure?
There is some information provided how much bytes the other side has
written. From this the payload length can be deducted without having a
length field in the structure itself. There were some more comments on
this, we thought about it and now we have a le16 length field in the
structure. More clear and probably more future proof.
However it was strange behavior / worth a warning / need to truncate
when more than 8 bytes payload for CAN classic or more than 64 bytes
payload for CAN FD were present.
+
+If transmission of a CAN frame type is requested for which support has
+not been negotiated \field{result} shall be set to
+VIRTIO_CAN_RESULT_NOT_OK and the message shall not be scheduled for
+transmission.
This probably should rather be something like "The device MUST reject
any CAN frame type for which support has not been negotiated with
VIRTIO_CAN_RESULT_NOT_OK in \field{result} and MUST NOT schedule the
message for transmission."
Reworded as proposed.
+
+If \field{can_id} or field{sdu} length are out of range or the CAN
+controller is in an invalid state \field{result} shall be set to
+VIRTIO_CAN_RESULT_NOT_OK and the message shall not be scheduled for
+transmission.
Same here.
"shall" => "MUST". Done.
+
+If the parameters are valid the message is scheduled for transmission.
+
+If feature VIRTIO_CAN_F_CAN_LATE_TX_ACK has been negotiated the
+transmission request MUST be marked as used with \field{result} set to
+VIRTIO_CAN_OK after the CAN controller acknowledged the successful
+transmission on the CAN bus. Without feature
+VIRTIO_CAN_F_CAN_LATE_TX_ACK negotiated the transmission request MAY
+already be marked as used with \field{result} set to VIRTIO_CAN_OK when
+the transmission request has been processed by the virtio CAN device and
+send down the protocol stack being scheduled for transmission.
+
+\subsubsection{CAN Message Reception}\label{sec:Device Types / CAN Device / Device Operation / CAN Message Reception}
+
+Messages can be received by providing empty incoming buffers to the
+virtqueue \field{Rxq}.
+
+\begin{lstlisting}
+struct virtio_can_rx {
+#define VIRTIO_CAN_RX 0x0101u
+ le16 msg_type;
+ le16 reserved;
+ le32 flags;
+ le32 can_id;
+ u8 sdu[];
+};
+\end{lstlisting}
+
+If the feature VIRTIO_CAN_F_CAN_FD has been negotiated the maximal
+possible \field{sdu} length is 64, if the feature has not been
+negotiated the maximal possible \field{sdu} length is 8.
+
+The actual length of the \field{sdu} is calculated from the length of the
+driver read-only descriptors.
So the provided descriptors must fit at least the size of the structure
plus whatever length the negotiated frame sizes support? Can the driver
negotiate F_CAN_FD and still provide descriptors that only allow an sdu
of length 8?
Negotiating CAN FD and not allowing for 64 bytes payload would be like
having an Ethernet device not providing enough room to receive a 1518
bytes frame with 1500 bytes payload. Means this was a bug.
+
+\subsubsection{BusOff Indication}\label{sec:Device Types / CAN Device / Device Operation / BusOff Indication}
+
+There are certain error conditions so that the physical CAN controller
+has to stop participating in CAN communication on the bus. If such an
+error condition occurs the device informs the driver about the
+unsolicited CAN controller state change by a CAN BusOff indication.
+
+\begin{lstlisting}
+struct virtio_can_event_ind {
+#define VIRTIO_CAN_BUSOFF_IND 0x0301u
+ le16 msg_type;
+};
+\end{lstlisting}
+
+After bus-off detection the CAN controller is in STOPPED state. The CAN
+module does not participate in bus communication any more so all CAN
s/module/device/ ?
Yes, device.
(And the rarely used message will be eliminated in the next version
together with the queue in favor of a single bit in the config space.
Already mentioned above.)
+messages pending for transmission are put into the used queue with
+\field{result} VIRTIO_CAN_RESULT_NOT_OK.
