Re: [PATCH] mc146818rtc: add a way to generate RTC interrupts via QMP

[Daniil Tatianin]

On 4/26/24 11:39 AM, Markus Armbruster wrote:

> Daniil Tatianin <d-tatianin@yandex-team.ru> writes:
>
> > This can be used to force-synchronize the time in guest after a long
> > stop-cont pause, which can be useful for serverless-type workload.
> >
> > Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
> > ---
> >   hw/rtc/mc146818rtc.c         | 15 +++++++++++++++
> >   include/hw/rtc/mc146818rtc.h |  1 +
> >   qapi/misc-target.json        | 16 ++++++++++++++++
> >   3 files changed, 32 insertions(+)
> >
> > diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
> > index f4c1869232..6980a78d5f 100644
> > --- a/hw/rtc/mc146818rtc.c
> > +++ b/hw/rtc/mc146818rtc.c
> > @@ -116,6 +116,21 @@ void qmp_rtc_reset_reinjection(Error **errp)
> >       }
> >   }
> >   
> > +void qmp_rtc_notify(Error **errp)
> > +{
> > +    MC146818RtcState *s;
> > +
> > +    /*
> > +     * See:
> > +     * https://www.kernel.org/doc/Documentation/virtual/kvm/timekeeping.txt
> > +     */
> > +    QLIST_FOREACH(s, &rtc_devices, link) {
> > +        s->cmos_data[RTC_REG_B] |= REG_B_UIE;
> > +        s->cmos_data[RTC_REG_C] |= REG_C_IRQF | REG_C_UF;
> > +        qemu_irq_raise(s->irq);
> > +    }
> > +}
> > +
> Note for later: qmp_rtc_notify() works on all realized mc146818rtc
> devices.  Other kinds of RTC devices are silently ignored.  Just like
> qmp_rtc_reset_reinjection().
>
> >   static bool rtc_policy_slew_deliver_irq(MC146818RtcState *s)
> >   {
> >       kvm_reset_irq_delivered();
> > diff --git a/include/hw/rtc/mc146818rtc.h b/include/hw/rtc/mc146818rtc.h
> > index 97cec0b3e8..5229dffbbd 100644
> > --- a/include/hw/rtc/mc146818rtc.h
> > +++ b/include/hw/rtc/mc146818rtc.h
> > @@ -56,5 +56,6 @@ MC146818RtcState *mc146818_rtc_init(ISABus *bus, int 
> > base_year,
> >   void mc146818rtc_set_cmos_data(MC146818RtcState *s, int addr, int val);
> >   int mc146818rtc_get_cmos_data(MC146818RtcState *s, int addr);
> >   void qmp_rtc_reset_reinjection(Error **errp);
> > +void qmp_rtc_notify(Error **errp);
> >   
> >   #endif /* HW_RTC_MC146818RTC_H */
> > diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> > index 4e0a6492a9..20457b0acc 100644
> > --- a/qapi/misc-target.json
> > +++ b/qapi/misc-target.json
> > @@ -19,6 +19,22 @@
>     ##
>     # @rtc-reset-reinjection:
>     #
>     # This command will reset the RTC interrupt reinjection backlog.  Can
>     # be used if another mechanism to synchronize guest time is in effect,
>     # for example QEMU guest agent's guest-set-time command.
>     #
>     # Since: 2.1
>     #
>     # Example:
>     #
>     #     -> { "execute": "rtc-reset-reinjection" }
>     #     <- { "return": {} }
>     ##
> >   { 'command': 'rtc-reset-reinjection',
> >     'if': 'TARGET_I386' }
> >   
> > +##
> > +# @rtc-notify:
> > +#
> > +# Generate an RTC interrupt.
> Our QMP command to generate NMIs is called inject-nmi.  Call this one
> inject-rtc-irq for consistency?  rtc-inject-irq?

This makes sense, I'll rename in the next version. Thanks.

> > +#
> > +# Since: 9.1
> > +#
> > +# Example:
> > +#
> > +#     -> { "execute": "rtc-notify" }
> > +#     <- { "return": {} }
> > +#
> > +##
> > +{ 'command': 'rtc-notify',
> > +  'if': 'TARGET_I386' }
> > +
> As noted above, both commands silently ignore RTCs other than
> mc146818rtc.
>
> They're only available with TARGET_I386.
>
> As long as all machines available with TARGET_I386 can only ever contain
> mc146818rtc RTCs, ignoring other RTCs is a non-problem in practice.
>
> Feels a bit fragile to me.  Thoughts?

Feels a bit fragile indeed, but this code has been there since 2.1, and 
I guess no one really found this to be a problem.

> >   ##
> >   # @SevState:
> >   #