Alex Bennée <alex.ben...@linaro.org> writes: > I can see the use for this but I'd like to know what you are testing > with. We only have very basic smoketests in check-tcg but I've tested > with the latest arm-semihosting tests and they are all fine so no > regressions there.
I'm adding semihosting support to picolibc (https://keithp.com/picolibc/) and need a way to automate tests for it's SYS_READC support, so eventually I'll have automated tests there, but that depends on qemu support... > Please keep version history bellow --- so they get dropped when the > patch is applied. Sure, I'll edit the mail before sending. In my repo, I'm leaving the version history in git so I can keep track of it. >> v4: >> Add qemu_semihosting_console_init to stubs/semihost.c for >> hosts that don't support semihosting > > This doesn't appear to be in the diff which is why I'm seeing a compile > failure for non-CONFIG_SEMIHOST machines. However... Argh! Just git operation failure -- I'm building another patch on top of this (for RISC-V semihosting support) and the stubs/semihost.c change got caught in there. My overall check for changes missed this mistake. >> + >> +#include <poll.h> > > Headers should go at the top. Thanks; I've fixed this file and hw/semihosting/console.c >> case TARGET_SYS_READC: >> - qemu_log_mask(LOG_UNIMP, "%s: SYS_READC not implemented", __func__); >> - return 0; >> + return qemu_semihosting_console_inc(env); > > I'm not sure this would be correct if there was no character available. > The docs imply it blocks although don't say so explicitly AFAICT. Here's what the docs say: https://static.docs.arm.com/100863/0200/semihosting.pdf Return On exit, the RETURN REGISTER contains the byte read from the console. If this call didn't block, they'd have to define a value which indicated that no byte was available? Openocd also implements SYS_READC using 'getchar()', which definitely blocks. So, at least qemu would be the same. I realize it's really weird to block the whole emulation for this call, but given the target environment (deeply embedded devices), it's quite convenient as the whole qemu process blocks, instead of spinning. >> + /* connect semihosting console input if requested */ >> + qemu_semihosting_console_init(); >> + > > I'd rather rename qemu_semihosting_connect_chardevs to > qemu_semihosting_init and keep all our bits of semihosting setup in > there rather than having multiple calls out of vl.c I also thought this would be a nice cleanup. However, the last caller to qemu_chr_fe_set_handlers gets the focus for input, and connect_chardevs is called before the serial ports and monitor are initialized, so semihosting gets pushed aside and stdio input ends up hooked to the monitor instead. Adding this function and placing the call after the other stdio users get hooked up means that semihosting starts with the input focus. -- -keith
signature.asc
Description: PGP signature
