qemu-devel
[Top][All Lists]
Advanced
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 1/3] hw/mips/boston: Massage memory map information

From: BALATON Zoltan
Subject: Re: [PATCH v2 1/3] hw/mips/boston: Massage memory map information
Date: Wed, 29 Sep 2021 17:32:49 +0200 (CEST) 
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);
reply via email to
[Prev in Thread] Current Thread [Next in Thread]