Re: [0.14?][PATCH 1/4] ioapic: Implement EOI handling for level-triggered IRQs

[Anthony Liguori <anthony@codemonkey.ws>]

On 02/03/2011 08:55 AM, Jan Kiszka wrote:
> Add the missing EOI broadcast from local APIC to the IOAPICs on
> completion of level-triggered IRQs. This ensures that a still asserted
> IRQ source properly re-triggers an APIC IRQ.
>
> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com>
> ---
>   hw/apic.c   |    9 ++++++---
>   hw/ioapic.c |   43 +++++++++++++++++++++++++++++++++++++++++--
>   hw/ioapic.h |   20 ++++++++++++++++++++
>   3 files changed, 67 insertions(+), 5 deletions(-)
>   create mode 100644 hw/ioapic.h
>
> diff --git a/hw/apic.c b/hw/apic.c
> index ff581f0..05a115f 100644
> --- a/hw/apic.c
> +++ b/hw/apic.c
> @@ -18,6 +18,7 @@
>    */
>   #include "hw.h"
>   #include "apic.h"
> +#include "ioapic.h"
>   #include "qemu-timer.h"
>   #include "host-utils.h"
>   #include "sysbus.h"
> @@ -57,7 +58,8 @@
>
>   #define ESR_ILLEGAL_ADDRESS (1<<  7)
>
> -#define APIC_SV_ENABLE (1<<  8)
> +#define APIC_SV_DIRECTED_IO             (1<<12)
> +#define APIC_SV_ENABLE                  (1<<8)
>
>   #define MAX_APICS 255
>   #define MAX_APIC_WORDS 8
> @@ -420,8 +422,9 @@ static void apic_eoi(APICState *s)
>       if (isrv<  0)
>           return;
>       reset_bit(s->isr, isrv);
> -    /* XXX: send the EOI packet to the APIC bus to allow the I/O APIC to
> -            set the remote IRR bit for level triggered interrupts. */
> +    if (!(s->spurious_vec&  APIC_SV_DIRECTED_IO)&&  get_bit(s->tmr, isrv)) {
> +        ioapic_eio_broadcast(isrv);
> +    }
>       apic_update_irq(s);
>   }
>
> diff --git a/hw/ioapic.c b/hw/ioapic.c
> index 2109568..443c579 100644
> --- a/hw/ioapic.c
> +++ b/hw/ioapic.c
> @@ -23,6 +23,7 @@
>   #include "hw.h"
>   #include "pc.h"
>   #include "apic.h"
> +#include "ioapic.h"
>   #include "qemu-timer.h"
>   #include "host-utils.h"
>   #include "sysbus.h"
> @@ -36,7 +37,10 @@
>   #define DPRINTF(fmt, ...)
>   #endif
>
> -#define IOAPIC_LVT_MASKED 		(1<<16)
> +#define MAX_IOAPICS                     1
> +
> +#define IOAPIC_LVT_MASKED               (1<<  16)
> +#define IOAPIC_LVT_REMOTE_IRR           (1<<  14)
>
>   #define IOAPIC_TRIGGER_EDGE		0
>   #define IOAPIC_TRIGGER_LEVEL		1
> @@ -61,6 +65,8 @@ struct IOAPICState {
>       uint64_t ioredtbl[IOAPIC_NUM_PINS];
>   };
>
> +static IOAPICState *ioapics[MAX_IOAPICS];
> +
>   static void ioapic_service(IOAPICState *s)
>   {
>       uint8_t i;
> @@ -83,8 +89,11 @@ static void ioapic_service(IOAPICState *s)
>                   dest_mode = (entry>>  11)&  1;
>                   delivery_mode = (entry>>  8)&  7;
>                   polarity = (entry>>  13)&  1;
> -                if (trig_mode == IOAPIC_TRIGGER_EDGE)
> +                if (trig_mode == IOAPIC_TRIGGER_EDGE) {
>                       s->irr&= ~mask;
> +                } else {
> +                    s->ioredtbl[i] |= IOAPIC_LVT_REMOTE_IRR;
> +                }
>                   if (delivery_mode == IOAPIC_DM_EXTINT)
>                       vector = pic_read_irq(isa_pic);
>                   else
> @@ -131,6 +140,29 @@ static void ioapic_set_irq(void *opaque, int vector, int level)
>       }
>   }
>
> +void ioapic_eio_broadcast(int vector)
>    

You mean iopaic_eoi_broadcast(), no?

Regards,

Anthony Liguori

> +{
> +    IOAPICState *s;
> +    uint64_t entry;
> +    int i, n;
> +
> +    for (i = 0; i<  MAX_IOAPICS; i++) {
> +        s = ioapics[i];
> +        if (!s) {
> +            continue;
> +        }
> +        for (n = 0; n<  IOAPIC_NUM_PINS; n++) {
> +            entry = s->ioredtbl[n];
> +            if ((entry&  IOAPIC_LVT_REMOTE_IRR)&&  (entry&  0xff) == vector) {
> +                s->ioredtbl[n] = entry&  ~IOAPIC_LVT_REMOTE_IRR;
> +                if (!(entry&  IOAPIC_LVT_MASKED)&&  (s->irr&  (1<<  n))) {
> +                    ioapic_service(s);
> +                }
> +            }
> +        }
> +    }
> +}
> +
>   static uint32_t ioapic_mem_readl(void *opaque, target_phys_addr_t addr)
>   {
>       IOAPICState *s = opaque;
> @@ -240,6 +272,11 @@ static int ioapic_init1(SysBusDevice *dev)
>   {
>       IOAPICState *s = FROM_SYSBUS(IOAPICState, dev);
>       int io_memory;
> +    static int ioapic_no;
> +
> +    if (ioapic_no>= MAX_IOAPICS) {
> +        return -1;
> +    }
>
>       io_memory = cpu_register_io_memory(ioapic_mem_read,
>                                          ioapic_mem_write, s,
> @@ -248,6 +285,8 @@ static int ioapic_init1(SysBusDevice *dev)
>
>       qdev_init_gpio_in(&dev->qdev, ioapic_set_irq, IOAPIC_NUM_PINS);
>
> +    ioapics[ioapic_no++] = s;
> +
>       return 0;
>   }
>
> diff --git a/hw/ioapic.h b/hw/ioapic.h
> new file mode 100644
> index 0000000..c441567
> --- /dev/null
> +++ b/hw/ioapic.h
> @@ -0,0 +1,20 @@
> +/*
> + *  ioapic.c IOAPIC emulation logic
> + *
> + *  Copyright (c) 2011 Jan Kiszka, Siemens AG
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see<http://www.gnu.org/licenses/>.
> + */
> +
> +void ioapic_eio_broadcast(int vector);
>