Re: [Qemu-devel] Question about hw/timer/hpet.c, hw/intc/ioapic.c and polarity

[Paolo Bonzini]

On 05/04/2016 20:30, Bill Paul wrote:
>>> Researching the code history, I see that the inversion logic was added in
>>> 2013 in order to fix a problem with HPET usage in Linux. However
>>> something about the way this was done looks wrong to me. In the case
>>> where we actually want to signal an interrupt because the timer has
>>> expired, and the IRQ is larger than 15, the code calls qemu_irq_lower()
>>> instead of qemu_irq_raise(). Eventually this results in ioapic_set_irq()
>>> being called with level = 0. The problem is, ioapic_set_irq() will only
>>> call ioapic_service() to notify the quest of an interrupt if level == 1.
>>>
>>> Given this, I can't understand how this is supposed to work. The hpet.c
>>> code seems to treat qemu_irq_raise() and qemu_irq_lower() as though they
>>> can trigger active high or active low interrupts, but the ioapic.c code
>>> doesn't support any polarity settings. The only way to actually trigger
>>> an interrupt to the guest is to "raise" (assert) the interrupt by
>>> calling qemu_irq_raise().
>>
>> I think that commit 0d63b2d ("hpet: inverse polarity when pin above
>> ISA_NUM_IRQS", 2013-12-11) can be reverted.  The code was probably
>> tested on KVM only, but KVM now is *also* ignoring the IOAPIC polarity
>> (commit 100943c54e09, "kvm: x86: ignore ioapic polarity", 2014-03-13).
> 
> I wonder how it worked on KVM.

See commit 100943c54e09 (that's a kernel commit).  KVM actually did
invert the sense of the IRQ line, though that was very buggy too due to
doing so very naively (and that's why it was ripped out).

Thanks,

Paolo