Re: [Qemu-devel] [PATCH v4 4/5] prepare to create HPET, RTC and i8254 through composition

2012-07-20 Thread Markus Armbruster 
Anthony Liguori aligu...@us.ibm.com writes:

 Markus Armbruster arm...@redhat.com writes:

 Wanpeng Li liw...@linux.vnet.ibm.com writes:

 [CCing ML]
  
  From: Anthony Liguori aligu...@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.

Re: [Qemu-devel] [PATCH v4 4/5] prepare to create HPET, RTC and i8254 through composition

2012-07-19 Thread Anthony Liguori 
Markus Armbruster arm...@redhat.com writes:

 Wanpeng Li liw...@linux.vnet.ibm.com writes:

 [CCing ML]
  
  From: Anthony Liguori aligu...@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

Re: [Qemu-devel] [PATCH v4 4/5] prepare to create HPET, RTC and i8254 through composition

2012-07-19 Thread Anthony Liguori 
Wanpeng Li liw...@linux.vnet.ibm.com writes:

 [CCing ML]
  
  From: Anthony Liguori aligu...@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 aligu...@us.ibm.com
 Signed-off-by: Wanpeng Li liw...@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_SUPPORT0
  
 -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   0xfed0
  #define HPET_CLK_PERIOD 1000ULL /* 1000 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 */
 +

Re: [Qemu-devel] [PATCH v4 4/5] prepare to create HPET, RTC and i8254 through composition

2012-07-19 Thread Wanpeng Li 
On Thu, Jul 19, 2012 at 03:23:32PM -0500, Anthony Liguori wrote:
Wanpeng Li liw...@linux.vnet.ibm.com writes:

 [CCing ML]
  
  From: Anthony Liguori aligu...@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 aligu...@us.ibm.com
 Signed-off-by: Wanpeng Li liw...@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_SUPPORT0
  
 -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   0xfed0
  #define HPET_CLK_PERIOD 1000ULL /* 1000 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];
 +
 +/*

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

2012-07-18 Thread Wanpeng Li 
[CCing ML]
 
 From: Anthony Liguori aligu...@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 aligu...@us.ibm.com
Signed-off-by: Wanpeng Li liw...@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_SUPPORT0
 
-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   0xfed0
 #define HPET_CLK_PERIOD 1000ULL /* 1000 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  =

Re: [Qemu-devel] [PATCH v4 4/5] prepare to create HPET, RTC and i8254 through composition

2012-07-18 Thread Markus Armbruster 
Wanpeng Li liw...@linux.vnet.ibm.com writes:

 [CCing ML]
  
  From: Anthony Liguori aligu...@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?