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

[liu ping fan]

On Sun, Sep 29, 2013 at 3:56 AM, Michael S. Tsirkin <address@hidden> wrote:
> On Thu, Sep 12, 2013 at 11:25:15AM +0800, Liu Ping Fan wrote:
>> On PC, 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.
>> (Will enable them after introducing pc 1.6 compat)
>>
>> Signed-off-by: Liu Ping Fan <address@hidden>
>> ---
>>  hw/timer/hpet.c | 13 +++++++++++--
>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
>> index 8429eb3..46903b9 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,12 @@
>>
>>  #define HPET_MSI_SUPPORT        0
>>
>> +/* For bug compat, using only IRQ2. Soon it will be fixed as
>> + * 0xff0104ULL, i.e using IRQ16~23, IRQ8 and IRQ2
>
> So users are expected to stick a bitmask of legal
> pins here?
> I think that's a bit too much rope to give to users.
> Don't you think?
>
Sorry, not understand your meaning exactly.  But the scene will be:
guest kernel polls the ability bitmask, and pick up one pin which is
not occupied or can be shared with the level-trigger and low-active.
So is it rope?

Thanks and regards,
Pingfan
>> after
>> + * introducing pc-1.6 compat.
>> + */
>> +#define HPET_TN_INT_CAP_DEFAULT 0x4ULL
>> +
>>  #define TYPE_HPET "hpet"
>>  #define HPET(obj) OBJECT_CHECK(HPETState, (obj), TYPE_HPET)
>>
>> @@ -73,6 +80,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 +671,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 +761,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
>>