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

[Peter Maydell <peter.maydell@linaro.org>]

On Mon, 20 Sept 2021 at 13:25, 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 which is reported corresponds to the difference between the guest
> reference time and the reference time kept in softmmu/rtc.c.
>
> For instance adding 20s to the guest RTC value will report 20. Adding
> an extra 20s to the guest RTC value will report 20 + 20 = 40.
>
> The inclusion of qapi/qapi-types-misc-target.h in hw/rtl/pl031.c
> require to compile the PL031 with specific_ss.add() to avoid
> ./qapi/qapi-types-misc-target.h:18:13: error: attempt to use poisoned
> "TARGET_<ARCH>".
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Thanks. This looks plausible to me (well, it would ;-)) but
I would appreciate review from Paolo or somebody else who
understands the rtc_change feature and handling.

> ---
>
> v1 -> v2:
> - Use Peter's implementation and remove subsection
>
> Tested with the following script run on guest:
>
>   #!/bin/sh
>   old=$(hwclock --show | cut -f1-7 -d' ')
>   oldabs=$(date +%s -d "$old")
>   newabs=$(expr $oldabs + $1)
>   new=$(date -d @"$newabs")
>   echo Old: $oldabs $old
>   echo New: $newabs $new
>   hwclock --set --date "$new"
>
> and compared with x86 behavior.
> ---
>  hw/rtc/meson.build |  2 +-
>  hw/rtc/pl031.c     | 10 +++++++++-
>  2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/hw/rtc/meson.build b/hw/rtc/meson.build
> index 7cecdee5ddb..8fd8d8f9a71 100644
> --- a/hw/rtc/meson.build
> +++ b/hw/rtc/meson.build
> @@ -2,7 +2,7 @@
>  softmmu_ss.add(when: 'CONFIG_DS1338', if_true: files('ds1338.c'))
>  softmmu_ss.add(when: 'CONFIG_M41T80', if_true: files('m41t80.c'))
>  softmmu_ss.add(when: 'CONFIG_M48T59', if_true: files('m48t59.c'))
> -softmmu_ss.add(when: 'CONFIG_PL031', if_true: files('pl031.c'))
> +specific_ss.add(when: 'CONFIG_PL031', if_true: files('pl031.c'))
>  softmmu_ss.add(when: 'CONFIG_TWL92230', if_true: files('twl92230.c'))
>  softmmu_ss.add(when: ['CONFIG_ISA_BUS', 'CONFIG_M48T59'], if_true: files('m48t59-isa.c'))
>  softmmu_ss.add(when: 'CONFIG_XLNX_ZYNQMP', if_true: files('xlnx-zynqmp-rtc.c'))
> diff --git a/hw/rtc/pl031.c b/hw/rtc/pl031.c
> index 2bbb2062ac8..e7ced90b025 100644
> --- a/hw/rtc/pl031.c
> +++ b/hw/rtc/pl031.c
> @@ -24,6 +24,7 @@
>  #include "qemu/log.h"
>  #include "qemu/module.h"
>  #include "trace.h"
> +#include "qapi/qapi-events-misc-target.h"
>
>  #define RTC_DR      0x00    /* Data read register */
>  #define RTC_MR      0x04    /* Match register */
> @@ -136,10 +137,17 @@ static void pl031_write(void * opaque, hwaddr offset,
>      trace_pl031_write(offset, value);
>
>      switch (offset) {
> -    case RTC_LR:
> +    case RTC_LR: {
> +        struct tm tm;
> +
>          s->tick_offset += value - pl031_get_count(s);
> +
> +        qemu_get_timedate(&tm, s->tick_offset);
> +        qapi_event_send_rtc_change(qemu_timedate_diff(&tm));
> +
>          pl031_set_alarm(s);
>          break;
> +    }
>      case RTC_MR:
>          s->mr = value;
>          pl031_set_alarm(s);

-- PMM