* [virtio-comment] [RFC PATCH v3 1/4] virtio-rtc: Add initial device specification
2023-12-18 6:42 [virtio-comment] [RFC PATCH v3 0/4] virtio-rtc: Add device specification Peter Hilber
@ 2023-12-18 6:42 ` Peter Hilber
2024-01-20 10:07 ` [virtio-comment] " Parav Pandit
2023-12-18 6:42 ` [virtio-comment] [RFC PATCH v3 2/4] virtio-rtc: Add initial normative statements Peter Hilber
` (3 subsequent siblings)
4 siblings, 1 reply; 24+ messages in thread
From: Peter Hilber @ 2023-12-18 6:42 UTC (permalink / raw)
To: virtio-comment; +Cc: Peter Hilber, Cornelia Huck, Parav Pandit
This is a draft for a Real-Time Clock (RTC) device. The virtio-rtc
device provides information about current time through one or more
clocks.
The normative statements for this device follow in the next patch.
For this device, there is an RFC Linux kernel driver which is being
upstreamed, and a proprietary device implementation.
Open Questions
==============
Arm Generic Timer Clock Types
-----------------------------
This spec version still distinguishes the Arm Generic Timer virtual and
physical counters:
/* Arm Generic Timer CNTVCT_EL0, Counter-timer Virtual Count register */
#define VIRTIO_RTC_COUNTER_ARM_VIRT 0
/* Arm Generic Timer CNTPCT_EL0, Counter-timer Physical Count register */
#define VIRTIO_RTC_COUNTER_ARM_PHYS 1
But there might be no benefit distinguishing them, as per a discussion
on the RFC Linux kernel driver [4].
So unless somebody identifies some use in distinguishing Arm virtual and
physical counters, this distinction will be dropped in v4.
virtio-rtc Functionality for Net Device
---------------------------------------
There is interest into sharing virtio-rtc functionality with the
virtio-net device [6]. It is proposed to encapsulate the requests resp.
responses in the pseudo-fields command-specific-data resp.
command-specific-result.
This handles natural alignment of the payload in a way which is
compatible with
- existing virtio-net controlq commands, and
- with the standalone virtio-rtc messages.
Additional Information
======================
The spec does not specify how a driver should interpret clock readings,
esp. also not how to perform clock synchronization.
The draft uses the "Timer/Clock" device id which is already part of the
specification. This device id was registered a long time ago and should
be unused according to the author's information. The name "RTC" was
determined to be the best for a device which focuses on current time,
but is up to discussion.
Any feedback is appreciated very much.
Changelog
=========
v3:
- Address comments from Parav Pandit.
- Split off normative requirements into a second commit [2].
- Merge readq and controlq into requestq [3].
- Don't guard cross-timestamping with feature bit [3].
- Pad request headers to 64 bit [2].
- Rename Virtio status codes to match UNIX error names [2].
- Avoid Virtio status code clashes with net controlq ack values.
- Reword to refer more to "requests", rather than "messages" [2].
- Rephrase some sentences [2].
- Use integer data types without "__" prefixes [2].
- Reduce clock id width to 16 bits [5].
- Make VIRTIO_RTC_FLAG_CROSS_CAP a bit mask rather than a bit index.
v2:
- Address comments from Cornelia Huck.
- Add VIRTIO_RTC_M_CROSS_CAP message [1].
- Fix various minor issues and improve wording [1].
- Add several clarifications regarding device error statuses.
[1] https://lists.oasis-open.org/archives/virtio-comment/202304/msg00523.html
[2] https://lore.kernel.org/virtio-comment/b59a7dda-06fe-cff9-df61-b90aa4e50836@opensynergy.com/t/#mffb93800fea11d6dda9e151078abedd6ff1c0f1e
[3] https://lore.kernel.org/virtio-comment/b59a7dda-06fe-cff9-df61-b90aa4e50836@opensynergy.com/t/#m94efd0aa9b9c2b96a246b79ef8bfc3bf64ebe791
[4] https://lore.kernel.org/lkml/20230630171052.985577-1-peter.hilber@opensynergy.com/T/#m65fa1d715933360498c4e33d7225e4220215a9d6
[5] https://lore.kernel.org/virtio-comment/b59a7dda-06fe-cff9-df61-b90aa4e50836@opensynergy.com/t/#mf00ce330228c28556d735eb9597469048c5d8b62
[6] https://lore.kernel.org/virtio-comment/b59a7dda-06fe-cff9-df61-b90aa4e50836@opensynergy.com/t/#mf00ce330228c28556d735eb9597469048c5d8b62
Signed-off-by: Peter Hilber <peter.hilber@opensynergy.com>
---
content.tex | 3 +-
device-types/rtc/description.tex | 328 +++++++++++++++++++++++++++++++
2 files changed, 330 insertions(+), 1 deletion(-)
create mode 100644 device-types/rtc/description.tex
diff --git a/content.tex b/content.tex
index 98b9c319a666..f2b534ce97e6 100644
--- a/content.tex
+++ b/content.tex
@@ -683,7 +683,7 @@ \chapter{Device Types}\label{sec:Device Types}
\hline
16 & GPU device \\
\hline
-17 & Timer/Clock device \\
+17 & RTC (Timer/Clock) device \\
\hline
18 & Input device \\
\hline
@@ -769,6 +769,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/rtc/description.tex}
\chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
diff --git a/device-types/rtc/description.tex b/device-types/rtc/description.tex
new file mode 100644
index 000000000000..abfa2206894e
--- /dev/null
+++ b/device-types/rtc/description.tex
@@ -0,0 +1,328 @@
+\section{RTC Device}\label{sec:Device Types / RTC Device}
+
+The RTC (Real Time Clock) device provides information about current
+time. The device can provide different clocks, e.g.\ for the UTC or TAI
+time standards, or for physical time elapsed since some past epoch. The
+driver can read the clocks with simple or more accurate methods.
+
+\subsection{Device ID}\label{sec:Device Types / RTC Device / Device ID}
+
+17
+
+\subsection{Virtqueues}\label{sec:Device Types / RTC Device / Virtqueues}
+
+\begin{description}
+\item[0] requestq
+\end{description}
+
+The driver enqueues requests to the requestq.
+
+\subsection{Feature bits}\label{sec:Device Types / RTC Device / Feature bits}
+
+No device-specific feature bits are defined yet.
+
+\subsection{Device configuration layout}\label{sec:Device Types / RTC Device / Device configuration layout}
+
+There is no configuration data for the device.
+
+\subsection{Device Initialization}\label{sec:Device Types / RTC Device / Device Initialization}
+
+The device determines the set of clocks. The device can provide zero or
+more clocks.
+
+\subsection{Device Operation}\label{sec:Device Types / RTC Device / Device Operation}
+
+For the requestq, the driver sends a message with a request, and
+receives the response in the device-writable part of the message. The
+requestq uses common request and response headers.
+
+\begin{lstlisting}
+/* common request header */
+struct virtio_rtc_req_head {
+ le16 msg_type;
+ u8 reserved[6];
+};
+
+/* common response header */
+struct virtio_rtc_resp_head {
+ u8 status;
+ u8 reserved[7];
+};
+\end{lstlisting}
+
+The \field{msg_type} field identifies the message type.
+
+The \field{status} field indicates whether the device successfully
+executed the request. The device sets the \field{status} field to one of
+the following values:
+
+\begin{lstlisting}
+#define VIRTIO_RTC_S_OK 0
+#define VIRTIO_RTC_S_EOPNOTSUPP 2
+#define VIRTIO_RTC_S_ENODEV 3
+#define VIRTIO_RTC_S_EINVAL 4
+#define VIRTIO_RTC_S_EIO 5
+\end{lstlisting}
+
+VIRTIO_RTC_S_OK indicates that the device successfully executed the
+request.
+
+If \field{status} is not VIRTIO_RTC_S_OK, the value of other response
+fields is undefined.
+
+VIRTIO_RTC_S_EOPNOTSUPP indicates that the device could not execute the
+specific request due to an implementation limitation. The device also
+returns status VIRTIO_RTC_S_EOPNOTSUPP for requests with unknown values
+in the fields \field{msg_type} or \field{hw_counter}.
+
+VIRTIO_RTC_S_ENODEV indicates that the \field{clock_id} field value
+supplied with the request does not identify a clock.
+
+VIRTIO_RTC_S_EINVAL indicates that
+
+\begin{itemize}
+\item the driver request values are not allowed by the specification or
+\item the device read-only part of the message, or device write-only
+ part of the message, is too small.
+\end{itemize}
+
+VIRTIO_RTC_S_EIO indicates that the device did not execute the request
+due to an error which was not caused by invalid input from the driver.
+
+All \field{reserved} fields are written as zero, and their value is
+ignored.
+
+The set of clocks does not change after feature negotiation completion,
+until device reset. The set of clocks should not change on device reset
+either (similar to negotiated features). Clock identifiers are
+zero-based, dense indices. All fields named \field{clock_id} contain
+clock identifiers.
+
+\subsubsection{Control Requests}\label{sec:Device Types / RTC Device / Device Operation / Control Requests}
+
+Through \emph{control requests}, the driver requests information about
+the device capabilities. The driver enqueues control requests in the
+requestq.
+
+\begin{description}
+
+\item[VIRTIO_RTC_REQ_CFG] discovers the number of clocks.
+
+\begin{lstlisting}
+#define VIRTIO_RTC_REQ_CFG 0x1000 /* message type */
+
+struct virtio_rtc_req_cfg {
+ struct virtio_rtc_req_head head;
+ /* no request params */
+};
+
+struct virtio_rtc_resp_cfg {
+ struct virtio_rtc_resp_head head;
+ le16 num_clocks;
+ u8 reserved[6];
+};
+\end{lstlisting}
+
+Field \field{num_clocks} contains the number of clocks. A device
+provides zero or more clocks. Valid clock ids are those smaller than
+\field{num_clocks}.
+
+\item[VIRTIO_RTC_REQ_CLOCK_CAP] discovers the capabilities of the clock
+identified by the \field{clock_id} field.
+
+\begin{lstlisting}
+#define VIRTIO_RTC_REQ_CLOCK_CAP 0x1001 /* message type */
+
+struct virtio_rtc_req_clock_cap {
+ struct virtio_rtc_req_head head;
+ le16 clock_id;
+ u8 reserved[6];
+};
+
+struct virtio_rtc_resp_clock_cap {
+ struct virtio_rtc_resp_head head;
+ le16 type;
+ u8 reserved[6];
+};
+\end{lstlisting}
+
+The \field{type} field identifies the clock type. A device provides
+zero or more clocks for a clock type. The following clock types are
+defined:
+
+\begin{lstlisting}
+#define VIRTIO_RTC_CLOCK_UTC 0
+#define VIRTIO_RTC_CLOCK_TAI 1
+#define VIRTIO_RTC_CLOCK_MONO 2
+\end{lstlisting}
+
+VIRTIO_RTC_CLOCK_UTC uses the UTC (Coordinated Universal Time) time
+standard. The device uses the time epoch of January 1, 1970, 00:00
+UTC. This is the same epoch as \emph{Unix time}.
+
+VIRTIO_RTC_CLOCK_TAI uses the TAI (International Atomic Time) time
+standard. The device uses the time epoch of January 1, 1970, 00:00
+TAI.
+
+VIRTIO_RTC_CLOCK_MONO uses monotonic physical time (SI seconds
+subdivisions) since some unspecified epoch. The VIRTIO_RTC_CLOCK_MONO
+epoch is before or during device reset.
+
+Additional clock types may be standardized in the future.
+Implementation-specific clock type definitions are not recommended and
+use a clock type id greater than or equal to 0xF000.
+
+\item[VIRTIO_RTC_REQ_CROSS_CAP] discovers whether the device supports
+cross-timestamping for a particular pair of clock and hardware counter.
+
+\begin{lstlisting}
+#define VIRTIO_RTC_REQ_CROSS_CAP 0x1002 /* message type */
+
+struct virtio_rtc_req_cross_cap {
+ struct virtio_rtc_req_head head;
+ le16 clock_id;
+ le16 hw_counter;
+ u8 reserved[4];
+};
+
+#define VIRTIO_RTC_FLAG_CROSS_CAP (1 << 0)
+
+struct virtio_rtc_resp_cross_cap {
+ struct virtio_rtc_resp_head head;
+ u8 flags;
+ u8 reserved[7];
+};
+\end{lstlisting}
+
+The \field{clock_id} field identifies the clock, and the
+\field{hw_counter} field identifies the hardware counter, for which
+cross-timestamp support is probed. The device sets flag
+\field{VIRTIO_RTC_FLAG_CROSS_CAP} in the \field{flags} field if the
+clock supports cross-timestamping for the particular clock and hardware
+counter, and clears the flag otherwise.
+
+\end{description}
+
+\subsubsection{Read Requests}\label{sec:Device Types / RTC Device / Device Operation / Read Requests}
+
+Through \emph{read requests}, the driver requests clock readings from
+the device. The driver enqueues read requests in the requestq. The
+device obtains device-side clock readings and forwards these clock
+readings to the driver. The driver may enhance and interpret the clock
+readings through methods which are beyond the scope of this
+specification.
+
+Once DRIVER_OK has been set, the device should support reading every
+clock, even when a clock may yet have to be aligned to reference time
+sources.
+
+In general,
+
+\begin{itemize}
+\item clocks may jump backwards or forward, and
+\item the clock frequency may change. Clocks may be \emph{slewed},
+ i.e.\ clocks may run at a frequency other than their current
+ best frequency estimate.
+\end{itemize}
+
+E.g., a clock of type VIRTIO_RTC_CLOCK_UTC can insert UTC leap seconds.
+
+As long as a clock does not jump backwards, the driver clock readings
+increase monotonically:
+
+\begin{itemize}
+\item As long as a clock does not jump backwards in-between device-side
+ clock readings, the driver-side readings for that clock increase
+ monotonically as well, in the order in which the driver
+ marks read requests as available.
+
+\item The device marks read requests for the same clock as used in
+ the order in which the messages were marked as available.
+\end{itemize}
+
+For a clock of type VIRTIO_RTC_CLOCK_MONO, the device always returns
+monotonically increasing clock readings.
+
+The unit of all \field{clock_reading} fields is 1 nanosecond for all
+clock types which are part of the device specification so
+far.\footnote{For time epochs in year 1970 or later, this means that
+time until at least year 2553 can be represented in the \field{le64
+clock_reading} fields.}
+
+\begin{description}
+
+\item[VIRTIO_RTC_REQ_READ] reads the clock identified by the
+\field{clock_id} field. The device supports this message for every
+clock.
+
+\begin{lstlisting}
+#define VIRTIO_RTC_REQ_READ 0x0001 /* message type */
+
+struct virtio_rtc_req_read {
+ struct virtio_rtc_req_head head;
+ le16 clock_id;
+ u8 reserved[6];
+};
+
+struct virtio_rtc_resp_read {
+ struct virtio_rtc_resp_head head;
+ le64 clock_reading;
+};
+\end{lstlisting}
+
+\field{clock_reading} is a device-side clock reading obtained after the
+message was marked as available.
+
+\item[VIRTIO_RTC_REQ_READ_CROSS] returns a cross-timestamp for the clock
+identified by the \field{clock_id} field.\footnote{Cross-timestamping
+is similar to the ptp_kvm mechanism in the Linux kernel.} This message
+may yield better performance than using VIRTIO_RTC_REQ_READ.
+
+\begin{lstlisting}
+#define VIRTIO_RTC_REQ_READ_CROSS 0x0002 /* message type */
+
+struct virtio_rtc_req_read_cross {
+ struct virtio_rtc_req_head head;
+ le16 clock_id;
+ le16 hw_counter;
+ u8 reserved[4];
+};
+
+struct virtio_rtc_resp_read_cross {
+ struct virtio_rtc_resp_head head;
+ le64 clock_reading;
+ le64 counter_cycles;
+};
+\end{lstlisting}
+
+The \field{hw_counter} field specifies the hardware counter for which
+the driver requests a cross-timestamp.
+
+Cross-timestamping returns a \field{clock_reading}, and an associated
+hardware counter value, \field{counter_cycles}. The
+\field{counter_cycles} field is the approximate or precise value which
+the driver would have read at the \field{clock_reading} time instant
+from the hardware counter identified by \field{hw_counter}. How the
+device determines the value which the driver would have seen is beyond
+the scope of this specification.
+
+The following hardware counter identifiers are specified:
+
+\begin{lstlisting}
+/* Arm Generic Timer CNTVCT_EL0, Counter-timer Virtual Count register */
+#define VIRTIO_RTC_COUNTER_ARM_VIRT 0
+/* Arm Generic Timer CNTPCT_EL0, Counter-timer Physical Count register */
+#define VIRTIO_RTC_COUNTER_ARM_PHYS 1
+/* x86 Time-Stamp Counter */
+#define VIRTIO_RTC_COUNTER_X86_TSC 2
+\end{lstlisting}
+
+Additional hardware counter identifiers may be standardized in the
+future. Implementation-specific hardware counter identifiers are not
+recommended and are greater than or equal to 0xF000.
+
+The driver can determine whether the device supports
+VIRTIO_RTC_REQ_READ_CROSS for a specific clock and \field{hw_counter}
+through VIRTIO_RTC_REQ_CROSS_CAP.
+
+\end{description}
--
2.40.1
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
^ permalink raw reply related [flat|nested] 24+ messages in thread* [virtio-comment] RE: [RFC PATCH v3 1/4] virtio-rtc: Add initial device specification
2023-12-18 6:42 ` [virtio-comment] [RFC PATCH v3 1/4] virtio-rtc: Add initial " Peter Hilber
@ 2024-01-20 10:07 ` Parav Pandit
2024-01-24 15:39 ` [virtio-comment] " Peter Hilber
0 siblings, 1 reply; 24+ messages in thread
From: Parav Pandit @ 2024-01-20 10:07 UTC (permalink / raw)
To: Peter Hilber, virtio-comment@lists.oasis-open.org; +Cc: Cornelia Huck
Hi Peter,
> From: Peter Hilber <peter.hilber@opensynergy.com>
> Sent: Monday, December 18, 2023 12:13 PM
>
> This is a draft for a Real-Time Clock (RTC) device. The virtio-rtc device
> provides information about current time through one or more clocks.
>
> The normative statements for this device follow in the next patch.
>
> For this device, there is an RFC Linux kernel driver which is being
> upstreamed, and a proprietary device implementation.
>
> Open Questions
> ==============
>
> Arm Generic Timer Clock Types
> -----------------------------
>
> This spec version still distinguishes the Arm Generic Timer virtual and
> physical counters:
>
> /* Arm Generic Timer CNTVCT_EL0, Counter-timer Virtual Count
> register */
> #define VIRTIO_RTC_COUNTER_ARM_VIRT 0
> /* Arm Generic Timer CNTPCT_EL0, Counter-timer Physical Count
> register */
> #define VIRTIO_RTC_COUNTER_ARM_PHYS 1
>
> But there might be no benefit distinguishing them, as per a discussion on the
> RFC Linux kernel driver [4].
>
> So unless somebody identifies some use in distinguishing Arm virtual and
> physical counters, this distinction will be dropped in v4.
>
> virtio-rtc Functionality for Net Device
> ---------------------------------------
>
> There is interest into sharing virtio-rtc functionality with the virtio-net device
> [6]. It is proposed to encapsulate the requests resp.
> responses in the pseudo-fields command-specific-data resp.
> command-specific-result.
>
> This handles natural alignment of the payload in a way which is compatible
> with
>
> - existing virtio-net controlq commands, and
>
> - with the standalone virtio-rtc messages.
>
> Additional Information
> ======================
>
> The spec does not specify how a driver should interpret clock readings, esp.
> also not how to perform clock synchronization.
>
> The draft uses the "Timer/Clock" device id which is already part of the
> specification. This device id was registered a long time ago and should be
> unused according to the author's information. The name "RTC" was
> determined to be the best for a device which focuses on current time, but is
> up to discussion.
>
> Any feedback is appreciated very much.
>
> Changelog
> =========
>
> v3:
>
> - Address comments from Parav Pandit.
> - Split off normative requirements into a second commit [2].
> - Merge readq and controlq into requestq [3].
> - Don't guard cross-timestamping with feature bit [3].
> - Pad request headers to 64 bit [2].
> - Rename Virtio status codes to match UNIX error names [2].
> - Avoid Virtio status code clashes with net controlq ack values.
> - Reword to refer more to "requests", rather than "messages" [2].
> - Rephrase some sentences [2].
> - Use integer data types without "__" prefixes [2].
> - Reduce clock id width to 16 bits [5].
> - Make VIRTIO_RTC_FLAG_CROSS_CAP a bit mask rather than a bit index.
>
> v2:
>
> - Address comments from Cornelia Huck.
> - Add VIRTIO_RTC_M_CROSS_CAP message [1].
> - Fix various minor issues and improve wording [1].
> - Add several clarifications regarding device error statuses.
>
> [1] https://lists.oasis-open.org/archives/virtio-
> comment/202304/msg00523.html
> [2] https://lore.kernel.org/virtio-comment/b59a7dda-06fe-cff9-df61-
> b90aa4e50836@opensynergy.com/t/#mffb93800fea11d6dda9e151078abedd
> 6ff1c0f1e
> [3] https://lore.kernel.org/virtio-comment/b59a7dda-06fe-cff9-df61-
> b90aa4e50836@opensynergy.com/t/#m94efd0aa9b9c2b96a246b79ef8bfc3bf
> 64ebe791
> [4] https://lore.kernel.org/lkml/20230630171052.985577-1-
> peter.hilber@opensynergy.com/T/#m65fa1d715933360498c4e33d7225e4220
> 215a9d6
> [5] https://lore.kernel.org/virtio-comment/b59a7dda-06fe-cff9-df61-
> b90aa4e50836@opensynergy.com/t/#mf00ce330228c28556d735eb9597469
> 048c5d8b62
> [6] https://lore.kernel.org/virtio-comment/b59a7dda-06fe-cff9-df61-
> b90aa4e50836@opensynergy.com/t/#mf00ce330228c28556d735eb9597469
> 048c5d8b62
>
> Signed-off-by: Peter Hilber <peter.hilber@opensynergy.com>
> ---
> content.tex | 3 +-
> device-types/rtc/description.tex | 328 +++++++++++++++++++++++++++++++
> 2 files changed, 330 insertions(+), 1 deletion(-) create mode 100644 device-
> types/rtc/description.tex
>
> diff --git a/content.tex b/content.tex
> index 98b9c319a666..f2b534ce97e6 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -683,7 +683,7 @@ \chapter{Device Types}\label{sec:Device Types}
> \hline
> 16 & GPU device \\
> \hline
> -17 & Timer/Clock device \\
> +17 & RTC (Timer/Clock) device \\
> \hline
> 18 & Input device \\
> \hline
> @@ -769,6 +769,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/rtc/description.tex}
>
> \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
>
> diff --git a/device-types/rtc/description.tex b/device-types/rtc/description.tex
> new file mode 100644
> index 000000000000..abfa2206894e
> --- /dev/null
> +++ b/device-types/rtc/description.tex
> @@ -0,0 +1,328 @@
> +\section{RTC Device}\label{sec:Device Types / RTC Device}
> +
> +The RTC (Real Time Clock) device provides information about current
> +time. The device can provide different clocks, e.g.\ for the UTC or TAI
> +time standards, or for physical time elapsed since some past epoch. The
> +driver can read the clocks with simple or more accurate methods.
> +
> +\subsection{Device ID}\label{sec:Device Types / RTC Device / Device ID}
> +
> +17
> +
> +\subsection{Virtqueues}\label{sec:Device Types / RTC Device /
> +Virtqueues}
> +
> +\begin{description}
> +\item[0] requestq
> +\end{description}
> +
> +The driver enqueues requests to the requestq.
> +
> +\subsection{Feature bits}\label{sec:Device Types / RTC Device / Feature
> +bits}
> +
> +No device-specific feature bits are defined yet.
> +
> +\subsection{Device configuration layout}\label{sec:Device Types / RTC
> +Device / Device configuration layout}
> +
> +There is no configuration data for the device.
> +
> +\subsection{Device Initialization}\label{sec:Device Types / RTC Device
> +/ Device Initialization}
> +
> +The device determines the set of clocks. The device can provide zero or
> +more clocks.
> +
Now that we bring the generic basic facilities to define device capabilities and resources in [1],
Num_clocks can be structured a clock resources.
Please refer to max_limits in [1] as capabilities.
[1] https://lists.oasis-open.org/archives/virtio-comment/202401/msg00095.html
We should bring a rtc capabilities.
> +\subsection{Device Operation}\label{sec:Device Types / RTC Device /
> +Device Operation}
> +
> +For the requestq, the driver sends a message with a request, and
> +receives the response in the device-writable part of the message. The
> +requestq uses common request and response headers.
> +
Now without defining any rtc specific structure, we benefit from generic structure.
Only two things needed.
1. define cap identifier
2. define cap structure
> +\begin{lstlisting}
> +/* common request header */
> +struct virtio_rtc_req_head {
> + le16 msg_type;
> + u8 reserved[6];
> +};
> +
> +/* common response header */
> +struct virtio_rtc_resp_head {
> + u8 status;
> + u8 reserved[7];
> +};
> +\end{lstlisting}
> +
> +The \field{msg_type} field identifies the message type.
> +
> +The \field{status} field indicates whether the device successfully
> +executed the request. The device sets the \field{status} field to one
> +of the following values:
> +
> +\begin{lstlisting}
> +#define VIRTIO_RTC_S_OK 0
> +#define VIRTIO_RTC_S_EOPNOTSUPP 2
> +#define VIRTIO_RTC_S_ENODEV 3
> +#define VIRTIO_RTC_S_EINVAL 4
> +#define VIRTIO_RTC_S_EIO 5
> +\end{lstlisting}
> +
> +VIRTIO_RTC_S_OK indicates that the device successfully executed the
> +request.
> +
> +If \field{status} is not VIRTIO_RTC_S_OK, the value of other response
> +fields is undefined.
> +
> +VIRTIO_RTC_S_EOPNOTSUPP indicates that the device could not execute
> the
> +specific request due to an implementation limitation. The device also
> +returns status VIRTIO_RTC_S_EOPNOTSUPP for requests with unknown
> values
> +in the fields \field{msg_type} or \field{hw_counter}.
> +
> +VIRTIO_RTC_S_ENODEV indicates that the \field{clock_id} field value
> +supplied with the request does not identify a clock.
> +
> +VIRTIO_RTC_S_EINVAL indicates that
> +
> +\begin{itemize}
> +\item the driver request values are not allowed by the specification or
> +\item the device read-only part of the message, or device write-only
> + part of the message, is too small.
> +\end{itemize}
> +
> +VIRTIO_RTC_S_EIO indicates that the device did not execute the request
> +due to an error which was not caused by invalid input from the driver.
> +
> +All \field{reserved} fields are written as zero, and their value is
> +ignored.
> +
> +The set of clocks does not change after feature negotiation completion,
> +until device reset. The set of clocks should not change on device reset
> +either (similar to negotiated features). Clock identifiers are
> +zero-based, dense indices. All fields named \field{clock_id} contain
> +clock identifiers.
> +
> +\subsubsection{Control Requests}\label{sec:Device Types / RTC Device /
> +Device Operation / Control Requests}
> +
> +Through \emph{control requests}, the driver requests information about
> +the device capabilities. The driver enqueues control requests in the
> +requestq.
> +
> +\begin{description}
> +
> +\item[VIRTIO_RTC_REQ_CFG] discovers the number of clocks.
> +
> +\begin{lstlisting}
> +#define VIRTIO_RTC_REQ_CFG 0x1000 /* message type */
> +
> +struct virtio_rtc_req_cfg {
> + struct virtio_rtc_req_head head;
> + /* no request params */
> +};
> +
> +struct virtio_rtc_resp_cfg {
> + struct virtio_rtc_resp_head head;
> + le16 num_clocks;
> + u8 reserved[6];
> +};
> +\end{lstlisting}
> +
> +Field \field{num_clocks} contains the number of clocks. A device
> +provides zero or more clocks. Valid clock ids are those smaller than
> +\field{num_clocks}.
> +
> +\item[VIRTIO_RTC_REQ_CLOCK_CAP] discovers the capabilities of the clock
> +identified by the \field{clock_id} field.
> +
> +\begin{lstlisting}
> +#define VIRTIO_RTC_REQ_CLOCK_CAP 0x1001 /* message type */
> +
Each clock identified by unique resource (clock) identifier and this resource now has the attributes of time.
> +struct virtio_rtc_req_clock_cap {
> + struct virtio_rtc_req_head head;
> + le16 clock_id;
> + u8 reserved[6];
> +};
> +
> +struct virtio_rtc_resp_clock_cap {
> + struct virtio_rtc_resp_head head;
> + le16 type;
> + u8 reserved[6];
> +};
> +\end{lstlisting}
> +
> +The \field{type} field identifies the clock type. A device provides
> +zero or more clocks for a clock type. The following clock types are
> +defined:
> +
> +\begin{lstlisting}
> +#define VIRTIO_RTC_CLOCK_UTC 0
> +#define VIRTIO_RTC_CLOCK_TAI 1
> +#define VIRTIO_RTC_CLOCK_MONO 2
> +\end{lstlisting}
> +
> +VIRTIO_RTC_CLOCK_UTC uses the UTC (Coordinated Universal Time) time
> +standard. The device uses the time epoch of January 1, 1970, 00:00 UTC.
> +This is the same epoch as \emph{Unix time}.
> +
> +VIRTIO_RTC_CLOCK_TAI uses the TAI (International Atomic Time) time
> +standard. The device uses the time epoch of January 1, 1970, 00:00 TAI.
> +
> +VIRTIO_RTC_CLOCK_MONO uses monotonic physical time (SI seconds
> +subdivisions) since some unspecified epoch. The
> VIRTIO_RTC_CLOCK_MONO
> +epoch is before or during device reset.
> +
> +Additional clock types may be standardized in the future.
> +Implementation-specific clock type definitions are not recommended and
> +use a clock type id greater than or equal to 0xF000.
> +
> +\item[VIRTIO_RTC_REQ_CROSS_CAP] discovers whether the device
> supports
> +cross-timestamping for a particular pair of clock and hardware counter.
> +
> +\begin{lstlisting}
> +#define VIRTIO_RTC_REQ_CROSS_CAP 0x1002 /* message type */
> +
> +struct virtio_rtc_req_cross_cap {
> + struct virtio_rtc_req_head head;
> + le16 clock_id;
> + le16 hw_counter;
> + u8 reserved[4];
> +};
> +
> +#define VIRTIO_RTC_FLAG_CROSS_CAP (1 << 0)
> +
> +struct virtio_rtc_resp_cross_cap {
> + struct virtio_rtc_resp_head head;
> + u8 flags;
> + u8 reserved[7];
> +};
> +\end{lstlisting}
> +
> +The \field{clock_id} field identifies the clock, and the
> +\field{hw_counter} field identifies the hardware counter, for which
> +cross-timestamp support is probed. The device sets flag
> +\field{VIRTIO_RTC_FLAG_CROSS_CAP} in the \field{flags} field if the
> +clock supports cross-timestamping for the particular clock and hardware
> +counter, and clears the flag otherwise.
> +
Hw_counter is also the resource attribute.
> +\end{description}
> +
> +\subsubsection{Read Requests}\label{sec:Device Types / RTC Device /
> +Device Operation / Read Requests}
> +
> +Through \emph{read requests}, the driver requests clock readings from
> +the device. The driver enqueues read requests in the requestq. The
> +device obtains device-side clock readings and forwards these clock
> +readings to the driver. The driver may enhance and interpret the clock
> +readings through methods which are beyond the scope of this
> +specification.
> +
Reading the resource using generic query resource command of [2] will return the time value.
So only time definition is needed in the resource.
[2] https://lists.oasis-open.org/archives/virtio-comment/202401/msg00096.html
> +Once DRIVER_OK has been set, the device should support reading every
> +clock, even when a clock may yet have to be aligned to reference time
> +sources.
> +
> +In general,
> +
> +\begin{itemize}
> +\item clocks may jump backwards or forward, and \item the clock
> +frequency may change. Clocks may be \emph{slewed},
> + i.e.\ clocks may run at a frequency other than their current
> + best frequency estimate.
> +\end{itemize}
> +
> +E.g., a clock of type VIRTIO_RTC_CLOCK_UTC can insert UTC leap seconds.
> +
> +As long as a clock does not jump backwards, the driver clock readings
> +increase monotonically:
> +
> +\begin{itemize}
> +\item As long as a clock does not jump backwards in-between device-side
> + clock readings, the driver-side readings for that clock increase
> + monotonically as well, in the order in which the driver
> + marks read requests as available.
> +
> +\item The device marks read requests for the same clock as used in
> + the order in which the messages were marked as available.
> +\end{itemize}
> +
> +For a clock of type VIRTIO_RTC_CLOCK_MONO, the device always returns
> +monotonically increasing clock readings.
> +
> +The unit of all \field{clock_reading} fields is 1 nanosecond for all
> +clock types which are part of the device specification so
> +far.\footnote{For time epochs in year 1970 or later, this means that
> +time until at least year 2553 can be represented in the \field{le64
> +clock_reading} fields.}
> +
> +\begin{description}
> +
> +\item[VIRTIO_RTC_REQ_READ] reads the clock identified by the
> +\field{clock_id} field. The device supports this message for every
> +clock.
> +
> +\begin{lstlisting}
> +#define VIRTIO_RTC_REQ_READ 0x0001 /* message type */
> +
> +struct virtio_rtc_req_read {
> + struct virtio_rtc_req_head head;
> + le16 clock_id;
> + u8 reserved[6];
> +};
> +
> +struct virtio_rtc_resp_read {
> + struct virtio_rtc_resp_head head;
> + le64 clock_reading;
> +};
> +\end{lstlisting}
> +
> +\field{clock_reading} is a device-side clock reading obtained after the
> +message was marked as available.
> +
> +\item[VIRTIO_RTC_REQ_READ_CROSS] returns a cross-timestamp for the
> +clock identified by the \field{clock_id}
> +field.\footnote{Cross-timestamping
> +is similar to the ptp_kvm mechanism in the Linux kernel.} This message
> +may yield better performance than using VIRTIO_RTC_REQ_READ.
> +
> +\begin{lstlisting}
> +#define VIRTIO_RTC_REQ_READ_CROSS 0x0002 /* message type */
> +
> +struct virtio_rtc_req_read_cross {
> + struct virtio_rtc_req_head head;
> + le16 clock_id;
> + le16 hw_counter;
> + u8 reserved[4];
> +};
> +
> +struct virtio_rtc_resp_read_cross {
> + struct virtio_rtc_resp_head head;
> + le64 clock_reading;
> + le64 counter_cycles;
> +};
> +\end{lstlisting}
> +
> +The \field{hw_counter} field specifies the hardware counter for which
> +the driver requests a cross-timestamp.
> +
> +Cross-timestamping returns a \field{clock_reading}, and an associated
> +hardware counter value, \field{counter_cycles}. The
> +\field{counter_cycles} field is the approximate or precise value which
> +the driver would have read at the \field{clock_reading} time instant
> +from the hardware counter identified by \field{hw_counter}. How the
> +device determines the value which the driver would have seen is beyond
> +the scope of this specification.
> +
> +The following hardware counter identifiers are specified:
> +
> +\begin{lstlisting}
> +/* Arm Generic Timer CNTVCT_EL0, Counter-timer Virtual Count register
> +*/ #define VIRTIO_RTC_COUNTER_ARM_VIRT 0
> +/* Arm Generic Timer CNTPCT_EL0, Counter-timer Physical Count register
> +*/ #define VIRTIO_RTC_COUNTER_ARM_PHYS 1
> +/* x86 Time-Stamp Counter */
> +#define VIRTIO_RTC_COUNTER_X86_TSC 2
> +\end{lstlisting}
> +
> +Additional hardware counter identifiers may be standardized in the
> +future. Implementation-specific hardware counter identifiers are not
> +recommended and are greater than or equal to 0xF000.
> +
> +The driver can determine whether the device supports
> +VIRTIO_RTC_REQ_READ_CROSS for a specific clock and \field{hw_counter}
> +through VIRTIO_RTC_REQ_CROSS_CAP.
> +
> +\end{description}
> --
> 2.40.1
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
^ permalink raw reply [flat|nested] 24+ messages in thread* [virtio-comment] Re: [RFC PATCH v3 1/4] virtio-rtc: Add initial device specification
2024-01-20 10:07 ` [virtio-comment] " Parav Pandit
@ 2024-01-24 15:39 ` Peter Hilber
2024-01-28 6:15 ` [virtio-comment] " Parav Pandit
0 siblings, 1 reply; 24+ messages in thread
From: Peter Hilber @ 2024-01-24 15:39 UTC (permalink / raw)
To: Parav Pandit, virtio-comment@lists.oasis-open.org; +Cc: Cornelia Huck
On 20.01.24 11:07, Parav Pandit wrote:
> Hi Peter,
>
>> From: Peter Hilber <peter.hilber@opensynergy.com>
>> Sent: Monday, December 18, 2023 12:13 PM
>>
>> This is a draft for a Real-Time Clock (RTC) device. The virtio-rtc device
>> provides information about current time through one or more clocks.
>>
>> The normative statements for this device follow in the next patch.
>>
>> For this device, there is an RFC Linux kernel driver which is being
>> upstreamed, and a proprietary device implementation.
>>
>> Open Questions
>> ==============
>>
>> Arm Generic Timer Clock Types
>> -----------------------------
>>
>> This spec version still distinguishes the Arm Generic Timer virtual and
>> physical counters:
>>
>> /* Arm Generic Timer CNTVCT_EL0, Counter-timer Virtual Count
>> register */
>> #define VIRTIO_RTC_COUNTER_ARM_VIRT 0
>> /* Arm Generic Timer CNTPCT_EL0, Counter-timer Physical Count
>> register */
>> #define VIRTIO_RTC_COUNTER_ARM_PHYS 1
>>
>> But there might be no benefit distinguishing them, as per a discussion on the
>> RFC Linux kernel driver [4].
>>
>> So unless somebody identifies some use in distinguishing Arm virtual and
>> physical counters, this distinction will be dropped in v4.
>>
>> virtio-rtc Functionality for Net Device
>> ---------------------------------------
>>
>> There is interest into sharing virtio-rtc functionality with the virtio-net device
>> [6]. It is proposed to encapsulate the requests resp.
>> responses in the pseudo-fields command-specific-data resp.
>> command-specific-result.
>>
>> This handles natural alignment of the payload in a way which is compatible
>> with
>>
>> - existing virtio-net controlq commands, and
>>
>> - with the standalone virtio-rtc messages.
>>
>> Additional Information
>> ======================
>>
>> The spec does not specify how a driver should interpret clock readings, esp.
>> also not how to perform clock synchronization.
>>
>> The draft uses the "Timer/Clock" device id which is already part of the
>> specification. This device id was registered a long time ago and should be
>> unused according to the author's information. The name "RTC" was
>> determined to be the best for a device which focuses on current time, but is
>> up to discussion.
>>
>> Any feedback is appreciated very much.
>>
>> Changelog
>> =========
>>
>> v3:
>>
>> - Address comments from Parav Pandit.
>> - Split off normative requirements into a second commit [2].
>> - Merge readq and controlq into requestq [3].
>> - Don't guard cross-timestamping with feature bit [3].
>> - Pad request headers to 64 bit [2].
>> - Rename Virtio status codes to match UNIX error names [2].
>> - Avoid Virtio status code clashes with net controlq ack values.
>> - Reword to refer more to "requests", rather than "messages" [2].
>> - Rephrase some sentences [2].
>> - Use integer data types without "__" prefixes [2].
>> - Reduce clock id width to 16 bits [5].
>> - Make VIRTIO_RTC_FLAG_CROSS_CAP a bit mask rather than a bit index.
>>
>> v2:
>>
>> - Address comments from Cornelia Huck.
>> - Add VIRTIO_RTC_M_CROSS_CAP message [1].
>> - Fix various minor issues and improve wording [1].
>> - Add several clarifications regarding device error statuses.
>>
>> [1] https://lists.oasis-open.org/archives/virtio-
>> comment/202304/msg00523.html
>> [2] https://lore.kernel.org/virtio-comment/b59a7dda-06fe-cff9-df61-
>> b90aa4e50836@opensynergy.com/t/#mffb93800fea11d6dda9e151078abedd
>> 6ff1c0f1e
>> [3] https://lore.kernel.org/virtio-comment/b59a7dda-06fe-cff9-df61-
>> b90aa4e50836@opensynergy.com/t/#m94efd0aa9b9c2b96a246b79ef8bfc3bf
>> 64ebe791
>> [4] https://lore.kernel.org/lkml/20230630171052.985577-1-
>> peter.hilber@opensynergy.com/T/#m65fa1d715933360498c4e33d7225e4220
>> 215a9d6
>> [5] https://lore.kernel.org/virtio-comment/b59a7dda-06fe-cff9-df61-
>> b90aa4e50836@opensynergy.com/t/#mf00ce330228c28556d735eb9597469
>> 048c5d8b62
>> [6] https://lore.kernel.org/virtio-comment/b59a7dda-06fe-cff9-df61-
>> b90aa4e50836@opensynergy.com/t/#mf00ce330228c28556d735eb9597469
>> 048c5d8b62
>>
>> Signed-off-by: Peter Hilber <peter.hilber@opensynergy.com>
>> ---
>> content.tex | 3 +-
>> device-types/rtc/description.tex | 328 +++++++++++++++++++++++++++++++
>> 2 files changed, 330 insertions(+), 1 deletion(-) create mode 100644 device-
>> types/rtc/description.tex
>>
>> diff --git a/content.tex b/content.tex
>> index 98b9c319a666..f2b534ce97e6 100644
>> --- a/content.tex
>> +++ b/content.tex
>> @@ -683,7 +683,7 @@ \chapter{Device Types}\label{sec:Device Types}
>> \hline
>> 16 & GPU device \\
>> \hline
>> -17 & Timer/Clock device \\
>> +17 & RTC (Timer/Clock) device \\
>> \hline
>> 18 & Input device \\
>> \hline
>> @@ -769,6 +769,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/rtc/description.tex}
>>
>> \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
>>
>> diff --git a/device-types/rtc/description.tex b/device-types/rtc/description.tex
>> new file mode 100644
>> index 000000000000..abfa2206894e
>> --- /dev/null
>> +++ b/device-types/rtc/description.tex
>> @@ -0,0 +1,328 @@
>> +\section{RTC Device}\label{sec:Device Types / RTC Device}
>> +
>> +The RTC (Real Time Clock) device provides information about current
>> +time. The device can provide different clocks, e.g.\ for the UTC or TAI
>> +time standards, or for physical time elapsed since some past epoch. The
>> +driver can read the clocks with simple or more accurate methods.
>> +
>> +\subsection{Device ID}\label{sec:Device Types / RTC Device / Device ID}
>> +
>> +17
>> +
>> +\subsection{Virtqueues}\label{sec:Device Types / RTC Device /
>> +Virtqueues}
>> +
>> +\begin{description}
>> +\item[0] requestq
>> +\end{description}
>> +
>> +The driver enqueues requests to the requestq.
>> +
>> +\subsection{Feature bits}\label{sec:Device Types / RTC Device / Feature
>> +bits}
>> +
>> +No device-specific feature bits are defined yet.
>> +
>> +\subsection{Device configuration layout}\label{sec:Device Types / RTC
>> +Device / Device configuration layout}
>> +
>> +There is no configuration data for the device.
>> +
>> +\subsection{Device Initialization}\label{sec:Device Types / RTC Device
>> +/ Device Initialization}
>> +
>> +The device determines the set of clocks. The device can provide zero or
>> +more clocks.
>> +
> Now that we bring the generic basic facilities to define device capabilities and resources in [1],
> Num_clocks can be structured a clock resources.
>
> Please refer to max_limits in [1] as capabilities.
> [1] https://lists.oasis-open.org/archives/virtio-comment/202401/msg00095.html
>
> We should bring a rtc capabilities.
>
Hi Parav,
thank you for the suggestions! I think that it is a good idea to
standardize message layouts. But I also think this is late and overly
complex to become the default RTC interface. In particular, OpenSynergy has
no plans at the moment to support administration virtqueues (and the newly
proposed device capabilities and device resources) in the RTC prototype
implementation.
At the moment I see the following problems with using device capabilities
and resources:
- Device capabilities and resources were initially proposed last week. It
remains to be seen how and when they will be accepted into the standard,
and when the Linux kernel driver will support them. Especially the
dynamic resource (de-)allocation seems a tricky topic to me.
By contrast, RTC was originally proposed 10 months ago, and has had an
open-source driver available since 7 months. In the past it was discussed
that the net device could be supported through the net controlq.
- Administration virtqueues are only defined for Virtio Over PCI Bus. Our
use cases also need Virtio Over MMIO, and maybe others in the future.
- The requests VIRTIO_RTC_REQ_CROSS_CAP, VIRTIO_RTC_REQ_READ_CROSS, and
likely any requests added in the future do not fit into the current
device resource pattern (as detailed below), so would need to get their
own virtqueue; or the admin virtqueue would need to allow device-specific
commands.
Also, alarms would need to be addressed, perhaps as dedicated resources.
- For the RTC device, using the device resources adds considerable
implementation overhead even for the simplest of implementations, with no
direct gain.
Both capabilities and resources can change dynamically. This would
necessitate a number of requirements for RTC how to deal with this. Some
extra generic requirements would likely also be required.
E.g. the device resources draft does not address how resource removal
would interact with resource utilization in other queues (e.g. alarmq).
The only direct improvement I see is that the driver can dynamically
indicate which clocks it wants to use, which is not present in RTC RFC
spec v3. However, resource-saving features such as negotiating
VIRTIO_RTC_REQ_READ_CROSS support (VIRTIO_RTC_F_READ_CROSS) were
explicitly requested to be dropped from the spec in the past.
- The admin virtqueue spec says:
It is the responsibility of the driver to ensure strict request
ordering for commands, because they will be consumed with no order
constraints.
But the RTC requirements mandate certain constraints for the RTC (clock)
read requests processing order (so that the driver will not see backwards
clock jumps where there are none).
Maybe there are more incompatibilities here; I didn't check this
thoroughly.
So I propose to stay with the current interface as default.
It should be possible to introduce the device capabilities and resources as
an alternative interface, protected with a feature bit. This feature bit
would then also omit the device-specific virtqueues. The alternative
interface would need to address at least some of the above issues, such as
how to dynamically create and destroy clocks. The alternative interface can
perhaps also omit some features which are not required by its users.
I added some extra comments below, but I think the above should be
discussed first.
Best regards,
Peter
[snip]
>> +\item[VIRTIO_RTC_REQ_CROSS_CAP] discovers whether the device
>> supports
>> +cross-timestamping for a particular pair of clock and hardware counter.
>> +
>> +\begin{lstlisting}
>> +#define VIRTIO_RTC_REQ_CROSS_CAP 0x1002 /* message type */
>> +
>> +struct virtio_rtc_req_cross_cap {
>> + struct virtio_rtc_req_head head;
>> + le16 clock_id;
>> + le16 hw_counter;
>> + u8 reserved[4];
>> +};
>> +
>> +#define VIRTIO_RTC_FLAG_CROSS_CAP (1 << 0)
>> +
>> +struct virtio_rtc_resp_cross_cap {
>> + struct virtio_rtc_resp_head head;
>> + u8 flags;
>> + u8 reserved[7];
>> +};
>> +\end{lstlisting}
>> +
>> +The \field{clock_id} field identifies the clock, and the
>> +\field{hw_counter} field identifies the hardware counter, for which
>> +cross-timestamp support is probed. The device sets flag
>> +\field{VIRTIO_RTC_FLAG_CROSS_CAP} in the \field{flags} field if the
>> +clock supports cross-timestamping for the particular clock and hardware
>> +counter, and clears the flag otherwise.
>> +
> Hw_counter is also the resource attribute.
>
But a clock may support multiple hw_counters; therefore the driver provides
the hw_counter in the request. So it would not fit into the
VIRTIO_ADMIN_CMD_RESOURCE_QUERY pattern out-of-the-box.
>> +\end{description}
>> +
>> +\subsubsection{Read Requests}\label{sec:Device Types / RTC Device /
>> +Device Operation / Read Requests}
>> +
>> +Through \emph{read requests}, the driver requests clock readings from
>> +the device. The driver enqueues read requests in the requestq. The
>> +device obtains device-side clock readings and forwards these clock
>> +readings to the driver. The driver may enhance and interpret the clock
>> +readings through methods which are beyond the scope of this
>> +specification.
>> +
> Reading the resource using generic query resource command of [2] will return the time value.
> So only time definition is needed in the resource.
> [2] https://lists.oasis-open.org/archives/virtio-comment/202401/msg00096.html
>
Treating current time as a resource attribute appears questionable. First,
it continuously changes, while other attributes don't. Second, it could
also be a slow operation when reading e.g. from a GNSS clock attached over
a slow interface.
Also, the query resource command does not fit read request
VIRTIO_RTC_REQ_READ_CROSS, which requires the hw_counter parameter, and
returns extra results.
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
^ permalink raw reply [flat|nested] 24+ messages in thread* [virtio-comment] RE: [RFC PATCH v3 1/4] virtio-rtc: Add initial device specification
2024-01-24 15:39 ` [virtio-comment] " Peter Hilber
@ 2024-01-28 6:15 ` Parav Pandit
2024-02-08 11:57 ` Peter Hilber
0 siblings, 1 reply; 24+ messages in thread
From: Parav Pandit @ 2024-01-28 6:15 UTC (permalink / raw)
To: Peter Hilber, virtio-comment@lists.oasis-open.org; +Cc: Cornelia Huck
> From: Peter Hilber <peter.hilber@opensynergy.com>
> Sent: Wednesday, January 24, 2024 9:09 PM
>
> On 20.01.24 11:07, Parav Pandit wrote:
> > Hi Peter,
> >
> >> From: Peter Hilber <peter.hilber@opensynergy.com>
> >> Sent: Monday, December 18, 2023 12:13 PM
> >>
> >> This is a draft for a Real-Time Clock (RTC) device. The virtio-rtc
> >> device provides information about current time through one or more
> clocks.
> >>
> >> The normative statements for this device follow in the next patch.
> >>
> >> For this device, there is an RFC Linux kernel driver which is being
> >> upstreamed, and a proprietary device implementation.
> >>
> >> Open Questions
> >> ==============
> >>
> >> Arm Generic Timer Clock Types
> >> -----------------------------
> >>
> >> This spec version still distinguishes the Arm Generic Timer virtual
> >> and physical counters:
> >>
> >> /* Arm Generic Timer CNTVCT_EL0, Counter-timer Virtual Count
> >> register */
> >> #define VIRTIO_RTC_COUNTER_ARM_VIRT 0
> >> /* Arm Generic Timer CNTPCT_EL0, Counter-timer Physical Count
> >> register */
> >> #define VIRTIO_RTC_COUNTER_ARM_PHYS 1
> >>
> >> But there might be no benefit distinguishing them, as per a
> >> discussion on the RFC Linux kernel driver [4].
> >>
> >> So unless somebody identifies some use in distinguishing Arm virtual
> >> and physical counters, this distinction will be dropped in v4.
> >>
> >> virtio-rtc Functionality for Net Device
> >> ---------------------------------------
> >>
> >> There is interest into sharing virtio-rtc functionality with the
> >> virtio-net device [6]. It is proposed to encapsulate the requests resp.
> >> responses in the pseudo-fields command-specific-data resp.
> >> command-specific-result.
> >>
> >> This handles natural alignment of the payload in a way which is
> >> compatible with
> >>
> >> - existing virtio-net controlq commands, and
> >>
> >> - with the standalone virtio-rtc messages.
> >>
> >> Additional Information
> >> ======================
> >>
> >> The spec does not specify how a driver should interpret clock readings,
> esp.
> >> also not how to perform clock synchronization.
> >>
> >> The draft uses the "Timer/Clock" device id which is already part of
> >> the specification. This device id was registered a long time ago and
> >> should be unused according to the author's information. The name
> >> "RTC" was determined to be the best for a device which focuses on
> >> current time, but is up to discussion.
> >>
> >> Any feedback is appreciated very much.
> >>
> >> Changelog
> >> =========
> >>
> >> v3:
> >>
> >> - Address comments from Parav Pandit.
> >> - Split off normative requirements into a second commit [2].
> >> - Merge readq and controlq into requestq [3].
> >> - Don't guard cross-timestamping with feature bit [3].
> >> - Pad request headers to 64 bit [2].
> >> - Rename Virtio status codes to match UNIX error names [2].
> >> - Avoid Virtio status code clashes with net controlq ack values.
> >> - Reword to refer more to "requests", rather than "messages" [2].
> >> - Rephrase some sentences [2].
> >> - Use integer data types without "__" prefixes [2].
> >> - Reduce clock id width to 16 bits [5].
> >> - Make VIRTIO_RTC_FLAG_CROSS_CAP a bit mask rather than a bit index.
> >>
> >> v2:
> >>
> >> - Address comments from Cornelia Huck.
> >> - Add VIRTIO_RTC_M_CROSS_CAP message [1].
> >> - Fix various minor issues and improve wording [1].
> >> - Add several clarifications regarding device error statuses.
> >>
> >> [1] https://lists.oasis-open.org/archives/virtio-
> >> comment/202304/msg00523.html
> >> [2] https://lore.kernel.org/virtio-comment/b59a7dda-06fe-cff9-df61-
> >>
> b90aa4e50836@opensynergy.com/t/#mffb93800fea11d6dda9e151078abedd
> >> 6ff1c0f1e
> >> [3] https://lore.kernel.org/virtio-comment/b59a7dda-06fe-cff9-df61-
> >>
> b90aa4e50836@opensynergy.com/t/#m94efd0aa9b9c2b96a246b79ef8bfc3bf
> >> 64ebe791
> >> [4] https://lore.kernel.org/lkml/20230630171052.985577-1-
> >>
> peter.hilber@opensynergy.com/T/#m65fa1d715933360498c4e33d7225e4220
> >> 215a9d6
> >> [5] https://lore.kernel.org/virtio-comment/b59a7dda-06fe-cff9-df61-
> >>
> b90aa4e50836@opensynergy.com/t/#mf00ce330228c28556d735eb9597469
> >> 048c5d8b62
> >> [6] https://lore.kernel.org/virtio-comment/b59a7dda-06fe-cff9-df61-
> >>
> b90aa4e50836@opensynergy.com/t/#mf00ce330228c28556d735eb9597469
> >> 048c5d8b62
> >>
> >> Signed-off-by: Peter Hilber <peter.hilber@opensynergy.com>
> >> ---
> >> content.tex | 3 +-
> >> device-types/rtc/description.tex | 328
> >> +++++++++++++++++++++++++++++++
> >> 2 files changed, 330 insertions(+), 1 deletion(-) create mode
> >> 100644 device- types/rtc/description.tex
> >>
> >> diff --git a/content.tex b/content.tex index
> >> 98b9c319a666..f2b534ce97e6 100644
> >> --- a/content.tex
> >> +++ b/content.tex
> >> @@ -683,7 +683,7 @@ \chapter{Device Types}\label{sec:Device Types}
> >> \hline
> >> 16 & GPU device \\
> >> \hline
> >> -17 & Timer/Clock device \\
> >> +17 & RTC (Timer/Clock) device \\
> >> \hline
> >> 18 & Input device \\
> >> \hline
> >> @@ -769,6 +769,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/rtc/description.tex}
> >>
> >> \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
> >>
> >> diff --git a/device-types/rtc/description.tex
> >> b/device-types/rtc/description.tex
> >> new file mode 100644
> >> index 000000000000..abfa2206894e
> >> --- /dev/null
> >> +++ b/device-types/rtc/description.tex
> >> @@ -0,0 +1,328 @@
> >> +\section{RTC Device}\label{sec:Device Types / RTC Device}
> >> +
> >> +The RTC (Real Time Clock) device provides information about current
> >> +time. The device can provide different clocks, e.g.\ for the UTC or
> >> +TAI time standards, or for physical time elapsed since some past
> >> +epoch. The driver can read the clocks with simple or more accurate
> methods.
> >> +
> >> +\subsection{Device ID}\label{sec:Device Types / RTC Device / Device
> >> +ID}
> >> +
> >> +17
> >> +
> >> +\subsection{Virtqueues}\label{sec:Device Types / RTC Device /
> >> +Virtqueues}
> >> +
> >> +\begin{description}
> >> +\item[0] requestq
> >> +\end{description}
> >> +
> >> +The driver enqueues requests to the requestq.
> >> +
> >> +\subsection{Feature bits}\label{sec:Device Types / RTC Device /
> >> +Feature bits}
> >> +
> >> +No device-specific feature bits are defined yet.
> >> +
> >> +\subsection{Device configuration layout}\label{sec:Device Types /
> >> +RTC Device / Device configuration layout}
> >> +
> >> +There is no configuration data for the device.
> >> +
> >> +\subsection{Device Initialization}\label{sec:Device Types / RTC
> >> +Device / Device Initialization}
> >> +
> >> +The device determines the set of clocks. The device can provide zero
> >> +or more clocks.
> >> +
> > Now that we bring the generic basic facilities to define device
> > capabilities and resources in [1], Num_clocks can be structured a clock
> resources.
> >
> > Please refer to max_limits in [1] as capabilities.
> > [1]
> > https://lists.oasis-open.org/archives/virtio-comment/202401/msg00095.h
> > tml
> >
> > We should bring a rtc capabilities.
> >
>
> Hi Parav,
>
> thank you for the suggestions! I think that it is a good idea to standardize
> message layouts. But I also think this is late and overly complex to become
> the default RTC interface.
It takes out the complexity or functionality that RTC tries to define and puts in generic infra.
Same was done for net device for its capabilities.
> In particular, OpenSynergy has no plans at the
> moment to support administration virtqueues (and the newly proposed
> device capabilities and device resources) in the RTC prototype
> implementation.
>
For RTC and any new facilities, I believe movement is not applicable because until now there wasn't anything.
> At the moment I see the following problems with using device capabilities
> and resources:
>
> - Device capabilities and resources were initially proposed last week. It
> remains to be seen how and when they will be accepted into the standard,
> and when the Linux kernel driver will support them.
The first user should be able to implement it.
> Especially the
> dynamic resource (de-)allocation seems a tricky topic to me.
>
Ok. Lets discuss.
Also implicit device resource is also to be considered which are kind of default resource exist on the device, which driver would not create/destroy.
> By contrast, RTC was originally proposed 10 months ago, and has had an
> open-source driver available since 7 months. In the past it was discussed
> that the net device could be supported through the net controlq.
>
I assume that driver is available for reference, prototype etc, purpose and not to use it in any kind of production environment before standardizing the spec.
Otherwise there is something fundamentally broken in the virtio spec standardization in 1.x era.
> - Administration virtqueues are only defined for Virtio Over PCI Bus. Our
> use cases also need Virtio Over MMIO, and maybe others in the future.
>
AQ are like any other virtqueues. So MMIO registers to expose AQ is straightforward.
Once you have it over PCI, this can be supported.
> - The requests VIRTIO_RTC_REQ_CROSS_CAP,
> VIRTIO_RTC_REQ_READ_CROSS, and
> likely any requests added in the future do not fit into the current
> device resource pattern (as detailed below), so would need to get their
> own virtqueue; or the admin virtqueue would need to allow device-specific
> commands.
>
Let me read more below.
> Also, alarms would need to be addressed, perhaps as dedicated resources.
>
If I understand correctly, alarms are created by the driver so it can be a resource.
I think the tricky part is persistent alarms across device reset.
Does the Linux driver clock has concept of persistent alarm? Or is this for some other OS?
I believe persistent alarms also falls in the implicit resource category to me.
> - For the RTC device, using the device resources adds considerable
> implementation overhead even for the simplest of implementations, with
> no
> direct gain.
>
Practically it is just structured in generic format than device specific requests and response headers defined in this patch.
> Both capabilities and resources can change dynamically. This would
> necessitate a number of requirements for RTC how to deal with this. Some
> extra generic requirements would likely also be required.
>
Device capabilities in RTC generally should not change dynamically within the device.
For example, following structs and fields which are defined in this patch belongs to capabilities.
struct virtio_rtc_resp_cfg.num_clocks.
Device resource:
Clock -> identified with the clock_id
Resource attributes (instead of cap, as resource attribute).
struct virtio_rtc_resp_clock_cap.type
struct virtio_rtc_req_cross_cap .hw_counter and flags.
Do we have M x N matching of M clocks and N counter counters?
In my view, it was M clocks and 1 hw counter of the cpu.
Can you please clarify?
> E.g. the device resources draft does not address how resource removal
> would interact with resource utilization in other queues (e.g. alarmq).
>
Good point.
We do have explicit statement added in the resources draft that,
+The device MUST complete the command with \field{status}
+VIRTIO_ADMIN_STATUS_EBUSY if the resource
+modified or destroyd is linked to other resource and if the device cannot perform
+the requested VIRTIO_ADMIN_CMD_RESOURCE_MODIFY or VIRTIO_ADMIN_CMD_RESOURCE_DESTROY
+commands.
In your case the other resource is the VQ (alarmq).
So one needs to destroy the alarmq before destroying the RTC clock resource (if it is explicit resource).
We should extend the resources definition to indicate other objects such as VQ (and not just linked resource).
> The only direct improvement I see is that the driver can dynamically
> indicate which clocks it wants to use, which is not present in RTC RFC
> spec v3. However, resource-saving features such as negotiating
> VIRTIO_RTC_REQ_READ_CROSS support (VIRTIO_RTC_F_READ_CROSS) were
> explicitly requested to be dropped from the spec in the past.
>
> - The admin virtqueue spec says:
>
> It is the responsibility of the driver to ensure strict request
> ordering for commands, because they will be consumed with no order
> constraints.
>
> But the RTC requirements mandate certain constraints for the RTC (clock)
> read requests processing order (so that the driver will not see backwards
> clock jumps where there are none).
>
Yes. above requirement to be met.
One easy option is for the driver to instruct this to the device explicitly to maintain the order.
Second option is to extend the spec for strict ordering for this new device type where driver ordering cannot be done.
> Maybe there are more incompatibilities here; I didn't check this
> thoroughly.
>
> So I propose to stay with the current interface as default.
>
> It should be possible to introduce the device capabilities and resources as an
> alternative interface, protected with a feature bit. This feature bit would then
> also omit the device-specific virtqueues. The alternative interface would need
> to address at least some of the above issues, such as how to dynamically
> create and destroy clocks. The alternative interface can perhaps also omit
> some features which are not required by its users.
>
> I added some extra comments below, but I think the above should be
> discussed first.
>
> Best regards,
>
> Peter
>
> [snip]
> >> +\item[VIRTIO_RTC_REQ_CROSS_CAP] discovers whether the device
> >> supports
> >> +cross-timestamping for a particular pair of clock and hardware counter.
> >> +
> >> +\begin{lstlisting}
> >> +#define VIRTIO_RTC_REQ_CROSS_CAP 0x1002 /* message type */
> >> +
> >> +struct virtio_rtc_req_cross_cap {
> >> + struct virtio_rtc_req_head head;
> >> + le16 clock_id;
> >> + le16 hw_counter;
> >> + u8 reserved[4];
> >> +};
> >> +
> >> +#define VIRTIO_RTC_FLAG_CROSS_CAP (1 << 0)
> >> +
> >> +struct virtio_rtc_resp_cross_cap {
> >> + struct virtio_rtc_resp_head head;
> >> + u8 flags;
> >> + u8 reserved[7];
> >> +};
> >> +\end{lstlisting}
> >> +
> >> +The \field{clock_id} field identifies the clock, and the
> >> +\field{hw_counter} field identifies the hardware counter, for which
> >> +cross-timestamp support is probed. The device sets flag
> >> +\field{VIRTIO_RTC_FLAG_CROSS_CAP} in the \field{flags} field if the
> >> +clock supports cross-timestamping for the particular clock and
> >> +hardware counter, and clears the flag otherwise.
> >> +
> > Hw_counter is also the resource attribute.
> >
>
> But a clock may support multiple hw_counters; therefore the driver provides
> the hw_counter in the request. So it would not fit into the
> VIRTIO_ADMIN_CMD_RESOURCE_QUERY pattern out-of-the-box.
>
Right. We should possibly be able to extend the QUERY command to accept device type specific input if the hw_counters are in huge quantities.
> >> +\end{description}
> >> +
> >> +\subsubsection{Read Requests}\label{sec:Device Types / RTC Device /
> >> +Device Operation / Read Requests}
> >> +
> >> +Through \emph{read requests}, the driver requests clock readings
> >> +from the device. The driver enqueues read requests in the requestq.
> >> +The device obtains device-side clock readings and forwards these
> >> +clock readings to the driver. The driver may enhance and interpret
> >> +the clock readings through methods which are beyond the scope of
> >> +this specification.
> >> +
> > Reading the resource using generic query resource command of [2] will
> return the time value.
> > So only time definition is needed in the resource.
> > [2]
> > https://lists.oasis-open.org/archives/virtio-comment/202401/msg00096.h
> > tml
> >
>
> Treating current time as a resource attribute appears questionable. First, it
> continuously changes, while other attributes don't.
It is a resource_specific_data. So it is free to continuously change.
A packet counter, flow counter etc similar examples, which will also have constantly changing value.
> Second, it could also be a
> slow operation when reading e.g. from a GNSS clock attached over a slow
> interface.
>
If I understand correctly, this has no effect on structuring it as a resource of some cvq command.
> Also, the query resource command does not fit read request
> VIRTIO_RTC_REQ_READ_CROSS, which requires the hw_counter parameter,
> and returns extra results.
Query resource command extension should be extended to accept this resource specific data apart from rid.
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [virtio-comment] RE: [RFC PATCH v3 1/4] virtio-rtc: Add initial device specification
2024-01-28 6:15 ` [virtio-comment] " Parav Pandit
@ 2024-02-08 11:57 ` Peter Hilber
2024-02-09 11:43 ` Cornelia Huck
0 siblings, 1 reply; 24+ messages in thread
From: Peter Hilber @ 2024-02-08 11:57 UTC (permalink / raw)
To: Parav Pandit, virtio-comment@lists.oasis-open.org; +Cc: Cornelia Huck
On 28.01.24 07:15, Parav Pandit wrote:
>
>
>> From: Peter Hilber <peter.hilber@opensynergy.com>
>> Sent: Wednesday, January 24, 2024 9:09 PM
>>
>> On 20.01.24 11:07, Parav Pandit wrote:
>>> Hi Peter,
>>>
>>>> From: Peter Hilber <peter.hilber@opensynergy.com>
>>>> Sent: Monday, December 18, 2023 12:13 PM
[snip]
>>>> +\subsection{Device Initialization}\label{sec:Device Types / RTC
>>>> +Device / Device Initialization}
>>>> +
>>>> +The device determines the set of clocks. The device can provide zero
>>>> +or more clocks.
>>>> +
>>> Now that we bring the generic basic facilities to define device
>>> capabilities and resources in [1], Num_clocks can be structured a clock
>> resources.
>>>
>>> Please refer to max_limits in [1] as capabilities.
>>> [1]
>>> https://lists.oasis-open.org/archives/virtio-comment/202401/msg00095.h
>>> tml
>>>
>>> We should bring a rtc capabilities.
>>>
>>
>> Hi Parav,
>>
>> thank you for the suggestions! I think that it is a good idea to standardize
>> message layouts. But I also think this is late and overly complex to become
>> the default RTC interface.
> It takes out the complexity or functionality that RTC tries to define and puts in generic infra.
> Same was done for net device for its capabilities.
>
I disagree about the complexity, and prefer to keep the RTC device without
admin virtqueues. Below I am making a proposal how RTC in the net device
could likely be used with a polished resources paradigm, without creating a
dependency for RTC standardization.
Using the resources commands as RTC commands would result in an interface
which is clearly more complex than the current interface. The resources
commands are not an adequate abstraction for all device functionality which
is not a net device data path.
The only advantage to the net device (not to the RTC device) would be that
its top level interfaces have a more unified look. My proposal 4) below
would retain this advantage. With the original resources sketch, the actual
RTC-specific interface would get more complicated for the net device as
well.
In general, virtio devices have arbitrary command types and it is possible
to add new command types through feature bits. Why should just 4 types of
commands (CREATE, MODIFY, QUERY, DESTROY) be good enough for RTC clocks
(and also not hinder RTC development in the future)? Any existing, and any
new, commands would be merged into artificial commands (typically QUERY).
The current RTC draft has at least 4 commands on a clock resource which
would need to be mapped to a single artificial QUERY command (CLOCK_CAP,
CROSS_CAP, READ, READ_CROSS).
Combining naturally distinct commands will increase the complexity for
specifying and implementing the command. Simple devices also do not
dynamically create or remove resources, so the resource paradigm is even
less useful here.
The many issues discussed below confirm that complexity actually increases,
and indicate that the resources proposal cannot be considered complete and
mature yet.
Also, admin virtqueues have explicitly been defined and implemented as a
Virtio Over PCI Bus specific interface. Therefore, they cannot be a
mandatory dependency of a general-purpose device.
I see four different options for the net device to have RTC commands:
1) Make the RTC device a member device of the net device.
2) Put the RTC requests into the controlq, as discussed previously.
3) Introduce new group administration commands which encapsulate the RTC
requests (similar to how it was discussed for the controlq).
4) Once the resource proposal is sufficiently complete and consistent,
encapsulate the RTC requests in resource QUERY, resource MODIFY, or
similar commands. This must be done in such a way as to not restrict
future extensions to the RTC device.
The QUERY and MODIFY commands then need to have both a command-specific
data and command-specific result.
Clock ID fields in the RTC requests would then correspond to
virtio_admin_cmd_resource_cmd_hdr.rid. The clock ID field types should
then not be smaller than the rid field.
The RTC requirements about writing to and reading from buffers appear to be
compatible with the admin virtqueue, except for the zero-padding of short
buffers in the admin virtqueue, which could also be added to RTC.
RTC could also reuse the status definitions of the admin virtqueue.
All four above options allow to standardize RTC without depending on the
resources standardization (and PoC) progress.
The latter two options should also allow putting RTC into the admin
virtqueue for the net device. For standalone use, all four use a
device-specific queue.
>> In particular, OpenSynergy has no plans at the
>> moment to support administration virtqueues (and the newly proposed
>> device capabilities and device resources) in the RTC prototype
>> implementation.
>>
> For RTC and any new facilities, I believe movement is not applicable because until now there wasn't anything.
>
I don't understand why this would be irrelevant information when discussing
about complexity and standardization progress.
>> At the moment I see the following problems with using device capabilities
>> and resources:
>>
>> - Device capabilities and resources were initially proposed last week. It
>> remains to be seen how and when they will be accepted into the standard,
>> and when the Linux kernel driver will support them.
> The first user should be able to implement it.
>
This comment was about the delay that using resources would imply on
standardizing and implementing the RTC device.
>> Especially the
>> dynamic resource (de-)allocation seems a tricky topic to me.
>>
> Ok. Lets discuss.
> Also implicit device resource is also to be considered which are kind of default resource exist on the device, which driver would not create/destroy.
>
This is not yet described in the resource proposal.
>> By contrast, RTC was originally proposed 10 months ago, and has had an
>> open-source driver available since 7 months. In the past it was discussed
>> that the net device could be supported through the net controlq.
>>
> I assume that driver is available for reference, prototype etc, purpose and not to use it in any kind of production environment before standardizing the spec.
> Otherwise there is something fundamentally broken in the virtio spec standardization in 1.x era.
>
I agree. But my point here is that proposing new non-trivial, draft stage,
transport-specific features as a prerequisite for a long proposed feature
(RTC) can unduly delay or even kill the long proposed feature.
What if e.g. capabilities and resources were now added to RTC, but in 10
months, somebody says, drop it, I have a supposedly better idea, which
cannot be conditional on a feature bit?
>> - Administration virtqueues are only defined for Virtio Over PCI Bus. Our
>> use cases also need Virtio Over MMIO, and maybe others in the future.
>>
> AQ are like any other virtqueues. So MMIO registers to expose AQ is straightforward.
> Once you have it over PCI, this can be supported.
>
I do not understand the previous sentence.
Admin virtqueues have explicitly been defined and implemented as a Virtio
Over PCI Bus specific interface. Therefore they cannot be a mandatory
dependency of a general-purpose, PCI-independent device.
>> - The requests VIRTIO_RTC_REQ_CROSS_CAP,
>> VIRTIO_RTC_REQ_READ_CROSS, and
>> likely any requests added in the future do not fit into the current
>> device resource pattern (as detailed below), so would need to get their
>> own virtqueue; or the admin virtqueue would need to allow device-specific
>> commands.
>>
> Let me read more below.
>
>> Also, alarms would need to be addressed, perhaps as dedicated resources.
>>
> If I understand correctly, alarms are created by the driver so it can be a resource.
> I think the tricky part is persistent alarms across device reset.
> Does the Linux driver clock has concept of persistent alarm? Or is this for some other OS?
Linux kernel RTC drivers report whether a (usually physical) RTC device
alarm is currently configured or not, which the Linux kernel queries i.a.
on boot or resume from sleep.
>
> I believe persistent alarms also falls in the implicit resource category to me.
>
>> - For the RTC device, using the device resources adds considerable
>> implementation overhead even for the simplest of implementations, with
>> no
>> direct gain.
>>
> Practically it is just structured in generic format than device specific requests and response headers defined in this patch.
>
As I will argue below, there will actually be a need to introduce
RTC-specific nested headers.
>> Both capabilities and resources can change dynamically. This would
>> necessitate a number of requirements for RTC how to deal with this. Some
>> extra generic requirements would likely also be required.
>>
> Device capabilities in RTC generally should not change dynamically within the device.
> For example, following structs and fields which are defined in this patch belongs to capabilities.
> struct virtio_rtc_resp_cfg.num_clocks.
As discussed, there is currently no use case for dynamic clock
creation/removal. But it should be possible to support this use case in the
future.
>
> Device resource:
> Clock -> identified with the clock_id
>
> Resource attributes (instead of cap, as resource attribute).
> struct virtio_rtc_resp_clock_cap.type
> struct virtio_rtc_req_cross_cap .hw_counter and flags.
I do not understand this comment. virtio_rtc_req_cross_cap is the request
from the driver, and .hw_counter is a field in the request. flags is part
of the response.
>
> Do we have M x N matching of M clocks and N counter counters?
> In my view, it was M clocks and 1 hw counter of the cpu.
> Can you please clarify?
>
As already discussed for patch v2 [8], it could be different HW counters,
e.g. TSC and Intel ART, or TSC and HPET, for the same virtio-rtc clock.
>> E.g. the device resources draft does not address how resource removal
>> would interact with resource utilization in other queues (e.g. alarmq).
>>
> Good point.
> We do have explicit statement added in the resources draft that,
> +The device MUST complete the command with \field{status}
> +VIRTIO_ADMIN_STATUS_EBUSY if the resource
> +modified or destroyd is linked to other resource and if the device cannot perform
> +the requested VIRTIO_ADMIN_CMD_RESOURCE_MODIFY or VIRTIO_ADMIN_CMD_RESOURCE_DESTROY
> +commands.
>
> In your case the other resource is the VQ (alarmq).
> So one needs to destroy the alarmq before destroying the RTC clock resource (if it is explicit resource).
>
> We should extend the resources definition to indicate other objects such as VQ (and not just linked resource).
>
So if there are two clocks which support alarm, neither can be destroyed
without the other because the alarmq depends on both? As a resource
management scheme, this does not make sense to me.
Also, dynamically creating/destroying virtqueues is adding even more
complexity, and not described in the resources proposal.
>> The only direct improvement I see is that the driver can dynamically
>> indicate which clocks it wants to use, which is not present in RTC RFC
>> spec v3. However, resource-saving features such as negotiating
>> VIRTIO_RTC_REQ_READ_CROSS support (VIRTIO_RTC_F_READ_CROSS) were
>> explicitly requested to be dropped from the spec in the past.
>>
>> - The admin virtqueue spec says:
>>
>> It is the responsibility of the driver to ensure strict request
>> ordering for commands, because they will be consumed with no order
>> constraints.
>>
>> But the RTC requirements mandate certain constraints for the RTC (clock)
>> read requests processing order (so that the driver will not see backwards
>> clock jumps where there are none).
>>
> Yes. above requirement to be met.
> One easy option is for the driver to instruct this to the device explicitly to maintain the order.
> Second option is to extend the spec for strict ordering for this new device type where driver ordering cannot be done.
>
I now also saw another, somewhat contradictory admin virtqueue requirement:
The device MUST process commands on a given admin virtqueue in
the order in which they are queued.
This is also a limitation, since it would e.g. force the device to finish
reading out a slow clock before reading a fast clock.
>> Maybe there are more incompatibilities here; I didn't check this
>> thoroughly.
[snip]
>>>> +\item[VIRTIO_RTC_REQ_CROSS_CAP] discovers whether the device
>>>> supports
>>>> +cross-timestamping for a particular pair of clock and hardware counter.
>>>> +
>>>> +\begin{lstlisting}
>>>> +#define VIRTIO_RTC_REQ_CROSS_CAP 0x1002 /* message type */
>>>> +
>>>> +struct virtio_rtc_req_cross_cap {
>>>> + struct virtio_rtc_req_head head;
>>>> + le16 clock_id;
>>>> + le16 hw_counter;
>>>> + u8 reserved[4];
>>>> +};
>>>> +
>>>> +#define VIRTIO_RTC_FLAG_CROSS_CAP (1 << 0)
>>>> +
>>>> +struct virtio_rtc_resp_cross_cap {
>>>> + struct virtio_rtc_resp_head head;
>>>> + u8 flags;
>>>> + u8 reserved[7];
>>>> +};
>>>> +\end{lstlisting}
>>>> +
>>>> +The \field{clock_id} field identifies the clock, and the
>>>> +\field{hw_counter} field identifies the hardware counter, for which
>>>> +cross-timestamp support is probed. The device sets flag
>>>> +\field{VIRTIO_RTC_FLAG_CROSS_CAP} in the \field{flags} field if the
>>>> +clock supports cross-timestamping for the particular clock and
>>>> +hardware counter, and clears the flag otherwise.
>>>> +
>>> Hw_counter is also the resource attribute.
>>>
>>
>> But a clock may support multiple hw_counters; therefore the driver provides
>> the hw_counter in the request. So it would not fit into the
>> VIRTIO_ADMIN_CMD_RESOURCE_QUERY pattern out-of-the-box.
>>
> Right. We should possibly be able to extend the QUERY command to accept device type specific input if the hw_counters are in huge quantities.
>
But is it then still one QUERY command, or is it effectively multiple
commands merged into one?
>>>> +\end{description}
>>>> +
>>>> +\subsubsection{Read Requests}\label{sec:Device Types / RTC Device /
>>>> +Device Operation / Read Requests}
>>>> +
>>>> +Through \emph{read requests}, the driver requests clock readings
>>>> +from the device. The driver enqueues read requests in the requestq.
>>>> +The device obtains device-side clock readings and forwards these
>>>> +clock readings to the driver. The driver may enhance and interpret
>>>> +the clock readings through methods which are beyond the scope of
>>>> +this specification.
>>>> +
>>> Reading the resource using generic query resource command of [2] will
>> return the time value.
>>> So only time definition is needed in the resource.
>>> [2]
>>> https://lists.oasis-open.org/archives/virtio-comment/202401/msg00096.h
>>> tml
>>>
>>
>> Treating current time as a resource attribute appears questionable. First, it
>> continuously changes, while other attributes don't.
> It is a resource_specific_data. So it is free to continuously change.
> A packet counter, flow counter etc similar examples, which will also have constantly changing value.
>
Every time the driver just wants to read the clock, the device is forced to
return diverse data it has on the resource as well.
Also, "resources: Add theory of operation for device resources" now says:
++Managing resources using administration commands using administration virtqueues
++are assumed to be slower operations compared to data path operations such as
++handling tranmitq or receiveq.
But it must be possible to implement reading the clock as a fast command.
>> Second, it could also be a
>> slow operation when reading e.g. from a GNSS clock attached over a slow
>> interface.
>>
> If I understand correctly, this has no effect on structuring it as a resource of some cvq command.
>
But on the performance during probe.
>> Also, the query resource command does not fit read request
>> VIRTIO_RTC_REQ_READ_CROSS, which requires the hw_counter parameter,
>> and returns extra results.
> Query resource command extension should be extended to accept this resource specific data apart from rid.
What if the device doesn't support cross-timestamping? What if the driver
is not actually interested into the cross-timestamp - does the device still
need to obtain it?
Thanks for the discussion,
Peter
[8] https://lore.kernel.org/all/b59a7dda-06fe-cff9-df61-b90aa4e50836@opensynergy.com/
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [virtio-comment] RE: [RFC PATCH v3 1/4] virtio-rtc: Add initial device specification
2024-02-08 11:57 ` Peter Hilber
@ 2024-02-09 11:43 ` Cornelia Huck
0 siblings, 0 replies; 24+ messages in thread
From: Cornelia Huck @ 2024-02-09 11:43 UTC (permalink / raw)
To: Peter Hilber, Parav Pandit, virtio-comment@lists.oasis-open.org
On Thu, Feb 08 2024, Peter Hilber <peter.hilber@opensynergy.com> wrote:
> On 28.01.24 07:15, Parav Pandit wrote:
>>
>>
>>> From: Peter Hilber <peter.hilber@opensynergy.com>
>>> Sent: Wednesday, January 24, 2024 9:09 PM
>>>
>>> On 20.01.24 11:07, Parav Pandit wrote:
>>>> Hi Peter,
>>>>
>>>>> From: Peter Hilber <peter.hilber@opensynergy.com>
>>>>> Sent: Monday, December 18, 2023 12:13 PM
>
> [snip]
>
>>>>> +\subsection{Device Initialization}\label{sec:Device Types / RTC
>>>>> +Device / Device Initialization}
>>>>> +
>>>>> +The device determines the set of clocks. The device can provide zero
>>>>> +or more clocks.
>>>>> +
>>>> Now that we bring the generic basic facilities to define device
>>>> capabilities and resources in [1], Num_clocks can be structured a clock
>>> resources.
>>>>
>>>> Please refer to max_limits in [1] as capabilities.
>>>> [1]
>>>> https://lists.oasis-open.org/archives/virtio-comment/202401/msg00095.h
>>>> tml
>>>>
>>>> We should bring a rtc capabilities.
>>>>
>>>
>>> Hi Parav,
>>>
>>> thank you for the suggestions! I think that it is a good idea to standardize
>>> message layouts. But I also think this is late and overly complex to become
>>> the default RTC interface.
>> It takes out the complexity or functionality that RTC tries to define and puts in generic infra.
>> Same was done for net device for its capabilities.
>>
>
> I disagree about the complexity, and prefer to keep the RTC device without
> admin virtqueues. Below I am making a proposal how RTC in the net device
> could likely be used with a polished resources paradigm, without creating a
> dependency for RTC standardization.
>
> Using the resources commands as RTC commands would result in an interface
> which is clearly more complex than the current interface. The resources
> commands are not an adequate abstraction for all device functionality which
> is not a net device data path.
>
> The only advantage to the net device (not to the RTC device) would be that
> its top level interfaces have a more unified look. My proposal 4) below
> would retain this advantage. With the original resources sketch, the actual
> RTC-specific interface would get more complicated for the net device as
> well.
>
> In general, virtio devices have arbitrary command types and it is possible
> to add new command types through feature bits. Why should just 4 types of
> commands (CREATE, MODIFY, QUERY, DESTROY) be good enough for RTC clocks
> (and also not hinder RTC development in the future)? Any existing, and any
> new, commands would be merged into artificial commands (typically QUERY).
> The current RTC draft has at least 4 commands on a clock resource which
> would need to be mapped to a single artificial QUERY command (CLOCK_CAP,
> CROSS_CAP, READ, READ_CROSS).
>
> Combining naturally distinct commands will increase the complexity for
> specifying and implementing the command. Simple devices also do not
> dynamically create or remove resources, so the resource paradigm is even
> less useful here.
>
> The many issues discussed below confirm that complexity actually increases,
> and indicate that the resources proposal cannot be considered complete and
> mature yet.
>
> Also, admin virtqueues have explicitly been defined and implemented as a
> Virtio Over PCI Bus specific interface. Therefore, they cannot be a
> mandatory dependency of a general-purpose device.
>
> I see four different options for the net device to have RTC commands:
>
> 1) Make the RTC device a member device of the net device.
>
> 2) Put the RTC requests into the controlq, as discussed previously.
>
> 3) Introduce new group administration commands which encapsulate the RTC
> requests (similar to how it was discussed for the controlq).
>
> 4) Once the resource proposal is sufficiently complete and consistent,
> encapsulate the RTC requests in resource QUERY, resource MODIFY, or
> similar commands. This must be done in such a way as to not restrict
> future extensions to the RTC device.
>
> The QUERY and MODIFY commands then need to have both a command-specific
> data and command-specific result.
>
> Clock ID fields in the RTC requests would then correspond to
> virtio_admin_cmd_resource_cmd_hdr.rid. The clock ID field types should
> then not be smaller than the rid field.
>
> The RTC requirements about writing to and reading from buffers appear to be
> compatible with the admin virtqueue, except for the zero-padding of short
> buffers in the admin virtqueue, which could also be added to RTC.
>
> RTC could also reuse the status definitions of the admin virtqueue.
>
> All four above options allow to standardize RTC without depending on the
> resources standardization (and PoC) progress.
>
> The latter two options should also allow putting RTC into the admin
> virtqueue for the net device. For standalone use, all four use a
> device-specific queue.
Thanks for the discussion and examples here, this helps.
>
>>> In particular, OpenSynergy has no plans at the
>>> moment to support administration virtqueues (and the newly proposed
>>> device capabilities and device resources) in the RTC prototype
>>> implementation.
>>>
>> For RTC and any new facilities, I believe movement is not applicable because until now there wasn't anything.
>>
>
> I don't understand why this would be irrelevant information when discussing
> about complexity and standardization progress.
>
>>> At the moment I see the following problems with using device capabilities
>>> and resources:
>>>
>>> - Device capabilities and resources were initially proposed last week. It
>>> remains to be seen how and when they will be accepted into the standard,
>>> and when the Linux kernel driver will support them.
>> The first user should be able to implement it.
>>
>
> This comment was about the delay that using resources would imply on
> standardizing and implementing the RTC device.
>
>>> Especially the
>>> dynamic resource (de-)allocation seems a tricky topic to me.
>>>
>> Ok. Lets discuss.
>> Also implicit device resource is also to be considered which are kind of default resource exist on the device, which driver would not create/destroy.
>>
>
> This is not yet described in the resource proposal.
>
>>> By contrast, RTC was originally proposed 10 months ago, and has had an
>>> open-source driver available since 7 months. In the past it was discussed
>>> that the net device could be supported through the net controlq.
>>>
>> I assume that driver is available for reference, prototype etc, purpose and not to use it in any kind of production environment before standardizing the spec.
>> Otherwise there is something fundamentally broken in the virtio spec standardization in 1.x era.
>>
>
> I agree. But my point here is that proposing new non-trivial, draft stage,
> transport-specific features as a prerequisite for a long proposed feature
> (RTC) can unduly delay or even kill the long proposed feature.
I agree -- we have something that looks close to being ready to go, and
another set of features that, while looking interesting, are nowhere
ready to be merged. I don't think we want to have the latter hold up the
former, when the complex feature still needs resources for review and
discussion _and_ can be added on top of the original device later on.
[In case you ask: I currently don't have resources myself to deal with
the admin/resource proposal. There's a reason why it just sits on my
todo list -- I'm focusing to keep at least the project per se running
with voting etc., and that's a stretch already -- apologies for the
permanent delays in that department...]
>
> What if e.g. capabilities and resources were now added to RTC, but in 10
> months, somebody says, drop it, I have a supposedly better idea, which
> cannot be conditional on a feature bit?
>
>>> - Administration virtqueues are only defined for Virtio Over PCI Bus. Our
>>> use cases also need Virtio Over MMIO, and maybe others in the future.
>>>
>> AQ are like any other virtqueues. So MMIO registers to expose AQ is straightforward.
>> Once you have it over PCI, this can be supported.
>>
>
> I do not understand the previous sentence.
>
> Admin virtqueues have explicitly been defined and implemented as a Virtio
> Over PCI Bus specific interface. Therefore they cannot be a mandatory
> dependency of a general-purpose, PCI-independent device.
We can, in theory, add support for other transports as well -- but that
needs someone to actually do it, aka resources again. I'd consider
support for this on the MMIO transport to be the bare minimum.
<snipping a bunch of interesting discussion>
I'd be in favour of addressing the smaller comments for this proposal
and then go ahead with it. We always can extend things later on.
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
^ permalink raw reply [flat|nested] 24+ messages in thread
* [virtio-comment] [RFC PATCH v3 2/4] virtio-rtc: Add initial normative statements
2023-12-18 6:42 [virtio-comment] [RFC PATCH v3 0/4] virtio-rtc: Add device specification Peter Hilber
2023-12-18 6:42 ` [virtio-comment] [RFC PATCH v3 1/4] virtio-rtc: Add initial " Peter Hilber
@ 2023-12-18 6:42 ` Peter Hilber
2023-12-18 6:42 ` [virtio-comment] [RFC PATCH v3 3/4] virtio-rtc: Add alarm feature Peter Hilber
` (2 subsequent siblings)
4 siblings, 0 replies; 24+ messages in thread
From: Peter Hilber @ 2023-12-18 6:42 UTC (permalink / raw)
To: virtio-comment; +Cc: Peter Hilber, Cornelia Huck, Parav Pandit
Add the normative statements for the initial device specification.
Signed-off-by: Peter Hilber <peter.hilber@opensynergy.com>
---
conformance.tex | 2 +
device-types/rtc/description.tex | 207 ++++++++++++++++++++++++
device-types/rtc/device-conformance.tex | 9 ++
device-types/rtc/driver-conformance.tex | 9 ++
4 files changed, 227 insertions(+)
create mode 100644 device-types/rtc/device-conformance.tex
create mode 100644 device-types/rtc/driver-conformance.tex
diff --git a/conformance.tex b/conformance.tex
index 863f9c5825bf..c9a8137f50bf 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -153,6 +153,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/rtc/driver-conformance.tex}
\conformance{\section}{Device Conformance}\label{sec:Conformance / Device Conformance}
@@ -240,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/rtc/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/device-types/rtc/description.tex b/device-types/rtc/description.tex
index abfa2206894e..7707472f1663 100644
--- a/device-types/rtc/description.tex
+++ b/device-types/rtc/description.tex
@@ -98,6 +98,108 @@ \subsection{Device Operation}\label{sec:Device Types / RTC Device / Device Opera
zero-based, dense indices. All fields named \field{clock_id} contain
clock identifiers.
+\drivernormative{\subsubsection}{Device Operation}{Device Types / RTC Device / Device Operation}
+
+If the \field{struct virtio_rtc_resp_head} field \field{status} is not
+VIRTIO_RTC_S_OK, the driver MUST NOT interpret response fields other
+than \field{status}.
+
+The driver MUST set \emph{reserved} fields in the device-readable part
+of the message to zero.
+
+The driver MUST NOT interpret \emph{reserved} fields in the
+device-writable part of the message.
+
+The driver MUST NOT interpret unnamed bits in \emph{flags} fields in the
+device-writable part of the message.
+
+The driver MUST put the request into the device-readable part of the
+message.
+
+The driver MUST allocate enough space for the response in the
+device-writable part of a requestq message.
+
+\devicenormative{\subsubsection}{Device Operation}{Device Types / RTC Device / Device Operation}
+
+For \field{struct virtio_rtc_resp_head}, the device MUST set the
+\field{status} field to VIRTIO_RTC_S_OK if the device successfully
+executed the request.
+
+For \field{struct virtio_rtc_resp_head}, the device MUST set the
+\field{status} field to a status other than VIRTIO_RTC_S_OK if the
+device did not successfully execute the request.
+
+For \field{struct virtio_rtc_resp_head}, the device MUST set the
+\field{status} field to VIRTIO_RTC_S_EOPNOTSUPP if the device could not
+execute the specific request due to an implementation limitation.
+
+For \field{struct virtio_rtc_resp_head}, the device MUST set the
+\field{status} field to VIRTIO_RTC_S_EOPNOTSUPP for a request with a
+value of the \field{msg_type} field which is not described in this
+specification.
+
+For \field{struct virtio_rtc_resp_head}, the device MUST set the
+\field{status} field to VIRTIO_RTC_S_EOPNOTSUPP for a request with a
+value of the \field{hw_counter} field which is neither described in this
+specification nor otherwise known to the implementation.
+
+For \field{struct virtio_rtc_resp_head}, the device MUST set the
+\field{status} field to VIRTIO_RTC_S_ENODEV if the \field{clock_id}
+field value supplied with the request does not identify a clock.
+
+For \field{struct virtio_rtc_resp_head}, the device MUST set the
+\field{status} field to VIRTIO_RTC_S_EINVAL if the request values are
+inconsistent with the specification and if the inconsistence is not
+described by the requirements which stipulate status
+VIRTIO_RTC_S_EOPNOTSUPP or VIRTIO_RTC_S_ENODEV.
+
+For \field{struct virtio_rtc_resp_head}, the device MUST set the
+\field{status} field to VIRTIO_RTC_S_EINVAL if the device read-only part
+of the message is too small.
+
+For \field{struct virtio_rtc_resp_head}, the device MUST set the
+\field{status} field to VIRTIO_RTC_S_EINVAL if the device write-only
+part of the message is too small, unless the \field{status} field does
+not fit into the device write-only part.
+
+For \field{struct virtio_rtc_resp_head}, the device MUST NOT set the
+\field{status} field if the \field{status} field does not fit into the
+device write-only part.
+
+For \field{struct virtio_rtc_resp_head}, the device MUST set the
+\field{status} field to VIRTIO_RTC_S_EIO if none of the previous
+requirements in this document stipulated another \field{status}.
+
+If the device read-only part of a message M is bigger than the size of
+the request specified for message M, the device MUST ignore the
+additional space.
+
+If the device write-only part of a message M is bigger than the size of
+the response specified for message M, the device MUST ignore the
+additional space.
+
+The device MUST NOT interpret \emph{reserved} fields in the
+device-readable part of the message.
+
+The device MUST set \emph{reserved} fields in the device-writable part
+of the message to zero.
+
+The device MUST set unnamed bits in \emph{flags} fields in the
+device-writable part of the message to zero.
+
+After feature negotiation completion the device MUST NOT change the set
+of clocks until device reset.
+
+The device SHOULD NOT change the set of clocks on a device reset after
+the first device reset.
+
+The device MUST use non-negative integers, which are smaller than the
+number of clocks, as clock identifiers.
+
+For a fixed request value V, the device SHOULD either, for every request
+with value V, always execute successfully, or, for every request with
+value V, always set a fixed \field{status} other than VIRTIO_RTC_S_OK.
+
\subsubsection{Control Requests}\label{sec:Device Types / RTC Device / Device Operation / Control Requests}
Through \emph{control requests}, the driver requests information about
@@ -203,6 +305,42 @@ \subsubsection{Control Requests}\label{sec:Device Types / RTC Device / Device Op
\end{description}
+\drivernormative{\paragraph}{Control Requests}{Device Types / RTC Device / Device Operation / Control Requests}
+
+For VIRTIO_RTC_REQ_CROSS_CAP, the driver MUST set \field{hw_counter} to
+one of the hardware counter identifiers defined in this specification,
+or to a value greater than or equal to 0xF000.
+
+\devicenormative{\paragraph}{Control Requests}{Device Types / RTC Device / Device Operation / Control Requests}
+
+For any clock of type VIRTIO_RTC_CLOCK_UTC, the device MUST use the UTC
+time standard (Coordinated Universal Time).
+
+For any clock of type VIRTIO_RTC_CLOCK_UTC, the device MUST use the time
+epoch of January 1, 1970, 00:00 UTC.
+
+For any clock of type VIRTIO_RTC_CLOCK_TAI, the device MUST use the TAI
+time standard (International Atomic Time).
+
+For any clock of type VIRTIO_RTC_CLOCK_TAI, the device MUST use the time
+epoch of January 1, 1970, 00:00 TAI.
+
+For any clock of type VIRTIO_RTC_CLOCK_MONO, the device MUST use SI
+seconds subdivisions.
+
+For any clock of type VIRTIO_RTC_CLOCK_MONO, the device MUST use an
+epoch at a time instant before or during device reset.
+
+For VIRTIO_RTC_REQ_CLOCK_CAP, the device MUST set \field{type} to
+one of the clock types defined in this specification, or to a value
+greater than or equal to 0xF000.
+
+For a VIRTIO_RTC_REQ_CROSS_CAP message M, the device MUST set the
+VIRTIO_RTC_FLAG_CROSS_CAP flag in the \field{flags} field if the device
+would respond to a VIRTIO_RTC_REQ_READ_CROSS message with the same
+\field{hw_counter} and \field{clock_id} values as in M with status
+VIRTIO_RTC_S_OK, and clear the flag otherwise.
+
\subsubsection{Read Requests}\label{sec:Device Types / RTC Device / Device Operation / Read Requests}
Through \emph{read requests}, the driver requests clock readings from
@@ -326,3 +464,72 @@ \subsubsection{Read Requests}\label{sec:Device Types / RTC Device / Device Opera
through VIRTIO_RTC_REQ_CROSS_CAP.
\end{description}
+
+\drivernormative{\paragraph}{Read Requests}{Device Types / RTC Device / Device Operation / Read Requests}
+
+For VIRTIO_RTC_REQ_READ_CROSS, the driver MUST set \field{hw_counter} to
+one of the hardware counter identifiers defined in this specification,
+or to a value greater than or equal to 0xF000.
+
+\devicenormative{\paragraph}{Read Requests}{Device Types / RTC Device / Device Operation / Read Requests}
+
+The device SHOULD continuously support reading of all clocks once
+DRIVER_OK has been set.
+
+For any clock of type VIRTIO_RTC_CLOCK_UTC, the device MAY insert UTC
+leap seconds.
+
+For any clock of type VIRTIO_RTC_CLOCK_UTC, the device MAY smear UTC
+leap seconds through clock slewing.
+
+For any clock C and read requests \emph{A} and \emph{B} which read C,
+\emph{A} being the message which the driver marks as available before
+\emph{B}, the device MUST obtain the \field{clock_reading} value for the
+message \emph{A} response before the \field{clock_reading} value for the
+message \emph{B} response, or the device MUST obtain the same
+\field{clock_reading} value for both \emph{A} and \emph{B}.
+
+For any clock C, the device MUST mark all read requests reading C as
+used in the total order in which the driver marked these messages as
+available.
+
+For any clock C of type VIRTIO_RTC_CLOCK_MONO and read requests
+\emph{A} and \emph{B} which read C, \emph{A} being the message which the
+driver marks as available before \emph{B}, the device MUST set the
+\field{clock_reading} value for the message \emph{B} response to a value
+greater than or equal to the \field{clock_reading} value for the message
+\emph{A} response.
+
+The device MUST support VIRTIO_RTC_REQ_READ for every clock.
+
+For VIRTIO_RTC_REQ_READ and for any clock type listed in this
+specification, the device MUST use the nanosecond as unit for field
+\field{clock_reading}.
+
+For read requests, the device MUST obtain the \field{clock_reading}
+response value after the read request is marked as available.
+
+For VIRTIO_RTC_REQ_READ_CROSS, the device MUST set
+\field{counter_cycles} to a value which approximates the value which the
+driver would have read from the hardware counter identified by
+\field{hw_counter} at the time instant when the device read the
+\field{clock_reading} value.
+
+For VIRTIO_RTC_REQ_READ_CROSS, the device MUST set \field{status}
+to a value other than \field{VIRTIO_RTC_S_OK} if the device cannot
+determine the approximate value which the driver would have read from
+the hardware counter identified by \field{hw_counter} at the time
+instant when the device read the \field{clock_reading} value.
+
+For any clock C, and VIRTIO_RTC_REQ_READ_CROSS messages \emph{A} and
+\emph{B} which read C and which specify the same hardware counter
+\emph{H} through field \field{hw_counter}, \emph{A} being the message
+which the driver marks as available before \emph{B}, the device MUST set
+the \field{counter_cycles} value for \emph{B} to a value which \emph{H}
+shows after the \field{counter_cycles} value of \emph{A}, or the device
+MUST set the same \field{counter_cycles} value for \emph{A} and
+\emph{B}.
+
+For VIRTIO_RTC_REQ_READ_CROSS and for any clock type listed in
+this specification, the device MUST use the nanosecond as unit for
+field \field{clock_reading}.
diff --git a/device-types/rtc/device-conformance.tex b/device-types/rtc/device-conformance.tex
new file mode 100644
index 000000000000..4303cd450542
--- /dev/null
+++ b/device-types/rtc/device-conformance.tex
@@ -0,0 +1,9 @@
+\conformance{\subsection}{RTC Device Conformance}\label{sec:Conformance / Device Conformance / RTC Device Conformance}
+
+An RTC device MUST conform to the following normative statements:
+
+\begin{itemize}
+\item \ref{devicenormative:Device Types / RTC Device / Device Operation}
+\item \ref{devicenormative:Device Types / RTC Device / Device Operation / Control Requests}
+\item \ref{devicenormative:Device Types / RTC Device / Device Operation / Read Requests}
+\end{itemize}
diff --git a/device-types/rtc/driver-conformance.tex b/device-types/rtc/driver-conformance.tex
new file mode 100644
index 000000000000..689c18d158d0
--- /dev/null
+++ b/device-types/rtc/driver-conformance.tex
@@ -0,0 +1,9 @@
+\conformance{\subsection}{RTC Driver Conformance}\label{sec:Conformance / Driver Conformance / RTC Driver Conformance}
+
+An RTC driver MUST conform to the following normative statements:
+
+\begin{itemize}
+\item \ref{drivernormative:Device Types / RTC Device / Device Operation}
+\item \ref{drivernormative:Device Types / RTC Device / Device Operation / Control Requests}
+\item \ref{drivernormative:Device Types / RTC Device / Device Operation / Read Requests}
+\end{itemize}
--
2.40.1
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
^ permalink raw reply related [flat|nested] 24+ messages in thread* [virtio-comment] [RFC PATCH v3 3/4] virtio-rtc: Add alarm feature
2023-12-18 6:42 [virtio-comment] [RFC PATCH v3 0/4] virtio-rtc: Add device specification Peter Hilber
2023-12-18 6:42 ` [virtio-comment] [RFC PATCH v3 1/4] virtio-rtc: Add initial " Peter Hilber
2023-12-18 6:42 ` [virtio-comment] [RFC PATCH v3 2/4] virtio-rtc: Add initial normative statements Peter Hilber
@ 2023-12-18 6:42 ` Peter Hilber
2023-12-22 18:57 ` Cornelia Huck
2024-01-20 10:16 ` [virtio-comment] " Parav Pandit
2023-12-18 6:42 ` [virtio-comment] [RFC PATCH v3 4/4] virtio-rtc: Add normative statements for " Peter Hilber
2024-07-25 4:53 ` [virtio-comment] [RFC PATCH v3 0/4] virtio-rtc: Add device specification Michael S. Tsirkin
4 siblings, 2 replies; 24+ messages in thread
From: Peter Hilber @ 2023-12-18 6:42 UTC (permalink / raw)
To: virtio-comment; +Cc: Peter Hilber, Cornelia Huck, Parav Pandit
Add the VIRTIO_RTC_F_ALARM feature (without normative statements).
The intended use case is: A driver needs to react when an alarm time has
been reached, but the driver may be in a sleep state or powered off at
alarm time. The alarm feature can resume and notify the driver in this
case. Alarms may be retained across device resets (including reset on
boot).
Peculiarities
-------------
Unlike usual alarm clocks, a virtio-rtc alarm-capable clock may step
autonomously at any time: An alarm may change back from "expired" to
"not expired" before the driver has started processing an alarm
notification.
To address the above, and the device resets, define "alarm expiration"
in such a way that the driver always has a chance to react to an alarm,
and make the device always responsible for notifying the driver about an
alarm expiration.
The VIRTIO_RTC_REQ_SET_ALARM_ENABLED request is there so that the Linux
ioctls RTC_AIE_ON and RTC_AIE_OFF only need to emit one request.
Open Questions
--------------
- A Linux driver could use the virtio-rtc real-time alarm, through the
Linux RTC device class, for both CLOCK_REALTIME_ALARM and (monotonic)
CLOCK_BOOTTIME_ALARM alarms. But then, after a clock step, a
CLOCK_BOOTTIME_ALARM alarm may be late (or early). Perhaps this can be
handled by the driver also setting a virtio-rtc monotonic alarm, so
may not need to be addressed by the spec.
- A request returning the minimum and maximum allowed alarm time might
still be added.
Signed-off-by: Peter Hilber <peter.hilber@opensynergy.com>
---
device-types/rtc/description.tex | 256 ++++++++++++++++++++++++++++++-
1 file changed, 254 insertions(+), 2 deletions(-)
diff --git a/device-types/rtc/description.tex b/device-types/rtc/description.tex
index 7707472f1663..fcd2f14186e6 100644
--- a/device-types/rtc/description.tex
+++ b/device-types/rtc/description.tex
@@ -4,6 +4,7 @@ \section{RTC Device}\label{sec:Device Types / RTC Device}
time. The device can provide different clocks, e.g.\ for the UTC or TAI
time standards, or for physical time elapsed since some past epoch. The
driver can read the clocks with simple or more accurate methods.
+Optionally, the driver can set an alarm.
\subsection{Device ID}\label{sec:Device Types / RTC Device / Device ID}
@@ -13,13 +14,23 @@ \subsection{Virtqueues}\label{sec:Device Types / RTC Device / Virtqueues}
\begin{description}
\item[0] requestq
+\item[1] alarmq
\end{description}
The driver enqueues requests to the requestq.
+Through the alarmq, the device notifies the driver about alarm
+expirations. The alarmq exists only if VIRTIO_RTC_F_ALARM was
+negotiated.
+
\subsection{Feature bits}\label{sec:Device Types / RTC Device / Feature bits}
-No device-specific feature bits are defined yet.
+\begin{description}
+\item[VIRTIO_RTC_F_ALARM (0)] Device supports alarm.
+\end{description}
+
+VIRTIO_RTC_F_ALARM determines whether the device supports setting an
+alarm for some of the clocks.
\subsection{Device configuration layout}\label{sec:Device Types / RTC Device / Device configuration layout}
@@ -244,7 +255,8 @@ \subsubsection{Control Requests}\label{sec:Device Types / RTC Device / Device Op
struct virtio_rtc_resp_clock_cap {
struct virtio_rtc_resp_head head;
le16 type;
- u8 reserved[6];
+ u8 flags;
+ u8 reserved[5];
};
\end{lstlisting}
@@ -274,6 +286,18 @@ \subsubsection{Control Requests}\label{sec:Device Types / RTC Device / Device Op
Implementation-specific clock type definitions are not recommended and
use a clock type id greater than or equal to 0xF000.
+The \field{flags} field provides the following information:
+
+\begin{lstlisting}
+#define VIRTIO_RTC_FLAG_ALARM_CAP (1 << 0)
+\end{lstlisting}
+
+\begin{itemize}
+\item If VIRTIO_RTC_F_ALARM was negotiated, flag
+ \field{VIRTIO_RTC_FLAG_ALARM_CAP} indicates that the clock
+ supports an alarm.
+\end{itemize}
+
\item[VIRTIO_RTC_REQ_CROSS_CAP] discovers whether the device supports
cross-timestamping for a particular pair of clock and hardware counter.
@@ -533,3 +557,231 @@ \subsubsection{Read Requests}\label{sec:Device Types / RTC Device / Device Opera
For VIRTIO_RTC_REQ_READ_CROSS and for any clock type listed in
this specification, the device MUST use the nanosecond as unit for
field \field{clock_reading}.
+
+\subsubsection{Alarm Operation}\label{sec:Device Types / RTC Device / Device Operation / Alarm Operation}
+
+Through the optional alarm feature, the driver can set an alarm time. On
+alarm expiration, the device notifies the driver. On alarm expiration,
+the device may also wake up the driver, while the driver is in a sleep
+state, or while the driver is powered off. How this is done is beyond
+the scope of the specification. The driver can set one alarm time per
+clock, if the clock supports this.
+
+The device may retain alarm times across device resets.\footnote{Drivers
+ may reset the device on boot or on resume from sleep state. It
+ can make sense for the device to retain the alarm time then,
+ similar to other alarm clocks.}
+
+The alarm feature, and the associated alarmq for notifications from the
+device, are available if VIRTIO_RTC_F_ALARM was negotiated. In addition,
+if the driver previously set an alarm time, even if the device no longer
+both
+
+\begin{itemize}
+\item is live and
+\item has negotiated VIRTIO_RTC_F_ALARM,
+\end{itemize}
+
+the device may still execute implementation-specific actions on alarm
+expiration.
+
+An alarm expires
+
+\begin{itemize}
+\item when the associated clock progresses (also: steps) from a time
+ prior to the alarm time to the alarm time, or to a time after
+ the alarm time, or
+
+\item when the driver sets an alarm time which is not in the future, or
+
+\item when the device is reset, if the alarm time is retained and not in
+ the future.\footnote{The device is always responsible for
+ detecting alarm expiration events. This avoids that the driver
+ needs to reason about when it shall poll for alarm expiration.}
+\end{itemize}
+
+When an alarm expires, the driver can disable it. Otherwise, the alarm
+expires each time when one of the above expiration events occurs, even
+if it occurred before.\footnote{This avoids that the driver may
+ miss an alarm when the clock steps backwards after alarm
+ expiration, but before the driver has resumed operation. This
+ also facilitates distinct drivers using the same device,
+ e.g.\ a driver in the bootloader, and a driver in the OS.}
+
+On alarm expiration, the device executes the alarm actions. The alarm
+actions are:
+
+\begin{itemize}
+\item The device notifies the driver through the alarmq. If the device
+ is not live, or no buffers are available in the alarmq, the
+ device will notify once the device is live and buffers are
+ available.
+
+\item Optionally, the device executes other, implementation-specific,
+ actions. The device may execute those immediately, regardless of
+ the device state.
+\end{itemize}
+
+An alarm expiration becomes obsolete
+
+\begin{itemize}
+\item once the clock jumps backwards, before the alarm time, or
+
+\item once the driver sets an alarm time, or
+
+\item once another alarm expiration event happens.
+\end{itemize}
+
+If an alarm expiration becomes obsolete, it is unspecified which alarm
+actions the device executes for this alarm expiration, and the device
+stops executing these alarm actions after a grace period.
+
+The driver-visible settings of an alarm consist of two elements:
+
+\begin{itemize}
+\item \field{driver_alarm_time}, a valid time for the corresponding
+ clock, and
+
+\item \field{alarm_enabled}, a boolean. While \field{alarm_enabled} is
+ true, \field{driver_alarm_time} is the actual alarm time.
+ While \field{alarm_enabled} is false, the device will act as if
+ the alarm time was in the future, so that the alarm will not
+ expire.
+\end{itemize}
+
+Initially, \field{driver_alarm_time} is an allowed alarm time, and
+\field{alarm_enabled} is false.
+
+\paragraph{Alarm Control Requests}
+
+If VIRTIO_RTC_F_ALARM is negotiated,
+
+\begin{itemize}
+\item the driver can determine if a clock supports an alarm through the
+ VIRTIO_RTC_FLAG_ALARM_CAP flag in the VIRTIO_RTC_REQ_CLOCK_CAP
+ response,
+
+\item the driver can enqueue the alarm control requests into the
+ requestq: VIRTIO_RTC_REQ_READ_ALARM, VIRTIO_RTC_REQ_SET_ALARM,
+ and VIRTIO_RTC_REQ_SET_ALARM_ENABLED.
+\end{itemize}
+
+The unit of all \field{alarm_time} fields is 1 nanosecond for all
+clock types which are part of the device specification so
+far.
+
+\begin{description}
+\item[VIRTIO_RTC_REQ_READ_ALARM] reads the current alarm.
+
+\begin{lstlisting}
+#define VIRTIO_RTC_REQ_READ_ALARM 0x1003 /* message type */
+
+struct virtio_rtc_req_read_alarm {
+ struct virtio_rtc_req_head head;
+ le16 clock_id;
+ u8 reserved[6];
+};
+
+struct virtio_rtc_resp_read_alarm {
+ struct virtio_rtc_resp_head head;
+ le64 alarm_time;
+#define VIRTIO_RTC_FLAG_ALARM_ENABLED (1 << 0)
+ u8 flags;
+ u8 reserved[7];
+};
+\end{lstlisting}
+
+\field{clock_id} identifies the alarm through its associated clock.
+Field \field{alarm_time} returns \field{driver_alarm_time}. In field
+\field{flags}, flag \field{VIRTIO_RTC_FLAG_ALARM_ENABLED} indicates
+whether \field{alarm_enabled} is true.
+
+\item[VIRTIO_RTC_REQ_SET_ALARM] sets the alarm.
+
+\begin{lstlisting}
+#define VIRTIO_RTC_REQ_SET_ALARM 0x1004 /* message type */
+
+struct virtio_rtc_req_set_alarm {
+ struct virtio_rtc_req_head head;
+ le64 alarm_time;
+ le16 clock_id;
+ /* flag: VIRTIO_RTC_FLAG_ALARM_ENABLED */
+ u8 flags;
+ u8 reserved[5];
+};
+
+struct virtio_rtc_resp_set_alarm {
+ struct virtio_rtc_resp_head head;
+ /* no response params */
+};
+\end{lstlisting}
+
+\field{clock_id} identifies the alarm through its associated clock.
+Field \field{alarm_time} sets \field{driver_alarm_time}. If flag
+\field{VIRTIO_RTC_FLAG_ALARM_ENABLED} is set in field \field{flags}, the
+device sets \field{alarm_enabled} to true; otherwise, the device sets
+\field{alarm_enabled} to false.
+
+\item[VIRTIO_RTC_REQ_SET_ALARM_ENABLED] sets \field{alarm_enabled}.
+
+\begin{lstlisting}
+#define VIRTIO_RTC_REQ_SET_ALARM_ENABLED 0x1005 /* message type */
+
+struct virtio_rtc_req_set_alarm_enabled {
+ struct virtio_rtc_req_head head;
+ le16 clock_id;
+ /* flag: VIRTIO_RTC_FLAG_ALARM_ENABLED */
+ u8 flags;
+ u8 reserved[5];
+};
+
+struct virtio_rtc_resp_set_alarm_enabled {
+ struct virtio_rtc_resp_head head;
+ /* no response params */
+};
+\end{lstlisting}
+
+\field{clock_id} identifies the alarm through its associated clock. If
+flag \field{VIRTIO_RTC_FLAG_ALARM_ENABLED} is set in field
+\field{flags}, the device sets \field{alarm_enabled} to true; otherwise,
+the device sets \field{alarm_enabled} to false.
+
+When processing this request, the device retains
+\field{driver_alarm_time}.
+\end{description}
+
+\paragraph{Alarm Notifications}
+
+If the alarmq is present, the driver should make buffers available in
+the alarmq, which the device uses for alarm notification messages. All
+alarmq fields are device-writable. The alarmq uses a common notification
+header.
+
+\begin{lstlisting}
+/* common notification header */
+struct virtio_rtc_notif_head {
+ le16 msg_type;
+ u8 reserved[6];
+};
+\end{lstlisting}
+
+The \field{msg_type} field identifies the message type.
+
+\begin{description}
+\item[VIRTIO_RTC_NOTIF_ALARM] notifies the driver about an alarm
+ expiration.
+
+\begin{lstlisting}
+#define VIRTIO_RTC_NOTIF_ALARM 0x2000 /* message type */
+
+struct virtio_rtc_notif_alarm {
+ struct virtio_rtc_notif_head head;
+ le16 clock_id;
+ u8 reserved[6];
+};
+\end{lstlisting}
+
+\end{description}
+
+\field{clock_id} identifies the expired alarm through its associated
+clock.
--
2.40.1
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [virtio-comment] [RFC PATCH v3 3/4] virtio-rtc: Add alarm feature
2023-12-18 6:42 ` [virtio-comment] [RFC PATCH v3 3/4] virtio-rtc: Add alarm feature Peter Hilber
@ 2023-12-22 18:57 ` Cornelia Huck
2023-12-25 4:18 ` Jason Wang
2024-01-11 11:43 ` Peter Hilber
2024-01-20 10:16 ` [virtio-comment] " Parav Pandit
1 sibling, 2 replies; 24+ messages in thread
From: Cornelia Huck @ 2023-12-22 18:57 UTC (permalink / raw)
To: Peter Hilber, virtio-comment; +Cc: Peter Hilber, Parav Pandit
On Mon, Dec 18 2023, Peter Hilber <peter.hilber@opensynergy.com> wrote:
> Add the VIRTIO_RTC_F_ALARM feature (without normative statements).
>
> The intended use case is: A driver needs to react when an alarm time has
> been reached, but the driver may be in a sleep state or powered off at
> alarm time. The alarm feature can resume and notify the driver in this
> case. Alarms may be retained across device resets (including reset on
> boot).
Does the driver have some kind of control or information about whether
alarms are retained? I.e. to start with a clean slate, if wanted.
I'm not familiar with RTC, so I'm not sure if that even makes sense, or
if I have missed something in the spec.
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [virtio-comment] [RFC PATCH v3 3/4] virtio-rtc: Add alarm feature
2023-12-22 18:57 ` Cornelia Huck
@ 2023-12-25 4:18 ` Jason Wang
2024-01-11 11:43 ` Peter Hilber
2024-01-11 11:43 ` Peter Hilber
1 sibling, 1 reply; 24+ messages in thread
From: Jason Wang @ 2023-12-25 4:18 UTC (permalink / raw)
To: Cornelia Huck; +Cc: Peter Hilber, virtio-comment, Parav Pandit
On Sat, Dec 23, 2023 at 2:57 AM Cornelia Huck <cohuck@redhat.com> wrote:
>
> On Mon, Dec 18 2023, Peter Hilber <peter.hilber@opensynergy.com> wrote:
>
> > Add the VIRTIO_RTC_F_ALARM feature (without normative statements).
> >
> > The intended use case is: A driver needs to react when an alarm time has
> > been reached, but the driver may be in a sleep state or powered off at
> > alarm time. The alarm feature can resume and notify the driver in this
> > case. Alarms may be retained across device resets (including reset on
> > boot).
>
> Does the driver have some kind of control or information about whether
> alarms are retained? I.e. to start with a clean slate, if wanted.
It might be otherwise we may have security implications?
Userspace driver set the alarm, and the device was shifted to kernel
driver which may get this alarm accidentally?
Thanks
>
> I'm not familiar with RTC, so I'm not sure if that even makes sense, or
> if I have missed something in the spec.
>
>
> This publicly archived list offers a means to provide input to the
> OASIS Virtual I/O Device (VIRTIO) TC.
>
> In order to verify user consent to the Feedback License terms and
> to minimize spam in the list archive, subscription is required
> before posting.
>
> Subscribe: virtio-comment-subscribe@lists.oasis-open.org
> Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
> List help: virtio-comment-help@lists.oasis-open.org
> List archive: https://lists.oasis-open.org/archives/virtio-comment/
> Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
> List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
> Committee: https://www.oasis-open.org/committees/virtio/
> Join OASIS: https://www.oasis-open.org/join/
>
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [virtio-comment] [RFC PATCH v3 3/4] virtio-rtc: Add alarm feature
2023-12-25 4:18 ` Jason Wang
@ 2024-01-11 11:43 ` Peter Hilber
0 siblings, 0 replies; 24+ messages in thread
From: Peter Hilber @ 2024-01-11 11:43 UTC (permalink / raw)
To: Jason Wang, Cornelia Huck; +Cc: virtio-comment, Parav Pandit
On 25.12.23 05:18, Jason Wang wrote:
> On Sat, Dec 23, 2023 at 2:57 AM Cornelia Huck <cohuck@redhat.com> wrote:
>>
>> On Mon, Dec 18 2023, Peter Hilber <peter.hilber@opensynergy.com> wrote:
>>
>>> Add the VIRTIO_RTC_F_ALARM feature (without normative statements).
>>>
>>> The intended use case is: A driver needs to react when an alarm time has
>>> been reached, but the driver may be in a sleep state or powered off at
>>> alarm time. The alarm feature can resume and notify the driver in this
>>> case. Alarms may be retained across device resets (including reset on
>>> boot).
>>
>> Does the driver have some kind of control or information about whether
>> alarms are retained? I.e. to start with a clean slate, if wanted.
>
> It might be otherwise we may have security implications?
>
> Userspace driver set the alarm, and the device was shifted to kernel
> driver which may get this alarm accidentally?
As per my answer to Cornelia's comment, a driver could avoid getting such
an alarm (at least after tightening the spec a bit).
I can also add a requirement that the driver should, on an alarm
notification, read the virtio-rtc clock, to avoid misinterpretation (also
due to race conditions such as when setting a new alarm time while the old
is about to expire).
Thanks for the comment,
Peter
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [virtio-comment] [RFC PATCH v3 3/4] virtio-rtc: Add alarm feature
2023-12-22 18:57 ` Cornelia Huck
2023-12-25 4:18 ` Jason Wang
@ 2024-01-11 11:43 ` Peter Hilber
2024-01-15 15:54 ` Cornelia Huck
1 sibling, 1 reply; 24+ messages in thread
From: Peter Hilber @ 2024-01-11 11:43 UTC (permalink / raw)
To: Cornelia Huck, virtio-comment; +Cc: Parav Pandit, Jason Wang
On 22.12.23 19:57, Cornelia Huck wrote:
> On Mon, Dec 18 2023, Peter Hilber <peter.hilber@opensynergy.com> wrote:
>
>> Add the VIRTIO_RTC_F_ALARM feature (without normative statements).
>>
>> The intended use case is: A driver needs to react when an alarm time has
>> been reached, but the driver may be in a sleep state or powered off at
>> alarm time. The alarm feature can resume and notify the driver in this
>> case. Alarms may be retained across device resets (including reset on
>> boot).
>
> Does the driver have some kind of control or information about whether
> alarms are retained? I.e. to start with a clean slate, if wanted.
As of now, the driver can disable the alarm through
VIRTIO_RTC_REQ_SET_ALARM_ENABLED. If the driver does this before making
buffers available to the alarmq, the device behaves like starting with a
clean slate, with two exceptions:
- VIRTIO_RTC_REQ_READ_ALARM might return a different alarm time than the
one at the first reset (but the alarm would be disabled).
If adding the minimum allowed alarm time to the spec, as was discussed in
the "Open Questions" section of the patch, initializing to the minimum
allowed alarm time could also become part of the "clean slate", so that
this exception would be removed.
- The requirements currently allow a "grace period" after disabling through
VIRTIO_RTC_REQ_SET_ALARM_ENABLED, during which the device could still
give the alarm notification, or execute custom alarm actions.
The draft spec permits alarm actions to continue for a short time after
the alarm has become obsolete, in order to not unnecessarily restrict
implementations. While I would not consider a sporadic-looking alarm
notification (which the driver can easily recognize as such) to be a
problem, the spec does not require to immediately cancel an obsolete
custom alarm action either.
But the requirements could be tightened so that all the actions have to
be completed or canceled before the device marks
VIRTIO_RTC_REQ_SET_ALARM_ENABLED as used:
If the driver successfully requests VIRTIO_RTC_REQ_SET_ALARM, or
VIRTIO_RTC_REQ_SET_ALARM_ENABLED, for clock C, the device MUST stop
serving any previous alarm expiration event for C before the device
marks the message as used.
This would remove the second exception.
I think I will just do the two above changes, if nobody objects.
Thanks for the comment,
Peter
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [virtio-comment] [RFC PATCH v3 3/4] virtio-rtc: Add alarm feature
2024-01-11 11:43 ` Peter Hilber
@ 2024-01-15 15:54 ` Cornelia Huck
2024-01-16 11:06 ` Peter Hilber
0 siblings, 1 reply; 24+ messages in thread
From: Cornelia Huck @ 2024-01-15 15:54 UTC (permalink / raw)
To: Peter Hilber, virtio-comment; +Cc: Parav Pandit, Jason Wang
On Thu, Jan 11 2024, Peter Hilber <peter.hilber@opensynergy.com> wrote:
> On 22.12.23 19:57, Cornelia Huck wrote:
>> On Mon, Dec 18 2023, Peter Hilber <peter.hilber@opensynergy.com> wrote:
>>
>>> Add the VIRTIO_RTC_F_ALARM feature (without normative statements).
>>>
>>> The intended use case is: A driver needs to react when an alarm time has
>>> been reached, but the driver may be in a sleep state or powered off at
>>> alarm time. The alarm feature can resume and notify the driver in this
>>> case. Alarms may be retained across device resets (including reset on
>>> boot).
>>
>> Does the driver have some kind of control or information about whether
>> alarms are retained? I.e. to start with a clean slate, if wanted.
>
> As of now, the driver can disable the alarm through
> VIRTIO_RTC_REQ_SET_ALARM_ENABLED. If the driver does this before making
> buffers available to the alarmq, the device behaves like starting with a
> clean slate, with two exceptions:
>
> - VIRTIO_RTC_REQ_READ_ALARM might return a different alarm time than the
> one at the first reset (but the alarm would be disabled).
>
> If adding the minimum allowed alarm time to the spec, as was discussed in
> the "Open Questions" section of the patch, initializing to the minimum
> allowed alarm time could also become part of the "clean slate", so that
> this exception would be removed.
Makes sense to me.
>
> - The requirements currently allow a "grace period" after disabling through
> VIRTIO_RTC_REQ_SET_ALARM_ENABLED, during which the device could still
> give the alarm notification, or execute custom alarm actions.
>
> The draft spec permits alarm actions to continue for a short time after
> the alarm has become obsolete, in order to not unnecessarily restrict
> implementations. While I would not consider a sporadic-looking alarm
> notification (which the driver can easily recognize as such) to be a
> problem, the spec does not require to immediately cancel an obsolete
> custom alarm action either.
>
> But the requirements could be tightened so that all the actions have to
> be completed or canceled before the device marks
> VIRTIO_RTC_REQ_SET_ALARM_ENABLED as used:
>
> If the driver successfully requests VIRTIO_RTC_REQ_SET_ALARM, or
> VIRTIO_RTC_REQ_SET_ALARM_ENABLED, for clock C, the device MUST stop
> serving any previous alarm expiration event for C before the device
> marks the message as used.
>
> This would remove the second exception.
This makes sense to me as well.
>
> I think I will just do the two above changes, if nobody objects.
Maybe also add a sentence that describes what the driver needs to do if
it doesn't want to get existing alarms? That might make things easier
for people who wish to write a driver, even if all of the needed
information can already be found in the spec.
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [virtio-comment] [RFC PATCH v3 3/4] virtio-rtc: Add alarm feature
2024-01-15 15:54 ` Cornelia Huck
@ 2024-01-16 11:06 ` Peter Hilber
0 siblings, 0 replies; 24+ messages in thread
From: Peter Hilber @ 2024-01-16 11:06 UTC (permalink / raw)
To: Cornelia Huck, virtio-comment; +Cc: Parav Pandit, Jason Wang
On 15.01.24 16:54, Cornelia Huck wrote:
> On Thu, Jan 11 2024, Peter Hilber <peter.hilber@opensynergy.com> wrote:
>
>> On 22.12.23 19:57, Cornelia Huck wrote:
>>> On Mon, Dec 18 2023, Peter Hilber <peter.hilber@opensynergy.com> wrote:
>>>
>>>> Add the VIRTIO_RTC_F_ALARM feature (without normative statements).
>>>>
>>>> The intended use case is: A driver needs to react when an alarm time has
>>>> been reached, but the driver may be in a sleep state or powered off at
>>>> alarm time. The alarm feature can resume and notify the driver in this
>>>> case. Alarms may be retained across device resets (including reset on
>>>> boot).
>>>
>>> Does the driver have some kind of control or information about whether
>>> alarms are retained? I.e. to start with a clean slate, if wanted.
>>
>> As of now, the driver can disable the alarm through
>> VIRTIO_RTC_REQ_SET_ALARM_ENABLED. If the driver does this before making
>> buffers available to the alarmq, the device behaves like starting with a
>> clean slate, with two exceptions:
>>
>> - VIRTIO_RTC_REQ_READ_ALARM might return a different alarm time than the
>> one at the first reset (but the alarm would be disabled).
>>
>> If adding the minimum allowed alarm time to the spec, as was discussed in
>> the "Open Questions" section of the patch, initializing to the minimum
>> allowed alarm time could also become part of the "clean slate", so that
>> this exception would be removed.
>
> Makes sense to me.
>
>>
>> - The requirements currently allow a "grace period" after disabling through
>> VIRTIO_RTC_REQ_SET_ALARM_ENABLED, during which the device could still
>> give the alarm notification, or execute custom alarm actions.
>>
>> The draft spec permits alarm actions to continue for a short time after
>> the alarm has become obsolete, in order to not unnecessarily restrict
>> implementations. While I would not consider a sporadic-looking alarm
>> notification (which the driver can easily recognize as such) to be a
>> problem, the spec does not require to immediately cancel an obsolete
>> custom alarm action either.
>>
>> But the requirements could be tightened so that all the actions have to
>> be completed or canceled before the device marks
>> VIRTIO_RTC_REQ_SET_ALARM_ENABLED as used:
>>
>> If the driver successfully requests VIRTIO_RTC_REQ_SET_ALARM, or
>> VIRTIO_RTC_REQ_SET_ALARM_ENABLED, for clock C, the device MUST stop
>> serving any previous alarm expiration event for C before the device
>> marks the message as used.
>>
>> This would remove the second exception.
>
> This makes sense to me as well.
>
>>
>> I think I will just do the two above changes, if nobody objects.
>
> Maybe also add a sentence that describes what the driver needs to do if
> it doesn't want to get existing alarms? That might make things easier
> for people who wish to write a driver, even if all of the needed
> information can already be found in the spec.
>
I will add a corresponding remark.
Thanks for the suggestion,
Peter
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
^ permalink raw reply [flat|nested] 24+ messages in thread
* [virtio-comment] RE: [RFC PATCH v3 3/4] virtio-rtc: Add alarm feature
2023-12-18 6:42 ` [virtio-comment] [RFC PATCH v3 3/4] virtio-rtc: Add alarm feature Peter Hilber
2023-12-22 18:57 ` Cornelia Huck
@ 2024-01-20 10:16 ` Parav Pandit
2024-01-24 15:40 ` [virtio-comment] " Peter Hilber
1 sibling, 1 reply; 24+ messages in thread
From: Parav Pandit @ 2024-01-20 10:16 UTC (permalink / raw)
To: Peter Hilber, virtio-comment@lists.oasis-open.org; +Cc: Cornelia Huck
> From: Peter Hilber <peter.hilber@opensynergy.com>
> Sent: Monday, December 18, 2023 12:13 PM
>
> Add the VIRTIO_RTC_F_ALARM feature (without normative statements).
>
> The intended use case is: A driver needs to react when an alarm time has
> been reached, but the driver may be in a sleep state or powered off at alarm
> time. The alarm feature can resume and notify the driver in this case. Alarms
> may be retained across device resets (including reset on boot).
>
> Peculiarities
> -------------
>
> Unlike usual alarm clocks, a virtio-rtc alarm-capable clock may step
> autonomously at any time: An alarm may change back from "expired" to "not
> expired" before the driver has started processing an alarm notification.
>
> To address the above, and the device resets, define "alarm expiration"
> in such a way that the driver always has a chance to react to an alarm, and
> make the device always responsible for notifying the driver about an alarm
> expiration.
>
> The VIRTIO_RTC_REQ_SET_ALARM_ENABLED request is there so that the
> Linux ioctls RTC_AIE_ON and RTC_AIE_OFF only need to emit one request.
>
> Open Questions
> --------------
>
> - A Linux driver could use the virtio-rtc real-time alarm, through the
> Linux RTC device class, for both CLOCK_REALTIME_ALARM and (monotonic)
> CLOCK_BOOTTIME_ALARM alarms. But then, after a clock step, a
> CLOCK_BOOTTIME_ALARM alarm may be late (or early). Perhaps this can be
> handled by the driver also setting a virtio-rtc monotonic alarm, so
> may not need to be addressed by the spec.
>
> - A request returning the minimum and maximum allowed alarm time might
> still be added.
>
> Signed-off-by: Peter Hilber <peter.hilber@opensynergy.com>
> ---
> device-types/rtc/description.tex | 256 ++++++++++++++++++++++++++++++-
> 1 file changed, 254 insertions(+), 2 deletions(-)
>
> diff --git a/device-types/rtc/description.tex b/device-types/rtc/description.tex
> index 7707472f1663..fcd2f14186e6 100644
> --- a/device-types/rtc/description.tex
> +++ b/device-types/rtc/description.tex
> @@ -4,6 +4,7 @@ \section{RTC Device}\label{sec:Device Types / RTC Device}
> time. The device can provide different clocks, e.g.\ for the UTC or TAI time
> standards, or for physical time elapsed since some past epoch. The driver can
> read the clocks with simple or more accurate methods.
> +Optionally, the driver can set an alarm.
>
> \subsection{Device ID}\label{sec:Device Types / RTC Device / Device ID}
>
> @@ -13,13 +14,23 @@ \subsection{Virtqueues}\label{sec:Device Types / RTC
> Device / Virtqueues}
>
> \begin{description}
> \item[0] requestq
> +\item[1] alarmq
> \end{description}
>
> The driver enqueues requests to the requestq.
>
> +Through the alarmq, the device notifies the driver about alarm
> +expirations. The alarmq exists only if VIRTIO_RTC_F_ALARM was
> +negotiated.
> +
I think is a good example of a need of notification queue from device to driver that has multiple use as generic object.
On NIC side also we have been thinking to have VQ that holds only the completions as currently packed and splitvq unaligned and shorter completions are performance reducing areas.
A queue which has 1 to 8 physical addresses, and which can hold 8 to 64B of notification will be useful.
This way per descriptor posting and DMA is not needed.
These notifications also work very fast with least amount of descriptor caching on the device.
For alarms this may not be concern at all in current form but overall, such a generic infra has multiple uses.
So, I suggest we bring the notification queue.
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
^ permalink raw reply [flat|nested] 24+ messages in thread* [virtio-comment] Re: [RFC PATCH v3 3/4] virtio-rtc: Add alarm feature
2024-01-20 10:16 ` [virtio-comment] " Parav Pandit
@ 2024-01-24 15:40 ` Peter Hilber
2024-01-28 6:30 ` [virtio-comment] " Parav Pandit
0 siblings, 1 reply; 24+ messages in thread
From: Peter Hilber @ 2024-01-24 15:40 UTC (permalink / raw)
To: Parav Pandit, virtio-comment@lists.oasis-open.org; +Cc: Cornelia Huck
On 20.01.24 11:16, Parav Pandit wrote:
>
>
>> From: Peter Hilber <peter.hilber@opensynergy.com>
>> Sent: Monday, December 18, 2023 12:13 PM
>>
>> Add the VIRTIO_RTC_F_ALARM feature (without normative statements).
>>
>> The intended use case is: A driver needs to react when an alarm time has
>> been reached, but the driver may be in a sleep state or powered off at alarm
>> time. The alarm feature can resume and notify the driver in this case. Alarms
>> may be retained across device resets (including reset on boot).
>>
>> Peculiarities
>> -------------
>>
>> Unlike usual alarm clocks, a virtio-rtc alarm-capable clock may step
>> autonomously at any time: An alarm may change back from "expired" to "not
>> expired" before the driver has started processing an alarm notification.
>>
>> To address the above, and the device resets, define "alarm expiration"
>> in such a way that the driver always has a chance to react to an alarm, and
>> make the device always responsible for notifying the driver about an alarm
>> expiration.
>>
>> The VIRTIO_RTC_REQ_SET_ALARM_ENABLED request is there so that the
>> Linux ioctls RTC_AIE_ON and RTC_AIE_OFF only need to emit one request.
>>
>> Open Questions
>> --------------
>>
>> - A Linux driver could use the virtio-rtc real-time alarm, through the
>> Linux RTC device class, for both CLOCK_REALTIME_ALARM and (monotonic)
>> CLOCK_BOOTTIME_ALARM alarms. But then, after a clock step, a
>> CLOCK_BOOTTIME_ALARM alarm may be late (or early). Perhaps this can be
>> handled by the driver also setting a virtio-rtc monotonic alarm, so
>> may not need to be addressed by the spec.
>>
>> - A request returning the minimum and maximum allowed alarm time might
>> still be added.
>>
>> Signed-off-by: Peter Hilber <peter.hilber@opensynergy.com>
>> ---
>> device-types/rtc/description.tex | 256 ++++++++++++++++++++++++++++++-
>> 1 file changed, 254 insertions(+), 2 deletions(-)
>>
>> diff --git a/device-types/rtc/description.tex b/device-types/rtc/description.tex
>> index 7707472f1663..fcd2f14186e6 100644
>> --- a/device-types/rtc/description.tex
>> +++ b/device-types/rtc/description.tex
>> @@ -4,6 +4,7 @@ \section{RTC Device}\label{sec:Device Types / RTC Device}
>> time. The device can provide different clocks, e.g.\ for the UTC or TAI time
>> standards, or for physical time elapsed since some past epoch. The driver can
>> read the clocks with simple or more accurate methods.
>> +Optionally, the driver can set an alarm.
>>
>> \subsection{Device ID}\label{sec:Device Types / RTC Device / Device ID}
>>
>> @@ -13,13 +14,23 @@ \subsection{Virtqueues}\label{sec:Device Types / RTC
>> Device / Virtqueues}
>>
>> \begin{description}
>> \item[0] requestq
>> +\item[1] alarmq
>> \end{description}
>>
>> The driver enqueues requests to the requestq.
>>
>> +Through the alarmq, the device notifies the driver about alarm
>> +expirations. The alarmq exists only if VIRTIO_RTC_F_ALARM was
>> +negotiated.
>> +
> I think is a good example of a need of notification queue from device to driver that has multiple use as generic object.
> On NIC side also we have been thinking to have VQ that holds only the completions as currently packed and splitvq unaligned and shorter completions are performance reducing areas.
>
> A queue which has 1 to 8 physical addresses, and which can hold 8 to 64B of notification will be useful.
> This way per descriptor posting and DMA is not needed.
> These notifications also work very fast with least amount of descriptor caching on the device.
>
> For alarms this may not be concern at all in current form but overall, such a generic infra has multiple uses.
> So, I suggest we bring the notification queue.
IIUC this notification queue still needs to be described.
The alarmq is a dedicated queue on purpose, to make it less intrusive to
wake the driver up while the driver may be in sleep mode (by making only
the corresponding interrupt a wake-up interrupt). Sharing the queue with
non-alarm notifications might create problems in the future.
In my reply for patch 1 I proposed to add this as an alternative interface
protected by a feature bit. This interface could cover the notification
queue as well. But I am not sure that the alarm feature will be useful for
net devices.
Thanks for the comment,
Peter
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
^ permalink raw reply [flat|nested] 24+ messages in thread* [virtio-comment] RE: [RFC PATCH v3 3/4] virtio-rtc: Add alarm feature
2024-01-24 15:40 ` [virtio-comment] " Peter Hilber
@ 2024-01-28 6:30 ` Parav Pandit
2024-01-29 16:51 ` Cornelia Huck
0 siblings, 1 reply; 24+ messages in thread
From: Parav Pandit @ 2024-01-28 6:30 UTC (permalink / raw)
To: Peter Hilber, virtio-comment@lists.oasis-open.org; +Cc: Cornelia Huck
> From: Peter Hilber <peter.hilber@opensynergy.com>
> Sent: Wednesday, January 24, 2024 9:11 PM
>
> On 20.01.24 11:16, Parav Pandit wrote:
> >
> >
> >> From: Peter Hilber <peter.hilber@opensynergy.com>
> >> Sent: Monday, December 18, 2023 12:13 PM
> >>
> >> Add the VIRTIO_RTC_F_ALARM feature (without normative statements).
> >>
> >> The intended use case is: A driver needs to react when an alarm time
> >> has been reached, but the driver may be in a sleep state or powered
> >> off at alarm time. The alarm feature can resume and notify the driver
> >> in this case. Alarms may be retained across device resets (including reset
> on boot).
> >>
> >> Peculiarities
> >> -------------
> >>
> >> Unlike usual alarm clocks, a virtio-rtc alarm-capable clock may step
> >> autonomously at any time: An alarm may change back from "expired" to
> >> "not expired" before the driver has started processing an alarm
> notification.
> >>
> >> To address the above, and the device resets, define "alarm expiration"
> >> in such a way that the driver always has a chance to react to an
> >> alarm, and make the device always responsible for notifying the
> >> driver about an alarm expiration.
> >>
> >> The VIRTIO_RTC_REQ_SET_ALARM_ENABLED request is there so that the
> >> Linux ioctls RTC_AIE_ON and RTC_AIE_OFF only need to emit one request.
> >>
> >> Open Questions
> >> --------------
> >>
> >> - A Linux driver could use the virtio-rtc real-time alarm, through the
> >> Linux RTC device class, for both CLOCK_REALTIME_ALARM and
> (monotonic)
> >> CLOCK_BOOTTIME_ALARM alarms. But then, after a clock step, a
> >> CLOCK_BOOTTIME_ALARM alarm may be late (or early). Perhaps this can
> be
> >> handled by the driver also setting a virtio-rtc monotonic alarm, so
> >> may not need to be addressed by the spec.
> >>
> >> - A request returning the minimum and maximum allowed alarm time
> might
> >> still be added.
> >>
> >> Signed-off-by: Peter Hilber <peter.hilber@opensynergy.com>
> >> ---
> >> device-types/rtc/description.tex | 256
> >> ++++++++++++++++++++++++++++++-
> >> 1 file changed, 254 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/device-types/rtc/description.tex
> >> b/device-types/rtc/description.tex
> >> index 7707472f1663..fcd2f14186e6 100644
> >> --- a/device-types/rtc/description.tex
> >> +++ b/device-types/rtc/description.tex
> >> @@ -4,6 +4,7 @@ \section{RTC Device}\label{sec:Device Types / RTC
> >> Device} time. The device can provide different clocks, e.g.\ for the
> >> UTC or TAI time standards, or for physical time elapsed since some
> >> past epoch. The driver can read the clocks with simple or more accurate
> methods.
> >> +Optionally, the driver can set an alarm.
> >>
> >> \subsection{Device ID}\label{sec:Device Types / RTC Device / Device
> >> ID}
> >>
> >> @@ -13,13 +14,23 @@ \subsection{Virtqueues}\label{sec:Device Types /
> >> RTC Device / Virtqueues}
> >>
> >> \begin{description}
> >> \item[0] requestq
> >> +\item[1] alarmq
> >> \end{description}
> >>
> >> The driver enqueues requests to the requestq.
> >>
> >> +Through the alarmq, the device notifies the driver about alarm
> >> +expirations. The alarmq exists only if VIRTIO_RTC_F_ALARM was
> >> +negotiated.
> >> +
> > I think is a good example of a need of notification queue from device to
> driver that has multiple use as generic object.
> > On NIC side also we have been thinking to have VQ that holds only the
> completions as currently packed and splitvq unaligned and shorter
> completions are performance reducing areas.
> >
> > A queue which has 1 to 8 physical addresses, and which can hold 8 to 64B
> of notification will be useful.
> > This way per descriptor posting and DMA is not needed.
> > These notifications also work very fast with least amount of descriptor
> caching on the device.
> >
> > For alarms this may not be concern at all in current form but overall, such a
> generic infra has multiple uses.
> > So, I suggest we bring the notification queue.
>
> IIUC this notification queue still needs to be described.
>
Yes.
What we need is something like,
A notification queue is a queue that holds device to driver notifications.
The device writes the notification entry, the driver consumes it.
Each notification entry consists of N bytes written by the device. Typically N bytes range from 8 to 64B.
Posting every descriptor of 16B for such small size entry is very inefficient; hence a notification queue is simpler enough that does not have one to one mapping of notification entry and descriptor.
A Notification descriptor points to a page memory. Each page holds one or more notification entries.
Number of notification entries in a descriptor = page_size / notification entry size;
For example, a notification queue consists of 2 4K pages, with 16B notification entry, can receive 512 notifications, with only two descriptors posting, offering 256X reduction in descriptors management at just 0.39% descriptor table size than current packed vq format.
> The alarmq is a dedicated queue on purpose, to make it less intrusive to wake
> the driver up while the driver may be in sleep mode (by making only the
> corresponding interrupt a wake-up interrupt). Sharing the queue with non-
> alarm notifications might create problems in the future.
>
I wasn’t suggesting to share alarm notification with other notifications.
> In my reply for patch 1 I proposed to add this as an alternative interface
> protected by a feature bit. This interface could cover the notification queue
> as well.
Yes. It can.
> But I am not sure that the alarm feature will be useful for net
> devices.
>
Unlikely net will use in any near future.
However, a generic notification queue infrastructure can be of good use across device types.
One of the devices should introduce so other can benefit otherwise, spec stays on the inferior path to attempt to utilize current scheme.
For example, Stephen had similar suggest on net patches to offer things to generic devices.
> Thanks for the comment,
>
> Peter
^ permalink raw reply [flat|nested] 24+ messages in thread* [virtio-comment] RE: [RFC PATCH v3 3/4] virtio-rtc: Add alarm feature
2024-01-28 6:30 ` [virtio-comment] " Parav Pandit
@ 2024-01-29 16:51 ` Cornelia Huck
2024-02-01 5:53 ` Parav Pandit
0 siblings, 1 reply; 24+ messages in thread
From: Cornelia Huck @ 2024-01-29 16:51 UTC (permalink / raw)
To: Parav Pandit, Peter Hilber, virtio-comment@lists.oasis-open.org
On Sun, Jan 28 2024, Parav Pandit <parav@nvidia.com> wrote:
>> From: Peter Hilber <peter.hilber@opensynergy.com>
>> Sent: Wednesday, January 24, 2024 9:11 PM
>>
>> On 20.01.24 11:16, Parav Pandit wrote:
>> >
>> >
>> >> From: Peter Hilber <peter.hilber@opensynergy.com>
>> >> Sent: Monday, December 18, 2023 12:13 PM
>> >> @@ -13,13 +14,23 @@ \subsection{Virtqueues}\label{sec:Device Types /
>> >> RTC Device / Virtqueues}
>> >>
>> >> \begin{description}
>> >> \item[0] requestq
>> >> +\item[1] alarmq
>> >> \end{description}
>> >>
>> >> The driver enqueues requests to the requestq.
>> >>
>> >> +Through the alarmq, the device notifies the driver about alarm
>> >> +expirations. The alarmq exists only if VIRTIO_RTC_F_ALARM was
>> >> +negotiated.
>> >> +
>> > I think is a good example of a need of notification queue from device to
>> driver that has multiple use as generic object.
>> > On NIC side also we have been thinking to have VQ that holds only the
>> completions as currently packed and splitvq unaligned and shorter
>> completions are performance reducing areas.
>> >
>> > A queue which has 1 to 8 physical addresses, and which can hold 8 to 64B
>> of notification will be useful.
>> > This way per descriptor posting and DMA is not needed.
>> > These notifications also work very fast with least amount of descriptor
>> caching on the device.
>> >
>> > For alarms this may not be concern at all in current form but overall, such a
>> generic infra has multiple uses.
>> > So, I suggest we bring the notification queue.
>>
>> IIUC this notification queue still needs to be described.
>>
> Yes.
> What we need is something like,
>
> A notification queue is a queue that holds device to driver notifications.
> The device writes the notification entry, the driver consumes it.
> Each notification entry consists of N bytes written by the device. Typically N bytes range from 8 to 64B.
> Posting every descriptor of 16B for such small size entry is very inefficient; hence a notification queue is simpler enough that does not have one to one mapping of notification entry and descriptor.
> A Notification descriptor points to a page memory. Each page holds one or more notification entries.
> Number of notification entries in a descriptor = page_size / notification entry size;
> For example, a notification queue consists of 2 4K pages, with 16B notification entry, can receive 512 notifications, with only two descriptors posting, offering 256X reduction in descriptors management at just 0.39% descriptor table size than current packed vq format.
This looks not entirely unlike ccw two-stage-indicators (which is in
turn preceded by other uses of adapter interrupts with indicators,
e.g. in QDIO). Can we learn from those earlier implementations?
However, while this looks like an interesting addition, I'm not sure we
should make rtc wait for all of that (including your other suggestions)
to be ready and approved -- a simpler rtc implementation has its
advantages as well (like already being there, for example.) We can
easily add things via feature bits later.
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
^ permalink raw reply [flat|nested] 24+ messages in thread* [virtio-comment] RE: [RFC PATCH v3 3/4] virtio-rtc: Add alarm feature
2024-01-29 16:51 ` Cornelia Huck
@ 2024-02-01 5:53 ` Parav Pandit
0 siblings, 0 replies; 24+ messages in thread
From: Parav Pandit @ 2024-02-01 5:53 UTC (permalink / raw)
To: Cornelia Huck, Peter Hilber, virtio-comment@lists.oasis-open.org
> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Monday, January 29, 2024 10:22 PM
>
> On Sun, Jan 28 2024, Parav Pandit <parav@nvidia.com> wrote:
>
> >> From: Peter Hilber <peter.hilber@opensynergy.com>
> >> Sent: Wednesday, January 24, 2024 9:11 PM
> >>
> >> On 20.01.24 11:16, Parav Pandit wrote:
> >> >
> >> >
> >> >> From: Peter Hilber <peter.hilber@opensynergy.com>
> >> >> Sent: Monday, December 18, 2023 12:13 PM @@ -13,13 +14,23 @@
> >> >> \subsection{Virtqueues}\label{sec:Device Types / RTC Device /
> >> >> Virtqueues}
> >> >>
> >> >> \begin{description}
> >> >> \item[0] requestq
> >> >> +\item[1] alarmq
> >> >> \end{description}
> >> >>
> >> >> The driver enqueues requests to the requestq.
> >> >>
> >> >> +Through the alarmq, the device notifies the driver about alarm
> >> >> +expirations. The alarmq exists only if VIRTIO_RTC_F_ALARM was
> >> >> +negotiated.
> >> >> +
> >> > I think is a good example of a need of notification queue from
> >> > device to
> >> driver that has multiple use as generic object.
> >> > On NIC side also we have been thinking to have VQ that holds only
> >> > the
> >> completions as currently packed and splitvq unaligned and shorter
> >> completions are performance reducing areas.
> >> >
> >> > A queue which has 1 to 8 physical addresses, and which can hold 8
> >> > to 64B
> >> of notification will be useful.
> >> > This way per descriptor posting and DMA is not needed.
> >> > These notifications also work very fast with least amount of
> >> > descriptor
> >> caching on the device.
> >> >
> >> > For alarms this may not be concern at all in current form but
> >> > overall, such a
> >> generic infra has multiple uses.
> >> > So, I suggest we bring the notification queue.
> >>
> >> IIUC this notification queue still needs to be described.
> >>
> > Yes.
> > What we need is something like,
> >
> > A notification queue is a queue that holds device to driver notifications.
> > The device writes the notification entry, the driver consumes it.
> > Each notification entry consists of N bytes written by the device. Typically N
> bytes range from 8 to 64B.
> > Posting every descriptor of 16B for such small size entry is very inefficient;
> hence a notification queue is simpler enough that does not have one to one
> mapping of notification entry and descriptor.
> > A Notification descriptor points to a page memory. Each page holds one or
> more notification entries.
> > Number of notification entries in a descriptor = page_size /
> > notification entry size; For example, a notification queue consists of 2 4K
> pages, with 16B notification entry, can receive 512 notifications, with only
> two descriptors posting, offering 256X reduction in descriptors management
> at just 0.39% descriptor table size than current packed vq format.
>
> This looks not entirely unlike ccw two-stage-indicators (which is in turn
> preceded by other uses of adapter interrupts with indicators, e.g. in QDIO).
> Can we learn from those earlier implementations?
>
There are many implementations to learn from.
Not sure of your exact suggestion.
> However, while this looks like an interesting addition, I'm not sure we should
> make rtc wait for all of that (including your other suggestions) to be ready
> and approved -- a simpler rtc implementation has its advantages as well (like
> already being there, for example.) We can easily add things via feature bits
> later.
Alarm is the new feature posted for the first time if I recollect right, notification queue is for the alarm.
So the simple rtc without alarm using admin infrastructure can be part of the spec at earliest.
I am fine with "already being there" suggestion too, if we are ok to apply to wider areas than rtc.
Do you agree?
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
^ permalink raw reply [flat|nested] 24+ messages in thread
* [virtio-comment] [RFC PATCH v3 4/4] virtio-rtc: Add normative statements for alarm feature
2023-12-18 6:42 [virtio-comment] [RFC PATCH v3 0/4] virtio-rtc: Add device specification Peter Hilber
` (2 preceding siblings ...)
2023-12-18 6:42 ` [virtio-comment] [RFC PATCH v3 3/4] virtio-rtc: Add alarm feature Peter Hilber
@ 2023-12-18 6:42 ` Peter Hilber
2024-07-25 4:53 ` [virtio-comment] [RFC PATCH v3 0/4] virtio-rtc: Add device specification Michael S. Tsirkin
4 siblings, 0 replies; 24+ messages in thread
From: Peter Hilber @ 2023-12-18 6:42 UTC (permalink / raw)
To: virtio-comment; +Cc: Peter Hilber, Cornelia Huck, Parav Pandit
Add the normative statements for the alarm feature added previously.
Signed-off-by: Peter Hilber <peter.hilber@opensynergy.com>
---
device-types/rtc/description.tex | 150 ++++++++++++++++++++++++
device-types/rtc/device-conformance.tex | 4 +
device-types/rtc/driver-conformance.tex | 2 +
3 files changed, 156 insertions(+)
diff --git a/device-types/rtc/description.tex b/device-types/rtc/description.tex
index fcd2f14186e6..e9d730167404 100644
--- a/device-types/rtc/description.tex
+++ b/device-types/rtc/description.tex
@@ -32,6 +32,11 @@ \subsection{Feature bits}\label{sec:Device Types / RTC Device / Feature bits}
VIRTIO_RTC_F_ALARM determines whether the device supports setting an
alarm for some of the clocks.
+\devicenormative{\subsubsection}{Feature bits}{Device Types / RTC Device / Feature bits}
+
+The device SHOULD offer VIRTIO_RTC_F_ALARM if the device can support
+setting an alarm for any of its clocks.
+
\subsection{Device configuration layout}\label{sec:Device Types / RTC Device / Device configuration layout}
There is no configuration data for the device.
@@ -558,6 +563,12 @@ \subsubsection{Read Requests}\label{sec:Device Types / RTC Device / Device Opera
this specification, the device MUST use the nanosecond as unit for
field \field{clock_reading}.
+If VIRTIO_RTC_F_ALARM was negotiated, and the device sent an alarm
+notification N for clock C with alarm time A, the device MUST, for all
+read requests of C which the driver marks as available after the device
+marked N as used, return a \field{clock_reading} which does not precede
+A, unless C stepped backwards before A.
+
\subsubsection{Alarm Operation}\label{sec:Device Types / RTC Device / Device Operation / Alarm Operation}
Through the optional alarm feature, the driver can set an alarm time. On
@@ -652,6 +663,79 @@ \subsubsection{Alarm Operation}\label{sec:Device Types / RTC Device / Device Ope
Initially, \field{driver_alarm_time} is an allowed alarm time, and
\field{alarm_enabled} is false.
+\devicenormative{\paragraph}{Alarm Operation}{Device Types / RTC Device / Device Operation / Alarm Operation}
+
+The device MAY retain both \field{driver_alarm_time} and
+\field{alarm_enabled} of a clock across a device reset.
+
+If the device did not retain \field{driver_alarm_time} and
+\field{alarm_enabled} of a clock across a device reset, the device MUST
+initialize \field{driver_alarm_time} to an allowed alarm time.
+
+If the device did not retain \field{driver_alarm_time} and
+\field{alarm_enabled} of a clock across a device reset, the device MUST
+initialize \field{alarm_enabled} to false.
+
+While \field{alarm_enabled} for a clock is true, the device MUST set the
+alarm time to \field{driver_alarm_time}.
+
+While \field{alarm_enabled} for a clock is false, the device MUST act as
+if the alarm time was in the future.
+
+If VIRTIO_RTC_F_ALARM was negotiated, the device MUST support the alarm
+messages, VIRTIO_RTC_REQ_READ_ALARM, VIRTIO_RTC_REQ_SET_ALARM,
+VIRTIO_RTC_REQ_SET_ALARM_ENABLED, and VIRTIO_RTC_NOTIF_ALARM, for one or
+more clocks.
+
+If VIRTIO_RTC_F_ALARM was not negotiated, the device MUST NOT support the
+alarm messages.
+
+The device MUST set flag VIRTIO_RTC_FLAG_ALARM_CAP in \field{struct
+virtio_rtc_resp_clock_cap.flags} if the respective clock supports alarm
+messages, and clear the flag otherwise.
+
+The device MUST consider it an alarm expiration event when the
+associated clock progresses (also: steps) from a time prior to the alarm
+time to the alarm time, or to a time after the alarm time.
+
+The device MUST consider it an alarm expiration event when the
+driver sets an alarm time which the associated clock has already reached
+or passed.
+
+If the device retained \field{driver_alarm_time} and
+\field{alarm_enabled} of a clock across a device reset, and the clock
+has already reached or passed the alarm time, the device MUST consider
+this device reset an alarm expiration event.
+
+If an alarm expiration event E happens, the device MUST start serving
+the alarm expiration event E.
+
+If the driver successfully requests VIRTIO_RTC_REQ_SET_ALARM, or
+VIRTIO_RTC_REQ_SET_ALARM_ENABLED, for clock C, the device MUST stop
+serving any previous alarm expiration event for C, within a grace
+period.
+
+If a clock C steps to a time previous to C's alarm time, the device MUST
+stop serving any previous alarm expiration event for C, within a grace
+period.
+
+If an alarm expiration event happens for clock C, the device MUST stop
+serving any previous alarm expiration event for C, within a grace
+period.
+
+If the device is currently serving an alarm expiration event E, the
+device MUST send a single VIRTIO_RTC_NOTIF_ALARM notification for E, as
+soon as an alarmq buffer is available for this purpose.
+
+While the device is serving an alarm expiration event, the device MAY
+execute implementation-specific alarm actions.
+
+The device MAY ignore the device status when executing
+implementation-specific alarm actions.
+
+The device MAY ignore whether VIRTIO_RTC_F_ALARM was negotiated when
+executing implementation-specific alarm actions.
+
\paragraph{Alarm Control Requests}
If VIRTIO_RTC_F_ALARM is negotiated,
@@ -750,6 +834,59 @@ \subsubsection{Alarm Operation}\label{sec:Device Types / RTC Device / Device Ope
\field{driver_alarm_time}.
\end{description}
+\drivernormative{\subparagraph}{Alarm Control Requests}{Device Types / RTC Device / Device Operation / Alarm Operation / Alarm Control Requests}
+
+For VIRTIO_RTC_REQ_SET_ALARM and for any clock type listed in this
+specification, the driver MUST use the nanosecond as unit for the
+\field{alarm_time} field.
+
+\devicenormative{\subparagraph}{Alarm Control Requests}{Device Types / RTC Device / Device Operation / Alarm Operation / Alarm Control Requests}
+
+If VIRTIO_RTC_F_ALARM was not negotiated, the device MUST set status
+VIRTIO_RTC_S_ENODEV for VIRTIO_RTC_REQ_READ_ALARM,
+VIRTIO_RTC_REQ_SET_ALARM, and VIRTIO_RTC_REQ_SET_ALARM_ENABLED.
+
+If the clock does not support alarm messages, the device MUST set status
+VIRTIO_RTC_S_ENODEV for VIRTIO_RTC_REQ_READ_ALARM,
+VIRTIO_RTC_REQ_SET_ALARM, and VIRTIO_RTC_REQ_SET_ALARM_ENABLED.
+
+For VIRTIO_RTC_REQ_READ_ALARM, the device MUST set field
+\field{alarm_time} to \field{driver_alarm_time}.
+
+For VIRTIO_RTC_REQ_READ_ALARM, the device MUST set flag
+\field{VIRTIO_RTC_FLAG_ALARM_ENABLED} in field \field{flags} if
+\field{alarm_enabled} is true, and clear the flag otherwise.
+
+For VIRTIO_RTC_REQ_READ_ALARM and for any clock type listed in this
+specification, the device MUST use the nanosecond as unit for the
+\field{alarm_time} field.
+
+If the device sets status VIRTIO_RTC_S_OK for VIRTIO_RTC_REQ_SET_ALARM,
+the device MUST set \field{driver_alarm_time} to the time
+represented by field \field{alarm_time}.
+
+If the device sets status VIRTIO_RTC_S_OK for VIRTIO_RTC_REQ_SET_ALARM,
+the device MUST set \field{alarm_enabled} to true if flag
+\field{VIRTIO_RTC_FLAG_ALARM_ENABLED} is set in field \field{flags}.
+
+If the device sets status VIRTIO_RTC_S_OK for VIRTIO_RTC_REQ_SET_ALARM,
+the device MUST set \field{alarm_enabled} to false if flag
+\field{VIRTIO_RTC_FLAG_ALARM_ENABLED} is cleared in field \field{flags}.
+
+If the device sets status VIRTIO_RTC_S_OK for
+VIRTIO_RTC_REQ_SET_ALARM_ENABLED, the device MUST set
+\field{alarm_enabled} to true if flag
+\field{VIRTIO_RTC_FLAG_ALARM_ENABLED} is set in field \field{flags}.
+
+If the device sets status VIRTIO_RTC_S_OK for
+VIRTIO_RTC_REQ_SET_ALARM_ENABLED, the device MUST set
+\field{alarm_enabled} to false if flag
+\field{VIRTIO_RTC_FLAG_ALARM_ENABLED} is cleared in field \field{flags}.
+
+If the device sets status VIRTIO_RTC_S_OK for
+VIRTIO_RTC_REQ_SET_ALARM_ENABLED, the device MUST retain
+\field{driver_alarm_time}.
+
\paragraph{Alarm Notifications}
If the alarmq is present, the driver should make buffers available in
@@ -785,3 +922,16 @@ \subsubsection{Alarm Operation}\label{sec:Device Types / RTC Device / Device Ope
\field{clock_id} identifies the expired alarm through its associated
clock.
+
+\drivernormative{\subparagraph}{Alarm Notifications}{Device Types / RTC Device / Device Operation / Alarm Operation / Alarm Notifications}
+
+If VIRTIO_RTC_F_ALARM was negotiated, the driver SHOULD populate the
+alarmq with buffers.
+
+The driver MUST allocate enough space for any alarmq message in the
+device-writable part of an alarmq buffer.
+
+\devicenormative{\subparagraph}{Alarm Notifications}{Device Types / RTC Device / Device Operation / Alarm Operation / Alarm Notifications}
+
+The device MUST NOT send a VIRTIO_RTC_NOTIF_ALARM notification for a
+clock which does not support alarm messages.
diff --git a/device-types/rtc/device-conformance.tex b/device-types/rtc/device-conformance.tex
index 4303cd450542..705691a7319f 100644
--- a/device-types/rtc/device-conformance.tex
+++ b/device-types/rtc/device-conformance.tex
@@ -3,7 +3,11 @@
An RTC device MUST conform to the following normative statements:
\begin{itemize}
+\item \ref{devicenormative:Device Types / RTC Device / Feature bits}
\item \ref{devicenormative:Device Types / RTC Device / Device Operation}
\item \ref{devicenormative:Device Types / RTC Device / Device Operation / Control Requests}
\item \ref{devicenormative:Device Types / RTC Device / Device Operation / Read Requests}
+\item \ref{devicenormative:Device Types / RTC Device / Device Operation / Alarm Operation}
+\item \ref{devicenormative:Device Types / RTC Device / Device Operation / Alarm Operation / Alarm Control Requests}
+\item \ref{devicenormative:Device Types / RTC Device / Device Operation / Alarm Operation / Alarm Notifications}
\end{itemize}
diff --git a/device-types/rtc/driver-conformance.tex b/device-types/rtc/driver-conformance.tex
index 689c18d158d0..a87c4cde99c2 100644
--- a/device-types/rtc/driver-conformance.tex
+++ b/device-types/rtc/driver-conformance.tex
@@ -6,4 +6,6 @@
\item \ref{drivernormative:Device Types / RTC Device / Device Operation}
\item \ref{drivernormative:Device Types / RTC Device / Device Operation / Control Requests}
\item \ref{drivernormative:Device Types / RTC Device / Device Operation / Read Requests}
+\item \ref{drivernormative:Device Types / RTC Device / Device Operation / Alarm Operation / Alarm Control Requests}
+\item \ref{drivernormative:Device Types / RTC Device / Device Operation / Alarm Operation / Alarm Notifications}
\end{itemize}
--
2.40.1
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [virtio-comment] [RFC PATCH v3 0/4] virtio-rtc: Add device specification
2023-12-18 6:42 [virtio-comment] [RFC PATCH v3 0/4] virtio-rtc: Add device specification Peter Hilber
` (3 preceding siblings ...)
2023-12-18 6:42 ` [virtio-comment] [RFC PATCH v3 4/4] virtio-rtc: Add normative statements for " Peter Hilber
@ 2024-07-25 4:53 ` Michael S. Tsirkin
2024-07-25 8:32 ` David Woodhouse
4 siblings, 1 reply; 24+ messages in thread
From: Michael S. Tsirkin @ 2024-07-25 4:53 UTC (permalink / raw)
To: Peter Hilber; +Cc: virtio-comment, Cornelia Huck, Parav Pandit
On Mon, Dec 18, 2023 at 07:42:49AM +0100, Peter Hilber wrote:
> This iteration of the virtio-rtc RFC spec
>
> - addresses the comments from Parav (patch 1),
>
> - separates the normative statements into a dedicated patch (patch 2), and
>
> - adds an optional alarm feature (patch 3-4).
>
> Summary
> -------
>
> The RTC (Real Time Clock) device provides information about current
> time. The device can provide different clocks, e.g. for the UTC or TAI
> time standards, or for physical time elapsed since some past epoch. The
> driver can read the clocks with simple or more accurate methods.
> Optionally, the driver can set an alarm.
What is the status here?
--
MST
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [virtio-comment] [RFC PATCH v3 0/4] virtio-rtc: Add device specification
2024-07-25 4:53 ` [virtio-comment] [RFC PATCH v3 0/4] virtio-rtc: Add device specification Michael S. Tsirkin
@ 2024-07-25 8:32 ` David Woodhouse
2024-08-09 12:53 ` Peter Hilber
0 siblings, 1 reply; 24+ messages in thread
From: David Woodhouse @ 2024-07-25 8:32 UTC (permalink / raw)
To: Michael S. Tsirkin, Peter Hilber
Cc: virtio-comment, Cornelia Huck, Parav Pandit
[-- Attachment #1: Type: text/plain, Size: 1407 bytes --]
On Thu, 2024-07-25 at 00:53 -0400, Michael S. Tsirkin wrote:
> On Mon, Dec 18, 2023 at 07:42:49AM +0100, Peter Hilber wrote:
> > This iteration of the virtio-rtc RFC spec
> >
> > - addresses the comments from Parav (patch 1),
> >
> > - separates the normative statements into a dedicated patch (patch 2), and
> >
> > - adds an optional alarm feature (patch 3-4).
> >
> > Summary
> > -------
> >
> > The RTC (Real Time Clock) device provides information about current
> > time. The device can provide different clocks, e.g. for the UTC or TAI
> > time standards, or for physical time elapsed since some past epoch. The
> > driver can read the clocks with simple or more accurate methods.
> > Optionally, the driver can set an alarm.
>
> What is the status here?
I believe Peter is on vacation and planning to publish an updated
version of the specification in the second week of August. From
https://lore.kernel.org/virtio-comment/85c93b42-41a2-42c4-a168-55079bbfff71@opensynergy.com/
dwmw2> With this, I think we're at a point where I can seriously propose this
dwmw2> be adopted into your virtio-rtc specification? Do you have that in a
dwmw2> public git tree that I can work from to provide an actual patch?
Peter> I am sorry, but I think I will not be able to publish a specification
Peter> update until I go on vacation. I will be back in the week of August 5th.
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [virtio-comment] [RFC PATCH v3 0/4] virtio-rtc: Add device specification
2024-07-25 8:32 ` David Woodhouse
@ 2024-08-09 12:53 ` Peter Hilber
0 siblings, 0 replies; 24+ messages in thread
From: Peter Hilber @ 2024-08-09 12:53 UTC (permalink / raw)
To: David Woodhouse, Michael S. Tsirkin
Cc: virtio-comment, Cornelia Huck, Parav Pandit
On 25.07.24 10:32, David Woodhouse wrote:
> On Thu, 2024-07-25 at 00:53 -0400, Michael S. Tsirkin wrote:
>> On Mon, Dec 18, 2023 at 07:42:49AM +0100, Peter Hilber wrote:
>>> This iteration of the virtio-rtc RFC spec
>>>
>>> - addresses the comments from Parav (patch 1),
>>>
>>> - separates the normative statements into a dedicated patch (patch 2), and
>>>
>>> - adds an optional alarm feature (patch 3-4).
>>>
>>> Summary
>>> -------
>>>
>>> The RTC (Real Time Clock) device provides information about current
>>> time. The device can provide different clocks, e.g. for the UTC or TAI
>>> time standards, or for physical time elapsed since some past epoch. The
>>> driver can read the clocks with simple or more accurate methods.
>>> Optionally, the driver can set an alarm.
>>
>> What is the status here?
The RFC spec patch series v4 was sent in May [1]. I plan to send out RFC
spec patch series v5, mostly according to the discussion with David, soon.
Unfortunately, due to the holiday season, it might still take one or two
weeks for internal reviews to conclude.
The Linux kernel driver implementation according to RFC spec series v5 is
still WIP. The implementation according to RFC spec v3 is available [2].
Best regards,
Peter
>
> I believe Peter is on vacation and planning to publish an updated
> version of the specification in the second week of August. From
> https://lore.kernel.org/virtio-comment/85c93b42-41a2-42c4-a168-55079bbfff71@opensynergy.com/
>
> dwmw2> With this, I think we're at a point where I can seriously propose this
> dwmw2> be adopted into your virtio-rtc specification? Do you have that in a
> dwmw2> public git tree that I can work from to provide an actual patch?
>
> Peter> I am sorry, but I think I will not be able to publish a specification
> Peter> update until I go on vacation. I will be back in the week of August 5th.
[1] https://lore.kernel.org/virtio-comment/20240522170003.52565-1-peter.hilber@opensynergy.com/T/#t
[2] https://lore.kernel.org/all/20231218073849.35294-1-peter.hilber@opensynergy.com/#r
^ permalink raw reply [flat|nested] 24+ messages in thread