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: Mon, 11 Mar 2024 19:28:50 +0100 [thread overview]
Message-ID: <a4babfc9-f4da-4dfe-8431-eb819f5801eb@opensynergy.com> (raw)
In-Reply-To: <2024030817030063a909f0@mail.local>
On 08.03.24 18:03, Alexandre Belloni wrote:
> Hello,
>
> I'll start by saying that I'm sorry, I have a very very high level
> knowledge about what virtio is.
>
> On 18/12/2023 08:38:45+0100, Peter Hilber wrote:
>> Expose the virtio-rtc UTC clock as an RTC clock to userspace, if it is
>> present. Support RTC alarm if the virtio-rtc alarm feature is present.
>> The
>> virtio-rtc device signals an alarm by marking an alarmq buffer as used.
>>
>> Peculiarities
>> -------------
>>
>> A virtio-rtc clock is a bit special for an RTC clock in that
>>
>> - the clock may step (also backwards) autonomously at any time and
>>
>> - the device, and its notification mechanism, will be reset during boot
>> or
>> resume from sleep.
>>
>> The virtio-rtc device avoids that the driver might miss an alarm. The
>> device signals an alarm whenever the clock has reached or passed the
>> alarm
>> time, and also when the device is reset (on boot or resume from sleep),
>> if
>> the alarm time is in the past.
>>
>> Open Issue
>> ----------
>>
>> The CLOCK_BOOTTIME_ALARM will use the RTC clock to wake up from sleep,
>> and
>> implicitly assumes that no RTC clock steps will occur during sleep. The
>> RTC
>> class driver does not know whether the current alarm is a real-time
>> alarm
>> or a boot-time alarm.
>>
>> 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.
[1] https://lore.kernel.org/all/20231218073849.35294-6-peter.hilber@opensynergy.com/
>
> The other thing is that I don't quite get the point of the RTC alarm
> versus a regular timer in this context.
RTC alarms allow to resume from suspend and poweroff (esp. also through
alarmtimers), which is of interest in embedded virtualization. In my
understanding RTC is ATM the only way to do this.
(I was indeed thinking about adding an alternate alarmtimer backend for
CLOCK_BOOTTIME_ALARM, which should deal with the CLOCK_REALTIME_ALARM vs
CLOCK_BOOTTIME_ALARM issue which is described in the commit message.)
>
>
> [...]
>
>> +static const struct rtc_class_ops viortc_class_with_alarm_ops = {
>> + .read_time = viortc_class_read_time,
>> + .read_alarm = viortc_class_read_alarm,
>> + .set_alarm = viortc_class_set_alarm,
>> + .alarm_irq_enable = viortc_class_alarm_irq_enable,
>> +};
>> +
>> +static const struct rtc_class_ops viortc_class_no_alarm_ops = {
>> + .read_time = viortc_class_read_time,
>> +};
>> +
>
> [...]
>
>> +/**
>> +/**
>> + * viortc_class_init() - init RTC class wrapper and device
>> + * @viortc: device data
>> + * @vio_clk_id: virtio_rtc clock id
>> + * @have_alarm: expose alarm ops
>> + * @parent_dev: virtio device
>> + *
>> + * Context: Process context.
>> + * Return: RTC class wrapper on success, ERR_PTR otherwise.
>> + */
>> +struct viortc_class *viortc_class_init(struct viortc_dev *viortc,
>> + u16 vio_clk_id, bool have_alarm,
>> + struct device *parent_dev)
>> +{
>> + struct viortc_class *viortc_class;
>> + struct rtc_device *rtc;
>> +
>> + viortc_class =
>> + devm_kzalloc(parent_dev, sizeof(*viortc_class),
>> GFP_KERNEL);
>> + if (!viortc_class)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + viortc_class->viortc = viortc;
>> +
>> + rtc = devm_rtc_allocate_device(parent_dev);
>> + if (IS_ERR(rtc))
>> + return ERR_PTR(PTR_ERR(rtc));
>> +
>> + viortc_class->rtc = rtc;
>> +
>> + clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->features);
>> +
>> + rtc->ops = have_alarm ? &viortc_class_with_alarm_ops :
>> + &viortc_class_no_alarm_ops;
>
> Don't do this, simply clear the alarm feature.
>
OK (sorry, was obviously very inelegant).
Best regards,
Peter
next prev parent reply other threads:[~2024-03-11 18:29 UTC|newest]
Thread overview: 84+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-18 7:38 [RFC PATCH v3 0/7] Add virtio_rtc module and related changes Peter Hilber
2023-12-18 7:38 ` [virtio-dev] " Peter Hilber
2023-12-18 7:38 ` Peter Hilber
2023-12-18 7:38 ` [RFC PATCH v3 1/7] timekeeping: Fix cross-timestamp interpolation on counter wrap Peter Hilber
2024-02-19 16:35 ` [tip: timers/core] " tip-bot2 for Peter Hilber
2023-12-18 7:38 ` [RFC PATCH v3 2/7] timekeeping: Fix cross-timestamp interpolation corner case decision Peter Hilber
2024-02-19 16:35 ` [tip: timers/core] " tip-bot2 for Peter Hilber
2023-12-18 7:38 ` [RFC PATCH v3 3/7] timekeeping: Fix cross-timestamp interpolation for non-x86 Peter Hilber
2024-02-19 16:35 ` [tip: timers/core] " tip-bot2 for Peter Hilber
2023-12-18 7:38 ` [RFC PATCH v3 4/7] virtio_rtc: Add module and driver core Peter Hilber
2023-12-18 7:38 ` [virtio-dev] " Peter Hilber
2023-12-18 7:38 ` Peter Hilber
2023-12-18 7:38 ` [RFC PATCH v3 5/7] virtio_rtc: Add PTP clocks Peter Hilber
2023-12-18 7:38 ` [virtio-dev] " Peter Hilber
2023-12-18 7:38 ` Peter Hilber
2024-06-15 8:01 ` David Woodhouse
2024-06-20 12:01 ` Peter Hilber
2024-06-20 14:33 ` David Woodhouse
2023-12-18 7:38 ` [RFC PATCH v3 6/7] virtio_rtc: Add Arm Generic Timer cross-timestamping Peter Hilber
2023-12-18 7:38 ` [virtio-dev] " Peter Hilber
2023-12-18 7:38 ` Peter Hilber
2024-06-15 7:50 ` David Woodhouse
2024-06-20 12:06 ` Peter Hilber
2023-12-18 7:38 ` [RFC PATCH v3 7/7] virtio_rtc: Add RTC class driver Peter Hilber
2023-12-18 7:38 ` [virtio-dev] " Peter Hilber
2024-03-08 17:03 ` Alexandre Belloni
2024-03-11 18:28 ` Peter Hilber [this message]
2024-03-11 19:46 ` Alexandre Belloni
2024-03-13 9:13 ` Peter Hilber
2024-03-07 14:02 ` [RFC PATCH v3 0/7] Add virtio_rtc module and related changes David Woodhouse
2024-03-07 14:02 ` David Woodhouse
2024-03-08 10:32 ` Peter Hilber
2024-03-08 10:32 ` Peter Hilber
2024-03-08 12:33 ` David Woodhouse
2024-03-08 12:33 ` David Woodhouse
2024-03-11 18:24 ` Peter Hilber
2024-03-11 18:24 ` Peter Hilber
2024-03-12 17:15 ` David Woodhouse
2024-03-12 17:15 ` David Woodhouse
2024-03-13 9:45 ` Peter Hilber
2024-03-13 9:45 ` Peter Hilber
2024-03-13 11:18 ` Alexandre Belloni
2024-03-13 11:18 ` Alexandre Belloni
2024-03-13 12:29 ` David Woodhouse
2024-03-13 12:29 ` David Woodhouse
2024-03-13 12:58 ` Alexandre Belloni
2024-03-13 12:58 ` Alexandre Belloni
2024-03-13 14:06 ` David Woodhouse
2024-03-13 14:06 ` David Woodhouse
2024-03-13 14:50 ` Alexandre Belloni
2024-03-13 14:50 ` Alexandre Belloni
2024-03-13 20:12 ` Andrew Lunn
2024-03-13 20:12 ` Andrew Lunn
2024-03-14 9:13 ` Peter Hilber
2024-03-14 9:13 ` Peter Hilber
2024-03-13 17:50 ` Peter Hilber
2024-03-13 17:50 ` Peter Hilber
2024-03-13 14:15 ` Peter Hilber
2024-03-13 14:15 ` Peter Hilber
2024-03-13 12:45 ` David Woodhouse
2024-03-13 12:45 ` David Woodhouse
2024-03-13 17:50 ` Peter Hilber
2024-03-13 17:50 ` Peter Hilber
2024-03-13 18:18 ` David Woodhouse
2024-03-13 18:18 ` David Woodhouse
2024-03-14 10:13 ` Peter Hilber
2024-03-14 10:13 ` Peter Hilber
2024-03-14 14:19 ` David Woodhouse
2024-03-14 14:19 ` David Woodhouse
2024-03-19 13:47 ` Peter Hilber
2024-03-19 13:47 ` Peter Hilber
2024-03-20 17:22 ` David Woodhouse
2024-03-20 17:22 ` David Woodhouse
2024-06-15 8:40 ` David Woodhouse
2024-06-20 12:37 ` Peter Hilber
2024-06-20 16:19 ` David Woodhouse
2024-06-21 8:45 ` David Woodhouse
2024-06-25 19:01 ` [RFC PATCH v2] ptp: Add vDSO-style vmclock support David Woodhouse
2024-06-25 21:34 ` Thomas Gleixner
2024-06-25 21:48 ` David Woodhouse
2024-06-25 22:22 ` John Stultz
2024-06-26 8:32 ` David Woodhouse
2024-06-26 16:43 ` Richard Cochran
2024-06-21 14:02 ` [RFC PATCH v3 0/7] Add virtio_rtc module and related changes David Woodhouse
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a4babfc9-f4da-4dfe-8431-eb819f5801eb@opensynergy.com \
--to=peter.hilber@opensynergy.com \
--cc=a.zummo@towertech.it \
--cc=alexandre.belloni@bootlin.com \
--cc=jasowang@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rtc@vger.kernel.org \
--cc=mst@redhat.com \
--cc=virtio-dev@lists.oasis-open.org \
--cc=virtualization@lists.linux.dev \
--cc=xuanzhuo@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.