| Message ID | 1328055113-30031-1-git-send-email-grant.likely@secretlab.ca |
|---|---|
| State | New |
| Headers | show |
On 01.02.2012, at 01:11, Grant Likely wrote: > If compiled with CONFIG_FDT, allow user to specify a device tree file using > the -dtb argument. If the machine supports it then the dtb will be loaded > into memory and passed to the kernel on boot. > > v2: - Enable for all arm platforms by making arm_boot use the filename directly > - Fix style issues > > Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com> > Signed-off-by: Grant Likely <grant.likely@secretlab.ca> > Cc: Paul Brook <paul@codesourcery.com> > Cc: Peter Maydell <peter.maydell@linaro.org> > Cc: Rob Herring <rob.herring@calxeda.com> > Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com> > --- > Makefile.target | 1 + > configure | 1 + > hw/arm_boot.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++---- > hw/boards.h | 1 + > qemu-options.hx | 9 +++++ > vl.c | 9 +++++ > 6 files changed, 113 insertions(+), 8 deletions(-) > > diff --git a/Makefile.target b/Makefile.target > index 68481a3..5e465ec 100644 > --- a/Makefile.target > +++ b/Makefile.target > @@ -363,6 +363,7 @@ obj-arm-y += vexpress.o > obj-arm-y += strongarm.o > obj-arm-y += collie.o > obj-arm-y += pl041.o lm4549.o > +obj-arm-$(CONFIG_FDT) += device_tree.o > > obj-sh4-y = shix.o r2d.o sh7750.o sh7750_regnames.o tc58128.o > obj-sh4-y += sh_timer.o sh_serial.o sh_intc.o sh_pci.o sm501.o > diff --git a/configure b/configure > index f69e08f..2856897 100755 > --- a/configure > +++ b/configure > @@ -3411,6 +3411,7 @@ case "$target_arch2" in > gdb_xml_files="arm-core.xml arm-vfp.xml arm-vfp3.xml arm-neon.xml" > target_phys_bits=32 > target_llong_alignment=4 > + target_libs_softmmu="$fdt_libs" > ;; > cris) > target_nptl="yes" > diff --git a/hw/arm_boot.c b/hw/arm_boot.c > index 5f163fd..377d202 100644 > --- a/hw/arm_boot.c > +++ b/hw/arm_boot.c > @@ -7,11 +7,14 @@ > * This code is licensed under the GPL. > */ > > +#include "config.h" > #include "hw.h" > #include "arm-misc.h" > #include "sysemu.h" > +#include "boards.h" > #include "loader.h" > #include "elf.h" > +#include "device_tree.h" > > #define KERNEL_ARGS_ADDR 0x100 > #define KERNEL_LOAD_ADDR 0x00010000 > @@ -207,6 +210,71 @@ static void set_kernel_args_old(const struct arm_boot_info *info, > } > } > > +static int load_dtb(target_phys_addr_t addr, const struct arm_boot_info *binfo) > +{ > +#ifdef CONFIG_FDT > + uint32_t mem_reg_property[] = { cpu_to_be32(binfo->loader_start), > + cpu_to_be32(binfo->ram_size) }; > + void *fdt = NULL; > + char *filename; > + int size, rc; > + > + if (!current_dtb_filename) { > + return 0; > + } > + > + filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, current_dtb_filename); > + if (!filename) { > + fprintf(stderr, "Couldn't open dtb file %s\n", current_dtb_filename); > + return -1; > + } > + > + fdt = load_device_tree(filename, &size); > + if (!fdt) { > + fprintf(stderr, "Couldn't open dtb file %s\n", filename); > + g_free(filename); > + return -1; > + } > + g_free(filename); > + > + rc = qemu_devtree_setprop(fdt, "/memory", "reg", mem_reg_property, > + sizeof(mem_reg_property)); > + if (rc < 0) { > + fprintf(stderr, "couldn't set /memory/reg\n"); > + } > + > + rc = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs", > + binfo->kernel_cmdline); > + if (rc < 0) { > + fprintf(stderr, "couldn't set /chosen/bootargs\n"); > + } > + > + if (binfo->initrd_size) { > + rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-start", > + binfo->loader_start + INITRD_LOAD_ADDR); > + if (rc < 0) { > + fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n"); > + } > + > + rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-end", > + binfo->loader_start + INITRD_LOAD_ADDR + > + binfo->initrd_size); > + if (rc < 0) { > + fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n"); > + } > + } > + > + cpu_physical_memory_write(addr, fdt, size); > + > + return 0; > + > +#else > + fprintf(stderr, "Platform requested a device tree, " > + "but qemu was compiled without fdt support\n"); > + return -1; > +#endif > +} > + > static void do_cpu_reset(void *opaque) > { > CPUState *env = opaque; > @@ -221,12 +289,14 @@ static void do_cpu_reset(void *opaque) > } else { > if (env == first_cpu) { > env->regs[15] = info->loader_start; > - if (old_param) { > - set_kernel_args_old(info, info->initrd_size, > + if (!current_dtb_filename) { > + if (old_param) { > + set_kernel_args_old(info, info->initrd_size, > + info->loader_start); > + } else { > + set_kernel_args(info, info->initrd_size, > info->loader_start); > - } else { > - set_kernel_args(info, info->initrd_size, > - info->loader_start); > + } > } > } else { > info->secondary_cpu_reset_hook(env, info); > @@ -242,7 +312,7 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info) > int n; > int is_linux = 0; > uint64_t elf_entry; > - target_phys_addr_t entry; > + target_phys_addr_t entry, dtb_start; > int big_endian; > > /* Load the kernel. */ > @@ -301,8 +371,23 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info) > } else { > initrd_size = 0; > } > + info->initrd_size = initrd_size; > + > + /* Place the DTB after the initrd in memory */ > + dtb_start = TARGET_PAGE_ALIGN(info->loader_start + INITRD_LOAD_ADDR + > + initrd_size); > + if (load_dtb(dtb_start, info)) { > + exit(1); > + } > + > bootloader[4] = info->board_id; > - bootloader[5] = info->loader_start + KERNEL_ARGS_ADDR; > + /* for device tree boot, we pass the DTB directly in r2. Otherwise > + * we point to the kernel args */ > + if (current_dtb_filename) { > + bootloader[5] = dtb_start; > + } else { > + bootloader[5] = info->loader_start + KERNEL_ARGS_ADDR; > + } > bootloader[6] = entry; > for (n = 0; n < sizeof(bootloader) / 4; n++) { > bootloader[n] = tswap32(bootloader[n]); > @@ -312,7 +397,6 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info) > if (info->nb_cpus > 1) { > info->write_secondary_boot(env, info); > } > - info->initrd_size = initrd_size; This seems to be an unrelated change. Any reason for moving that line up? > } > info->is_linux = is_linux; > > diff --git a/hw/boards.h b/hw/boards.h > index f6d3784..d06776c 100644 > --- a/hw/boards.h > +++ b/hw/boards.h > @@ -34,5 +34,6 @@ typedef struct QEMUMachine { > int qemu_register_machine(QEMUMachine *m); > > extern QEMUMachine *current_machine; > +extern const char *current_dtb_filename; > I'm not sure I like that naming. It really should only contain the cmdline option. If I have a default dtb file for my machine, I'd like to keep this variable empty. Also, are you sure that boards.h is the right place for the extern declaration? In fact, is the global a good idea in the first place? We could also just change machine->init() and pass the dtb in there. In a QOM world these would become machine device properties anyways. machine->init(ram_size, boot_devices, kernel_filename, kernel_cmdline, initrd_filename, cpu_model); Essentially we shouldn't treat -dtb any different than -kernel or -initrd. It's also useful for more than ARM, namely embedded ppc systems. But I can easily post a follow-up patch for those. Alex
> We could also just change machine->init() and pass the dtb in there. In a > QOM world these would become machine device properties anyways. > > machine->init(ram_size, boot_devices, > kernel_filename, kernel_cmdline, initrd_filename, > cpu_model); > > Essentially we shouldn't treat -dtb any different than -kernel or -initrd. > It's also useful for more than ARM, namely embedded ppc systems. But I can > easily post a follow-up patch for those. Changing machine->init means you have to touch every single board file, and clone the exact same code for every machine that uses arm_boot.c. All of which will be rewritten in the near future. machine->init is a particularly suckiy interface to start with, we want to be using it less, not more. It's not like we're going support multiple machine instanced. At least not before machine->init is removed altogether. Paul
On 01.02.2012, at 02:35, Paul Brook wrote: >> We could also just change machine->init() and pass the dtb in there. In a >> QOM world these would become machine device properties anyways. >> >> machine->init(ram_size, boot_devices, >> kernel_filename, kernel_cmdline, initrd_filename, >> cpu_model); >> >> Essentially we shouldn't treat -dtb any different than -kernel or -initrd. >> It's also useful for more than ARM, namely embedded ppc systems. But I can >> easily post a follow-up patch for those. > > Changing machine->init means you have to touch every single board file, and > clone the exact same code for every machine that uses arm_boot.c. All of > which will be rewritten in the near future. Well, the dt file name would have to be passed into the generic arm_boot.c function, yes. But that's something that we need to do at one point in time either way, because machines will want to have default dtb file names. > machine->init is a particularly suckiy interface to start with, we want to be > using it less, not more. It's not like we're going support multiple machine > instanced. At least not before machine->init is removed altogether. I do see your point on not extending legacy interfaces though and not bloating up the patch. In fact, I'm indifferent enough on the actual implementation atm, as long as the command line interface (or whatever the user sees) is reasonably sane. And it is IMHO. So if it makes everything easier, do it using a global, but keep in mind that this will need refactoring. Alex
On 01/31/2012 07:35 PM, Paul Brook wrote: >> We could also just change machine->init() and pass the dtb in there. In a >> QOM world these would become machine device properties anyways. >> >> machine->init(ram_size, boot_devices, >> kernel_filename, kernel_cmdline, initrd_filename, >> cpu_model); >> >> Essentially we shouldn't treat -dtb any different than -kernel or -initrd. >> It's also useful for more than ARM, namely embedded ppc systems. But I can >> easily post a follow-up patch for those. > > Changing machine->init means you have to touch every single board file, and > clone the exact same code for every machine that uses arm_boot.c. All of > which will be rewritten in the near future. > > machine->init is a particularly suckiy interface to start with, we want to be > using it less, not more. It's not like we're going support multiple machine > instanced. At least not before machine->init is removed altogether. The right solution to this problem is to make an arm-kernel-loader device that has a dtb property. It's a very small amount of code and fits with our longer term strategy of killing off machine->init. Regards, Anthony Liguori > > Paul >
On 01/31/2012 07:44 PM, Alexander Graf wrote: > > On 01.02.2012, at 02:35, Paul Brook wrote: > >>> We could also just change machine->init() and pass the dtb in there. In a >>> QOM world these would become machine device properties anyways. >>> >>> machine->init(ram_size, boot_devices, >>> kernel_filename, kernel_cmdline, initrd_filename, >>> cpu_model); >>> >>> Essentially we shouldn't treat -dtb any different than -kernel or -initrd. >>> It's also useful for more than ARM, namely embedded ppc systems. But I can >>> easily post a follow-up patch for those. >> >> Changing machine->init means you have to touch every single board file, and >> clone the exact same code for every machine that uses arm_boot.c. All of >> which will be rewritten in the near future. > > Well, the dt file name would have to be passed into the generic arm_boot.c function, yes. But that's something that we need to do at one point in time either way, because machines will want to have default dtb file names. > >> machine->init is a particularly suckiy interface to start with, we want to be >> using it less, not more. It's not like we're going support multiple machine >> instanced. At least not before machine->init is removed altogether. > > I do see your point on not extending legacy interfaces though and not bloating up the patch. In fact, I'm indifferent enough on the actual implementation atm, as long as the command line interface (or whatever the user sees) is reasonably sane. And it is IMHO. So if it makes everything easier, do it using a global, but keep in mind that this will need refactoring. Globals are even worse! Can't you hear the kernel loader begging to be turned into a device? It's pleading with us to stop abusing other parts of QEMU and make it a first class citizen of QEMU. Regards, Anthony Liguori > > > Alex > >
On Wed, Feb 1, 2012 at 12:37 PM, Anthony Liguori <anthony@codemonkey.ws> wrote: > On 01/31/2012 07:44 PM, Alexander Graf wrote: >> >> >> On 01.02.2012, at 02:35, Paul Brook wrote: >> >>>> We could also just change machine->init() and pass the dtb in there. In >>>> a >>>> QOM world these would become machine device properties anyways. >>>> >>>> machine->init(ram_size, boot_devices, >>>> kernel_filename, kernel_cmdline, initrd_filename, >>>> cpu_model); >>>> >>>> Essentially we shouldn't treat -dtb any different than -kernel or >>>> -initrd. >>>> It's also useful for more than ARM, namely embedded ppc systems. But I >>>> can >>>> easily post a follow-up patch for those. >>> >>> >>> Changing machine->init means you have to touch every single board file, >>> and >>> clone the exact same code for every machine that uses arm_boot.c. All of >>> which will be rewritten in the near future. >> >> >> Well, the dt file name would have to be passed into the generic arm_boot.c >> function, yes. But that's something that we need to do at one point in time >> either way, because machines will want to have default dtb file names. >> >>> machine->init is a particularly suckiy interface to start with, we want >>> to be >>> using it less, not more. It's not like we're going support multiple >>> machine >>> instanced. At least not before machine->init is removed altogether. >> >> >> I do see your point on not extending legacy interfaces though and not >> bloating up the patch. In fact, I'm indifferent enough on the actual >> implementation atm, as long as the command line interface (or whatever the >> user sees) is reasonably sane. And it is IMHO. So if it makes everything >> easier, do it using a global, but keep in mind that this will need >> refactoring. > > > Globals are even worse! > > Can't you hear the kernel loader begging to be turned into a device? It's > pleading with us to stop abusing other parts of QEMU and make it a first > class citizen of QEMU. Is there some kind of initialisation phase where such a device can do its thing? Unless I'm missing something a "loader" device will be racing the rest of the VM after reset to populate the memory with the desired contents, no? John
On 01/31/2012 08:40 PM, John Williams wrote: > On Wed, Feb 1, 2012 at 12:37 PM, Anthony Liguori<anthony@codemonkey.ws> wrote: >> >> Globals are even worse! >> >> Can't you hear the kernel loader begging to be turned into a device? It's >> pleading with us to stop abusing other parts of QEMU and make it a first >> class citizen of QEMU. > > Is there some kind of initialisation phase where such a device can do its thing? > > Unless I'm missing something a "loader" device will be racing the rest > of the VM after reset to populate the memory with the desired > contents, no? How does it race? Devices normally never touch memory so a loader device will be the only thing mucking with memory. Regards, Anthony Liguori > John
On 1 February 2012 13:04, Anthony Liguori <anthony@codemonkey.ws> wrote: > How does it race? Devices normally never touch memory so a loader device > will be the only thing mucking with memory. The obvious one is "loader reset function wants to set starting PC to entry point of kernel/etc" vs "CPU device reset wants to set starting PC to hardware-mandated reset vector". We have this at the moment, of course, and I think we implicitly rely on reset handlers being called in order of registration... (The other irritating case is where the CPU device reset wants to read the starting PC out of memory, like the Cortex-M3, but really that one is because we don't distinguish "going into reset" from "coming out of reset".) -- PMM
On 02/01/2012 07:10 AM, Peter Maydell wrote: > On 1 February 2012 13:04, Anthony Liguori<anthony@codemonkey.ws> wrote: >> How does it race? Devices normally never touch memory so a loader device >> will be the only thing mucking with memory. > > The obvious one is "loader reset function wants to set starting PC to > entry point of kernel/etc" vs "CPU device reset wants to set starting > PC to hardware-mandated reset vector". We have this at the moment, of > course, and I think we implicitly rely on reset handlers being called > in order of registration... I'm a bit confused, why can't the kernel loader be implemented in terms of a firmware blob? This is what we do for x86 and it solves this problem robustly. Isn't it just a matter of a few instructions to do a jmp to a known location? Regards, Anthony Liguori > > (The other irritating case is where the CPU device reset wants > to read the starting PC out of memory, like the Cortex-M3, but > really that one is because we don't distinguish "going into reset" > from "coming out of reset".) > > -- PMM >
On 01.02.2012, at 14:25, Anthony Liguori wrote: > On 02/01/2012 07:10 AM, Peter Maydell wrote: >> On 1 February 2012 13:04, Anthony Liguori<anthony@codemonkey.ws> wrote: >>> How does it race? Devices normally never touch memory so a loader device >>> will be the only thing mucking with memory. >> >> The obvious one is "loader reset function wants to set starting PC to >> entry point of kernel/etc" vs "CPU device reset wants to set starting >> PC to hardware-mandated reset vector". We have this at the moment, of >> course, and I think we implicitly rely on reset handlers being called >> in order of registration... > > I'm a bit confused, why can't the kernel loader be implemented in terms of a firmware blob? > > This is what we do for x86 and it solves this problem robustly. Isn't it just a matter of a few instructions to do a jmp to a known location? Only if you have non-semi-hosted modes. For e500 for example, we don't have a bios flash region mapped through mmio available. So we would have to write the "jump to kernel" code into ram. But where in RAM? Linux starts at address 0, so that one's taken. Alex
On 02/01/2012 07:32 AM, Alexander Graf wrote: > > On 01.02.2012, at 14:25, Anthony Liguori wrote: > >> On 02/01/2012 07:10 AM, Peter Maydell wrote: >>> On 1 February 2012 13:04, Anthony Liguori<anthony@codemonkey.ws> wrote: >>>> How does it race? Devices normally never touch memory so a loader device >>>> will be the only thing mucking with memory. >>> >>> The obvious one is "loader reset function wants to set starting PC to >>> entry point of kernel/etc" vs "CPU device reset wants to set starting >>> PC to hardware-mandated reset vector". We have this at the moment, of >>> course, and I think we implicitly rely on reset handlers being called >>> in order of registration... >> >> I'm a bit confused, why can't the kernel loader be implemented in terms of a firmware blob? >> >> This is what we do for x86 and it solves this problem robustly. Isn't it just a matter of a few instructions to do a jmp to a known location? > > Only if you have non-semi-hosted modes. For e500 for example, we don't have a bios flash region mapped through mmio available. So we would have to write the "jump to kernel" code into ram. But where in RAM? Linux starts at address 0, so that one's taken. The processor has to have a defined sequence where IP is fixed to a specific value, no? How else would the real hardware bootstrap software? Regards, Anthony Liguori > > Alex > >
On 01.02.2012, at 14:44, Anthony Liguori wrote: > On 02/01/2012 07:32 AM, Alexander Graf wrote: >> >> On 01.02.2012, at 14:25, Anthony Liguori wrote: >> >>> On 02/01/2012 07:10 AM, Peter Maydell wrote: >>>> On 1 February 2012 13:04, Anthony Liguori<anthony@codemonkey.ws> wrote: >>>>> How does it race? Devices normally never touch memory so a loader device >>>>> will be the only thing mucking with memory. >>>> >>>> The obvious one is "loader reset function wants to set starting PC to >>>> entry point of kernel/etc" vs "CPU device reset wants to set starting >>>> PC to hardware-mandated reset vector". We have this at the moment, of >>>> course, and I think we implicitly rely on reset handlers being called >>>> in order of registration... >>> >>> I'm a bit confused, why can't the kernel loader be implemented in terms of a firmware blob? >>> >>> This is what we do for x86 and it solves this problem robustly. Isn't it just a matter of a few instructions to do a jmp to a known location? >> >> Only if you have non-semi-hosted modes. For e500 for example, we don't have a bios flash region mapped through mmio available. So we would have to write the "jump to kernel" code into ram. But where in RAM? Linux starts at address 0, so that one's taken. > > The processor has to have a defined sequence where IP is fixed to a specific value, no? > > How else would the real hardware bootstrap software? Real hardware boots u-boot which initializes lots of things and then goes into the actual booting of Linux. Today, we're doing semi-hosting though, without u-boot. We just directly boot into Linux. That's why I'm saying things don't work out all that simple with semi-hosted environments. Now you could argue that semi-hosting is a bad thing, but we'll always have to have it. On s390 for example, semi-hosting is how real hardware works. Or at least the parts that are visible to end users. Especially when you model PV machines, you'll have a hard time with fixed reset IPs too. However, couldn't we model some wiring that allows our dash-kernel-boot-device to override the reset vector on CPUs? Alex
On 02/01/2012 07:49 AM, Alexander Graf wrote: > > On 01.02.2012, at 14:44, Anthony Liguori wrote: > >> On 02/01/2012 07:32 AM, Alexander Graf wrote: >>> >>> On 01.02.2012, at 14:25, Anthony Liguori wrote: >>> >>>> On 02/01/2012 07:10 AM, Peter Maydell wrote: >>>>> On 1 February 2012 13:04, Anthony Liguori<anthony@codemonkey.ws> wrote: >>>>>> How does it race? Devices normally never touch memory so a loader device >>>>>> will be the only thing mucking with memory. >>>>> >>>>> The obvious one is "loader reset function wants to set starting PC to >>>>> entry point of kernel/etc" vs "CPU device reset wants to set starting >>>>> PC to hardware-mandated reset vector". We have this at the moment, of >>>>> course, and I think we implicitly rely on reset handlers being called >>>>> in order of registration... >>>> >>>> I'm a bit confused, why can't the kernel loader be implemented in terms of a firmware blob? >>>> >>>> This is what we do for x86 and it solves this problem robustly. Isn't it just a matter of a few instructions to do a jmp to a known location? >>> >>> Only if you have non-semi-hosted modes. For e500 for example, we don't have a bios flash region mapped through mmio available. So we would have to write the "jump to kernel" code into ram. But where in RAM? Linux starts at address 0, so that one's taken. >> >> The processor has to have a defined sequence where IP is fixed to a specific value, no? >> >> How else would the real hardware bootstrap software? > > Real hardware boots u-boot which initializes lots of things and then goes into the actual booting of Linux. Today, we're doing semi-hosting though, without u-boot. We just directly boot into Linux. Fine, but to boot u-boot, the real hardware must set IP to something that's most likely an offset into ROM flash. Why can't we bootstrap semi-hosted mode by having a ROM somewhere that just redirects IP? It doesn't have to be a full blown u-boot. > > That's why I'm saying things don't work out all that simple with semi-hosted environments. Now you could argue that semi-hosting is a bad thing, but we'll always have to have it. On s390 for example, semi-hosting is how real hardware works. Or at least the parts that are visible to end users. Especially when you model PV machines, you'll have a hard time with fixed reset IPs too. s390 is a special case because "real hardware" is not actually real hardware. It's a VM. Regards, Anthony Liguori > However, couldn't we model some wiring that allows our dash-kernel-boot-device to override the reset vector on CPUs? > > > Alex > >
On 01.02.2012, at 14:52, Anthony Liguori wrote: > On 02/01/2012 07:49 AM, Alexander Graf wrote: >> >> On 01.02.2012, at 14:44, Anthony Liguori wrote: >> >>> On 02/01/2012 07:32 AM, Alexander Graf wrote: >>>> >>>> On 01.02.2012, at 14:25, Anthony Liguori wrote: >>>> >>>>> On 02/01/2012 07:10 AM, Peter Maydell wrote: >>>>>> On 1 February 2012 13:04, Anthony Liguori<anthony@codemonkey.ws> wrote: >>>>>>> How does it race? Devices normally never touch memory so a loader device >>>>>>> will be the only thing mucking with memory. >>>>>> >>>>>> The obvious one is "loader reset function wants to set starting PC to >>>>>> entry point of kernel/etc" vs "CPU device reset wants to set starting >>>>>> PC to hardware-mandated reset vector". We have this at the moment, of >>>>>> course, and I think we implicitly rely on reset handlers being called >>>>>> in order of registration... >>>>> >>>>> I'm a bit confused, why can't the kernel loader be implemented in terms of a firmware blob? >>>>> >>>>> This is what we do for x86 and it solves this problem robustly. Isn't it just a matter of a few instructions to do a jmp to a known location? >>>> >>>> Only if you have non-semi-hosted modes. For e500 for example, we don't have a bios flash region mapped through mmio available. So we would have to write the "jump to kernel" code into ram. But where in RAM? Linux starts at address 0, so that one's taken. >>> >>> The processor has to have a defined sequence where IP is fixed to a specific value, no? >>> >>> How else would the real hardware bootstrap software? >> >> Real hardware boots u-boot which initializes lots of things and then goes into the actual booting of Linux. Today, we're doing semi-hosting though, without u-boot. We just directly boot into Linux. > > Fine, but to boot u-boot, the real hardware must set IP to something that's most likely an offset into ROM flash. > > Why can't we bootstrap semi-hosted mode by having a ROM somewhere that just redirects IP? > > It doesn't have to be a full blown u-boot. That would work, yes. > >> >> That's why I'm saying things don't work out all that simple with semi-hosted environments. Now you could argue that semi-hosting is a bad thing, but we'll always have to have it. On s390 for example, semi-hosting is how real hardware works. Or at least the parts that are visible to end users. Especially when you model PV machines, you'll have a hard time with fixed reset IPs too. > > s390 is a special case because "real hardware" is not actually real hardware. It's a VM. Sure, but how would we model things there? Our model needs to be flexible enough to cope with these oddballs. In fact, s390 is even more complicated. For DASD boot, the CPU is stalled at first and instead the DASD controller reads some instructions from memory that then bootstrap the bootloader. But IIUC that's only the case for DASD boot. For zfcp boot, you basically get semi-hosting. Alex
On 02/01/2012 07:55 AM, Alexander Graf wrote: > > On 01.02.2012, at 14:52, Anthony Liguori wrote: >> Fine, but to boot u-boot, the real hardware must set IP to something that's most likely an offset into ROM flash. >> >> Why can't we bootstrap semi-hosted mode by having a ROM somewhere that just redirects IP? >> >> It doesn't have to be a full blown u-boot. > > That would work, yes. > >> >>> >>> That's why I'm saying things don't work out all that simple with semi-hosted environments. Now you could argue that semi-hosting is a bad thing, but we'll always have to have it. On s390 for example, semi-hosting is how real hardware works. Or at least the parts that are visible to end users. Especially when you model PV machines, you'll have a hard time with fixed reset IPs too. >> >> s390 is a special case because "real hardware" is not actually real hardware. It's a VM. > > Sure, but how would we model things there? Our model needs to be flexible enough to cope with these oddballs. > > In fact, s390 is even more complicated. For DASD boot, the CPU is stalled at first and instead the DASD controller reads some instructions from memory that then bootstrap the bootloader. But IIUC that's only the case for DASD boot. For zfcp boot, you basically get semi-hosting. Once CPUs are modeled QOM, my expectation is that we'll have something like a CPU::halted property. As part of realize, a CPU would set halted = true and that is what would trigger the CPU execution (be it through TCG or KVM). There is no reason that on s390, the CPU realize function couldn't avoid setting halted=true and instead allow another device (with a wider view of the system) to perform some additional initialization work and then set the CPU halted property to true. This is all about what causes the system to start running. Once we move to a property realize() model, it gives us a lot more flexibility to work through these types of dependency issues. Regards, Anthony Liguori > > Alex > >
On Tue, Jan 31, 2012 at 6:44 PM, Alexander Graf <agraf@suse.de> wrote: > > On 01.02.2012, at 02:35, Paul Brook wrote: > >>> We could also just change machine->init() and pass the dtb in there. In a >>> QOM world these would become machine device properties anyways. >>> >>> machine->init(ram_size, boot_devices, >>> kernel_filename, kernel_cmdline, initrd_filename, >>> cpu_model); >>> >>> Essentially we shouldn't treat -dtb any different than -kernel or -initrd. >>> It's also useful for more than ARM, namely embedded ppc systems. But I can >>> easily post a follow-up patch for those. >> >> Changing machine->init means you have to touch every single board file, and >> clone the exact same code for every machine that uses arm_boot.c. All of >> which will be rewritten in the near future. > > Well, the dt file name would have to be passed into the generic arm_boot.c function, yes. But that's something that we need to do at one point in time either way, because machines will want to have default dtb file names. > >> machine->init is a particularly suckiy interface to start with, we want to be >> using it less, not more. It's not like we're going support multiple machine >> instanced. At least not before machine->init is removed altogether. > > I do see your point on not extending legacy interfaces though and not bloating up the patch. In fact, I'm indifferent enough on the actual implementation atm, as long as the command line interface (or whatever the user sees) is reasonably sane. And it is IMHO. So if it makes everything easier, do it using a global, but keep in mind that this will need refactoring. That's certainly my expectation. My initial instinct was also to handle it the say way as initrd and kernel pointers, but as Paul pointed out it requires touching all init functions which is a dead end effort when ->init() gets killed off. This patch is trivial to get the functionality into qemu without making it any more difficult for whoever creates the arm-kernel-loader device that Anthony is talking about. g.
On 01.02.2012, at 18:38, Grant Likely wrote: > On Tue, Jan 31, 2012 at 6:44 PM, Alexander Graf <agraf@suse.de> wrote: >> >> On 01.02.2012, at 02:35, Paul Brook wrote: >> >>>> We could also just change machine->init() and pass the dtb in there. In a >>>> QOM world these would become machine device properties anyways. >>>> >>>> machine->init(ram_size, boot_devices, >>>> kernel_filename, kernel_cmdline, initrd_filename, >>>> cpu_model); >>>> >>>> Essentially we shouldn't treat -dtb any different than -kernel or -initrd. >>>> It's also useful for more than ARM, namely embedded ppc systems. But I can >>>> easily post a follow-up patch for those. >>> >>> Changing machine->init means you have to touch every single board file, and >>> clone the exact same code for every machine that uses arm_boot.c. All of >>> which will be rewritten in the near future. >> >> Well, the dt file name would have to be passed into the generic arm_boot.c function, yes. But that's something that we need to do at one point in time either way, because machines will want to have default dtb file names. >> >>> machine->init is a particularly suckiy interface to start with, we want to be >>> using it less, not more. It's not like we're going support multiple machine >>> instanced. At least not before machine->init is removed altogether. >> >> I do see your point on not extending legacy interfaces though and not bloating up the patch. In fact, I'm indifferent enough on the actual implementation atm, as long as the command line interface (or whatever the user sees) is reasonably sane. And it is IMHO. So if it makes everything easier, do it using a global, but keep in mind that this will need refactoring. > > That's certainly my expectation. My initial instinct was also to > handle it the say way as initrd and kernel pointers, but as Paul > pointed out it requires touching all init functions which is a dead > end effort when ->init() gets killed off. This patch is trivial to > get the functionality into qemu without making it any more difficult > for whoever creates the arm-kernel-loader device that Anthony is > talking about. Yeah, I agree. Let's separate the QOM efforts from making things work for now. I don't want to have yet another if=ahci or hotplug magic where I'm waiting for a year for salvation that never came. Let's get the feature in and model the whole thing properly with all cases taken into account. This way we at least don't forget about dtbs when modeling the arm-kernel-loader device :) Alex
On 1 February 2012 01:10, Alexander Graf <agraf@suse.de> wrote: > On 01.02.2012, at 01:11, Grant Likely wrote: >> @@ -312,7 +397,6 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info) >> if (info->nb_cpus > 1) { >> info->write_secondary_boot(env, info); >> } >> - info->initrd_size = initrd_size; > > This seems to be an unrelated change. Any reason for moving that line up? I've just discovered the reason for this, which is that load_dtb() reads info->initrd_size, so without this change it all works fine except that we silently don't pass the initrd to the kernel :-) -- PMM
diff --git a/Makefile.target b/Makefile.target index 68481a3..5e465ec 100644 --- a/Makefile.target +++ b/Makefile.target @@ -363,6 +363,7 @@ obj-arm-y += vexpress.o obj-arm-y += strongarm.o obj-arm-y += collie.o obj-arm-y += pl041.o lm4549.o +obj-arm-$(CONFIG_FDT) += device_tree.o obj-sh4-y = shix.o r2d.o sh7750.o sh7750_regnames.o tc58128.o obj-sh4-y += sh_timer.o sh_serial.o sh_intc.o sh_pci.o sm501.o diff --git a/configure b/configure index f69e08f..2856897 100755 --- a/configure +++ b/configure @@ -3411,6 +3411,7 @@ case "$target_arch2" in gdb_xml_files="arm-core.xml arm-vfp.xml arm-vfp3.xml arm-neon.xml" target_phys_bits=32 target_llong_alignment=4 + target_libs_softmmu="$fdt_libs" ;; cris) target_nptl="yes" diff --git a/hw/arm_boot.c b/hw/arm_boot.c index 5f163fd..377d202 100644 --- a/hw/arm_boot.c +++ b/hw/arm_boot.c @@ -7,11 +7,14 @@ * This code is licensed under the GPL. */ +#include "config.h" #include "hw.h" #include "arm-misc.h" #include "sysemu.h" +#include "boards.h" #include "loader.h" #include "elf.h" +#include "device_tree.h" #define KERNEL_ARGS_ADDR 0x100 #define KERNEL_LOAD_ADDR 0x00010000 @@ -207,6 +210,71 @@ static void set_kernel_args_old(const struct arm_boot_info *info, } } +static int load_dtb(target_phys_addr_t addr, const struct arm_boot_info *binfo) +{ +#ifdef CONFIG_FDT + uint32_t mem_reg_property[] = { cpu_to_be32(binfo->loader_start), + cpu_to_be32(binfo->ram_size) }; + void *fdt = NULL; + char *filename; + int size, rc; + + if (!current_dtb_filename) { + return 0; + } + + filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, current_dtb_filename); + if (!filename) { + fprintf(stderr, "Couldn't open dtb file %s\n", current_dtb_filename); + return -1; + } + + fdt = load_device_tree(filename, &size); + if (!fdt) { + fprintf(stderr, "Couldn't open dtb file %s\n", filename); + g_free(filename); + return -1; + } + g_free(filename); + + rc = qemu_devtree_setprop(fdt, "/memory", "reg", mem_reg_property, + sizeof(mem_reg_property)); + if (rc < 0) { + fprintf(stderr, "couldn't set /memory/reg\n"); + } + + rc = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs", + binfo->kernel_cmdline); + if (rc < 0) { + fprintf(stderr, "couldn't set /chosen/bootargs\n"); + } + + if (binfo->initrd_size) { + rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-start", + binfo->loader_start + INITRD_LOAD_ADDR); + if (rc < 0) { + fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n"); + } + + rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-end", + binfo->loader_start + INITRD_LOAD_ADDR + + binfo->initrd_size); + if (rc < 0) { + fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n"); + } + } + + cpu_physical_memory_write(addr, fdt, size); + + return 0; + +#else + fprintf(stderr, "Platform requested a device tree, " + "but qemu was compiled without fdt support\n"); + return -1; +#endif +} + static void do_cpu_reset(void *opaque) { CPUState *env = opaque; @@ -221,12 +289,14 @@ static void do_cpu_reset(void *opaque) } else { if (env == first_cpu) { env->regs[15] = info->loader_start; - if (old_param) { - set_kernel_args_old(info, info->initrd_size, + if (!current_dtb_filename) { + if (old_param) { + set_kernel_args_old(info, info->initrd_size, + info->loader_start); + } else { + set_kernel_args(info, info->initrd_size, info->loader_start); - } else { - set_kernel_args(info, info->initrd_size, - info->loader_start); + } } } else { info->secondary_cpu_reset_hook(env, info); @@ -242,7 +312,7 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info) int n; int is_linux = 0; uint64_t elf_entry; - target_phys_addr_t entry; + target_phys_addr_t entry, dtb_start; int big_endian; /* Load the kernel. */ @@ -301,8 +371,23 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info) } else { initrd_size = 0; } + info->initrd_size = initrd_size; + + /* Place the DTB after the initrd in memory */ + dtb_start = TARGET_PAGE_ALIGN(info->loader_start + INITRD_LOAD_ADDR + + initrd_size); + if (load_dtb(dtb_start, info)) { + exit(1); + } + bootloader[4] = info->board_id; - bootloader[5] = info->loader_start + KERNEL_ARGS_ADDR; + /* for device tree boot, we pass the DTB directly in r2. Otherwise + * we point to the kernel args */ + if (current_dtb_filename) { + bootloader[5] = dtb_start; + } else { + bootloader[5] = info->loader_start + KERNEL_ARGS_ADDR; + } bootloader[6] = entry; for (n = 0; n < sizeof(bootloader) / 4; n++) { bootloader[n] = tswap32(bootloader[n]); @@ -312,7 +397,6 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info) if (info->nb_cpus > 1) { info->write_secondary_boot(env, info); } - info->initrd_size = initrd_size; } info->is_linux = is_linux; diff --git a/hw/boards.h b/hw/boards.h index f6d3784..d06776c 100644 --- a/hw/boards.h +++ b/hw/boards.h @@ -34,5 +34,6 @@ typedef struct QEMUMachine { int qemu_register_machine(QEMUMachine *m); extern QEMUMachine *current_machine; +extern const char *current_dtb_filename; #endif diff --git a/qemu-options.hx b/qemu-options.hx index 3a07ae8..309a403 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -1964,6 +1964,15 @@ Use @var{file1} and @var{file2} as modules and pass arg=foo as parameter to the first module. ETEXI +DEF("dtb", HAS_ARG, QEMU_OPTION_dtb, \ + "-dtb file use 'file' as device tree image\n", QEMU_ARCH_ARM) +STEXI +@item -dtb @var{file} +@findex -dtb +Use @var{file} as a device tree binary (dtb) image and pass it to the kernel +on boot. +ETEXI + STEXI @end table ETEXI diff --git a/vl.c b/vl.c index d88a18c..b22eae3 100644 --- a/vl.c +++ b/vl.c @@ -1154,6 +1154,7 @@ void pcmcia_info(Monitor *mon) static QEMUMachine *first_machine = NULL; QEMUMachine *current_machine = NULL; +const char *current_dtb_filename = NULL; int qemu_register_machine(QEMUMachine *m) { @@ -2304,6 +2305,9 @@ int main(int argc, char **argv, char **envp) case QEMU_OPTION_initrd: initrd_filename = optarg; break; + case QEMU_OPTION_dtb: + current_dtb_filename = optarg; + break; case QEMU_OPTION_hda: { char buf[256]; @@ -3241,6 +3245,11 @@ int main(int argc, char **argv, char **envp) exit(1); } + if (!linux_boot && current_dtb_filename != NULL) { + fprintf(stderr, "-dtb only allowed with -kernel option\n"); + exit(1); + } + os_set_line_buffering(); if (init_timer_alarm() < 0) {
If compiled with CONFIG_FDT, allow user to specify a device tree file using the -dtb argument. If the machine supports it then the dtb will be loaded into memory and passed to the kernel on boot. v2: - Enable for all arm platforms by making arm_boot use the filename directly - Fix style issues Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com> Signed-off-by: Grant Likely <grant.likely@secretlab.ca> Cc: Paul Brook <paul@codesourcery.com> Cc: Peter Maydell <peter.maydell@linaro.org> Cc: Rob Herring <rob.herring@calxeda.com> Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com> --- Makefile.target | 1 + configure | 1 + hw/arm_boot.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++---- hw/boards.h | 1 + qemu-options.hx | 9 +++++ vl.c | 9 +++++ 6 files changed, 113 insertions(+), 8 deletions(-)