[Qemu-devel] RE: [PATCH 9/9] powerpc/kvm: Add MPC85xx board support

[Hollis Blanchard <hollisb@us.ibm.com>]

On Tue, 2009-01-20 at 11:09 +0800, Liu Yu wrote:
> > -----Original Message-----
> > From: kvm-ppc-owner@vger.kernel.org 
> > [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of Hollis Blanchard
> > Sent: Saturday, January 17, 2009 2:10 AM
> > To: Liu Yu-B13201
> > Cc: qemu-devel@nongnu.org; kvm-ppc@vger.kernel.org
> > Subject: RE: [PATCH 9/9] powerpc/kvm: Add MPC85xx board support
> > 
> > Hmm, I have no idea why I didn't see this patch. Maybe it got 
> > caught in
> > a spam filter.
> 
> I should have cc to you...
> 
> > 
> > On Fri, 2009-01-16 at 16:22 +0800, Liu Yu wrote:
> > > > -----Original Message-----
> > > > From: Liu Yu-B13201 
> > > > Sent: Thursday, January 15, 2009 8:34 PM
> > > > To: qemu-devel@nongnu.org
> > > > Cc: kvm-ppc@vger.kernel.org; Liu Yu-B13201
> > > > Subject: [PATCH 9/9] powerpc/kvm: Add MPC85xx board support
> > > > 
> > > > MPC85xx adopts E500 core.
> > > > This patch hopes to support all MPC85xx boards.
> > > > 
> > > > Signed-off-by: Liu Yu <yu.liu@freescale.com>
> > > > --- /dev/null
> > > > +++ b/hw/ppce500_mpc85xx.c
> > > > +static void mpc85xx_init(ram_addr_t ram_size, int vga_ram_size,
> > > > +    		const char *boot_device, DisplayState *ds,
> > > > +    		const char *kernel_filename,
> > > > +    		const char *kernel_cmdline,
> > > > +    		const char *initrd_filename,
> > > > +    		const char *cpu_model)
> > > > +{
> > > > +    PCIBus *pci_bus;
> > > > +    CPUState *env;
> > > > +    uint64_t elf_entry;
> > > > +    uint64_t elf_lowaddr;
> > > > +    target_ulong entry=0;
> > > > +    target_ulong loadaddr=UIMAGE_LOAD_BASE;
> > > > +    target_long kernel_size=0;
> > > > +    target_ulong dt_base=DTB_LOAD_BASE;
> > > > +    target_ulong initrd_base=INITRD_LOAD_BASE;
> > > > +    target_long initrd_size=0;
> > > > +    void *fdt;
> > > > +    int i=0;
> > > > +    unsigned int pci_irq_nrs[4] = {1, 2, 3, 4};
> > > > +    qemu_irq *irqs, *mpic, *pci_irqs;
> > > > +    SerialState * serial[2];
> > > > +
> > > > +    /* Setup CPU */
> > > > +    env = ppce500_init(&ram_size);
> > > > +
> > > > +    /* 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 = mpic_init(MPC85xx_MPIC_REGS_BASE, 1, &irqs, NULL);
> > > > +
> > > > +    /* Serial */
> > > > +    if (serial_hds[0])
> > > > +	serial[0] = serial_mm_init(MPC85xx_SERIAL0_REGS_BASE,
> > > > +			0, mpic[12+26], 399193,
> > > > +			serial_hds[0], 1);
> > > > +
> > > > +    if (serial_hds[1])
> > > > +	serial[0] = serial_mm_init(MPC85xx_SERIAL1_REGS_BASE,
> > > > +			0, mpic[12+26], 399193,
> > > > +			serial_hds[0], 1);
> > > > +
> > > > +    /* PCI */
> > > > +    pci_irqs = qemu_malloc(sizeof(qemu_irq) * 4);
> > > > +    pci_irqs[0] = mpic[pci_irq_nrs[0]];
> > > > +    pci_irqs[1] = mpic[pci_irq_nrs[1]];
> > > > +    pci_irqs[2] = mpic[pci_irq_nrs[2]];
> > > > +    pci_irqs[3] = mpic[pci_irq_nrs[3]];
> > > > +    pci_bus = ppce500_pci_init(pci_irqs, MPC85xx_PCI_REGS_BASE);
> > > > +    if (!pci_bus)
> > > > +	printf("couldn't create PCI controller!\n");
> > > > +
> > > > +    isa_mmio_init(MPC85xx_PCI_IO, MPC85xx_PCI_IOLEN);
> > > > +
> > > > +    /* Register mem */
> > > > +    cpu_register_physical_memory(0, ram_size, 0);
> > > 
> > > Hollis, I didn't see the memeory register like this in 440 code.
> > > Is that a bug?
> > 
> > RAM is allocated in vl.c (see the call to qemu_vmalloc()), so it looks
> > like you're actually allocating twice the amount of memory you should
> > be.
> > 
> 
> It's odd.
> I cannot make it work without cpu_register_physical_memory() which
> ultimately call kvm_set_phys_mem().
> But qemu_vmalloc() seems just to call mmap().
> Did I miss anything?

OK, I was *really* confused on this one for a while... you're correct,
you must call cpu_register_physical_memory().

On 4xx, that is done by the memory controller emulation
(sdram_set_bcr()). Since you aren't using that code, you do need to call
it yourself.

-- 
Hollis Blanchard
IBM Linux Technology Center