On Wed, 2023-08-30 at 21:20 +0100, David Woodhouse wrote: > From: David Woodhouse <d...@amazon.co.uk> > > The interrupt from timer 0 in legacy mode is supposed to go to IRQ 0 on > the i8259 and IRQ 2 on the I/O APIC. The generic x86 GSI handling can't > cope with IRQ numbers differing between the two chips (despite it also > being the case for PCI INTx routing), so add a special case for the HPET. > > IRQ 2 isn't valid on the i8259; it's the cascade IRQ and would be > interpreted as spurious interrupt on the secondary PIC. So we can fix > up all attempts to deliver IRQ2, to actually deliver to IRQ0 on the PIC. > > Signed-off-by: David Woodhouse <d...@amazon.co.uk> > --- > It ain't ever so pretty, but it's prettier than the INTx routing hack > that I just documented and at least this one doesn't rely on guest > behaviour.
I haven't come up with a better way of doing it, and nobody seemed to
care. Shall I post an identical patch without the [RFC] to see if it
elicits more of a response?
>
> Do we have tests for HPET interrupt delivery that can be extended to
> cover this?
>
>
> hw/i386/x86.c | 19 +++++++++++++++----
> hw/timer/hpet.c | 5 ++++-
> 2 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index a88a126123..0d2c74f2d9 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -602,13 +602,24 @@ DeviceState *cpu_get_current_apic(void)
> void gsi_handler(void *opaque, int n, int level)
> {
> GSIState *s = opaque;
> -
> + int i8259_pin = n;
> trace_x86_gsi_interrupt(n, level);
> switch (n) {
> - case 0 ... ISA_NUM_IRQS - 1:
> - if (s->i8259_irq[n]) {
> + case 2:
> + /*
> + * Special case for HPET legacy mode, which is defined as routing
> HPET
> + * timer 0 to IRQ2 of the I/O APIC and IRQ0 of the i8259 PIC. Since
> + * IRQ2 on the i8259 is the cascade, it isn't otherwise valid so we
> + * handle it via this special case.
> + */
> + i8259_pin = 0;
> + /* fall through */
> + case 0:
> + case 1:
> + case 3 ... ISA_NUM_IRQS - 1:
> + if (s->i8259_irq[i8259_pin]) {
> /* Under KVM, Kernel will forward to both PIC and IOAPIC */
> - qemu_set_irq(s->i8259_irq[n], level);
> + qemu_set_irq(s->i8259_irq[i8259_pin], level);
> }
> /* fall through */
> case ISA_NUM_IRQS ... IOAPIC_NUM_PINS - 1:
> diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
> index 6998094233..9f740ffdee 100644
> --- a/hw/timer/hpet.c
> +++ b/hw/timer/hpet.c
> @@ -196,8 +196,11 @@ static void update_irq(struct HPETTimer *timer, int set)
> /* if LegacyReplacementRoute bit is set, HPET specification requires
> * timer0 be routed to IRQ0 in NON-APIC or IRQ2 in the I/O APIC,
> * timer1 be routed to IRQ8 in NON-APIC or IRQ8 in the I/O APIC.
> + *
> + * There is a special case in the x86 gsi_handler() which converts
> + * IRQ2 into IRQ0 for the i8259 PIC and makes this work correctly.
> */
> - route = (timer->tn == 0) ? 0 : RTC_ISA_IRQ;
> + route = (timer->tn == 0) ? 2 : RTC_ISA_IRQ;
> } else {
> route = timer_int_route(timer);
> }
smime.p7s
Description: S/MIME cryptographic signature
