Hi Peter, On 9/16/21 8:24 PM, Peter Maydell wrote: > On Thu, 16 Sept 2021 at 18:19, Eric Auger <eric.au...@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));
Thank you for the explanation. It works on my end and indeed looks similar to what other rtc do. I will respin accordingly adding your Sob. Thanks Eric > > 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 >
