[v4,4/5] prepare to create HPET, RTC and i8254 through composition

[CCing ML]
 
 From: Anthony Liguori <aliguori@us.ibm.com>

The HPET usually sits on the LPC bus (which replaces ISA in modern systems).
It's sometimes a dedicated chip but can certain co-exist in a Super IO chip.
I think in terms of where it would live in this hypothetical device model,
putting it in the PIIX is rational.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
---
 hw/hpet.c           |   39 ++-------------------------------------
 hw/hpet_emul.h      |   41 +++++++++++++++++++++++++++++++++++++++++
 hw/i8254.c          |    2 +-
 hw/i8254_internal.h |    2 +-
 hw/mc146818rtc.c    |   26 --------------------------
 hw/mc146818rtc.h    |   30 ++++++++++++++++++++++++++++++
 6 files changed, 75 insertions(+), 65 deletions(-)

Wanpeng Li <liwanp@linux.vnet.ibm.com> writes:

> [CCing ML]
>  
>  From: Anthony Liguori <aliguori@us.ibm.com>
>
> The HPET usually sits on the LPC bus (which replaces ISA in modern systems).
> It's sometimes a dedicated chip but can certain co-exist in a Super IO chip.
> I think in terms of where it would live in this hypothetical device model,
> putting it in the PIIX is rational.

Could you explain briefly why you have to move struct definitions from
.c to .h?

Wanpeng Li <liwanp@linux.vnet.ibm.com> writes:

> [CCing ML]
>  
>  From: Anthony Liguori <aliguori@us.ibm.com>

Each of these devices should be a separate patch.

Please don't just send patches from branches of mine.  Spend some time
to understand the code and break things up appropriately.

Regards,

Anthony Liguori

> The HPET usually sits on the LPC bus (which replaces ISA in modern systems).
> It's sometimes a dedicated chip but can certain co-exist in a Super IO chip.
> I think in terms of where it would live in this hypothetical device model,
> putting it in the PIIX is rational.
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
> ---
>  hw/hpet.c           |   39 ++-------------------------------------
>  hw/hpet_emul.h      |   41 +++++++++++++++++++++++++++++++++++++++++
>  hw/i8254.c          |    2 +-
>  hw/i8254_internal.h |    2 +-
>  hw/mc146818rtc.c    |   26 --------------------------
>  hw/mc146818rtc.h    |   30 ++++++++++++++++++++++++++++++
>  6 files changed, 75 insertions(+), 65 deletions(-)
>
> diff --git a/hw/hpet.c b/hw/hpet.c
> index fd3ddca..fc0ff6c 100644
> --- a/hw/hpet.c
> +++ b/hw/hpet.c
> @@ -42,41 +42,6 @@
>  
>  #define HPET_MSI_SUPPORT        0
>  
> -struct HPETState;
> -typedef struct HPETTimer {  /* timers */
> -    uint8_t tn;             /*timer number*/
> -    QEMUTimer *qemu_timer;
> -    struct HPETState *state;
> -    /* Memory-mapped, software visible timer registers */
> -    uint64_t config;        /* configuration/cap */
> -    uint64_t cmp;           /* comparator */
> -    uint64_t fsb;           /* FSB route */
> -    /* Hidden register state */
> -    uint64_t period;        /* Last value written to comparator */
> -    uint8_t wrap_flag;      /* timer pop will indicate wrap for one-shot 32-bit
> -                             * mode. Next pop will be actual timer expiration.
> -                             */
> -} HPETTimer;
> -
> -typedef struct HPETState {
> -    SysBusDevice busdev;
> -    MemoryRegion iomem;
> -    uint64_t hpet_offset;
> -    qemu_irq irqs[HPET_NUM_IRQ_ROUTES];
> -    uint32_t flags;
> -    uint8_t rtc_irq_level;
> -    qemu_irq pit_enabled;
> -    uint8_t num_timers;
> -    HPETTimer timer[HPET_MAX_TIMERS];
> -
> -    /* Memory-mapped, software visible registers */
> -    uint64_t capability;        /* capabilities */
> -    uint64_t config;            /* configuration */
> -    uint64_t isr;               /* interrupt status reg */
> -    uint64_t hpet_counter;      /* main counter */
> -    uint8_t  hpet_id;           /* instance id */
> -} HPETState;
> -
>  static uint32_t hpet_in_legacy_mode(HPETState *s)
>  {
>      return s->config & HPET_CFG_LEGACY;
> @@ -278,7 +243,7 @@ static const VMStateDescription vmstate_hpet_timer = {
>  };
>  
>  static const VMStateDescription vmstate_hpet = {
> -    .name = "hpet",
> +    .name = TYPE_HPET,
>      .version_id = 2,
>      .minimum_version_id = 1,
>      .minimum_version_id_old = 1,
> @@ -746,7 +711,7 @@ static void hpet_device_class_init(ObjectClass *klass, void *data)
>  }
>  
>  static TypeInfo hpet_device_info = {
> -    .name          = "hpet",
> +    .name          = TYPE_HPET,
>      .parent        = TYPE_SYS_BUS_DEVICE,
>      .instance_size = sizeof(HPETState),
>      .class_init    = hpet_device_class_init,
> diff --git a/hw/hpet_emul.h b/hw/hpet_emul.h
> index 757f79f..836c5c8 100644
> --- a/hw/hpet_emul.h
> +++ b/hw/hpet_emul.h
> @@ -13,6 +13,9 @@
>  #ifndef QEMU_HPET_EMUL_H
>  #define QEMU_HPET_EMUL_H
>  
> +#include "hw.h"
> +#include "sysbus.h"
> +
>  #define HPET_BASE               0xfed00000
>  #define HPET_CLK_PERIOD         10000000ULL /* 10000000 femtoseconds == 10ns*/
>  
> @@ -71,4 +74,42 @@ struct hpet_fw_config
>  } QEMU_PACKED;
>  
>  extern struct hpet_fw_config hpet_cfg;
> +
> +#define TYPE_HPET "hpet"
> +
> +struct HPETState;
> +typedef struct HPETTimer {  /* timers */
> +    uint8_t tn;             /*timer number*/
> +    QEMUTimer *qemu_timer;
> +    struct HPETState *state;
> +    /* Memory-mapped, software visible timer registers */
> +    uint64_t config;        /* configuration/cap */
> +    uint64_t cmp;           /* comparator */
> +    uint64_t fsb;           /* FSB route */
> +    /* Hidden register state */
> +    uint64_t period;        /* Last value written to comparator */
> +    uint8_t wrap_flag;      /* timer pop will indicate wrap for one-shot 32-bit
> +                             * mode. Next pop will be actual timer expiration.
> +                             */
> +} HPETTimer;
> +
> +typedef struct HPETState {
> +    SysBusDevice busdev;
> +    MemoryRegion iomem;
> +    uint64_t hpet_offset;
> +    qemu_irq irqs[HPET_NUM_IRQ_ROUTES];
> +    uint32_t flags;
> +    uint8_t rtc_irq_level;
> +    qemu_irq pit_enabled;
> +    uint8_t num_timers;
> +    HPETTimer timer[HPET_MAX_TIMERS];
> +
> +    /* Memory-mapped, software visible registers */
> +    uint64_t capability;        /* capabilities */
> +    uint64_t config;            /* configuration */
> +    uint64_t isr;               /* interrupt status reg */
> +    uint64_t hpet_counter;      /* main counter */
> +    uint8_t  hpet_id;           /* instance id */
> +} HPETState;
> +
>  #endif
> diff --git a/hw/i8254.c b/hw/i8254.c
> index 77bd5e8..9d855ec 100644
> --- a/hw/i8254.c
> +++ b/hw/i8254.c
> @@ -346,7 +346,7 @@ static void pit_class_initfn(ObjectClass *klass, void *data)
>  }
>  
>  static TypeInfo pit_info = {
> -    .name          = "isa-pit",
> +    .name          = TYPE_PIT,
>      .parent        = TYPE_PIT_COMMON,
>      .instance_size = sizeof(PITCommonState),
>      .class_init    = pit_class_initfn,
> diff --git a/hw/i8254_internal.h b/hw/i8254_internal.h
> index 686f0c2..542f7c1 100644
> --- a/hw/i8254_internal.h
> +++ b/hw/i8254_internal.h
> @@ -26,7 +26,6 @@
>  #define QEMU_I8254_INTERNAL_H
>  
>  #include "hw.h"
> -#include "pc.h"
>  #include "isa.h"
>  
>  typedef struct PITChannelState {
> @@ -57,6 +56,7 @@ typedef struct PITCommonState {
>      PITChannelState channels[3];
>  } PITCommonState;
>  
> +#define TYPE_PIT "isa-pit"
>  #define TYPE_PIT_COMMON "pit-common"
>  #define PIT_COMMON(obj) \
>       OBJECT_CHECK(PITCommonState, (obj), TYPE_PIT_COMMON)
> diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
> index 3777f85..22f0bb2 100644
> --- a/hw/mc146818rtc.c
> +++ b/hw/mc146818rtc.c
> @@ -47,32 +47,6 @@
>  
>  #define RTC_REINJECT_ON_ACK_COUNT 20
>  
> -typedef struct RTCState {
> -    ISADevice dev;
> -    MemoryRegion io;
> -    uint8_t cmos_data[128];
> -    uint8_t cmos_index;
> -    struct tm current_tm;
> -    int32_t base_year;
> -    qemu_irq irq;
> -    qemu_irq sqw_irq;
> -    int it_shift;
> -    /* periodic timer */
> -    QEMUTimer *periodic_timer;
> -    int64_t next_periodic_time;
> -    /* second update */
> -    int64_t next_second_time;
> -    uint16_t irq_reinject_on_ack_count;
> -    uint32_t irq_coalesced;
> -    uint32_t period;
> -    QEMUTimer *coalesced_timer;
> -    QEMUTimer *second_timer;
> -    QEMUTimer *second_timer2;
> -    Notifier clock_reset_notifier;
> -    LostTickPolicy lost_tick_policy;
> -    Notifier suspend_notifier;
> -} RTCState;
> -
>  static void rtc_set_time(RTCState *s);
>  static void rtc_copy_date(RTCState *s);
>  
> diff --git a/hw/mc146818rtc.h b/hw/mc146818rtc.h
> index f286b6a..dbd5561 100644
> --- a/hw/mc146818rtc.h
> +++ b/hw/mc146818rtc.h
> @@ -3,9 +3,39 @@
>  
>  #include "isa.h"
>  #include "mc146818rtc_regs.h"
> +#include "notify.h"
> +
> +#define TYPE_RTC "mc146818rtc"
> +
> +typedef struct RTCState {
> +    ISADevice dev;
> +    MemoryRegion io;
> +    uint8_t cmos_data[128];
> +    uint8_t cmos_index;
> +    struct tm current_tm;
> +    int32_t base_year;
> +    qemu_irq irq;
> +    qemu_irq sqw_irq;
> +    int it_shift;
> +    /* periodic timer */
> +    QEMUTimer *periodic_timer;
> +    int64_t next_periodic_time;
> +    /* second update */
> +    int64_t next_second_time;
> +    uint16_t irq_reinject_on_ack_count;
> +    uint32_t irq_coalesced;
> +    uint32_t period;
> +    QEMUTimer *coalesced_timer;
> +    QEMUTimer *second_timer;
> +    QEMUTimer *second_timer2;
> +    Notifier clock_reset_notifier;
> +    LostTickPolicy lost_tick_policy;
> +    Notifier suspend_notifier;
> +} RTCState;
>  
>  ISADevice *rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq);
>  void rtc_set_memory(ISADevice *dev, int addr, int val);
>  void rtc_set_date(ISADevice *dev, const struct tm *tm);
>  
>  #endif /* !MC146818RTC_H */
> +
> -- 
> 1.7.5.4

Markus Armbruster <armbru@redhat.com> writes:

> Wanpeng Li <liwanp@linux.vnet.ibm.com> writes:
>
>> [CCing ML]
>>  
>>  From: Anthony Liguori <aliguori@us.ibm.com>
>>
>> The HPET usually sits on the LPC bus (which replaces ISA in modern systems).
>> It's sometimes a dedicated chip but can certain co-exist in a Super IO chip.
>> I think in terms of where it would live in this hypothetical device model,
>> putting it in the PIIX is rational.
>
> Could you explain briefly why you have to move struct definitions from
> .c to .h?

This patch shouldn't be in this series.  The reason to do this is so
that the PIIX3 can do composition and have the HPET/PIT as child
devices.  But that isn't in this series so having this patch isn't
useful.

Regards,

Anthony Liguori

On Thu, Jul 19, 2012 at 03:23:32PM -0500, Anthony Liguori wrote:
>Wanpeng Li <liwanp@linux.vnet.ibm.com> writes:
>
>> [CCing ML]
>>  
>>  From: Anthony Liguori <aliguori@us.ibm.com>
>
>Each of these devices should be a separate patch.
>
>Please don't just send patches from branches of mine.  Spend some time
>to understand the code and break things up appropriately.

Yes, I will. But some guys still object export structure from *.c to
*.h, It also lead to compile error which I send you. :-)

Regards,
Wanpeng Li
>
>Regards,
>
>Anthony Liguori
>
>> The HPET usually sits on the LPC bus (which replaces ISA in modern systems).
>> It's sometimes a dedicated chip but can certain co-exist in a Super IO chip.
>> I think in terms of where it would live in this hypothetical device model,
>> putting it in the PIIX is rational.
>>
>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>> Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
>> ---
>>  hw/hpet.c           |   39 ++-------------------------------------
>>  hw/hpet_emul.h      |   41 +++++++++++++++++++++++++++++++++++++++++
>>  hw/i8254.c          |    2 +-
>>  hw/i8254_internal.h |    2 +-
>>  hw/mc146818rtc.c    |   26 --------------------------
>>  hw/mc146818rtc.h    |   30 ++++++++++++++++++++++++++++++
>>  6 files changed, 75 insertions(+), 65 deletions(-)
>>
>> diff --git a/hw/hpet.c b/hw/hpet.c
>> index fd3ddca..fc0ff6c 100644
>> --- a/hw/hpet.c
>> +++ b/hw/hpet.c
>> @@ -42,41 +42,6 @@
>>  
>>  #define HPET_MSI_SUPPORT        0
>>  
>> -struct HPETState;
>> -typedef struct HPETTimer {  /* timers */
>> -    uint8_t tn;             /*timer number*/
>> -    QEMUTimer *qemu_timer;
>> -    struct HPETState *state;
>> -    /* Memory-mapped, software visible timer registers */
>> -    uint64_t config;        /* configuration/cap */
>> -    uint64_t cmp;           /* comparator */
>> -    uint64_t fsb;           /* FSB route */
>> -    /* Hidden register state */
>> -    uint64_t period;        /* Last value written to comparator */
>> -    uint8_t wrap_flag;      /* timer pop will indicate wrap for one-shot 32-bit
>> -                             * mode. Next pop will be actual timer expiration.
>> -                             */
>> -} HPETTimer;
>> -
>> -typedef struct HPETState {
>> -    SysBusDevice busdev;
>> -    MemoryRegion iomem;
>> -    uint64_t hpet_offset;
>> -    qemu_irq irqs[HPET_NUM_IRQ_ROUTES];
>> -    uint32_t flags;
>> -    uint8_t rtc_irq_level;
>> -    qemu_irq pit_enabled;
>> -    uint8_t num_timers;
>> -    HPETTimer timer[HPET_MAX_TIMERS];
>> -
>> -    /* Memory-mapped, software visible registers */
>> -    uint64_t capability;        /* capabilities */
>> -    uint64_t config;            /* configuration */
>> -    uint64_t isr;               /* interrupt status reg */
>> -    uint64_t hpet_counter;      /* main counter */
>> -    uint8_t  hpet_id;           /* instance id */
>> -} HPETState;
>> -
>>  static uint32_t hpet_in_legacy_mode(HPETState *s)
>>  {
>>      return s->config & HPET_CFG_LEGACY;
>> @@ -278,7 +243,7 @@ static const VMStateDescription vmstate_hpet_timer = {
>>  };
>>  
>>  static const VMStateDescription vmstate_hpet = {
>> -    .name = "hpet",
>> +    .name = TYPE_HPET,
>>      .version_id = 2,
>>      .minimum_version_id = 1,
>>      .minimum_version_id_old = 1,
>> @@ -746,7 +711,7 @@ static void hpet_device_class_init(ObjectClass *klass, void *data)
>>  }
>>  
>>  static TypeInfo hpet_device_info = {
>> -    .name          = "hpet",
>> +    .name          = TYPE_HPET,
>>      .parent        = TYPE_SYS_BUS_DEVICE,
>>      .instance_size = sizeof(HPETState),
>>      .class_init    = hpet_device_class_init,
>> diff --git a/hw/hpet_emul.h b/hw/hpet_emul.h
>> index 757f79f..836c5c8 100644
>> --- a/hw/hpet_emul.h
>> +++ b/hw/hpet_emul.h
>> @@ -13,6 +13,9 @@
>>  #ifndef QEMU_HPET_EMUL_H
>>  #define QEMU_HPET_EMUL_H
>>  
>> +#include "hw.h"
>> +#include "sysbus.h"
>> +
>>  #define HPET_BASE               0xfed00000
>>  #define HPET_CLK_PERIOD         10000000ULL /* 10000000 femtoseconds == 10ns*/
>>  
>> @@ -71,4 +74,42 @@ struct hpet_fw_config
>>  } QEMU_PACKED;
>>  
>>  extern struct hpet_fw_config hpet_cfg;
>> +
>> +#define TYPE_HPET "hpet"
>> +
>> +struct HPETState;
>> +typedef struct HPETTimer {  /* timers */
>> +    uint8_t tn;             /*timer number*/
>> +    QEMUTimer *qemu_timer;
>> +    struct HPETState *state;
>> +    /* Memory-mapped, software visible timer registers */
>> +    uint64_t config;        /* configuration/cap */
>> +    uint64_t cmp;           /* comparator */
>> +    uint64_t fsb;           /* FSB route */
>> +    /* Hidden register state */
>> +    uint64_t period;        /* Last value written to comparator */
>> +    uint8_t wrap_flag;      /* timer pop will indicate wrap for one-shot 32-bit
>> +                             * mode. Next pop will be actual timer expiration.
>> +                             */
>> +} HPETTimer;
>> +
>> +typedef struct HPETState {
>> +    SysBusDevice busdev;
>> +    MemoryRegion iomem;
>> +    uint64_t hpet_offset;
>> +    qemu_irq irqs[HPET_NUM_IRQ_ROUTES];
>> +    uint32_t flags;
>> +    uint8_t rtc_irq_level;
>> +    qemu_irq pit_enabled;
>> +    uint8_t num_timers;
>> +    HPETTimer timer[HPET_MAX_TIMERS];
>> +
>> +    /* Memory-mapped, software visible registers */
>> +    uint64_t capability;        /* capabilities */
>> +    uint64_t config;            /* configuration */
>> +    uint64_t isr;               /* interrupt status reg */
>> +    uint64_t hpet_counter;      /* main counter */
>> +    uint8_t  hpet_id;           /* instance id */
>> +} HPETState;
>> +
>>  #endif
>> diff --git a/hw/i8254.c b/hw/i8254.c
>> index 77bd5e8..9d855ec 100644
>> --- a/hw/i8254.c
>> +++ b/hw/i8254.c
>> @@ -346,7 +346,7 @@ static void pit_class_initfn(ObjectClass *klass, void *data)
>>  }
>>  
>>  static TypeInfo pit_info = {
>> -    .name          = "isa-pit",
>> +    .name          = TYPE_PIT,
>>      .parent        = TYPE_PIT_COMMON,
>>      .instance_size = sizeof(PITCommonState),
>>      .class_init    = pit_class_initfn,
>> diff --git a/hw/i8254_internal.h b/hw/i8254_internal.h
>> index 686f0c2..542f7c1 100644
>> --- a/hw/i8254_internal.h
>> +++ b/hw/i8254_internal.h
>> @@ -26,7 +26,6 @@
>>  #define QEMU_I8254_INTERNAL_H
>>  
>>  #include "hw.h"
>> -#include "pc.h"
>>  #include "isa.h"
>>  
>>  typedef struct PITChannelState {
>> @@ -57,6 +56,7 @@ typedef struct PITCommonState {
>>      PITChannelState channels[3];
>>  } PITCommonState;
>>  
>> +#define TYPE_PIT "isa-pit"
>>  #define TYPE_PIT_COMMON "pit-common"
>>  #define PIT_COMMON(obj) \
>>       OBJECT_CHECK(PITCommonState, (obj), TYPE_PIT_COMMON)
>> diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
>> index 3777f85..22f0bb2 100644
>> --- a/hw/mc146818rtc.c
>> +++ b/hw/mc146818rtc.c
>> @@ -47,32 +47,6 @@
>>  
>>  #define RTC_REINJECT_ON_ACK_COUNT 20
>>  
>> -typedef struct RTCState {
>> -    ISADevice dev;
>> -    MemoryRegion io;
>> -    uint8_t cmos_data[128];
>> -    uint8_t cmos_index;
>> -    struct tm current_tm;
>> -    int32_t base_year;
>> -    qemu_irq irq;
>> -    qemu_irq sqw_irq;
>> -    int it_shift;
>> -    /* periodic timer */
>> -    QEMUTimer *periodic_timer;
>> -    int64_t next_periodic_time;
>> -    /* second update */
>> -    int64_t next_second_time;
>> -    uint16_t irq_reinject_on_ack_count;
>> -    uint32_t irq_coalesced;
>> -    uint32_t period;
>> -    QEMUTimer *coalesced_timer;
>> -    QEMUTimer *second_timer;
>> -    QEMUTimer *second_timer2;
>> -    Notifier clock_reset_notifier;
>> -    LostTickPolicy lost_tick_policy;
>> -    Notifier suspend_notifier;
>> -} RTCState;
>> -
>>  static void rtc_set_time(RTCState *s);
>>  static void rtc_copy_date(RTCState *s);
>>  
>> diff --git a/hw/mc146818rtc.h b/hw/mc146818rtc.h
>> index f286b6a..dbd5561 100644
>> --- a/hw/mc146818rtc.h
>> +++ b/hw/mc146818rtc.h
>> @@ -3,9 +3,39 @@
>>  
>>  #include "isa.h"
>>  #include "mc146818rtc_regs.h"
>> +#include "notify.h"
>> +
>> +#define TYPE_RTC "mc146818rtc"
>> +
>> +typedef struct RTCState {
>> +    ISADevice dev;
>> +    MemoryRegion io;
>> +    uint8_t cmos_data[128];
>> +    uint8_t cmos_index;
>> +    struct tm current_tm;
>> +    int32_t base_year;
>> +    qemu_irq irq;
>> +    qemu_irq sqw_irq;
>> +    int it_shift;
>> +    /* periodic timer */
>> +    QEMUTimer *periodic_timer;
>> +    int64_t next_periodic_time;
>> +    /* second update */
>> +    int64_t next_second_time;
>> +    uint16_t irq_reinject_on_ack_count;
>> +    uint32_t irq_coalesced;
>> +    uint32_t period;
>> +    QEMUTimer *coalesced_timer;
>> +    QEMUTimer *second_timer;
>> +    QEMUTimer *second_timer2;
>> +    Notifier clock_reset_notifier;
>> +    LostTickPolicy lost_tick_policy;
>> +    Notifier suspend_notifier;
>> +} RTCState;
>>  
>>  ISADevice *rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq);
>>  void rtc_set_memory(ISADevice *dev, int addr, int val);
>>  void rtc_set_date(ISADevice *dev, const struct tm *tm);
>>  
>>  #endif /* !MC146818RTC_H */
>> +
>> -- 
>> 1.7.5.4
>

Anthony Liguori <aliguori@us.ibm.com> writes:

> Markus Armbruster <armbru@redhat.com> writes:
>
>> Wanpeng Li <liwanp@linux.vnet.ibm.com> writes:
>>
>>> [CCing ML]
>>>  
>>>  From: Anthony Liguori <aliguori@us.ibm.com>
>>>
>>> The HPET usually sits on the LPC bus (which replaces ISA in modern systems).
>>> It's sometimes a dedicated chip but can certain co-exist in a Super IO chip.
>>> I think in terms of where it would live in this hypothetical device model,
>>> putting it in the PIIX is rational.
>>
>> Could you explain briefly why you have to move struct definitions from
>> .c to .h?
>
> This patch shouldn't be in this series.  The reason to do this is so
> that the PIIX3 can do composition and have the HPET/PIT as child
> devices.  But that isn't in this series so having this patch isn't
> useful.

Let's drop it then for now.