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  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  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-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  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 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  wrote:
> > Il 11/10/2013 04:59, liu ping fan ha scritto:
> >> On Thu, Oct 10, 2013 at 7:41 PM, Michael S. Tsirkin  
> >> 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-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  wrote:
>> Il 11/10/2013 04:59, liu ping fan ha scritto:
>>> On Thu, Oct 10, 2013 at 7:41 PM, Michael S. Tsirkin  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



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  wrote:
> Il 11/10/2013 04:59, liu ping fan ha scritto:
>> On Thu, Oct 10, 2013 at 7:41 PM, Michael S. Tsirkin  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 04:59, liu ping fan ha scritto:
> On Thu, Oct 10, 2013 at 7:41 PM, Michael S. Tsirkin  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-10 Thread liu ping fan
On Thu, Oct 10, 2013 at 5:11 PM, Paolo Bonzini  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 
>> ---
>>  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  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



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 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: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 
> >> ---
> >>  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: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 
>> ---
>>  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 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 
> ---
>  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 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 
> ---
>  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



[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 
---
 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