[PATCH] target/riscv/kvm: use qemu_chr_fe_write_all() in SBI_EXT_DBCN_CONSOLE_WRITE_BYTE

[PATCH] target/riscv/kvm: use qemu_chr_fe_write_all() in SBI_EXT_DBCN_CONSOLE_WRITE_BYTE

[Daniel Henrique Barboza]

The SBI spec states, for console write byte:

"This is a blocking SBI call and it will only return after writing the
specified byte to the debug console. It will also return, with
SBI_ERR_FAILED, if there are I/O errors."

Being a blocker call will either succeed writing the byte or error out,
it's feasible to use the blocking qemu_chr_fe_write_all() instead of
qemu_chr_fe_write(). This is also how SBI_EXT_DBCN_CONSOLE_WRITE is
implemented, so we're also being more consistent.

Last but not the least, we will duck possible changes in
qemu_chr_fe_write() where ret = 0 will have a 'zero byte written'
semantic [1] - something that we're not ready to deal in this current
state.

[1] https://lore.kernel.org/qemu-devel/ CAFEAcA_kEndvNtw4EHySXWwQPoGs029yAzZGGBcV=zGHaj7KUQ@mail.gmail.com/

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 target/riscv/kvm/kvm-cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index e1a04be20f..86724e5c44 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -1605,7 +1605,7 @@ static void kvm_riscv_handle_sbi_dbcn(CPUState *cs, struct kvm_run *run)
         break;
     case SBI_EXT_DBCN_CONSOLE_WRITE_BYTE:
         ch = run->riscv_sbi.args[0];
-        ret = qemu_chr_fe_write(serial_hd(0)->be, &ch, sizeof(ch));
+        ret = qemu_chr_fe_write_all(serial_hd(0)->be, &ch, sizeof(ch));
 
         if (ret < 0) {
             error_report("SBI_EXT_DBCN_CONSOLE_WRITE_BYTE: error when "
-- 
2.49.0

Re: [PATCH] target/riscv/kvm: use qemu_chr_fe_write_all() in SBI_EXT_DBCN_CONSOLE_WRITE_BYTE

[Philippe Mathieu-Daudé]

On 5/6/25 11:00, Daniel Henrique Barboza wrote:
> The SBI spec states, for console write byte:
> 
> "This is a blocking SBI call and it will only return after writing the
> specified byte to the debug console. It will also return, with
> SBI_ERR_FAILED, if there are I/O errors."
> 
> Being a blocker call will either succeed writing the byte or error out,
> it's feasible to use the blocking qemu_chr_fe_write_all() instead of
> qemu_chr_fe_write(). This is also how SBI_EXT_DBCN_CONSOLE_WRITE is
> implemented, so we're also being more consistent.
> 
> Last but not the least, we will duck possible changes in
> qemu_chr_fe_write() where ret = 0 will have a 'zero byte written'
> semantic [1] - something that we're not ready to deal in this current
> state.
> 
> [1] https://lore.kernel.org/qemu-devel/ CAFEAcA_kEndvNtw4EHySXWwQPoGs029yAzZGGBcV=zGHaj7KUQ@mail.gmail.com/
> 
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>   target/riscv/kvm/kvm-cpu.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Thank you very much Daniel!

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

Re: [PATCH] target/riscv/kvm: use qemu_chr_fe_write_all() in SBI_EXT_DBCN_CONSOLE_WRITE_BYTE

[Daniel Henrique Barboza]

On 6/5/25 6:26 AM, Philippe Mathieu-Daudé wrote:
> On 5/6/25 11:00, Daniel Henrique Barboza wrote:
>> The SBI spec states, for console write byte:
>>
>> "This is a blocking SBI call and it will only return after writing the
>> specified byte to the debug console. It will also return, with
>> SBI_ERR_FAILED, if there are I/O errors."
>>
>> Being a blocker call will either succeed writing the byte or error out,
>> it's feasible to use the blocking qemu_chr_fe_write_all() instead of
>> qemu_chr_fe_write(). This is also how SBI_EXT_DBCN_CONSOLE_WRITE is
>> implemented, so we're also being more consistent.
>>
>> Last but not the least, we will duck possible changes in
>> qemu_chr_fe_write() where ret = 0 will have a 'zero byte written'
>> semantic [1] - something that we're not ready to deal in this current
>> state.
>>
>> [1] https://lore.kernel.org/qemu-devel/ CAFEAcA_kEndvNtw4EHySXWwQPoGs029yAzZGGBcV=zGHaj7KUQ@mail.gmail.com/
>>
>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>> ---
>>   target/riscv/kvm/kvm-cpu.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Thank you very much Daniel!

No problem :)

FYI I just sent a v2 with a small change in the commit msg. Your ack was kept.
Thanks,


Daniel

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