On 20.04.2021 16:07, Roger Pau Monne wrote:
> --- a/xen/arch/x86/hvm/rtc.c
> +++ b/xen/arch/x86/hvm/rtc.c
> @@ -46,15 +46,6 @@
> #define epoch_year 1900
> #define get_year(x) (x + epoch_year)
>
> -enum rtc_mode {
> - rtc_mode_no_ack,
> - rtc_mode_strict
> -};
> -
> -/* This must be in sync with how hvmloader sets the ACPI WAET flags. */
> -#define mode_is(d, m) ((void)(d), rtc_mode_##m == rtc_mode_no_ack)
> -#define rtc_mode_is(s, m) mode_is(vrtc_domain(s), m)
Leaving aside my concerns about this removal, I think some form of
reference to hvmloader and its respective behavior should remain
here, presumably in form of a (replacement) comment.
> @@ -337,8 +336,7 @@ int pt_update_irq(struct vcpu *v)
> {
> if ( pt->pending_intr_nr )
> {
> - /* RTC code takes care of disabling the timer itself. */
> - if ( (pt->irq != RTC_IRQ || !pt->priv) && pt_irq_masked(pt) &&
> + if ( pt_irq_masked(pt) &&
> /* Level interrupts should be asserted even if masked. */
> !pt->level )
> {
I'm struggling to relate this to any other part of the patch. In
particular I can't find the case where a periodic timer would be
registered with RTC_IRQ and a NULL private pointer. The only use
I can find is with a non-NULL pointer, which would mean the "else"
path is always taken at present for the RTC case (which you now
change).
Jan
