| Message ID | 20220801131039.1693913-6-clg@kaod.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | ppc: QOM'ify 405 board | expand |
On 8/1/22 10:10, Cédric Le Goater wrote: > This moves all the code previously done in the ppc405ep_init() routine > under ppc405_soc_realize(). > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > hw/ppc/ppc405.h | 12 ++-- > hw/ppc/ppc405_boards.c | 12 ++-- > hw/ppc/ppc405_uc.c | 151 ++++++++++++++++++++--------------------- > 3 files changed, 84 insertions(+), 91 deletions(-) > > diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h > index c8cddb71733a..5e4e96d86ceb 100644 > --- a/hw/ppc/ppc405.h > +++ b/hw/ppc/ppc405.h > @@ -74,9 +74,14 @@ struct Ppc405SoCState { > MemoryRegion sram; > MemoryRegion ram_memories[2]; > hwaddr ram_bases[2], ram_sizes[2]; > + bool do_dram_init; > > MemoryRegion *dram_mr; > hwaddr ram_size; > + > + uint32_t sysclk; > + PowerPCCPU *cpu; > + DeviceState *uic; > }; > > /* PowerPC 405 core */ > @@ -85,11 +90,4 @@ ram_addr_t ppc405_set_bootinfo(CPUPPCState *env, ram_addr_t ram_size); > void ppc4xx_plb_init(CPUPPCState *env); > void ppc405_ebc_init(CPUPPCState *env); > > -PowerPCCPU *ppc405ep_init(MemoryRegion *address_space_mem, > - MemoryRegion ram_memories[2], > - hwaddr ram_bases[2], > - hwaddr ram_sizes[2], > - uint32_t sysclk, DeviceState **uicdev, > - int do_init); > - > #endif /* PPC405_H */ > diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c > index 96db52c5a309..363cb0770506 100644 > --- a/hw/ppc/ppc405_boards.c > +++ b/hw/ppc/ppc405_boards.c > @@ -237,9 +237,7 @@ static void ppc405_init(MachineState *machine) > Ppc405MachineState *ppc405 = PPC405_MACHINE(machine); > MachineClass *mc = MACHINE_GET_CLASS(machine); > const char *kernel_filename = machine->kernel_filename; > - PowerPCCPU *cpu; > MemoryRegion *sysmem = get_system_memory(); > - DeviceState *uicdev; > > if (machine->ram_size != mc->default_ram_size) { > char *sz = size_to_str(mc->default_ram_size); > @@ -254,12 +252,12 @@ static void ppc405_init(MachineState *machine) > machine->ram_size, &error_fatal); > object_property_set_link(OBJECT(&ppc405->soc), "dram", > OBJECT(machine->ram), &error_abort); > + object_property_set_bool(OBJECT(&ppc405->soc), "dram-init", > + !(kernel_filename == NULL), &error_abort); > + object_property_set_uint(OBJECT(&ppc405->soc), "sys-clk", 33333333, > + &error_abort); > qdev_realize(DEVICE(&ppc405->soc), NULL, &error_abort); > > - cpu = ppc405ep_init(sysmem, ppc405->soc.ram_memories, ppc405->soc.ram_bases, > - ppc405->soc.ram_sizes, > - 33333333, &uicdev, kernel_filename == NULL ? 0 : 1); > - > /* allocate and load BIOS */ > if (machine->firmware) { > MemoryRegion *bios = g_new(MemoryRegion, 1); > @@ -315,7 +313,7 @@ static void ppc405_init(MachineState *machine) > > /* Load ELF kernel and rootfs.cpio */ > } else if (kernel_filename && !machine->firmware) { > - boot_from_kernel(machine, cpu); > + boot_from_kernel(machine, ppc405->soc.cpu); > } > } > > diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c > index 156e839b8283..59612504bf3f 100644 > --- a/hw/ppc/ppc405_uc.c > +++ b/hw/ppc/ppc405_uc.c > @@ -1432,134 +1432,131 @@ static void ppc405ep_cpc_init (CPUPPCState *env, clk_setup_t clk_setup[8], > #endif > } > > -PowerPCCPU *ppc405ep_init(MemoryRegion *address_space_mem, > - MemoryRegion ram_memories[2], > - hwaddr ram_bases[2], > - hwaddr ram_sizes[2], > - uint32_t sysclk, DeviceState **uicdevp, > - int do_init) > +static void ppc405_soc_realize(DeviceState *dev, Error **errp) > { > + Ppc405SoCState *s = PPC405_SOC(dev); > clk_setup_t clk_setup[PPC405EP_CLK_NB], tlb_clk_setup; > qemu_irq dma_irqs[4], gpt_irqs[5], mal_irqs[4]; > - PowerPCCPU *cpu; > CPUPPCState *env; > - DeviceState *uicdev; > - SysBusDevice *uicsbd; > + Error *err = NULL; > + > + /* XXX: fix this ? */ So, this comment, originally from ppc405_boards.c, was added by commit 1a6c088620368 and it seemed to make reference to something with the refering to the ram_* values: /* XXX: fix this */ ram_bases[0] = 0x00000000; ram_sizes[0] = 0x08000000; ram_bases[1] = 0x00000000; ram_sizes[1] = 0x00000000; (...) No more context is provided aside from a git-svn-id from savannah.nongnu.org. If no one can provide more context about what is to be fixed here, I'll remove the comment. > + memory_region_init_alias(&s->ram_memories[0], OBJECT(s), > + "ef405ep.ram.alias", s->dram_mr, 0, s->ram_size); As I mentioned in patch 2, ef405ep.ram.alias can be renamed to ppc405.ram.alias ... > + s->ram_bases[0] = 0; > + s->ram_sizes[0] = s->ram_size; > + memory_region_init(&s->ram_memories[1], OBJECT(s), "ef405ep.ram1", 0); And this can be renamed to ef405ep.ram1. If you agree with the rename I can amend it in the tree. Thanks, Daniel > + s->ram_bases[1] = 0x00000000; > + s->ram_sizes[1] = 0x00000000; > + > + /* allocate SRAM */ > + memory_region_init_ram(&s->sram, OBJECT(s), "ef405ep.sram", > + PPC405EP_SRAM_SIZE, &err); > + if (err) { > + error_propagate(errp, err); > + return; > + } > + memory_region_add_subregion(get_system_memory(), PPC405EP_SRAM_BASE, > + &s->sram); > > memset(clk_setup, 0, sizeof(clk_setup)); > + > /* init CPUs */ > - cpu = ppc4xx_init(POWERPC_CPU_TYPE_NAME("405ep"), > + s->cpu = ppc4xx_init(POWERPC_CPU_TYPE_NAME("405ep"), > &clk_setup[PPC405EP_CPU_CLK], > - &tlb_clk_setup, sysclk); > - env = &cpu->env; > + &tlb_clk_setup, s->sysclk); > + env = &s->cpu->env; > clk_setup[PPC405EP_CPU_CLK].cb = tlb_clk_setup.cb; > clk_setup[PPC405EP_CPU_CLK].opaque = tlb_clk_setup.opaque; > - /* Internal devices init */ > - /* Memory mapped devices registers */ > + > + /* CPU control */ > + ppc405ep_cpc_init(env, clk_setup, s->sysclk); > + > /* PLB arbitrer */ > ppc4xx_plb_init(env); > + > /* PLB to OPB bridge */ > ppc4xx_pob_init(env); > + > /* OBP arbitrer */ > ppc4xx_opba_init(0xef600600); > + > /* Universal interrupt controller */ > - uicdev = qdev_new(TYPE_PPC_UIC); > - uicsbd = SYS_BUS_DEVICE(uicdev); > + s->uic = qdev_new(TYPE_PPC_UIC); > > - object_property_set_link(OBJECT(uicdev), "cpu", OBJECT(cpu), > + object_property_set_link(OBJECT(s->uic), "cpu", OBJECT(s->cpu), > &error_fatal); > - sysbus_realize_and_unref(uicsbd, &error_fatal); > - > - sysbus_connect_irq(uicsbd, PPCUIC_OUTPUT_INT, > - qdev_get_gpio_in(DEVICE(cpu), PPC40x_INPUT_INT)); > - sysbus_connect_irq(uicsbd, PPCUIC_OUTPUT_CINT, > - qdev_get_gpio_in(DEVICE(cpu), PPC40x_INPUT_CINT)); > + if (!sysbus_realize(SYS_BUS_DEVICE(s->uic), errp)) { > + return; > + } > > - *uicdevp = uicdev; > + sysbus_connect_irq(SYS_BUS_DEVICE(s->uic), PPCUIC_OUTPUT_INT, > + qdev_get_gpio_in(DEVICE(s->cpu), PPC40x_INPUT_INT)); > + sysbus_connect_irq(SYS_BUS_DEVICE(s->uic), PPCUIC_OUTPUT_CINT, > + qdev_get_gpio_in(DEVICE(s->cpu), PPC40x_INPUT_CINT)); > > /* SDRAM controller */ > - /* XXX 405EP has no ECC interrupt */ > - ppc4xx_sdram_init(env, qdev_get_gpio_in(uicdev, 17), 2, ram_memories, > - ram_bases, ram_sizes, do_init); > + /* XXX 405EP has no ECC interrupt */ > + ppc4xx_sdram_init(env, qdev_get_gpio_in(s->uic, 17), 2, s->ram_memories, > + s->ram_bases, s->ram_sizes, s->do_dram_init); > + > /* External bus controller */ > ppc405_ebc_init(env); > + > /* DMA controller */ > - dma_irqs[0] = qdev_get_gpio_in(uicdev, 5); > - dma_irqs[1] = qdev_get_gpio_in(uicdev, 6); > - dma_irqs[2] = qdev_get_gpio_in(uicdev, 7); > - dma_irqs[3] = qdev_get_gpio_in(uicdev, 8); > + dma_irqs[0] = qdev_get_gpio_in(s->uic, 5); > + dma_irqs[1] = qdev_get_gpio_in(s->uic, 6); > + dma_irqs[2] = qdev_get_gpio_in(s->uic, 7); > + dma_irqs[3] = qdev_get_gpio_in(s->uic, 8); > ppc405_dma_init(env, dma_irqs); > - /* IIC controller */ > + > + /* I2C controller */ > sysbus_create_simple(TYPE_PPC4xx_I2C, 0xef600500, > - qdev_get_gpio_in(uicdev, 2)); > + qdev_get_gpio_in(s->uic, 2)); > /* GPIO */ > ppc405_gpio_init(0xef600700); > + > /* Serial ports */ > if (serial_hd(0) != NULL) { > - serial_mm_init(address_space_mem, 0xef600300, 0, > - qdev_get_gpio_in(uicdev, 0), > + serial_mm_init(get_system_memory(), 0xef600300, 0, > + qdev_get_gpio_in(s->uic, 0), > PPC_SERIAL_MM_BAUDBASE, serial_hd(0), > DEVICE_BIG_ENDIAN); > } > if (serial_hd(1) != NULL) { > - serial_mm_init(address_space_mem, 0xef600400, 0, > - qdev_get_gpio_in(uicdev, 1), > + serial_mm_init(get_system_memory(), 0xef600400, 0, > + qdev_get_gpio_in(s->uic, 1), > PPC_SERIAL_MM_BAUDBASE, serial_hd(1), > DEVICE_BIG_ENDIAN); > } > + > /* OCM */ > ppc405_ocm_init(env); > + > /* GPT */ > - gpt_irqs[0] = qdev_get_gpio_in(uicdev, 19); > - gpt_irqs[1] = qdev_get_gpio_in(uicdev, 20); > - gpt_irqs[2] = qdev_get_gpio_in(uicdev, 21); > - gpt_irqs[3] = qdev_get_gpio_in(uicdev, 22); > - gpt_irqs[4] = qdev_get_gpio_in(uicdev, 23); > + gpt_irqs[0] = qdev_get_gpio_in(s->uic, 19); > + gpt_irqs[1] = qdev_get_gpio_in(s->uic, 20); > + gpt_irqs[2] = qdev_get_gpio_in(s->uic, 21); > + gpt_irqs[3] = qdev_get_gpio_in(s->uic, 22); > + gpt_irqs[4] = qdev_get_gpio_in(s->uic, 23); > ppc4xx_gpt_init(0xef600000, gpt_irqs); > - /* PCI */ > - /* Uses UIC IRQs 3, 16, 18 */ > + > /* MAL */ > - mal_irqs[0] = qdev_get_gpio_in(uicdev, 11); > - mal_irqs[1] = qdev_get_gpio_in(uicdev, 12); > - mal_irqs[2] = qdev_get_gpio_in(uicdev, 13); > - mal_irqs[3] = qdev_get_gpio_in(uicdev, 14); > + mal_irqs[0] = qdev_get_gpio_in(s->uic, 11); > + mal_irqs[1] = qdev_get_gpio_in(s->uic, 12); > + mal_irqs[2] = qdev_get_gpio_in(s->uic, 13); > + mal_irqs[3] = qdev_get_gpio_in(s->uic, 14); > ppc4xx_mal_init(env, 4, 2, mal_irqs); > + > /* Ethernet */ > /* Uses UIC IRQs 9, 15, 17 */ > - /* CPU control */ > - ppc405ep_cpc_init(env, clk_setup, sysclk); > - > - return cpu; > -} > - > -static void ppc405_soc_realize(DeviceState *dev, Error **errp) > -{ > - Ppc405SoCState *s = PPC405_SOC(dev); > - Error *err = NULL; > - > - /* XXX: fix this ? */ > - memory_region_init_alias(&s->ram_memories[0], OBJECT(s), > - "ef405ep.ram.alias", s->dram_mr, 0, s->ram_size); > - s->ram_bases[0] = 0; > - s->ram_sizes[0] = s->ram_size; > - memory_region_init(&s->ram_memories[1], OBJECT(s), "ef405ep.ram1", 0); > - s->ram_bases[1] = 0x00000000; > - s->ram_sizes[1] = 0x00000000; > - > - /* allocate SRAM */ > - memory_region_init_ram(&s->sram, OBJECT(s), "ef405ep.sram", > - PPC405EP_SRAM_SIZE, &err); > - if (err) { > - error_propagate(errp, err); > - return; > - } > - memory_region_add_subregion(get_system_memory(), PPC405EP_SRAM_BASE, > - &s->sram); > } > > static Property ppc405_soc_properties[] = { > DEFINE_PROP_LINK("dram", Ppc405SoCState, dram_mr, TYPE_MEMORY_REGION, > MemoryRegion *), > + DEFINE_PROP_UINT32("sys-clk", Ppc405SoCState, sysclk, 0), > + DEFINE_PROP_BOOL("dram-init", Ppc405SoCState, do_dram_init, 0), > DEFINE_PROP_UINT64("ram-size", Ppc405SoCState, ram_size, 0), > DEFINE_PROP_END_OF_LIST(), > };
On Tue, 2 Aug 2022, Daniel Henrique Barboza wrote: > On 8/1/22 10:10, Cédric Le Goater wrote: >> This moves all the code previously done in the ppc405ep_init() routine >> under ppc405_soc_realize(). >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> hw/ppc/ppc405.h | 12 ++-- >> hw/ppc/ppc405_boards.c | 12 ++-- >> hw/ppc/ppc405_uc.c | 151 ++++++++++++++++++++--------------------- >> 3 files changed, 84 insertions(+), 91 deletions(-) >> >> diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h >> index c8cddb71733a..5e4e96d86ceb 100644 >> --- a/hw/ppc/ppc405.h >> +++ b/hw/ppc/ppc405.h >> @@ -74,9 +74,14 @@ struct Ppc405SoCState { >> MemoryRegion sram; >> MemoryRegion ram_memories[2]; >> hwaddr ram_bases[2], ram_sizes[2]; >> + bool do_dram_init; >> MemoryRegion *dram_mr; >> hwaddr ram_size; >> + >> + uint32_t sysclk; >> + PowerPCCPU *cpu; >> + DeviceState *uic; >> }; >> /* PowerPC 405 core */ >> @@ -85,11 +90,4 @@ ram_addr_t ppc405_set_bootinfo(CPUPPCState *env, >> ram_addr_t ram_size); >> void ppc4xx_plb_init(CPUPPCState *env); >> void ppc405_ebc_init(CPUPPCState *env); >> -PowerPCCPU *ppc405ep_init(MemoryRegion *address_space_mem, >> - MemoryRegion ram_memories[2], >> - hwaddr ram_bases[2], >> - hwaddr ram_sizes[2], >> - uint32_t sysclk, DeviceState **uicdev, >> - int do_init); >> - >> #endif /* PPC405_H */ >> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c >> index 96db52c5a309..363cb0770506 100644 >> --- a/hw/ppc/ppc405_boards.c >> +++ b/hw/ppc/ppc405_boards.c >> @@ -237,9 +237,7 @@ static void ppc405_init(MachineState *machine) >> Ppc405MachineState *ppc405 = PPC405_MACHINE(machine); >> MachineClass *mc = MACHINE_GET_CLASS(machine); >> const char *kernel_filename = machine->kernel_filename; >> - PowerPCCPU *cpu; >> MemoryRegion *sysmem = get_system_memory(); >> - DeviceState *uicdev; >> if (machine->ram_size != mc->default_ram_size) { >> char *sz = size_to_str(mc->default_ram_size); >> @@ -254,12 +252,12 @@ static void ppc405_init(MachineState *machine) >> machine->ram_size, &error_fatal); >> object_property_set_link(OBJECT(&ppc405->soc), "dram", >> OBJECT(machine->ram), &error_abort); >> + object_property_set_bool(OBJECT(&ppc405->soc), "dram-init", >> + !(kernel_filename == NULL), &error_abort); >> + object_property_set_uint(OBJECT(&ppc405->soc), "sys-clk", 33333333, >> + &error_abort); >> qdev_realize(DEVICE(&ppc405->soc), NULL, &error_abort); >> - cpu = ppc405ep_init(sysmem, ppc405->soc.ram_memories, >> ppc405->soc.ram_bases, >> - ppc405->soc.ram_sizes, >> - 33333333, &uicdev, kernel_filename == NULL ? 0 : >> 1); >> - >> /* allocate and load BIOS */ >> if (machine->firmware) { >> MemoryRegion *bios = g_new(MemoryRegion, 1); >> @@ -315,7 +313,7 @@ static void ppc405_init(MachineState *machine) >> /* Load ELF kernel and rootfs.cpio */ >> } else if (kernel_filename && !machine->firmware) { >> - boot_from_kernel(machine, cpu); >> + boot_from_kernel(machine, ppc405->soc.cpu); >> } >> } >> diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c >> index 156e839b8283..59612504bf3f 100644 >> --- a/hw/ppc/ppc405_uc.c >> +++ b/hw/ppc/ppc405_uc.c >> @@ -1432,134 +1432,131 @@ static void ppc405ep_cpc_init (CPUPPCState *env, >> clk_setup_t clk_setup[8], >> #endif >> } >> -PowerPCCPU *ppc405ep_init(MemoryRegion *address_space_mem, >> - MemoryRegion ram_memories[2], >> - hwaddr ram_bases[2], >> - hwaddr ram_sizes[2], >> - uint32_t sysclk, DeviceState **uicdevp, >> - int do_init) >> +static void ppc405_soc_realize(DeviceState *dev, Error **errp) >> { >> + Ppc405SoCState *s = PPC405_SOC(dev); >> clk_setup_t clk_setup[PPC405EP_CLK_NB], tlb_clk_setup; >> qemu_irq dma_irqs[4], gpt_irqs[5], mal_irqs[4]; >> - PowerPCCPU *cpu; >> CPUPPCState *env; >> - DeviceState *uicdev; >> - SysBusDevice *uicsbd; >> + Error *err = NULL; >> + >> + /* XXX: fix this ? */ > > So, this comment, originally from ppc405_boards.c, was added by commit > 1a6c088620368 and it seemed to make reference to something with the refering > to the ram_* values: > > > /* XXX: fix this */ > ram_bases[0] = 0x00000000; > ram_sizes[0] = 0x08000000; > ram_bases[1] = 0x00000000; > ram_sizes[1] = 0x00000000; > (...) > > > No more context is provided aside from a git-svn-id from savannah.nongnu.org. > > If no one can provide more context about what is to be fixed here, I'll > remove the comment. I'm not sure about it because I don't know 405 and only vaguely remember how this was on 440/460EX but I think it might be that the memory controller can be programmed to map RAM to different places but we don't fully emulate that nor the different chunks/DIMM sockets that could be possible and just map all system RAM to address 0 which is what most guest firmware or OS does anyway. Maybe I'm wrong and don't remember this correctly, the SDRAM controller model in ppc4xx_devs.c seems to do some mapping but I think this is what the comment might refer to or something similar. If so, I don't think it's worth emulating this more precisely unless we know about a guest which needs this. Regards, BALATON Zoltan
On 8/2/22 23:24, BALATON Zoltan wrote: > On Tue, 2 Aug 2022, Daniel Henrique Barboza wrote: >> On 8/1/22 10:10, Cédric Le Goater wrote: >>> This moves all the code previously done in the ppc405ep_init() routine >>> under ppc405_soc_realize(). >>> >>> Signed-off-by: Cédric Le Goater <clg@kaod.org> >>> --- >>> hw/ppc/ppc405.h | 12 ++-- >>> hw/ppc/ppc405_boards.c | 12 ++-- >>> hw/ppc/ppc405_uc.c | 151 ++++++++++++++++++++--------------------- >>> 3 files changed, 84 insertions(+), 91 deletions(-) >>> >>> diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h >>> index c8cddb71733a..5e4e96d86ceb 100644 >>> --- a/hw/ppc/ppc405.h >>> +++ b/hw/ppc/ppc405.h >>> @@ -74,9 +74,14 @@ struct Ppc405SoCState { >>> MemoryRegion sram; >>> MemoryRegion ram_memories[2]; >>> hwaddr ram_bases[2], ram_sizes[2]; >>> + bool do_dram_init; >>> MemoryRegion *dram_mr; >>> hwaddr ram_size; >>> + >>> + uint32_t sysclk; >>> + PowerPCCPU *cpu; >>> + DeviceState *uic; >>> }; >>> /* PowerPC 405 core */ >>> @@ -85,11 +90,4 @@ ram_addr_t ppc405_set_bootinfo(CPUPPCState *env, ram_addr_t ram_size); >>> void ppc4xx_plb_init(CPUPPCState *env); >>> void ppc405_ebc_init(CPUPPCState *env); >>> -PowerPCCPU *ppc405ep_init(MemoryRegion *address_space_mem, >>> - MemoryRegion ram_memories[2], >>> - hwaddr ram_bases[2], >>> - hwaddr ram_sizes[2], >>> - uint32_t sysclk, DeviceState **uicdev, >>> - int do_init); >>> - >>> #endif /* PPC405_H */ >>> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c >>> index 96db52c5a309..363cb0770506 100644 >>> --- a/hw/ppc/ppc405_boards.c >>> +++ b/hw/ppc/ppc405_boards.c >>> @@ -237,9 +237,7 @@ static void ppc405_init(MachineState *machine) >>> Ppc405MachineState *ppc405 = PPC405_MACHINE(machine); >>> MachineClass *mc = MACHINE_GET_CLASS(machine); >>> const char *kernel_filename = machine->kernel_filename; >>> - PowerPCCPU *cpu; >>> MemoryRegion *sysmem = get_system_memory(); >>> - DeviceState *uicdev; >>> if (machine->ram_size != mc->default_ram_size) { >>> char *sz = size_to_str(mc->default_ram_size); >>> @@ -254,12 +252,12 @@ static void ppc405_init(MachineState *machine) >>> machine->ram_size, &error_fatal); >>> object_property_set_link(OBJECT(&ppc405->soc), "dram", >>> OBJECT(machine->ram), &error_abort); >>> + object_property_set_bool(OBJECT(&ppc405->soc), "dram-init", >>> + !(kernel_filename == NULL), &error_abort); >>> + object_property_set_uint(OBJECT(&ppc405->soc), "sys-clk", 33333333, >>> + &error_abort); >>> qdev_realize(DEVICE(&ppc405->soc), NULL, &error_abort); >>> - cpu = ppc405ep_init(sysmem, ppc405->soc.ram_memories, ppc405->soc.ram_bases, >>> - ppc405->soc.ram_sizes, >>> - 33333333, &uicdev, kernel_filename == NULL ? 0 : 1); >>> - >>> /* allocate and load BIOS */ >>> if (machine->firmware) { >>> MemoryRegion *bios = g_new(MemoryRegion, 1); >>> @@ -315,7 +313,7 @@ static void ppc405_init(MachineState *machine) >>> /* Load ELF kernel and rootfs.cpio */ >>> } else if (kernel_filename && !machine->firmware) { >>> - boot_from_kernel(machine, cpu); >>> + boot_from_kernel(machine, ppc405->soc.cpu); >>> } >>> } >>> diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c >>> index 156e839b8283..59612504bf3f 100644 >>> --- a/hw/ppc/ppc405_uc.c >>> +++ b/hw/ppc/ppc405_uc.c >>> @@ -1432,134 +1432,131 @@ static void ppc405ep_cpc_init (CPUPPCState *env, clk_setup_t clk_setup[8], >>> #endif >>> } >>> -PowerPCCPU *ppc405ep_init(MemoryRegion *address_space_mem, >>> - MemoryRegion ram_memories[2], >>> - hwaddr ram_bases[2], >>> - hwaddr ram_sizes[2], >>> - uint32_t sysclk, DeviceState **uicdevp, >>> - int do_init) >>> +static void ppc405_soc_realize(DeviceState *dev, Error **errp) >>> { >>> + Ppc405SoCState *s = PPC405_SOC(dev); >>> clk_setup_t clk_setup[PPC405EP_CLK_NB], tlb_clk_setup; >>> qemu_irq dma_irqs[4], gpt_irqs[5], mal_irqs[4]; >>> - PowerPCCPU *cpu; >>> CPUPPCState *env; >>> - DeviceState *uicdev; >>> - SysBusDevice *uicsbd; >>> + Error *err = NULL; >>> + >>> + /* XXX: fix this ? */ >> >> So, this comment, originally from ppc405_boards.c, was added by commit >> 1a6c088620368 and it seemed to make reference to something with the refering >> to the ram_* values: >> >> >> /* XXX: fix this */ >> ram_bases[0] = 0x00000000; >> ram_sizes[0] = 0x08000000; >> ram_bases[1] = 0x00000000; >> ram_sizes[1] = 0x00000000; >> (...) >> >> >> No more context is provided aside from a git-svn-id from savannah.nongnu.org. >> >> If no one can provide more context about what is to be fixed here, I'll >> remove the comment. > > I'm not sure about it because I don't know 405 and only vaguely remember how this was on 440/460EX but I think it might be that the memory controller can be programmed to map RAM to different places Yes. See : https://gitlab.com/huth/u-boot/-/blob/taihu/arch/powerpc/cpu/ppc4xx/sdram.c Thomas had to hack the SDRAM init sequence for the QEMU machine to boot. > but we don't fully emulate that nor the different chunks/DIMM sockets that could be possible and just map all system RAM to address 0 which is what most guest firmware or OS does anyway. It feels like it. At least for the 405, only one bank is used : 0000000000000000-ffffffffffffffff (prio 0, i/o): system 0000000000000000-0000000007ffffff (prio 0, i/o): sdram-containers 0000000000000000-0000000007ffffff (prio 0, ram): alias ef405ep.ram.alias @ppc405.ram 0000000000000000-0000000007ffffff The machine allocates a set of memory regions which are aliases on the machine RAM. These are passed to the SDRAM controller model which maps and unmaps container regions depending on the register settings. It could be simplified. Thanks, C. > Maybe I'm wrong and don't remember this correctly, the SDRAM controller model in ppc4xx_devs.c seems to do some mapping but I think this is what the comment might refer to or something similar. If so, I don't think it's worth emulating this more precisely unless we know about a guest which needs this. > > Regards, > BALATON Zoltan
On 8/2/22 21:18, Daniel Henrique Barboza wrote: > > > On 8/1/22 10:10, Cédric Le Goater wrote: >> This moves all the code previously done in the ppc405ep_init() routine >> under ppc405_soc_realize(). >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> hw/ppc/ppc405.h | 12 ++-- >> hw/ppc/ppc405_boards.c | 12 ++-- >> hw/ppc/ppc405_uc.c | 151 ++++++++++++++++++++--------------------- >> 3 files changed, 84 insertions(+), 91 deletions(-) >> >> diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h >> index c8cddb71733a..5e4e96d86ceb 100644 >> --- a/hw/ppc/ppc405.h >> +++ b/hw/ppc/ppc405.h >> @@ -74,9 +74,14 @@ struct Ppc405SoCState { >> MemoryRegion sram; >> MemoryRegion ram_memories[2]; >> hwaddr ram_bases[2], ram_sizes[2]; >> + bool do_dram_init; >> MemoryRegion *dram_mr; >> hwaddr ram_size; >> + >> + uint32_t sysclk; >> + PowerPCCPU *cpu; >> + DeviceState *uic; >> }; >> /* PowerPC 405 core */ >> @@ -85,11 +90,4 @@ ram_addr_t ppc405_set_bootinfo(CPUPPCState *env, ram_addr_t ram_size); >> void ppc4xx_plb_init(CPUPPCState *env); >> void ppc405_ebc_init(CPUPPCState *env); >> -PowerPCCPU *ppc405ep_init(MemoryRegion *address_space_mem, >> - MemoryRegion ram_memories[2], >> - hwaddr ram_bases[2], >> - hwaddr ram_sizes[2], >> - uint32_t sysclk, DeviceState **uicdev, >> - int do_init); >> - >> #endif /* PPC405_H */ >> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c >> index 96db52c5a309..363cb0770506 100644 >> --- a/hw/ppc/ppc405_boards.c >> +++ b/hw/ppc/ppc405_boards.c >> @@ -237,9 +237,7 @@ static void ppc405_init(MachineState *machine) >> Ppc405MachineState *ppc405 = PPC405_MACHINE(machine); >> MachineClass *mc = MACHINE_GET_CLASS(machine); >> const char *kernel_filename = machine->kernel_filename; >> - PowerPCCPU *cpu; >> MemoryRegion *sysmem = get_system_memory(); >> - DeviceState *uicdev; >> if (machine->ram_size != mc->default_ram_size) { >> char *sz = size_to_str(mc->default_ram_size); >> @@ -254,12 +252,12 @@ static void ppc405_init(MachineState *machine) >> machine->ram_size, &error_fatal); >> object_property_set_link(OBJECT(&ppc405->soc), "dram", >> OBJECT(machine->ram), &error_abort); >> + object_property_set_bool(OBJECT(&ppc405->soc), "dram-init", >> + !(kernel_filename == NULL), &error_abort); >> + object_property_set_uint(OBJECT(&ppc405->soc), "sys-clk", 33333333, >> + &error_abort); >> qdev_realize(DEVICE(&ppc405->soc), NULL, &error_abort); >> - cpu = ppc405ep_init(sysmem, ppc405->soc.ram_memories, ppc405->soc.ram_bases, >> - ppc405->soc.ram_sizes, >> - 33333333, &uicdev, kernel_filename == NULL ? 0 : 1); >> - >> /* allocate and load BIOS */ >> if (machine->firmware) { >> MemoryRegion *bios = g_new(MemoryRegion, 1); >> @@ -315,7 +313,7 @@ static void ppc405_init(MachineState *machine) >> /* Load ELF kernel and rootfs.cpio */ >> } else if (kernel_filename && !machine->firmware) { >> - boot_from_kernel(machine, cpu); >> + boot_from_kernel(machine, ppc405->soc.cpu); >> } >> } >> diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c >> index 156e839b8283..59612504bf3f 100644 >> --- a/hw/ppc/ppc405_uc.c >> +++ b/hw/ppc/ppc405_uc.c >> @@ -1432,134 +1432,131 @@ static void ppc405ep_cpc_init (CPUPPCState *env, clk_setup_t clk_setup[8], >> #endif >> } >> -PowerPCCPU *ppc405ep_init(MemoryRegion *address_space_mem, >> - MemoryRegion ram_memories[2], >> - hwaddr ram_bases[2], >> - hwaddr ram_sizes[2], >> - uint32_t sysclk, DeviceState **uicdevp, >> - int do_init) >> +static void ppc405_soc_realize(DeviceState *dev, Error **errp) >> { >> + Ppc405SoCState *s = PPC405_SOC(dev); >> clk_setup_t clk_setup[PPC405EP_CLK_NB], tlb_clk_setup; >> qemu_irq dma_irqs[4], gpt_irqs[5], mal_irqs[4]; >> - PowerPCCPU *cpu; >> CPUPPCState *env; >> - DeviceState *uicdev; >> - SysBusDevice *uicsbd; >> + Error *err = NULL; >> + >> + /* XXX: fix this ? */ > > So, this comment, originally from ppc405_boards.c, was added by commit > 1a6c088620368 and it seemed to make reference to something with the refering > to the ram_* values: > > > /* XXX: fix this */ > ram_bases[0] = 0x00000000; > ram_sizes[0] = 0x08000000; > ram_bases[1] = 0x00000000; > ram_sizes[1] = 0x00000000; > (...) > > > No more context is provided aside from a git-svn-id from savannah.nongnu.org. > > If no one can provide more context about what is to be fixed here, I'll > remove the comment. > > > >> + memory_region_init_alias(&s->ram_memories[0], OBJECT(s), >> + "ef405ep.ram.alias", s->dram_mr, 0, s->ram_size); > > As I mentioned in patch 2, ef405ep.ram.alias can be renamed to ppc405.ram.alias ... sure. > >> + s->ram_bases[0] = 0; >> + s->ram_sizes[0] = s->ram_size; >> + memory_region_init(&s->ram_memories[1], OBJECT(s), "ef405ep.ram1", 0); > > And this can be renamed to ef405ep.ram1. If you agree with the rename I > can amend it in the tree. I think we can do better and simply remove the second bank. it is unused ... I have patches QOMifying ppc4xx_sdram_init() but the current modelling makes things a bit complex, specially ppc4xx_sdram_banks() which is only used on the bamboo and sam460ex machines. C. > > > > Thanks, > > > Daniel > >> + s->ram_bases[1] = 0x00000000; >> + s->ram_sizes[1] = 0x00000000; >> + >> + /* allocate SRAM */ >> + memory_region_init_ram(&s->sram, OBJECT(s), "ef405ep.sram", >> + PPC405EP_SRAM_SIZE, &err); >> + if (err) { >> + error_propagate(errp, err); >> + return; >> + } >> + memory_region_add_subregion(get_system_memory(), PPC405EP_SRAM_BASE, >> + &s->sram); >> memset(clk_setup, 0, sizeof(clk_setup)); >> + >> /* init CPUs */ >> - cpu = ppc4xx_init(POWERPC_CPU_TYPE_NAME("405ep"), >> + s->cpu = ppc4xx_init(POWERPC_CPU_TYPE_NAME("405ep"), >> &clk_setup[PPC405EP_CPU_CLK], >> - &tlb_clk_setup, sysclk); >> - env = &cpu->env; >> + &tlb_clk_setup, s->sysclk); >> + env = &s->cpu->env; >> clk_setup[PPC405EP_CPU_CLK].cb = tlb_clk_setup.cb; >> clk_setup[PPC405EP_CPU_CLK].opaque = tlb_clk_setup.opaque; >> - /* Internal devices init */ >> - /* Memory mapped devices registers */ >> + >> + /* CPU control */ >> + ppc405ep_cpc_init(env, clk_setup, s->sysclk); >> + >> /* PLB arbitrer */ >> ppc4xx_plb_init(env); >> + >> /* PLB to OPB bridge */ >> ppc4xx_pob_init(env); >> + >> /* OBP arbitrer */ >> ppc4xx_opba_init(0xef600600); >> + >> /* Universal interrupt controller */ >> - uicdev = qdev_new(TYPE_PPC_UIC); >> - uicsbd = SYS_BUS_DEVICE(uicdev); >> + s->uic = qdev_new(TYPE_PPC_UIC); >> - object_property_set_link(OBJECT(uicdev), "cpu", OBJECT(cpu), >> + object_property_set_link(OBJECT(s->uic), "cpu", OBJECT(s->cpu), >> &error_fatal); >> - sysbus_realize_and_unref(uicsbd, &error_fatal); >> - >> - sysbus_connect_irq(uicsbd, PPCUIC_OUTPUT_INT, >> - qdev_get_gpio_in(DEVICE(cpu), PPC40x_INPUT_INT)); >> - sysbus_connect_irq(uicsbd, PPCUIC_OUTPUT_CINT, >> - qdev_get_gpio_in(DEVICE(cpu), PPC40x_INPUT_CINT)); >> + if (!sysbus_realize(SYS_BUS_DEVICE(s->uic), errp)) { >> + return; >> + } >> - *uicdevp = uicdev; >> + sysbus_connect_irq(SYS_BUS_DEVICE(s->uic), PPCUIC_OUTPUT_INT, >> + qdev_get_gpio_in(DEVICE(s->cpu), PPC40x_INPUT_INT)); >> + sysbus_connect_irq(SYS_BUS_DEVICE(s->uic), PPCUIC_OUTPUT_CINT, >> + qdev_get_gpio_in(DEVICE(s->cpu), PPC40x_INPUT_CINT)); >> /* SDRAM controller */ >> - /* XXX 405EP has no ECC interrupt */ >> - ppc4xx_sdram_init(env, qdev_get_gpio_in(uicdev, 17), 2, ram_memories, >> - ram_bases, ram_sizes, do_init); >> + /* XXX 405EP has no ECC interrupt */ >> + ppc4xx_sdram_init(env, qdev_get_gpio_in(s->uic, 17), 2, s->ram_memories, >> + s->ram_bases, s->ram_sizes, s->do_dram_init); >> + >> /* External bus controller */ >> ppc405_ebc_init(env); >> + >> /* DMA controller */ >> - dma_irqs[0] = qdev_get_gpio_in(uicdev, 5); >> - dma_irqs[1] = qdev_get_gpio_in(uicdev, 6); >> - dma_irqs[2] = qdev_get_gpio_in(uicdev, 7); >> - dma_irqs[3] = qdev_get_gpio_in(uicdev, 8); >> + dma_irqs[0] = qdev_get_gpio_in(s->uic, 5); >> + dma_irqs[1] = qdev_get_gpio_in(s->uic, 6); >> + dma_irqs[2] = qdev_get_gpio_in(s->uic, 7); >> + dma_irqs[3] = qdev_get_gpio_in(s->uic, 8); >> ppc405_dma_init(env, dma_irqs); >> - /* IIC controller */ >> + >> + /* I2C controller */ >> sysbus_create_simple(TYPE_PPC4xx_I2C, 0xef600500, >> - qdev_get_gpio_in(uicdev, 2)); >> + qdev_get_gpio_in(s->uic, 2)); >> /* GPIO */ >> ppc405_gpio_init(0xef600700); >> + >> /* Serial ports */ >> if (serial_hd(0) != NULL) { >> - serial_mm_init(address_space_mem, 0xef600300, 0, >> - qdev_get_gpio_in(uicdev, 0), >> + serial_mm_init(get_system_memory(), 0xef600300, 0, >> + qdev_get_gpio_in(s->uic, 0), >> PPC_SERIAL_MM_BAUDBASE, serial_hd(0), >> DEVICE_BIG_ENDIAN); >> } >> if (serial_hd(1) != NULL) { >> - serial_mm_init(address_space_mem, 0xef600400, 0, >> - qdev_get_gpio_in(uicdev, 1), >> + serial_mm_init(get_system_memory(), 0xef600400, 0, >> + qdev_get_gpio_in(s->uic, 1), >> PPC_SERIAL_MM_BAUDBASE, serial_hd(1), >> DEVICE_BIG_ENDIAN); >> } >> + >> /* OCM */ >> ppc405_ocm_init(env); >> + >> /* GPT */ >> - gpt_irqs[0] = qdev_get_gpio_in(uicdev, 19); >> - gpt_irqs[1] = qdev_get_gpio_in(uicdev, 20); >> - gpt_irqs[2] = qdev_get_gpio_in(uicdev, 21); >> - gpt_irqs[3] = qdev_get_gpio_in(uicdev, 22); >> - gpt_irqs[4] = qdev_get_gpio_in(uicdev, 23); >> + gpt_irqs[0] = qdev_get_gpio_in(s->uic, 19); >> + gpt_irqs[1] = qdev_get_gpio_in(s->uic, 20); >> + gpt_irqs[2] = qdev_get_gpio_in(s->uic, 21); >> + gpt_irqs[3] = qdev_get_gpio_in(s->uic, 22); >> + gpt_irqs[4] = qdev_get_gpio_in(s->uic, 23); >> ppc4xx_gpt_init(0xef600000, gpt_irqs); >> - /* PCI */ >> - /* Uses UIC IRQs 3, 16, 18 */ >> + >> /* MAL */ >> - mal_irqs[0] = qdev_get_gpio_in(uicdev, 11); >> - mal_irqs[1] = qdev_get_gpio_in(uicdev, 12); >> - mal_irqs[2] = qdev_get_gpio_in(uicdev, 13); >> - mal_irqs[3] = qdev_get_gpio_in(uicdev, 14); >> + mal_irqs[0] = qdev_get_gpio_in(s->uic, 11); >> + mal_irqs[1] = qdev_get_gpio_in(s->uic, 12); >> + mal_irqs[2] = qdev_get_gpio_in(s->uic, 13); >> + mal_irqs[3] = qdev_get_gpio_in(s->uic, 14); >> ppc4xx_mal_init(env, 4, 2, mal_irqs); >> + >> /* Ethernet */ >> /* Uses UIC IRQs 9, 15, 17 */ >> - /* CPU control */ >> - ppc405ep_cpc_init(env, clk_setup, sysclk); >> - >> - return cpu; >> -} >> - >> -static void ppc405_soc_realize(DeviceState *dev, Error **errp) >> -{ >> - Ppc405SoCState *s = PPC405_SOC(dev); >> - Error *err = NULL; >> - >> - /* XXX: fix this ? */ >> - memory_region_init_alias(&s->ram_memories[0], OBJECT(s), >> - "ef405ep.ram.alias", s->dram_mr, 0, s->ram_size); >> - s->ram_bases[0] = 0; >> - s->ram_sizes[0] = s->ram_size; >> - memory_region_init(&s->ram_memories[1], OBJECT(s), "ef405ep.ram1", 0); >> - s->ram_bases[1] = 0x00000000; >> - s->ram_sizes[1] = 0x00000000; >> - >> - /* allocate SRAM */ >> - memory_region_init_ram(&s->sram, OBJECT(s), "ef405ep.sram", >> - PPC405EP_SRAM_SIZE, &err); >> - if (err) { >> - error_propagate(errp, err); >> - return; >> - } >> - memory_region_add_subregion(get_system_memory(), PPC405EP_SRAM_BASE, >> - &s->sram); >> } >> static Property ppc405_soc_properties[] = { >> DEFINE_PROP_LINK("dram", Ppc405SoCState, dram_mr, TYPE_MEMORY_REGION, >> MemoryRegion *), >> + DEFINE_PROP_UINT32("sys-clk", Ppc405SoCState, sysclk, 0), >> + DEFINE_PROP_BOOL("dram-init", Ppc405SoCState, do_dram_init, 0), >> DEFINE_PROP_UINT64("ram-size", Ppc405SoCState, ram_size, 0), >> DEFINE_PROP_END_OF_LIST(), >> };
On 8/2/22 18:24, BALATON Zoltan wrote: > On Tue, 2 Aug 2022, Daniel Henrique Barboza wrote: >> On 8/1/22 10:10, Cédric Le Goater wrote: >>> This moves all the code previously done in the ppc405ep_init() routine >>> under ppc405_soc_realize(). >>> >>> Signed-off-by: Cédric Le Goater <clg@kaod.org> >>> --- >>> hw/ppc/ppc405.h | 12 ++-- >>> hw/ppc/ppc405_boards.c | 12 ++-- >>> hw/ppc/ppc405_uc.c | 151 ++++++++++++++++++++--------------------- >>> 3 files changed, 84 insertions(+), 91 deletions(-) >>> >>> diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h >>> index c8cddb71733a..5e4e96d86ceb 100644 >>> --- a/hw/ppc/ppc405.h >>> +++ b/hw/ppc/ppc405.h >>> @@ -74,9 +74,14 @@ struct Ppc405SoCState { >>> MemoryRegion sram; >>> MemoryRegion ram_memories[2]; >>> hwaddr ram_bases[2], ram_sizes[2]; >>> + bool do_dram_init; >>> MemoryRegion *dram_mr; >>> hwaddr ram_size; >>> + >>> + uint32_t sysclk; >>> + PowerPCCPU *cpu; >>> + DeviceState *uic; >>> }; >>> /* PowerPC 405 core */ >>> @@ -85,11 +90,4 @@ ram_addr_t ppc405_set_bootinfo(CPUPPCState *env, ram_addr_t ram_size); >>> void ppc4xx_plb_init(CPUPPCState *env); >>> void ppc405_ebc_init(CPUPPCState *env); >>> -PowerPCCPU *ppc405ep_init(MemoryRegion *address_space_mem, >>> - MemoryRegion ram_memories[2], >>> - hwaddr ram_bases[2], >>> - hwaddr ram_sizes[2], >>> - uint32_t sysclk, DeviceState **uicdev, >>> - int do_init); >>> - >>> #endif /* PPC405_H */ >>> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c >>> index 96db52c5a309..363cb0770506 100644 >>> --- a/hw/ppc/ppc405_boards.c >>> +++ b/hw/ppc/ppc405_boards.c >>> @@ -237,9 +237,7 @@ static void ppc405_init(MachineState *machine) >>> Ppc405MachineState *ppc405 = PPC405_MACHINE(machine); >>> MachineClass *mc = MACHINE_GET_CLASS(machine); >>> const char *kernel_filename = machine->kernel_filename; >>> - PowerPCCPU *cpu; >>> MemoryRegion *sysmem = get_system_memory(); >>> - DeviceState *uicdev; >>> if (machine->ram_size != mc->default_ram_size) { >>> char *sz = size_to_str(mc->default_ram_size); >>> @@ -254,12 +252,12 @@ static void ppc405_init(MachineState *machine) >>> machine->ram_size, &error_fatal); >>> object_property_set_link(OBJECT(&ppc405->soc), "dram", >>> OBJECT(machine->ram), &error_abort); >>> + object_property_set_bool(OBJECT(&ppc405->soc), "dram-init", >>> + !(kernel_filename == NULL), &error_abort); >>> + object_property_set_uint(OBJECT(&ppc405->soc), "sys-clk", 33333333, >>> + &error_abort); >>> qdev_realize(DEVICE(&ppc405->soc), NULL, &error_abort); >>> - cpu = ppc405ep_init(sysmem, ppc405->soc.ram_memories, ppc405->soc.ram_bases, >>> - ppc405->soc.ram_sizes, >>> - 33333333, &uicdev, kernel_filename == NULL ? 0 : 1); >>> - >>> /* allocate and load BIOS */ >>> if (machine->firmware) { >>> MemoryRegion *bios = g_new(MemoryRegion, 1); >>> @@ -315,7 +313,7 @@ static void ppc405_init(MachineState *machine) >>> /* Load ELF kernel and rootfs.cpio */ >>> } else if (kernel_filename && !machine->firmware) { >>> - boot_from_kernel(machine, cpu); >>> + boot_from_kernel(machine, ppc405->soc.cpu); >>> } >>> } >>> diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c >>> index 156e839b8283..59612504bf3f 100644 >>> --- a/hw/ppc/ppc405_uc.c >>> +++ b/hw/ppc/ppc405_uc.c >>> @@ -1432,134 +1432,131 @@ static void ppc405ep_cpc_init (CPUPPCState *env, clk_setup_t clk_setup[8], >>> #endif >>> } >>> -PowerPCCPU *ppc405ep_init(MemoryRegion *address_space_mem, >>> - MemoryRegion ram_memories[2], >>> - hwaddr ram_bases[2], >>> - hwaddr ram_sizes[2], >>> - uint32_t sysclk, DeviceState **uicdevp, >>> - int do_init) >>> +static void ppc405_soc_realize(DeviceState *dev, Error **errp) >>> { >>> + Ppc405SoCState *s = PPC405_SOC(dev); >>> clk_setup_t clk_setup[PPC405EP_CLK_NB], tlb_clk_setup; >>> qemu_irq dma_irqs[4], gpt_irqs[5], mal_irqs[4]; >>> - PowerPCCPU *cpu; >>> CPUPPCState *env; >>> - DeviceState *uicdev; >>> - SysBusDevice *uicsbd; >>> + Error *err = NULL; >>> + >>> + /* XXX: fix this ? */ >> >> So, this comment, originally from ppc405_boards.c, was added by commit >> 1a6c088620368 and it seemed to make reference to something with the refering >> to the ram_* values: >> >> >> /* XXX: fix this */ >> ram_bases[0] = 0x00000000; >> ram_sizes[0] = 0x08000000; >> ram_bases[1] = 0x00000000; >> ram_sizes[1] = 0x00000000; >> (...) >> >> >> No more context is provided aside from a git-svn-id from savannah.nongnu.org. >> >> If no one can provide more context about what is to be fixed here, I'll >> remove the comment. > > I'm not sure about it because I don't know 405 and only vaguely remember how this was on 440/460EX but I think it might be that the memory controller can be programmed to map RAM to different places but we don't fully emulate that nor the different chunks/DIMM sockets that could be possible and just map all system RAM to address 0 which is what most guest firmware or OS does anyway. Maybe I'm wrong and don't remember this correctly, the SDRAM controller model in ppc4xx_devs.c seems to do some mapping but I think this is what the comment might refer to or something similar. If so, I don't think it's worth emulating this more precisely unless we know about a guest which needs this. Thanks for the explanation! I'm not sure if this is something worth documenting in this comment or not. I guess it wouldn't hurt to mention "the memory controller can map RAM in different sockets but we don't implement that" or something similar. Or just remove the comment entirely. Both works for me as long as we get rid of the 'fix this' comment. It implies that we're doing something wrong on purpose, which doesn't seem to be the case. Thanks, Daniel > > Regards, > BALATON Zoltan
On Wed, 3 Aug 2022, Cédric Le Goater wrote: > On 8/2/22 21:18, Daniel Henrique Barboza wrote: >> On 8/1/22 10:10, Cédric Le Goater wrote: >>> This moves all the code previously done in the ppc405ep_init() routine >>> under ppc405_soc_realize(). >>> >>> Signed-off-by: Cédric Le Goater <clg@kaod.org> >>> --- >>> hw/ppc/ppc405.h | 12 ++-- >>> hw/ppc/ppc405_boards.c | 12 ++-- >>> hw/ppc/ppc405_uc.c | 151 ++++++++++++++++++++--------------------- >>> 3 files changed, 84 insertions(+), 91 deletions(-) >>> >>> diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h >>> index c8cddb71733a..5e4e96d86ceb 100644 >>> --- a/hw/ppc/ppc405.h >>> +++ b/hw/ppc/ppc405.h >>> @@ -74,9 +74,14 @@ struct Ppc405SoCState { >>> MemoryRegion sram; >>> MemoryRegion ram_memories[2]; >>> hwaddr ram_bases[2], ram_sizes[2]; >>> + bool do_dram_init; >>> MemoryRegion *dram_mr; >>> hwaddr ram_size; >>> + >>> + uint32_t sysclk; >>> + PowerPCCPU *cpu; >>> + DeviceState *uic; >>> }; >>> /* PowerPC 405 core */ >>> @@ -85,11 +90,4 @@ ram_addr_t ppc405_set_bootinfo(CPUPPCState *env, >>> ram_addr_t ram_size); >>> void ppc4xx_plb_init(CPUPPCState *env); >>> void ppc405_ebc_init(CPUPPCState *env); >>> -PowerPCCPU *ppc405ep_init(MemoryRegion *address_space_mem, >>> - MemoryRegion ram_memories[2], >>> - hwaddr ram_bases[2], >>> - hwaddr ram_sizes[2], >>> - uint32_t sysclk, DeviceState **uicdev, >>> - int do_init); >>> - >>> #endif /* PPC405_H */ >>> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c >>> index 96db52c5a309..363cb0770506 100644 >>> --- a/hw/ppc/ppc405_boards.c >>> +++ b/hw/ppc/ppc405_boards.c >>> @@ -237,9 +237,7 @@ static void ppc405_init(MachineState *machine) >>> Ppc405MachineState *ppc405 = PPC405_MACHINE(machine); >>> MachineClass *mc = MACHINE_GET_CLASS(machine); >>> const char *kernel_filename = machine->kernel_filename; >>> - PowerPCCPU *cpu; >>> MemoryRegion *sysmem = get_system_memory(); >>> - DeviceState *uicdev; >>> if (machine->ram_size != mc->default_ram_size) { >>> char *sz = size_to_str(mc->default_ram_size); >>> @@ -254,12 +252,12 @@ static void ppc405_init(MachineState *machine) >>> machine->ram_size, &error_fatal); >>> object_property_set_link(OBJECT(&ppc405->soc), "dram", >>> OBJECT(machine->ram), &error_abort); >>> + object_property_set_bool(OBJECT(&ppc405->soc), "dram-init", >>> + !(kernel_filename == NULL), &error_abort); >>> + object_property_set_uint(OBJECT(&ppc405->soc), "sys-clk", 33333333, >>> + &error_abort); >>> qdev_realize(DEVICE(&ppc405->soc), NULL, &error_abort); >>> - cpu = ppc405ep_init(sysmem, ppc405->soc.ram_memories, >>> ppc405->soc.ram_bases, >>> - ppc405->soc.ram_sizes, >>> - 33333333, &uicdev, kernel_filename == NULL ? 0 : >>> 1); >>> - >>> /* allocate and load BIOS */ >>> if (machine->firmware) { >>> MemoryRegion *bios = g_new(MemoryRegion, 1); >>> @@ -315,7 +313,7 @@ static void ppc405_init(MachineState *machine) >>> /* Load ELF kernel and rootfs.cpio */ >>> } else if (kernel_filename && !machine->firmware) { >>> - boot_from_kernel(machine, cpu); >>> + boot_from_kernel(machine, ppc405->soc.cpu); >>> } >>> } >>> diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c >>> index 156e839b8283..59612504bf3f 100644 >>> --- a/hw/ppc/ppc405_uc.c >>> +++ b/hw/ppc/ppc405_uc.c >>> @@ -1432,134 +1432,131 @@ static void ppc405ep_cpc_init (CPUPPCState *env, >>> clk_setup_t clk_setup[8], >>> #endif >>> } >>> -PowerPCCPU *ppc405ep_init(MemoryRegion *address_space_mem, >>> - MemoryRegion ram_memories[2], >>> - hwaddr ram_bases[2], >>> - hwaddr ram_sizes[2], >>> - uint32_t sysclk, DeviceState **uicdevp, >>> - int do_init) >>> +static void ppc405_soc_realize(DeviceState *dev, Error **errp) >>> { >>> + Ppc405SoCState *s = PPC405_SOC(dev); >>> clk_setup_t clk_setup[PPC405EP_CLK_NB], tlb_clk_setup; >>> qemu_irq dma_irqs[4], gpt_irqs[5], mal_irqs[4]; >>> - PowerPCCPU *cpu; >>> CPUPPCState *env; >>> - DeviceState *uicdev; >>> - SysBusDevice *uicsbd; >>> + Error *err = NULL; >>> + >>> + /* XXX: fix this ? */ >> >> So, this comment, originally from ppc405_boards.c, was added by commit >> 1a6c088620368 and it seemed to make reference to something with the >> refering >> to the ram_* values: >> >> >> /* XXX: fix this */ >> ram_bases[0] = 0x00000000; >> ram_sizes[0] = 0x08000000; >> ram_bases[1] = 0x00000000; >> ram_sizes[1] = 0x00000000; >> (...) >> >> >> No more context is provided aside from a git-svn-id from >> savannah.nongnu.org. >> >> If no one can provide more context about what is to be fixed here, I'll >> remove the comment. >> >> >> >>> + memory_region_init_alias(&s->ram_memories[0], OBJECT(s), >>> + "ef405ep.ram.alias", s->dram_mr, 0, >>> s->ram_size); >> >> As I mentioned in patch 2, ef405ep.ram.alias can be renamed to >> ppc405.ram.alias ... > > sure. > >> >>> + s->ram_bases[0] = 0; >>> + s->ram_sizes[0] = s->ram_size; >>> + memory_region_init(&s->ram_memories[1], OBJECT(s), "ef405ep.ram1", >>> 0); >> >> And this can be renamed to ef405ep.ram1. If you agree with the rename I >> can amend it in the tree. > > > I think we can do better and simply remove the second bank. it is unused ... > > I have patches QOMifying ppc4xx_sdram_init() but the current modelling > makes things a bit complex, specially ppc4xx_sdram_banks() which is only > used on the bamboo and sam460ex machines. We need to model the SDRAM controller for the sam460ex firmware at least partially so it gets past the memory test and init. If you change it please test that sam460ex still boots with firmware. I'm not sure 405 and 440 use the same sdram controller though or if the 460EX has a different one as these may have been updated in later SoCs to support newer RAM standards (I think the 460EX has DDR2) and the QEMU model is a bit messy sharing components between 405 and 440. When I've changed some of these for 460EX I've tried to name them so that 4xx means shared by all, 44x or 440 means 440 specific and 40x or similar for older SoCs only but this is probably not always correct. I could not find a clean way to separate these. QOMifying would make sense when cleaning this up otherwise it's just adding more boilerplate without any clear advantage IMO. If you want change these maybe you should get the docs for the emulated chips and consult those for differences. Unfortunately the 460ex does not seem to have docs available but it's similar to earlier IBM parts like 440GP which generally have docs. That's what I've used but still couldn't find all parts like the PCIe conroller that I had to implement based on what the firmware and drivers do (and only did that partially so it boots as I did not fully understand how it works and how these are modelled in QEMU). Regards, BALATON Zoltan
On 8/3/22 13:59, BALATON Zoltan wrote: > On Wed, 3 Aug 2022, Cédric Le Goater wrote: >> On 8/2/22 21:18, Daniel Henrique Barboza wrote: >>> On 8/1/22 10:10, Cédric Le Goater wrote: >>>> This moves all the code previously done in the ppc405ep_init() routine >>>> under ppc405_soc_realize(). >>>> >>>> Signed-off-by: Cédric Le Goater <clg@kaod.org> >>>> --- >>>> hw/ppc/ppc405.h | 12 ++-- >>>> hw/ppc/ppc405_boards.c | 12 ++-- >>>> hw/ppc/ppc405_uc.c | 151 ++++++++++++++++++++--------------------- >>>> 3 files changed, 84 insertions(+), 91 deletions(-) >>>> >>>> diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h >>>> index c8cddb71733a..5e4e96d86ceb 100644 >>>> --- a/hw/ppc/ppc405.h >>>> +++ b/hw/ppc/ppc405.h >>>> @@ -74,9 +74,14 @@ struct Ppc405SoCState { >>>> MemoryRegion sram; >>>> MemoryRegion ram_memories[2]; >>>> hwaddr ram_bases[2], ram_sizes[2]; >>>> + bool do_dram_init; >>>> MemoryRegion *dram_mr; >>>> hwaddr ram_size; >>>> + >>>> + uint32_t sysclk; >>>> + PowerPCCPU *cpu; >>>> + DeviceState *uic; >>>> }; >>>> /* PowerPC 405 core */ >>>> @@ -85,11 +90,4 @@ ram_addr_t ppc405_set_bootinfo(CPUPPCState *env, ram_addr_t ram_size); >>>> void ppc4xx_plb_init(CPUPPCState *env); >>>> void ppc405_ebc_init(CPUPPCState *env); >>>> -PowerPCCPU *ppc405ep_init(MemoryRegion *address_space_mem, >>>> - MemoryRegion ram_memories[2], >>>> - hwaddr ram_bases[2], >>>> - hwaddr ram_sizes[2], >>>> - uint32_t sysclk, DeviceState **uicdev, >>>> - int do_init); >>>> - >>>> #endif /* PPC405_H */ >>>> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c >>>> index 96db52c5a309..363cb0770506 100644 >>>> --- a/hw/ppc/ppc405_boards.c >>>> +++ b/hw/ppc/ppc405_boards.c >>>> @@ -237,9 +237,7 @@ static void ppc405_init(MachineState *machine) >>>> Ppc405MachineState *ppc405 = PPC405_MACHINE(machine); >>>> MachineClass *mc = MACHINE_GET_CLASS(machine); >>>> const char *kernel_filename = machine->kernel_filename; >>>> - PowerPCCPU *cpu; >>>> MemoryRegion *sysmem = get_system_memory(); >>>> - DeviceState *uicdev; >>>> if (machine->ram_size != mc->default_ram_size) { >>>> char *sz = size_to_str(mc->default_ram_size); >>>> @@ -254,12 +252,12 @@ static void ppc405_init(MachineState *machine) >>>> machine->ram_size, &error_fatal); >>>> object_property_set_link(OBJECT(&ppc405->soc), "dram", >>>> OBJECT(machine->ram), &error_abort); >>>> + object_property_set_bool(OBJECT(&ppc405->soc), "dram-init", >>>> + !(kernel_filename == NULL), &error_abort); >>>> + object_property_set_uint(OBJECT(&ppc405->soc), "sys-clk", 33333333, >>>> + &error_abort); >>>> qdev_realize(DEVICE(&ppc405->soc), NULL, &error_abort); >>>> - cpu = ppc405ep_init(sysmem, ppc405->soc.ram_memories, ppc405->soc.ram_bases, >>>> - ppc405->soc.ram_sizes, >>>> - 33333333, &uicdev, kernel_filename == NULL ? 0 : 1); >>>> - >>>> /* allocate and load BIOS */ >>>> if (machine->firmware) { >>>> MemoryRegion *bios = g_new(MemoryRegion, 1); >>>> @@ -315,7 +313,7 @@ static void ppc405_init(MachineState *machine) >>>> /* Load ELF kernel and rootfs.cpio */ >>>> } else if (kernel_filename && !machine->firmware) { >>>> - boot_from_kernel(machine, cpu); >>>> + boot_from_kernel(machine, ppc405->soc.cpu); >>>> } >>>> } >>>> diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c >>>> index 156e839b8283..59612504bf3f 100644 >>>> --- a/hw/ppc/ppc405_uc.c >>>> +++ b/hw/ppc/ppc405_uc.c >>>> @@ -1432,134 +1432,131 @@ static void ppc405ep_cpc_init (CPUPPCState *env, clk_setup_t clk_setup[8], >>>> #endif >>>> } >>>> -PowerPCCPU *ppc405ep_init(MemoryRegion *address_space_mem, >>>> - MemoryRegion ram_memories[2], >>>> - hwaddr ram_bases[2], >>>> - hwaddr ram_sizes[2], >>>> - uint32_t sysclk, DeviceState **uicdevp, >>>> - int do_init) >>>> +static void ppc405_soc_realize(DeviceState *dev, Error **errp) >>>> { >>>> + Ppc405SoCState *s = PPC405_SOC(dev); >>>> clk_setup_t clk_setup[PPC405EP_CLK_NB], tlb_clk_setup; >>>> qemu_irq dma_irqs[4], gpt_irqs[5], mal_irqs[4]; >>>> - PowerPCCPU *cpu; >>>> CPUPPCState *env; >>>> - DeviceState *uicdev; >>>> - SysBusDevice *uicsbd; >>>> + Error *err = NULL; >>>> + >>>> + /* XXX: fix this ? */ >>> >>> So, this comment, originally from ppc405_boards.c, was added by commit >>> 1a6c088620368 and it seemed to make reference to something with the refering >>> to the ram_* values: >>> >>> >>> /* XXX: fix this */ >>> ram_bases[0] = 0x00000000; >>> ram_sizes[0] = 0x08000000; >>> ram_bases[1] = 0x00000000; >>> ram_sizes[1] = 0x00000000; >>> (...) >>> >>> >>> No more context is provided aside from a git-svn-id from savannah.nongnu.org. >>> >>> If no one can provide more context about what is to be fixed here, I'll >>> remove the comment. >>> >>> >>> >>>> + memory_region_init_alias(&s->ram_memories[0], OBJECT(s), >>>> + "ef405ep.ram.alias", s->dram_mr, 0, s->ram_size); >>> >>> As I mentioned in patch 2, ef405ep.ram.alias can be renamed to ppc405.ram.alias ... >> >> sure. >> >>> >>>> + s->ram_bases[0] = 0; >>>> + s->ram_sizes[0] = s->ram_size; >>>> + memory_region_init(&s->ram_memories[1], OBJECT(s), "ef405ep.ram1", 0); >>> >>> And this can be renamed to ef405ep.ram1. If you agree with the rename I >>> can amend it in the tree. >> >> >> I think we can do better and simply remove the second bank. it is unused ... >> >> I have patches QOMifying ppc4xx_sdram_init() but the current modelling >> makes things a bit complex, specially ppc4xx_sdram_banks() which is only >> used on the bamboo and sam460ex machines. > > We need to model the SDRAM controller for the sam460ex firmware at least partially so it gets past the memory test and init. If you change it please test that sam460ex still boots with firmware. Sure. QOM'ifying models reveals the implementation shortcuts. The ppc405 board was quite messy and it's getting better with the removal of ppc405ep_init(). ppc4xx_sdram_init() is the next step but it is quite localized in the SoC. No hurries. Thanks, C. > I'm not sure 405 and 440 use the same sdram controller though or if the 460EX has a different one as these may have been updated in later SoCs to support newer RAM standards (I think the 460EX has DDR2) and the QEMU model is a bit messy sharing components between 405 and 440. When I've changed some of these for 460EX I've tried to name them so that 4xx means shared by all, 44x or 440 means 440 specific and 40x or similar for older SoCs only but this is probably not always correct. I could not find a clean way to separate these. QOMifying would make sense when cleaning this up otherwise it's just adding more boilerplate without any clear advantage IMO. > > If you want change these maybe you should get the docs for the emulated chips and consult those for differences. Unfortunately the 460ex does not seem to have docs available but it's similar to earlier IBM parts like 440GP which generally have docs. That's what I've used but still couldn't find all parts like the PCIe conroller that I had to implement based on what the firmware and drivers do (and only did that partially so it boots as I did not fully understand how it works and how these are modelled in QEMU). > > Regards, > BALATON Zoltan
diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h index c8cddb71733a..5e4e96d86ceb 100644 --- a/hw/ppc/ppc405.h +++ b/hw/ppc/ppc405.h @@ -74,9 +74,14 @@ struct Ppc405SoCState { MemoryRegion sram; MemoryRegion ram_memories[2]; hwaddr ram_bases[2], ram_sizes[2]; + bool do_dram_init; MemoryRegion *dram_mr; hwaddr ram_size; + + uint32_t sysclk; + PowerPCCPU *cpu; + DeviceState *uic; }; /* PowerPC 405 core */ @@ -85,11 +90,4 @@ ram_addr_t ppc405_set_bootinfo(CPUPPCState *env, ram_addr_t ram_size); void ppc4xx_plb_init(CPUPPCState *env); void ppc405_ebc_init(CPUPPCState *env); -PowerPCCPU *ppc405ep_init(MemoryRegion *address_space_mem, - MemoryRegion ram_memories[2], - hwaddr ram_bases[2], - hwaddr ram_sizes[2], - uint32_t sysclk, DeviceState **uicdev, - int do_init); - #endif /* PPC405_H */ diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c index 96db52c5a309..363cb0770506 100644 --- a/hw/ppc/ppc405_boards.c +++ b/hw/ppc/ppc405_boards.c @@ -237,9 +237,7 @@ static void ppc405_init(MachineState *machine) Ppc405MachineState *ppc405 = PPC405_MACHINE(machine); MachineClass *mc = MACHINE_GET_CLASS(machine); const char *kernel_filename = machine->kernel_filename; - PowerPCCPU *cpu; MemoryRegion *sysmem = get_system_memory(); - DeviceState *uicdev; if (machine->ram_size != mc->default_ram_size) { char *sz = size_to_str(mc->default_ram_size); @@ -254,12 +252,12 @@ static void ppc405_init(MachineState *machine) machine->ram_size, &error_fatal); object_property_set_link(OBJECT(&ppc405->soc), "dram", OBJECT(machine->ram), &error_abort); + object_property_set_bool(OBJECT(&ppc405->soc), "dram-init", + !(kernel_filename == NULL), &error_abort); + object_property_set_uint(OBJECT(&ppc405->soc), "sys-clk", 33333333, + &error_abort); qdev_realize(DEVICE(&ppc405->soc), NULL, &error_abort); - cpu = ppc405ep_init(sysmem, ppc405->soc.ram_memories, ppc405->soc.ram_bases, - ppc405->soc.ram_sizes, - 33333333, &uicdev, kernel_filename == NULL ? 0 : 1); - /* allocate and load BIOS */ if (machine->firmware) { MemoryRegion *bios = g_new(MemoryRegion, 1); @@ -315,7 +313,7 @@ static void ppc405_init(MachineState *machine) /* Load ELF kernel and rootfs.cpio */ } else if (kernel_filename && !machine->firmware) { - boot_from_kernel(machine, cpu); + boot_from_kernel(machine, ppc405->soc.cpu); } } diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c index 156e839b8283..59612504bf3f 100644 --- a/hw/ppc/ppc405_uc.c +++ b/hw/ppc/ppc405_uc.c @@ -1432,134 +1432,131 @@ static void ppc405ep_cpc_init (CPUPPCState *env, clk_setup_t clk_setup[8], #endif } -PowerPCCPU *ppc405ep_init(MemoryRegion *address_space_mem, - MemoryRegion ram_memories[2], - hwaddr ram_bases[2], - hwaddr ram_sizes[2], - uint32_t sysclk, DeviceState **uicdevp, - int do_init) +static void ppc405_soc_realize(DeviceState *dev, Error **errp) { + Ppc405SoCState *s = PPC405_SOC(dev); clk_setup_t clk_setup[PPC405EP_CLK_NB], tlb_clk_setup; qemu_irq dma_irqs[4], gpt_irqs[5], mal_irqs[4]; - PowerPCCPU *cpu; CPUPPCState *env; - DeviceState *uicdev; - SysBusDevice *uicsbd; + Error *err = NULL; + + /* XXX: fix this ? */ + memory_region_init_alias(&s->ram_memories[0], OBJECT(s), + "ef405ep.ram.alias", s->dram_mr, 0, s->ram_size); + s->ram_bases[0] = 0; + s->ram_sizes[0] = s->ram_size; + memory_region_init(&s->ram_memories[1], OBJECT(s), "ef405ep.ram1", 0); + s->ram_bases[1] = 0x00000000; + s->ram_sizes[1] = 0x00000000; + + /* allocate SRAM */ + memory_region_init_ram(&s->sram, OBJECT(s), "ef405ep.sram", + PPC405EP_SRAM_SIZE, &err); + if (err) { + error_propagate(errp, err); + return; + } + memory_region_add_subregion(get_system_memory(), PPC405EP_SRAM_BASE, + &s->sram); memset(clk_setup, 0, sizeof(clk_setup)); + /* init CPUs */ - cpu = ppc4xx_init(POWERPC_CPU_TYPE_NAME("405ep"), + s->cpu = ppc4xx_init(POWERPC_CPU_TYPE_NAME("405ep"), &clk_setup[PPC405EP_CPU_CLK], - &tlb_clk_setup, sysclk); - env = &cpu->env; + &tlb_clk_setup, s->sysclk); + env = &s->cpu->env; clk_setup[PPC405EP_CPU_CLK].cb = tlb_clk_setup.cb; clk_setup[PPC405EP_CPU_CLK].opaque = tlb_clk_setup.opaque; - /* Internal devices init */ - /* Memory mapped devices registers */ + + /* CPU control */ + ppc405ep_cpc_init(env, clk_setup, s->sysclk); + /* PLB arbitrer */ ppc4xx_plb_init(env); + /* PLB to OPB bridge */ ppc4xx_pob_init(env); + /* OBP arbitrer */ ppc4xx_opba_init(0xef600600); + /* Universal interrupt controller */ - uicdev = qdev_new(TYPE_PPC_UIC); - uicsbd = SYS_BUS_DEVICE(uicdev); + s->uic = qdev_new(TYPE_PPC_UIC); - object_property_set_link(OBJECT(uicdev), "cpu", OBJECT(cpu), + object_property_set_link(OBJECT(s->uic), "cpu", OBJECT(s->cpu), &error_fatal); - sysbus_realize_and_unref(uicsbd, &error_fatal); - - sysbus_connect_irq(uicsbd, PPCUIC_OUTPUT_INT, - qdev_get_gpio_in(DEVICE(cpu), PPC40x_INPUT_INT)); - sysbus_connect_irq(uicsbd, PPCUIC_OUTPUT_CINT, - qdev_get_gpio_in(DEVICE(cpu), PPC40x_INPUT_CINT)); + if (!sysbus_realize(SYS_BUS_DEVICE(s->uic), errp)) { + return; + } - *uicdevp = uicdev; + sysbus_connect_irq(SYS_BUS_DEVICE(s->uic), PPCUIC_OUTPUT_INT, + qdev_get_gpio_in(DEVICE(s->cpu), PPC40x_INPUT_INT)); + sysbus_connect_irq(SYS_BUS_DEVICE(s->uic), PPCUIC_OUTPUT_CINT, + qdev_get_gpio_in(DEVICE(s->cpu), PPC40x_INPUT_CINT)); /* SDRAM controller */ - /* XXX 405EP has no ECC interrupt */ - ppc4xx_sdram_init(env, qdev_get_gpio_in(uicdev, 17), 2, ram_memories, - ram_bases, ram_sizes, do_init); + /* XXX 405EP has no ECC interrupt */ + ppc4xx_sdram_init(env, qdev_get_gpio_in(s->uic, 17), 2, s->ram_memories, + s->ram_bases, s->ram_sizes, s->do_dram_init); + /* External bus controller */ ppc405_ebc_init(env); + /* DMA controller */ - dma_irqs[0] = qdev_get_gpio_in(uicdev, 5); - dma_irqs[1] = qdev_get_gpio_in(uicdev, 6); - dma_irqs[2] = qdev_get_gpio_in(uicdev, 7); - dma_irqs[3] = qdev_get_gpio_in(uicdev, 8); + dma_irqs[0] = qdev_get_gpio_in(s->uic, 5); + dma_irqs[1] = qdev_get_gpio_in(s->uic, 6); + dma_irqs[2] = qdev_get_gpio_in(s->uic, 7); + dma_irqs[3] = qdev_get_gpio_in(s->uic, 8); ppc405_dma_init(env, dma_irqs); - /* IIC controller */ + + /* I2C controller */ sysbus_create_simple(TYPE_PPC4xx_I2C, 0xef600500, - qdev_get_gpio_in(uicdev, 2)); + qdev_get_gpio_in(s->uic, 2)); /* GPIO */ ppc405_gpio_init(0xef600700); + /* Serial ports */ if (serial_hd(0) != NULL) { - serial_mm_init(address_space_mem, 0xef600300, 0, - qdev_get_gpio_in(uicdev, 0), + serial_mm_init(get_system_memory(), 0xef600300, 0, + qdev_get_gpio_in(s->uic, 0), PPC_SERIAL_MM_BAUDBASE, serial_hd(0), DEVICE_BIG_ENDIAN); } if (serial_hd(1) != NULL) { - serial_mm_init(address_space_mem, 0xef600400, 0, - qdev_get_gpio_in(uicdev, 1), + serial_mm_init(get_system_memory(), 0xef600400, 0, + qdev_get_gpio_in(s->uic, 1), PPC_SERIAL_MM_BAUDBASE, serial_hd(1), DEVICE_BIG_ENDIAN); } + /* OCM */ ppc405_ocm_init(env); + /* GPT */ - gpt_irqs[0] = qdev_get_gpio_in(uicdev, 19); - gpt_irqs[1] = qdev_get_gpio_in(uicdev, 20); - gpt_irqs[2] = qdev_get_gpio_in(uicdev, 21); - gpt_irqs[3] = qdev_get_gpio_in(uicdev, 22); - gpt_irqs[4] = qdev_get_gpio_in(uicdev, 23); + gpt_irqs[0] = qdev_get_gpio_in(s->uic, 19); + gpt_irqs[1] = qdev_get_gpio_in(s->uic, 20); + gpt_irqs[2] = qdev_get_gpio_in(s->uic, 21); + gpt_irqs[3] = qdev_get_gpio_in(s->uic, 22); + gpt_irqs[4] = qdev_get_gpio_in(s->uic, 23); ppc4xx_gpt_init(0xef600000, gpt_irqs); - /* PCI */ - /* Uses UIC IRQs 3, 16, 18 */ + /* MAL */ - mal_irqs[0] = qdev_get_gpio_in(uicdev, 11); - mal_irqs[1] = qdev_get_gpio_in(uicdev, 12); - mal_irqs[2] = qdev_get_gpio_in(uicdev, 13); - mal_irqs[3] = qdev_get_gpio_in(uicdev, 14); + mal_irqs[0] = qdev_get_gpio_in(s->uic, 11); + mal_irqs[1] = qdev_get_gpio_in(s->uic, 12); + mal_irqs[2] = qdev_get_gpio_in(s->uic, 13); + mal_irqs[3] = qdev_get_gpio_in(s->uic, 14); ppc4xx_mal_init(env, 4, 2, mal_irqs); + /* Ethernet */ /* Uses UIC IRQs 9, 15, 17 */ - /* CPU control */ - ppc405ep_cpc_init(env, clk_setup, sysclk); - - return cpu; -} - -static void ppc405_soc_realize(DeviceState *dev, Error **errp) -{ - Ppc405SoCState *s = PPC405_SOC(dev); - Error *err = NULL; - - /* XXX: fix this ? */ - memory_region_init_alias(&s->ram_memories[0], OBJECT(s), - "ef405ep.ram.alias", s->dram_mr, 0, s->ram_size); - s->ram_bases[0] = 0; - s->ram_sizes[0] = s->ram_size; - memory_region_init(&s->ram_memories[1], OBJECT(s), "ef405ep.ram1", 0); - s->ram_bases[1] = 0x00000000; - s->ram_sizes[1] = 0x00000000; - - /* allocate SRAM */ - memory_region_init_ram(&s->sram, OBJECT(s), "ef405ep.sram", - PPC405EP_SRAM_SIZE, &err); - if (err) { - error_propagate(errp, err); - return; - } - memory_region_add_subregion(get_system_memory(), PPC405EP_SRAM_BASE, - &s->sram); } static Property ppc405_soc_properties[] = { DEFINE_PROP_LINK("dram", Ppc405SoCState, dram_mr, TYPE_MEMORY_REGION, MemoryRegion *), + DEFINE_PROP_UINT32("sys-clk", Ppc405SoCState, sysclk, 0), + DEFINE_PROP_BOOL("dram-init", Ppc405SoCState, do_dram_init, 0), DEFINE_PROP_UINT64("ram-size", Ppc405SoCState, ram_size, 0), DEFINE_PROP_END_OF_LIST(), };
This moves all the code previously done in the ppc405ep_init() routine under ppc405_soc_realize(). Signed-off-by: Cédric Le Goater <clg@kaod.org> --- hw/ppc/ppc405.h | 12 ++-- hw/ppc/ppc405_boards.c | 12 ++-- hw/ppc/ppc405_uc.c | 151 ++++++++++++++++++++--------------------- 3 files changed, 84 insertions(+), 91 deletions(-)