| Message ID | 20210929151211.108-2-jiaxun.yang@flygoat.com |
|---|---|
| State | New |
| Headers | show |
| Series | hw/mips/boston: ELF kernel support | expand |
On Wed, 29 Sep 2021, Jiaxun Yang wrote: > Use memmap array to uinfy address of memory map. > That would allow us reuse address information for FDT generation. > > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > -- > v2: Fix minor style issue, fix uart map size > --- > hw/mips/boston.c | 95 ++++++++++++++++++++++++++++++++++++------------ > 1 file changed, 71 insertions(+), 24 deletions(-) > > diff --git a/hw/mips/boston.c b/hw/mips/boston.c > index 20b06865b2..5c720440fb 100644 > --- a/hw/mips/boston.c > +++ b/hw/mips/boston.c > @@ -64,6 +64,44 @@ struct BostonState { > hwaddr fdt_base; > }; > > +enum { > + BOSTON_LOWDDR, > + BOSTON_PCIE0, > + BOSTON_PCIE1, > + BOSTON_PCIE2, > + BOSTON_PCIE2_MMIO, > + BOSTON_CM, > + BOSTON_GIC, > + BOSTON_CDMM, > + BOSTON_CPC, > + BOSTON_PLATREG, > + BOSTON_UART, > + BOSTON_LCD, > + BOSTON_FLASH, > + BOSTON_PCIE1_MMIO, > + BOSTON_PCIE0_MMIO, > + BOSTON_HIGHDDR, > +}; > + > +static const MemMapEntry boston_memmap[] = { > + [BOSTON_LOWDDR] = { 0x0, 0x10000000 }, What's the advantage of having it in an array as opposed to just have simple defines for these? I did not see a case where you go through these as array elements so it seems it's just an unnecessarily complex way to write boston_memmap[BOSTON_LOWADDR] insted of BOSTON_LOWADDR. Did I miss something where having an array helps? Regards, BALATON Zoltan > + [BOSTON_PCIE0] = { 0x10000000, 0x2000000 }, > + [BOSTON_PCIE1] = { 0x12000000, 0x2000000 }, > + [BOSTON_PCIE2] = { 0x14000000, 0x2000000 }, > + [BOSTON_PCIE2_MMIO] = { 0x16000000, 0x100000 }, > + [BOSTON_CM] = { 0x16100000, 0x20000 }, > + [BOSTON_GIC] = { 0x16120000, 0x20000 }, > + [BOSTON_CDMM] = { 0x16140000, 0x8000 }, > + [BOSTON_CPC] = { 0x16200000, 0x8000 }, > + [BOSTON_PLATREG] = { 0x17ffd000, 0x1000 }, > + [BOSTON_UART] = { 0x17ffe000, 0x20 }, > + [BOSTON_LCD] = { 0x17fff000, 0x8 }, > + [BOSTON_FLASH] = { 0x18000000, 0x8000000 }, > + [BOSTON_PCIE1_MMIO] = { 0x20000000, 0x20000000 }, > + [BOSTON_PCIE0_MMIO] = { 0x40000000, 0x40000000 }, > + [BOSTON_HIGHDDR] = { 0x80000000, 0x0 }, > +}; > + > enum boston_plat_reg { > PLAT_FPGA_BUILD = 0x00, > PLAT_CORE_CL = 0x04, > @@ -275,24 +313,22 @@ type_init(boston_register_types) > > static void gen_firmware(uint32_t *p, hwaddr kernel_entry, hwaddr fdt_addr) > { > - const uint32_t cm_base = 0x16100000; > - const uint32_t gic_base = 0x16120000; > - const uint32_t cpc_base = 0x16200000; > - > /* Move CM GCRs */ > bl_gen_write_ulong(&p, > cpu_mips_phys_to_kseg1(NULL, GCR_BASE_ADDR + GCR_BASE_OFS), > - cm_base); > + boston_memmap[BOSTON_CM].base); > > /* Move & enable GIC GCRs */ > bl_gen_write_ulong(&p, > - cpu_mips_phys_to_kseg1(NULL, cm_base + GCR_GIC_BASE_OFS), > - gic_base | GCR_GIC_BASE_GICEN_MSK); > + cpu_mips_phys_to_kseg1(NULL, > + boston_memmap[BOSTON_CM].base + GCR_GIC_BASE_OFS), > + boston_memmap[BOSTON_GIC].base | GCR_GIC_BASE_GICEN_MSK); > > /* Move & enable CPC GCRs */ > bl_gen_write_ulong(&p, > - cpu_mips_phys_to_kseg1(NULL, cm_base + GCR_CPC_BASE_OFS), > - cpc_base | GCR_CPC_BASE_CPCEN_MSK); > + cpu_mips_phys_to_kseg1(NULL, > + boston_memmap[BOSTON_CM].base + GCR_CPC_BASE_OFS), > + boston_memmap[BOSTON_CPC].base | GCR_CPC_BASE_CPCEN_MSK); > > /* > * Setup argument registers to follow the UHI boot protocol: > @@ -333,8 +369,9 @@ static const void *boston_fdt_filter(void *opaque, const void *fdt_orig, > ram_low_sz = MIN(256 * MiB, machine->ram_size); > ram_high_sz = machine->ram_size - ram_low_sz; > qemu_fdt_setprop_sized_cells(fdt, "/memory@0", "reg", > - 1, 0x00000000, 1, ram_low_sz, > - 1, 0x90000000, 1, ram_high_sz); > + 1, boston_memmap[BOSTON_LOWDDR].base, 1, ram_low_sz, > + 1, boston_memmap[BOSTON_HIGHDDR].base + ram_low_sz, > + 1, ram_high_sz); > > fdt = g_realloc(fdt, fdt_totalsize(fdt)); > qemu_fdt_dumpdtb(fdt, fdt_sz); > @@ -438,11 +475,13 @@ static void boston_mach_init(MachineState *machine) > sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->cps), 0, 0, 1); > > flash = g_new(MemoryRegion, 1); > - memory_region_init_rom(flash, NULL, "boston.flash", 128 * MiB, > + memory_region_init_rom(flash, NULL, "boston.flash", boston_memmap[BOSTON_FLASH].size, > &error_fatal); > - memory_region_add_subregion_overlap(sys_mem, 0x18000000, flash, 0); > + memory_region_add_subregion_overlap(sys_mem, boston_memmap[BOSTON_FLASH].base, > + flash, 0); > > - memory_region_add_subregion_overlap(sys_mem, 0x80000000, machine->ram, 0); > + memory_region_add_subregion_overlap(sys_mem, boston_memmap[BOSTON_HIGHDDR].base, > + machine->ram, 0); > > ddr_low_alias = g_new(MemoryRegion, 1); > memory_region_init_alias(ddr_low_alias, NULL, "boston_low.ddr", > @@ -451,32 +490,40 @@ static void boston_mach_init(MachineState *machine) > memory_region_add_subregion_overlap(sys_mem, 0, ddr_low_alias, 0); > > xilinx_pcie_init(sys_mem, 0, > - 0x10000000, 32 * MiB, > - 0x40000000, 1 * GiB, > + boston_memmap[BOSTON_PCIE0].base, > + boston_memmap[BOSTON_PCIE0].size, > + boston_memmap[BOSTON_PCIE0_MMIO].base, > + boston_memmap[BOSTON_PCIE0_MMIO].size, > get_cps_irq(&s->cps, 2), false); > > xilinx_pcie_init(sys_mem, 1, > - 0x12000000, 32 * MiB, > - 0x20000000, 512 * MiB, > + boston_memmap[BOSTON_PCIE1].base, > + boston_memmap[BOSTON_PCIE1].size, > + boston_memmap[BOSTON_PCIE1_MMIO].base, > + boston_memmap[BOSTON_PCIE1_MMIO].size, > get_cps_irq(&s->cps, 1), false); > > pcie2 = xilinx_pcie_init(sys_mem, 2, > - 0x14000000, 32 * MiB, > - 0x16000000, 1 * MiB, > + boston_memmap[BOSTON_PCIE2].base, > + boston_memmap[BOSTON_PCIE2].size, > + boston_memmap[BOSTON_PCIE2_MMIO].base, > + boston_memmap[BOSTON_PCIE2_MMIO].size, > get_cps_irq(&s->cps, 0), true); > > platreg = g_new(MemoryRegion, 1); > memory_region_init_io(platreg, NULL, &boston_platreg_ops, s, > - "boston-platregs", 0x1000); > - memory_region_add_subregion_overlap(sys_mem, 0x17ffd000, platreg, 0); > + "boston-platregs", > + boston_memmap[BOSTON_PLATREG].size); > + memory_region_add_subregion_overlap(sys_mem, > + boston_memmap[BOSTON_PLATREG].base, platreg, 0); > > - s->uart = serial_mm_init(sys_mem, 0x17ffe000, 2, > + s->uart = serial_mm_init(sys_mem, boston_memmap[BOSTON_UART].base, 2, > get_cps_irq(&s->cps, 3), 10000000, > serial_hd(0), DEVICE_NATIVE_ENDIAN); > > lcd = g_new(MemoryRegion, 1); > memory_region_init_io(lcd, NULL, &boston_lcd_ops, s, "boston-lcd", 0x8); > - memory_region_add_subregion_overlap(sys_mem, 0x17fff000, lcd, 0); > + memory_region_add_subregion_overlap(sys_mem, boston_memmap[BOSTON_LCD].base, lcd, 0); > > chr = qemu_chr_new("lcd", "vc:320x240", NULL); > qemu_chr_fe_init(&s->lcd_display, chr, NULL); >
在 2021/9/29 16:32, BALATON Zoltan 写道: > On Wed, 29 Sep 2021, Jiaxun Yang wrote: >> Use memmap array to uinfy address of memory map. >> That would allow us reuse address information for FDT generation. >> >> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> >> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> -- >> v2: Fix minor style issue, fix uart map size >> --- >> hw/mips/boston.c | 95 ++++++++++++++++++++++++++++++++++++------------ >> 1 file changed, 71 insertions(+), 24 deletions(-) >> >> diff --git a/hw/mips/boston.c b/hw/mips/boston.c >> index 20b06865b2..5c720440fb 100644 >> --- a/hw/mips/boston.c >> +++ b/hw/mips/boston.c >> @@ -64,6 +64,44 @@ struct BostonState { >> hwaddr fdt_base; >> }; >> >> +enum { >> + BOSTON_LOWDDR, >> + BOSTON_PCIE0, >> + BOSTON_PCIE1, >> + BOSTON_PCIE2, >> + BOSTON_PCIE2_MMIO, >> + BOSTON_CM, >> + BOSTON_GIC, >> + BOSTON_CDMM, >> + BOSTON_CPC, >> + BOSTON_PLATREG, >> + BOSTON_UART, >> + BOSTON_LCD, >> + BOSTON_FLASH, >> + BOSTON_PCIE1_MMIO, >> + BOSTON_PCIE0_MMIO, >> + BOSTON_HIGHDDR, >> +}; >> + >> +static const MemMapEntry boston_memmap[] = { >> + [BOSTON_LOWDDR] = { 0x0, 0x10000000 }, > > What's the advantage of having it in an array as opposed to just have > simple defines for these? I did not see a case where you go through > these as array elements so it seems it's just an unnecessarily complex > way to write boston_memmap[BOSTON_LOWADDR] insted of BOSTON_LOWADDR. > Did I miss something where having an array helps? I do consider it as sort of common practice which will make address allocation much more clear and easier for subsequent FDT generation patch to take both address and size. Many new boards such as arm-virt, arm-sbsa and riscv-virt is using MemMapEntry to manage their address allocation. Thanks. - Jiaxun > > Regards, > BALATON Zoltan >
diff --git a/hw/mips/boston.c b/hw/mips/boston.c index 20b06865b2..5c720440fb 100644 --- a/hw/mips/boston.c +++ b/hw/mips/boston.c @@ -64,6 +64,44 @@ struct BostonState { hwaddr fdt_base; }; +enum { + BOSTON_LOWDDR, + BOSTON_PCIE0, + BOSTON_PCIE1, + BOSTON_PCIE2, + BOSTON_PCIE2_MMIO, + BOSTON_CM, + BOSTON_GIC, + BOSTON_CDMM, + BOSTON_CPC, + BOSTON_PLATREG, + BOSTON_UART, + BOSTON_LCD, + BOSTON_FLASH, + BOSTON_PCIE1_MMIO, + BOSTON_PCIE0_MMIO, + BOSTON_HIGHDDR, +}; + +static const MemMapEntry boston_memmap[] = { + [BOSTON_LOWDDR] = { 0x0, 0x10000000 }, + [BOSTON_PCIE0] = { 0x10000000, 0x2000000 }, + [BOSTON_PCIE1] = { 0x12000000, 0x2000000 }, + [BOSTON_PCIE2] = { 0x14000000, 0x2000000 }, + [BOSTON_PCIE2_MMIO] = { 0x16000000, 0x100000 }, + [BOSTON_CM] = { 0x16100000, 0x20000 }, + [BOSTON_GIC] = { 0x16120000, 0x20000 }, + [BOSTON_CDMM] = { 0x16140000, 0x8000 }, + [BOSTON_CPC] = { 0x16200000, 0x8000 }, + [BOSTON_PLATREG] = { 0x17ffd000, 0x1000 }, + [BOSTON_UART] = { 0x17ffe000, 0x20 }, + [BOSTON_LCD] = { 0x17fff000, 0x8 }, + [BOSTON_FLASH] = { 0x18000000, 0x8000000 }, + [BOSTON_PCIE1_MMIO] = { 0x20000000, 0x20000000 }, + [BOSTON_PCIE0_MMIO] = { 0x40000000, 0x40000000 }, + [BOSTON_HIGHDDR] = { 0x80000000, 0x0 }, +}; + enum boston_plat_reg { PLAT_FPGA_BUILD = 0x00, PLAT_CORE_CL = 0x04, @@ -275,24 +313,22 @@ type_init(boston_register_types) static void gen_firmware(uint32_t *p, hwaddr kernel_entry, hwaddr fdt_addr) { - const uint32_t cm_base = 0x16100000; - const uint32_t gic_base = 0x16120000; - const uint32_t cpc_base = 0x16200000; - /* Move CM GCRs */ bl_gen_write_ulong(&p, cpu_mips_phys_to_kseg1(NULL, GCR_BASE_ADDR + GCR_BASE_OFS), - cm_base); + boston_memmap[BOSTON_CM].base); /* Move & enable GIC GCRs */ bl_gen_write_ulong(&p, - cpu_mips_phys_to_kseg1(NULL, cm_base + GCR_GIC_BASE_OFS), - gic_base | GCR_GIC_BASE_GICEN_MSK); + cpu_mips_phys_to_kseg1(NULL, + boston_memmap[BOSTON_CM].base + GCR_GIC_BASE_OFS), + boston_memmap[BOSTON_GIC].base | GCR_GIC_BASE_GICEN_MSK); /* Move & enable CPC GCRs */ bl_gen_write_ulong(&p, - cpu_mips_phys_to_kseg1(NULL, cm_base + GCR_CPC_BASE_OFS), - cpc_base | GCR_CPC_BASE_CPCEN_MSK); + cpu_mips_phys_to_kseg1(NULL, + boston_memmap[BOSTON_CM].base + GCR_CPC_BASE_OFS), + boston_memmap[BOSTON_CPC].base | GCR_CPC_BASE_CPCEN_MSK); /* * Setup argument registers to follow the UHI boot protocol: @@ -333,8 +369,9 @@ static const void *boston_fdt_filter(void *opaque, const void *fdt_orig, ram_low_sz = MIN(256 * MiB, machine->ram_size); ram_high_sz = machine->ram_size - ram_low_sz; qemu_fdt_setprop_sized_cells(fdt, "/memory@0", "reg", - 1, 0x00000000, 1, ram_low_sz, - 1, 0x90000000, 1, ram_high_sz); + 1, boston_memmap[BOSTON_LOWDDR].base, 1, ram_low_sz, + 1, boston_memmap[BOSTON_HIGHDDR].base + ram_low_sz, + 1, ram_high_sz); fdt = g_realloc(fdt, fdt_totalsize(fdt)); qemu_fdt_dumpdtb(fdt, fdt_sz); @@ -438,11 +475,13 @@ static void boston_mach_init(MachineState *machine) sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->cps), 0, 0, 1); flash = g_new(MemoryRegion, 1); - memory_region_init_rom(flash, NULL, "boston.flash", 128 * MiB, + memory_region_init_rom(flash, NULL, "boston.flash", boston_memmap[BOSTON_FLASH].size, &error_fatal); - memory_region_add_subregion_overlap(sys_mem, 0x18000000, flash, 0); + memory_region_add_subregion_overlap(sys_mem, boston_memmap[BOSTON_FLASH].base, + flash, 0); - memory_region_add_subregion_overlap(sys_mem, 0x80000000, machine->ram, 0); + memory_region_add_subregion_overlap(sys_mem, boston_memmap[BOSTON_HIGHDDR].base, + machine->ram, 0); ddr_low_alias = g_new(MemoryRegion, 1); memory_region_init_alias(ddr_low_alias, NULL, "boston_low.ddr", @@ -451,32 +490,40 @@ static void boston_mach_init(MachineState *machine) memory_region_add_subregion_overlap(sys_mem, 0, ddr_low_alias, 0); xilinx_pcie_init(sys_mem, 0, - 0x10000000, 32 * MiB, - 0x40000000, 1 * GiB, + boston_memmap[BOSTON_PCIE0].base, + boston_memmap[BOSTON_PCIE0].size, + boston_memmap[BOSTON_PCIE0_MMIO].base, + boston_memmap[BOSTON_PCIE0_MMIO].size, get_cps_irq(&s->cps, 2), false); xilinx_pcie_init(sys_mem, 1, - 0x12000000, 32 * MiB, - 0x20000000, 512 * MiB, + boston_memmap[BOSTON_PCIE1].base, + boston_memmap[BOSTON_PCIE1].size, + boston_memmap[BOSTON_PCIE1_MMIO].base, + boston_memmap[BOSTON_PCIE1_MMIO].size, get_cps_irq(&s->cps, 1), false); pcie2 = xilinx_pcie_init(sys_mem, 2, - 0x14000000, 32 * MiB, - 0x16000000, 1 * MiB, + boston_memmap[BOSTON_PCIE2].base, + boston_memmap[BOSTON_PCIE2].size, + boston_memmap[BOSTON_PCIE2_MMIO].base, + boston_memmap[BOSTON_PCIE2_MMIO].size, get_cps_irq(&s->cps, 0), true); platreg = g_new(MemoryRegion, 1); memory_region_init_io(platreg, NULL, &boston_platreg_ops, s, - "boston-platregs", 0x1000); - memory_region_add_subregion_overlap(sys_mem, 0x17ffd000, platreg, 0); + "boston-platregs", + boston_memmap[BOSTON_PLATREG].size); + memory_region_add_subregion_overlap(sys_mem, + boston_memmap[BOSTON_PLATREG].base, platreg, 0); - s->uart = serial_mm_init(sys_mem, 0x17ffe000, 2, + s->uart = serial_mm_init(sys_mem, boston_memmap[BOSTON_UART].base, 2, get_cps_irq(&s->cps, 3), 10000000, serial_hd(0), DEVICE_NATIVE_ENDIAN); lcd = g_new(MemoryRegion, 1); memory_region_init_io(lcd, NULL, &boston_lcd_ops, s, "boston-lcd", 0x8); - memory_region_add_subregion_overlap(sys_mem, 0x17fff000, lcd, 0); + memory_region_add_subregion_overlap(sys_mem, boston_memmap[BOSTON_LCD].base, lcd, 0); chr = qemu_chr_new("lcd", "vc:320x240", NULL); qemu_chr_fe_init(&s->lcd_display, chr, NULL);