Re: [RFC PATCH v3 7/7] virtio_rtc: Add RTC class driver

From: Peter Hilber <peter.hilber@opensynergy.com>
To: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: linux-kernel@vger.kernel.org, virtualization@lists.linux.dev,
	virtio-dev@lists.oasis-open.org, linux-rtc@vger.kernel.org,
	"Michael S. Tsirkin" <mst@redhat.com>,
	Jason Wang <jasowang@redhat.com>,
	Xuan Zhuo <xuanzhuo@linux.alibaba.com>,
	Alessandro Zummo <a.zummo@towertech.it>
Subject: Re: [RFC PATCH v3 7/7] virtio_rtc: Add RTC class driver
Date: Wed, 13 Mar 2024 10:13:44 +0100	[thread overview]
Message-ID: <976430eb-55fc-4811-bf29-a37eaea30885@opensynergy.com> (raw)
In-Reply-To: <202403111946106887ed4c@mail.local>

On 11.03.24 20:46, Alexandre Belloni wrote:
> On 11/03/2024 19:28:50+0100, Peter Hilber wrote:
>>>> Perhaps this might be handled by the driver also setting a virtio-rtc
>>>> monotonic alarm (which uses a clock similar to CLOCK_BOOTTIME_ALARM).
>>>> The
>>>> virtio-rtc monotonic alarm would just be used to wake up in case it was
>>>> a
>>>> CLOCK_BOOTTIME_ALARM alarm.
>>>>
>>>> Otherwise, the behavior should not differ from other RTC class drivers.
>>>>
>>>
>>> What I don't quite get is how this is actually related to RTCs. This
>>> would be a super imprecise mechanism to get the current time and date
>>> from the host to the guest which is what I think your are trying to do,
>>> especially since this is not supporting UIE.
>>> The host system clock may come from reading the RTC at some point in
>>> time but more likely from another source so is it really the best
>>> synchronization mechanism?
>>
>> Hello,
>>
>> thank you for your comments.
>>
>> The main motivation to have the RTC class driver is the RTC alarm
>> (discussed below).
>>
>> As for synchronization, virtio_rtc also offers a PTP clock [1] which will
>> be more precise, but which needs a user space daemon. As for RTC-based
>> initial synchronization, my idea was to propose, in a second step, an
>> optional op for rtc_class_ops, which would read the clock with nanosecond
>> precision. This optional op could then be used in rtc_hctosys(), so there
>> would be no need for UIE waiting.
> 
> This would be a clear NAK, rtc_hctosys should use UIE to have proper
> synchronisation. It currently does a very bad job reading the RTC and it
> is a pity it has been mandated by systemd as useerspace is definitively
> better placed to set the system time. I'm still very tempted delaying
> everyone's boot by one second and make rtc_hctosys precise for all the
> supported HW and not just a single driver.
> 

OK. I plan to add a PPS feature to virtio_rtc so that it can support UIE.
AFAIU this is not required for the initial driver version.

Thanks for the comments,

Peter

[...]