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

[Parav Pandit <parav@nvidia.com>]

> 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?