From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=44382 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OGDIC-0006YN-4J for qemu-devel@nongnu.org; Sun, 23 May 2010 11:40:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OGDI9-0008UO-EF for qemu-devel@nongnu.org; Sun, 23 May 2010 11:40:43 -0400 Received: from fmmailgate02.web.de ([217.72.192.227]:56649) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OGDI8-0008UB-Pr for qemu-devel@nongnu.org; Sun, 23 May 2010 11:40:41 -0400 Message-ID: <4BF94C77.8060801@web.de> Date: Sun, 23 May 2010 17:40:39 +0200 From: Jan Kiszka MIME-Version: 1.0 References: In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig0729610EB65103EFF1409940" Sender: jan.kiszka@web.de Subject: [Qemu-devel] Re: [PATCH, RFC 2/4] hpet: don't use any static state 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) --------------enig0729610EB65103EFF1409940 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Blue Swirl wrote: > Signed-off-by: Blue Swirl > --- > hw/hpet.c | 68 +++++++++++++++++++++++++++++-------------------= ------- > hw/hpet_emul.h | 4 +- > hw/pc.c | 8 ++++-- > hw/pc.h | 3 +- > hw/pc_piix.c | 3 +- > 5 files changed, 47 insertions(+), 39 deletions(-) >=20 > diff --git a/hw/hpet.c b/hw/hpet.c > index 8729fb2..f24e054 100644 > --- a/hw/hpet.c > +++ b/hw/hpet.c > @@ -37,14 +37,11 @@ > #define DPRINTF(...) > #endif >=20 > -static HPETState *hpet_statep; > - > -uint32_t hpet_in_legacy_mode(void) > +uint32_t hpet_in_legacy_mode(void *opaque) uint32_t hpet_in_legacy_mode(HPETState *s) please (will become DeviceState with my patches, but it should not be void in any case). > { > - if (hpet_statep) > - return hpet_statep->config & HPET_CFG_LEGACY; > - else > - return 0; > + HPETState *s =3D opaque; > + > + return s->config & HPET_CFG_LEGACY; > } >=20 > static uint32_t timer_int_route(struct HPETTimer *timer) > @@ -54,9 +51,9 @@ static uint32_t timer_int_route(struct HPETTimer *tim= er) > return route; > } >=20 > -static uint32_t hpet_enabled(void) > +static uint32_t hpet_enabled(HPETState *s) > { > - return hpet_statep->config & HPET_CFG_ENABLE; > + return s->config & HPET_CFG_ENABLE; > } >=20 > static uint32_t timer_is_periodic(HPETTimer *t) > @@ -106,10 +103,10 @@ static int deactivating_bit(uint64_t old, > uint64_t new, uint64_t mask) > return ((old & mask) && !(new & mask)); > } >=20 > -static uint64_t hpet_get_ticks(void) > +static uint64_t hpet_get_ticks(HPETState *s) > { > uint64_t ticks; > - ticks =3D ns_to_ticks(qemu_get_clock(vm_clock) + hpet_statep->hpet= _offset); > + ticks =3D ns_to_ticks(qemu_get_clock(vm_clock) + s->hpet_offset); > return ticks; > } >=20 > @@ -139,7 +136,7 @@ static void update_irq(struct HPETTimer *timer) > qemu_irq irq; > int route; >=20 > - if (timer->tn <=3D 1 && hpet_in_legacy_mode()) { > + if (timer->tn <=3D 1 && hpet_in_legacy_mode(timer->state)) { > /* if LegacyReplacementRoute bit is set, HPET specification re= quires > * timer0 be routed to IRQ0 in NON-APIC or IRQ2 in the I/O API= C, > * timer1 be routed to IRQ8 in NON-APIC or IRQ8 in the I/O API= C. > @@ -152,7 +149,7 @@ static void update_irq(struct HPETTimer *timer) > route=3Dtimer_int_route(timer); > irq=3Dtimer->state->irqs[route]; > } > - if (timer_enabled(timer) && hpet_enabled()) { > + if (timer_enabled(timer) && hpet_enabled(timer->state)) { > qemu_irq_pulse(irq); > } > } > @@ -161,7 +158,7 @@ static void hpet_pre_save(void *opaque) > { > HPETState *s =3D opaque; > /* save current counter value */ > - s->hpet_counter =3D hpet_get_ticks(); > + s->hpet_counter =3D hpet_get_ticks(s); > } >=20 > static int hpet_post_load(void *opaque, int version_id) > @@ -216,7 +213,7 @@ static void hpet_timer(void *opaque) > uint64_t diff; >=20 > uint64_t period =3D t->period; > - uint64_t cur_tick =3D hpet_get_ticks(); > + uint64_t cur_tick =3D hpet_get_ticks(t->state); >=20 > if (timer_is_periodic(t) && period !=3D 0) { > if (t->config & HPET_TN_32BIT) { > @@ -244,7 +241,7 @@ static void hpet_set_timer(HPETTimer *t) > { > uint64_t diff; > uint32_t wrap_diff; /* how many ticks until we wrap? */ > - uint64_t cur_tick =3D hpet_get_ticks(); > + uint64_t cur_tick =3D hpet_get_ticks(t->state); >=20 > /* whenever new timer is being set up, make sure wrap_flag is 0 */= > t->wrap_flag =3D 0; > @@ -326,17 +323,19 @@ static uint32_t hpet_ram_readl(void *opaque, > target_phys_addr_t addr) > DPRINTF("qemu: invalid HPET_CFG + 4 hpet_ram_readl \n"= ); > return 0; > case HPET_COUNTER: > - if (hpet_enabled()) > - cur_tick =3D hpet_get_ticks(); > - else > + if (hpet_enabled(s)) { > + cur_tick =3D hpet_get_ticks(s); > + } else { > cur_tick =3D s->hpet_counter; > + } > DPRINTF("qemu: reading counter =3D %" PRIx64 "\n", cu= r_tick); > return cur_tick; > case HPET_COUNTER + 4: > - if (hpet_enabled()) > - cur_tick =3D hpet_get_ticks(); > - else > + if (hpet_enabled(s)) { > + cur_tick =3D hpet_get_ticks(s); > + } else { > cur_tick =3D s->hpet_counter; > + } > DPRINTF("qemu: reading counter + 4 =3D %" PRIx64 "\n"= , > cur_tick); > return cur_tick >> 32; > case HPET_STATUS: > @@ -419,8 +418,9 @@ static void hpet_ram_writel(void *opaque, > target_phys_addr_t addr, > | new_val; > } > timer->config &=3D ~HPET_TN_SETVAL; > - if (hpet_enabled()) > + if (hpet_enabled(s)) { > hpet_set_timer(timer); > + } > break; > case HPET_TN_CMP + 4: // comparator register high order > DPRINTF("qemu: hpet_ram_writel HPET_TN_CMP + 4\n"); > @@ -439,8 +439,9 @@ static void hpet_ram_writel(void *opaque, > target_phys_addr_t addr, > | new_val << 32; > } > timer->config &=3D ~HPET_TN_SETVAL; > - if (hpet_enabled()) > + if (hpet_enabled(s)) { > hpet_set_timer(timer); > + } > break; > case HPET_TN_ROUTE + 4: > DPRINTF("qemu: hpet_ram_writel HPET_TN_ROUTE + 4\n"); > @@ -467,7 +468,7 @@ static void hpet_ram_writel(void *opaque, > target_phys_addr_t addr, > } > else if (deactivating_bit(old_val, new_val, HPET_CFG_E= NABLE)) { > /* Halt main counter and disable interrupt generat= ion. */ > - s->hpet_counter =3D hpet_get_ticks(); > + s->hpet_counter =3D hpet_get_ticks(s); > for (i =3D 0; i < HPET_NUM_TIMERS; i++) > hpet_del_timer(&s->timer[i]); > } > @@ -485,16 +486,18 @@ static void hpet_ram_writel(void *opaque, > target_phys_addr_t addr, > /* FIXME: need to handle level-triggered interrupts */= > break; > case HPET_COUNTER: > - if (hpet_enabled()) > - printf("qemu: Writing counter while HPET enabled!\n= "); > + if (hpet_enabled(s)) { > + printf("qemu: Writing counter while HPET enabled!\= n"); Missed it in my series as well: Should become DPRINTF at this chance. Also below. > + } > s->hpet_counter =3D (s->hpet_counter & 0xffffffff000000= 00ULL) > | value; > DPRINTF("qemu: HPET counter written. ctr =3D %#x -> %" > PRIx64 "\n", > value, s->hpet_counter); > break; > case HPET_COUNTER + 4: > - if (hpet_enabled()) > - printf("qemu: Writing counter while HPET enabled!\n= "); > + if (hpet_enabled(s)) { > + printf("qemu: Writing counter while HPET enabled!\= n"); > + } > s->hpet_counter =3D (s->hpet_counter & 0xffffffffULL) > | (((uint64_t)value) << 32); > DPRINTF("qemu: HPET counter + 4 written. ctr =3D %#x ->= > %" PRIx64 "\n", > @@ -563,15 +566,14 @@ static void hpet_reset(void *opaque) { > count =3D 1; > } >=20 > - > -void hpet_init(qemu_irq *irq) { > +HPETState *hpet_init(qemu_irq *irq) > +{ > int i, iomemtype; > HPETState *s; >=20 > DPRINTF ("hpet_init\n"); >=20 > s =3D qemu_mallocz(sizeof(HPETState)); > - hpet_statep =3D s; > s->irqs =3D irq; > for (i=3D0; i HPETTimer *timer =3D &s->timer[i]; > @@ -583,4 +585,6 @@ void hpet_init(qemu_irq *irq) { > iomemtype =3D cpu_register_io_memory(hpet_ram_read, > hpet_ram_write, s); > cpu_register_physical_memory(HPET_BASE, 0x400, iomemtype); > + > + return s; > } > diff --git a/hw/hpet_emul.h b/hw/hpet_emul.h > index cfd95b4..88dbf0d 100644 > --- a/hw/hpet_emul.h > +++ b/hw/hpet_emul.h > @@ -75,8 +75,8 @@ typedef struct HPETState { > } HPETState; >=20 > #if defined TARGET_I386 > -extern uint32_t hpet_in_legacy_mode(void); > -extern void hpet_init(qemu_irq *irq); > +uint32_t hpet_in_legacy_mode(void *opaque); > +HPETState *hpet_init(qemu_irq *irq); > #endif >=20 > #endif > diff --git a/hw/pc.c b/hw/pc.c > index 5a703e1..9f1a9d6 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -83,7 +83,7 @@ static void rtc_irq_handler(IsaIrqState *isa, int lev= el) > * mode is established while interrupt is raised. We want it to > * be lowered in any case. > */ > - if (!hpet_in_legacy_mode() || !level) { > + if ((isa->hpet_state && !hpet_in_legacy_mode(isa->hpet_state)) || = !level) { > isa_set_irq(isa, RTC_ISA_IRQ, level); > } > } > @@ -945,7 +945,7 @@ static void cpu_request_exit(void *opaque, int > irq, int level) > } > } >=20 > -void pc_basic_device_init(qemu_irq *isa_irq, > +void pc_basic_device_init(qemu_irq *isa_irq, IsaIrqState *isa, > FDCtrl **floppy_controller, > ISADevice **rtc_state) > { > @@ -966,8 +966,10 @@ void pc_basic_device_init(qemu_irq *isa_irq, >=20 > pit =3D pit_init(0x40, isa_reserve_irq(0)); > pcspk_init(pit); > + > + isa->hpet_state =3D NULL; > if (!no_hpet) { > - hpet_init(isa_irq); > + isa->hpet_state =3D hpet_init(isa_irq); > } >=20 > for(i =3D 0; i < MAX_SERIAL_PORTS; i++) { > diff --git a/hw/pc.h b/hw/pc.h > index 73cccef..3e085b9 100644 > --- a/hw/pc.h > +++ b/hw/pc.h > @@ -42,6 +42,7 @@ void irq_info(Monitor *mon); > typedef struct isa_irq_state { > qemu_irq *i8259; > qemu_irq *ioapic; > + void *hpet_state; > } IsaIrqState; >=20 > void isa_irq_handler(void *opaque, int n, int level); > @@ -94,7 +95,7 @@ void pc_memory_init(ram_addr_t ram_size, > ram_addr_t *above_4g_mem_size_p); > qemu_irq *pc_allocate_cpu_irq(void); > void pc_vga_init(PCIBus *pci_bus); > -void pc_basic_device_init(qemu_irq *isa_irq, > +void pc_basic_device_init(qemu_irq *isa_irq, IsaIrqState *isa, > FDCtrl **floppy_controller, > ISADevice **rtc_state); > void pc_init_ne2k_isa(NICInfo *nd); > diff --git a/hw/pc_piix.c b/hw/pc_piix.c > index 70f563a..1479a28 100644 > --- a/hw/pc_piix.c > +++ b/hw/pc_piix.c > @@ -93,7 +93,8 @@ static void pc_init1(ram_addr_t ram_size, > pc_vga_init(pci_enabled? pci_bus: NULL); >=20 > /* init basic PC hardware */ > - pc_basic_device_init(isa_irq, &floppy_controller, &rtc_state); > + pc_basic_device_init(isa_irq, isa_irq_state, &floppy_controller, > + &rtc_state); >=20 > for(i =3D 0; i < nb_nics; i++) { > NICInfo *nd =3D &nd_table[i]; I have a longer hpet fix/qdev'ify/enhance series queued [1], held back until the device_show thing is through. Your patch break it, but it makes sense. Will rebase on top. Jan [1] git://git.kiszka.org/qemu.git queues/hpet --------------enig0729610EB65103EFF1409940 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 iEYEARECAAYFAkv5THcACgkQitSsb3rl5xSyHwCg0O8+m3FhufSaETwPxdHChYoD R3UAoNpiIGAhuBAnK4A0F5gmxEdHFshJ =6PTW -----END PGP SIGNATURE----- --------------enig0729610EB65103EFF1409940--