On 22.12.2012, at 03:15, Scott Wood wrote:
> The two checks with abort() guard against potential QEMU-internal
> problems, but the EOI check stops the guest from causing updates to queue
> position -1 and other havoc if it writes EOI with no interrupt in
> service.
>
> Signed-off-by: Scott Wood <scottw...@freescale.com>
Did you ever actually experience this? MAX_IRQ should match the memory region
size, so we shouldn't be able to receive any interrupt above it.
I might be inclined to accept an assert() there for internal sanity checking
though. The last hunk looks fine.
Alex
> ---
> hw/openpic.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/hw/openpic.c b/hw/openpic.c
> index 5accff5..a3fcefd 100644
> --- a/hw/openpic.c
> +++ b/hw/openpic.c
> @@ -289,6 +289,10 @@ static void IRQ_check(OpenPICState *opp, IRQ_queue_t *q)
> irq = base + offset;
> map &= ~(1UL << offset);
>
> + if (irq >= MAX_IRQ) {
> + abort();
> + }
> +
> DPRINTF("IRQ_check: irq %d set ivpr_pr=%d pr=%d\n",
> irq, IVPR_PRIORITY(opp->src[irq].ivpr), priority);
>
> @@ -428,6 +432,11 @@ static void openpic_set_irq(void *opaque, int n_IRQ, int
> level)
> OpenPICState *opp = opaque;
> IRQ_src_t *src;
>
> + if (n_IRQ >= MAX_IRQ) {
> + fprintf(stderr, "%s: IRQ %d out of range\n", __func__, n_IRQ);
> + abort();
> + }
> +
> src = &opp->src[n_IRQ];
> DPRINTF("openpic: set irq %d = %d ivpr=0x%08x\n",
> n_IRQ, level, src->ivpr);
> @@ -923,6 +932,12 @@ static void openpic_cpu_write_internal(void *opaque,
> hwaddr addr,
> case 0xB0: /* EOI */
> DPRINTF("EOI\n");
> s_IRQ = IRQ_get_next(opp, &dst->servicing);
> +
> + if (s_IRQ < 0) {
> + DPRINTF("%s: EOI with no interrupt in service\n", __func__);
> + break;
> + }
> +
> IRQ_resetbit(&dst->servicing, s_IRQ);
> /* Set up next servicing IRQ */
> s_IRQ = IRQ_get_next(opp, &dst->servicing);
> --
> 1.7.9.5
>
>
