On Fri, Feb 14, 2025 at 1:52 PM Clément Chigot <chi...@adacore.com> wrote:
>
> Hi Alistair,
>
> I've an issue following this patch. When the system is reset (e.g
> using HTIF syscalls), the fifo might not be empty and thus some
> characters are lost.
> I discovered it on a Windows host. But by extending
> "TX_INTERRUPT_TRIGGER_DELAY_NS" to a huge value, I'm able to reproduce
> on Linux as well.
The root cause of my issue was unrelated to these early shutdowns. On
Windows, the character device behind `-serial mon:stdio`
(char-win-stdio) doesn't provide an `add_watch` method. Therefore,
`qemu_chr_fe_add_watch` calls always result in an error, flushing the
fifo. I saw in @Philippe Mathieu-Daudé patch about pl011 that
`G_SOURCE_CONTINUE` is returned instead of calling it and it does
work. @Alistair Francis do you remember if there was a reason for
calling `add_watch` ?
> I've tried to flush within an unrealized function but it didn't work.
> Any suggestions ?
FTR, I still have found a solution here using
qemu_register_shutdown_notifier. Though I'm wondering if this is
useful: the cases where a shutdown occurs between two "fifo_update"
seems really narrow, but they could happen.
@Philippe Mathieu-Daudé AFAICT, the new pl011 and other char devices
implementing write fifo have the same issue. Thus, pinging you here to
get your advice.
Thanks,
Clément
> > static void sifive_uart_reset_enter(Object *obj, ResetType type)
> > {
> > ...
> > + fifo8_create(&s->tx_fifo, SIFIVE_UART_TX_FIFO_SIZE);
>
> I'm also wondering if that part could not lead to memory leak.
> `fifo8_destroy` is never called and AFAIK, there are ways to reset a
> device dynamically (e.g snapshot, though not sure if it's supported
> here).
>
> Thanks, Clément
