From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=44337 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OGDI7-0006Sj-A3 for qemu-devel@nongnu.org; Sun, 23 May 2010 11:40:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OGDI5-0008TG-5H for qemu-devel@nongnu.org; Sun, 23 May 2010 11:40:39 -0400 Received: from fmmailgate02.web.de ([217.72.192.227]:56621) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OGDI4-0008T5-IR for qemu-devel@nongnu.org; Sun, 23 May 2010 11:40:37 -0400 Message-ID: <4BF94C6E.8080800@web.de> Date: Sun, 23 May 2010 17:40:30 +0200 From: Jan Kiszka MIME-Version: 1.0 References: In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enigB4C6D637C48B61D815CBA090" Sender: jan.kiszka@web.de Subject: [Qemu-devel] Re: [PATCH, RFC 1/4] mc146818: move hpet handling to pc.c List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: qemu-devel This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enigB4C6D637C48B61D815CBA090 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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. >=20 > Signed-off-by: Blue Swirl > --- > hw/mc146818rtc.c | 37 +++++++------------------------------ > hw/mc146818rtc.h | 2 ++ > hw/pc.c | 32 +++++++++++++++++++++++++++----- > 3 files changed, 36 insertions(+), 35 deletions(-) >=20 > 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" >=20 > //#define DEBUG_CMOS > @@ -94,19 +93,6 @@ typedef struct RTCState { > QEMUTimer *second_timer2; > } RTCState; >=20 > -static void rtc_irq_raise(qemu_irq irq) > -{ > - /* When HPET is operating in legacy mode, RTC interrupts are disab= led > - * 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); >=20 > @@ -131,7 +117,7 @@ static void rtc_coalesced_timer(void *opaque) > if (s->irq_coalesced !=3D 0) { > apic_reset_irq_delivered(); > s->cmos_data[RTC_REG_C] |=3D 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; >=20 > period_code =3D s->cmos_data[RTC_REG_A] & 0x0f; > -#if defined TARGET_I386 > - /* disable periodic timer if hpet is in legacy mode, since interru= pts 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 =3D !hpet_in_legacy_mode(); > -#else > - enable_pie =3D 1; > -#endif > if (period_code !=3D 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 <=3D 2) > period_code +=3D 7; > @@ -194,14 +171,14 @@ static void rtc_periodic_timer(void *opaque) > if (s->irq_reinject_on_ack_count >=3D RTC_REINJECT_ON_ACK_= COUNT) > s->irq_reinject_on_ack_count =3D 0; =09 > 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] =3D=3D s->current_tm.tm_hou= r)) { >=20 > s->cmos_data[RTC_REG_C] |=3D 0xa0; > - rtc_irq_raise(s->irq); > + qemu_irq_raise(s->irq); > } > } >=20 > @@ -438,7 +415,7 @@ static void rtc_update_second2(void *opaque) > s->cmos_data[RTC_REG_C] |=3D REG_C_UF; > if (s->cmos_data[RTC_REG_B] & REG_B_UIE) { > s->cmos_data[RTC_REG_C] |=3D REG_C_IRQF; > - rtc_irq_raise(s->irq); > + qemu_irq_raise(s->irq); > } >=20 > /* clear update in progress bit */ > @@ -588,7 +565,7 @@ static int rtc_initfn(ISADevice *dev) > { > RTCState *s =3D DO_UPCAST(RTCState, dev, dev); > int base =3D 0x70; > - int isairq =3D 8; > + int isairq =3D RTC_ISA_IRQ; >=20 > isa_init_irq(dev, &s->irq, isairq); >=20 > 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); >=20 > +#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 { >=20 > static struct e820_table e820_table; >=20 > -void isa_irq_handler(void *opaque, int n, int level) > +static void isa_set_irq(IsaIrqState *isa, int n, int level) > { > - IsaIrqState *isa =3D (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 disab= led. > + * 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 =3D (IsaIrqState *)opaque; > + > + if (n =3D=3D RTC_ISA_IRQ) { > + rtc_irq_handler(isa, level); > + } else { > + isa_set_irq(isa, n, level); > + } > +} >=20 > static void ioport80_write(void *opaque, uint32_t addr, uint32_t data)= > { Jan --------------enigB4C6D637C48B61D815CBA090 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.9 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iEYEARECAAYFAkv5THIACgkQitSsb3rl5xStUQCfcMReWOosp0kWhU6G7cUoqHS1 GNAAn02PyGSM4smmphDiGUlEfzPjCy83 =poUQ -----END PGP SIGNATURE----- --------------enigB4C6D637C48B61D815CBA090--