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

[Paolo Bonzini <pbonzini@redhat.com>]

Il 27/08/2013 11:01, liu ping fan ha scritto:
> On Tue, Aug 27, 2013 at 4:45 PM, Paolo Bonzini <pbonzini@redhat.com> 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?

Because the versioned machine types are supposed not to change the guest
ABI.  This sometime requires bug-compatibility; for example, this
register is part of the guest ABI.

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

Yes, the compat property mechanism can be used for this.

Paolo

> 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;
>>>      }
>>>
>>