Message ID | b9f395cf16e881c230c596855168c8ca5b899c93.1338512893.git.peter.crosthwaite@petalogix.com |
---|---|
State | New |
Headers | show |
On 1 June 2012 02:16, Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com> wrote: > @@ -240,10 +241,13 @@ static int load_dtb(target_phys_addr_t addr, const struct arm_boot_info *binfo) > 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"); > + machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0); > + if (machine_opts && qemu_opt_get(machine_opts, "append")) { > + rc = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs", > + binfo->kernel_cmdline); > + if (rc < 0) { > + fprintf(stderr, "couldn't set /chosen/bootargs\n"); > + } > } Can you just check for binfo->kernel_cmdline being NULL or not rather than rereading the option via qemu_opt_get? The latter seems pretty ugly. -- PMM
On Fri, Jun 1, 2012 at 11:44 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 1 June 2012 02:16, Peter A. G. Crosthwaite > <peter.crosthwaite@petalogix.com> wrote: >> @@ -240,10 +241,13 @@ static int load_dtb(target_phys_addr_t addr, const struct arm_boot_info *binfo) >> 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"); >> + machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0); >> + if (machine_opts && qemu_opt_get(machine_opts, "append")) { >> + rc = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs", >> + binfo->kernel_cmdline); >> + if (rc < 0) { >> + fprintf(stderr, "couldn't set /chosen/bootargs\n"); >> + } >> } > > Can you just check for binfo->kernel_cmdline being NULL or not > rather than rereading the option via qemu_opt_get? The latter > seems pretty ugly. > No, it wont work, vl.c will populate it with "\"\"": if (machine_opts) { ... kernel_cmdline = qemu_opt_get(machine_opts, "append"); } else { kernel_filename = initrd_filename = kernel_cmdline = NULL; } if (!kernel_cmdline) { kernel_cmdline = ""; } binfo->kernel_cmdline will not be null on the omission of -append. I did it this way as its the only way I can see where you can determine whether or not -apend happened. You could strcmp with "\"\"" and use that as your condition, but then you have a possibly piece of policy that an empty command means no update (i.e. you should still be able to explictly -apend "\"\"" to wipe out the dtb command line). Regards, Peter > -- PMM
On Fri, Jun 1, 2012 at 11:56 AM, Peter Crosthwaite <peter.crosthwaite@petalogix.com> wrote: > On Fri, Jun 1, 2012 at 11:44 AM, Peter Maydell <peter.maydell@linaro.org> wrote: >> On 1 June 2012 02:16, Peter A. G. Crosthwaite >> <peter.crosthwaite@petalogix.com> wrote: >>> @@ -240,10 +241,13 @@ static int load_dtb(target_phys_addr_t addr, const struct arm_boot_info *binfo) >>> 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"); >>> + machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0); >>> + if (machine_opts && qemu_opt_get(machine_opts, "append")) { >>> + rc = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs", >>> + binfo->kernel_cmdline); >>> + if (rc < 0) { >>> + fprintf(stderr, "couldn't set /chosen/bootargs\n"); >>> + } >>> } >> >> Can you just check for binfo->kernel_cmdline being NULL or not >> rather than rereading the option via qemu_opt_get? The latter >> seems pretty ugly. >> > > No, it wont work, > > vl.c will populate it with "\"\"": > Correction, vl.c will populate it with an empty string. The problem still stands though that an empty string explicitly passed to -append should mean wipe out dtb command line. > if (machine_opts) { > ... > kernel_cmdline = qemu_opt_get(machine_opts, "append"); > } else { > kernel_filename = initrd_filename = kernel_cmdline = NULL; > } > > if (!kernel_cmdline) { > kernel_cmdline = ""; > } > > > binfo->kernel_cmdline will not be null on the omission of -append. > > I did it this way as its the only way I can see where you can > determine whether or not -apend happened. You could strcmp with "\"\"" > and use that as your condition, but then you have a possibly piece of > policy that an empty command means no update (i.e. you should still be > able to explictly -apend "\"\"" to wipe out the dtb command line). > > Regards, > > Peter > >> -- PMM
diff --git a/hw/arm_boot.c b/hw/arm_boot.c index 8e25873b..d040c58 100644 --- a/hw/arm_boot.c +++ b/hw/arm_boot.c @@ -219,6 +219,7 @@ static int load_dtb(target_phys_addr_t addr, const struct arm_boot_info *binfo) void *fdt = NULL; char *filename; int size, rc; + QemuOpts *machine_opts; filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, binfo->dtb_filename); if (!filename) { @@ -240,10 +241,13 @@ static int load_dtb(target_phys_addr_t addr, const struct arm_boot_info *binfo) 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"); + machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0); + if (machine_opts && qemu_opt_get(machine_opts, "append")) { + 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) {
The dtb command line should only be overwritten if the user provides a command line. Otherwise whatever command line was in the dtb should stay unchanged. Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com> --- hw/arm_boot.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-)