Re: [Qemu-devel] [PATCH v5 1/5] hpet: inverse polarity when pin above ISA_NUM_IRQS

[liu ping fan]

On Sun, Sep 29, 2013 at 3:52 AM, Michael S. Tsirkin <address@hidden> wrote:
> On Thu, Sep 12, 2013 at 11:25:14AM +0800, Liu Ping Fan wrote:
>> According to hpet spec, hpet irq is high active. But according to
>> ICH spec, there is inversion before the input of ioapic. So the OS
>> will expect low active on this IRQ line.
>
>
>>(And this is observed on
>> bare metal).
>
> How does one test this on bare metal?
>
If changing the func hpet_timer_set_irq() in linux's hpet driver,
from acpi_register_gsi(NULL, irq, ACPI_LEVEL_SENSITIVE,
ACPI_ACTIVE_LOW);  to ACPI_ACTIVE_HIGH,  then run hpet_example.c on
bare metal, the modified kernel will complain about spurious irq and
disable the irq line.
>>
>> We fold the emulation of this inversion inside the hpet logic.
>>
>> Signed-off-by: Liu Ping Fan <address@hidden>
>
>
> Doesn't this affect cross-version migration?
> E.g. imagine that you migrate between systems
> with/without this fix.
>
No. the changing only affect "route >= ISA_NUM_IRQS",  For linux
guest, it use IRQ2/8 for hpet0/hpet1 which is reserved by kernel. It
work without this fix (I think windows is the same). But the
hpet_example.c(in linux) can not work without this fix. So no such
run-time instance before this bug fix.

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 fcd22ae..8429eb3 100644
>> --- a/hw/timer/hpet.c
>> +++ b/hw/timer/hpet.c
>> @@ -198,13 +198,23 @@ static void update_irq(struct HPETTimer *timer, int 
>> set)
>>      if (!set || !timer_enabled(timer) || !hpet_enabled(timer->state)) {
>>          s->isr &= ~mask;
>>          if (!timer_fsb_route(timer)) {
>> -            qemu_irq_lower(s->irqs[route]);
>> +            /* fold the ICH PIRQ# pin's internal inversion logic into hpet 
>> */
>> +            if (route >= ISA_NUM_IRQS) {
>> +                qemu_irq_raise(s->irqs[route]);
>> +            } else {
>> +                qemu_irq_lower(s->irqs[route]);
>> +            }
>>          }
>>      } else if (timer_fsb_route(timer)) {
>>          stl_le_phys(timer->fsb >> 32, timer->fsb & 0xffffffff);
>>      } else if (timer->config & HPET_TN_TYPE_LEVEL) {
>>          s->isr |= mask;
>> -        qemu_irq_raise(s->irqs[route]);
>> +        /* fold the ICH PIRQ# pin's internal inversion logic into hpet */
>> +        if (route >= ISA_NUM_IRQS) {
>> +            qemu_irq_lower(s->irqs[route]);
>> +        } else {
>> +            qemu_irq_raise(s->irqs[route]);
>> +        }
>>      } else {
>>          s->isr &= ~mask;
>>          qemu_irq_pulse(s->irqs[route]);
>> --
>> 1.8.1.4
>>