| Message ID | 20220210063009.1048751-4-shorne@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | OpenRISC Device Tree Support | expand |
Typo "device" in subject. On 10/2/22 07:30, Stafford Horne wrote: > Using the device tree means that qemu can now directly tell > the kernel what hardware is configured rather than use having > to maintain and update a separate device tree file. > > This patch adds device tree support for the OpenRISC simulator. > A device tree is built up based on the state of the configure > openrisc simulator. > > This is then dumpt to memory and the load address is passed to the "dumped"? > kernel in register r3. > > Signed-off-by: Stafford Horne <shorne@gmail.com> > --- > hw/openrisc/openrisc_sim.c | 158 ++++++++++++++++++++++++++++++++++++- > 1 file changed, 154 insertions(+), 4 deletions(-) > > diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c > index 5a0cc4d27e..d7c26af82c 100644 > --- a/hw/openrisc/openrisc_sim.c > +++ b/hw/openrisc/openrisc_sim.c > @@ -29,14 +29,20 @@ > #include "net/net.h" > #include "hw/loader.h" > #include "hw/qdev-properties.h" > +#include "exec/address-spaces.h" > +#include "sysemu/device_tree.h" > #include "sysemu/sysemu.h" > #include "hw/sysbus.h" > #include "sysemu/qtest.h" > #include "sysemu/reset.h" > #include "hw/core/split-irq.h" > > +#include <libfdt.h> Watch out, you now need to add TARGET_NEED_FDT=y to configs/targets/or1k-softmmu.mak.
On Thu, Feb 10, 2022 at 12:10:54PM +0100, Philippe Mathieu-Daudé wrote: > Typo "device" in subject. OK. > On 10/2/22 07:30, Stafford Horne wrote: > > Using the device tree means that qemu can now directly tell > > the kernel what hardware is configured rather than use having > > to maintain and update a separate device tree file. > > > > This patch adds device tree support for the OpenRISC simulator. > > A device tree is built up based on the state of the configure > > openrisc simulator. > > > > This is then dumpt to memory and the load address is passed to the > > "dumped"? Yes. > > kernel in register r3. > > > > Signed-off-by: Stafford Horne <shorne@gmail.com> > > --- > > hw/openrisc/openrisc_sim.c | 158 ++++++++++++++++++++++++++++++++++++- > > 1 file changed, 154 insertions(+), 4 deletions(-) > > > > diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c > > index 5a0cc4d27e..d7c26af82c 100644 > > --- a/hw/openrisc/openrisc_sim.c > > +++ b/hw/openrisc/openrisc_sim.c > > @@ -29,14 +29,20 @@ > > #include "net/net.h" > > #include "hw/loader.h" > > #include "hw/qdev-properties.h" > > +#include "exec/address-spaces.h" > > +#include "sysemu/device_tree.h" > > #include "sysemu/sysemu.h" > > #include "hw/sysbus.h" > > #include "sysemu/qtest.h" > > #include "sysemu/reset.h" > > #include "hw/core/split-irq.h" > > +#include <libfdt.h> > > Watch out, you now need to add TARGET_NEED_FDT=y > to configs/targets/or1k-softmmu.mak. OK, I will add it in this patch. I missed that part.
On Thu, 10 Feb 2022 at 06:46, Stafford Horne <shorne@gmail.com> wrote: > > Using the device tree means that qemu can now directly tell > the kernel what hardware is configured rather than use having > to maintain and update a separate device tree file. > > This patch adds device tree support for the OpenRISC simulator. > A device tree is built up based on the state of the configure > openrisc simulator. This sounds like it's support for creating a device tree? Support for loading a device tree would be "the user passes us a filename of a dtb file". (This is mostly a quibble about commit message wording.) > -static void openrisc_load_kernel(ram_addr_t ram_size, > +static hwaddr openrisc_load_kernel(ram_addr_t ram_size, > const char *kernel_filename) Indentation looks off now ? > { > long kernel_size; > uint64_t elf_entry; > + uint64_t high_addr; > hwaddr entry; > > if (kernel_filename && !qtest_enabled()) { > kernel_size = load_elf(kernel_filename, NULL, NULL, NULL, > - &elf_entry, NULL, NULL, NULL, 1, EM_OPENRISC, > - 1, 0); > + &elf_entry, NULL, &high_addr, NULL, 1, > + EM_OPENRISC, 1, 0); > entry = elf_entry; > if (kernel_size < 0) { > kernel_size = load_uimage(kernel_filename, > &entry, NULL, NULL, NULL, NULL); > + high_addr = entry + kernel_size; > } > if (kernel_size < 0) { > kernel_size = load_image_targphys(kernel_filename, > KERNEL_LOAD_ADDR, > ram_size - KERNEL_LOAD_ADDR); > + high_addr = KERNEL_LOAD_ADDR + kernel_size; > } > > if (entry <= 0) { > @@ -168,7 +181,139 @@ static void openrisc_load_kernel(ram_addr_t ram_size, > exit(1); > } > boot_info.bootstrap_pc = entry; > + > + return high_addr; > + } > + return 0; > +} > + > +static uint32_t openrisc_load_fdt(Or1ksimState *s, hwaddr load_start, > + uint64_t mem_size) Indentation again. > +{ > + uint32_t fdt_addr; > + int fdtsize = fdt_totalsize(s->fdt); > + > + if (fdtsize <= 0) { > + error_report("invalid device-tree"); > + exit(1); > + } > + > + /* We should put fdt right after the kernel */ You change this comment in patch 4 -- I think you might as well just use that text in this patch to start with. > + fdt_addr = ROUND_UP(load_start, 4); > + > + fdt_pack(s->fdt); fdt_pack() returns an error code -- you should check it. > + /* copy in the device tree */ > + qemu_fdt_dumpdtb(s->fdt, fdtsize); > + > + rom_add_blob_fixed_as("fdt", s->fdt, fdtsize, fdt_addr, > + &address_space_memory); > + > + return fdt_addr; > +} > + > +static void openrisc_create_fdt(Or1ksimState *s, > + const struct MemmapEntry *memmap, int num_cpus, uint64_t mem_size, > + const char *cmdline) Indentation. > +{ > + void *fdt; > + int cpu; > + char *nodename; > + int pic_ph; > + > + fdt = s->fdt = create_device_tree(&s->fdt_size); > + if (!fdt) { > + error_report("create_device_tree() failed"); > + exit(1); > + } > + > + qemu_fdt_setprop_string(fdt, "/", "compatible", "opencores,or1ksim"); > + qemu_fdt_setprop_cell(fdt, "/", "#address-cells", 0x1); > + qemu_fdt_setprop_cell(fdt, "/", "#size-cells", 0x1); > + > + nodename = g_strdup_printf("/memory@%lx", > + (long)memmap[OR1KSIM_DRAM].base); Use the appropriate format string macro for the type, rather than casting to long (here and below). > + qemu_fdt_add_subnode(fdt, nodename); > + qemu_fdt_setprop_cells(fdt, nodename, "reg", > + memmap[OR1KSIM_DRAM].base, mem_size); > + qemu_fdt_setprop_string(fdt, nodename, "device_type", "memory"); > + g_free(nodename); > + > + 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 < num_cpus; cpu++) { > + nodename = g_strdup_printf("/cpus/cpu@%d", cpu); > + qemu_fdt_add_subnode(fdt, nodename); > + qemu_fdt_setprop_string(fdt, nodename, "compatible", > + "opencores,or1200-rtlsvn481"); > + qemu_fdt_setprop_cell(fdt, nodename, "reg", cpu); > + qemu_fdt_setprop_cell(fdt, nodename, "clock-frequency", > + OR1KSIM_CLK_MHZ); > + g_free(nodename); > + } > + > + if (num_cpus > 0) { > + nodename = g_strdup_printf("/ompic@%lx", > + (long)memmap[OR1KSIM_OMPIC].base); > + qemu_fdt_add_subnode(fdt, nodename); > + qemu_fdt_setprop_string(fdt, nodename, "compatible", "openrisc,ompic"); > + qemu_fdt_setprop_cells(fdt, nodename, "reg", > + memmap[OR1KSIM_OMPIC].base, > + memmap[OR1KSIM_OMPIC].size); > + qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0); > + qemu_fdt_setprop_cell(fdt, nodename, "#interrupt-cells", 0); > + qemu_fdt_setprop_cell(fdt, nodename, "interrupts", OR1KSIM_OMPIC_IRQ); > + g_free(nodename); > } > + > + nodename = (char *)"/pic"; > + qemu_fdt_add_subnode(fdt, nodename); > + pic_ph = qemu_fdt_alloc_phandle(fdt); > + qemu_fdt_setprop_string(fdt, nodename, "compatible", > + "opencores,or1k-pic-level"); > + qemu_fdt_setprop_cell(fdt, nodename, "#interrupt-cells", 1); > + qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0); > + qemu_fdt_setprop_cell(fdt, nodename, "phandle", pic_ph); > + > + qemu_fdt_setprop_cell(fdt, "/", "interrupt-parent", pic_ph); > + > + if (nd_table[0].used) { > + nodename = g_strdup_printf("/ethoc@%lx", > + (long)memmap[OR1KSIM_ETHOC].base); > + qemu_fdt_add_subnode(fdt, nodename); > + qemu_fdt_setprop_string(fdt, nodename, "compatible", "opencores,ethoc"); > + qemu_fdt_setprop_cells(fdt, nodename, "reg", > + memmap[OR1KSIM_ETHOC].base, > + memmap[OR1KSIM_ETHOC].size); > + qemu_fdt_setprop_cell(fdt, nodename, "interrupts", OR1KSIM_ETHOC_IRQ); > + qemu_fdt_setprop(fdt, nodename, "big-endian", NULL, 0); > + > + qemu_fdt_add_subnode(fdt, "/aliases"); > + qemu_fdt_setprop_string(fdt, "/aliases", "enet0", nodename); > + g_free(nodename); > + } > + > + nodename = g_strdup_printf("/serial@%lx", > + (long)memmap[OR1KSIM_UART].base); > + qemu_fdt_add_subnode(fdt, nodename); > + qemu_fdt_setprop_string(fdt, nodename, "compatible", "ns16550a"); > + qemu_fdt_setprop_cells(fdt, nodename, "reg", > + memmap[OR1KSIM_UART].base, > + memmap[OR1KSIM_UART].size); > + qemu_fdt_setprop_cell(fdt, nodename, "interrupts", OR1KSIM_UART_IRQ); > + qemu_fdt_setprop_cell(fdt, nodename, "clock-frequency", OR1KSIM_CLK_MHZ); > + qemu_fdt_setprop(fdt, nodename, "big-endian", NULL, 0); > + > + qemu_fdt_add_subnode(fdt, "/chosen"); > + qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", nodename); > + if (cmdline) { > + qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline); > + } > + > + qemu_fdt_setprop_string(fdt, "/aliases", "uart0", nodename); I think the pattern the hw/arm/virt.c code uses, where the board code functions that create the UART, interrupt controller, etc devices on the board also have the code to add FDT nodes for them. Especially as the board gets new devices added over time, it's easier to check that the FDT nodes and the devices match up, and you don't have to awkwardly duplicate control-flow logic like "we only have an ethernet device if nd_table[0].used". But I won't insist that you rewrite this. > + g_free(nodename); > } > > static void openrisc_sim_init(MachineState *machine) > @@ -178,6 +323,7 @@ static void openrisc_sim_init(MachineState *machine) > OpenRISCCPU *cpus[2] = {}; > Or1ksimState *s = OR1KSIM_MACHINE(machine); > MemoryRegion *ram; > + hwaddr load_addr; > qemu_irq serial_irq; > int n; > unsigned int smp_cpus = machine->smp.cpus; > @@ -219,7 +365,11 @@ static void openrisc_sim_init(MachineState *machine) > serial_mm_init(get_system_memory(), or1ksim_memmap[OR1KSIM_UART].base, 0, > serial_irq, 115200, serial_hd(0), DEVICE_NATIVE_ENDIAN); > > - openrisc_load_kernel(ram_size, kernel_filename); > + openrisc_create_fdt(s, or1ksim_memmap, smp_cpus, machine->ram_size, > + machine->kernel_cmdline); > + > + load_addr = openrisc_load_kernel(ram_size, kernel_filename); > + boot_info.fdt_addr = openrisc_load_fdt(s, load_addr, machine->ram_size); > } If the user doesn't specify a kernel file, we'll still load the FDT, at address zero. Is that sensible/intended behaviour ? thanks -- PMM
On Thu, Feb 17, 2022 at 06:18:58PM +0000, Peter Maydell wrote: > On Thu, 10 Feb 2022 at 06:46, Stafford Horne <shorne@gmail.com> wrote: > > > > Using the device tree means that qemu can now directly tell > > the kernel what hardware is configured rather than use having > > to maintain and update a separate device tree file. > > > > This patch adds device tree support for the OpenRISC simulator. > > A device tree is built up based on the state of the configure > > openrisc simulator. > > This sounds like it's support for creating a device > tree? Support for loading a device tree would be "the > user passes us a filename of a dtb file". (This is mostly a > quibble about commit message wording.) Ah, yes I will fix this to say, "adds automatic device tree generation support" .... > > -static void openrisc_load_kernel(ram_addr_t ram_size, > > +static hwaddr openrisc_load_kernel(ram_addr_t ram_size, > > const char *kernel_filename) > > Indentation looks off now ? Fixed now. > > { > > long kernel_size; > > uint64_t elf_entry; > > + uint64_t high_addr; > > hwaddr entry; > > > > if (kernel_filename && !qtest_enabled()) { > > kernel_size = load_elf(kernel_filename, NULL, NULL, NULL, > > - &elf_entry, NULL, NULL, NULL, 1, EM_OPENRISC, > > - 1, 0); > > + &elf_entry, NULL, &high_addr, NULL, 1, > > + EM_OPENRISC, 1, 0); > > entry = elf_entry; > > if (kernel_size < 0) { > > kernel_size = load_uimage(kernel_filename, > > &entry, NULL, NULL, NULL, NULL); > > + high_addr = entry + kernel_size; > > } > > if (kernel_size < 0) { > > kernel_size = load_image_targphys(kernel_filename, > > KERNEL_LOAD_ADDR, > > ram_size - KERNEL_LOAD_ADDR); > > + high_addr = KERNEL_LOAD_ADDR + kernel_size; > > } > > > > if (entry <= 0) { > > @@ -168,7 +181,139 @@ static void openrisc_load_kernel(ram_addr_t ram_size, > > exit(1); > > } > > boot_info.bootstrap_pc = entry; > > + > > + return high_addr; > > + } > > + return 0; > > +} > > + > > +static uint32_t openrisc_load_fdt(Or1ksimState *s, hwaddr load_start, > > + uint64_t mem_size) > > Indentation again. Fixed. > > +{ > > + uint32_t fdt_addr; > > + int fdtsize = fdt_totalsize(s->fdt); > > + > > + if (fdtsize <= 0) { > > + error_report("invalid device-tree"); > > + exit(1); > > + } > > + > > + /* We should put fdt right after the kernel */ > > You change this comment in patch 4 -- I think you might as well > just use that text in this patch to start with. OK, I had that at first but I did this to be more techincally correct. I will simplify as you suggest. > > + fdt_addr = ROUND_UP(load_start, 4); > > + > > + fdt_pack(s->fdt); > > fdt_pack() returns an error code -- you should check it. OK. > > + /* copy in the device tree */ > > + qemu_fdt_dumpdtb(s->fdt, fdtsize); > > + > > + rom_add_blob_fixed_as("fdt", s->fdt, fdtsize, fdt_addr, > > + &address_space_memory); > > + > > + return fdt_addr; > > +} > > + > > +static void openrisc_create_fdt(Or1ksimState *s, > > + const struct MemmapEntry *memmap, int num_cpus, uint64_t mem_size, > > + const char *cmdline) > > Indentation. Right, fixed. > > +{ > > + void *fdt; > > + int cpu; > > + char *nodename; > > + int pic_ph; > > + > > + fdt = s->fdt = create_device_tree(&s->fdt_size); > > + if (!fdt) { > > + error_report("create_device_tree() failed"); > > + exit(1); > > + } > > + > > + qemu_fdt_setprop_string(fdt, "/", "compatible", "opencores,or1ksim"); > > + qemu_fdt_setprop_cell(fdt, "/", "#address-cells", 0x1); > > + qemu_fdt_setprop_cell(fdt, "/", "#size-cells", 0x1); > > + > > + nodename = g_strdup_printf("/memory@%lx", > > + (long)memmap[OR1KSIM_DRAM].base); > > Use the appropriate format string macro for the type, rather than > casting to long (here and below). Right good point. > > + qemu_fdt_add_subnode(fdt, nodename); > > + qemu_fdt_setprop_cells(fdt, nodename, "reg", > > + memmap[OR1KSIM_DRAM].base, mem_size); > > + qemu_fdt_setprop_string(fdt, nodename, "device_type", "memory"); > > + g_free(nodename); > > + > > + 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 < num_cpus; cpu++) { > > + nodename = g_strdup_printf("/cpus/cpu@%d", cpu); > > + qemu_fdt_add_subnode(fdt, nodename); > > + qemu_fdt_setprop_string(fdt, nodename, "compatible", > > + "opencores,or1200-rtlsvn481"); > > + qemu_fdt_setprop_cell(fdt, nodename, "reg", cpu); > > + qemu_fdt_setprop_cell(fdt, nodename, "clock-frequency", > > + OR1KSIM_CLK_MHZ); > > + g_free(nodename); > > + } > > + > > + if (num_cpus > 0) { > > + nodename = g_strdup_printf("/ompic@%lx", > > + (long)memmap[OR1KSIM_OMPIC].base); > > + qemu_fdt_add_subnode(fdt, nodename); > > + qemu_fdt_setprop_string(fdt, nodename, "compatible", "openrisc,ompic"); > > + qemu_fdt_setprop_cells(fdt, nodename, "reg", > > + memmap[OR1KSIM_OMPIC].base, > > + memmap[OR1KSIM_OMPIC].size); > > + qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0); > > + qemu_fdt_setprop_cell(fdt, nodename, "#interrupt-cells", 0); > > + qemu_fdt_setprop_cell(fdt, nodename, "interrupts", OR1KSIM_OMPIC_IRQ); > > + g_free(nodename); > > } > > + > > + nodename = (char *)"/pic"; > > + qemu_fdt_add_subnode(fdt, nodename); > > + pic_ph = qemu_fdt_alloc_phandle(fdt); > > + qemu_fdt_setprop_string(fdt, nodename, "compatible", > > + "opencores,or1k-pic-level"); > > + qemu_fdt_setprop_cell(fdt, nodename, "#interrupt-cells", 1); > > + qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0); > > + qemu_fdt_setprop_cell(fdt, nodename, "phandle", pic_ph); > > + > > + qemu_fdt_setprop_cell(fdt, "/", "interrupt-parent", pic_ph); > > + > > + if (nd_table[0].used) { > > + nodename = g_strdup_printf("/ethoc@%lx", > > + (long)memmap[OR1KSIM_ETHOC].base); > > + qemu_fdt_add_subnode(fdt, nodename); > > + qemu_fdt_setprop_string(fdt, nodename, "compatible", "opencores,ethoc"); > > + qemu_fdt_setprop_cells(fdt, nodename, "reg", > > + memmap[OR1KSIM_ETHOC].base, > > + memmap[OR1KSIM_ETHOC].size); > > + qemu_fdt_setprop_cell(fdt, nodename, "interrupts", OR1KSIM_ETHOC_IRQ); > > + qemu_fdt_setprop(fdt, nodename, "big-endian", NULL, 0); > > + > > + qemu_fdt_add_subnode(fdt, "/aliases"); > > + qemu_fdt_setprop_string(fdt, "/aliases", "enet0", nodename); > > + g_free(nodename); > > + } > > + > > + nodename = g_strdup_printf("/serial@%lx", > > + (long)memmap[OR1KSIM_UART].base); > > + qemu_fdt_add_subnode(fdt, nodename); > > + qemu_fdt_setprop_string(fdt, nodename, "compatible", "ns16550a"); > > + qemu_fdt_setprop_cells(fdt, nodename, "reg", > > + memmap[OR1KSIM_UART].base, > > + memmap[OR1KSIM_UART].size); > > + qemu_fdt_setprop_cell(fdt, nodename, "interrupts", OR1KSIM_UART_IRQ); > > + qemu_fdt_setprop_cell(fdt, nodename, "clock-frequency", OR1KSIM_CLK_MHZ); > > + qemu_fdt_setprop(fdt, nodename, "big-endian", NULL, 0); > > + > > + qemu_fdt_add_subnode(fdt, "/chosen"); > > + qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", nodename); > > + if (cmdline) { > > + qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline); > > + } > > + > > + qemu_fdt_setprop_string(fdt, "/aliases", "uart0", nodename); > > I think the pattern the hw/arm/virt.c code uses, where the board > code functions that create the UART, interrupt controller, etc > devices on the board also have the code to add FDT nodes for them. > Especially as the board gets new devices added over time, it's easier > to check that the FDT nodes and the devices match up, and you don't > have to awkwardly duplicate control-flow logic like "we only have > an ethernet device if nd_table[0].used". But I won't insist that > you rewrite this. Right, this makes sense. I might as well split it out. I did it that way for initrd. I it may make sense to do for UART, OMPIC and Ethernet here. > > + g_free(nodename); > > } > > > > static void openrisc_sim_init(MachineState *machine) > > @@ -178,6 +323,7 @@ static void openrisc_sim_init(MachineState *machine) > > OpenRISCCPU *cpus[2] = {}; > > Or1ksimState *s = OR1KSIM_MACHINE(machine); > > MemoryRegion *ram; > > + hwaddr load_addr; > > qemu_irq serial_irq; > > int n; > > unsigned int smp_cpus = machine->smp.cpus; > > @@ -219,7 +365,11 @@ static void openrisc_sim_init(MachineState *machine) > > serial_mm_init(get_system_memory(), or1ksim_memmap[OR1KSIM_UART].base, 0, > > serial_irq, 115200, serial_hd(0), DEVICE_NATIVE_ENDIAN); > > > > - openrisc_load_kernel(ram_size, kernel_filename); > > + openrisc_create_fdt(s, or1ksim_memmap, smp_cpus, machine->ram_size, > > + machine->kernel_cmdline); > > + > > + load_addr = openrisc_load_kernel(ram_size, kernel_filename); > > + boot_info.fdt_addr = openrisc_load_fdt(s, load_addr, machine->ram_size); > > } > > If the user doesn't specify a kernel file, we'll still load the FDT, > at address zero. Is that sensible/intended behaviour ? Good point, I guess we can add some space to not override the interrupt vectors. The system booting with no kernel will probably not very useful anyway. But I imaging one could connect a debugger and load some binary into memory and having the FDT in memory would be helpful. So moving the fdt offset to 0 + 1-page would give enough space. Does this sound OK? -Stafford
On Thu, 17 Feb 2022 at 21:39, Stafford Horne <shorne@gmail.com> wrote: > > On Thu, Feb 17, 2022 at 06:18:58PM +0000, Peter Maydell wrote: > > If the user doesn't specify a kernel file, we'll still load the FDT, > > at address zero. Is that sensible/intended behaviour ? > > Good point, I guess we can add some space to not override the interrupt > vectors. The system booting with no kernel will probably not very useful > anyway. But I imaging one could connect a debugger and load some binary into > memory and having the FDT in memory would be helpful. > > So moving the fdt offset to 0 + 1-page would give enough space. Does this sound > OK? I don't have a strong opinion -- you can not load the fdt, or you can define a "this is probably sensible for firmware" FDT load location. If you do define something like that you should document it, though. Bear in mind that if you put the FDT at 0+1-page then it will prevent users loading a bare-metal binary with eg the generic loader that starts at address 0, though (because it will show up as two overlapping ROM blobs). So unless you have a definite use case in mind that will want the fdt in memory it might be better to not load it. You can always add code to load it later if there is a concrete requirement later. -- PMM
diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c index 5a0cc4d27e..d7c26af82c 100644 --- a/hw/openrisc/openrisc_sim.c +++ b/hw/openrisc/openrisc_sim.c @@ -29,14 +29,20 @@ #include "net/net.h" #include "hw/loader.h" #include "hw/qdev-properties.h" +#include "exec/address-spaces.h" +#include "sysemu/device_tree.h" #include "sysemu/sysemu.h" #include "hw/sysbus.h" #include "sysemu/qtest.h" #include "sysemu/reset.h" #include "hw/core/split-irq.h" +#include <libfdt.h> + #define KERNEL_LOAD_ADDR 0x100 +#define OR1KSIM_CLK_MHZ 20000000 + #define TYPE_OR1KSIM_MACHINE MACHINE_TYPE_NAME("or1k-sim") #define OR1KSIM_MACHINE(obj) \ OBJECT_CHECK(Or1ksimState, (obj), TYPE_OR1KSIM_MACHINE) @@ -46,6 +52,8 @@ typedef struct Or1ksimState { MachineState parent_obj; /*< public >*/ + void *fdt; + int fdt_size; } Or1ksimState; @@ -74,6 +82,7 @@ static const struct MemmapEntry { static struct openrisc_boot_info { uint32_t bootstrap_pc; + uint32_t fdt_addr; } boot_info; static void main_cpu_reset(void *opaque) @@ -84,6 +93,7 @@ static void main_cpu_reset(void *opaque) cpu_reset(CPU(cpu)); cpu_set_pc(cs, boot_info.bootstrap_pc); + cpu_set_gpr(&cpu->env, 3, boot_info.fdt_addr); } static qemu_irq get_cpu_irq(OpenRISCCPU *cpus[], int cpunum, int irq_pin) @@ -137,26 +147,29 @@ static void openrisc_sim_ompic_init(hwaddr base, int num_cpus, sysbus_mmio_map(s, 0, base); } -static void openrisc_load_kernel(ram_addr_t ram_size, +static hwaddr openrisc_load_kernel(ram_addr_t ram_size, const char *kernel_filename) { long kernel_size; uint64_t elf_entry; + uint64_t high_addr; hwaddr entry; if (kernel_filename && !qtest_enabled()) { kernel_size = load_elf(kernel_filename, NULL, NULL, NULL, - &elf_entry, NULL, NULL, NULL, 1, EM_OPENRISC, - 1, 0); + &elf_entry, NULL, &high_addr, NULL, 1, + EM_OPENRISC, 1, 0); entry = elf_entry; if (kernel_size < 0) { kernel_size = load_uimage(kernel_filename, &entry, NULL, NULL, NULL, NULL); + high_addr = entry + kernel_size; } if (kernel_size < 0) { kernel_size = load_image_targphys(kernel_filename, KERNEL_LOAD_ADDR, ram_size - KERNEL_LOAD_ADDR); + high_addr = KERNEL_LOAD_ADDR + kernel_size; } if (entry <= 0) { @@ -168,7 +181,139 @@ static void openrisc_load_kernel(ram_addr_t ram_size, exit(1); } boot_info.bootstrap_pc = entry; + + return high_addr; + } + return 0; +} + +static uint32_t openrisc_load_fdt(Or1ksimState *s, hwaddr load_start, + uint64_t mem_size) +{ + uint32_t fdt_addr; + int fdtsize = fdt_totalsize(s->fdt); + + if (fdtsize <= 0) { + error_report("invalid device-tree"); + exit(1); + } + + /* We should put fdt right after the kernel */ + fdt_addr = ROUND_UP(load_start, 4); + + fdt_pack(s->fdt); + /* copy in the device tree */ + qemu_fdt_dumpdtb(s->fdt, fdtsize); + + rom_add_blob_fixed_as("fdt", s->fdt, fdtsize, fdt_addr, + &address_space_memory); + + return fdt_addr; +} + +static void openrisc_create_fdt(Or1ksimState *s, + const struct MemmapEntry *memmap, int num_cpus, uint64_t mem_size, + const char *cmdline) +{ + void *fdt; + int cpu; + char *nodename; + int pic_ph; + + fdt = s->fdt = create_device_tree(&s->fdt_size); + if (!fdt) { + error_report("create_device_tree() failed"); + exit(1); + } + + qemu_fdt_setprop_string(fdt, "/", "compatible", "opencores,or1ksim"); + qemu_fdt_setprop_cell(fdt, "/", "#address-cells", 0x1); + qemu_fdt_setprop_cell(fdt, "/", "#size-cells", 0x1); + + nodename = g_strdup_printf("/memory@%lx", + (long)memmap[OR1KSIM_DRAM].base); + qemu_fdt_add_subnode(fdt, nodename); + qemu_fdt_setprop_cells(fdt, nodename, "reg", + memmap[OR1KSIM_DRAM].base, mem_size); + qemu_fdt_setprop_string(fdt, nodename, "device_type", "memory"); + g_free(nodename); + + 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 < num_cpus; cpu++) { + nodename = g_strdup_printf("/cpus/cpu@%d", cpu); + qemu_fdt_add_subnode(fdt, nodename); + qemu_fdt_setprop_string(fdt, nodename, "compatible", + "opencores,or1200-rtlsvn481"); + qemu_fdt_setprop_cell(fdt, nodename, "reg", cpu); + qemu_fdt_setprop_cell(fdt, nodename, "clock-frequency", + OR1KSIM_CLK_MHZ); + g_free(nodename); + } + + if (num_cpus > 0) { + nodename = g_strdup_printf("/ompic@%lx", + (long)memmap[OR1KSIM_OMPIC].base); + qemu_fdt_add_subnode(fdt, nodename); + qemu_fdt_setprop_string(fdt, nodename, "compatible", "openrisc,ompic"); + qemu_fdt_setprop_cells(fdt, nodename, "reg", + memmap[OR1KSIM_OMPIC].base, + memmap[OR1KSIM_OMPIC].size); + qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0); + qemu_fdt_setprop_cell(fdt, nodename, "#interrupt-cells", 0); + qemu_fdt_setprop_cell(fdt, nodename, "interrupts", OR1KSIM_OMPIC_IRQ); + g_free(nodename); } + + nodename = (char *)"/pic"; + qemu_fdt_add_subnode(fdt, nodename); + pic_ph = qemu_fdt_alloc_phandle(fdt); + qemu_fdt_setprop_string(fdt, nodename, "compatible", + "opencores,or1k-pic-level"); + qemu_fdt_setprop_cell(fdt, nodename, "#interrupt-cells", 1); + qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0); + qemu_fdt_setprop_cell(fdt, nodename, "phandle", pic_ph); + + qemu_fdt_setprop_cell(fdt, "/", "interrupt-parent", pic_ph); + + if (nd_table[0].used) { + nodename = g_strdup_printf("/ethoc@%lx", + (long)memmap[OR1KSIM_ETHOC].base); + qemu_fdt_add_subnode(fdt, nodename); + qemu_fdt_setprop_string(fdt, nodename, "compatible", "opencores,ethoc"); + qemu_fdt_setprop_cells(fdt, nodename, "reg", + memmap[OR1KSIM_ETHOC].base, + memmap[OR1KSIM_ETHOC].size); + qemu_fdt_setprop_cell(fdt, nodename, "interrupts", OR1KSIM_ETHOC_IRQ); + qemu_fdt_setprop(fdt, nodename, "big-endian", NULL, 0); + + qemu_fdt_add_subnode(fdt, "/aliases"); + qemu_fdt_setprop_string(fdt, "/aliases", "enet0", nodename); + g_free(nodename); + } + + nodename = g_strdup_printf("/serial@%lx", + (long)memmap[OR1KSIM_UART].base); + qemu_fdt_add_subnode(fdt, nodename); + qemu_fdt_setprop_string(fdt, nodename, "compatible", "ns16550a"); + qemu_fdt_setprop_cells(fdt, nodename, "reg", + memmap[OR1KSIM_UART].base, + memmap[OR1KSIM_UART].size); + qemu_fdt_setprop_cell(fdt, nodename, "interrupts", OR1KSIM_UART_IRQ); + qemu_fdt_setprop_cell(fdt, nodename, "clock-frequency", OR1KSIM_CLK_MHZ); + qemu_fdt_setprop(fdt, nodename, "big-endian", NULL, 0); + + qemu_fdt_add_subnode(fdt, "/chosen"); + qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", nodename); + if (cmdline) { + qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline); + } + + qemu_fdt_setprop_string(fdt, "/aliases", "uart0", nodename); + + g_free(nodename); } static void openrisc_sim_init(MachineState *machine) @@ -178,6 +323,7 @@ static void openrisc_sim_init(MachineState *machine) OpenRISCCPU *cpus[2] = {}; Or1ksimState *s = OR1KSIM_MACHINE(machine); MemoryRegion *ram; + hwaddr load_addr; qemu_irq serial_irq; int n; unsigned int smp_cpus = machine->smp.cpus; @@ -219,7 +365,11 @@ static void openrisc_sim_init(MachineState *machine) serial_mm_init(get_system_memory(), or1ksim_memmap[OR1KSIM_UART].base, 0, serial_irq, 115200, serial_hd(0), DEVICE_NATIVE_ENDIAN); - openrisc_load_kernel(ram_size, kernel_filename); + openrisc_create_fdt(s, or1ksim_memmap, smp_cpus, machine->ram_size, + machine->kernel_cmdline); + + load_addr = openrisc_load_kernel(ram_size, kernel_filename); + boot_info.fdt_addr = openrisc_load_fdt(s, load_addr, machine->ram_size); } static void openrisc_sim_machine_init(ObjectClass *oc, void *data)
Using the device tree means that qemu can now directly tell the kernel what hardware is configured rather than use having to maintain and update a separate device tree file. This patch adds device tree support for the OpenRISC simulator. A device tree is built up based on the state of the configure openrisc simulator. This is then dumpt to memory and the load address is passed to the kernel in register r3. Signed-off-by: Stafford Horne <shorne@gmail.com> --- hw/openrisc/openrisc_sim.c | 158 ++++++++++++++++++++++++++++++++++++- 1 file changed, 154 insertions(+), 4 deletions(-)