From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=57564 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1OGDvJ-0006BL-OB
	for qemu-devel@nongnu.org; Sun, 23 May 2010 12:21:11 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69)
	(envelope-from <blauwirbel@gmail.com>) id 1OGDvH-00064A-34
	for qemu-devel@nongnu.org; Sun, 23 May 2010 12:21:09 -0400
Received: from mail-pv0-f173.google.com ([74.125.83.173]:51814)
	by eggs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <blauwirbel@gmail.com>) id 1OGDvG-00063x-La
	for qemu-devel@nongnu.org; Sun, 23 May 2010 12:21:07 -0400
Received: by pvg6 with SMTP id 6so1170741pvg.4
	for <qemu-devel@nongnu.org>; Sun, 23 May 2010 09:21:05 -0700 (PDT)
MIME-Version: 1.0
In-Reply-To: <4BF94C77.8060801@web.de>
References: <AANLkTikJsUKGDA_TNeg1AUGvojkGbkg8UIJDe9Ho1qoP@mail.gmail.com>
	<4BF94C77.8060801@web.de>
From: Blue Swirl <blauwirbel@gmail.com>
Date: Sun, 23 May 2010 16:20:45 +0000
Message-ID: <AANLkTikXDnjoBYDQIskeNGUKuFcoLjJu3raJR7qcE6Np@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: [Qemu-devel] Re: [PATCH, RFC 2/4] hpet: don't use any static state
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Jan Kiszka <jan.kiszka@web.de>
Cc: qemu-devel <qemu-devel@nongnu.org>

On Sun, May 23, 2010 at 3:40 PM, Jan Kiszka <jan.kiszka@web.de> wrote:
> Blue Swirl wrote:
>> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
>> ---
>> =C2=A0hw/hpet.c =C2=A0 =C2=A0 =C2=A0| =C2=A0 68 ++++++++++++++++++++++++=
+++++--------------------------
>> =C2=A0hw/hpet_emul.h | =C2=A0 =C2=A04 +-
>> =C2=A0hw/pc.c =C2=A0 =C2=A0 =C2=A0 =C2=A0| =C2=A0 =C2=A08 ++++--
>> =C2=A0hw/pc.h =C2=A0 =C2=A0 =C2=A0 =C2=A0| =C2=A0 =C2=A03 +-
>> =C2=A0hw/pc_piix.c =C2=A0 | =C2=A0 =C2=A03 +-
>> =C2=A05 files changed, 47 insertions(+), 39 deletions(-)
>>
>> 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 @@
>> =C2=A0#define DPRINTF(...)
>> =C2=A0#endif
>>
>> -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).

I tried that, but HPTState is not available for all cases where pc.h
is #included. DeviceState or ISADeviceState would be much better, the
callers have no need to access HPETState fields.

>> =C2=A0{
>> - =C2=A0 =C2=A0if (hpet_statep)
>> - =C2=A0 =C2=A0 =C2=A0 =C2=A0return hpet_statep->config & HPET_CFG_LEGAC=
Y;
>> - =C2=A0 =C2=A0else
>> - =C2=A0 =C2=A0 =C2=A0 =C2=A0return 0;
>> + =C2=A0 =C2=A0HPETState *s =3D opaque;
>> +
>> + =C2=A0 =C2=A0return s->config & HPET_CFG_LEGACY;
>> =C2=A0}
>>
>> =C2=A0static uint32_t timer_int_route(struct HPETTimer *timer)
>> @@ -54,9 +51,9 @@ static uint32_t timer_int_route(struct HPETTimer *time=
r)
>> =C2=A0 =C2=A0 =C2=A0return route;
>> =C2=A0}
>>
>> -static uint32_t hpet_enabled(void)
>> +static uint32_t hpet_enabled(HPETState *s)
>> =C2=A0{
>> - =C2=A0 =C2=A0return hpet_statep->config & HPET_CFG_ENABLE;
>> + =C2=A0 =C2=A0return s->config & HPET_CFG_ENABLE;
>> =C2=A0}
>>
>> =C2=A0static uint32_t timer_is_periodic(HPETTimer *t)
>> @@ -106,10 +103,10 @@ static int deactivating_bit(uint64_t old,
>> uint64_t new, uint64_t mask)
>> =C2=A0 =C2=A0 =C2=A0return ((old & mask) && !(new & mask));
>> =C2=A0}
>>
>> -static uint64_t hpet_get_ticks(void)
>> +static uint64_t hpet_get_ticks(HPETState *s)
>> =C2=A0{
>> =C2=A0 =C2=A0 =C2=A0uint64_t ticks;
>> - =C2=A0 =C2=A0ticks =3D ns_to_ticks(qemu_get_clock(vm_clock) + hpet_sta=
tep->hpet_offset);
>> + =C2=A0 =C2=A0ticks =3D ns_to_ticks(qemu_get_clock(vm_clock) + s->hpet_=
offset);
>> =C2=A0 =C2=A0 =C2=A0return ticks;
>> =C2=A0}
>>
>> @@ -139,7 +136,7 @@ static void update_irq(struct HPETTimer *timer)
>> =C2=A0 =C2=A0 =C2=A0qemu_irq irq;
>> =C2=A0 =C2=A0 =C2=A0int route;
>>
>> - =C2=A0 =C2=A0if (timer->tn <=3D 1 && hpet_in_legacy_mode()) {
>> + =C2=A0 =C2=A0if (timer->tn <=3D 1 && hpet_in_legacy_mode(timer->state)=
) {
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* if LegacyReplacementRoute bit is se=
t, HPET specification requires
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 * timer0 be routed to IRQ0 in NON-API=
C or IRQ2 in the I/O APIC,
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 * timer1 be routed to IRQ8 in NON-API=
C or IRQ8 in the I/O APIC.
>> @@ -152,7 +149,7 @@ static void update_irq(struct HPETTimer *timer)
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0route=3Dtimer_int_route(timer);
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0irq=3Dtimer->state->irqs[route];
>> =C2=A0 =C2=A0 =C2=A0}
>> - =C2=A0 =C2=A0if (timer_enabled(timer) && hpet_enabled()) {
>> + =C2=A0 =C2=A0if (timer_enabled(timer) && hpet_enabled(timer->state)) {
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0qemu_irq_pulse(irq);
>> =C2=A0 =C2=A0 =C2=A0}
>> =C2=A0}
>> @@ -161,7 +158,7 @@ static void hpet_pre_save(void *opaque)
>> =C2=A0{
>> =C2=A0 =C2=A0 =C2=A0HPETState *s =3D opaque;
>> =C2=A0 =C2=A0 =C2=A0/* save current counter value */
>> - =C2=A0 =C2=A0s->hpet_counter =3D hpet_get_ticks();
>> + =C2=A0 =C2=A0s->hpet_counter =3D hpet_get_ticks(s);
>> =C2=A0}
>>
>> =C2=A0static int hpet_post_load(void *opaque, int version_id)
>> @@ -216,7 +213,7 @@ static void hpet_timer(void *opaque)
>> =C2=A0 =C2=A0 =C2=A0uint64_t diff;
>>
>> =C2=A0 =C2=A0 =C2=A0uint64_t period =3D t->period;
>> - =C2=A0 =C2=A0uint64_t cur_tick =3D hpet_get_ticks();
>> + =C2=A0 =C2=A0uint64_t cur_tick =3D hpet_get_ticks(t->state);
>>
>> =C2=A0 =C2=A0 =C2=A0if (timer_is_periodic(t) && period !=3D 0) {
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (t->config & HPET_TN_32BIT) {
>> @@ -244,7 +241,7 @@ static void hpet_set_timer(HPETTimer *t)
>> =C2=A0{
>> =C2=A0 =C2=A0 =C2=A0uint64_t diff;
>> =C2=A0 =C2=A0 =C2=A0uint32_t wrap_diff; =C2=A0/* how many ticks until we=
 wrap? */
>> - =C2=A0 =C2=A0uint64_t cur_tick =3D hpet_get_ticks();
>> + =C2=A0 =C2=A0uint64_t cur_tick =3D hpet_get_ticks(t->state);
>>
>> =C2=A0 =C2=A0 =C2=A0/* whenever new timer is being set up, make sure wra=
p_flag is 0 */
>> =C2=A0 =C2=A0 =C2=A0t->wrap_flag =3D 0;
>> @@ -326,17 +323,19 @@ static uint32_t hpet_ram_readl(void *opaque,
>> target_phys_addr_t addr)
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0DPRINTF("q=
emu: invalid HPET_CFG + 4 hpet_ram_readl \n");
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return 0;
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0case HPET_COUNTER:
>> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (hpet_enable=
d())
>> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0c=
ur_tick =3D hpet_get_ticks();
>> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0else
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (hpet_enable=
d(s)) {
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0c=
ur_tick =3D hpet_get_ticks(s);
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0} else {
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0cur_tick =3D s->hpet_counter;
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0DPRINTF("q=
emu: reading counter =C2=A0=3D %" PRIx64 "\n", cur_tick);
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return cur=
_tick;
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0case HPET_COUNTER + 4:
>> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (hpet_enable=
d())
>> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0c=
ur_tick =3D hpet_get_ticks();
>> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0else
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (hpet_enable=
d(s)) {
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0c=
ur_tick =3D hpet_get_ticks(s);
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0} else {
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0cur_tick =3D s->hpet_counter;
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0DPRINTF("q=
emu: reading counter + 4 =C2=A0=3D %" PRIx64 "\n",
>> cur_tick);
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return cur=
_tick >> 32;
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0case HPET_STATUS:
>> @@ -419,8 +418,9 @@ static void hpet_ram_writel(void *opaque,
>> target_phys_addr_t addr,
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | new_val;
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0timer->con=
fig &=3D ~HPET_TN_SETVAL;
>> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (hpet_enable=
d())
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (hpet_enable=
d(s)) {
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0hpet_set_timer(timer);
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0break;
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0case HPET_TN_CMP + 4: //=
 comparator register high order
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0DPRINTF("q=
emu: hpet_ram_writel HPET_TN_CMP + 4\n");
>> @@ -439,8 +439,9 @@ static void hpet_ram_writel(void *opaque,
>> target_phys_addr_t addr,
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | new_val <<=
 32;
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0timer->con=
fig &=3D ~HPET_TN_SETVAL;
>> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (hpet_enable=
d())
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (hpet_enable=
d(s)) {
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0hpet_set_timer(timer);
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0break;
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0case HPET_TN_ROUTE + 4:
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0DPRINTF("q=
emu: hpet_ram_writel HPET_TN_ROUTE + 4\n");
>> @@ -467,7 +468,7 @@ static void hpet_ram_writel(void *opaque,
>> target_phys_addr_t addr,
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0else if (d=
eactivating_bit(old_val, new_val, HPET_CFG_ENABLE)) {
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0/* Halt main counter and disable interrupt generation. */
>> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0s=
->hpet_counter =3D hpet_get_ticks();
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0s=
->hpet_counter =3D hpet_get_ticks(s);
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0for (i =3D 0; i < HPET_NUM_TIMERS; i++)
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0hpet_del_timer(&s->timer[i]);
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
>> @@ -485,16 +486,18 @@ static void hpet_ram_writel(void *opaque,
>> target_phys_addr_t addr,
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* FIXME: =
need to handle level-triggered interrupts */
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0break;
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0case HPET_COUNTER:
>> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (hpet_enabled())
>> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 printf(=
"qemu: Writing counter while HPET enabled!\n");
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (hpet_enable=
d(s)) {
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0p=
rintf("qemu: Writing counter while HPET enabled!\n");
>
> Missed it in my series as well: Should become DPRINTF at this chance.
> Also below.

Well, I only touched the line because the indentation is off. Perhaps
included in your patch 'hpet: Coding style cleanups and some
refactorings'? The file should also be reindented if possible.

>
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 s->hpet_counter =
=3D (s->hpet_counter & 0xffffffff00000000ULL)
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0| value;
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 DPRINTF("qemu: H=
PET counter written. ctr =3D %#x -> %"
>> PRIx64 "\n",
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0value, s->hpet_counter);
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0case HPET_COUNTER + 4:
>> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (hpet_enabled())
>> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 printf(=
"qemu: Writing counter while HPET enabled!\n");
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (hpet_enable=
d(s)) {
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0p=
rintf("qemu: Writing counter while HPET enabled!\n");
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 s->hpet_counter =
=3D (s->hpet_counter & 0xffffffffULL)
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0| (((uint64_t)value)=
 << 32);
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 DPRINTF("qemu: H=
PET counter + 4 written. ctr =3D %#x ->
>> %" PRIx64 "\n",
>> @@ -563,15 +566,14 @@ static void hpet_reset(void *opaque) {
>> =C2=A0 =C2=A0 =C2=A0count =3D 1;
>> =C2=A0}
>>
>> -
>> -void hpet_init(qemu_irq *irq) {
>> +HPETState *hpet_init(qemu_irq *irq)
>> +{
>> =C2=A0 =C2=A0 =C2=A0int i, iomemtype;
>> =C2=A0 =C2=A0 =C2=A0HPETState *s;
>>
>> =C2=A0 =C2=A0 =C2=A0DPRINTF ("hpet_init\n");
>>
>> =C2=A0 =C2=A0 =C2=A0s =3D qemu_mallocz(sizeof(HPETState));
>> - =C2=A0 =C2=A0hpet_statep =3D s;
>> =C2=A0 =C2=A0 =C2=A0s->irqs =3D irq;
>> =C2=A0 =C2=A0 =C2=A0for (i=3D0; i<HPET_NUM_TIMERS; i++) {
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0HPETTimer *timer =3D &s->timer[i];
>> @@ -583,4 +585,6 @@ void hpet_init(qemu_irq *irq) {
>> =C2=A0 =C2=A0 =C2=A0iomemtype =3D cpu_register_io_memory(hpet_ram_read,
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 hpet_=
ram_write, s);
>> =C2=A0 =C2=A0 =C2=A0cpu_register_physical_memory(HPET_BASE, 0x400, iomem=
type);
>> +
>> + =C2=A0 =C2=A0return s;
>> =C2=A0}
>> 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 {
>> =C2=A0} HPETState;
>>
>> =C2=A0#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);
>> =C2=A0#endif
>>
>> =C2=A0#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 leve=
l)
>> =C2=A0 =C2=A0 =C2=A0 * mode is established while interrupt is raised. We=
 want it to
>> =C2=A0 =C2=A0 =C2=A0 * be lowered in any case.
>> =C2=A0 =C2=A0 =C2=A0 */
>> - =C2=A0 =C2=A0if (!hpet_in_legacy_mode() || !level) {
>> + =C2=A0 =C2=A0if ((isa->hpet_state && !hpet_in_legacy_mode(isa->hpet_st=
ate)) || !level) {
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0isa_set_irq(isa, RTC_ISA_IRQ, level);
>> =C2=A0 =C2=A0 =C2=A0}
>> =C2=A0}
>> @@ -945,7 +945,7 @@ static void cpu_request_exit(void *opaque, int
>> irq, int level)
>> =C2=A0 =C2=A0 =C2=A0}
>> =C2=A0}
>>
>> -void pc_basic_device_init(qemu_irq *isa_irq,
>> +void pc_basic_device_init(qemu_irq *isa_irq, IsaIrqState *isa,
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0FDCtrl **floppy_controller,
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0ISADevice **rtc_state)
>> =C2=A0{
>> @@ -966,8 +966,10 @@ void pc_basic_device_init(qemu_irq *isa_irq,
>>
>> =C2=A0 =C2=A0 =C2=A0pit =3D pit_init(0x40, isa_reserve_irq(0));
>> =C2=A0 =C2=A0 =C2=A0pcspk_init(pit);
>> +
>> + =C2=A0 =C2=A0isa->hpet_state =3D NULL;
>> =C2=A0 =C2=A0 =C2=A0if (!no_hpet) {
>> - =C2=A0 =C2=A0 =C2=A0 =C2=A0hpet_init(isa_irq);
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0isa->hpet_state =3D hpet_init(isa_irq);
>> =C2=A0 =C2=A0 =C2=A0}
>>
>> =C2=A0 =C2=A0 =C2=A0for(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);
>> =C2=A0typedef struct isa_irq_state {
>> =C2=A0 =C2=A0 =C2=A0qemu_irq *i8259;
>> =C2=A0 =C2=A0 =C2=A0qemu_irq *ioapic;
>> + =C2=A0 =C2=A0void *hpet_state;
>> =C2=A0} IsaIrqState;
>>
>> =C2=A0void isa_irq_handler(void *opaque, int n, int level);
>> @@ -94,7 +95,7 @@ void pc_memory_init(ram_addr_t ram_size,
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0ram_addr_t *above_4g_mem_size_p);
>> =C2=A0qemu_irq *pc_allocate_cpu_irq(void);
>> =C2=A0void 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,
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0FDCtrl **floppy_controller,
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0ISADevice **rtc_state);
>> =C2=A0void 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,
>> =C2=A0 =C2=A0 =C2=A0pc_vga_init(pci_enabled? pci_bus: NULL);
>>
>> =C2=A0 =C2=A0 =C2=A0/* init basic PC hardware */
>> - =C2=A0 =C2=A0pc_basic_device_init(isa_irq, &floppy_controller, &rtc_st=
ate);
>> + =C2=A0 =C2=A0pc_basic_device_init(isa_irq, isa_irq_state, &floppy_cont=
roller,
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 &rtc_state);
>>
>> =C2=A0 =C2=A0 =C2=A0for(i =3D 0; i < nb_nics; i++) {
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0NICInfo *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.

But I'd like to rebase my patches to at least 'hpet: Convert to qdev',
that would make the pc.h field part cleaner. Could you extract that
from the series and send it earlier?

>
> Jan
>
> [1] git://git.kiszka.org/qemu.git queues/hpet
>
>

Nice work, the patches look OK. Thanks also for the review!