diff mbox

[02/19] mpic: Unify numbering scheme

Message ID 1354974282-1915-3-git-send-email-agraf@suse.de
State New
Headers show

Commit Message

Alexander Graf Dec. 8, 2012, 1:44 p.m. UTC 
MPIC interrupt numbers in Linux (device tree) and in QEMU are different,
because QEMU takes the sparseness of the IRQ number space into account.

Remove that cleverness and instead assume a flat number space. This makes
the code easier to understand, because we are actually aligned with Linux
on the view of our worlds.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/openpic.c  |  287 ++++++++------------------------------------------------
 hw/ppc/e500.c |    4 +-
 2 files changed, 43 insertions(+), 248 deletions(-)

Comments

Scott Wood Dec. 10, 2012, 11:34 p.m. UTC | #1 
On 12/08/2012 07:44:25 AM, Alexander Graf wrote:
> MPIC interrupt numbers in Linux (device tree) and in QEMU are  
> different,
> because QEMU takes the sparseness of the IRQ number space into  
> account.
> 
> Remove that cleverness and instead assume a flat number space. This  
> makes
> the code easier to understand, because we are actually aligned with  
> Linux
> on the view of our worlds.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  hw/openpic.c  |  287  
> ++++++++------------------------------------------------
>  hw/ppc/e500.c |    4 +-
>  2 files changed, 43 insertions(+), 248 deletions(-)

Thanks!

This also helps accommodate Linux's (and probably other OSes') MPIC
driver, which on boot will initialize all vectors, whether the actual
interrupt is valid or not.  Currently QEMU rejects those accesses as
illegal MMIO; we only survive it now because we don't yet have support  
for
injecting a machine check on e500.

> diff --git a/hw/openpic.c b/hw/openpic.c
> index b30c853..0d858cc 100644
> --- a/hw/openpic.c
> +++ b/hw/openpic.c
> @@ -47,7 +47,7 @@
>  #endif
> 
>  #define MAX_CPU    15
> -#define MAX_IRQ   128
> +#define MAX_IRQ   256
>  #define MAX_TMR     4
>  #define VECTOR_BITS 8
>  #define MAX_IPI     4
> @@ -82,32 +82,25 @@ enum {
>  #define MPIC_MAX_CPU      1
>  #define MPIC_MAX_EXT     12
>  #define MPIC_MAX_INT     64
> -#define MPIC_MAX_MSG      4
> -#define MPIC_MAX_MSI      8
> -#define MPIC_MAX_TMR      MAX_TMR
> -#define MPIC_MAX_IPI      MAX_IPI
> -#define MPIC_MAX_IRQ     (MPIC_MAX_EXT + MPIC_MAX_INT + MPIC_MAX_TMR  
> + MPIC_MAX_MSG + MPIC_MAX_MSI + (MPIC_MAX_IPI * MPIC_MAX_CPU))
> +#define MPIC_MAX_IRQ     MAX_IRQ
> 
>  /* Interrupt definitions */
> -#define MPIC_EXT_IRQ      0
> -#define MPIC_INT_IRQ      (MPIC_EXT_IRQ + MPIC_MAX_EXT)
> -#define MPIC_TMR_IRQ      (MPIC_INT_IRQ + MPIC_MAX_INT)
> -#define MPIC_MSG_IRQ      (MPIC_TMR_IRQ + MPIC_MAX_TMR)
> -#define MPIC_MSI_IRQ      (MPIC_MSG_IRQ + MPIC_MAX_MSG)
> -#define MPIC_IPI_IRQ      (MPIC_MSI_IRQ + MPIC_MAX_MSI)
> +/* IRQs, accessible through the IRQ region */
> +#define MPIC_EXT_IRQ      0x00
> +#define MPIC_INT_IRQ      0x10
> +#define MPIC_MSG_IRQ      0xb0
> +#define MPIC_MSI_IRQ      0xe0

Where are MPIC_EXT/INT/MSG/MSI_IRQ used now?  Note that these are
specific to Freescale's MPIC.

> +/* These are available through separate regions, but
> +   for simplicity's sake mapped into the same number space */
> +#define MPIC_TMR_IRQ      0xf3
> +#define MPIC_IPI_IRQ      0xfb

Please don't do this, or at least choose different numbers.  0xf3 is a
valid MSI on p4080 (not to mention T4240 which goes beyond 256).

Again, what uses these defines?  Is it something later in the series?

-Scott
Scott Wood Dec. 10, 2012, 11:40 p.m. UTC | #2 
On 12/10/2012 05:34:19 PM, Scott Wood wrote:
> On 12/08/2012 07:44:25 AM, Alexander Graf wrote:
>>  /* Interrupt definitions */
>> -#define MPIC_EXT_IRQ      0
>> -#define MPIC_INT_IRQ      (MPIC_EXT_IRQ + MPIC_MAX_EXT)
>> -#define MPIC_TMR_IRQ      (MPIC_INT_IRQ + MPIC_MAX_INT)
>> -#define MPIC_MSG_IRQ      (MPIC_TMR_IRQ + MPIC_MAX_TMR)
>> -#define MPIC_MSI_IRQ      (MPIC_MSG_IRQ + MPIC_MAX_MSG)
>> -#define MPIC_IPI_IRQ      (MPIC_MSI_IRQ + MPIC_MAX_MSI)
>> +/* IRQs, accessible through the IRQ region */
>> +#define MPIC_EXT_IRQ      0x00
>> +#define MPIC_INT_IRQ      0x10
>> +#define MPIC_MSG_IRQ      0xb0
>> +#define MPIC_MSI_IRQ      0xe0
> 
> Where are MPIC_EXT/INT/MSG/MSI_IRQ used now?  Note that these are
> specific to Freescale's MPIC.
> 
>> +/* These are available through separate regions, but
>> +   for simplicity's sake mapped into the same number space */
>> +#define MPIC_TMR_IRQ      0xf3
>> +#define MPIC_IPI_IRQ      0xfb
> 
> Please don't do this, or at least choose different numbers.  0xf3 is a
> valid MSI on p4080 (not to mention T4240 which goes beyond 256).
> 
> Again, what uses these defines?  Is it something later in the series?

OK, the TMR/IPI are used by existing code that this patch doesn't  
remove, but I think that's not the case with EXT/INT/MSG/MSI.

-Scott
Alexander Graf Dec. 11, 2012, 8:14 a.m. UTC | #3 
On 11.12.2012, at 00:34, Scott Wood <scottwood@freescale.com> wrote:

> On 12/08/2012 07:44:25 AM, Alexander Graf wrote:
>> MPIC interrupt numbers in Linux (device tree) and in QEMU are different,
>> because QEMU takes the sparseness of the IRQ number space into account.
>> Remove that cleverness and instead assume a flat number space. This makes
>> the code easier to understand, because we are actually aligned with Linux
>> on the view of our worlds.
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>> hw/openpic.c  |  287 ++++++++------------------------------------------------
>> hw/ppc/e500.c |    4 +-
>> 2 files changed, 43 insertions(+), 248 deletions(-)
> 
> Thanks!
> 
> This also helps accommodate Linux's (and probably other OSes') MPIC
> driver, which on boot will initialize all vectors, whether the actual
> interrupt is valid or not.  Currently QEMU rejects those accesses as
> illegal MMIO; we only survive it now because we don't yet have support for
> injecting a machine check on e500.
> 
>> diff --git a/hw/openpic.c b/hw/openpic.c
>> index b30c853..0d858cc 100644
>> --- a/hw/openpic.c
>> +++ b/hw/openpic.c
>> @@ -47,7 +47,7 @@
>> #endif
>> #define MAX_CPU    15
>> -#define MAX_IRQ   128
>> +#define MAX_IRQ   256
>> #define MAX_TMR     4
>> #define VECTOR_BITS 8
>> #define MAX_IPI     4
>> @@ -82,32 +82,25 @@ enum {
>> #define MPIC_MAX_CPU      1
>> #define MPIC_MAX_EXT     12
>> #define MPIC_MAX_INT     64
>> -#define MPIC_MAX_MSG      4
>> -#define MPIC_MAX_MSI      8
>> -#define MPIC_MAX_TMR      MAX_TMR
>> -#define MPIC_MAX_IPI      MAX_IPI
>> -#define MPIC_MAX_IRQ     (MPIC_MAX_EXT + MPIC_MAX_INT + MPIC_MAX_TMR + MPIC_MAX_MSG + MPIC_MAX_MSI + (MPIC_MAX_IPI * MPIC_MAX_CPU))
>> +#define MPIC_MAX_IRQ     MAX_IRQ
>> /* Interrupt definitions */
>> -#define MPIC_EXT_IRQ      0
>> -#define MPIC_INT_IRQ      (MPIC_EXT_IRQ + MPIC_MAX_EXT)
>> -#define MPIC_TMR_IRQ      (MPIC_INT_IRQ + MPIC_MAX_INT)
>> -#define MPIC_MSG_IRQ      (MPIC_TMR_IRQ + MPIC_MAX_TMR)
>> -#define MPIC_MSI_IRQ      (MPIC_MSG_IRQ + MPIC_MAX_MSG)
>> -#define MPIC_IPI_IRQ      (MPIC_MSI_IRQ + MPIC_MAX_MSI)
>> +/* IRQs, accessible through the IRQ region */
>> +#define MPIC_EXT_IRQ      0x00
>> +#define MPIC_INT_IRQ      0x10
>> +#define MPIC_MSG_IRQ      0xb0
>> +#define MPIC_MSI_IRQ      0xe0
> 
> Where are MPIC_EXT/INT/MSG/MSI_IRQ used now?  Note that these are
> specific to Freescale's MPIC.
> 
>> +/* These are available through separate regions, but
>> +   for simplicity's sake mapped into the same number space */
>> +#define MPIC_TMR_IRQ      0xf3
>> +#define MPIC_IPI_IRQ      0xfb
> 
> Please don't do this, or at least choose different numbers.  0xf3 is a
> valid MSI on p4080 (not to mention T4240 which goes beyond 256).

Ah, that's where I was wondering myself too. I copied the above logic from Linux, which maps tmr and ipi to max_irq-x. But I agree that it sounds off.

I'll rework this one again to distinguish between src and internal interrupt lines.


Alex
Scott Wood Dec. 11, 2012, 5:39 p.m. UTC | #4 
On 12/11/2012 02:14:42 AM, Alexander Graf wrote:
> 
> 
> On 11.12.2012, at 00:34, Scott Wood <scottwood@freescale.com> wrote:
> 
> > On 12/08/2012 07:44:25 AM, Alexander Graf wrote:
> >> +/* These are available through separate regions, but
> >> +   for simplicity's sake mapped into the same number space */
> >> +#define MPIC_TMR_IRQ      0xf3
> >> +#define MPIC_IPI_IRQ      0xfb
> >
> > Please don't do this, or at least choose different numbers.  0xf3  
> is a
> > valid MSI on p4080 (not to mention T4240 which goes beyond 256).
> 
> Ah, that's where I was wondering myself too. I copied the above logic  
> from Linux, which maps tmr and ipi to max_irq-x. But I agree that it  
> sounds off.

Yeah, Linux was buggy and has since been fixed (specifically, we turned  
on MPIC_LARGE_VECTORS so that max_irq is large enough to not conflict).

-Scott
diff mbox

Patch

diff --git a/hw/openpic.c b/hw/openpic.c
index b30c853..0d858cc 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -47,7 +47,7 @@ 
 #endif
 
 #define MAX_CPU    15
-#define MAX_IRQ   128
+#define MAX_IRQ   256
 #define MAX_TMR     4
 #define VECTOR_BITS 8
 #define MAX_IPI     4
@@ -82,32 +82,25 @@  enum {
 #define MPIC_MAX_CPU      1
 #define MPIC_MAX_EXT     12
 #define MPIC_MAX_INT     64
-#define MPIC_MAX_MSG      4
-#define MPIC_MAX_MSI      8
-#define MPIC_MAX_TMR      MAX_TMR
-#define MPIC_MAX_IPI      MAX_IPI
-#define MPIC_MAX_IRQ     (MPIC_MAX_EXT + MPIC_MAX_INT + MPIC_MAX_TMR + MPIC_MAX_MSG + MPIC_MAX_MSI + (MPIC_MAX_IPI * MPIC_MAX_CPU))
+#define MPIC_MAX_IRQ     MAX_IRQ
 
 /* Interrupt definitions */
-#define MPIC_EXT_IRQ      0
-#define MPIC_INT_IRQ      (MPIC_EXT_IRQ + MPIC_MAX_EXT)
-#define MPIC_TMR_IRQ      (MPIC_INT_IRQ + MPIC_MAX_INT)
-#define MPIC_MSG_IRQ      (MPIC_TMR_IRQ + MPIC_MAX_TMR)
-#define MPIC_MSI_IRQ      (MPIC_MSG_IRQ + MPIC_MAX_MSG)
-#define MPIC_IPI_IRQ      (MPIC_MSI_IRQ + MPIC_MAX_MSI)
+/* IRQs, accessible through the IRQ region */
+#define MPIC_EXT_IRQ      0x00
+#define MPIC_INT_IRQ      0x10
+#define MPIC_MSG_IRQ      0xb0
+#define MPIC_MSI_IRQ      0xe0
+/* These are available through separate regions, but
+   for simplicity's sake mapped into the same number space */
+#define MPIC_TMR_IRQ      0xf3
+#define MPIC_IPI_IRQ      0xfb
 
 #define MPIC_GLB_REG_START        0x0
 #define MPIC_GLB_REG_SIZE         0x10F0
 #define MPIC_TMR_REG_START        0x10F0
 #define MPIC_TMR_REG_SIZE         0x220
-#define MPIC_EXT_REG_START        0x10000
-#define MPIC_EXT_REG_SIZE         0x180
-#define MPIC_INT_REG_START        0x10200
-#define MPIC_INT_REG_SIZE         0x800
-#define MPIC_MSG_REG_START        0x11600
-#define MPIC_MSG_REG_SIZE         0x100
-#define MPIC_MSI_REG_START        0x11C00
-#define MPIC_MSI_REG_SIZE         0x100
+#define MPIC_IRQ_REG_START        0x10000
+#define MPIC_IRQ_REG_SIZE         (MAX_IRQ * 0x20)
 #define MPIC_CPU_REG_START        0x20000
 #define MPIC_CPU_REG_SIZE         0x100 + ((MAX_CPU - 1) * 0x1000)
 
@@ -1205,193 +1198,44 @@  static uint32_t mpic_timer_read (void *opaque, hwaddr addr)
     return retval;
 }
 
-static void mpic_src_ext_write (void *opaque, hwaddr addr,
-                                uint32_t val)
+static void mpic_src_irq_write(void *opaque, hwaddr addr,
+                               uint64_t val, unsigned len)
 {
     openpic_t *mpp = opaque;
-    int idx = MPIC_EXT_IRQ;
+    int idx = addr / 0x20;
 
-    DPRINTF("%s: addr " TARGET_FMT_plx " <= %08x\n", __func__, addr, val);
-    if (addr & 0xF)
-        return;
-
-    if (addr < MPIC_EXT_REG_SIZE) {
-        idx += (addr & 0xFFF0) >> 5;
-        if (addr & 0x10) {
-            /* EXDE / IFEDE / IEEDE */
-            write_IRQreg_ide(mpp, idx, val);
-        } else {
-            /* EXVP / IFEVP / IEEVP */
-            write_IRQreg_ipvp(mpp, idx, val);
-        }
-    }
-}
-
-static uint32_t mpic_src_ext_read (void *opaque, hwaddr addr)
-{
-    openpic_t *mpp = opaque;
-    uint32_t retval;
-    int idx = MPIC_EXT_IRQ;
-
-    DPRINTF("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
-    retval = 0xFFFFFFFF;
-    if (addr & 0xF)
-        return retval;
-
-    if (addr < MPIC_EXT_REG_SIZE) {
-        idx += (addr & 0xFFF0) >> 5;
-        if (addr & 0x10) {
-            /* EXDE / IFEDE / IEEDE */
-            retval = read_IRQreg_ide(mpp, idx);
-        } else {
-            /* EXVP / IFEVP / IEEVP */
-            retval = read_IRQreg_ipvp(mpp, idx);
-        }
-        DPRINTF("%s: => %08x\n", __func__, retval);
-    }
-
-    return retval;
-}
-
-static void mpic_src_int_write (void *opaque, hwaddr addr,
-                                uint32_t val)
-{
-    openpic_t *mpp = opaque;
-    int idx = MPIC_INT_IRQ;
-
-    DPRINTF("%s: addr " TARGET_FMT_plx " <= %08x\n", __func__, addr, val);
-    if (addr & 0xF)
-        return;
-
-    if (addr < MPIC_INT_REG_SIZE) {
-        idx += (addr & 0xFFF0) >> 5;
-        if (addr & 0x10) {
-            /* EXDE / IFEDE / IEEDE */
-            write_IRQreg_ide(mpp, idx, val);
-        } else {
-            /* EXVP / IFEVP / IEEVP */
-            write_IRQreg_ipvp(mpp, idx, val);
-        }
-    }
-}
-
-static uint32_t mpic_src_int_read (void *opaque, hwaddr addr)
-{
-    openpic_t *mpp = opaque;
-    uint32_t retval;
-    int idx = MPIC_INT_IRQ;
-
-    DPRINTF("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
-    retval = 0xFFFFFFFF;
-    if (addr & 0xF)
-        return retval;
-
-    if (addr < MPIC_INT_REG_SIZE) {
-        idx += (addr & 0xFFF0) >> 5;
-        if (addr & 0x10) {
-            /* EXDE / IFEDE / IEEDE */
-            retval = read_IRQreg_ide(mpp, idx);
-        } else {
-            /* EXVP / IFEVP / IEEVP */
-            retval = read_IRQreg_ipvp(mpp, idx);
-        }
-        DPRINTF("%s: => %08x\n", __func__, retval);
-    }
-
-    return retval;
-}
-
-static void mpic_src_msg_write (void *opaque, hwaddr addr,
-                                uint32_t val)
-{
-    openpic_t *mpp = opaque;
-    int idx = MPIC_MSG_IRQ;
-
-    DPRINTF("%s: addr " TARGET_FMT_plx " <= %08x\n", __func__, addr, val);
+    DPRINTF("%s: addr " TARGET_FMT_plx " <= %08" PRIx64 "\n",
+            __func__, addr, val);
     if (addr & 0xF)
         return;
 
-    if (addr < MPIC_MSG_REG_SIZE) {
-        idx += (addr & 0xFFF0) >> 5;
-        if (addr & 0x10) {
-            /* EXDE / IFEDE / IEEDE */
-            write_IRQreg_ide(mpp, idx, val);
-        } else {
-            /* EXVP / IFEVP / IEEVP */
-            write_IRQreg_ipvp(mpp, idx, val);
-        }
-    }
-}
-
-static uint32_t mpic_src_msg_read (void *opaque, hwaddr addr)
-{
-    openpic_t *mpp = opaque;
-    uint32_t retval;
-    int idx = MPIC_MSG_IRQ;
-
-    DPRINTF("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
-    retval = 0xFFFFFFFF;
-    if (addr & 0xF)
-        return retval;
-
-    if (addr < MPIC_MSG_REG_SIZE) {
-        idx += (addr & 0xFFF0) >> 5;
-        if (addr & 0x10) {
-            /* EXDE / IFEDE / IEEDE */
-            retval = read_IRQreg_ide(mpp, idx);
-        } else {
-            /* EXVP / IFEVP / IEEVP */
-            retval = read_IRQreg_ipvp(mpp, idx);
-        }
-        DPRINTF("%s: => %08x\n", __func__, retval);
+    if (addr & 0x10) {
+        /* EXDE / IFEDE / IEEDE */
+        write_IRQreg_ide(mpp, idx, val);
+    } else {
+        /* EXVP / IFEVP / IEEVP */
+        write_IRQreg_ipvp(mpp, idx, val);
     }
-
-    return retval;
 }
 
-static void mpic_src_msi_write (void *opaque, hwaddr addr,
-                                uint32_t val)
-{
-    openpic_t *mpp = opaque;
-    int idx = MPIC_MSI_IRQ;
-
-    DPRINTF("%s: addr " TARGET_FMT_plx " <= %08x\n", __func__, addr, val);
-    if (addr & 0xF)
-        return;
-
-    if (addr < MPIC_MSI_REG_SIZE) {
-        idx += (addr & 0xFFF0) >> 5;
-        if (addr & 0x10) {
-            /* EXDE / IFEDE / IEEDE */
-            write_IRQreg_ide(mpp, idx, val);
-        } else {
-            /* EXVP / IFEVP / IEEVP */
-            write_IRQreg_ipvp(mpp, idx, val);
-        }
-    }
-}
-static uint32_t mpic_src_msi_read (void *opaque, hwaddr addr)
+static uint64_t mpic_src_irq_read(void *opaque, hwaddr addr, unsigned len)
 {
     openpic_t *mpp = opaque;
     uint32_t retval;
-    int idx = MPIC_MSI_IRQ;
+    int idx = addr / 0x20;
 
     DPRINTF("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
-    retval = 0xFFFFFFFF;
     if (addr & 0xF)
-        return retval;
+        return -1;
 
-    if (addr < MPIC_MSI_REG_SIZE) {
-        idx += (addr & 0xFFF0) >> 5;
-        if (addr & 0x10) {
-            /* EXDE / IFEDE / IEEDE */
-            retval = read_IRQreg_ide(mpp, idx);
-        } else {
-            /* EXVP / IFEVP / IEEVP */
-            retval = read_IRQreg_ipvp(mpp, idx);
-        }
-        DPRINTF("%s: => %08x\n", __func__, retval);
+    if (addr & 0x10) {
+        /* EXDE / IFEDE / IEEDE */
+        retval = read_IRQreg_ide(mpp, idx);
+    } else {
+        /* EXVP / IFEVP / IEEVP */
+        retval = read_IRQreg_ipvp(mpp, idx);
     }
+    DPRINTF("%s: => %08x\n", __func__, retval);
 
     return retval;
 }
@@ -1438,60 +1282,14 @@  static const MemoryRegionOps mpic_cpu_ops = {
     .endianness = DEVICE_BIG_ENDIAN,
 };
 
-static const MemoryRegionOps mpic_ext_ops = {
-    .old_mmio = {
-        .write = { openpic_buggy_write,
-                   openpic_buggy_write,
-                   mpic_src_ext_write,
-        },
-        .read  = { openpic_buggy_read,
-                   openpic_buggy_read,
-                   mpic_src_ext_read,
-        },
-    },
-    .endianness = DEVICE_BIG_ENDIAN,
-};
-
-static const MemoryRegionOps mpic_int_ops = {
-    .old_mmio = {
-        .write = { openpic_buggy_write,
-                   openpic_buggy_write,
-                   mpic_src_int_write,
-        },
-        .read  = { openpic_buggy_read,
-                   openpic_buggy_read,
-                   mpic_src_int_read,
-        },
-    },
-    .endianness = DEVICE_BIG_ENDIAN,
-};
-
-static const MemoryRegionOps mpic_msg_ops = {
-    .old_mmio = {
-        .write = { openpic_buggy_write,
-                   openpic_buggy_write,
-                   mpic_src_msg_write,
-        },
-        .read  = { openpic_buggy_read,
-                   openpic_buggy_read,
-                   mpic_src_msg_read,
-        },
-    },
+static const MemoryRegionOps mpic_irq_ops = {
+    .write = mpic_src_irq_write,
+    .read  = mpic_src_irq_read,
     .endianness = DEVICE_BIG_ENDIAN,
-};
-
-static const MemoryRegionOps mpic_msi_ops = {
-    .old_mmio = {
-        .write = { openpic_buggy_write,
-                   openpic_buggy_write,
-                   mpic_src_msi_write,
-        },
-        .read  = { openpic_buggy_read,
-                   openpic_buggy_read,
-                   mpic_src_msi_read,
-        },
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
     },
-    .endianness = DEVICE_BIG_ENDIAN,
 };
 
 qemu_irq *mpic_init (MemoryRegion *address_space, hwaddr base,
@@ -1507,10 +1305,7 @@  qemu_irq *mpic_init (MemoryRegion *address_space, hwaddr base,
     } const list[] = {
         {"glb", &mpic_glb_ops, MPIC_GLB_REG_START, MPIC_GLB_REG_SIZE},
         {"tmr", &mpic_tmr_ops, MPIC_TMR_REG_START, MPIC_TMR_REG_SIZE},
-        {"ext", &mpic_ext_ops, MPIC_EXT_REG_START, MPIC_EXT_REG_SIZE},
-        {"int", &mpic_int_ops, MPIC_INT_REG_START, MPIC_INT_REG_SIZE},
-        {"msg", &mpic_msg_ops, MPIC_MSG_REG_START, MPIC_MSG_REG_SIZE},
-        {"msi", &mpic_msi_ops, MPIC_MSI_REG_START, MPIC_MSI_REG_SIZE},
+        {"irq", &mpic_irq_ops, MPIC_IRQ_REG_START, MPIC_IRQ_REG_SIZE},
         {"cpu", &mpic_cpu_ops, MPIC_CPU_REG_START, MPIC_CPU_REG_SIZE},
     };
 
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 47e2d41..f3e97d8 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -502,13 +502,13 @@  void ppce500_init(PPCE500Params *params)
     /* Serial */
     if (serial_hds[0]) {
         serial_mm_init(ccsr_addr_space, MPC8544_SERIAL0_REGS_OFFSET,
-                       0, mpic[12+26], 399193,
+                       0, mpic[42], 399193,
                        serial_hds[0], DEVICE_BIG_ENDIAN);
     }
 
     if (serial_hds[1]) {
         serial_mm_init(ccsr_addr_space, MPC8544_SERIAL1_REGS_OFFSET,
-                       0, mpic[12+26], 399193,
+                       0, mpic[42], 399193,
                        serial_hds[1], DEVICE_BIG_ENDIAN);
     }