Re: [Qemu-devel] [PATCH] functional ARM semihosting under GDB

[Peter Maydell]

On 13 November 2014 23:55, Liviu Ionescu <address@hidden> wrote:
> The shortcomings addressed by this patch:
> - the semihosting trace messages disapeared when the GDB session was started
> - the semihosting exit code was not passed back to the host
> - the semihosting command line was not passed properly, because it generated a
> very large string, including the image full path
> - the stelaris/armv7m code did not pass the semihosting command line at all
> - the GDB use case, although allowed the image to be loaded via GDB, it still
> required the presence of the -kernel option on the command line
> - the -kernel option was not appropriate for usual applications

Thanks for sending this patch. There's definitely some good things
in this patch, but from my perspective the main issue with it is
that it's combining six different features and bug fixes into a
single commit. Could you separate them out into their own patches?
You can start out by separating out one or two and submitting
those. I've given my general opinions on each feature below, which
hopefully will suggest which ones to start with:

> - the semihosting trace messages disapeared when the GDB session was started

This (the extra command line option to specify where semihosting
should go) is definitely a feature we should add. I think it's
possible to make use of the QemuOpts infrastructure to support
 -semihosting  # current option name with existing semantics
 -semihosting target=gdb
 -semihosting target=native
 -semihosting target=auto   # same as plain "-semihosting"

something like (untested):
static QemuOptsList qemu_semihosting_opts = {
    .name = "semihosting",
    .implied_opt_name = "enable",
    .head = QTAILQ_HEAD_INITIALIZER(qemu_semihosting_opts.head),
    .desc = {
        {
            .name = "enable",
            .type = QEMU_OPT_BOOL,
        },{
            .name = "target",

        { /* end of list */ }
    },
};


> - the semihosting exit code was not passed back to the host

This is the change to return 1 if the reason code isn't ApplicationExit,
right? This seems a reasonable change.

> - the semihosting command line was not passed properly, because it generated a
> very large string, including the image full path
> - the stelaris/armv7m code did not pass the semihosting command line at all

These both sound like bugs that we should fix.

> - the GDB use case, although allowed the image to be loaded via GDB, it still
> required the presence of the -kernel option on the command line

The way we've approached this for other board models is simply to
remove the requirement for a -kernel option, so if you start the
model up without providing -kernel then we behave as the hardware would
(ie sit there doing nothing).

> A more generic option was added to specify the application file to be emulated
>
>     -image file-path

I'm pretty wary about this one, because we already have several image
loading options (-kernel, -bios) with complicated semantics that may
not be the same on different target architectures. What does your
"-image" option do that's different from the existing "-kernel" ?

thanks
-- PMM