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

[Peter Hilber <peter.hilber@opensynergy.com>]

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