The interrupt injection of the serial emulation is completely broken. It's just doing random toggling of the interrupt line, which can lead to complete console hangs. The real hardware asserts the interrupt line when a condition (RX/TX/Status) is met and the corresponding interrupt is enabled in the IER. It's deasserted when the condition is cleared or the corresponding interrupt is disabled in the IER. So the correct emulation just needs to check after each state change in the LSR or the IER which bits in the IIR need to be set and update the interrupt line accordingly. To avoid setting the same state over and over keep an internal state of the last set interrupt line state and only update via the kvm ioctl when the new state differs. Rename serial8250__inject_interrupts() to serial8250__update_consoles() which reflects what the function really is about. Signed-off-by: Thomas Gleixner --- tools/kvm/hw/serial.c | 74 ++++++++++++++++++++++-------------- tools/kvm/include/kvm/8250-serial.h | 2 tools/kvm/x86/kvm.c | 2 3 files changed, 49 insertions(+), 29 deletions(-) Index: linux-kvm/tools/kvm/hw/serial.c =================================================================== --- linux-kvm.orig/tools/kvm/hw/serial.c +++ linux-kvm/tools/kvm/hw/serial.c @@ -18,6 +18,7 @@ struct serial8250_device { u16 iobase; u8 irq; + u8 irq_state; u8 rbr; /* receive buffer */ u8 dll; @@ -81,6 +82,31 @@ static struct serial8250_device devices[ }, }; +static void serial8250_update_irq(struct kvm *kvm, struct serial8250_device *dev) +{ + u8 iir = 0; + + /* Data ready and rcv interrupt enabled ? */ + if ((dev->ier & UART_IER_RDI) && (dev->lsr & UART_LSR_DR)) + iir |= UART_IIR_RDI; + + /* Transmitter empty and interrupt enabled ? */ + if ((dev->ier & UART_IER_THRI) && (dev->lsr & UART_LSR_TEMT)) + iir |= UART_IIR_THRI; + + /* Now update the irq line, if necessary */ + if (!iir) { + dev->iir = UART_IIR_NO_INT; + if (dev->irq_state) + kvm__irq_line(kvm, dev->irq, 0); + } else { + dev->iir = iir; + if (!dev->irq_state) + kvm__irq_line(kvm, dev->irq, 1); + } + dev->irq_state = iir; +} + #define SYSRQ_PENDING_NONE 0 #define SYSRQ_PENDING_BREAK 1 #define SYSRQ_PENDING_CMD 2 @@ -128,7 +154,7 @@ static void serial8250__receive(struct k dev->lsr |= UART_LSR_DR; } -void serial8250__inject_interrupt(struct kvm *kvm) +void serial8250__update_consoles(struct kvm *kvm) { unsigned int i; @@ -139,17 +165,7 @@ void serial8250__inject_interrupt(struct serial8250__receive(kvm, dev); - if (dev->ier & UART_IER_RDI && dev->lsr & UART_LSR_DR) - dev->iir = UART_IIR_RDI; - else if (dev->ier & UART_IER_THRI) - dev->iir = UART_IIR_THRI; - else - dev->iir = UART_IIR_NO_INT; - - if (dev->iir != UART_IIR_NO_INT) { - kvm__irq_line(kvm, dev->irq, 0); - kvm__irq_line(kvm, dev->irq, 1); - } + serial8250_update_irq(kvm, dev); mutex_unlock(&dev->mutex); } @@ -194,19 +210,26 @@ static bool serial8250_out(struct ioport if (!(dev->mcr & UART_MCR_LOOP)) term_putc(CONSOLE_8250, addr, size, dev->id); + /* else FIXME: Inject data into rcv path for LOOP */ - dev->iir = UART_IIR_NO_INT; + /* + * Set transmitter and transmit hold register + * empty. We have no FIFO at the moment and + * on the TX side it's only interesting, when + * we could coalesce port io on the kernel + * kernel. + */ + dev->lsr |= UART_LSR_TEMT | UART_LSR_THRE; + break; } else { dev->dll = ioport__read8(data); } break; case UART_IER: - if (!(dev->lcr & UART_LCR_DLAB)) { + if (!(dev->lcr & UART_LCR_DLAB)) dev->ier = ioport__read8(data) & 0x3f; - kvm__irq_line(kvm, dev->irq, dev->ier ? 1 : 0); - } else { + else dev->dlm = ioport__read8(data); - } break; case UART_FCR: dev->fcr = ioport__read8(data); @@ -231,6 +254,8 @@ static bool serial8250_out(struct ioport break; } + serial8250_update_irq(kvm, dev); + mutex_unlock(&dev->mutex); return ret; @@ -257,7 +282,6 @@ static bool serial8250_in(struct ioport } else { ioport__write8(data, dev->rbr); dev->lsr &= ~UART_LSR_DR; - dev->iir = UART_IIR_NO_INT; } break; case UART_IER: @@ -266,15 +290,9 @@ static bool serial8250_in(struct ioport else ioport__write8(data, dev->ier); break; - case UART_IIR: { - u8 iir = dev->iir; - - if (dev->fcr & UART_FCR_ENABLE_FIFO) - iir |= 0xc0; - - ioport__write8(data, iir); + case UART_IIR: + ioport__write8(data, dev->iir); break; - } case UART_LCR: ioport__write8(data, dev->lcr); break; @@ -283,7 +301,6 @@ static bool serial8250_in(struct ioport break; case UART_LSR: ioport__write8(data, dev->lsr); - dev->lsr &= ~(UART_LSR_OE|UART_LSR_PE|UART_LSR_FE|UART_LSR_BI); break; case UART_MSR: ioport__write8(data, dev->msr); @@ -295,6 +312,9 @@ static bool serial8250_in(struct ioport ret = false; break; } + + serial8250_update_irq(kvm, dev); + mutex_unlock(&dev->mutex); return ret; Index: linux-kvm/tools/kvm/include/kvm/8250-serial.h =================================================================== --- linux-kvm.orig/tools/kvm/include/kvm/8250-serial.h +++ linux-kvm/tools/kvm/include/kvm/8250-serial.h @@ -4,7 +4,7 @@ struct kvm; void serial8250__init(struct kvm *kvm); -void serial8250__inject_interrupt(struct kvm *kvm); +void serial8250__update_consoles(struct kvm *kvm); void serial8250__inject_sysrq(struct kvm *kvm); #endif /* KVM__8250_SERIAL_H */ Index: linux-kvm/tools/kvm/x86/kvm.c =================================================================== --- linux-kvm.orig/tools/kvm/x86/kvm.c +++ linux-kvm/tools/kvm/x86/kvm.c @@ -351,6 +351,6 @@ void kvm__arch_setup_firmware(struct kvm void kvm__arch_periodic_poll(struct kvm *kvm) { - serial8250__inject_interrupt(kvm); + serial8250__update_consoles(kvm); virtio_console__inject_interrupt(kvm); } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/