Message ID | 20250318213209.2579218-27-richard.henderson@linaro.org |
---|---|
State | New |
Headers | show |
Series | accel/tcg, codebase: Build once patches | expand |
On 3/18/25 14:31, Richard Henderson wrote: > Avoid testing CONFIG_USER_ONLY in semihost.h. > The only function that's required is semihosting_enabled. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > include/semihosting/semihost.h | 29 ++--------------------------- > semihosting/user.c | 15 +++++++++++++++ > semihosting/meson.build | 2 ++ > 3 files changed, 19 insertions(+), 27 deletions(-) > create mode 100644 semihosting/user.c > > diff --git a/include/semihosting/semihost.h b/include/semihosting/semihost.h > index 97d2a2ba99..b03e637578 100644 > --- a/include/semihosting/semihost.h > +++ b/include/semihosting/semihost.h > @@ -26,32 +26,6 @@ typedef enum SemihostingTarget { > SEMIHOSTING_TARGET_GDB > } SemihostingTarget; > > -#ifdef CONFIG_USER_ONLY > -static inline bool semihosting_enabled(bool is_user) > -{ > - return true; > -} > - > -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; > -} > - > -static inline const char *semihosting_get_cmdline(void) > -{ > - return NULL; > -} > -#else /* !CONFIG_USER_ONLY */ > /** > * semihosting_enabled: > * @is_user: true if guest code is in usermode (i.e. not privileged) > @@ -59,17 +33,18 @@ static inline const char *semihosting_get_cmdline(void) > * Return true if guest code is allowed to make semihosting calls. > */ > bool semihosting_enabled(bool is_user); > + > SemihostingTarget semihosting_get_target(void); > const char *semihosting_get_arg(int i); > int semihosting_get_argc(void); > const char *semihosting_get_cmdline(void); > void semihosting_arg_fallback(const char *file, const char *cmd); > + > /* for vl.c hooks */ > void qemu_semihosting_enable(void); > int qemu_semihosting_config_options(const char *optstr); > void qemu_semihosting_chardev_init(void); > void qemu_semihosting_console_init(Chardev *); > -#endif /* CONFIG_USER_ONLY */ > void qemu_semihosting_guestfd_init(void); > > #endif /* SEMIHOST_H */ > diff --git a/semihosting/user.c b/semihosting/user.c > new file mode 100644 > index 0000000000..9473729beb > --- /dev/null > +++ b/semihosting/user.c > @@ -0,0 +1,15 @@ > +/* > + * Semihosting for user emulation > + * > + * Copyright (c) 2019 Linaro Ltd > + * > + * SPDX-License-Identifier: GPL-2.0-or-later > + */ > + > +#include "qemu/osdep.h" > +#include "semihosting/semihost.h" > + > +bool semihosting_enabled(bool is_user) > +{ > + return true; > +} > diff --git a/semihosting/meson.build b/semihosting/meson.build > index 86f5004bed..ab67f87e4f 100644 > --- a/semihosting/meson.build > +++ b/semihosting/meson.build > @@ -15,5 +15,7 @@ system_ss.add(when: ['CONFIG_SEMIHOSTING'], if_true: files( > 'stubs-system.c', > )) > > +user_ss.add(when: 'CONFIG_SEMIHOSTING', if_true: files('user.c')) > + > specific_ss.add(when: ['CONFIG_ARM_COMPATIBLE_SEMIHOSTING'], > if_true: files('arm-compat-semi.c')) Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
On 18/3/25 22:31, Richard Henderson wrote: > Avoid testing CONFIG_USER_ONLY in semihost.h. > The only function that's required is semihosting_enabled. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > include/semihosting/semihost.h | 29 ++--------------------------- > semihosting/user.c | 15 +++++++++++++++ > semihosting/meson.build | 2 ++ > 3 files changed, 19 insertions(+), 27 deletions(-) > create mode 100644 semihosting/user.c > diff --git a/semihosting/user.c b/semihosting/user.c > new file mode 100644 > index 0000000000..9473729beb > --- /dev/null > +++ b/semihosting/user.c > @@ -0,0 +1,15 @@ > +/* > + * Semihosting for user emulation > + * > + * Copyright (c) 2019 Linaro Ltd > + * > + * SPDX-License-Identifier: GPL-2.0-or-later > + */ > + > +#include "qemu/osdep.h" > +#include "semihosting/semihost.h" > + > +bool semihosting_enabled(bool is_user) > +{ While moving this code, we could also add: assert(is_user); > + return true; > +} Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
On 3/19/25 00:16, Philippe Mathieu-Daudé wrote: > On 18/3/25 22:31, Richard Henderson wrote: >> Avoid testing CONFIG_USER_ONLY in semihost.h. >> The only function that's required is semihosting_enabled. >> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> include/semihosting/semihost.h | 29 ++--------------------------- >> semihosting/user.c | 15 +++++++++++++++ >> semihosting/meson.build | 2 ++ >> 3 files changed, 19 insertions(+), 27 deletions(-) >> create mode 100644 semihosting/user.c > > >> diff --git a/semihosting/user.c b/semihosting/user.c >> new file mode 100644 >> index 0000000000..9473729beb >> --- /dev/null >> +++ b/semihosting/user.c >> @@ -0,0 +1,15 @@ >> +/* >> + * Semihosting for user emulation >> + * >> + * Copyright (c) 2019 Linaro Ltd >> + * >> + * SPDX-License-Identifier: GPL-2.0-or-later >> + */ >> + >> +#include "qemu/osdep.h" >> +#include "semihosting/semihost.h" >> + >> +bool semihosting_enabled(bool is_user) >> +{ > > While moving this code, we could also add: > > assert(is_user); I have added this as a separate patch. r~
On 3/18/25 14:31, Richard Henderson wrote: > Avoid testing CONFIG_USER_ONLY in semihost.h. > The only function that's required is semihosting_enabled. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> This breaks bsd-user, CONFIG_SEMIHOSTING is not defined in configs/targets/*bsd-user*, thus the user stub is not included. Since in user mode we always return true, we can simply add the stub inconditionnally for all the user binaries. See here for details and a patch fixing it [1] (also attached to this email). [1] https://github.com/pbo-linaro/qemu/commit/d105112e11e521ff82b328be5f8fdc2af38aa75b Regards, Pierrick
diff --git a/include/semihosting/semihost.h b/include/semihosting/semihost.h index 97d2a2ba99..b03e637578 100644 --- a/include/semihosting/semihost.h +++ b/include/semihosting/semihost.h @@ -26,32 +26,6 @@ typedef enum SemihostingTarget { SEMIHOSTING_TARGET_GDB } SemihostingTarget; -#ifdef CONFIG_USER_ONLY -static inline bool semihosting_enabled(bool is_user) -{ - return true; -} - -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; -} - -static inline const char *semihosting_get_cmdline(void) -{ - return NULL; -} -#else /* !CONFIG_USER_ONLY */ /** * semihosting_enabled: * @is_user: true if guest code is in usermode (i.e. not privileged) @@ -59,17 +33,18 @@ static inline const char *semihosting_get_cmdline(void) * Return true if guest code is allowed to make semihosting calls. */ bool semihosting_enabled(bool is_user); + SemihostingTarget semihosting_get_target(void); const char *semihosting_get_arg(int i); int semihosting_get_argc(void); const char *semihosting_get_cmdline(void); void semihosting_arg_fallback(const char *file, const char *cmd); + /* for vl.c hooks */ void qemu_semihosting_enable(void); int qemu_semihosting_config_options(const char *optstr); void qemu_semihosting_chardev_init(void); void qemu_semihosting_console_init(Chardev *); -#endif /* CONFIG_USER_ONLY */ void qemu_semihosting_guestfd_init(void); #endif /* SEMIHOST_H */ diff --git a/semihosting/user.c b/semihosting/user.c new file mode 100644 index 0000000000..9473729beb --- /dev/null +++ b/semihosting/user.c @@ -0,0 +1,15 @@ +/* + * Semihosting for user emulation + * + * Copyright (c) 2019 Linaro Ltd + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include "qemu/osdep.h" +#include "semihosting/semihost.h" + +bool semihosting_enabled(bool is_user) +{ + return true; +} diff --git a/semihosting/meson.build b/semihosting/meson.build index 86f5004bed..ab67f87e4f 100644 --- a/semihosting/meson.build +++ b/semihosting/meson.build @@ -15,5 +15,7 @@ system_ss.add(when: ['CONFIG_SEMIHOSTING'], if_true: files( 'stubs-system.c', )) +user_ss.add(when: 'CONFIG_SEMIHOSTING', if_true: files('user.c')) + specific_ss.add(when: ['CONFIG_ARM_COMPATIBLE_SEMIHOSTING'], if_true: files('arm-compat-semi.c'))
Avoid testing CONFIG_USER_ONLY in semihost.h. The only function that's required is semihosting_enabled. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- include/semihosting/semihost.h | 29 ++--------------------------- semihosting/user.c | 15 +++++++++++++++ semihosting/meson.build | 2 ++ 3 files changed, 19 insertions(+), 27 deletions(-) create mode 100644 semihosting/user.c