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

[Eric Auger <eric.auger@redhat.com>]

Hi Peter,
On 9/16/21 3:32 PM, Peter Maydell wrote:
> On Thu, 9 Sept 2021 at 13:24, Eric Auger <eric.auger@redhat.com> wrote:
>> The PL031 currently is not able to report guest RTC change to the QMP
>> monitor as opposed to mc146818 or spapr RTCs. This patch adds the call
>> to qapi_event_send_rtc_change() when the Load Register is written. The
>> value that is reported corresponds to the difference between the new
>> RTC value and the RTC value elapsed since the base.
>>
>> For instance adding 20s to the guest RTC value will report 20:
>> {'timestamp': {'seconds': 1631189494, 'microseconds': 16932},
>> 'event': 'RTC_CHANGE', 'data': {'offset': 20}}
>>
>> Adding another extra 20s to the guest RTC value will report 40:
>> {'timestamp': {'seconds': 1631189498, 'microseconds': 9708},
>> 'event': 'RTC_CHANGE', 'data': {'offset': 40}}
>>
>> To compute the offset we need to track the origin tick_offset (the one
>> computed at init time). So we need to migrate that field, which is done
>> in a dedicated subsection. The migration of this subsection is disabled
>> for machine types less or equal than 6.1.
>>
>> After migration, adding an extra 20s on the destination returns 60:
>> {'timestamp': {'seconds': 1631189522, 'microseconds': 13081},
>> 'event': 'RTC_CHANGE', 'data': {'offset': 60}}
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>
>> @@ -138,6 +140,7 @@ static void pl031_write(void * opaque, hwaddr offset,
>>      switch (offset) {
>>      case RTC_LR:
>>          s->tick_offset += value - pl031_get_count(s);
>> +        qapi_event_send_rtc_change(s->tick_offset - s->original_tick_offset);
>>          pl031_set_alarm(s);
>>          break;
>>      case RTC_MR:
> 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.

Thanks

Eric

>
> thanks
> -- PMM
>