On Wed, 2024-03-13 at 10:45 +0100, Peter Hilber wrote: > On 12.03.24 18:15, David Woodhouse wrote: > > On Mon, 2024-03-11 at 19:24 +0100, Peter Hilber wrote: > > > On 08.03.24 13:33, David Woodhouse wrote: > > > > On Fri, 2024-03-08 at 11:32 +0100, Peter Hilber wrote: > > > > > On 07.03.24 15:02, David Woodhouse wrote: > > > > > > Hm, should we allow UTC? If you tell me the time in UTC, then > > > > > > (sometimes) I still don't actually know what the time is, because some > > > > > > UTC seconds occur twice. UTC only makes sense if you provide the TAI > > > > > > offset, surely? Should the virtio_rtc specification make it mandatory > > > > > > to provide such? > > > > > > > > > > > > Otherwise you're just designing it to allow crappy hypervisors to > > > > > > expose incomplete information. > > > > > > > > > > > > > > > > Hi David, > > > > > > > > > > (adding virtio-comment@xxxxxxxxxxxxxxxxxxxx for spec discussion), > > > > > > > > > > thank you for your insightful comments. I think I take a broadly similar > > > > > view. The reason why the current spec and driver is like this is that I > > > > > took a pragmatic approach at first and only included features which work > > > > > out-of-the-box for the current Linux ecosystem. > > > > > > > > > > The current virtio_rtc features work similar to ptp_kvm, and therefore > > > > > can work out-of-the-box with time sync daemons such as chrony. > > > > > > > > > > As of RFC spec v3, UTC clock only is allowed. If mandating a TAI clock > > > > > as well, I am afraid that > > > > > > > > > > - in some (embedded) scenarios, the TAI clock may not be available > > > > > > > > > > - crappy hypervisors will pass off the UTC clock as the TAI clock. > > > > > > > > > > For the same reasons, I am also not sure about adding a *mandatory* TAI > > > > > offset to each readout. I don't know user-space software which would > > > > > leverage this already (at least not through the PTP clock interface). > > > > > And why would such software not go straight for the TAI clock instead? > > > > > > > > > > How about adding a requirement to the spec that the virtio-rtc device > > > > > SHOULD expose the TAI clock whenever it is available - would this > > > > > address your concerns? > > > > > > > > I think that would be too easy for implementors to miss, or decide not > > > > to obey. Or to get *wrong*, by exposing a TAI clock but actually > > > > putting UTC in it. > > > > > > > > I think I prefer to mandate the tai_offset field with the UTC clock. > > > > Crappy implementations will just set it to zero, but at least that > > > > gives a clear signal to the guests that it's *their* problem to > > > > resolve. > > > > > > To me there are some open questions regarding how this would work. Is there > > > a use case for this with the v3 clock reading methods, or would it be > > > enough to address this with the Virtio timekeeper? > > > > > > Looking at clock_adjtime(2), the tai_offset could be exposed, but probably > > > best alongside some additional information about leap seconds. I am not > > > aware about any user-space user. In addition, leap second smearing should > > > also be addressed. > > > > > > > Is there even a standard yet for leap-smearing? Will it be linear over > > 1000 seconds like UTC-SLS? Or semi-raised-cosine over 24 hours, which I > > think is what Google does? Meta does something different again, don't > > they? > > > > Exposing UTC as the only clock reference is bad enough; when leap > > seconds happen there's a whole second during which you don't *know* > > which second it is. It seems odd to me, for a precision clock to be > > deliberately ambiguous about what the time is! > > Just to be clear, the device can perfectly expose only a TAI reference > clock (or both UTC and TAI), the spec is just completely open about this, > as it tries to work for diverse use cases. As long as the guest *knows* what it's getting, sure. > > > > But if the virtio-rtc clock is defined as UTC and then expose something > > *different* in it, that's even worse. You potentially end up providing > > inaccurate time for a whole *day* leading up to the leap second. > > > > I think you're right that leap second smearing should be addressed. At > > the very least, by making it clear that the virtio-rtc clock which > > advertises UTC shall be used *only* for UTC, never UTC-SLS or any other > > yet-to-be-defined variant. > > > > Agreed. > > > Please make it explicit that any hypervisor which wants to advertise a > > smeared clock shall define a new type which specifies the precise > > smearing algorithm and cannot be conflated with the one you're defining > > here. > > > > I will add a requirement that the UTC clock can never have smeared/smoothed > leap seconds. Thanks. > I think that not every vendor would bother to first add a definition of a > smearing algorithm. Also, I think in some cases knowing the precise > smearing algorithm might not be important (when having the same time as the > hypervisor is enough and accuracy w.r.t. actual time is less important). > > So maybe I should add a VIRTIO_RTC_CLOCK_UTC_SMEARED clock type, which for > now could catch every UTC-like clock which smears/smoothes leap seconds, > where the vendor cannot be bothered to add the smearing algorithm to spec > and implementations. Please $DEITY no. Surely the whole point of this effort is to provide guests with precise and *unambiguous* knowledge of what the time is? Using UTC is bad enough, because for a UTC timestamp in the middle of a leap second the guest can't know know *which* occurrence of that leap second it is, so it might be wrong by a second. To resolve that ambiguity needs a leap indicator and/or tai_offset field. But if you allow and encourage the use of smeared time without even a specification of *how* it's smeared... that's even worse. You have an unknown inaccuracy of up to a second for whole periods of time around a leap second. That's surely the *antithesis* of what we're trying to do here? Without an actual definition of the smearing, how is a guest actually supposed to know what time it is? (I suppose you could add a tai_offset_nanoseconds field? I don't know that I want to *encourage* that thought process...) > As for UTC-SLS, this *could* also be added, although [1] says > > It is inappropriate to use Internet-Drafts as reference material or > to cite them other than as "work in progress." > > [1] https://datatracker.ietf.org/doc/html/draft-kuhn-leapsecond-00 > > > > > One other thing to note is I think we're being very naïve about the TSC > > > > on x86 hosts. Theoretically, the TSC for every vCPU might run at a > > > > different frequency, and even if they run at the same frequency they > > > > might be offset from each other. I'm happy to be naïve but I think we > > > > should be *explicitly* so, and just say for example that it's defined > > > > against vCPU0 so if other vCPUs are different then all bets are off. > > > > > > ATM Virtio has no notion of vCPUs, or vCPU topology. So I wonder if you > > > have an opinion on how to represent this in a platform-independent way. > > > > Well, it doesn't have a notion of TSCs either; you include that by > > implicit reference don't you? > > I think I can add a SHOULD requirement which vaguely refers to vCPU 0, or > boot vCPU. But the Virtio device is not necessarily hosted by a hypervisor, > so the device might not even know which vCPUs there are. E.g. there is even > interest to make virtio-rtc work as part of the virtio-net device (which > might be implemented in hardware). Sure, but those implementations aren't going to offer the TSC pairing at all, are they?
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
