Re: [Qemu-devel] [PATCH] Add -semihosting-config ....cmdline=string.
On 19 November 2014 22:05, Liviu Ionescu i...@livius.net wrote: A new sub-option was added to -semihosting-config to define the entire semihosting command line (cmdline=string). This string is passed down to armv7m.c; if not defined, for compatibility reasons, the -kernel -append values are used. The armv7m_init() and stellaris_init() interfaces were streamlined, to use the MachineState structure instead of separate strings. The semihosting_cmdline was added to the structures MachineState and arm_boot_info. I think you can avoid having to plumb the command line string into the MachineState and arm_boot_info structures, because you can just have the semihosting code look the option up by name: QemuOpts *opts = qemu_opts_find(qemu_find_opts(semihosting-config), NULL); cmdline = qemu_opt_get(opts, cmdline); if (cmdline) { ... } else { fall back to constructing from kernel/append args; } That will also automatically make the command line option work for A profile CPUs as well. thanks -- PMM
Re: [Qemu-devel] [PATCH] Add -semihosting-config ....cmdline=string.
On 20 Nov 2014, at 15:31, Peter Maydell peter.mayd...@linaro.org wrote: I think you can avoid having to plumb the command line string into the MachineState and arm_boot_info structures, because you can just have the semihosting code look the option up by name: QemuOpts *opts = qemu_opts_find(qemu_find_opts(semihosting-config), NULL); something is missing with the way options are handled, this call fails with BAD_ACCESS. I tried to debug the problem, but with little success so far. That will also automatically make the command line option work for A profile CPUs as well. where is the A profile code? regards, Liviu
Re: [Qemu-devel] [PATCH] Add -semihosting-config ....cmdline=string.
On 20 November 2014 15:03, Liviu Ionescu i...@livius.net wrote: On 20 Nov 2014, at 15:31, Peter Maydell peter.mayd...@linaro.org wrote: I think you can avoid having to plumb the command line string into the MachineState and arm_boot_info structures, because you can just have the semihosting code look the option up by name: QemuOpts *opts = qemu_opts_find(qemu_find_opts(semihosting-config), NULL); something is missing with the way options are handled, this call fails with BAD_ACCESS. What's this? It's not an error code in QEMU... I tried to debug the problem, but with little success so far. I'll have a look. That will also automatically make the command line option work for A profile CPUs as well. where is the A profile code? A profile CPUs generally boot via the code in hw/arm/boot.c. -- PMM
Re: [Qemu-devel] [PATCH] Add -semihosting-config ....cmdline=string.
On 20 Nov 2014, at 17:10, Peter Maydell peter.mayd...@linaro.org wrote: something is missing with the way options are handled, this call fails with BAD_ACCESS. What's this? It's not an error code in QEMU... no, it is a system condition, EXC_BAD_ACCESS, generally caused by a bad pointer. (on linux probably you get a signal). I also tried with .merge_lists = true, but no difference. Liviu
Re: [Qemu-devel] [PATCH] Add -semihosting-config ....cmdline=string.
On 20 November 2014 15:17, Liviu Ionescu i...@livius.net wrote: On 20 Nov 2014, at 17:10, Peter Maydell peter.mayd...@linaro.org wrote: something is missing with the way options are handled, this call fails with BAD_ACCESS. What's this? It's not an error code in QEMU... no, it is a system condition, EXC_BAD_ACCESS, generally caused by a bad pointer. (on linux probably you get a signal). Ah, right, SIGSEGV. Anyway, I had a quick play and it seems to work correctly for me: with this diff: diff --git a/target-arm/arm-semi.c b/target-arm/arm-semi.c index a8b83e6..d822641 100644 --- a/target-arm/arm-semi.c +++ b/target-arm/arm-semi.c @@ -36,6 +36,8 @@ #include exec/gdbstub.h #include hw/arm/arm.h #endif +#include qemu/option.h +#include qemu/config-file.h #define TARGET_SYS_OPEN0x01 #define TARGET_SYS_CLOSE 0x02 @@ -199,6 +201,11 @@ uint32_t do_arm_semihosting(CPUARMState *env) #else CPUARMState *ts = env; #endif +QemuOpts *opts; +const char *cmdline; +opts = qemu_opts_find(qemu_find_opts(semihosting-config), NULL); +cmdline = qemu_opt_get(opts, cmdline); +printf(got cmdline %s\n, cmdline ? cmdline : (null)); nr = env-regs[0]; args = env-regs[1]; diff --git a/vl.c b/vl.c index 9b69dc3..97eac57 100644 --- a/vl.c +++ b/vl.c @@ -565,6 +565,9 @@ static QemuOptsList qemu_semihosting_config_opts = { }, { .name = target, .type = QEMU_OPT_STRING, +}, { +.name = cmdline, +.type = QEMU_OPT_STRING, }, { /* end of list */ } }, it prints the command line string at every semihosting call, if there is one. (I just stuck it at the top of the function for testing since the test binary I happened to have to hand didn't do the 'get commandline' call.) NB: you'll want to stick this inside the !CONFIG_USER_ONLY ifdef, because on linux-user it'll compile but the qemu_find_opts() call will fail and then we'll segfault because we pass a NULL to qemu_opts_find(). -- PMM
Re: [Qemu-devel] [PATCH] Add -semihosting-config ....cmdline=string.
On 20 Nov 2014, at 17:39, Peter Maydell peter.mayd...@linaro.org wrote: NB: you'll want to stick this inside the !CONFIG_USER_ONLY did this. now it works for me, but please check if it compiles properly for other targets. regards, Liviu
[Qemu-devel] [PATCH] Add -semihosting-config ....cmdline=string.
A new sub-option was added to -semihosting-config to define the entire semihosting command line (cmdline=string). This string is passed down to armv7m.c; if not defined, for compatibility reasons, the -kernel -append values are used. The armv7m_init() and stellaris_init() interfaces were streamlined, to use the MachineState structure instead of separate strings. The semihosting_cmdline was added to the structures MachineState and arm_boot_info. Signed-off-by: Liviu Ionescu i...@livius.net --- hw/arm/armv7m.c | 15 ++- hw/arm/stellaris.c| 12 include/hw/arm/arm.h | 3 ++- include/hw/boards.h | 1 + qemu-options.hx | 10 +++--- target-arm/arm-semi.c | 19 +++ vl.c | 9 + 7 files changed, 52 insertions(+), 17 deletions(-) mode change 100644 = 100755 qemu-options.hx diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c index ef24ca4..696de12 100644 --- a/hw/arm/armv7m.c +++ b/hw/arm/armv7m.c @@ -13,6 +13,9 @@ #include elf.h #include sysemu/qtest.h #include qemu/error-report.h +#include hw/boards.h + +static struct arm_boot_info armv7m_binfo; /* Bitbanded IO. Each word corresponds to a single bit. */ @@ -168,7 +171,7 @@ static void armv7m_reset(void *opaque) qemu_irq *armv7m_init(MemoryRegion *system_memory, int flash_size, int sram_size, - const char *kernel_filename, const char *cpu_model) + MachineState *machine) { ARMCPU *cpu; CPUARMState *env; @@ -184,6 +187,9 @@ qemu_irq *armv7m_init(MemoryRegion *system_memory, MemoryRegion *flash = g_new(MemoryRegion, 1); MemoryRegion *hack = g_new(MemoryRegion, 1); +const char *kernel_filename = machine-kernel_filename; +const char *cpu_model = machine-cpu_model; + flash_size *= 1024; sram_size *= 1024; @@ -240,6 +246,13 @@ qemu_irq *armv7m_init(MemoryRegion *system_memory, exit(1); } +/* Fill-in a minimalistic boot info, required for semihosting */ +armv7m_binfo.kernel_filename = kernel_filename; +armv7m_binfo.kernel_cmdline = machine-kernel_cmdline; +armv7m_binfo.semihosting_cmdline = machine-semihosting_cmdline; + +env-boot_info = armv7m_binfo; + if (kernel_filename) { image_size = load_elf(kernel_filename, NULL, NULL, entry, lowaddr, NULL, big_endian, ELF_MACHINE, 1); diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c index 64bd4b4..f93326a 100644 --- a/hw/arm/stellaris.c +++ b/hw/arm/stellaris.c @@ -1198,7 +1198,7 @@ static stellaris_board_info stellaris_boards[] = { } }; -static void stellaris_init(const char *kernel_filename, const char *cpu_model, +static void stellaris_init(MachineState *machine, stellaris_board_info *board) { static const int uart_irq[] = {5, 6, 33, 34}; @@ -1223,7 +1223,7 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model, flash_size = ((board-dc0 0x) + 1) 1; sram_size = (board-dc0 18) + 1; pic = armv7m_init(get_system_memory(), - flash_size, sram_size, kernel_filename, cpu_model); + flash_size, sram_size, machine); if (board-dc1 (1 16)) { dev = sysbus_create_varargs(TYPE_STELLARIS_ADC, 0x40038000, @@ -1335,16 +1335,12 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model, /* FIXME: Figure out how to generate these from stellaris_boards. */ static void lm3s811evb_init(MachineState *machine) { -const char *cpu_model = machine-cpu_model; -const char *kernel_filename = machine-kernel_filename; -stellaris_init(kernel_filename, cpu_model, stellaris_boards[0]); +stellaris_init(machine, stellaris_boards[0]); } static void lm3s6965evb_init(MachineState *machine) { -const char *cpu_model = machine-cpu_model; -const char *kernel_filename = machine-kernel_filename; -stellaris_init(kernel_filename, cpu_model, stellaris_boards[1]); +stellaris_init(machine, stellaris_boards[1]); } static QEMUMachine lm3s811evb_machine = { diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h index cefc9e6..bae853c 100644 --- a/include/hw/arm/arm.h +++ b/include/hw/arm/arm.h @@ -17,7 +17,7 @@ /* armv7m.c */ qemu_irq *armv7m_init(MemoryRegion *system_memory, int flash_size, int sram_size, - const char *kernel_filename, const char *cpu_model); + MachineState *machine); /* arm_boot.c */ struct arm_boot_info { @@ -26,6 +26,7 @@ struct arm_boot_info { const char *kernel_cmdline; const char *initrd_filename; const char *dtb_filename; +const char *semihosting_cmdline; hwaddr loader_start; /* multicore boards that use the default secondary core boot functions * need to put the address of the secondary boot code, the boot reg, diff --git a/include/hw/boards.h