Re: hw/char: a question about watch callback function in serial

[LIU Zhiwei <zhiwei_liu@c-sky.com>]

[-- Attachment #1: Type: text/plain, Size: 4534 bytes --]



On 2020/6/5 10:19, LIU Zhiwei wrote:
>
>
> On 2020/6/4 21:32, Peter Maydell wrote:
>> On Thu, 4 Jun 2020 at 13:15, LIU Zhiwei<zhiwei_liu@c-sky.com>  wrote:
>>> I see many UART implementations have a G_IO_OUT | G_IO_HUP  callback function.
>>>
>>> In hw/serial.c, it is serial_watch_cb, setting by the following code,
>>>
>>>    s->watch_tag = qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP,
>>>
>>>                        serial_watch_cb, s);
>>>
>>> In hw/candence_uart.c, it is cadence_uart_xmit, setting by the following code,
>>>
>>>          guint r = qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP,
>>>
>>>                                          cadence_uart_xmit, s);
>>>
>>>
>>>
>>> I tried to call it with booting a Linux, but the interface will never be called.
>>>
>>> Can someone give a reasonable answer why needs this interface, or a way to call it.
>> This code is here to handle the case where the UART wants to pass
>> data to the chardev which is acting as the backend to the UART
>> (which might be host stdio, a TCP port, etc), but the backend
>> cannot accept data.
>>
>> Older UART code (eg hw/char/pl011.c) calls qemu_chr_fe_write_all()
>> to write data, but this is a blocking call and these calls are
>> usually marked with an XXX/TODO comment, because if the chardev
>> backend can't currently accept the data then execution of the
>> guest will be blocked until the backend does start to accept
>> data again.
>>
>> The solution to this bug was the introduction of the non-blocking
>> qemu_chr_fe_write() call. But to use the non-blocking call, the
>> UART emulation code now needs to handle the case where
>> qemu_chr_fe_write() says "I couldn't write all the data you asked
>> me to". In that case, it must use qemu_chr_fe_add_watch() to
>> request a callback when the chardev is able to accept new data,
>> so that it can try again. (It also needs to emulate telling the
>> guest that the transmit FIFO is not yet empty via whatever status
>> registers the UART has for that, because in the meantime guest
>> execution will continue with some of the data still not sent to
>> the chardev, but sitting in the emulated FIFO; and it needs to
>> correctly emulate "guest tried to write more data to a full FIFO".
>> Older UART emulations that use the blocking write_all function
>> don't need to bother with these details because there the tx
>> FIFO is always empty -- from the guest's perspective data written
>> to the tx FIFO drains instantaneously.)
>>
>> The common case execution path is "the chardev can accept the data
>> faster than the guest can feed it to the UART", in which case
>> qemu_chr_fe_write() will return 'wrote all the data' and the
>> UART never needs to call qemu_chr_fe_add_watch(). To exercise the
>> add-watch codepath you need to connect the UART to a chardev
>> that can be made to stop accepting data (for instance a pipe
>> or a unix domain socket where there's nothing on the other end
>> reading data.)
> Hi Peter,
>
> Thanks, it's really a reasonable answer. However I still have one 
> question.
>
> When I tried to verify the code-path, the callback is not triggered.
> The serial I used is hw/serial.c, back ended with a named pipe.
>
> The first step is make named pipe by
> mkfifo xpipe
> Then run a RISC-V Linux by the command
> gdb --args qemu-system-riscv64 -M virt -kernel fw_jump.elf -device loader,file=Image,addr=0x80200000 \
> -append "rootwait root=/dev/vda ro" -drive file=rootfs.ext2,format=raw,id=hd0 \
> -device virtio-blk-device,drive=hd0 -serial pipe:xpipe -smp 1
> Set a breakpoint on serial_watch_cb before run the Linux
> b serial_watch_cb
> Then run the Linux. The breakpoint will never matched.  I think "there 
> is nothing on the other end reading data".
>
> Then I tried another way to verify. Read the pipe a little later after 
> booting(There are some data in the FIFO already) by
> cat xpipe
> Now I can read some data out from the pipe. But it still can't match 
> the breakpoint. I think the reading  will set the G_IO_OUT condition,
> but it doesn't.
>
> Is there something wrong? Or could you show me a case./
> /
Hi folks,

I find it's the pipe buffer size is big enough to contain all the output 
of Linux booting.
So it will not call the qemu_chr_fe_add_watch.

If I output more characters more than 64KB,  it will call the 
qemu_chr_fe_add_watch and serial_watch_cb.

By this way, I verify the code-path.

Thanks Peter. It's really a right answer.

Zhiwei/
/
> /
> /Best Regards,
> Zhiwei/
> /
>> thanks
>> -- PMM
>


[-- Attachment #2: Type: text/html, Size: 6329 bytes --]