Blue Swirl wrote:
> Move hpet_in_legacy_mode check from mc146818.c to pc.c. Remove
> the optimization where the periodic timer is disabled if
> hpet is in legacy mode.
>
> Signed-off-by: Blue Swirl <blauwir...@gmail.com>
> ---
> hw/mc146818rtc.c | 37 +++++++------------------------------
> hw/mc146818rtc.h | 2 ++
> hw/pc.c | 32 +++++++++++++++++++++++++++-----
> 3 files changed, 36 insertions(+), 35 deletions(-)
>
> diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
> index 571c593..e0c33c5 100644
> --- a/hw/mc146818rtc.c
> +++ b/hw/mc146818rtc.c
> @@ -27,7 +27,6 @@
> #include "pc.h"
> #include "apic.h"
> #include "isa.h"
> -#include "hpet_emul.h"
> #include "mc146818rtc.h"
>
> //#define DEBUG_CMOS
> @@ -94,19 +93,6 @@ typedef struct RTCState {
> QEMUTimer *second_timer2;
> } RTCState;
>
> -static void rtc_irq_raise(qemu_irq irq)
> -{
> - /* When HPET is operating in legacy mode, RTC interrupts are disabled
> - * We block qemu_irq_raise, but not qemu_irq_lower, in case legacy
> - * mode is established while interrupt is raised. We want it to
> - * be lowered in any case
> - */
> -#if defined TARGET_I386
> - if (!hpet_in_legacy_mode())
> -#endif
> - qemu_irq_raise(irq);
> -}
> -
> static void rtc_set_time(RTCState *s);
> static void rtc_copy_date(RTCState *s);
>
> @@ -131,7 +117,7 @@ static void rtc_coalesced_timer(void *opaque)
> if (s->irq_coalesced != 0) {
> apic_reset_irq_delivered();
> s->cmos_data[RTC_REG_C] |= 0xc0;
> - rtc_irq_raise(s->irq);
> + qemu_irq_raise(s->irq);
> if (apic_get_irq_delivered()) {
> s->irq_coalesced--;
> }
> @@ -145,19 +131,10 @@ static void rtc_timer_update(RTCState *s,
> int64_t current_time)
> {
> int period_code, period;
> int64_t cur_clock, next_irq_clock;
> - int enable_pie;
>
> period_code = s->cmos_data[RTC_REG_A] & 0x0f;
> -#if defined TARGET_I386
> - /* disable periodic timer if hpet is in legacy mode, since interrupts are
> - * disabled anyway.
> - */
Does some dumb OS we care about (specifically in KVM mode) first enable
the periodic RTC, then discovers the HPET, switches over, forgetting
about the RTC? Otherwise: the guest will get what it deserves (degraded
performance).
> - enable_pie = !hpet_in_legacy_mode();
> -#else
> - enable_pie = 1;
> -#endif
> if (period_code != 0
> - && (((s->cmos_data[RTC_REG_B] & REG_B_PIE) && enable_pie)
> + && ((s->cmos_data[RTC_REG_B] & REG_B_PIE)
> || ((s->cmos_data[RTC_REG_B] & REG_B_SQWE) && s->sqw_irq))) {
> if (period_code <= 2)
> period_code += 7;
> @@ -194,14 +171,14 @@ static void rtc_periodic_timer(void *opaque)
> if (s->irq_reinject_on_ack_count >= RTC_REINJECT_ON_ACK_COUNT)
> s->irq_reinject_on_ack_count = 0;
> apic_reset_irq_delivered();
> - rtc_irq_raise(s->irq);
> + qemu_irq_raise(s->irq);
> if (!apic_get_irq_delivered()) {
> s->irq_coalesced++;
> rtc_coalesced_timer_update(s);
> }
> } else
> #endif
> - rtc_irq_raise(s->irq);
> + qemu_irq_raise(s->irq);
> }
> if (s->cmos_data[RTC_REG_B] & REG_B_SQWE) {
> /* Not square wave at all but we don't want 2048Hz interrupts!
> @@ -430,7 +407,7 @@ static void rtc_update_second2(void *opaque)
> s->cmos_data[RTC_HOURS_ALARM] == s->current_tm.tm_hour)) {
>
> s->cmos_data[RTC_REG_C] |= 0xa0;
> - rtc_irq_raise(s->irq);
> + qemu_irq_raise(s->irq);
> }
> }
>
> @@ -438,7 +415,7 @@ static void rtc_update_second2(void *opaque)
> s->cmos_data[RTC_REG_C] |= REG_C_UF;
> if (s->cmos_data[RTC_REG_B] & REG_B_UIE) {
> s->cmos_data[RTC_REG_C] |= REG_C_IRQF;
> - rtc_irq_raise(s->irq);
> + qemu_irq_raise(s->irq);
> }
>
> /* clear update in progress bit */
> @@ -588,7 +565,7 @@ static int rtc_initfn(ISADevice *dev)
> {
> RTCState *s = DO_UPCAST(RTCState, dev, dev);
> int base = 0x70;
> - int isairq = 8;
> + int isairq = RTC_ISA_IRQ;
>
> isa_init_irq(dev, &s->irq, isairq);
>
> diff --git a/hw/mc146818rtc.h b/hw/mc146818rtc.h
> index 6f46a68..d630485 100644
> --- a/hw/mc146818rtc.h
> +++ b/hw/mc146818rtc.h
> @@ -7,4 +7,6 @@ ISADevice *rtc_init(int base_year);
> void rtc_set_memory(ISADevice *dev, int addr, int val);
> void rtc_set_date(ISADevice *dev, const struct tm *tm);
>
> +#define RTC_ISA_IRQ 8
> +
> #endif /* !MC146818RTC_H */
> diff --git a/hw/pc.c b/hw/pc.c
> index e7f31d3..5a703e1 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -66,16 +66,38 @@ struct e820_table {
>
> static struct e820_table e820_table;
>
> -void isa_irq_handler(void *opaque, int n, int level)
> +static void isa_set_irq(IsaIrqState *isa, int n, int level)
> {
> - IsaIrqState *isa = (IsaIrqState *)opaque;
> -
> if (n < 16) {
> qemu_set_irq(isa->i8259[n], level);
> }
> - if (isa->ioapic)
> + if (isa->ioapic) {
> qemu_set_irq(isa->ioapic[n], level);
> -};
> + }
> +}
> +
> +static void rtc_irq_handler(IsaIrqState *isa, int level)
> +{
> + /* When HPET is operating in legacy mode, RTC interrupts are disabled.
> + * We block qemu_irq_raise, but not qemu_irq_lower, in case legacy
> + * mode is established while interrupt is raised. We want it to
> + * be lowered in any case.
> + */
> + if (!hpet_in_legacy_mode() || !level) {
> + isa_set_irq(isa, RTC_ISA_IRQ, level);
> + }
> +}
If you clear the RTC IRQ unconditionally, I could imagine that the
enable_pie removal is more than a de-optimization. At least in some
corner cases. But this clearing looks suspicious anyway. Instead, when
switching, the new IRQ line owner should set the level - once.
> +
> +void isa_irq_handler(void *opaque, int n, int level)
> +{
> + IsaIrqState *isa = (IsaIrqState *)opaque;
> +
> + if (n == RTC_ISA_IRQ) {
> + rtc_irq_handler(isa, level);
> + } else {
> + isa_set_irq(isa, n, level);
> + }
> +}
>
> static void ioport80_write(void *opaque, uint32_t addr, uint32_t data)
> {
Jan
signature.asc
Description: OpenPGP digital signature
