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

[Philippe Mathieu-Daudé]

On 29/4/24 11:41, Daniil Tatianin wrote:
> 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.
>
> Also add a comment to highlight the fact that this (and one other QMP
> command) only works for the MC146818 RTC controller.
>
> Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
> ---
>
> Changes since v0:
> - Rename to rtc-inject-irq to match other similar API
> - Add a comment to highlight that this only works for the I386 RTC
>
> Changes since v1:
> - Added a description below the QMP command to explain how it can be
>    used and what it does.
>
> ---
>   hw/rtc/mc146818rtc.c         | 20 ++++++++++++++++++++
>   include/hw/rtc/mc146818rtc.h |  1 +
>   qapi/misc-target.json        | 18 ++++++++++++++++++
>   3 files changed, 39 insertions(+)
>
> diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
> index f4c1869232..8501b55cbd 100644
> --- a/hw/rtc/mc146818rtc.c
> +++ b/hw/rtc/mc146818rtc.c
> @@ -107,6 +107,11 @@ static void rtc_coalesced_timer_update(MC146818RtcState *s)
>   static QLIST_HEAD(, MC146818RtcState) rtc_devices =
>       QLIST_HEAD_INITIALIZER(rtc_devices);
>   
> +/*
> + * NOTE:
> + * The two QMP functions below are _only_ implemented for the MC146818.
> + * All other RTC devices ignore this.
> + */
>   void qmp_rtc_reset_reinjection(Error **errp)
>   {
>       MC146818RtcState *s;
> @@ -116,6 +121,21 @@ void qmp_rtc_reset_reinjection(Error **errp)
>       }
>   }
>   
> +void qmp_rtc_inject_irq(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);
> +    }
> +}
> +
>   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..6cd9761d80 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_inject_irq(Error **errp);
>   
>   #endif /* HW_RTC_MC146818RTC_H */
> diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> index 4e0a6492a9..0f2479f8f4 100644
> --- a/qapi/misc-target.json
> +++ b/qapi/misc-target.json
> @@ -19,6 +19,24 @@
>   { 'command': 'rtc-reset-reinjection',
>     'if': 'TARGET_I386' }
>   
> +##
> +# @rtc-inject-irq:
> +#
> +# Inject an RTC interrupt. This command forces the guest to synchornize
> +# the time with RTC. This is useful after a long stop-cont pause, which
> +# is common for serverless-type workload.
> +#
> +# Since: 9.1
> +#
> +# Example:
> +#
> +#     -> { "execute": "rtc-inject-irq" }
> +#     <- { "return": {} }
> +#
> +##
> +{ 'command': 'rtc-inject-irq',
> +  'if': 'TARGET_I386' }

Why is that restricted to x86? Ah, this is specific to the MC146818
RTC... Other machines use hw accelerators and the MC146818, aren't
we interested in synchronizing them the same way?

Personally I'd name this command 'mc146818rtc-raise-irq-broadcast',
KISS.