Re: [Qemu-devel] [PATCH] Add -semihosting-config ....cmdline=string.

2014-11-20 Thread Peter Maydell
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.

2014-11-20 Thread Liviu Ionescu

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.

2014-11-20 Thread Peter Maydell
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.

2014-11-20 Thread Liviu Ionescu

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.

2014-11-20 Thread Peter Maydell
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.

2014-11-20 Thread Liviu Ionescu

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.

2014-11-19 Thread Liviu Ionescu
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