Lost interrupts with upstream KVM

Hi Gleb,

with latest kernel modules, namely beginning with 6bc0a1a235 (Remove irq_pending bitmap), I'm loosing interrupts with upstream's KVM support. After some bisecting, hair-pulling and a bit meditation I added a WARN_ON(kvm_cpu_has_interrupt(vcpu)) to kvm_vcpu_ioctl_interrupt, and it actually triggered right before the guest got stuck.

This didn't trigger with qemu-kvm (and -no-kvm-irqchip) yet but, on the other hand, I currently do not see a potential bug in upstream's kvm_arch_pre_run. Could you have a look if you can reproduce, specifically if this isn't a KVM kernel issue in the end?

Hi Gleb,

with latest kernel modules, namely beginning with 6bc0a1a235 (Remove irq_pending bitmap), I'm loosing interrupts with upstream's KVM support. After some bisecting, hair-pulling and a bit meditation I added a WARN_ON(kvm_cpu_has_interrupt(vcpu)) to kvm_vcpu_ioctl_interrupt, and it actually triggered right before the guest got stuck.

This didn't trigger with qemu-kvm (and -no-kvm-irqchip) yet but, on the other hand, I currently do not see a potential bug in upstream's kvm_arch_pre_run. Could you have a look if you can reproduce, specifically if this isn't a KVM kernel issue in the end?

Quoted 8 lines · Jan Kiszka

Hi Gleb,

with latest kernel modules, namely beginning with 6bc0a1a235 (Remove irq_pending bitmap), I'm loosing interrupts with upstream's KVM support. After some bisecting, hair-pulling and a bit meditation I added a WARN_ON(kvm_cpu_has_interrupt(vcpu)) to kvm_vcpu_ioctl_interrupt, and it actually triggered right before the guest got stuck.

Quoted 5 lines

This didn't trigger with qemu-kvm (and -no-kvm-irqchip) yet but, on the other hand, I currently do not see a potential bug in upstream's kvm_arch_pre_run. Could you have a look if you can reproduce, specifically if this isn't a KVM kernel issue in the end?

In kvm_cpu_exec() after calling kvm_arch_pre_run() env->exit_request is tested and function can exit without calling kvm_vcpu_ioctl(KVM_RUN). Can you check if this what happens in your case?

Quoted 14 lines · Gleb Natapov

Quoted 13 lines · Jan Kiszka

Hi Gleb,

with latest kernel modules, namely beginning with 6bc0a1a235 (Remove irq_pending bitmap), I'm loosing interrupts with upstream's KVM support. After some bisecting, hair-pulling and a bit meditation I added a WARN_ON(kvm_cpu_has_interrupt(vcpu)) to kvm_vcpu_ioctl_interrupt, and it actually triggered right before the guest got stuck.

This didn't trigger with qemu-kvm (and -no-kvm-irqchip) yet but, on the other hand, I currently do not see a potential bug in upstream's kvm_arch_pre_run. Could you have a look if you can reproduce, specifically if this isn't a KVM kernel issue in the end?

Quoted 3 lines

In kvm_cpu_exec() after calling kvm_arch_pre_run() env->exit_request is tested and function can exit without calling kvm_vcpu_ioctl(KVM_RUN). Can you check if this what happens in your case?

This path is executed quite frequently here. No obvious correlation with the lost IRQ.

Quoted 15 lines · Jan Kiszka

Quoted 14 lines · Gleb Natapov

Quoted 13 lines · Jan Kiszka

Hi Gleb,

with latest kernel modules, namely beginning with 6bc0a1a235 (Remove irq_pending bitmap), I'm loosing interrupts with upstream's KVM support. After some bisecting, hair-pulling and a bit meditation I added a WARN_ON(kvm_cpu_has_interrupt(vcpu)) to kvm_vcpu_ioctl_interrupt, and it actually triggered right before the guest got stuck.

This didn't trigger with qemu-kvm (and -no-kvm-irqchip) yet but, on the other hand, I currently do not see a potential bug in upstream's kvm_arch_pre_run. Could you have a look if you can reproduce, specifically if this isn't a KVM kernel issue in the end?

Quoted 6 lines

Quoted 3 lines

In kvm_cpu_exec() after calling kvm_arch_pre_run() env->exit_request is tested and function can exit without calling kvm_vcpu_ioctl(KVM_RUN). Can you check if this what happens in your case?

This path is executed quite frequently here. No obvious correlation with the lost IRQ.

If kvm_arch_pre_run() injected interrupt kvm_vcpu_ioctl(KVM_RUN) have to be executed before injecting another interrupt, so if on the fist call of kvm_cpu_exec() kvm_arch_pre_run() injected interrupt, but kvm_vcpu_ioctl(KVM_RUN) was not executed because of env->exit_request and on the next kvm_cpu_exec() other interrupt is injected the previous one will be lost.

Quoted 22 lines · Gleb Natapov

Quoted 15 lines · Jan Kiszka

Quoted 14 lines · Gleb Natapov

Quoted 13 lines · Jan Kiszka

Hi Gleb,

with latest kernel modules, namely beginning with 6bc0a1a235 (Remove irq_pending bitmap), I'm loosing interrupts with upstream's KVM support. After some bisecting, hair-pulling and a bit meditation I added a WARN_ON(kvm_cpu_has_interrupt(vcpu)) to kvm_vcpu_ioctl_interrupt, and it actually triggered right before the guest got stuck.

This didn't trigger with qemu-kvm (and -no-kvm-irqchip) yet but, on the other hand, I currently do not see a potential bug in upstream's kvm_arch_pre_run. Could you have a look if you can reproduce, specifically if this isn't a KVM kernel issue in the end?

Quoted 6 lines

Quoted 3 lines

In kvm_cpu_exec() after calling kvm_arch_pre_run() env->exit_request is tested and function can exit without calling kvm_vcpu_ioctl(KVM_RUN). Can you check if this what happens in your case?

This path is executed quite frequently here. No obvious correlation with the lost IRQ.

Quoted 6 lines

If kvm_arch_pre_run() injected interrupt kvm_vcpu_ioctl(KVM_RUN) have to be executed before injecting another interrupt, so if on the fist call of kvm_cpu_exec() kvm_arch_pre_run() injected interrupt, but kvm_vcpu_ioctl(KVM_RUN) was not executed because of env->exit_request and on the next kvm_cpu_exec() other interrupt is injected the previous one will be lost.

...and kvm_run->ready_for_interrupt_injection is not updated either in that case, right? That makes be wonder if KVM_INTERRUPT shouldn't better return an error in case the queue is full already.

Quoted 29 lines · Jan Kiszka

Quoted 28 lines · Gleb Natapov

Quoted 15 lines · Jan Kiszka

Quoted 14 lines · Gleb Natapov

Quoted 13 lines · Jan Kiszka

Hi Gleb,

with latest kernel modules, namely beginning with 6bc0a1a235 (Remove irq_pending bitmap), I'm loosing interrupts with upstream's KVM support. After some bisecting, hair-pulling and a bit meditation I added a WARN_ON(kvm_cpu_has_interrupt(vcpu)) to kvm_vcpu_ioctl_interrupt, and it actually triggered right before the guest got stuck.

This didn't trigger with qemu-kvm (and -no-kvm-irqchip) yet but, on the other hand, I currently do not see a potential bug in upstream's kvm_arch_pre_run. Could you have a look if you can reproduce, specifically if this isn't a KVM kernel issue in the end?

Quoted 6 lines

Quoted 3 lines

In kvm_cpu_exec() after calling kvm_arch_pre_run() env->exit_request is tested and function can exit without calling kvm_vcpu_ioctl(KVM_RUN). Can you check if this what happens in your case?

This path is executed quite frequently here. No obvious correlation with the lost IRQ.

If kvm_arch_pre_run() injected interrupt kvm_vcpu_ioctl(KVM_RUN) have to be executed before injecting another interrupt, so if on the fist call of kvm_cpu_exec() kvm_arch_pre_run() injected interrupt, but kvm_vcpu_ioctl(KVM_RUN) was not executed because of env->exit_request and on the next kvm_cpu_exec() other interrupt is injected the previous one will be lost.

Quoted 4 lines

...and kvm_run->ready_for_interrupt_injection is not updated either in that case, right? That makes be wonder if KVM_INTERRUPT shouldn't better return an error in case the queue is full already.

If kvm_vcpu_ioctl(KVM_RUN) is called, but exit happens before interrupt is injected kvm_run->ready_for_interrupt_injection should be update to reflect that fact.

Quoted 34 lines · Gleb Natapov

Quoted 33 lines · Jan Kiszka

Quoted 22 lines · Gleb Natapov

Quoted 15 lines · Jan Kiszka

Quoted 14 lines · Gleb Natapov

Quoted 13 lines · Jan Kiszka

Hi Gleb,

with latest kernel modules, namely beginning with 6bc0a1a235 (Remove irq_pending bitmap), I'm loosing interrupts with upstream's KVM support. After some bisecting, hair-pulling and a bit meditation I added a WARN_ON(kvm_cpu_has_interrupt(vcpu)) to kvm_vcpu_ioctl_interrupt, and it actually triggered right before the guest got stuck.

This didn't trigger with qemu-kvm (and -no-kvm-irqchip) yet but, on the other hand, I currently do not see a potential bug in upstream's kvm_arch_pre_run. Could you have a look if you can reproduce, specifically if this isn't a KVM kernel issue in the end?

Quoted 6 lines

Quoted 3 lines

In kvm_cpu_exec() after calling kvm_arch_pre_run() env->exit_request is tested and function can exit without calling kvm_vcpu_ioctl(KVM_RUN). Can you check if this what happens in your case?

This path is executed quite frequently here. No obvious correlation with the lost IRQ.

Quoted 6 lines

If kvm_arch_pre_run() injected interrupt kvm_vcpu_ioctl(KVM_RUN) have to be executed before injecting another interrupt, so if on the fist call of kvm_cpu_exec() kvm_arch_pre_run() injected interrupt, but kvm_vcpu_ioctl(KVM_RUN) was not executed because of env->exit_request and on the next kvm_cpu_exec() other interrupt is injected the previous one will be lost.

...and kvm_run->ready_for_interrupt_injection is not updated either in that case, right? That makes be wonder if KVM_INTERRUPT shouldn't better return an error in case the queue is full already.

Quoted 3 lines

If kvm_vcpu_ioctl(KVM_RUN) is called, but exit happens before interrupt is injected kvm_run->ready_for_interrupt_injection should be update to reflect that fact.

Yes, but in this case it isn't called if IIUC. So that is the problem upstream KVM faces?

Then again: What do you think is the proper long-term fix? Only adjusting upstream KVM (required anyway) or also making the kernel support more robust against this pattern?

Quoted 35 lines · Jan Kiszka

Quoted 34 lines · Gleb Natapov

Quoted 33 lines · Jan Kiszka

Quoted 22 lines · Gleb Natapov

Quoted 15 lines · Jan Kiszka

Quoted 14 lines · Gleb Natapov

Quoted 13 lines · Jan Kiszka

Hi Gleb,

with latest kernel modules, namely beginning with 6bc0a1a235 (Remove irq_pending bitmap), I'm loosing interrupts with upstream's KVM support. After some bisecting, hair-pulling and a bit meditation I added a WARN_ON(kvm_cpu_has_interrupt(vcpu)) to kvm_vcpu_ioctl_interrupt, and it actually triggered right before the guest got stuck.

This didn't trigger with qemu-kvm (and -no-kvm-irqchip) yet but, on the other hand, I currently do not see a potential bug in upstream's kvm_arch_pre_run. Could you have a look if you can reproduce, specifically if this isn't a KVM kernel issue in the end?

Quoted 6 lines

Quoted 3 lines

In kvm_cpu_exec() after calling kvm_arch_pre_run() env->exit_request is tested and function can exit without calling kvm_vcpu_ioctl(KVM_RUN). Can you check if this what happens in your case?

This path is executed quite frequently here. No obvious correlation with the lost IRQ.

Quoted 6 lines

If kvm_arch_pre_run() injected interrupt kvm_vcpu_ioctl(KVM_RUN) have to be executed before injecting another interrupt, so if on the fist call of kvm_cpu_exec() kvm_arch_pre_run() injected interrupt, but kvm_vcpu_ioctl(KVM_RUN) was not executed because of env->exit_request and on the next kvm_cpu_exec() other interrupt is injected the previous one will be lost.

...and kvm_run->ready_for_interrupt_injection is not updated either in that case, right? That makes be wonder if KVM_INTERRUPT shouldn't better return an error in case the queue is full already.

Quoted 6 lines

Quoted 3 lines

If kvm_vcpu_ioctl(KVM_RUN) is called, but exit happens before interrupt is injected kvm_run->ready_for_interrupt_injection should be update to reflect that fact.

Yes, but in this case it isn't called if IIUC. So that is the problem upstream KVM faces?

This is my guest. It tries to inject two different interrupt simultaneously and this is not supported (and not correct). It can be easily checked if you have reproducible case.

Quoted 3 lines

Then again: What do you think is the proper long-term fix? Only adjusting upstream KVM (required anyway) or also making the kernel support more robust against this pattern?

If my guest is correct no fix needed for KVM module (we can enhance API to return error as you suggested, but this will not fix buggy userspace). You are asking what do I think is the proper long-term fix then my answer is: merging qemu-kvm into qemu dropping whatever we have there currently ;)

Quoted 40 lines

Quoted 27 lines

Quoted 27 lines

Quoted 27 lines

Quoted 17 lines

Quoted 11 lines

Quoted 11 lines

Quoted 11 lines

with latest kernel modules, namely beginning with 6bc0a1a235 (Remove irq_pending bitmap), I'm loosing interrupts with upstream's KVM support. After some bisecting, hair-pulling and a bit meditation I added a WARN_ON(kvm_cpu_has_interrupt(vcpu)) to kvm_vcpu_ioctl_interrupt, and it actually triggered right before the guest got stuck.

This didn't trigger with qemu-kvm (and -no-kvm-irqchip) yet but, on the other hand, I currently do not see a potential bug in upstream's kvm_arch_pre_run. Could you have a look if you can reproduce, specifically if this isn't a KVM kernel issue in the end?

Quoted 6 lines

Quoted 3 lines

In kvm_cpu_exec() after calling kvm_arch_pre_run() env->exit_request is tested and function can exit without calling kvm_vcpu_ioctl(KVM_RUN). Can you check if this what happens in your case?

This path is executed quite frequently here. No obvious correlation with the lost IRQ.

Quoted 6 lines

If kvm_arch_pre_run() injected interrupt kvm_vcpu_ioctl(KVM_RUN) have to be executed before injecting another interrupt, so if on the fist call of kvm_cpu_exec() kvm_arch_pre_run() injected interrupt, but kvm_vcpu_ioctl(KVM_RUN) was not executed because of env->exit_request and on the next kvm_cpu_exec() other interrupt is injected the previous one will be lost.

...and kvm_run->ready_for_interrupt_injection is not updated either in that case, right? That makes be wonder if KVM_INTERRUPT shouldn't better return an error in case the queue is full already.

Quoted 6 lines

Quoted 3 lines

If kvm_vcpu_ioctl(KVM_RUN) is called, but exit happens before interrupt is injected kvm_run->ready_for_interrupt_injection should be update to reflect that fact.

Yes, but in this case it isn't called if IIUC. So that is the problem upstream KVM faces?

This is my guest. It tries to inject two different interrupt simultaneously and this is not supported (and not correct). It can be easily checked if you have reproducible case.

Quoted 3 lines

Then again: What do you think is the proper long-term fix? Only adjusting upstream KVM (required anyway) or also making the kernel support more robust against this pattern?

Quoted 5 lines

If my guest is correct no fix needed for KVM module (we can enhance API to return error as you suggested, but this will not fix buggy userspace). You are asking what do I think is the proper long-term fix then my answer is: merging qemu-kvm into qemu dropping whatever we have there currently ;)

As we won't merge libkvm's structure upstream, we won't see the same code structure in qemu one day that currently works (correctly) in qemu-kvm.

Quoted 45 lines · Jan Kiszka

Quoted 45 lines

Quoted 27 lines

Quoted 27 lines

Quoted 27 lines

Quoted 17 lines

Quoted 11 lines

Quoted 11 lines

Quoted 11 lines

with latest kernel modules, namely beginning with 6bc0a1a235 (Remove irq_pending bitmap), I'm loosing interrupts with upstream's KVM support. After some bisecting, hair-pulling and a bit meditation I added a WARN_ON(kvm_cpu_has_interrupt(vcpu)) to kvm_vcpu_ioctl_interrupt, and it actually triggered right before the guest got stuck.

This didn't trigger with qemu-kvm (and -no-kvm-irqchip) yet but, on the other hand, I currently do not see a potential bug in upstream's kvm_arch_pre_run. Could you have a look if you can reproduce, specifically if this isn't a KVM kernel issue in the end?

Quoted 6 lines

Quoted 3 lines

In kvm_cpu_exec() after calling kvm_arch_pre_run() env->exit_request is tested and function can exit without calling kvm_vcpu_ioctl(KVM_RUN). Can you check if this what happens in your case?

This path is executed quite frequently here. No obvious correlation with the lost IRQ.

Quoted 6 lines

If kvm_arch_pre_run() injected interrupt kvm_vcpu_ioctl(KVM_RUN) have to be executed before injecting another interrupt, so if on the fist call of kvm_cpu_exec() kvm_arch_pre_run() injected interrupt, but kvm_vcpu_ioctl(KVM_RUN) was not executed because of env->exit_request and on the next kvm_cpu_exec() other interrupt is injected the previous one will be lost.

...and kvm_run->ready_for_interrupt_injection is not updated either in that case, right? That makes be wonder if KVM_INTERRUPT shouldn't better return an error in case the queue is full already.

Quoted 6 lines

Quoted 3 lines

If kvm_vcpu_ioctl(KVM_RUN) is called, but exit happens before interrupt is injected kvm_run->ready_for_interrupt_injection should be update to reflect that fact.

Yes, but in this case it isn't called if IIUC. So that is the problem upstream KVM faces?

This is my guest. It tries to inject two different interrupt simultaneously and this is not supported (and not correct). It can be easily checked if you have reproducible case.

Quoted 3 lines

Then again: What do you think is the proper long-term fix? Only adjusting upstream KVM (required anyway) or also making the kernel support more robust against this pattern?

If my guest is correct no fix needed for KVM module (we can enhance API to return error as you suggested, but this will not fix buggy userspace). You are asking what do I think is the proper long-term fix then my answer is: merging qemu-kvm into qemu dropping whatever we have there currently ;)

Quoted 3 lines

As we won't merge libkvm's structure upstream, we won't see the same code structure in qemu one day that currently works (correctly) in qemu-kvm.

I hope we will merge it as close as realistically possible. And if the result is "not good enough" it will be morphed into "good enough" bit by bit using bisectable commits.