RE: [virtio-comment] [RFC PATCH v2] virtio-rtc: Add device specification.

[Parav Pandit <parav@nvidia.com>]

> From: Peter Hilber <peter.hilber@opensynergy.com>
> Sent: Wednesday, October 18, 2023 9:44 PM
> 
> On 18.10.23 07:36, Parav Pandit wrote:
> >
> >> From: Peter Hilber <peter.hilber@opensynergy.com>
> >> Sent: Tuesday, October 17, 2023 9:36 PM
> >> To: Parav Pandit <parav@nvidia.com>;
> >> virtio-comment@lists.oasis-open.org
> >> Cc: Cornelia Huck <cohuck@redhat.com>
> >> Subject: Re: [virtio-comment] [RFC PATCH v2] virtio-rtc: Add device
> >> specification.
> >>
> >> On 17.10.23 06:06, Parav Pandit wrote:
> >>>> From: Peter Hilber <peter.hilber@opensynergy.com>
> >>>> Sent: Friday, October 13, 2023 9:00 PM
> >>>>
> >>>> On 13.10.23 09:40, Parav Pandit wrote:
> >>>>>
> >>>>>> From: Peter Hilber <peter.hilber@opensynergy.com
> >>>>>
> >>>>>> Sent: Thursday, October 12, 2023 4:26 PM
> >>>>>
> >>>>>>> So am I missing something?
> >>>>>>> In an arbitrary, interface, once unplug is successful, i.e. when
> >>>>>>> the device exactly know that unplug is completed by the driver,
> >>>>>>> It can re-use the
> >>>>>> id.
> >>>>>>
> >>>>>> But this would require the device to remember previously
> >>>>>> unplugged
> >> clocks.
> >>>>>>
> >>>>> Not always, it can do when the clock wrap arounds.
> >>>>
> >>>> Sorry, I do not understand the reply. When the clock id is wrapping
> >>>> around, in the discussed scenario the device would need to know
> >>>> which clock had assigned id 0 in the past to avoid potential
> >>>> misinterpretation by the
> >> driver.
> >>>>
> >>>>> Since this interface does not have the complexity of hotplug and
> >>>>> unplug of the
> >>>> clocks, we should just keep it simple to a reasonable number of clocks.
> >>>>> The 64-bit value is not hurting at all, I just feel it is built for unknown.
> >>>>>
> >>>>> Any reason to not do hot plug/unplug of clocks now?
> >>>>>
> >>>>
> >>>> Current use cases do not require hotplugging, but I tried to make
> >>>> the spec future-proof.
> >>>>
> >>> If there is no use case for hotplugging a clock, we should keep
> >>> things simple
> >> to a reasonable width.
> >>>
> >>>
> >>
> >> I will change the width to 8 bit, although I don't think that makes things
> simpler.
> > If there are no hotplug clocks, than 8-bit or 16-bit is just enough right for
> reasonable number of clocks.
> > I donât have strong opinion for bit width.
> > 64-bit width without hot plug case seemed very large.
> > Please the practical range that you find suitable.
> >
> 
> OK, I will use 16 bits, this should in the context of the current spec satisfy all
> imaginable use cases.
> 
> >>
> >>>>>>>
> >>>>>>> This would work with 8-bit too.
> >>>>>>> Not that I have issue with 64-bits, but the use case for large
> >>>>>>> number of clocks
> >>>>>> under single virtio-rtc device is unclear.
> >>>>>>> And if there is no use case, we should have reasonable range.
> >>>>>>>
> >>>>>>
> >>>>>> Using a large id range might help in the future to avoid dealing
> >>>>>> with the whole class of id ABA problems, cf. the above example.
> >>>>> In my view the clocks are expected to give precision and accuracy,
> >>>>> so number
> >>>> of clocks in a system that occupies 64-bit wide sounds distant.
> >>>>> If it is used to not_care about finding the free id, it is ok.
> >>>>>
> >>>>
> >>>> Not caring about finding a concurrency-safe free id was the reason
> >>>> for 64 bits width.
> >>>>
> >>>>> Is hotplug unplug the clock (instead of device) is critical that
> >>>>> drives the clock
> >>>> id?
> >>>>>
> >>>>
> >>>> Clock id width > 8 bit was only considered to be able to easily
> >>>> support hotplugging in the future.
> >>>>
> >>>>>>
> >>>>>> To me using 64 bits to avoid the potential future ABA problems
> >>>>>> looks reasonable, but if others disagree, I can reduce to 8 bits.
> >>>>>>
> >>>>> It is not must, I just find the 64-bit level clocks a bit too much
> >>>>> specially at
> >>>> hotplug/unplug part.
> >>>>>
> >>>>
> >>>>
> >>>>>>>> With "unclear responsibility" I meant to refer to the
> >>>>>>>> responsibility for drafting a spec extension for virtio-net,
> >>>>>>>
> >>>>>>> You can count me to draft the virtio-net spec for responsibility.
> >>>>>>> I am sure others would join as there is common interest across
> >>>>>>> multiple device
> >>>>>> implementors to support it.
> >>>>>>> We know at least 4 stake holders have interests in it in
> >>>>>>> virtio-net (Alibaba,
> >>>>>> google, marvell, nvidia).
> >>>>>>>
> >>>>>>
> >>>>>> OK, thank you for the information.
> >>>>>>
> >>>>>> I think the arguments on how to have the RTC spec interact with
> >>>>>> the net device are now on the table. Maybe the discussion could
> >>>>>> continue on a draft spec extension for the net device?
> >>>>>>
> >>>>> I didnât follow the comment.
> >>>>> My suggestion is lets draft the spec _now_ that covers rtc
> >>>>> functionality as part
> >>>> of virtio-net device and as independent rtc device.
> >>>>> And if that requires me to work and draft the net part for you, I
> >>>>> am here to
> >>>> draft it.
> >>>>>
> >>>>> Would it work for you?
> >>>>>
> >>>>
> >>>> Of course, work on the net part could also start now.
> >>>>
> >>>> I would prefer if the net part could be discussed as a dedicated
> >>>> patch, on top of the patch(es) introducing the RTC device (and
> >>>> referring to
> >> those).
> >>>>
> >>> It will be separate patch (or patches) in device-type/net/description.tex.
> >>> It should be part of same patch series as rtc and net so both
> >>> devices agree
> >> and reserve necessary opcode ranges etc to utilize same infra.
> >>>
> >>
> >> I plan to send out RTC spec RFC v3, which should contain all the
> >> agreed changes from the v2 discussion, in November. Maybe the net
> >> related changes could then be put on top of that (and become part of a
> common RFC v4)?
> >>
> > Sure, we can possibly converge before posting v3 itself so that v4 does not
> require rewrite due to net.
> > This can be done by drafting v3 in such a way that it fits the v4 scenario.
> 
> AFAIU the only interoperability issue which was brought up in this discussion so
> far, for which there was no explicit agreement on resolution, is how to
> encapsulate RTC messages in the net controlq.
> 
> For this I propose to encapsulate the RTC requests resp. responses in the net
> controlq pseudo-fields command-specific-data resp.
> command-specific-result. (command-specific-result is being introduced through
> [1].)
> 
> This handles natural alignment of the payload in a way which is compatible with
> 
> - existing virtio-net controlq commands (including those being introduced
>   by [1]), and
> 
> - with the standalone virtio-rtc messages.

I will need to re-read the rtc v2, but the generic approach I had in mind is to take the ctrl vq opcode being common between net and rtc.
And follow the net's cvq command response format (donât recall if fullfils rtc or not).
And thatâs it. Rtc and net gets both same command, same response.
Net will have feature bit to indicate this functionality exists, for rtc it is implicit as its being and rtc device.
 
> [1] https://lore.kernel.org/virtio-comment/20231009013135.55890-1-
> xuanzhuo@linux.alibaba.com/#iZ31device-types:net:description.tex