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

2014-11-20 Thread Liviu Ionescu
Hi,

with the latest submitted patches, the functionality I expect for 
qemu-system-arm is complete. (I have some more cosmetic suggestions, to be 
discussed later).

to test the functionality, you can download an unit test application from:

https://dl.dropboxusercontent.com/u/78151643/gcm.elf


there are two use cases:

1) standalone (usually unit tests integrated in a continuous integration system)

$ ./qemu-system-arm -machine lm3s6965evb -kernel gcm.elf -semihosting-config 
target=native,cmdline=gcm --gtest_output=xml:gcm.xml

(notice the gcm.xml file created in the current folder)


2) via GDB

$ ./qemu-system-arm -machine lm3s6965evb -semihosting-config 
target=native,cmdline=gcm --gtest_output=xml:gcm.xml -gdb tcp::1234 -S

(notice the image is no longer passed here)

and from another terminal

$ arm-none-eabi-gdb gcm.elf
(gdb) target remote localhost:1234
(gdb) load
(gdb) system_reset
(gdb) break main
(gdb) continue
Breakpoint 1, main (argc=2, argv=0x21b0 argv_buf.5809) at 
../gmock/src/gmock_main.cc:49
49  {
(gdb) continue
Continuing.
[Inferior 1 (Remote target) exited normally]

(notice the system_reset required after loading the image to initialise the PC 
 SP)


regards,

Liviu


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

2014-11-19 Thread Liviu Ionescu

On 18 Nov 2014, at 21:46, Liviu Ionescu i...@livius.net wrote:

 once you integrate them...

thank you for integrating them.

so can I base my next patches on them?

 I'll work on another patch, to fix passing the semihosting command line down 
 to the armv7m code, since currently this triggers exceptions due to null 
 pointers.
 
 my suggestion would be to extend the -semihosting-config option with 
 cmdline=... instead of adding a separate option (in my first patch I used 
 -semihosting-cmdline).

is this ok?


Liviu




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

2014-11-19 Thread Peter Maydell
On 19 November 2014 11:37, Liviu Ionescu i...@livius.net wrote:

 On 18 Nov 2014, at 21:46, Liviu Ionescu i...@livius.net wrote:

 once you integrate them...

 thank you for integrating them.

 so can I base my next patches on them?

You can, yes. (Bear in mind that my target-arm.next
branch is a rebasing branch, so you can't git merge
or pull it; you need to fetch it and then rebase
your patchstack on it.)

 I'll work on another patch, to fix passing the semihosting
 command line down to the armv7m code, since currently this
 triggers exceptions due to null pointers.

Yes, that should be fixed.

 my suggestion would be to extend the -semihosting-config
 option with cmdline=... instead of adding a separate option
 (in my first patch I used -semihosting-cmdline).

On the A and R profile code path this command line
is taken from the existing -append option. Is
there a reason we can't just make the M profile
code do the same thing?

thanks
-- PMM



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

2014-11-19 Thread Liviu Ionescu

On 19 Nov 2014, at 14:08, Peter Maydell peter.mayd...@linaro.org wrote:

 ... (Bear in mind that my target-arm.next
 branch is a rebasing branch, so you can't git merge
 or pull it; you need to fetch it and then rebase
 your patchstack on it.)

I currently have two more branches in my git, for each of the patches applied.

I planned to make another branch, based on the -semihosting-config branch, and 
add the cmdline patches there.
 
 my suggestion would be to extend the -semihosting-config
 option with cmdline=... instead of adding a separate option
 (in my first patch I used -semihosting-cmdline).
 
 On the A and R profile code path this command line
 is taken from the existing -append option. Is
 there a reason we can't just make the M profile
 code do the same thing?

I already explained, but will do it again.

first, we are talking about two completely different use cases.

on the A profile you have a kernel, and when you start it, you 'append' to the 
kernel path some options, and this is what you pass to the bootloader. this has 
nothing to do with semihosting, the command line is always used by the 
bootloader to start the kernel.

on the M profile we do not have a kernel, but an image (that you do not want to 
accept as a different thing), and for normal emulation (i.e. without 
semihosting) we do not have any command line options.

now comes semihosting.

I do not know how you used semihosting for other platforms, but for arm, the 
current code breaks with faults, so I have my doubts it was really used (if it 
was, ok). 

anyway, the current code uses the same strategy as for the bootloader, i.e. 
tries to use the kernel full path and concatenate with the 'append' string. 

on M profiles, the semihosting startup code cannot afford to allocate very 
large buffers for the command line, as for the A profiles, and usually only 
some tens of bytes are available; with the full paths as argv[0] this buffer is 
already not enough, and the semihosting code returns an error, without passing 
anything back to the application.

both the J-Link GDB server and the openOCD GDB server have a method of passing 
a string as an *entire*, self-contained, command line to be returned by the 
semihosting SYS_GET_CMDLINE call, including the argv[0], without using anything 
else like executable path.

I consider this strategy to be reasonable for QEMU too, and I would insist on 
considering it.


so, for the A profile, nothing changes, use -kernel with the kernel name and 
-append with argv[1]...argv[n], as before.

for semihosting, regardless of profile, but especially to satisfy the M profile 
limitations, I suggest an explicit: 

-semihosting-config target=native,cmdline=myTest 1 2 3


I do not know if it makes any sense to start a linux kernel with semihosting 
enabled, but if it does, we can make the cmdline= optional, and, if missing and 
the -kernel and -append are present, we can apply the previous strategy, i.e. 
compose the long string from the full kernel path and the appended options. if 
it does not, we simplify things and always use cmdline= (my favourite).


regards,

Liviu


p.s. in my GNU ARM Eclipse branch I'll continue to use -image, and completely 
get rid of the misleading -kernel  -append options; I hope at a certain moment 
you'll accept qemu is also fit for non linux exclusive use cases.









 
 thanks
 -- PMM




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

2014-11-19 Thread Peter Maydell
On 19 November 2014 13:06, Liviu Ionescu i...@livius.net wrote:
 I already explained, but will do it again.

Sorry if I missed that; there's a lot of mail on qemu-devel...

 first, we are talking about two completely different use cases.

 on the A profile you have a kernel, and when you start it, you 'append' to 
 the kernel path some options, and this is what you pass to the bootloader. 
 this has nothing to do with semihosting, the command line is always used by 
 the bootloader to start the kernel.

 on the M profile we do not have a kernel, but an image (that you
 do not want to accept as a different thing), and for normal
 emulation (i.e. without semihosting) we do not have any command
 line options.

 now comes semihosting.

 I do not know how you used semihosting for other platforms, but for
 arm, the current code breaks with faults, so I have my doubts it was
 really used (if it was, ok).

It works fine on the A and R profile cores. The problems you
are encountering are with M profile in particular.

 anyway, the current code uses the same strategy as for the bootloader,
 i.e. tries to use the kernel full path and concatenate with the
 'append' string.

 on M profiles, the semihosting startup code cannot afford to allocate
 very large buffers for the command line, as for the A profiles, and
 usually only some tens of bytes are available; with the full paths
 as argv[0] this buffer is already not enough, and the semihosting
 code returns an error, without passing anything back to the application.

OK, this seems reasonable. You might want to pass an argv[0]
which wasn't the name of your binary anyway.

 so, for the A profile, nothing changes, use -kernel with the
 kernel name and -append with argv[1]...argv[n], as before.

I think we should support this new option in the same way for both
A profile and M profile.

 I do not know if it makes any sense to start a linux kernel with
 semihosting enabled

It does; there are kernel options to use semihosting for debug
console output.

, but if it does, we can make the cmdline= optional, and, if
 missing and the -kernel and -append are present, we can apply
 the previous strategy, i.e. compose the long string from the
 full kernel path and the appended options.

Yes, we should do this. (We have to make cmdline= optional anyway,
to support old and previously working command lines.)

I should probably also look at getting these new arguments
to work for the linux-user qemu binary (which has its
own separate option parsing code).

 p.s. in my GNU ARM Eclipse branch I'll continue to use -image,
 and completely get rid of the misleading -kernel  -append
 options; I hope at a certain moment you'll accept qemu is
 also fit for non linux exclusive use cases.

I absolutely think we should work with non-linux use cases
(and that those are the majority on M profile). It's just
that the option for providing a binary image to load
happens to be called -kernel regardless of whether the
thing you're loading is a Linux kernel or some other binary.
That's unfortunate and confusing, but it's not missing
functionality, and it's really hard to change our command
line options without breaking existing working setups.
(And any change absolutely has to be a very carefully
considered design which accounts for all architectures QEMU
supports, otherwise we're just making difficulties for
ourselves in future. I actually would like to see this
confusion cleaned up, but it's really a very hard
problem to solve properly.)

-- PMM



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

2014-11-19 Thread Liviu Ionescu

On 19 Nov 2014, at 15:20, Peter Maydell peter.mayd...@linaro.org wrote:

 Yes, we should do this. (We have to make cmdline= optional anyway,
 to support old and previously working command lines.)

ok, so I'll add the optional 'cmdline=' to the newly added 
'-semihosting-config'. since this was not used before, it will not break any 
compatibility.

 I should probably also look at getting these new arguments
 to work for the linux-user qemu binary (which has its
 own separate option parsing code).

sure, you can do it, but -kernel and -append will continue to work, as before.

 I absolutely think we should work with non-linux use cases ...

great!

as I said, I offer to catalyse and coordinate development for cortex-m related 
ones, to use them as part of the GNU ARM Eclipse project.

 ... and it's really hard to change our command
 line options without breaking existing working setups.

in my patch, the -image was implemented as a kind of special alias, the 
definition was finally copied to -kernel, so it should not break any working 
setup.

but no problem, I'll keep it local to my branch.


regards,

Liviu




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

2014-11-18 Thread Liviu Ionescu

On 17 Nov 2014, at 14:32, Peter Maydell peter.mayd...@linaro.org wrote:

 You can start out by separating out one or two and submitting
 those.

just did that, I submitted two patches, one to add the -semihosting-config and 
one to fix the exit code.

if you have further comments, please let me know.

once you integrate them, I'll work on another patch, to fix passing the 
semihosting command line down to the armv7m code, since currently this triggers 
exceptions due to null pointers.

my suggestion would be to extend the -semihosting-config option with 
cmdline=... instead of adding a separate option (in my first patch I used 
-semihosting-cmdline).

what do you think?


regards,

Liviu




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

2014-11-17 Thread Peter Maydell
On 13 November 2014 23:55, Liviu Ionescu i...@livius.net 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



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

2014-11-17 Thread Peter Maydell
On 17 November 2014 12:32, Peter Maydell peter.mayd...@linaro.org wrote:
 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

...and then my mail client sent that mail half-composed. The
missing fragment:

 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,
 .type = QEMU_OPT_STRING,
 }
 { /* end of list */ }
 },
 };

[and if you don't specify target then we default to 'auto'.]

Compare the handling of 'rtc' and similar options: you get
to have an implied option name so -foo is treated like
-foo enable=true, which gives us backwards compatibility
with our existing option, and a syntax for specifying the
target that fits in with the other command line options we
already have.

thanks
-- PMM



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

2014-11-17 Thread Liviu Ionescu

On 17 Nov 2014, at 14:32, Peter Maydell peter.mayd...@linaro.org wrote:

 that it's combining six different features and bug fixes into a
 single commit. Could you separate them out into their own patches?

sure. in practical terms, this requires separate branches and each be applied 
to master, right?

my experience with git is not as good as I would like (but improving), and, 
considering all changes are now in a single branch, could you suggest a simple 
way to do this?

 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
 ...

will try this

 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 ?

there are two issues here:

- try to completely ignore your use case of qemu; you have a simple embedded 
application, or a unit test, that you want to run under qemu, in a similar way 
you run it on the physical board; you read the qemu manual and you find out 
that you need to load a linux kernel; this makes absolutely no sense, the small 
elf you want to run has absolutely nothing to do with any kernel. the -kernel 
option is also accompanied by several other options, like -initrd, etc that 
also make no sense for regular embedded applications. 

- when using the -kernel option, there is also an -append option, to tell the 
kernel to start with a command line constructed by concatenating the kernel 
full path and the given options; this does not apply to non-linux images, for 
those the options are passed via semihosting entirely, including the argv[0]. 
hence the -semihosting-cmdline, that pairs with -image, not with -kernel, which 
pairs with -append, but with different functionality.

so, -image is more general than -kernel, and is a better match for general use, 
while -kernel is specific to unix/linux emulations. 


regards,

Liviu




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

2014-11-17 Thread Peter Maydell
On 17 November 2014 14:32, Liviu Ionescu i...@livius.net wrote:
 On 17 Nov 2014, at 14:32, Peter Maydell peter.mayd...@linaro.org wrote:

 that it's combining six different features and bug fixes into a
 single commit. Could you separate them out into their own patches?

 sure. in practical terms, this requires separate branches and each be
 applied to master, right?

Not necessarily. You could have one branch with six commits
on it, rebase that branch on master, and just send the patch or
two at the bottom of the stack to the mailing list.

 my experience with git is not as good as I would like (but improving),
 and, considering all changes are now in a single branch, could you
 suggest a simple way to do this?

Personally I use stgit as my tool for patch-management,
but plain git will also work fine here. You want to do an
interactive rebase, and you can then use the edit action
in the rebase for splitting the commit. This looks like
a decent explanation of how to do it:
http://git-scm.com/book/en/v2/Git-Tools-Rewriting-History#Splitting-a-Commit

Note that 'git add' has options for adding only some patch
hunks to a commit (rather than the default of adding all
the changes made to a file into the commit); see the man
page's section on interactive mode and the --patch option.

 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 ?

 there are two issues here:

 - try to completely ignore your use case of qemu; you have a simple embedded 
 application, or a unit test, that you want to run under qemu, in a similar 
 way you run it on the physical board; you read the qemu manual and you find 
 out that you need to load a linux kernel; this makes absolutely no sense, the 
 small elf you want to run has absolutely nothing to do with any kernel. the 
 -kernel option is also accompanied by several other options, like -initrd, 
 etc that also make no sense for regular embedded applications.

This is basically saying the -kernel option is not a great name
for that switch, right? If you pass it an ELF file it will do
the right thing, it's just not obvious (and possibly not documented)
that it will.

I'm sympathetic to the desire to clean up our argument handling
here, but we already have a confusing mess of options which have
different semantics in different situations, and it's going to
need real care to avoid just making the situation worse by
adding yet another option to the mess.

thanks
-- PMM



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

2014-11-17 Thread Liviu Ionescu

On 17 Nov 2014, at 14:35, Peter Maydell peter.mayd...@linaro.org wrote:

 Compare the handling of 'rtc' and similar options: you get
 to have an implied option name so -foo is treated like
 -foo enable=true,

I already considered this in a previous tentative version, but the 
implementation doesn't work like this, it always consumes the next option, for 
example calling with '... -rtc -semihosting ...' triggers:

qemu-system-arm: -rtc -semihosting: Invalid parameter '-semihosting'


if you can point to an option with optional parameters that works properly 
without them, I guess I can replicate the implementation.

regards,

Liviu




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

2014-11-13 Thread Liviu Ionescu

to test the patch, you can download an unit test application from:

https://dl.dropboxusercontent.com/u/78151643/gcm.elf


there are two use cases:

1) standalone (usually unit tests integrated in a continuous integration system)

$ ./qemu-system-arm -machine lm3s6965evb -image gcm.elf -semihosting 
-semihosting-cmdline gcm --gtest_output=xml:gcm.xml

(notice the gcm.xml file created in the current folder)


2) via GDB

$ ./qemu-system-arm -machine lm3s6965evb -semihosting-target native 
-semihosting-cmdline gcm --gtest_output=xml:gcm.xml -gdb tcp::1234 -S

(notice the image is no longer passed here)

and from another terminal

$ arm-none-eabi-gdb gcm.elf
(gdb) target remote localhost:1234
(gdb) load
(gdb) system_reset
(gdb) break main
(gdb) continue
Breakpoint 1, main (argc=2, argv=0x21b0 argv_buf.5809) at 
../gmock/src/gmock_main.cc:49
49  {
(gdb) continue
Continuing.
[Inferior 1 (Remote target) exited normally]

(notice the system_reset required after loading the image to initialise the PC 
 SP)


regards,

Liviu





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

2014-11-13 Thread Liviu Ionescu

On 14 Nov 2014, at 02:25, Liviu Ionescu i...@livius.net wrote:

 (gdb) system_reset

in fact this is 

(gdb) mon system_reset


Liviu