Re: [Qemu-devel] [PATCH v2 3/3] hpet: entitle more irq pins for hpet

[liu ping fan]

On Tue, Aug 27, 2013 at 4:45 PM, Paolo Bonzini <address@hidden> wrote:
> Il 27/08/2013 10:10, Liu Ping Fan ha scritto:
>> On q35 machine, 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.
>
> First of all, the backwards-compatible q35 machines should also use the
> old value.
>
Sorry but could you tell me why even on q35, we can only use IRQ2 for hpet?

> Second, the HPET should _not_ know the machine types.  You need to add a
> qdev property as I suggested in my reply to v1.  The default value
> should be 0xFF0104.  Then you can add the compatibility value in
> hw/i386/pc_piix.c and hw/i386/pc_q35.c.
>
So export the hpet's property at board-level?

Regards,
Pingfan
>> ---
>>  hw/timer/hpet.c | 14 ++++++++++++--
>>  1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
>> index 1139448..92cd4fa 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,11 @@
>>
>>  #define HPET_MSI_SUPPORT        0
>>
>> +/* only IRQ2 allowed for pc-1.6 and former */
>> +#define HPET_TN_INT_CAP_PC (0x4ULL << 32)
>> +/* Hpet can use non-legacy IRQ16~23, and an IRQ2 ,IRQ8 */
>> +#define HPET_TN_INT_CAP_Q35 (0xff0104ULL << 32)
>> +
>>  #define TYPE_HPET "hpet"
>>  #define HPET(obj) OBJECT_CHECK(HPETState, (obj), TYPE_HPET)
>>
>> @@ -663,8 +669,12 @@ 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 */
>> +        if (qemu_check_machine("pc-q35")) {
>> +            timer->config |=  HPET_TN_INT_CAP_Q35;
>> +        } else {
>> +            timer->config |=  HPET_TN_INT_CAP_PC;
>> +        }
>>          timer->period = 0ULL;
>>          timer->wrap_flag = 0;
>>      }
>>
>