Re: [PATCH v8 4/4] virtio-rtc: Add normative statements for alarm feature

[Matias Ezequiel Vara Larsen <mvaralar@redhat.com>]

On Thu, Mar 06, 2025 at 10:51:12AM +0100, Peter Hilber wrote:
> Add the normative statements for the alarm feature added previously.
> 
> Signed-off-by: Peter Hilber <quic_philber@quicinc.com>
> ---
> 
> Notes:
>     v8:
>     
>     - Explicitly describe alarm enabled status, where relevant (Matias
>       Ezequiel Vara Larsen).
>     
>     - Simplify requirement about clock readings after alarm (Matias Ezequiel
>       Vara Larsen).
>     
>     - Move requirements to serve alarm expiration events before requirements
>       about stopping to serve (Matias Ezequiel Vara Larsen).
>     
>     - Change word order from "field X" to "the X field" (Matias Ezequiel
>       Vara Larsen).
>     
>     - Change word order from "flag X" to "the X flag" or "X".
>     
>     v7:
>     
>     - Remove inadvertent v6 changes, which should have been part of
>       preceding patches.
>     
>     v4:
>     
>     - Update normative statements to match v4 changes to non-normative
>       statements (patch 3).
>     
>     - Driver should read clock to confirm that alarm has expired.
>     
>     - Formatting and wording improvements.
> 
>  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 93bcf178b042..91afae7c2110 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.
> +

I think you can remove `can` here.

>  \subsection{Device configuration layout}\label{sec:Device Types / RTC Device / Device configuration layout}
>  
>  There is no configuration data for the device.
> @@ -712,6 +717,11 @@ \subsubsection{Read Requests}\label{sec:Device Types / RTC Device / Device Opera
>  specification, the device MUST use the nanosecond as unit for the
>  \field{clock_reading} field.
>  
> +If the device sent an alarm notification for clock C with alarm time A,
> +the device MUST, for all read requests of C which the driver marks as
> +available after the notification, return a \field{clock_reading} which
> +does not precede A (except if C stepped backwards to 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
> @@ -819,6 +829,76 @@ \subsubsection{Alarm Operation}\label{sec:Device Types / RTC Device / Device Ope
>  To prevent the above issues, the driver also marks buffers in the alarmq
>  as available only after completing the above steps for all clocks.
>  
> +\devicenormative{\paragraph}{Alarm Operation}{Device Types / RTC Device / Device Operation / Alarm Operation}
> +
> +The device MAY retain both alarm time and alarm enabled status of a
> +clock across a device reset.
> +
> +If the device did not retain alarm time and alarm enabled status of a
> +clock across a device reset, the device MUST initialize alarm time to 0.
> +
> +If the device did not retain alarm time and alarm enabled status of a
> +clock across a device reset, the device MUST disable the alarm.
> +
> +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.

`has been negotiated ...` I think there are others below.

> +
> +The device MUST set the VIRTIO_RTC_FLAG_ALARM_CAP flag 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, while the
> +alarm is enabled.
> +
> +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, while also setting the alarm to enabled.
> +
> +The device MUST consider it an alarm expiration event when the driver
> +sets the alarm to enabled, if the clock has already reached or passed
> +the alarm time.
> +
> +If the device retained alarm time and alarm enabled status 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 the alarm is enabled.
> +
> +If an alarm expiration event E happens, the device MUST start serving
> +the alarm expiration event E.
> +
> +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.
> +
> +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.
> +
> +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.
> +
>  \paragraph{Alarm Control Requests}
>  
>  If VIRTIO_RTC_F_ALARM is negotiated,
> @@ -913,6 +993,59 @@ \subsubsection{Alarm Operation}\label{sec:Device Types / RTC Device / Device Ope
>  
>  \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 the
> +\field{alarm_time} field to the alarm time.
> +
> +For VIRTIO_RTC_REQ_READ_ALARM, the device MUST set the
> +VIRTIO_RTC_FLAG_ALARM_ENABLED flag in the \field{flags} field if the
> +alarm is enabled, 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.
> +
> +For VIRTIO_RTC_REQ_SET_ALARM, the device MUST accept any
> +\field{alarm_time} value.
> +
> +If the device sets status VIRTIO_RTC_S_OK for VIRTIO_RTC_REQ_SET_ALARM,
> +the device MUST set the alarm time to the time represented by the
> +\field{alarm_time} field.
> +
> +If the device sets status VIRTIO_RTC_S_OK for VIRTIO_RTC_REQ_SET_ALARM,
> +the device MUST enable the alarm if VIRTIO_RTC_FLAG_ALARM_ENABLED is set
> +in the \field{flags} field.
> +
> +If the device sets status VIRTIO_RTC_S_OK for VIRTIO_RTC_REQ_SET_ALARM,
> +the device MUST disable the alarm if VIRTIO_RTC_FLAG_ALARM_ENABLED is
> +cleared in the \field{flags} field.
> +
> +If the device sets status VIRTIO_RTC_S_OK for
> +VIRTIO_RTC_REQ_SET_ALARM_ENABLED, the device MUST enable the alarm if
> +VIRTIO_RTC_FLAG_ALARM_ENABLED is set in the \field{flags} field.
> +
> +If the device sets status VIRTIO_RTC_S_OK for
> +VIRTIO_RTC_REQ_SET_ALARM_ENABLED, the device MUST disable the alarm if
> +VIRTIO_RTC_FLAG_ALARM_ENABLED is cleared in the \field{flags} field.
> +
> +If the device sets status VIRTIO_RTC_S_OK for
> +VIRTIO_RTC_REQ_SET_ALARM_ENABLED, the device MUST retain the alarm time.
> +
>  \paragraph{Alarm Notifications}
>  
>  If the alarmq is present, the driver should make buffers available in
> @@ -948,3 +1081,20 @@ \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.
> +
> +If the driver receives a VIRTIO_RTC_NOTIF_ALARM notification, the driver
> +SHOULD read the associated clock instead of assuming that the alarm time
> +which the driver set last has been reached.
> +
> +\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.43.0
>