Re: [Qemu-devel] [PATCH 1/5] kvm/powerpc: Enable MPIC for E500 platform.

[Blue Swirl <blauwirbel@gmail.com>]

On 2/17/09, Liu Yu <yu.liu@freescale.com> wrote:
> MPIC and OpenPIC have very similar design.
>  So a lot of code can be reused.
>
>  Modification mainly include:
>  1. keep struct openpic_t to the maximum size of both MPIC and OpenPIC.
>  2. endianess swap.
>    MPIC has the same endianess as target, so no need to swap for MPIC.

I don't think this is correct, the host can still be different endian
from target.

>  3. using different init functions and function pointers for reset and irq raise.

You didn't register the reset handler with qemu_register_reset.

>  Haven't test OpenPIC.
>
>  Signed-off-by: Liu Yu <yu.liu@freescale.com>

>  +static void mpic_src_write (void *opaque, uint32_t addr, uint32_t val)

>  +    if (addr < 0x180) {
>  +        idx = MPIC_EXT_IRQ;
>  +    } else if (addr >= 0x200 && addr < 0xa00) {
>  +        idx = MPIC_INT_IRQ;
>  +        addr -= 0x200;
>  +    } else if (addr >= 0x1600 && addr < 0x1700) {
>  +        idx = MPIC_MSG_IRQ;
>  +        addr -= 0x1600;
>  +    } else if (addr >= 0x1C00 && addr < 0x1D00) {
>  +        idx = MPIC_MSI_IRQ;
>  +        addr -= 0x1C00;
>  +    } else {
>  +        return;

It would be faster and simpler to register different handlers for
these memory areas, same goes for mpic_src_read, mpic_writel and
mpic_readl.

>  +    register_savevm("mpic", 0, 1, openpic_save, openpic_load,  mpp);

When the save format changes, the version number should be bumped.