Re: [PATCH 0/5] semihosting: Reduce target specific code

["Alex Bennée" <alex.bennee@linaro.org>]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> This series makes semihosting config.c and console.c
> target agnostic, building them once, removing symbol
> collision of the following functions in the single
> binary:

Queued to semihosting/next, thanks.

>  - qemu_semihosting_chardev_init
>  - qemu_semihosting_config_options
>  - qemu_semihosting_config_opts
>  - qemu_semihosting_enable
>  - semihosting_arg_fallback
>  - semihosting_enabled
>  - semihosting_get_argc
>  - semihosting_get_target
>
> This function is still problematic, being built for
> each target:
>
>  - qemu_semihosting_guestfd_init
>
> Note, it depends on CONFIG_ARM_COMPATIBLE_SEMIHOSTING
> which is target specific, so doesn't scale in a
> heterogeneous setup like the ZynqMP machine, having
> ARM cores with CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y and
> MicroBlaze ones with CONFIG_ARM_COMPATIBLE_SEMIHOSTING=n.

Does MicroBlaze even do semihosting?

> I suppose the semihosting API needs rework to consider
> the CPUClass? I'll let that investigation for the
> maintainer ;)

Hmm most of it is already handled as EXCP_SEMIHOST exceptions are dealt
with withing the target specific exception handlers.
do_common_semihosting could be renamed though - do_armc_semihosting()
maybe?

If we have the full list of CPUs at qemu_semihosting_chardev_init() time
we could then selectively do the bits of qemu_semihosting_guestfd_init()
depending on what combination we have. For normal open/read/write stuff
I think they could co-exist.

Two independent cores could still write to stdout (0) though. Fixing
that would need a per-cpu semihosting config.

>
> Regards,
>
> Phil.
>
> Philippe Mathieu-Daudé (5):
>   semihosting/syscalls: Include missing 'exec/cpu-defs.h' header
>   semihosting/uaccess: Include missing 'exec/cpu-all.h' header
>   semihosting/arm-compat: Include missing 'cpu.h' header
>   semihosting/console: Avoid including 'cpu.h'
>   semihosting/meson: Build config.o and console.o once
>
>  include/semihosting/console.h  | 2 --
>  include/semihosting/syscalls.h | 1 +
>  semihosting/arm-compat-semi.c  | 1 +
>  semihosting/console.c          | 3 ++-
>  semihosting/uaccess.c          | 1 +
>  semihosting/meson.build        | 9 ++++++---
>  6 files changed, 11 insertions(+), 6 deletions(-)

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro