Update semihosting to v7.2.0 #10
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Please open a PR on sdk-ng side that pulls this change. |
|
awojasinski
force-pushed
the
semihosting-v7.2.0
branch
from
d54901c to
3f0acca
Compare
|
fixed mips compilation |
There were 3 copies of these flags. Place them in the file with gdb_do_syscall, with which they belong. Reviewed-by: Alex Bennée <[email protected]> Reviewed-by: Peter Maydell <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit 94b14fe)
We have two copies of these structures, and require them in semihosting/ going forward. Reviewed-by: Alex Bennée <[email protected]> Reviewed-by: Peter Maydell <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit 7c56c2d)
Define constants for the errno values defined by the gdb remote fileio protocol. Reviewed-by: Alex Bennée <[email protected]> Reviewed-by: Peter Maydell <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit 9814483)
We had two sets of variables: arg_start/arg_end, and arg_strings/env_strings. In linuxload.c, we set the first pair to the bounds of the argv strings, but in elfload.c, we set the first pair to the bounds of the argv pointers and the second pair to the bounds of the argv strings. Remove arg_start/arg_end, replacing them with the standard argc/argv/envc/envp values. Retain arg_strings/env_strings with the meaning we were using in elfload.c. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/714 Reviewed-by: Peter Maydell <[email protected]> Reviewed-by: Philippe Mathieu-Daudé <[email protected]> Signed-off-by: Richard Henderson <[email protected]> Message-Id: <[email protected]> Signed-off-by: Laurent Vivier <[email protected]> (cherry picked from commit 60f1c80)
Currently we mishandle the --semihosting-config option if the user specifies it on the command line more than once. For example with: --semihosting-config target=gdb --semihosting-config arg=foo,arg=bar the function qemu_semihosting_config_options() is called twice, once for each argument. But that function expects to be called only once, and it always unconditionally sets the semihosting.enabled, semihost_chardev and semihosting.target variables. This means that if any of those options were set anywhere except the last --semihosting-config option on the command line, those settings are ignored. In the example above, 'target=gdb' in the first option is overridden by an implied default 'target=auto' in the second. The QemuOptsList machinery has a flag for handling this kind of "option group is setting global state": by setting .merge_lists = true; we make the machinery merge all the --semihosting-config arguments the user passes into a single set of options and call our qemu_semihosting_config_options() just once. Signed-off-by: Peter Maydell <[email protected]> Reviewed-by: Luc Michel <[email protected]> Message-id: [email protected] (cherry picked from commit 90c072e)
We have a subdirectory for semihosting; move this file out of exec. Rename to emphasize the contents are a replacement for the functions in linux-user/bsd-user uaccess.c. Reviewed-by: Peter Maydell <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit c89a14a)
Within do_interrupt, we hold the iothread lock, which is required for Chardev access for the console, and for the round trip for use_gdb_syscalls(). Reviewed-by: Philippe Mathieu-Daudé <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit 8ec7e3c)
From the Unified Hosting Interface, MD01069 Reference Manual, version 1.1.6, 06 July 2015. Reviewed-by: Philippe Mathieu-Daudé <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit 7ba6e53)
We don't implement it with _WIN32 hosts, and the syscalls are missing from the gdb remote file i/o interface. Since we can't implement them universally, drop them. Reviewed-by: Philippe Mathieu-Daudé <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit 6863e92)
While CONFIG_SEMIHOSTING is currently only set for softmmu, this will not continue to be true. Reviewed-by: Peter Maydell <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit 259739c)
Rather that static (and not even inline) functions within a header, move the functions to semihosting/uaccess.c. Reviewed-by: Peter Maydell <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit 0a92218)
Mirror the interface of the user-only function of the same name. Use probe_access_flags for the common case of ram, and cpu_memory_rw_debug for the uncommon case of mmio. Reviewed-by: Peter Maydell <[email protected]> Signed-off-by: Richard Henderson <[email protected]> --- v3: Use probe_access_flags (pmm) (cherry picked from commit 5f9ca6f)
We are not currently bounding the search to the 1024 bytes that we allocated, possibly overrunning the buffer. Use softmmu_strlen_user to find the length and allocate the correct size from the beginning. Reviewed-by: Alex Bennée <[email protected]> Reviewed-by: Peter Maydell <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit 3d5e2b4)
In arm-compat-semi.c, we have more advanced treatment of guest file descriptors than we do in other implementations. Split out GuestFD and related functions to a new file so that they can be shared. Reviewed-by: Alex Bennée <[email protected]> Reviewed-by: Peter Maydell <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit 1c6ff72)
Do not store 'err' into errno only to read it back immediately. Use 'ret' for the return value, not 'reg0'. Reviewed-by: Peter Maydell <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit 5aadd18)
The err parameter is non-zero if and only if an error occured. Use this instead of ret == -1 for determining if we need to update the saved errno. This fixes the errno setting of SYS_ISTTY, which returns 0 on error, not -1. Reviewed-by: Alex Bennée <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit 709fe27)
Do not read from the gdb struct stat buffer if the callback is reporting an error. Use common_semi_cb to finish returning results. Reviewed-by: Alex Bennée <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit 84ca0df)
Use common_semi_cb to return results instead of calling set_swi_errno and common_semi_set_ret directly. Reviewed-by: Alex Bennée <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit 4cfeff4)
Perform the cleanup in the FIXME comment in common_semi_gdb_syscall. Do not modify guest registers until the syscall is complete, which in the gdbstub case is asynchronous. In the synchronous non-gdbstub case, use common_semi_set_ret to set the result. Merge set_swi_errno into common_semi_cb. Rely on the latter for combined return value / errno setting. Reviewed-by: Alex Bennée <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit ed3a06b)
This header is not private to the top-level semihosting directory, so place it in the public include directory. Reviewed-by: Alex Bennée <[email protected]> Reviewed-by: Peter Maydell <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit bb3b882)
The value is zero, and gdb always opens files in binary mode. Reviewed-by: Alex Bennée <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit a1a2a3e)
Load the entire 64-bit size value. While we're at it, use offsetof instead of an integer constant. Reviewed-by: Peter Maydell <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit cd7f29e)
We already have some larger ifdef blocks for ARM and RISCV; split the function into multiple implementations per arch. Reviewed-by: Peter Maydell <[email protected]> Reviewed-by: Alistair Francis <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit ef9c5ea)
Allow more than one character to be read at one time. Will be used by m68k and nios2 semihosting for stdio. Reviewed-by: Luc Michel <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit e7fb6f3)
Rename qemu_semihosting_connect_chardevs to qemu_semihosting_chardev_init; pass the result directly to qemu_semihosting_console_init. Store the chardev in SemihostingConsole instead of SemihostingConfig, which lets us drop semihosting_get_chardev. Reviewed-by: Luc Michel <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit fb08790)
Will replace qemu_semihosting_console_{outs,outc},
but we need more plumbing first.
Reviewed-by: Luc Michel <[email protected]>
Signed-off-by: Richard Henderson <[email protected]>
(cherry picked from commit cd66f20)
Add a GuestFDType for connecting to the semihosting console. Hook up to read, write, isatty, and fstat syscalls. Note that the arm-specific syscall flen cannot be applied to the console, because the console is not a descriptor exposed to the guest. Reviewed-by: Luc Michel <[email protected]> Reviewed-by: Alex Bennée <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit 008e147)
For arm-compat, initialize console_{in,out}_gf;
otherwise, initialize stdio file descriptors.
This will go some way to cleaning up arm-compat, and
will allow other semihosting to use normal stdio.
Reviewed-by: Luc Michel <[email protected]>
Reviewed-by: Alex Bennée <[email protected]>
Signed-off-by: Richard Henderson <[email protected]>
(cherry picked from commit e4a4aaa)
Reviewed-by: Luc Michel <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit 1577eec)
Reviewed-by: Luc Michel <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit 5d77289)
This function has been replaced by *_write. Reviewed-by: Luc Michel <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit 004d2ab)
Reviewed-by: Luc Michel <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit 7281550)
This function has been replaced by *_write. Reviewed-by: Luc Michel <[email protected]> Reviewed-by: Alex Bennée <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit 2d010c2)
This will be used for implementing the xtensa select_one system call. Choose "poll" over "select" so that we can reuse Glib's g_poll constants and to avoid struct timeval. Reviewed-by: Luc Michel <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit 1b9177f)
The function is no longer used. Reviewed-by: Philippe Mathieu-Daudé <[email protected]> Signed-off-by: Richard Henderson <[email protected]> Message-Id: <[email protected]> Signed-off-by: Philippe Mathieu-Daudé <[email protected]> (cherry picked from commit 938fcd7)
The UHI specification does not have an EFAULT value, and further specifies that "undefined UHI operations should not return control to the target". So, log the error and abort. Signed-off-by: Richard Henderson <[email protected]> Reviewed-by: Philippe Mathieu-Daudé <[email protected]> Message-Id: <[email protected]> Signed-off-by: Philippe Mathieu-Daudé <[email protected]> (cherry picked from commit d53a3ed)
We don't implement it with _WIN32 hosts, and the syscall is missing from the gdb remote file i/o interface. Since we can't implement it universally, drop it. Signed-off-by: Richard Henderson <[email protected]> Reviewed-by: Philippe Mathieu-Daudé <[email protected]> Message-Id: <[email protected]> Signed-off-by: Philippe Mathieu-Daudé <[email protected]> (cherry picked from commit 3d748e4)
This separates guest file descriptors from host file descriptors, and utilizes shared infrastructure for integration with gdbstub. Signed-off-by: Richard Henderson <[email protected]> Reviewed-by: Philippe Mathieu-Daudé <[email protected]> Message-Id: <[email protected]> Signed-off-by: Philippe Mathieu-Daudé <[email protected]> (cherry picked from commit 18639a2)
Use semihost_sys_write and/or qemu_semihosting_console_write for implementing plog. When using gdbstub, copy the temp string below the stack so that gdb has a guest address from which to perform the log. Signed-off-by: Richard Henderson <[email protected]> Reviewed-by: Philippe Mathieu-Daudé <[email protected]> Message-Id: <[email protected]> Signed-off-by: Philippe Mathieu-Daudé <[email protected]> (cherry picked from commit ea42106)
…_write() failure The documentation comment for qemu_semihosting_console_write() says * Returns: number of bytes written -- this should only ever be short * on some sort of i/o error. and the callsites rely on this. However, the implementation code path which sends console output to a chardev doesn't honour this, and will return negative values on error. Bring it into line with the other implementation codepaths and the documentation, so that it returns 0 on error. Spotted by Coverity, because console_write() passes the return value to unlock_user(), which doesn't accept a negative length. Resolves: Coverity CID 1490288 Signed-off-by: Peter Maydell <[email protected]> Reviewed-by: Richard Henderson <[email protected]> Message-Id: <[email protected]> Signed-off-by: Alex Bennée <[email protected]> Reviewed-by: Philippe Mathieu-Daudé <[email protected]> Message-Id: <[email protected]> (cherry picked from commit aed04e6)
The console_write() semihosting function outputs guest data from a buffer; it doesn't update that buffer. It therefore doesn't need to pass a length value to unlock_user(), but can pass 0, meaning "do not copy any data back to the guest memory". Signed-off-by: Peter Maydell <[email protected]> Reviewed-by: Richard Henderson <[email protected]> Message-Id: <[email protected]> Signed-off-by: Alex Bennée <[email protected]> Message-Id: <[email protected]> (cherry picked from commit 45704e8)
The SET_ARG() macro returns an error indication; we check this in the TARGET_SYS_GET_CMDLINE case but not when we use it in implementing TARGET_SYS_ELAPSED. Check for and handle the errors via the do_fault codepath, and update the comment documenting the SET_ARG() and GET_ARG() macros to note how they handle memory access errors. Resolves: Coverity CID 1490287 Signed-off-by: Peter Maydell <[email protected]> Reviewed-by: Richard Henderson <[email protected]> Message-Id: <[email protected]> Signed-off-by: Alex Bennée <[email protected]> Message-Id: <[email protected]> (cherry picked from commit fed49cd)
The TARGET_SYS_TMPNAM implementation has two bugs spotted by Coverity: * confusion about whether 'len' has the length of the string including or excluding the terminating NUL means we lock_user() len bytes of memory but memcpy() len + 1 bytes * In the error-exit cases we forget to free() the buffer that asprintf() returned to us Resolves: Coverity CID 1490285, 1490289 Signed-off-by: Peter Maydell <[email protected]> Reviewed-by: Richard Henderson <[email protected]> Message-Id: <[email protected]> Signed-off-by: Alex Bennée <[email protected]> Message-Id: <[email protected]> (cherry picked from commit 9b1268f)
Currently our semihosting implementations generally prohibit use of semihosting calls in system emulation from the guest userspace. This is a very long standing behaviour justified originally "to provide some semblance of security" (since code with access to the semihosting ABI can do things like read and write arbitrary files on the host system). However, it is sometimes useful to be able to run trusted guest code which performs semihosting calls from guest userspace, notably for test code. Add a command line suboption to the existing semihosting-config option group so that you can explicitly opt in to semihosting from guest userspace with -semihosting-config userspace=on (There is no equivalent option for the user-mode emulator, because there by definition all code runs in userspace and has access to semihosting already.) This commit adds the infrastructure for the command line option and adds a bool 'is_user' parameter to the function semihosting_userspace_enabled() that target code can use to check whether it should be permitting the semihosting call for userspace. It mechanically makes all the callsites pass 'false', so they continue checking "is semihosting enabled in general". Subsequent commits will make each target that implements semihosting honour the userspace=on option by passing the correct value and removing whatever "don't do this for userspace" checking they were doing by hand. Signed-off-by: Peter Maydell <[email protected]> Acked-by: Alex Bennée <[email protected]> Reviewed-by: Alistair Francis <[email protected]> Reviewed-by: Philippe Mathieu-Daudé <[email protected]> Reviewed-by: Richard Henderson <[email protected]> Message-Id: <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit 5202861)
Honour the commandline -semihosting-config userspace=on option, instead of always permitting userspace semihosting calls in system emulation mode, by passing the correct value to the is_userspace argument of semihosting_enabled(). Note that this is a behaviour change: if the user wants to do semihosting calls from userspace they must now specifically enable them on the command line. MIPS semihosting is not implemented for linux-user builds. Signed-off-by: Peter Maydell <[email protected]> Reviewed-by: Philippe Mathieu-Daudé <[email protected]> Reviewed-by: Richard Henderson <[email protected]> Message-Id: <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit b35d740)
The old link has moved but it seems the document is now hosted on Arm's github along with a license update to CC-BY-SA-4.0. Signed-off-by: Alex Bennée <[email protected]> Reviewed-by: Richard Henderson <[email protected]> Message-Id: <[email protected]> (cherry picked from commit 424d5ec)
Use g_get_tmp_dir() to get the directory to use for temporary files. Signed-off-by: Bin Meng <[email protected]> Reviewed-by: Alex Bennée <[email protected]> Signed-off-by: Alex Bennée <[email protected]> Reviewed-by: Philippe Mathieu-Daudé <[email protected]> Reviewed-by: Richard Henderson <[email protected]> Message-Id: <[email protected]> Message-Id: <[email protected]> (cherry picked from commit 3878d0c)
Decode 'break 1' during translation, rather than doing it again during exception processing. Reviewed-by: Peter Maydell <[email protected]> Signed-off-by: Richard Henderson <[email protected]> Message-Id: <[email protected]> (cherry picked from commit 24ca313)
Honour the commandline -semihosting-config userspace=on option, instead of always permitting userspace semihosting calls in system emulation mode, by passing the correct value to the is_userspace argument of semihosting_enabled(). Note that this is a behaviour change: if the user wants to do semihosting calls from userspace they must now specifically enable them on the command line. nios2 semihosting is not implemented for linux-user builds. Signed-off-by: Peter Maydell <[email protected]> Reviewed-by: Richard Henderson <[email protected]> Message-Id: <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit cab9f19)
awojasinski
force-pushed
the
semihosting-v7.2.0
branch
from
3f0acca to
b2a5fad
Compare
|
@stephanosio tests are passing zephyrproject-rtos/sdk-ng#822 |
stephanosio
approved these changes
Oct 22, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR pulls in changes made to semihosting part of QEMU between
v7.0.0andv7.2.0. The change is needed as it fixes zephyrproject-rtos/zephyr#79359 bug discovered during development of the new flash emulation backend for the flash driver zephyrproject-rtos/zephyr#76639. Finding the one specific commit that fixes issue is very hard. There is no commit that fixes it but rather some change that apart from main purpose fixed it. So instead of trying to find right one and back porting it with a need of some alignment I decided to pull full diff betweenv7.2.0andv7.0.0. This way it shouldn't cause any merge conflicts during update to the newer QEMU revision.