Re: [Qemu-devel] [PATCH] functional ARM semihosting under GDB
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
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
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
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
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
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
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
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
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
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
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
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
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
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