| Message ID | 20210920122535.269988-1-eric.auger@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] hw/rtc/pl031: Send RTC_CHANGE QMP event | expand |
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
On Thu, 23 Sept 2021 at 14:29, Peter Maydell <peter.maydell@linaro.org> wrote: > > 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. Ping? Review from somebody who understands rtc_change would still be nice... -- PMM
On 11/1/21 17:04, Peter Maydell wrote: > On Thu, 23 Sept 2021 at 14:29, Peter Maydell <peter.maydell@linaro.org> wrote: >> >> 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. > > Ping? Review from somebody who understands rtc_change would > still be nice... The change looks good to me (sorry I missed this v2). x86 also has some logic in the migration post-load, that might end up sending the event. However, that's best done separately after understanding and documenting exactly what x86 is doing. Paolo
On Mon, 15 Nov 2021 at 12:49, Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 11/1/21 17:04, Peter Maydell wrote: > > On Thu, 23 Sept 2021 at 14:29, Peter Maydell <peter.maydell@linaro.org> wrote: > >> > >> 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. > > > > Ping? Review from somebody who understands rtc_change would > > still be nice... > > The change looks good to me (sorry I missed this v2). x86 also has some > logic in the migration post-load, that might end up sending the event. > However, that's best done separately after understanding and documenting > exactly what x86 is doing. Thanks; I've applied this to target-arm.next for 6.2. -- PMM
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);