Re: [PATCH] hw/rtc/pl031: Send RTC_CHANGE QMP event

[Peter Maydell]

On Thu, 16 Sept 2021 at 18:19, Eric Auger <eric.auger@redhat.com> wrote:
>
> Hi Peter,
> On 9/16/21 3:32 PM, Peter Maydell wrote:
> > None of the other users of qapi_event_send_rtc_change()
> > seem to have to track the baseline time like this. Shouldn't
> > this be doing something involving using qemu_ref_timedate()
> > as the baseline that it uses to figure out the change value ?
> > (The other users do this via qemu_timedate_diff() but since we
> > start with a value-in-seconds we might want to expose some other
> > API in softmmu/rtc.c.)
>
> I struggled understanding the various kinds of clocks modeled in qemu
> and the PL031 implementation. Both devices calling
> qapi_event_send_rtc_change() seem to store the base rtc in their state
> (mc146818rtc.c cmos data or spapr_rtc tas_ld(args, )) and then
> effectivelly call qemu_timedate_diff() on this base rtc value. I did not
> figure to do the equivalent with the pl031. I will further investigate
> how I can mimic their implementation though.

mc146818rtc.c calls qapi_event_send_rtc_change() in rtc_set_time().
It looks to me like that is just "when the guest writes to an
RTC register such that the guest RTC time has changed, use
qemu_timedate_diff() to find out the delta between that and what
the softmmu/rtc.c clock has as its baseline time, and then pass
that delta in seconds to qapi_event_send_rtc_change()".
This RTC has a lot of separate day/month/year registers, so the
implementation's idea of "current guest RTC setting" is a
complicated thing that it fills in into a struct tm, and which
qemu_timedate_diff() then parses back out into a "seconds" value.

spapr_rtc() is a hypercall implementation, so the guest passes it
a complete set of year/month/day/etc values all at once to set the time.

pl031 is a lot simpler as it is essentially just a count of
time in seconds, which we set up as "seconds since the Unix epoch".
But the basic principle is the same: the device contains the state
that tells you what it thinks the guest RTC value is now, and the
value we want to pass qapi_event_send_rtc_change() is the
difference between that and the reference time kept in
softmmu/rtc.c.

I think what you want is probably:

    struct tm tm;

    qemu_get_timedate(&tm, s->tick_offset);
    qapi_event_send_rtc_change(qemu_timedate_diff(&tm));

But I'm not 100% sure. I also feel like this is doing a
roundtrip from seconds to struct tm to seconds again, which
seems a bit silly (but it might matter if the rtc is configured
to 'localtime'? At any rate it's not completely clear that
it's always a no-op roundtrip).

I've cc'd Paolo who might be a bit more familiar with the rtc code
than I am.

-- PMM