Re: [PATCH 1/2] hw/char: riscv_htif: Use blocking qemu_chr_fe_write_all

[Daniel Henrique Barboza]

On 8/14/24 10:54 PM, Alistair Francis wrote:
> The current approach of using qemu_chr_fe_write() and ignoring the
> return values results in dropped charecters [1]. Ideally we want to
> report FIFO status to the guest, but the HTIF isn't a real UART, so we
> don't really have a way to do that.
>
> Instead let's just use qemu_chr_fe_write_all() so at least we don't drop
> charecters.
>
> 1: https://gitlab.com/qemu-project/qemu/-/issues/2114
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---

Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

>   hw/char/riscv_htif.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/hw/char/riscv_htif.c b/hw/char/riscv_htif.c
> index 9bef60def1..d40b542948 100644
> --- a/hw/char/riscv_htif.c
> +++ b/hw/char/riscv_htif.c
> @@ -218,7 +218,11 @@ static void htif_handle_tohost_write(HTIFState *s, 
> uint64_t val_written)
>                       tswap64(syscall[3]) == HTIF_CONSOLE_CMD_PUTC) {
>                       uint8_t ch;
>                       cpu_physical_memory_read(tswap64(syscall[2]), &ch, 1);
> -                    qemu_chr_fe_write(&s->chr, &ch, 1);
> +                    /*
> +                     * XXX this blocks entire thread. Rewrite to use
> +                     * qemu_chr_fe_write and background I/O callbacks
> +                     */
> +                    qemu_chr_fe_write_all(&s->chr, &ch, 1);
>                       resp = 0x100 | (uint8_t)payload;
>                   } else {
>                       qemu_log_mask(LOG_UNIMP,
> @@ -237,7 +241,11 @@ static void htif_handle_tohost_write(HTIFState *s, 
> uint64_t val_written)
>               return;
>           } else if (cmd == HTIF_CONSOLE_CMD_PUTC) {
>               uint8_t ch = (uint8_t)payload;
> -            qemu_chr_fe_write(&s->chr, &ch, 1);
> +            /*
> +             * XXX this blocks entire thread. Rewrite to use
> +             * qemu_chr_fe_write and background I/O callbacks
> +             */
> +            qemu_chr_fe_write_all(&s->chr, &ch, 1);
>               resp = 0x100 | (uint8_t)payload;
>           } else {
>               qemu_log("HTIF device %d: unknown command\n", device);