On Sun, Sep 29, 2013 at 3:52 AM, Michael S. Tsirkin <m...@redhat.com> 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 <pingf...@linux.vnet.ibm.com>
>
>
> 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
>>
