Re: [virtio-comment] [RFC PATCH v3 3/4] virtio-rtc: Add alarm feature

[Cornelia Huck <cohuck@redhat.com>]

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.