qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Bill Paul <wpaul@windriver.com>
To: qemu-devel@nongnu.org
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Eduardo Habkost <ehabkost@redhat.com>,
	Richard Henderson <rth@twiddle.net>
Subject: [Qemu-devel] Question about hw/timer/hpet.c, hw/intc/ioapic.c and polarity
Date: Mon, 4 Apr 2016 14:42:46 -0700	[thread overview]
Message-ID: <201604041442.46614.wpaul@windriver.com> (raw)

I'm testing some of the HPET handling code in VxWorks using QEMU 2.4.1 and 
I've encountered something which looks a little odd that I'm hoping someone 
can clarify for me. First, some background:

The HPET timer supports three kinds of interrupt delivery:

Legacy: HPET can use the same IRQs as the old 8254 timer (IRQ2, IRQ8)
I/O APIC: HPET can use any of the first 32 I/O APIC IRQs in the system
FSB: HPET uses "front-side bus" mode, i.e interrupts are routed right to the 
local APIC (I/O APIC is bypassed)

By default, VxWorks uses front-side bus mode, and that seems to work fine. 
However I wanted to try I/O APIC mode too, and that seems to behave in a funny 
way. In particular, the following code in hw/timer/hpet.c puzzles me:

    if (!set || !timer_enabled(timer) || !hpet_enabled(timer->state)) {
        s->isr &= ~mask;
        if (!timer_fsb_route(timer)) {
            /* 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)) {
        address_space_stl_le(&address_space_memory, timer->fsb >> 32,
                             timer->fsb & 0xffffffff, MEMTXATTRS_UNSPECIFIED,
                             NULL);
    } else if (timer->config & HPET_TN_TYPE_LEVEL) {
        s->isr |= mask;
        /* 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]);
    }

Note the part that inverts the interrupt handling logic for ICH PIRQ pins. I 
don't understand how this is supposed to work. If I use level triggered IRQ2 
or IRQ8 in VxWorks, then things work as expected. But if I use IRQ21, the HPET 
timer interrupt service routine is immediately called, even though the timer 
hasn't expired yet. The ISR reads 0 from the HPET status register, indicating 
that no timers have events pending, so it just returns. The first 
qemu_irq_raise() call is triggered because hpet_enabled() returns true, but 
timer_enabled() returns false.

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().

Right now all I know is that VxWorks' usage of the HPET seems to work on real 
hardware, but not on QEMU. I suspect that the changes made to hpet.c here may 
have only "fixed" the problem with Linux by introducing some non-standard 
behavior that happens to pacify Linux's particular usage model.

Can someone comment on whether or not this inversion logic is really still 
necessary in Linux? Is there maybe a better way to handle this?

-Bill

-- 
=============================================================================
-Bill Paul            (510) 749-2329 | Senior Member of Technical Staff,
                 wpaul@windriver.com | Master of Unix-Fu - Wind River Systems
=============================================================================
   "I put a dollar in a change machine. Nothing changed." - George Carlin
=============================================================================

             reply	other threads:[~2016-04-04 23:01 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-04 21:42 Bill Paul [this message]
2016-04-05 13:20 ` [Qemu-devel] Question about hw/timer/hpet.c, hw/intc/ioapic.c and polarity Paolo Bonzini
2016-04-05 18:30   ` Bill Paul
2016-04-05 23:04     ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201604041442.46614.wpaul@windriver.com \
    --to=wpaul@windriver.com \
    --cc=ehabkost@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).