On Mon, Jan 30, 2023 at 12:32:22PM -0500, Dave Voutila wrote:
> vmd's serial console has a race condition and can generate interrupt
> storms as the ns8250 device constantly asserts and deasserts its irq.
>
> Easiest way to see this is on older, slower hardware OR running nested
> such as OpenBSD guest inside OpenBSD vmm atop Linux KVM. I don't know
> enough about how consoles work to understand why, but if you use a shell
> with history that lets you hit the up-arrow it can trigger this fairly
> consistently.
>
> When triggered, you will experience the serial console "lock up" and the
> vmd process for the vm spike > 100% (the cpu thread is constantly
> exiting due to interrupt and the event thread is constantly firing
> ioctls). Anywhere from 10s to a few minutes later, it may stop and
> control in the console resumes.
>
> btrace shows the vm exit rate spike with the exit reason being external
> interrupt.
>
> AFAICT what happens is the kevent for data available on the com1 fd
> fires and, if it was !EV_WRITE, we assert/deassert the irq even if we
> weren't ready to actually receive data off the fd. This keeps kicking
> the poor vcpu for no reason making it slower to get the ns8250 into a
> state where it is slow to get back into a LSR_RXRDY state.
>
> OK?
>
This needs to be widely tested. IIRC there were old linux kernels that
behaved differently and were sensitive to where/when irqs were injected
for the serial ports, leading to various issues like needing to press a
key for the console to advance, and/or seeing "too much work" messages
from the kernel.
I'd recommend testing on a bunch of platforms.
-ml
>
> diff refs/heads/master refs/heads/vmd-ns8250
> commit - ef14a9e8cae106563ff9ce15d913365f9ad3fa0e
> commit + 5c76914a8c33243ec5ccc82689d0dadaa7cae666
> blob - dbb6568714c192447a99017fcf92bcc4bcc90ba6
> blob + 96e1f2533691205bdeb8e8b6dafee92603494c44
> --- usr.sbin/vmd/ns8250.c
> +++ usr.sbin/vmd/ns8250.c
> @@ -153,14 +153,15 @@ com_rcv_event(int fd, short kind, void *arg)
> return;
> }
>
> - if ((com1_dev.regs.lsr & LSR_RXRDY) == 0)
> + if ((com1_dev.regs.lsr & LSR_RXRDY) == 0) {
> com_rcv(&com1_dev, (uintptr_t)arg, 0);
>
> - /* If pending interrupt, inject */
> - if ((com1_dev.regs.iir & IIR_NOPEND) == 0) {
> - /* XXX: vcpu_id */
> - vcpu_assert_pic_irq((uintptr_t)arg, 0, com1_dev.irq);
> - vcpu_deassert_pic_irq((uintptr_t)arg, 0, com1_dev.irq);
> + /* If pending interrupt, inject */
> + if ((com1_dev.regs.iir & IIR_NOPEND) == 0) {
> + /* XXX: vcpu_id */
> + vcpu_assert_pic_irq((uintptr_t)arg, 0, com1_dev.irq);
> + vcpu_deassert_pic_irq((uintptr_t)arg, 0, com1_dev.irq);
> + }
> }
>
> mutex_unlock(&com1_dev.mutex);
