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

[Paolo Bonzini <pbonzini@redhat.com>]

Il 30/09/2013 11:06, Michael S. Tsirkin ha scritto:
> On Mon, Sep 30, 2013 at 04:02:29PM +0800, liu ping fan wrote:
>> On Sun, Sep 29, 2013 at 12:15 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>>> On Sun, Sep 29, 2013 at 11:49:41AM +0800, liu ping fan wrote:
>>>> On Sun, Sep 29, 2013 at 3:56 AM, Michael S. Tsirkin <mst@redhat.com> 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 <pingfank@linux.vnet.ibm.com>
>>>>>> ---
>>>>>>  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?
>>>
>>> I merely say that it's better to make this a bool or bit property.
>>> UINT32 is too much flexibility imho.
>>>
>> The interrupt capability is export to guest by register
>> Tn_INT_ROUTE_CAP[63:32]. So it is useless to make them as a bit
>> property. Do you think so?
>>
>> Regards
>> Pingfan
> 
> I think we merely need to support two modes:
> - qemu 1.6 and older compatible
> - compatible to actual hardware
> 
> Why would we let users configure an arbitrary
> configuration which isn't compatible to either
> old qemu or real hardware?

The actual setting depends on the chipset.  For example, the "real"
PIIX4 is the same as the QEMU PIIX4, only Q35 uses the new value.

If in the future we had a chipset with more than 24 GSIs, you would have
a third possibility.

Paolo