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

["Michael S. Tsirkin" <mst@redhat.com>]

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 <pingfank@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_SUPPORT        0
> >>  
> >> +/* 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 |=  0x00000004ULL << 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