Re: [Qemu-devel] [PATCH v6 2/5] hpet: enable to entitle more irq pins for hpet

2013-10-14 Thread Michael S. Tsirkin
On Fri, Oct 11, 2013 at 05:18:30PM +0800, liu ping fan wrote:
 On Fri, Oct 11, 2013 at 4:38 PM, Paolo Bonzini pbonz...@redhat.com wrote:
  Il 11/10/2013 04:59, liu ping fan ha scritto:
  On Thu, Oct 10, 2013 at 7:41 PM, Michael S. Tsirkin m...@redhat.com 
  wrote:
  On Thu, Oct 10, 2013 at 11:46:42AM +0200, Paolo Bonzini wrote:
  Il 10/10/2013 11:41, Michael S. Tsirkin ha scritto:
  Are you sure?  This is not done for any other compat property.
 
  Paolo
  It's done if we use the property from C.
  See PCI_HOST_PROP_PCI_HOLE64_SIZE.
 
  You want compiler to catch errors, that's
  much better than a runtime failure.
 
  I agree, but I think there should be no need to use the property from C.
 
  Paolo
 
  Well this patchset does use it from C.
  If it's done it needs a macro.
 
  hpet.h is the ideal place to put the macro, so pc.c can see it. But
  what about PC_COMPAT_1_7 in pc.h? I think it is not right to include
  hpet.h. So can I do not use marco in pc.h?
 
  I think you shouldn't need the macro (no need to use the property from
  C, only from compat properties).
 
 We need to tell the compat and then decide to set intcap in
 pc_basic_device_init()
 
 uint8_t compat = object_property_get_int(OBJECT(hpet),
 intcap, NULL);
 if (!compat) {
 qdev_prop_set_uint32(hpet, intcap, 0xff0104);
 }
 
 Regards,
 Ping Fan

So if you use it from C, please use a macro.
If not, no need to.




Re: [Qemu-devel] [PATCH v6 2/5] hpet: enable to entitle more irq pins for hpet

2013-10-14 Thread Michael S. Tsirkin
On Fri, Oct 11, 2013 at 10:59:40AM +0800, liu ping fan wrote:
 On Thu, Oct 10, 2013 at 7:41 PM, Michael S. Tsirkin m...@redhat.com wrote:
  On Thu, Oct 10, 2013 at 11:46:42AM +0200, Paolo Bonzini wrote:
  Il 10/10/2013 11:41, Michael S. Tsirkin ha scritto:
Are you sure?  This is not done for any other compat property.
   
Paolo
   It's done if we use the property from C.
   See PCI_HOST_PROP_PCI_HOLE64_SIZE.
  
   You want compiler to catch errors, that's
   much better than a runtime failure.
 
  I agree, but I think there should be no need to use the property from C.
 
  Paolo
 
  Well this patchset does use it from C.
  If it's done it needs a macro.
 
 hpet.h is the ideal place to put the macro, so pc.c can see it. But
 what about PC_COMPAT_1_7 in pc.h? I think it is not right to include
 hpet.h.

Why not?

 So can I do not use marco in pc.h?
 
 Thanks and regards,
 Ping Fan



Re: [Qemu-devel] [PATCH v6 2/5] hpet: enable to entitle more irq pins for hpet

2013-10-14 Thread liu ping fan
On Mon, Oct 14, 2013 at 10:18 PM, Michael S. Tsirkin m...@redhat.com wrote:
 On Fri, Oct 11, 2013 at 10:59:40AM +0800, liu ping fan wrote:
 On Thu, Oct 10, 2013 at 7:41 PM, Michael S. Tsirkin m...@redhat.com wrote:
  On Thu, Oct 10, 2013 at 11:46:42AM +0200, Paolo Bonzini wrote:
  Il 10/10/2013 11:41, Michael S. Tsirkin ha scritto:
Are you sure?  This is not done for any other compat property.
   
Paolo
   It's done if we use the property from C.
   See PCI_HOST_PROP_PCI_HOLE64_SIZE.
  
   You want compiler to catch errors, that's
   much better than a runtime failure.
 
  I agree, but I think there should be no need to use the property from C.
 
  Paolo
 
  Well this patchset does use it from C.
  If it's done it needs a macro.

 hpet.h is the ideal place to put the macro, so pc.c can see it. But
 what about PC_COMPAT_1_7 in pc.h? I think it is not right to include
 hpet.h.

 Why not?

Since pc.h is included by so many hpet unrelated drivers, if pc.h
include hpet.h, then we will export the internal of hpet struct.

Regards,
Ping Fan



Re: [Qemu-devel] [PATCH v6 2/5] hpet: enable to entitle more irq pins for hpet

2013-10-11 Thread Paolo Bonzini
Il 11/10/2013 04:59, liu ping fan ha scritto:
 On Thu, Oct 10, 2013 at 7:41 PM, Michael S. Tsirkin m...@redhat.com wrote:
 On Thu, Oct 10, 2013 at 11:46:42AM +0200, Paolo Bonzini wrote:
 Il 10/10/2013 11:41, Michael S. Tsirkin ha scritto:
 Are you sure?  This is not done for any other compat property.

 Paolo
 It's done if we use the property from C.
 See PCI_HOST_PROP_PCI_HOLE64_SIZE.

 You want compiler to catch errors, that's
 much better than a runtime failure.

 I agree, but I think there should be no need to use the property from C.

 Paolo

 Well this patchset does use it from C.
 If it's done it needs a macro.
 
 hpet.h is the ideal place to put the macro, so pc.c can see it. But
 what about PC_COMPAT_1_7 in pc.h? I think it is not right to include
 hpet.h. So can I do not use marco in pc.h?

I think you shouldn't need the macro (no need to use the property from
C, only from compat properties).

Paolo




Re: [Qemu-devel] [PATCH v6 2/5] hpet: enable to entitle more irq pins for hpet

2013-10-11 Thread liu ping fan
On Fri, Oct 11, 2013 at 4:38 PM, Paolo Bonzini pbonz...@redhat.com wrote:
 Il 11/10/2013 04:59, liu ping fan ha scritto:
 On Thu, Oct 10, 2013 at 7:41 PM, Michael S. Tsirkin m...@redhat.com wrote:
 On Thu, Oct 10, 2013 at 11:46:42AM +0200, Paolo Bonzini wrote:
 Il 10/10/2013 11:41, Michael S. Tsirkin ha scritto:
 Are you sure?  This is not done for any other compat property.

 Paolo
 It's done if we use the property from C.
 See PCI_HOST_PROP_PCI_HOLE64_SIZE.

 You want compiler to catch errors, that's
 much better than a runtime failure.

 I agree, but I think there should be no need to use the property from C.

 Paolo

 Well this patchset does use it from C.
 If it's done it needs a macro.

 hpet.h is the ideal place to put the macro, so pc.c can see it. But
 what about PC_COMPAT_1_7 in pc.h? I think it is not right to include
 hpet.h. So can I do not use marco in pc.h?

 I think you shouldn't need the macro (no need to use the property from
 C, only from compat properties).

We need to tell the compat and then decide to set intcap in
pc_basic_device_init()

uint8_t compat = object_property_get_int(OBJECT(hpet),
intcap, NULL);
if (!compat) {
qdev_prop_set_uint32(hpet, intcap, 0xff0104);
}

Regards,
Ping Fan



Re: [Qemu-devel] [PATCH v6 2/5] hpet: enable to entitle more irq pins for hpet

2013-10-11 Thread Paolo Bonzini
Il 11/10/2013 11:18, liu ping fan ha scritto:
 On Fri, Oct 11, 2013 at 4:38 PM, Paolo Bonzini pbonz...@redhat.com wrote:
 Il 11/10/2013 04:59, liu ping fan ha scritto:
 On Thu, Oct 10, 2013 at 7:41 PM, Michael S. Tsirkin m...@redhat.com wrote:
 On Thu, Oct 10, 2013 at 11:46:42AM +0200, Paolo Bonzini wrote:
 Il 10/10/2013 11:41, Michael S. Tsirkin ha scritto:
 Are you sure?  This is not done for any other compat property.

 Paolo
 It's done if we use the property from C.
 See PCI_HOST_PROP_PCI_HOLE64_SIZE.

 You want compiler to catch errors, that's
 much better than a runtime failure.

 I agree, but I think there should be no need to use the property from C.

 Paolo

 Well this patchset does use it from C.
 If it's done it needs a macro.

 hpet.h is the ideal place to put the macro, so pc.c can see it. But
 what about PC_COMPAT_1_7 in pc.h? I think it is not right to include
 hpet.h. So can I do not use marco in pc.h?

 I think you shouldn't need the macro (no need to use the property from
 C, only from compat properties).

 We need to tell the compat and then decide to set intcap in
 pc_basic_device_init()
 
 uint8_t compat = object_property_get_int(OBJECT(hpet),
 intcap, NULL);
 if (!compat) {
 qdev_prop_set_uint32(hpet, intcap, 0xff0104);
 }

No, that can be done via global properties as well.

Paolo



[Qemu-devel] [PATCH v6 2/5] hpet: enable to entitle more irq pins for hpet

2013-10-10 Thread Liu Ping Fan
On q35, IRQ2/8 can be reserved for hpet timer 0/1. And pin 16~23
of ioapic can be dynamically assigned to hpet as guest chooses.
So we introduce intcap property to do that. (currently, its value
is IRQ2. Later, it should be set by board.)

Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com
---
 hw/timer/hpet.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index 8429eb3..5b11be4 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -25,6 +25,7 @@
  */
 
 #include hw/hw.h
+#include hw/boards.h
 #include hw/i386/pc.h
 #include ui/console.h
 #include qemu/timer.h
@@ -42,6 +43,9 @@
 
 #define HPET_MSI_SUPPORT0
 
+/* Will fix: intcap is set by board, and should be 0 if nobody sets. */
+#define HPET_TN_INT_CAP_DEFAULT 0x4ULL
+
 #define TYPE_HPET hpet
 #define HPET(obj) OBJECT_CHECK(HPETState, (obj), TYPE_HPET)
 
@@ -73,6 +77,7 @@ typedef struct HPETState {
 uint8_t rtc_irq_level;
 qemu_irq pit_enabled;
 uint8_t num_timers;
+uint32_t intcap;
 HPETTimer timer[HPET_MAX_TIMERS];
 
 /* Memory-mapped, software visible registers */
@@ -663,8 +668,8 @@ static void hpet_reset(DeviceState *d)
 if (s-flags  (1  HPET_MSI_SUPPORT)) {
 timer-config |= HPET_TN_FSB_CAP;
 }
-/* advertise availability of ioapic inti2 */
-timer-config |=  0x0004ULL  32;
+/* advertise availability of ioapic int */
+timer-config |=  (uint64_t)s-intcap  32;
 timer-period = 0ULL;
 timer-wrap_flag = 0;
 }
@@ -753,6 +758,7 @@ static void hpet_realize(DeviceState *dev, Error **errp)
 static Property hpet_device_properties[] = {
 DEFINE_PROP_UINT8(timers, HPETState, num_timers, HPET_MIN_TIMERS),
 DEFINE_PROP_BIT(msi, HPETState, flags, HPET_MSI_SUPPORT, false),
+DEFINE_PROP_UINT32(intcap, HPETState, intcap, HPET_TN_INT_CAP_DEFAULT),
 DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
1.8.1.4




Re: [Qemu-devel] [PATCH v6 2/5] hpet: enable to entitle more irq pins for hpet

2013-10-10 Thread Paolo Bonzini
Il 10/10/2013 09:56, Liu Ping Fan ha scritto:
 On q35, IRQ2/8 can be reserved for hpet timer 0/1. And pin 16~23
 of ioapic can be dynamically assigned to hpet as guest chooses.
 So we introduce intcap property to do that. (currently, its value
 is IRQ2. Later, it should be set by board.)
 
 Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com
 ---
  hw/timer/hpet.c | 10 --
  1 file changed, 8 insertions(+), 2 deletions(-)
 
 diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
 index 8429eb3..5b11be4 100644
 --- a/hw/timer/hpet.c
 +++ b/hw/timer/hpet.c
 @@ -25,6 +25,7 @@
   */
  
  #include hw/hw.h
 +#include hw/boards.h
  #include hw/i386/pc.h
  #include ui/console.h
  #include qemu/timer.h
 @@ -42,6 +43,9 @@
  
  #define HPET_MSI_SUPPORT0
  
 +/* Will fix: intcap is set by board, and should be 0 if nobody sets. */
 +#define HPET_TN_INT_CAP_DEFAULT 0x4ULL
 +
  #define TYPE_HPET hpet
  #define HPET(obj) OBJECT_CHECK(HPETState, (obj), TYPE_HPET)
  
 @@ -73,6 +77,7 @@ typedef struct HPETState {
  uint8_t rtc_irq_level;
  qemu_irq pit_enabled;
  uint8_t num_timers;
 +uint32_t intcap;
  HPETTimer timer[HPET_MAX_TIMERS];
  
  /* Memory-mapped, software visible registers */
 @@ -663,8 +668,8 @@ static void hpet_reset(DeviceState *d)
  if (s-flags  (1  HPET_MSI_SUPPORT)) {
  timer-config |= HPET_TN_FSB_CAP;
  }
 -/* advertise availability of ioapic inti2 */
 -timer-config |=  0x0004ULL  32;
 +/* advertise availability of ioapic int */
 +timer-config |=  (uint64_t)s-intcap  32;
  timer-period = 0ULL;
  timer-wrap_flag = 0;
  }
 @@ -753,6 +758,7 @@ static void hpet_realize(DeviceState *dev, Error **errp)
  static Property hpet_device_properties[] = {
  DEFINE_PROP_UINT8(timers, HPETState, num_timers, HPET_MIN_TIMERS),
  DEFINE_PROP_BIT(msi, HPETState, flags, HPET_MSI_SUPPORT, false),
 +DEFINE_PROP_UINT32(intcap, HPETState, intcap, HPET_TN_INT_CAP_DEFAULT),
  DEFINE_PROP_END_OF_LIST(),
  };
  
 

According to Michael's request, a zero intcap should be detected in
hpet_realize and give an error.

Paolo



Re: [Qemu-devel] [PATCH v6 2/5] hpet: enable to entitle more irq pins for hpet

2013-10-10 Thread Michael S. Tsirkin
On Thu, Oct 10, 2013 at 03:56:16PM +0800, Liu Ping Fan wrote:
 On q35, IRQ2/8 can be reserved for hpet timer 0/1. And pin 16~23
 of ioapic can be dynamically assigned to hpet as guest chooses.
 So we introduce intcap property to do that. (currently, its value
 is IRQ2. Later, it should be set by board.)
 
 Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com
 ---
  hw/timer/hpet.c | 10 --
  1 file changed, 8 insertions(+), 2 deletions(-)
 
 diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
 index 8429eb3..5b11be4 100644
 --- a/hw/timer/hpet.c
 +++ b/hw/timer/hpet.c
 @@ -25,6 +25,7 @@
   */
  
  #include hw/hw.h
 +#include hw/boards.h
  #include hw/i386/pc.h
  #include ui/console.h
  #include qemu/timer.h
 @@ -42,6 +43,9 @@
  
  #define HPET_MSI_SUPPORT0
  
 +/* Will fix: intcap is set by board, and should be 0 if nobody sets. */
 +#define HPET_TN_INT_CAP_DEFAULT 0x4ULL
 +
  #define TYPE_HPET hpet
  #define HPET(obj) OBJECT_CHECK(HPETState, (obj), TYPE_HPET)
  
 @@ -73,6 +77,7 @@ typedef struct HPETState {
  uint8_t rtc_irq_level;
  qemu_irq pit_enabled;
  uint8_t num_timers;
 +uint32_t intcap;
  HPETTimer timer[HPET_MAX_TIMERS];
  
  /* Memory-mapped, software visible registers */
 @@ -663,8 +668,8 @@ static void hpet_reset(DeviceState *d)
  if (s-flags  (1  HPET_MSI_SUPPORT)) {
  timer-config |= HPET_TN_FSB_CAP;
  }
 -/* advertise availability of ioapic inti2 */
 -timer-config |=  0x0004ULL  32;
 +/* advertise availability of ioapic int */
 +timer-config |=  (uint64_t)s-intcap  32;
  timer-period = 0ULL;
  timer-wrap_flag = 0;
  }
 @@ -753,6 +758,7 @@ static void hpet_realize(DeviceState *dev, Error **errp)
  static Property hpet_device_properties[] = {
  DEFINE_PROP_UINT8(timers, HPETState, num_timers, HPET_MIN_TIMERS),
  DEFINE_PROP_BIT(msi, HPETState, flags, HPET_MSI_SUPPORT, false),
 +DEFINE_PROP_UINT32(intcap, HPETState, intcap, HPET_TN_INT_CAP_DEFAULT),
  DEFINE_PROP_END_OF_LIST(),
  };

Please add a macro for this name as you use it in other
files later.

  
 -- 
 1.8.1.4



Re: [Qemu-devel] [PATCH v6 2/5] hpet: enable to entitle more irq pins for hpet

2013-10-10 Thread Paolo Bonzini
Il 10/10/2013 11:16, Michael S. Tsirkin ha scritto:
 On Thu, Oct 10, 2013 at 03:56:16PM +0800, Liu Ping Fan wrote:
 On q35, IRQ2/8 can be reserved for hpet timer 0/1. And pin 16~23
 of ioapic can be dynamically assigned to hpet as guest chooses.
 So we introduce intcap property to do that. (currently, its value
 is IRQ2. Later, it should be set by board.)

 Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com
 ---
  hw/timer/hpet.c | 10 --
  1 file changed, 8 insertions(+), 2 deletions(-)

 diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
 index 8429eb3..5b11be4 100644
 --- a/hw/timer/hpet.c
 +++ b/hw/timer/hpet.c
 @@ -25,6 +25,7 @@
   */
  
  #include hw/hw.h
 +#include hw/boards.h
  #include hw/i386/pc.h
  #include ui/console.h
  #include qemu/timer.h
 @@ -42,6 +43,9 @@
  
  #define HPET_MSI_SUPPORT0
  
 +/* Will fix: intcap is set by board, and should be 0 if nobody sets. */
 +#define HPET_TN_INT_CAP_DEFAULT 0x4ULL
 +
  #define TYPE_HPET hpet
  #define HPET(obj) OBJECT_CHECK(HPETState, (obj), TYPE_HPET)
  
 @@ -73,6 +77,7 @@ typedef struct HPETState {
  uint8_t rtc_irq_level;
  qemu_irq pit_enabled;
  uint8_t num_timers;
 +uint32_t intcap;
  HPETTimer timer[HPET_MAX_TIMERS];
  
  /* Memory-mapped, software visible registers */
 @@ -663,8 +668,8 @@ static void hpet_reset(DeviceState *d)
  if (s-flags  (1  HPET_MSI_SUPPORT)) {
  timer-config |= HPET_TN_FSB_CAP;
  }
 -/* advertise availability of ioapic inti2 */
 -timer-config |=  0x0004ULL  32;
 +/* advertise availability of ioapic int */
 +timer-config |=  (uint64_t)s-intcap  32;
  timer-period = 0ULL;
  timer-wrap_flag = 0;
  }
 @@ -753,6 +758,7 @@ static void hpet_realize(DeviceState *dev, Error **errp)
  static Property hpet_device_properties[] = {
  DEFINE_PROP_UINT8(timers, HPETState, num_timers, HPET_MIN_TIMERS),
  DEFINE_PROP_BIT(msi, HPETState, flags, HPET_MSI_SUPPORT, false),
 +DEFINE_PROP_UINT32(intcap, HPETState, intcap, 
 HPET_TN_INT_CAP_DEFAULT),
  DEFINE_PROP_END_OF_LIST(),
  };
 
 Please add a macro for this name as you use it in other
 files later.

Are you sure?  This is not done for any other compat property.

Paolo



Re: [Qemu-devel] [PATCH v6 2/5] hpet: enable to entitle more irq pins for hpet

2013-10-10 Thread Michael S. Tsirkin
On Thu, Oct 10, 2013 at 11:33:07AM +0200, Paolo Bonzini wrote:
 Il 10/10/2013 11:16, Michael S. Tsirkin ha scritto:
  On Thu, Oct 10, 2013 at 03:56:16PM +0800, Liu Ping Fan wrote:
  On q35, IRQ2/8 can be reserved for hpet timer 0/1. And pin 16~23
  of ioapic can be dynamically assigned to hpet as guest chooses.
  So we introduce intcap property to do that. (currently, its value
  is IRQ2. Later, it should be set by board.)
 
  Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com
  ---
   hw/timer/hpet.c | 10 --
   1 file changed, 8 insertions(+), 2 deletions(-)
 
  diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
  index 8429eb3..5b11be4 100644
  --- a/hw/timer/hpet.c
  +++ b/hw/timer/hpet.c
  @@ -25,6 +25,7 @@
*/
   
   #include hw/hw.h
  +#include hw/boards.h
   #include hw/i386/pc.h
   #include ui/console.h
   #include qemu/timer.h
  @@ -42,6 +43,9 @@
   
   #define HPET_MSI_SUPPORT0
   
  +/* Will fix: intcap is set by board, and should be 0 if nobody sets. */
  +#define HPET_TN_INT_CAP_DEFAULT 0x4ULL
  +
   #define TYPE_HPET hpet
   #define HPET(obj) OBJECT_CHECK(HPETState, (obj), TYPE_HPET)
   
  @@ -73,6 +77,7 @@ typedef struct HPETState {
   uint8_t rtc_irq_level;
   qemu_irq pit_enabled;
   uint8_t num_timers;
  +uint32_t intcap;
   HPETTimer timer[HPET_MAX_TIMERS];
   
   /* Memory-mapped, software visible registers */
  @@ -663,8 +668,8 @@ static void hpet_reset(DeviceState *d)
   if (s-flags  (1  HPET_MSI_SUPPORT)) {
   timer-config |= HPET_TN_FSB_CAP;
   }
  -/* advertise availability of ioapic inti2 */
  -timer-config |=  0x0004ULL  32;
  +/* advertise availability of ioapic int */
  +timer-config |=  (uint64_t)s-intcap  32;
   timer-period = 0ULL;
   timer-wrap_flag = 0;
   }
  @@ -753,6 +758,7 @@ static void hpet_realize(DeviceState *dev, Error 
  **errp)
   static Property hpet_device_properties[] = {
   DEFINE_PROP_UINT8(timers, HPETState, num_timers, HPET_MIN_TIMERS),
   DEFINE_PROP_BIT(msi, HPETState, flags, HPET_MSI_SUPPORT, false),
  +DEFINE_PROP_UINT32(intcap, HPETState, intcap, 
  HPET_TN_INT_CAP_DEFAULT),
   DEFINE_PROP_END_OF_LIST(),
   };
  
  Please add a macro for this name as you use it in other
  files later.
 
 Are you sure?  This is not done for any other compat property.
 
 Paolo

It's done if we use the property from C.
See PCI_HOST_PROP_PCI_HOLE64_SIZE.

You want compiler to catch errors, that's
much better than a runtime failure.

-- 
MST



Re: [Qemu-devel] [PATCH v6 2/5] hpet: enable to entitle more irq pins for hpet

2013-10-10 Thread Paolo Bonzini
Il 10/10/2013 11:41, Michael S. Tsirkin ha scritto:
  Are you sure?  This is not done for any other compat property.
  
  Paolo
 It's done if we use the property from C.
 See PCI_HOST_PROP_PCI_HOLE64_SIZE.
 
 You want compiler to catch errors, that's
 much better than a runtime failure.

I agree, but I think there should be no need to use the property from C.

Paolo



Re: [Qemu-devel] [PATCH v6 2/5] hpet: enable to entitle more irq pins for hpet

2013-10-10 Thread Michael S. Tsirkin
On Thu, Oct 10, 2013 at 11:46:42AM +0200, Paolo Bonzini wrote:
 Il 10/10/2013 11:41, Michael S. Tsirkin ha scritto:
   Are you sure?  This is not done for any other compat property.
   
   Paolo
  It's done if we use the property from C.
  See PCI_HOST_PROP_PCI_HOLE64_SIZE.
  
  You want compiler to catch errors, that's
  much better than a runtime failure.
 
 I agree, but I think there should be no need to use the property from C.
 
 Paolo

Well this patchset does use it from C.
If it's done it needs a macro.



Re: [Qemu-devel] [PATCH v6 2/5] hpet: enable to entitle more irq pins for hpet

2013-10-10 Thread liu ping fan
On Thu, Oct 10, 2013 at 5:11 PM, Paolo Bonzini pbonz...@redhat.com wrote:
 Il 10/10/2013 09:56, Liu Ping Fan ha scritto:
 On q35, IRQ2/8 can be reserved for hpet timer 0/1. And pin 16~23
 of ioapic can be dynamically assigned to hpet as guest chooses.
 So we introduce intcap property to do that. (currently, its value
 is IRQ2. Later, it should be set by board.)

 Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com
 ---
  hw/timer/hpet.c | 10 --
  1 file changed, 8 insertions(+), 2 deletions(-)

 diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
 index 8429eb3..5b11be4 100644
 --- a/hw/timer/hpet.c
 +++ b/hw/timer/hpet.c
 @@ -25,6 +25,7 @@
   */

  #include hw/hw.h
 +#include hw/boards.h
  #include hw/i386/pc.h
  #include ui/console.h
  #include qemu/timer.h
 @@ -42,6 +43,9 @@

  #define HPET_MSI_SUPPORT0

 +/* Will fix: intcap is set by board, and should be 0 if nobody sets. */
 +#define HPET_TN_INT_CAP_DEFAULT 0x4ULL
 +
  #define TYPE_HPET hpet
  #define HPET(obj) OBJECT_CHECK(HPETState, (obj), TYPE_HPET)

 @@ -73,6 +77,7 @@ typedef struct HPETState {
  uint8_t rtc_irq_level;
  qemu_irq pit_enabled;
  uint8_t num_timers;
 +uint32_t intcap;
  HPETTimer timer[HPET_MAX_TIMERS];

  /* Memory-mapped, software visible registers */
 @@ -663,8 +668,8 @@ static void hpet_reset(DeviceState *d)
  if (s-flags  (1  HPET_MSI_SUPPORT)) {
  timer-config |= HPET_TN_FSB_CAP;
  }
 -/* advertise availability of ioapic inti2 */
 -timer-config |=  0x0004ULL  32;
 +/* advertise availability of ioapic int */
 +timer-config |=  (uint64_t)s-intcap  32;
  timer-period = 0ULL;
  timer-wrap_flag = 0;
  }
 @@ -753,6 +758,7 @@ static void hpet_realize(DeviceState *dev, Error **errp)
  static Property hpet_device_properties[] = {
  DEFINE_PROP_UINT8(timers, HPETState, num_timers, HPET_MIN_TIMERS),
  DEFINE_PROP_BIT(msi, HPETState, flags, HPET_MSI_SUPPORT, false),
 +DEFINE_PROP_UINT32(intcap, HPETState, intcap, 
 HPET_TN_INT_CAP_DEFAULT),
  DEFINE_PROP_END_OF_LIST(),
  };



 According to Michael's request, a zero intcap should be detected in
 hpet_realize and give an error.

Will fix.

Thanks and regards,
Ping Fan



Re: [Qemu-devel] [PATCH v6 2/5] hpet: enable to entitle more irq pins for hpet

2013-10-10 Thread liu ping fan
On Thu, Oct 10, 2013 at 7:41 PM, Michael S. Tsirkin m...@redhat.com wrote:
 On Thu, Oct 10, 2013 at 11:46:42AM +0200, Paolo Bonzini wrote:
 Il 10/10/2013 11:41, Michael S. Tsirkin ha scritto:
   Are you sure?  This is not done for any other compat property.
  
   Paolo
  It's done if we use the property from C.
  See PCI_HOST_PROP_PCI_HOLE64_SIZE.
 
  You want compiler to catch errors, that's
  much better than a runtime failure.

 I agree, but I think there should be no need to use the property from C.

 Paolo

 Well this patchset does use it from C.
 If it's done it needs a macro.

hpet.h is the ideal place to put the macro, so pc.c can see it. But
what about PC_COMPAT_1_7 in pc.h? I think it is not right to include
hpet.h. So can I do not use marco in pc.h?

Thanks and regards,
Ping Fan