| Message ID | AANLkTiklEOofhSQ0zt9ep2yV3zMeOdxseX9QkWyV3uXV@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
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 <blauwirbel@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
On Sun, May 23, 2010 at 3:40 PM, Jan Kiszka <jan.kiszka@web.de> wrote: > 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 <blauwirbel@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). No idea. The performance penalty also depends on the trigger frequency. >> - 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. This was how the original version worked, the check with hpet_in_legacy_mode() was only used for qemu_irq_raise(), not qemu_irq_lower(). The new handler will be called for both cases, so the check must consider also the level. >> + >> +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 > >
Blue Swirl wrote: > On Sun, May 23, 2010 at 3:40 PM, Jan Kiszka <jan.kiszka@web.de> wrote: >> 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 <blauwirbel@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). > > No idea. The performance penalty also depends on the trigger frequency. I think now it is OK to leave it ticking. We are currently lacking proper RTC routing through ACPI to SCI [1, Figure 11]. Adding this will add a parallel user of the RTC IRQ line. I briefly thought about some user registration API for the RTC, but that appears over-engineered on second thought. Let's go the simple path. > >>> - 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. > > This was how the original version worked, the check with > hpet_in_legacy_mode() was only used for qemu_irq_raise(), not > qemu_irq_lower(). The new handler will be called for both cases, so > the check must consider also the level. I think both new and old code is wrong. I'm reworking this ATM. Actually, the hpet handling belongs to, well, the hpet. That avoids having to deal with all those details via complex APIs. But the coalescing mess is still causing headaches to me, at least when trying to come up with something long-term ready. Jan [1] http://www.intel.com/hardwaredesign/hpetspec_1.pdf
On Mon, May 24, 2010 at 3:30 PM, Jan Kiszka <jan.kiszka@web.de> wrote: > Blue Swirl wrote: >> On Sun, May 23, 2010 at 3:40 PM, Jan Kiszka <jan.kiszka@web.de> wrote: >>> 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 <blauwirbel@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). >> >> No idea. The performance penalty also depends on the trigger frequency. > > I think now it is OK to leave it ticking. > > We are currently lacking proper RTC routing through ACPI to SCI [1, > Figure 11]. Adding this will add a parallel user of the RTC IRQ line. I What a poor picture BTW, even the arrow heads are missing. Would you have a pointer for the SCI specs? > briefly thought about some user registration API for the RTC, but that > appears over-engineered on second thought. Let's go the simple path. I think it's easier to add some logic to HPET to route the RTC/HPET/i8254 irqs. If there is no HPET, the irqs are routed directly. >> >>>> - 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. >> >> This was how the original version worked, the check with >> hpet_in_legacy_mode() was only used for qemu_irq_raise(), not >> qemu_irq_lower(). The new handler will be called for both cases, so >> the check must consider also the level. > > I think both new and old code is wrong. I'm reworking this ATM. > Actually, the hpet handling belongs to, well, the hpet. That avoids I see, that would make things simpler. > having to deal with all those details via complex APIs. But the > coalescing mess is still causing headaches to me, at least when trying > to come up with something long-term ready. Maybe the coalescing should be pushed to APIC, or even generalized.
Blue Swirl wrote: > On Mon, May 24, 2010 at 3:30 PM, Jan Kiszka <jan.kiszka@web.de> wrote: >> Blue Swirl wrote: >>> On Sun, May 23, 2010 at 3:40 PM, Jan Kiszka <jan.kiszka@web.de> wrote: >>>> 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 <blauwirbel@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). >>> No idea. The performance penalty also depends on the trigger frequency. >> I think now it is OK to leave it ticking. >> >> We are currently lacking proper RTC routing through ACPI to SCI [1, >> Figure 11]. Adding this will add a parallel user of the RTC IRQ line. I > > What a poor picture BTW, even the arrow heads are missing. Would you > have a pointer for the SCI specs? Nope. But my current understanding is that those gates map to ACPI_BITMASK_RT_CLOCK_STATUS and ACPI_BITMASK_RT_CLOCK_ENABLE in our acpi code, and SCI handling is already there. Just takes inject the RTC IRQs when all bits are set. > >> briefly thought about some user registration API for the RTC, but that >> appears over-engineered on second thought. Let's go the simple path. > > I think it's easier to add some logic to HPET to route the > RTC/HPET/i8254 irqs. If there is no HPET, the irqs are routed > directly. That's what I did, at least for the RTC. Haven't looked at the i8254-vs.-HPET bits yet as I wanted to send out the series. :) > >>>>> - 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. >>> This was how the original version worked, the check with >>> hpet_in_legacy_mode() was only used for qemu_irq_raise(), not >>> qemu_irq_lower(). The new handler will be called for both cases, so >>> the check must consider also the level. >> I think both new and old code is wrong. I'm reworking this ATM. >> Actually, the hpet handling belongs to, well, the hpet. That avoids > > I see, that would make things simpler. > >> having to deal with all those details via complex APIs. But the >> coalescing mess is still causing headaches to me, at least when trying >> to come up with something long-term ready. > > Maybe the coalescing should be pushed to APIC, or even generalized. The latter has happened, hopefully in the right direction. Jan
On Mon, May 24, 2010 at 10:20:31PM +0200, Jan Kiszka wrote: > > Maybe the coalescing should be pushed to APIC, or even generalized. > > The latter has happened, hopefully in the right direction. > What do you mean by that? -- Gleb.
On Mon, May 24, 2010 at 07:58:28PM +0000, Blue Swirl wrote: > > having to deal with all those details via complex APIs. But the > > coalescing mess is still causing headaches to me, at least when trying > > to come up with something long-term ready. > > Maybe the coalescing should be pushed to APIC, or even generalized. At APIC level you miss some information. What if RTC frequency was changed? What if it was reset? -- Gleb.
Gleb Natapov wrote: > On Mon, May 24, 2010 at 10:20:31PM +0200, Jan Kiszka wrote: >>> Maybe the coalescing should be pushed to APIC, or even generalized. >> The latter has happened, hopefully in the right direction. >> > What do you mean by that? The latter: I tried to generalize. Please look at patch 7 & 8 of my HPET series. Jan
On Tue, May 25, 2010 at 08:09:02AM +0200, Jan Kiszka wrote: > Gleb Natapov wrote: > > On Mon, May 24, 2010 at 10:20:31PM +0200, Jan Kiszka wrote: > >>> Maybe the coalescing should be pushed to APIC, or even generalized. > >> The latter has happened, hopefully in the right direction. > >> > > What do you mean by that? > > The latter: I tried to generalize. Please look at patch 7 & 8 of my HPET > series. > Already did. I hope you will have more luck with this (obviously correct) approach than I had two years ago. -- Gleb.
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. - */ - 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); + } +} + +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) {
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 <blauwirbel@gmail.com> --- hw/mc146818rtc.c | 37 +++++++------------------------------ hw/mc146818rtc.h | 2 ++ hw/pc.c | 32 +++++++++++++++++++++++++++----- 3 files changed, 36 insertions(+), 35 deletions(-)