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

[Cornelia Huck <cohuck@redhat.com>]

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.