Re: [PATCH 2/5] qemu/kvm: E500 core emulation

2009-01-12 Thread Hollis Blanchard
On Monday 12 January 2009 03:02:10 Liu Yu wrote:
> > -Original Message-
> > From: kvm-ppc-ow...@vger.kernel.org 
> > [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Hollis Blanchard
> > Sent: Saturday, January 10, 2009 4:16 AM
> > To: Liu Yu-B13201
> > Cc: kvm-ppc@vger.kernel.org
> > Subject: Re: [PATCH 2/5] qemu/kvm: E500 core emulation
> > 
> > On Fri, 2009-01-09 at 15:56 +0800, Liu Yu wrote:
> > > Signed-off-by: Liu Yu 
> > > ---
> > >  hw/ppce500.c |   74 
> > ++
> > >  hw/ppce500.h |   66 
> > +++
> > >  2 files changed, 140 insertions(+), 0 deletions(-)
> > >  create mode 100644 hw/ppce500.c
> > >  create mode 100644 hw/ppce500.h
> > > 
> > > diff --git a/hw/ppce500.c b/hw/ppce500.c
> > > new file mode 100644
> > > index 000..13eab47
> > > --- /dev/null
> > > +++ b/hw/ppce500.c
> > > @@ -0,0 +1,74 @@
> > > +/*
> > > + * Qemu PowerPC E500 core emualtion
> > > + *
> > > + * Copyright (C) 2009 Freescale Semiconductor, Inc. All 
> > rights reserved.
> > > + *
> > > + * Author: Yu Liu,     
> > > + *
> > > + * This file is derived from hw/ppc440.c
> > > + * the copyright for that material belongs to the original owners.
> > 
> > I guess you based this off an old version of ppc440.c, which 
> > isn't good
> > because I improved that code a lot when merging in to qemu.
> > 
> > > + * This is free software; you can redistribute it and/or modify
> > > + * it under the terms of  the GNU General  Public License 
> > as published by
> > > + * the Free Software Foundation;  either version 2 of the  
> > License, or
> > > + * (at your option) any later version.
> > > + */
> > > +
> > > +#include "hw.h"
> > > +#include "pc.h"
> > > +#include "hw/isa.h"
> > > +#include "ppce500.h"
> > > +#include "sysemu.h"
> > > +
> > > +#define PPCE500_SERIAL0_REGS   0x4500
> > > +#define PPCE500_SERIAL1_REGS   0x4600
> > > +#define PPCE500_MPIC_REGS  0x4
> > > +#define bytes_to_mb(a) (a>>20)
> > > +
> > > +CPUState *ppce500_init(ram_addr_t *ram_size,
> > > +   qemu_irq **pic,
> > > +   target_ulong ccsrbar_base)
> > > +{
> > > +    qemu_irq *mpic_irq, *irqs;
> > > +    SerialState * serial[2];
> > > +    int ram_stick_sizes[] = {512<<20, 256<<20}; /* in bytes */
> > 
> > Your memory controller only supports 512M and 256M?
> 
> I don't emulate the memory controller, so I think qemu/e500 coulde
> support all kinds capacities.
> 
> 
> > 
> > > +    int i;
> > > +    ram_addr_t tmp_ram_size;
> > > +    CPUState *env;
> > > +
> > > +    /* Setup Memory */
> > > +    tmp_ram_size = *ram_size;
> > > +
> > > +    for (i=0; 
> > i<(sizeof(ram_stick_sizes)/sizeof(ram_stick_sizes[0])); i++)
> > > +   while ((tmp_ram_size/ram_stick_sizes[i]) > 0)
> > > +       tmp_ram_size -= ram_stick_sizes[i];
> > > +
> > > +    if (tmp_ram_size)
> > > +   *ram_size -= tmp_ram_size;
> > 
> > This code was all refactored into a much nicer form. Look at
> > ppc4xx_sdram_adjust(), and rename it if you need it too.
> 
> As I haven't emulated memory controller, do I need to do this?

No, you don't. In that case you should remove this ram code entirely.

(The only reason we emulated the memory controller is because some of the 
early Linux bootwrapper code on 4xx uses it.)

-- 
Hollis Blanchard
IBM Linux Technology Center
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 2/5] qemu/kvm: E500 core emulation

2009-01-12 Thread Liu Yu

> -Original Message-
> From: kvm-ppc-ow...@vger.kernel.org 
> [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Hollis Blanchard
> Sent: Saturday, January 10, 2009 4:16 AM
> To: Liu Yu-B13201
> Cc: kvm-ppc@vger.kernel.org
> Subject: Re: [PATCH 2/5] qemu/kvm: E500 core emulation
> 
> On Fri, 2009-01-09 at 15:56 +0800, Liu Yu wrote:
> > Signed-off-by: Liu Yu 
> > ---
> >  hw/ppce500.c |   74 
> ++
> >  hw/ppce500.h |   66 
> +++
> >  2 files changed, 140 insertions(+), 0 deletions(-)
> >  create mode 100644 hw/ppce500.c
> >  create mode 100644 hw/ppce500.h
> > 
> > diff --git a/hw/ppce500.c b/hw/ppce500.c
> > new file mode 100644
> > index 000..13eab47
> > --- /dev/null
> > +++ b/hw/ppce500.c
> > @@ -0,0 +1,74 @@
> > +/*
> > + * Qemu PowerPC E500 core emualtion
> > + *
> > + * Copyright (C) 2009 Freescale Semiconductor, Inc. All 
> rights reserved.
> > + *
> > + * Author: Yu Liu, 
> > + *
> > + * This file is derived from hw/ppc440.c
> > + * the copyright for that material belongs to the original owners.
> 
> I guess you based this off an old version of ppc440.c, which 
> isn't good
> because I improved that code a lot when merging in to qemu.
> 
> > + * This is free software; you can redistribute it and/or modify
> > + * it under the terms of  the GNU General  Public License 
> as published by
> > + * the Free Software Foundation;  either version 2 of the  
> License, or
> > + * (at your option) any later version.
> > + */
> > +
> > +#include "hw.h"
> > +#include "pc.h"
> > +#include "hw/isa.h"
> > +#include "ppce500.h"
> > +#include "sysemu.h"
> > +
> > +#define PPCE500_SERIAL0_REGS   0x4500
> > +#define PPCE500_SERIAL1_REGS   0x4600
> > +#define PPCE500_MPIC_REGS  0x4
> > +#define bytes_to_mb(a) (a>>20)
> > +
> > +CPUState *ppce500_init(ram_addr_t *ram_size,
> > +   qemu_irq **pic,
> > +   target_ulong ccsrbar_base)
> > +{
> > +qemu_irq *mpic_irq, *irqs;
> > +SerialState * serial[2];
> > +int ram_stick_sizes[] = {512<<20, 256<<20}; /* in bytes */
> 
> Your memory controller only supports 512M and 256M?

I don't emulate the memory controller, so I think qemu/e500 coulde
support all kinds capacities.
 

> 
> > +int i;
> > +ram_addr_t tmp_ram_size;
> > +CPUState *env;
> > +
> > +/* Setup Memory */
> > +tmp_ram_size = *ram_size;
> > +
> > +for (i=0; 
> i<(sizeof(ram_stick_sizes)/sizeof(ram_stick_sizes[0])); i++)
> > +   while ((tmp_ram_size/ram_stick_sizes[i]) > 0)
> > +   tmp_ram_size -= ram_stick_sizes[i];
> > +
> > +if (tmp_ram_size)
> > +   *ram_size -= tmp_ram_size;
> 
> This code was all refactored into a much nicer form. Look at
> ppc4xx_sdram_adjust(), and rename it if you need it too.

As I haven't emulated memory controller, do I need to do this?

> 
> > +env = cpu_ppc_init("e500v2_v30");
> > +if (!env) {
> > +   fprintf(stderr, "Unable to initilize CPU!\n");
> > +   exit(1);
> > +}
> > +
> > +/* mpic */
> > +irqs = qemu_mallocz(sizeof(qemu_irq) * MPIC_OUTPUT_NB);
> > +irqs[MPIC_OUTPUT_INT] = ((qemu_irq 
> *)env->irq_inputs)[PPCE500_INPUT_INT];
> > +irqs[MPIC_OUTPUT_CINT] = ((qemu_irq 
> *)env->irq_inputs)[PPCE500_INPUT_CINT];
> > +mpic_irq = mpic_init(ccsrbar_base + PPCE500_MPIC_REGS, 
> 1, &irqs, NULL);
> > +*pic = mpic_irq;
> > +
> > +/* serial */
> > +if (serial_hds[0])
> > +   serial[0] = serial_mm_init(ccsrbar_base + PPCE500_SERIAL0_REGS,
> > +   0, mpic_irq[12+26], 399193,
> > +   serial_hds[0], 1);
> > +
> > +if (serial_hds[1])
> > +   serial[0] = serial_mm_init(ccsrbar_base + PPCE500_SERIAL1_REGS,
> > +   0, mpic_irq[12+26], 399193,
> > +   serial_hds[0], 1);
> > +
> > +return env;
> > +}
> 
> e500 is a core, but the code you have here is initializing chip state.
> These should be separated.
Fixed. Thanks

> 
> > diff --git a/hw/ppce500.h b/hw/ppce500.h
> > new file mode 100644
> > index 000..3fa8604
> > --- /dev/null
> > +++ b/hw/ppce500.h
> > @@ -0,0 +1,66 @@
> > +/*
> > + * QEMU PowerPC E500

Re: [PATCH 2/5] qemu/kvm: E500 core emulation

2009-01-09 Thread Hollis Blanchard
On Fri, 2009-01-09 at 15:56 +0800, Liu Yu wrote:
> Signed-off-by: Liu Yu 
> ---
>  hw/ppce500.c |   74 
> ++
>  hw/ppce500.h |   66 +++
>  2 files changed, 140 insertions(+), 0 deletions(-)
>  create mode 100644 hw/ppce500.c
>  create mode 100644 hw/ppce500.h
> 
> diff --git a/hw/ppce500.c b/hw/ppce500.c
> new file mode 100644
> index 000..13eab47
> --- /dev/null
> +++ b/hw/ppce500.c
> @@ -0,0 +1,74 @@
> +/*
> + * Qemu PowerPC E500 core emualtion
> + *
> + * Copyright (C) 2009 Freescale Semiconductor, Inc. All rights reserved.
> + *
> + * Author: Yu Liu, 
> + *
> + * This file is derived from hw/ppc440.c
> + * the copyright for that material belongs to the original owners.

I guess you based this off an old version of ppc440.c, which isn't good
because I improved that code a lot when merging in to qemu.

> + * This is free software; you can redistribute it and/or modify
> + * it under the terms of  the GNU General  Public License as published by
> + * the Free Software Foundation;  either version 2 of the  License, or
> + * (at your option) any later version.
> + */
> +
> +#include "hw.h"
> +#include "pc.h"
> +#include "hw/isa.h"
> +#include "ppce500.h"
> +#include "sysemu.h"
> +
> +#define PPCE500_SERIAL0_REGS 0x4500
> +#define PPCE500_SERIAL1_REGS 0x4600
> +#define PPCE500_MPIC_REGS0x4
> +#define bytes_to_mb(a) (a>>20)
> +
> +CPUState *ppce500_init(ram_addr_t *ram_size,
> + qemu_irq **pic,
> + target_ulong ccsrbar_base)
> +{
> +qemu_irq *mpic_irq, *irqs;
> +SerialState * serial[2];
> +int ram_stick_sizes[] = {512<<20, 256<<20}; /* in bytes */

Your memory controller only supports 512M and 256M?

> +int i;
> +ram_addr_t tmp_ram_size;
> +CPUState *env;
> +
> +/* Setup Memory */
> +tmp_ram_size = *ram_size;
> +
> +for (i=0; i<(sizeof(ram_stick_sizes)/sizeof(ram_stick_sizes[0])); i++)
> + while ((tmp_ram_size/ram_stick_sizes[i]) > 0)
> + tmp_ram_size -= ram_stick_sizes[i];
> +
> +if (tmp_ram_size)
> + *ram_size -= tmp_ram_size;

This code was all refactored into a much nicer form. Look at
ppc4xx_sdram_adjust(), and rename it if you need it too.

> +env = cpu_ppc_init("e500v2_v30");
> +if (!env) {
> + fprintf(stderr, "Unable to initilize CPU!\n");
> + exit(1);
> +}
> +
> +/* mpic */
> +irqs = qemu_mallocz(sizeof(qemu_irq) * MPIC_OUTPUT_NB);
> +irqs[MPIC_OUTPUT_INT] = ((qemu_irq *)env->irq_inputs)[PPCE500_INPUT_INT];
> +irqs[MPIC_OUTPUT_CINT] = ((qemu_irq 
> *)env->irq_inputs)[PPCE500_INPUT_CINT];
> +mpic_irq = mpic_init(ccsrbar_base + PPCE500_MPIC_REGS, 1, &irqs, NULL);
> +*pic = mpic_irq;
> +
> +/* serial */
> +if (serial_hds[0])
> + serial[0] = serial_mm_init(ccsrbar_base + PPCE500_SERIAL0_REGS,
> + 0, mpic_irq[12+26], 399193,
> + serial_hds[0], 1);
> +
> +if (serial_hds[1])
> + serial[0] = serial_mm_init(ccsrbar_base + PPCE500_SERIAL1_REGS,
> + 0, mpic_irq[12+26], 399193,
> + serial_hds[0], 1);
> +
> +return env;
> +}

e500 is a core, but the code you have here is initializing chip state.
These should be separated.

> diff --git a/hw/ppce500.h b/hw/ppce500.h
> new file mode 100644
> index 000..3fa8604
> --- /dev/null
> +++ b/hw/ppce500.h
> @@ -0,0 +1,66 @@
> +/*
> + * QEMU PowerPC E500 emulation shared definitions
> + *
> + * Copyright (C) 2009 Freescale Semiconductor, Inc. All rights reserved.
> + *
> + * Author: Yu Liu, 
> + *
> + * This file is derived from hw/ppc440.h
> + * the copyright for that material belongs to the original owners.
> + *
> + * This is free software; you can redistribute it and/or modify
> + * it under the terms of  the GNU General  Public License as published by
> + * the Free Software Foundation;  either version 2 of the  License, or
> + * (at your option) any later version.
> + */
> +
> +#if !defined(PPC_E500_H)
> +#define PPC_E500_H
> +
> +#include "hw.h"
> +#include "pc.h"
> +#include "qemu-timer.h"
> +#include "sysemu.h"
> +#include "exec-all.h"
> +#include "boards.h"
> +
> +/* PowerPC E500 XXX  initialization */
> +extern CPUState *ppce500_init(ram_addr_t *ram_size,
> + qemu_irq **pic,
> + target_ulong ccsrbar_base);
> +
> +enum {
> +PPCUIC_OUTPUT_INT = 0,
> +PPCUIC_OUTPUT_CINT = 1,
> +PPCUIC_OUTPUT_NB,
> +};

UIC is the name of the 4xx interrupt controller, so I suspect this isn't
actually used in you code.

> +struct  pci_outbound {
> +uint32_t potar;
> +uint32_t potear;
> +uint32_t powbar;
> +uint32_t powar;
> +};
> +
> +struct pci_inbound {
> +uint32_t pitar;
> +uint32_t piwbar;
> +uint32_t piwbear;
> +uint32_t piwar;
> +};
> +
> +PCIBus *ppce500_pci_init(qemu_irq *pic, target_phys_addr_t registers);
> +
> +/* mpic.c */
> +/* MPIC have 5 outputs per CPU c