[PATCH 0/3] hw/mips/boston: ELF kernel support

[PATCH 0/3] hw/mips/boston: ELF kernel support

[Jiaxun Yang]

Jiaxun Yang (3):
  hw/mips/boston: Massage memory map information
  hw/mips/boston: Allow loading elf kernel and dtb
  hw/mips/boston: Add FDT generator

 hw/mips/boston.c | 351 +++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 323 insertions(+), 28 deletions(-)

-- 
2.32.0

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

[Jiaxun Yang]

Use memmap array to unfiy address of memory map.
That would allow us reuse address information for FDT generation.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 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..a5746ede65 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,        0x1000 },
+    [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);
-- 
2.32.0

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

[Philippe Mathieu-Daudé]

On 7/29/21 5:39 AM, Jiaxun Yang wrote:
> Use memmap array to unfiy address of memory map.
> That would allow us reuse address information for FDT generation.
> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>  hw/mips/boston.c | 95 ++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 71 insertions(+), 24 deletions(-)

> +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,        0x1000 },

0x20?

> +    [BOSTON_LCD] =        { 0x17fff000,           0x8 },
> +    [BOSTON_FLASH] =      { 0x18000000,     0x8000000 },
> +    [BOSTON_PCIE1_MMIO] = { 0x20000000,    0x20000000 },
> +    [BOSTON_PCIE0_MMIO] = { 0x40000000,    0x40000000 },
> +    [BOSTON_HIGHDDR] =    { 0x80000000,           0x0 },
> +};

>      /*
>       * 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);

Misplaced comma.

BTW shouldn't we check whether ram_high_sz > 0 before adding this cell?

Otherwise:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

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

[Philippe Mathieu-Daudé]

On 7/29/21 9:55 AM, Philippe Mathieu-Daudé wrote:
> On 7/29/21 5:39 AM, Jiaxun Yang wrote:
>> Use memmap array to unfiy address of memory map.

Typo 'unify'.

>> That would allow us reuse address information for FDT generation.
>>
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> ---
>>  hw/mips/boston.c | 95 ++++++++++++++++++++++++++++++++++++------------
>>  1 file changed, 71 insertions(+), 24 deletions(-)
> 
>> +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,        0x1000 },
> 
> 0x20?
> 
>> +    [BOSTON_LCD] =        { 0x17fff000,           0x8 },
>> +    [BOSTON_FLASH] =      { 0x18000000,     0x8000000 },
>> +    [BOSTON_PCIE1_MMIO] = { 0x20000000,    0x20000000 },
>> +    [BOSTON_PCIE0_MMIO] = { 0x40000000,    0x40000000 },
>> +    [BOSTON_HIGHDDR] =    { 0x80000000,           0x0 },
>> +};
> 
>>      /*
>>       * 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);
> 
> Misplaced comma.
> 
> BTW shouldn't we check whether ram_high_sz > 0 before adding this cell?
> 
> Otherwise:
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 

[PATCH 2/3] hw/mips/boston: Allow loading elf kernel and dtb

[Jiaxun Yang]

ELF kernel allows us debugging much easier with DWARF symbols.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 hw/mips/boston.c | 38 ++++++++++++++++++++++++++++++++++----
 1 file changed, 34 insertions(+), 4 deletions(-)

diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index a5746ede65..42b31a1ce4 100644
--- a/hw/mips/boston.c
+++ b/hw/mips/boston.c
@@ -20,6 +20,7 @@
 #include "qemu/osdep.h"
 #include "qemu/units.h"
 
+#include "elf.h"
 #include "hw/boards.h"
 #include "hw/char/serial.h"
 #include "hw/ide/pci.h"
@@ -546,10 +547,39 @@ static void boston_mach_init(MachineState *machine)
             exit(1);
         }
     } else if (machine->kernel_filename) {
-        fit_err = load_fit(&boston_fit_loader, machine->kernel_filename, s);
-        if (fit_err) {
-            error_report("unable to load FIT image");
-            exit(1);
+        uint64_t kernel_entry, kernel_low, kernel_high, kernel_size;
+
+        kernel_size = load_elf(machine->kernel_filename, NULL,
+                           cpu_mips_kseg0_to_phys, NULL,
+                           (uint64_t *)&kernel_entry,
+                           (uint64_t *)&kernel_low, (uint64_t *)&kernel_high,
+                           NULL, 0, EM_MIPS, 1, 0);
+
+        if (kernel_size) {
+            hwaddr dtb_paddr = QEMU_ALIGN_UP(kernel_high, 64 * KiB);
+            hwaddr dtb_vaddr = cpu_mips_phys_to_kseg0(NULL, dtb_paddr);
+
+            s->kernel_entry = kernel_entry;
+            if (machine->dtb) {
+                int dt_size;
+                const void *dtb_file_data, *dtb_load_data;
+
+                dtb_file_data = load_device_tree(machine->dtb, &dt_size);
+                dtb_load_data = boston_fdt_filter(s, dtb_file_data, NULL, &dtb_vaddr);
+
+                /* Calculate real fdt size after filter */
+                dt_size = fdt_totalsize(dtb_load_data);
+                rom_add_blob_fixed("dtb", dtb_load_data, dt_size, dtb_paddr);
+                g_free((void *) dtb_file_data);
+                g_free((void *) dtb_load_data);
+            }
+        } else {
+            /* Try to load file as FIT */
+            fit_err = load_fit(&boston_fit_loader, machine->kernel_filename, s);
+            if (fit_err) {
+                error_report("unable to load kernel image");
+                exit(1);
+            }
         }
 
         gen_firmware(memory_region_get_ram_ptr(flash) + 0x7c00000,
-- 
2.32.0

Re: [PATCH 2/3] hw/mips/boston: Allow loading elf kernel and dtb

[Philippe Mathieu-Daudé]

On 7/29/21 5:39 AM, Jiaxun Yang wrote:
> ELF kernel allows us debugging much easier with DWARF symbols.

You can load the symbols using the 'loader' device:
docs/system/generic-loader.rst.

> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>  hw/mips/boston.c | 38 ++++++++++++++++++++++++++++++++++----
>  1 file changed, 34 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/mips/boston.c b/hw/mips/boston.c
> index a5746ede65..42b31a1ce4 100644
> --- a/hw/mips/boston.c
> +++ b/hw/mips/boston.c
> @@ -20,6 +20,7 @@
>  #include "qemu/osdep.h"
>  #include "qemu/units.h"
>  
> +#include "elf.h"
>  #include "hw/boards.h"
>  #include "hw/char/serial.h"
>  #include "hw/ide/pci.h"
> @@ -546,10 +547,39 @@ static void boston_mach_init(MachineState *machine)
>              exit(1);
>          }
>      } else if (machine->kernel_filename) {
> -        fit_err = load_fit(&boston_fit_loader, machine->kernel_filename, s);
> -        if (fit_err) {
> -            error_report("unable to load FIT image");
> -            exit(1);
> +        uint64_t kernel_entry, kernel_low, kernel_high, kernel_size;
> +
> +        kernel_size = load_elf(machine->kernel_filename, NULL,
> +                           cpu_mips_kseg0_to_phys, NULL,
> +                           (uint64_t *)&kernel_entry,
> +                           (uint64_t *)&kernel_low, (uint64_t *)&kernel_high,
> +                           NULL, 0, EM_MIPS, 1, 0);
> +
> +        if (kernel_size) {
> +            hwaddr dtb_paddr = QEMU_ALIGN_UP(kernel_high, 64 * KiB);
> +            hwaddr dtb_vaddr = cpu_mips_phys_to_kseg0(NULL, dtb_paddr);
> +
> +            s->kernel_entry = kernel_entry;
> +            if (machine->dtb) {
> +                int dt_size;
> +                const void *dtb_file_data, *dtb_load_data;

g_autofree is preferred.

Otherwise:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> +
> +                dtb_file_data = load_device_tree(machine->dtb, &dt_size);
> +                dtb_load_data = boston_fdt_filter(s, dtb_file_data, NULL, &dtb_vaddr);
> +
> +                /* Calculate real fdt size after filter */
> +                dt_size = fdt_totalsize(dtb_load_data);
> +                rom_add_blob_fixed("dtb", dtb_load_data, dt_size, dtb_paddr);
> +                g_free((void *) dtb_file_data);
> +                g_free((void *) dtb_load_data);
> +            }
> +        } else {
> +            /* Try to load file as FIT */
> +            fit_err = load_fit(&boston_fit_loader, machine->kernel_filename, s);
> +            if (fit_err) {
> +                error_report("unable to load kernel image");
> +                exit(1);
> +            }
>          }
>  
>          gen_firmware(memory_region_get_ram_ptr(flash) + 0x7c00000,
> 

Re: [PATCH 2/3] hw/mips/boston: Allow loading elf kernel and dtb

[Philippe Mathieu-Daudé]

On 7/29/21 10:02 AM, Philippe Mathieu-Daudé wrote:
> On 7/29/21 5:39 AM, Jiaxun Yang wrote:
>> ELF kernel allows us debugging much easier with DWARF symbols.
> 
> You can load the symbols using the 'loader' device:
> docs/system/generic-loader.rst.
> 
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> ---
>>  hw/mips/boston.c | 38 ++++++++++++++++++++++++++++++++++----
>>  1 file changed, 34 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/mips/boston.c b/hw/mips/boston.c
>> index a5746ede65..42b31a1ce4 100644
>> --- a/hw/mips/boston.c
>> +++ b/hw/mips/boston.c
>> @@ -20,6 +20,7 @@
>>  #include "qemu/osdep.h"
>>  #include "qemu/units.h"
>>  
>> +#include "elf.h"
>>  #include "hw/boards.h"
>>  #include "hw/char/serial.h"
>>  #include "hw/ide/pci.h"
>> @@ -546,10 +547,39 @@ static void boston_mach_init(MachineState *machine)
>>              exit(1);
>>          }
>>      } else if (machine->kernel_filename) {
>> -        fit_err = load_fit(&boston_fit_loader, machine->kernel_filename, s);
>> -        if (fit_err) {
>> -            error_report("unable to load FIT image");
>> -            exit(1);
>> +        uint64_t kernel_entry, kernel_low, kernel_high, kernel_size;
>> +
>> +        kernel_size = load_elf(machine->kernel_filename, NULL,
>> +                           cpu_mips_kseg0_to_phys, NULL,
>> +                           (uint64_t *)&kernel_entry,
>> +                           (uint64_t *)&kernel_low, (uint64_t *)&kernel_high,
>> +                           NULL, 0, EM_MIPS, 1, 0);
>> +
>> +        if (kernel_size) {
>> +            hwaddr dtb_paddr = QEMU_ALIGN_UP(kernel_high, 64 * KiB);
>> +            hwaddr dtb_vaddr = cpu_mips_phys_to_kseg0(NULL, dtb_paddr);
>> +
>> +            s->kernel_entry = kernel_entry;
>> +            if (machine->dtb) {
>> +                int dt_size;
>> +                const void *dtb_file_data, *dtb_load_data;
> 
> g_autofree is preferred.
> 
> Otherwise:
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
>> +
>> +                dtb_file_data = load_device_tree(machine->dtb, &dt_size);
>> +                dtb_load_data = boston_fdt_filter(s, dtb_file_data, NULL, &dtb_vaddr);
>> +
>> +                /* Calculate real fdt size after filter */
>> +                dt_size = fdt_totalsize(dtb_load_data);
>> +                rom_add_blob_fixed("dtb", dtb_load_data, dt_size, dtb_paddr);
>> +                g_free((void *) dtb_file_data);
>> +                g_free((void *) dtb_load_data);

Also, no need to cast.

>> +            }
>> +        } else {
>> +            /* Try to load file as FIT */
>> +            fit_err = load_fit(&boston_fit_loader, machine->kernel_filename, s);
>> +            if (fit_err) {
>> +                error_report("unable to load kernel image");
>> +                exit(1);
>> +            }
>>          }
>>  
>>          gen_firmware(memory_region_get_ram_ptr(flash) + 0x7c00000,
>>
> 

[PATCH 3/3] hw/mips/boston: Add FDT generator

[Jiaxun Yang]

Generate FDT on our own if no dtb argument supplied.
Avoid introduce unused device in FDT with user supplied dtb.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 hw/mips/boston.c | 238 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 228 insertions(+), 10 deletions(-)

diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index 42b31a1ce4..aaa79b9da7 100644
--- a/hw/mips/boston.c
+++ b/hw/mips/boston.c
@@ -49,6 +49,13 @@ typedef struct BostonState BostonState;
 DECLARE_INSTANCE_CHECKER(BostonState, BOSTON,
                          TYPE_BOSTON)
 
+#define FDT_IRQ_TYPE_NONE       0
+#define FDT_IRQ_TYPE_LEVEL_HIGH 4
+#define FDT_GIC_SHARED          0
+#define FDT_GIC_LOCAL           1
+#define FDT_BOSTON_CLK_SYS      1
+#define FDT_BOSTON_CLK_CPU      2
+
 struct BostonState {
     SysBusDevice parent_obj;
 
@@ -435,6 +442,214 @@ xilinx_pcie_init(MemoryRegion *sys_mem, uint32_t bus_nr,
     return XILINX_PCIE_HOST(dev);
 }
 
+
+static void fdt_create_pcie(void *fdt, int gic_ph, int irq, hwaddr reg_base,
+                            hwaddr reg_size, hwaddr mmio_base, hwaddr mmio_size)
+{
+    int i;
+    char *name, *intc_name;
+    uint32_t intc_ph;
+    uint32_t interrupt_map[4][6];
+
+    intc_ph = qemu_fdt_alloc_phandle(fdt);
+    name = g_strdup_printf("/soc/pci@%lx", (long)reg_base);
+    qemu_fdt_add_subnode(fdt, name);
+    qemu_fdt_setprop_string(fdt, name, "compatible", "xlnx,axi-pcie-host-1.00.a");
+    qemu_fdt_setprop_string(fdt, name, "device_type", "pci");
+    qemu_fdt_setprop_cells(fdt, name, "reg", reg_base, reg_size);
+
+    qemu_fdt_setprop_cell(fdt, name, "#address-cells", 3);
+    qemu_fdt_setprop_cell(fdt, name, "#size-cells", 2);
+    qemu_fdt_setprop_cell(fdt, name, "#interrupt-cells", 1);
+
+    qemu_fdt_setprop_cell(fdt, name, "interrupt-parent", gic_ph);
+    qemu_fdt_setprop_cells(fdt, name, "interrupts", FDT_GIC_SHARED, irq,
+                            FDT_IRQ_TYPE_LEVEL_HIGH);
+
+    qemu_fdt_setprop_cells(fdt, name, "ranges", 0x02000000, 0, mmio_base,
+                            mmio_base, 0, mmio_size);
+    qemu_fdt_setprop_cells(fdt, name, "bus-range", 0x00, 0xff);
+
+
+
+    intc_name = g_strdup_printf("%s/interrupt-controller", name);
+    qemu_fdt_add_subnode(fdt, intc_name);
+    qemu_fdt_setprop(fdt, intc_name, "interrupt-controller", NULL, 0);
+    qemu_fdt_setprop_cell(fdt, intc_name, "#address-cells", 0);
+    qemu_fdt_setprop_cell(fdt, intc_name, "#interrupt-cells", 1);
+    qemu_fdt_setprop_cell(fdt, intc_name, "phandle", intc_ph);
+
+    qemu_fdt_setprop_cells(fdt, name, "interrupt-map-mask", 0, 0, 0, 7);
+    for (i = 0; i < 4; i++) {
+        uint32_t *irqmap = interrupt_map[i];
+
+        irqmap[0] = cpu_to_be32(0);
+        irqmap[1] = cpu_to_be32(0);
+        irqmap[2] = cpu_to_be32(0);
+        irqmap[3] = cpu_to_be32(i + 1);
+        irqmap[4] = cpu_to_be32(intc_ph);
+        irqmap[5] = cpu_to_be32(i + 1);
+    }
+    qemu_fdt_setprop(fdt, name, "interrupt-map", &interrupt_map, sizeof(interrupt_map));
+
+    g_free(intc_name);
+    g_free(name);
+}
+
+static const void *create_fdt(BostonState *s, const MemMapEntry *memmap, int *dt_size)
+{
+    void *fdt;
+    int cpu;
+    MachineState *mc = s->mach;
+    uint32_t platreg_ph, gic_ph, clk_ph;
+    char *name, *gic_name, *platreg_name, *stdout_name;
+
+    fdt = create_device_tree(dt_size);
+    if (!fdt) {
+        error_report("create_device_tree() failed");
+        exit(1);
+    }
+
+    platreg_ph = qemu_fdt_alloc_phandle(fdt);
+    gic_ph = qemu_fdt_alloc_phandle(fdt);
+    clk_ph = qemu_fdt_alloc_phandle(fdt);
+
+    qemu_fdt_setprop_string(fdt, "/", "model", "img,boston");
+    qemu_fdt_setprop_string(fdt, "/", "compatible", "img,boston");
+    qemu_fdt_setprop_cell(fdt, "/", "#size-cells", 0x1);
+    qemu_fdt_setprop_cell(fdt, "/", "#address-cells", 0x1);
+
+
+    qemu_fdt_add_subnode(fdt, "/cpus");
+    qemu_fdt_setprop_cell(fdt, "/cpus", "#size-cells", 0x0);
+    qemu_fdt_setprop_cell(fdt, "/cpus", "#address-cells", 0x1);
+
+    for (cpu = 0; cpu < mc->smp.cpus; cpu++) {
+        name = g_strdup_printf("/cpus/cpu@%d", cpu);
+        qemu_fdt_add_subnode(fdt, name);
+        qemu_fdt_setprop_string(fdt, name, "compatible", "img,mips");
+        qemu_fdt_setprop_string(fdt, name, "status", "okay");
+        qemu_fdt_setprop_cell(fdt, name, "reg", cpu);
+        qemu_fdt_setprop_string(fdt, name, "device_type", "cpu");
+        qemu_fdt_setprop_cells(fdt, name, "clocks", clk_ph, FDT_BOSTON_CLK_CPU);
+        g_free(name);
+    }
+
+    qemu_fdt_add_subnode(fdt, "/soc");
+    qemu_fdt_setprop(fdt, "/soc", "ranges", NULL, 0);
+    qemu_fdt_setprop_string(fdt, "/soc", "compatible", "simple-bus");
+    qemu_fdt_setprop_cell(fdt, "/soc", "#size-cells", 0x1);
+    qemu_fdt_setprop_cell(fdt, "/soc", "#address-cells", 0x1);
+
+    fdt_create_pcie(fdt, gic_ph, 2, memmap[BOSTON_PCIE0].base, memmap[BOSTON_PCIE0].size,
+                    memmap[BOSTON_PCIE0_MMIO].base, memmap[BOSTON_PCIE0_MMIO].size);
+
+    fdt_create_pcie(fdt, gic_ph, 1, memmap[BOSTON_PCIE1].base, memmap[BOSTON_PCIE1].size,
+                    memmap[BOSTON_PCIE1_MMIO].base, memmap[BOSTON_PCIE1_MMIO].size);
+
+    fdt_create_pcie(fdt, gic_ph, 0, memmap[BOSTON_PCIE2].base, memmap[BOSTON_PCIE2].size,
+                    memmap[BOSTON_PCIE2_MMIO].base, memmap[BOSTON_PCIE2_MMIO].size);
+
+    /* GIC with it's timer node */
+    gic_name = g_strdup_printf("/soc/interrupt-controller@%lx",
+                                (long)memmap[BOSTON_GIC].base);
+    qemu_fdt_add_subnode(fdt, gic_name);
+    qemu_fdt_setprop_string(fdt, gic_name, "compatible", "mti,gic");
+    qemu_fdt_setprop_cells(fdt, gic_name, "reg", memmap[BOSTON_GIC].base,
+                            memmap[BOSTON_GIC].size);
+    qemu_fdt_setprop(fdt, gic_name, "interrupt-controller", NULL, 0);
+    qemu_fdt_setprop_cell(fdt, gic_name, "#interrupt-cells", 3);
+    qemu_fdt_setprop_cell(fdt, gic_name, "phandle", gic_ph);
+
+    name = g_strdup_printf("%s/timer", gic_name);
+    qemu_fdt_add_subnode(fdt, name);
+    qemu_fdt_setprop_string(fdt, name, "compatible", "mti,gic-timer");
+    qemu_fdt_setprop_cells(fdt, name, "interrupts", FDT_GIC_LOCAL, 1,
+                            FDT_IRQ_TYPE_NONE);
+    qemu_fdt_setprop_cells(fdt, name, "clocks", clk_ph, FDT_BOSTON_CLK_CPU);
+    g_free(name);
+    g_free(gic_name);
+
+    /* CDMM node */
+    name = g_strdup_printf("/soc/cdmm@%lx", (long)memmap[BOSTON_CDMM].base);
+    qemu_fdt_add_subnode(fdt, name);
+    qemu_fdt_setprop_string(fdt, name, "compatible", "mti,mips-cdmm");
+    qemu_fdt_setprop_cells(fdt, name, "reg", memmap[BOSTON_CDMM].base,
+                            memmap[BOSTON_CDMM].size);
+    g_free(name);
+
+    /* CPC node */
+    name = g_strdup_printf("/soc/cpc@%lx", (long)memmap[BOSTON_CPC].base);
+    qemu_fdt_add_subnode(fdt, name);
+    qemu_fdt_setprop_string(fdt, name, "compatible", "mti,mips-cpc");
+    qemu_fdt_setprop_cells(fdt, name, "reg", memmap[BOSTON_CPC].base,
+                            memmap[BOSTON_CPC].size);
+    g_free(name);
+
+    /* platreg and it's clk node */
+    platreg_name = g_strdup_printf("/soc/system-controller@%lx",
+                                    (long)memmap[BOSTON_PLATREG].base);
+    qemu_fdt_add_subnode(fdt, platreg_name);
+    {
+        static const char * const compat[2] = {"img,boston-platform-regs", "syscon"};
+        qemu_fdt_setprop_string_array(fdt, platreg_name, "compatible", (char **)&compat,
+                                      ARRAY_SIZE(compat));
+    }
+    qemu_fdt_setprop_cells(fdt, platreg_name, "reg", memmap[BOSTON_PLATREG].base,
+                            memmap[BOSTON_PLATREG].size);
+    qemu_fdt_setprop_cell(fdt, platreg_name, "phandle", platreg_ph);
+
+    name = g_strdup_printf("%s/clock", platreg_name);
+    qemu_fdt_add_subnode(fdt, name);
+    qemu_fdt_setprop_string(fdt, name, "compatible", "img,boston-clock");
+    qemu_fdt_setprop_cell(fdt, name, "#clock-cells", 1);
+    qemu_fdt_setprop_cell(fdt, name, "phandle", clk_ph);
+    g_free(name);
+    g_free(platreg_name);
+
+    /* reboot node */
+    name = g_strdup_printf("/soc/reboot");
+    qemu_fdt_add_subnode(fdt, name);
+    qemu_fdt_setprop_string(fdt, name, "compatible", "syscon-reboot");
+    qemu_fdt_setprop_cell(fdt, name, "regmap", platreg_ph);
+    qemu_fdt_setprop_cell(fdt, name, "offset", 0x10);
+    qemu_fdt_setprop_cell(fdt, name, "mask", 0x10);
+    g_free(name);
+
+    /* uart node */
+    name = g_strdup_printf("/soc/uart@%lx", (long)memmap[BOSTON_UART].base);
+    qemu_fdt_add_subnode(fdt, name);
+    qemu_fdt_setprop_string(fdt, name, "compatible", "ns16550a");
+    qemu_fdt_setprop_cells(fdt, name, "reg", memmap[BOSTON_UART].base,
+                            memmap[BOSTON_UART].size);
+    qemu_fdt_setprop_cell(fdt, name, "reg-shift", 0x2);
+    qemu_fdt_setprop_cell(fdt, name, "interrupt-parent", gic_ph);
+    qemu_fdt_setprop_cells(fdt, name, "interrupts", FDT_GIC_SHARED, 3,
+                            FDT_IRQ_TYPE_LEVEL_HIGH);
+    qemu_fdt_setprop_cells(fdt, name, "clocks", clk_ph, FDT_BOSTON_CLK_SYS);
+
+    qemu_fdt_add_subnode(fdt, "/chosen");
+    stdout_name = g_strdup_printf("%s:115200", name);
+    qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", stdout_name);
+    g_free(stdout_name);
+    g_free(name);
+
+    /* lcd node */
+    name = g_strdup_printf("/soc/lcd@%lx", (long)memmap[BOSTON_LCD].base);
+    qemu_fdt_add_subnode(fdt, name);
+    qemu_fdt_setprop_string(fdt, name, "compatible", "img,boston-lcd");
+    qemu_fdt_setprop_cells(fdt, name, "reg", memmap[BOSTON_LCD].base,
+                            memmap[BOSTON_LCD].size);
+    g_free(name);
+
+    name = g_strdup_printf("/memory@0");
+    qemu_fdt_add_subnode(fdt, name);
+    qemu_fdt_setprop_string(fdt, name, "device_type", "memory");
+    g_free(name);
+
+    return fdt;
+}
+
 static void boston_mach_init(MachineState *machine)
 {
     DeviceState *dev;
@@ -556,23 +771,26 @@ static void boston_mach_init(MachineState *machine)
                            NULL, 0, EM_MIPS, 1, 0);
 
         if (kernel_size) {
+            int dt_size;
+            const void *dtb_file_data, *dtb_load_data;
             hwaddr dtb_paddr = QEMU_ALIGN_UP(kernel_high, 64 * KiB);
             hwaddr dtb_vaddr = cpu_mips_phys_to_kseg0(NULL, dtb_paddr);
 
             s->kernel_entry = kernel_entry;
-            if (machine->dtb) {
-                int dt_size;
-                const void *dtb_file_data, *dtb_load_data;
 
+            if (machine->dtb) {
                 dtb_file_data = load_device_tree(machine->dtb, &dt_size);
-                dtb_load_data = boston_fdt_filter(s, dtb_file_data, NULL, &dtb_vaddr);
-
-                /* Calculate real fdt size after filter */
-                dt_size = fdt_totalsize(dtb_load_data);
-                rom_add_blob_fixed("dtb", dtb_load_data, dt_size, dtb_paddr);
-                g_free((void *) dtb_file_data);
-                g_free((void *) dtb_load_data);
+            } else {
+                dtb_file_data = create_fdt(s, boston_memmap, &dt_size);
             }
+
+            dtb_load_data = boston_fdt_filter(s, dtb_file_data, NULL, &dtb_vaddr);
+
+            /* Calculate real fdt size after filter */
+            dt_size = fdt_totalsize(dtb_load_data);
+            rom_add_blob_fixed("dtb", dtb_load_data, dt_size, dtb_paddr);
+            g_free((void *) dtb_file_data);
+            g_free((void *) dtb_load_data);
         } else {
             /* Try to load file as FIT */
             fit_err = load_fit(&boston_fit_loader, machine->kernel_filename, s);
-- 
2.32.0

Re: [PATCH 3/3] hw/mips/boston: Add FDT generator

[Philippe Mathieu-Daudé]

Hi Jiaxun,

Cc'ing David (FDT) / Huacai for review help.

On 7/29/21 5:39 AM, Jiaxun Yang wrote:
> Generate FDT on our own if no dtb argument supplied.
> Avoid introduce unused device in FDT with user supplied dtb.

"Avoid introducing"

> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>  hw/mips/boston.c | 238 +++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 228 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/mips/boston.c b/hw/mips/boston.c
> index 42b31a1ce4..aaa79b9da7 100644
> --- a/hw/mips/boston.c
> +++ b/hw/mips/boston.c
> @@ -49,6 +49,13 @@ typedef struct BostonState BostonState;
>  DECLARE_INSTANCE_CHECKER(BostonState, BOSTON,
>                           TYPE_BOSTON)
>  
> +#define FDT_IRQ_TYPE_NONE       0
> +#define FDT_IRQ_TYPE_LEVEL_HIGH 4
> +#define FDT_GIC_SHARED          0
> +#define FDT_GIC_LOCAL           1
> +#define FDT_BOSTON_CLK_SYS      1
> +#define FDT_BOSTON_CLK_CPU      2
> +
>  struct BostonState {
>      SysBusDevice parent_obj;
>  
> @@ -435,6 +442,214 @@ xilinx_pcie_init(MemoryRegion *sys_mem, uint32_t bus_nr,
>      return XILINX_PCIE_HOST(dev);
>  }
>  
> +
> +static void fdt_create_pcie(void *fdt, int gic_ph, int irq, hwaddr reg_base,
> +                            hwaddr reg_size, hwaddr mmio_base, hwaddr mmio_size)
> +{
> +    int i;
> +    char *name, *intc_name;
> +    uint32_t intc_ph;
> +    uint32_t interrupt_map[4][6];

Could you add definitions for 4 / 6?

> +
> +    intc_ph = qemu_fdt_alloc_phandle(fdt);
> +    name = g_strdup_printf("/soc/pci@%lx", (long)reg_base);
> +    qemu_fdt_add_subnode(fdt, name);
> +    qemu_fdt_setprop_string(fdt, name, "compatible", "xlnx,axi-pcie-host-1.00.a");
> +    qemu_fdt_setprop_string(fdt, name, "device_type", "pci");
> +    qemu_fdt_setprop_cells(fdt, name, "reg", reg_base, reg_size);
> +
> +    qemu_fdt_setprop_cell(fdt, name, "#address-cells", 3);
> +    qemu_fdt_setprop_cell(fdt, name, "#size-cells", 2);
> +    qemu_fdt_setprop_cell(fdt, name, "#interrupt-cells", 1);
> +
> +    qemu_fdt_setprop_cell(fdt, name, "interrupt-parent", gic_ph);
> +    qemu_fdt_setprop_cells(fdt, name, "interrupts", FDT_GIC_SHARED, irq,
> +                            FDT_IRQ_TYPE_LEVEL_HIGH);
> +
> +    qemu_fdt_setprop_cells(fdt, name, "ranges", 0x02000000, 0, mmio_base,
> +                            mmio_base, 0, mmio_size);
> +    qemu_fdt_setprop_cells(fdt, name, "bus-range", 0x00, 0xff);
> +
> +
> +
> +    intc_name = g_strdup_printf("%s/interrupt-controller", name);
> +    qemu_fdt_add_subnode(fdt, intc_name);
> +    qemu_fdt_setprop(fdt, intc_name, "interrupt-controller", NULL, 0);
> +    qemu_fdt_setprop_cell(fdt, intc_name, "#address-cells", 0);
> +    qemu_fdt_setprop_cell(fdt, intc_name, "#interrupt-cells", 1);
> +    qemu_fdt_setprop_cell(fdt, intc_name, "phandle", intc_ph);
> +
> +    qemu_fdt_setprop_cells(fdt, name, "interrupt-map-mask", 0, 0, 0, 7);
> +    for (i = 0; i < 4; i++) {

4 -> ARRAY_SIZE(interrupt_map)

> +        uint32_t *irqmap = interrupt_map[i];
> +
> +        irqmap[0] = cpu_to_be32(0);
> +        irqmap[1] = cpu_to_be32(0);
> +        irqmap[2] = cpu_to_be32(0);
> +        irqmap[3] = cpu_to_be32(i + 1);
> +        irqmap[4] = cpu_to_be32(intc_ph);
> +        irqmap[5] = cpu_to_be32(i + 1);
> +    }
> +    qemu_fdt_setprop(fdt, name, "interrupt-map", &interrupt_map, sizeof(interrupt_map));
> +
> +    g_free(intc_name);
> +    g_free(name);
> +}
> +
> +static const void *create_fdt(BostonState *s, const MemMapEntry *memmap, int *dt_size)
> +{
> +    void *fdt;
> +    int cpu;
> +    MachineState *mc = s->mach;
> +    uint32_t platreg_ph, gic_ph, clk_ph;
> +    char *name, *gic_name, *platreg_name, *stdout_name;
> +
> +    fdt = create_device_tree(dt_size);
> +    if (!fdt) {
> +        error_report("create_device_tree() failed");
> +        exit(1);
> +    }
> +
> +    platreg_ph = qemu_fdt_alloc_phandle(fdt);
> +    gic_ph = qemu_fdt_alloc_phandle(fdt);
> +    clk_ph = qemu_fdt_alloc_phandle(fdt);
> +
> +    qemu_fdt_setprop_string(fdt, "/", "model", "img,boston");
> +    qemu_fdt_setprop_string(fdt, "/", "compatible", "img,boston");
> +    qemu_fdt_setprop_cell(fdt, "/", "#size-cells", 0x1);
> +    qemu_fdt_setprop_cell(fdt, "/", "#address-cells", 0x1);
> +
> +
> +    qemu_fdt_add_subnode(fdt, "/cpus");
> +    qemu_fdt_setprop_cell(fdt, "/cpus", "#size-cells", 0x0);
> +    qemu_fdt_setprop_cell(fdt, "/cpus", "#address-cells", 0x1);
> +
> +    for (cpu = 0; cpu < mc->smp.cpus; cpu++) {
> +        name = g_strdup_printf("/cpus/cpu@%d", cpu);
> +        qemu_fdt_add_subnode(fdt, name);
> +        qemu_fdt_setprop_string(fdt, name, "compatible", "img,mips");
> +        qemu_fdt_setprop_string(fdt, name, "status", "okay");
> +        qemu_fdt_setprop_cell(fdt, name, "reg", cpu);
> +        qemu_fdt_setprop_string(fdt, name, "device_type", "cpu");
> +        qemu_fdt_setprop_cells(fdt, name, "clocks", clk_ph, FDT_BOSTON_CLK_CPU);
> +        g_free(name);
> +    }
> +
> +    qemu_fdt_add_subnode(fdt, "/soc");
> +    qemu_fdt_setprop(fdt, "/soc", "ranges", NULL, 0);
> +    qemu_fdt_setprop_string(fdt, "/soc", "compatible", "simple-bus");
> +    qemu_fdt_setprop_cell(fdt, "/soc", "#size-cells", 0x1);
> +    qemu_fdt_setprop_cell(fdt, "/soc", "#address-cells", 0x1);
> +
> +    fdt_create_pcie(fdt, gic_ph, 2, memmap[BOSTON_PCIE0].base, memmap[BOSTON_PCIE0].size,
> +                    memmap[BOSTON_PCIE0_MMIO].base, memmap[BOSTON_PCIE0_MMIO].size);
> +
> +    fdt_create_pcie(fdt, gic_ph, 1, memmap[BOSTON_PCIE1].base, memmap[BOSTON_PCIE1].size,
> +                    memmap[BOSTON_PCIE1_MMIO].base, memmap[BOSTON_PCIE1_MMIO].size);
> +
> +    fdt_create_pcie(fdt, gic_ph, 0, memmap[BOSTON_PCIE2].base, memmap[BOSTON_PCIE2].size,
> +                    memmap[BOSTON_PCIE2_MMIO].base, memmap[BOSTON_PCIE2_MMIO].size);
> +
> +    /* GIC with it's timer node */
> +    gic_name = g_strdup_printf("/soc/interrupt-controller@%lx",
> +                                (long)memmap[BOSTON_GIC].base);

No need to cast: "lx" -> HWADDR_PRIx.

> +    qemu_fdt_add_subnode(fdt, gic_name);
> +    qemu_fdt_setprop_string(fdt, gic_name, "compatible", "mti,gic");
> +    qemu_fdt_setprop_cells(fdt, gic_name, "reg", memmap[BOSTON_GIC].base,
> +                            memmap[BOSTON_GIC].size);
> +    qemu_fdt_setprop(fdt, gic_name, "interrupt-controller", NULL, 0);
> +    qemu_fdt_setprop_cell(fdt, gic_name, "#interrupt-cells", 3);
> +    qemu_fdt_setprop_cell(fdt, gic_name, "phandle", gic_ph);
> +
> +    name = g_strdup_printf("%s/timer", gic_name);
> +    qemu_fdt_add_subnode(fdt, name);
> +    qemu_fdt_setprop_string(fdt, name, "compatible", "mti,gic-timer");
> +    qemu_fdt_setprop_cells(fdt, name, "interrupts", FDT_GIC_LOCAL, 1,
> +                            FDT_IRQ_TYPE_NONE);
> +    qemu_fdt_setprop_cells(fdt, name, "clocks", clk_ph, FDT_BOSTON_CLK_CPU);
> +    g_free(name);
> +    g_free(gic_name);
> +
> +    /* CDMM node */
> +    name = g_strdup_printf("/soc/cdmm@%lx", (long)memmap[BOSTON_CDMM].base);
> +    qemu_fdt_add_subnode(fdt, name);
> +    qemu_fdt_setprop_string(fdt, name, "compatible", "mti,mips-cdmm");
> +    qemu_fdt_setprop_cells(fdt, name, "reg", memmap[BOSTON_CDMM].base,
> +                            memmap[BOSTON_CDMM].size);
> +    g_free(name);
> +
> +    /* CPC node */
> +    name = g_strdup_printf("/soc/cpc@%lx", (long)memmap[BOSTON_CPC].base);
> +    qemu_fdt_add_subnode(fdt, name);
> +    qemu_fdt_setprop_string(fdt, name, "compatible", "mti,mips-cpc");
> +    qemu_fdt_setprop_cells(fdt, name, "reg", memmap[BOSTON_CPC].base,
> +                            memmap[BOSTON_CPC].size);
> +    g_free(name);
> +
> +    /* platreg and it's clk node */
> +    platreg_name = g_strdup_printf("/soc/system-controller@%lx",
> +                                    (long)memmap[BOSTON_PLATREG].base);
> +    qemu_fdt_add_subnode(fdt, platreg_name);
> +    {
> +        static const char * const compat[2] = {"img,boston-platform-regs", "syscon"};

Why not declare in prologue?

> +        qemu_fdt_setprop_string_array(fdt, platreg_name, "compatible", (char **)&compat,
> +                                      ARRAY_SIZE(compat));
> +    }
> +    qemu_fdt_setprop_cells(fdt, platreg_name, "reg", memmap[BOSTON_PLATREG].base,
> +                            memmap[BOSTON_PLATREG].size);
> +    qemu_fdt_setprop_cell(fdt, platreg_name, "phandle", platreg_ph);
> +
> +    name = g_strdup_printf("%s/clock", platreg_name);
> +    qemu_fdt_add_subnode(fdt, name);
> +    qemu_fdt_setprop_string(fdt, name, "compatible", "img,boston-clock");
> +    qemu_fdt_setprop_cell(fdt, name, "#clock-cells", 1);
> +    qemu_fdt_setprop_cell(fdt, name, "phandle", clk_ph);
> +    g_free(name);
> +    g_free(platreg_name);
> +
> +    /* reboot node */
> +    name = g_strdup_printf("/soc/reboot");
> +    qemu_fdt_add_subnode(fdt, name);
> +    qemu_fdt_setprop_string(fdt, name, "compatible", "syscon-reboot");
> +    qemu_fdt_setprop_cell(fdt, name, "regmap", platreg_ph);
> +    qemu_fdt_setprop_cell(fdt, name, "offset", 0x10);
> +    qemu_fdt_setprop_cell(fdt, name, "mask", 0x10);
> +    g_free(name);
> +
> +    /* uart node */
> +    name = g_strdup_printf("/soc/uart@%lx", (long)memmap[BOSTON_UART].base);
> +    qemu_fdt_add_subnode(fdt, name);
> +    qemu_fdt_setprop_string(fdt, name, "compatible", "ns16550a");
> +    qemu_fdt_setprop_cells(fdt, name, "reg", memmap[BOSTON_UART].base,
> +                            memmap[BOSTON_UART].size);
> +    qemu_fdt_setprop_cell(fdt, name, "reg-shift", 0x2);
> +    qemu_fdt_setprop_cell(fdt, name, "interrupt-parent", gic_ph);
> +    qemu_fdt_setprop_cells(fdt, name, "interrupts", FDT_GIC_SHARED, 3,
> +                            FDT_IRQ_TYPE_LEVEL_HIGH);
> +    qemu_fdt_setprop_cells(fdt, name, "clocks", clk_ph, FDT_BOSTON_CLK_SYS);
> +
> +    qemu_fdt_add_subnode(fdt, "/chosen");
> +    stdout_name = g_strdup_printf("%s:115200", name);
> +    qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", stdout_name);
> +    g_free(stdout_name);
> +    g_free(name);
> +
> +    /* lcd node */
> +    name = g_strdup_printf("/soc/lcd@%lx", (long)memmap[BOSTON_LCD].base);
> +    qemu_fdt_add_subnode(fdt, name);
> +    qemu_fdt_setprop_string(fdt, name, "compatible", "img,boston-lcd");
> +    qemu_fdt_setprop_cells(fdt, name, "reg", memmap[BOSTON_LCD].base,
> +                            memmap[BOSTON_LCD].size);
> +    g_free(name);
> +
> +    name = g_strdup_printf("/memory@0");
> +    qemu_fdt_add_subnode(fdt, name);
> +    qemu_fdt_setprop_string(fdt, name, "device_type", "memory");
> +    g_free(name);
> +
> +    return fdt;
> +}
> +
>  static void boston_mach_init(MachineState *machine)
>  {
>      DeviceState *dev;
> @@ -556,23 +771,26 @@ static void boston_mach_init(MachineState *machine)
>                             NULL, 0, EM_MIPS, 1, 0);
>  
>          if (kernel_size) {
> +            int dt_size;
> +            const void *dtb_file_data, *dtb_load_data;
>              hwaddr dtb_paddr = QEMU_ALIGN_UP(kernel_high, 64 * KiB);
>              hwaddr dtb_vaddr = cpu_mips_phys_to_kseg0(NULL, dtb_paddr);
>  
>              s->kernel_entry = kernel_entry;
> -            if (machine->dtb) {
> -                int dt_size;
> -                const void *dtb_file_data, *dtb_load_data;
>  
> +            if (machine->dtb) {
>                  dtb_file_data = load_device_tree(machine->dtb, &dt_size);
> -                dtb_load_data = boston_fdt_filter(s, dtb_file_data, NULL, &dtb_vaddr);
> -
> -                /* Calculate real fdt size after filter */
> -                dt_size = fdt_totalsize(dtb_load_data);
> -                rom_add_blob_fixed("dtb", dtb_load_data, dt_size, dtb_paddr);
> -                g_free((void *) dtb_file_data);
> -                g_free((void *) dtb_load_data);
> +            } else {
> +                dtb_file_data = create_fdt(s, boston_memmap, &dt_size);
>              }
> +
> +            dtb_load_data = boston_fdt_filter(s, dtb_file_data, NULL, &dtb_vaddr);
> +
> +            /* Calculate real fdt size after filter */
> +            dt_size = fdt_totalsize(dtb_load_data);
> +            rom_add_blob_fixed("dtb", dtb_load_data, dt_size, dtb_paddr);
> +            g_free((void *) dtb_file_data);
> +            g_free((void *) dtb_load_data);

(see previous patch, no need to cast).

>          } else {
>              /* Try to load file as FIT */
>              fit_err = load_fit(&boston_fit_loader, machine->kernel_filename, s);
> 

Few comments but overall looks good.

Regards,

Phil.