Re: [Qemu-devel] [PATCH v3 2/2] semihosting: add --semihosting-config arg sub-argument

[Leon Alrae <leon.alrae@imgtec.com>]

On 18/05/2015 17:18, Peter Maydell wrote:
> On 8 May 2015 at 12:41, Leon Alrae <leon.alrae@imgtec.com> wrote:
>> Add new "arg" sub-argument to the --semihosting-config allowing to pass
>> multiple input argument separately. It is required for example by UHI
>> semihosting to construct argc and argv.
>>
>> Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
>> ---
>>  include/exec/semihost.h | 12 ++++++++++++
>>  qemu-options.hx         | 19 ++++++++++++++-----
>>  vl.c                    | 33 +++++++++++++++++++++++++++++++++
>>  3 files changed, 59 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/exec/semihost.h b/include/exec/semihost.h
>> index c2f0bcb..6e4e8c0 100644
>> --- a/include/exec/semihost.h
>> +++ b/include/exec/semihost.h
>> @@ -36,9 +36,21 @@ static inline SemihostingTarget semihosting_get_target(void)
>>  {
>>      return SEMIHOSTING_TARGET_AUTO;
>>  }
>> +
>> +static inline const char *semihosting_get_arg(int i)
>> +{
>> +    return NULL;
>> +}
>> +
>> +static inline int semihosting_get_argc(void)
>> +{
>> +    return 0;
>> +}
>>  #else
>>  bool semihosting_enabled(void);
>>  SemihostingTarget semihosting_get_target(void);
>> +const char *semihosting_get_arg(int i);
>> +int semihosting_get_argc(void);
>>  #endif
>>
>>  #endif
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index ec356f6..84ae6c2 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -3296,14 +3296,23 @@ STEXI
>>  Enable semihosting mode (ARM, M68K, Xtensa only).
>>  ETEXI
>>  DEF("semihosting-config", HAS_ARG, QEMU_OPTION_semihosting_config,
>> -    "-semihosting-config [enable=on|off,]target=native|gdb|auto   semihosting configuration\n",
>> +    "-semihosting-config [enable=on|off][,target=native|gdb|auto][,arg=str[,...]]\n" \
>> +    "                semihosting configuration\n",
>>  QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA | QEMU_ARCH_LM32)
>>  STEXI
>> -@item -semihosting-config [enable=on|off,]target=native|gdb|auto
>> +@item -semihosting-config [enable=on|off][,target=native|gdb|auto][,arg=str[,...]]
>>  @findex -semihosting-config
>> -Enable semihosting and define where the semihosting calls will be addressed,
>> -to QEMU (@code{native}) or to GDB (@code{gdb}). The default is @code{auto}, which means
>> -@code{gdb} during debug sessions and @code{native} otherwise (ARM, M68K, Xtensa only).
>> +Enable and configure semihosting (ARM, M68K, Xtensa only).
>> +@table @option
>> +@item target=@code{native|gdb|auto}
>> +Defines where the semihosting calls will be addressed, to QEMU (@code{native})
>> +or to GDB (@code{gdb}). The default is @code{auto}, which means @code{gdb}
>> +during debug sessions and @code{native} otherwise.
>> +@item arg=@var{str1},arg=@var{str2},...
>> +Allows the user to pass input arguments, can be used multiple times to build up
>> +a list. This is a replacement for the old-style -kernel/-append method of
>> +passing a command line to semihosting.
>> +@end table
> 
> You need to say how this interacts with the -kernel/-append option
> (ie what happens if you specify both).

I don't see any correlation between semihosting options and "-append"
which is described as "kernel command line". I know that ARM semihosting
uses it as cmdline, therefore I'd wanted to leave the meaning of
"-append" option specific to a target semihosting implementation. But
perhaps specifying it globally would be better here, I can add something
like "If both are specified, -kernel/-append are ignored (-kernel is
used to load an image, but the path won't be passed to semihosting)" if
we are happy with that.

> Also, you haven't actually changed anything so at the moment -arg doesn't do
> anything.

Yes, but is that an issue in this case? Commit message explains why -arg
is required and UHI patches are already on the mailing list.

> The only semihosting target in-tree which cares about arguments
> at the moment is ARM.

I left it untouched as I'm aware that Liviu has some more ARM
semihosting stuff pending.

> Ideally semihost.h's "get me the command line" function(s) should
> handle "use -arg if present, fall back to -kernel + -append if not"
> so the target specific semihosting code doesn't need to care.

Agreed. We could initialize semihosting.argv[0] with -kernel and argv[1]
with -append string if in semihosting mode and no semihosting args have
been specified. I'll update it in v4.

Thanks,
Leon