kvm: Check irqchip mode before assign irqfd
PATCH 1/1 kvm: Check irqchip mode before assign irqfd
When assigning kvm irqfd we didn't check the irqchip mode but we allow KVM_IRQFD to succeed with all the irqchip modes. However it does not make much sense to create irqfd even without the kernel chips. Let's provide a arch-dependent helper to check whether a specific irqfd is allowed by the arch. At least for x86, it should make sense to check:
- when irqchip mode is NONE, all irqfds should be disallowed, and,
- when irqchip mode is SPLIT, irqfds that are with resamplefd should be disallowed.
For either of the case, previously we'll silently ignore the irq or the irq ack event if the irqchip mode is incorrect. However that can cause misterious guest behaviors and it can be hard to triage. Let's fail KVM_IRQFD even earlier to detect these incorrect configurations.
--- arch/x86/kvm/irq.c | 7 +++++++ arch/x86/kvm/irq.h | 1 + virt/kvm/eventfd.c | 9 +++++++++ 3 files changed, 17 insertions(+)
arch/x86/kvm/irq.c +7 -0
@@ -172,3 +172,10 @@ void __kvm_migrate_timers(struct kvm_vcpu *vcpu)__kvm_migrate_apic_timer (vcpu);__kvm_migrate_pit_timer (vcpu); } + +bool kvm_arch_irqfd_allowed (struct kvm *kvm,struct kvm_irqfd *args) +{ +bool resample = args->flags &KVM_IRQFD_FLAG_RESAMPLE ; + +return resample ?irqchip_kernel (kvm) :irqchip_in_kernel (kvm); +}
arch/x86/kvm/irq.h +1 -0
@@ -114,6 +114,7 @@ static inline int irqchip_in_kernel(struct kvm *kvm)return mode !=KVM_IRQCHIP_NONE ; } +bool kvm_arch_irqfd_allowed (struct kvm *kvm,struct kvm_irqfd *args);void kvm_inject_pending_timer_irqs (struct kvm_vcpu *vcpu);void kvm_inject_apic_timer_irqs (struct kvm_vcpu *vcpu);void kvm_apic_nmi_wd_deliver (struct kvm_vcpu *vcpu);
virt/kvm/eventfd.c +9 -0
@@ -44,6 +44,12 @@static struct workqueue_struct *irqfd_cleanup_wq; +bool __attribute__((weak)) +kvm_arch_irqfd_allowed (struct kvm *kvm,struct kvm_irqfd *args) +{ +return true; +} +static void irqfd_inject (struct work_struct *work) {
@@ -297,6 +303,9 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args)if (!kvm_arch_intc_initialized (kvm))return -EAGAIN ; +if (!kvm_arch_irqfd_allowed (kvm, args)) +return -EINVAL ; + irqfd =kzalloc (sizeof (*irqfd),GFP_KERNEL_ACCOUNT );if (!irqfd)return -ENOMEM ;
Signature · 1 line
2.17.1
Quoted 16 lines · Peter Xu
When assigning kvm irqfd we didn't check the irqchip mode but we allow
KVM_IRQFDto succeed with all the irqchip modes. However it does not make much sense to create irqfd even without the kernel chips. Let's provide a arch-dependent helper to check whether a specific irqfd is allowed by the arch. At least for x86, it should make sense to check:- when irqchip mode is NONE, all irqfds should be disallowed, and,
- when irqchip mode is SPLIT, irqfds that are with resamplefd should be disallowed.
For either of the case, previously we'll silently ignore the irq or the irq ack event if the irqchip mode is incorrect. However that can cause misterious guest behaviors and it can be hard to triage. Let's fail
KVM_IRQFDeven earlier to detect these incorrect configurations.
Quoted 5 lines
CC Paolo Bonzini <[email protected]>CC Radim Krčmář <[email protected]>CC Alex Williamson <[email protected]>CC Eduardo Habkost <[email protected]>Signed-off-by Peter Xu <[email protected]>
Note: haven't tested, but IIUC QEMU's vfio will naturally fall back to no-irqfd mode (actually virtio seems to also have this but virtio should not be affected after all) if the KVM_IRQFD ioctl failed so I feel like this patch could also at least fix the broken guests reported besides any future fixes from QEMU side on the issue:
https://bugs.launchpad.net/qemu/+bug/1826422
Thanks,
Signature · 1 line
Peter Xu
Quoted 23 lines · Peter Xu
Quoted 16 lines · Peter Xu
When assigning kvm irqfd we didn't check the irqchip mode but we allow
KVM_IRQFDto succeed with all the irqchip modes. However it does not make much sense to create irqfd even without the kernel chips. Let's provide a arch-dependent helper to check whether a specific irqfd is allowed by the arch. At least for x86, it should make sense to check:- when irqchip mode is NONE, all irqfds should be disallowed, and,
- when irqchip mode is SPLIT, irqfds that are with resamplefd should be disallowed.
For either of the case, previously we'll silently ignore the irq or the irq ack event if the irqchip mode is incorrect. However that can cause misterious guest behaviors and it can be hard to triage. Let's fail
KVM_IRQFDeven earlier to detect these incorrect configurations.Quoted 5 lines
CC Paolo Bonzini <[email protected]>CC Radim Krčmář <[email protected]>CC Alex Williamson <[email protected]>CC Eduardo Habkost <[email protected]>Signed-off-by Peter Xu <[email protected]>
Quoted 7 lines
Note: haven't tested, but IIUC QEMU's vfio will naturally fall back to no-irqfd mode (actually virtio seems to also have this but virtio should not be affected after all) if the
KVM_IRQFDioctl failed so I feel like this patch could also at least fix the broken guests reported besides any future fixes from QEMU side on the issue:
Ping - Paolo, should patch still worth to have before the complete fix between split irqchip & resamplefds?
Thanks,
Signature · 1 line
Peter Xu
Quoted 73 lines · Peter Xu
When assigning kvm irqfd we didn't check the irqchip mode but we allow
KVM_IRQFDto succeed with all the irqchip modes. However it does not make much sense to create irqfd even without the kernel chips. Let's provide a arch-dependent helper to check whether a specific irqfd is allowed by the arch. At least for x86, it should make sense to check:- when irqchip mode is NONE, all irqfds should be disallowed, and,
- when irqchip mode is SPLIT, irqfds that are with resamplefd should be disallowed.
For either of the case, previously we'll silently ignore the irq or the irq ack event if the irqchip mode is incorrect. However that can cause misterious guest behaviors and it can be hard to triage. Let's fail
KVM_IRQFDeven earlier to detect these incorrect configurations.CC Paolo Bonzini <[email protected]>CC Radim Krčmář <[email protected]>CC Alex Williamson <[email protected]>CC Eduardo Habkost <[email protected]>Signed-off-by Peter Xu <[email protected]>--- arch/x86/kvm/irq.c | 7 +++++++ arch/x86/kvm/irq.h | 1 + virt/kvm/eventfd.c | 9 +++++++++ 3 files changed, 17 insertions(+)arch/x86/kvm/irq.c +7
-0@@ -172,3 +172,10 @@ void __kvm_migrate_timers(struct kvm_vcpu *vcpu)__kvm_migrate_apic_timer (vcpu);__kvm_migrate_pit_timer (vcpu); } + +bool kvm_arch_irqfd_allowed (struct kvm *kvm,struct kvm_irqfd *args) +{ +bool resample = args->flags &KVM_IRQFD_FLAG_RESAMPLE ; + +return resample ?irqchip_kernel (kvm) :irqchip_in_kernel (kvm); +}arch/x86/kvm/irq.h +1
-0@@ -114,6 +114,7 @@ static inline int irqchip_in_kernel(struct kvm *kvm)return mode !=KVM_IRQCHIP_NONE ; } +bool kvm_arch_irqfd_allowed (struct kvm *kvm,struct kvm_irqfd *args);void kvm_inject_pending_timer_irqs (struct kvm_vcpu *vcpu);void kvm_inject_apic_timer_irqs (struct kvm_vcpu *vcpu);void kvm_apic_nmi_wd_deliver (struct kvm_vcpu *vcpu);virt/kvm/eventfd.c +6
-0@@ -44,6 +44,12 @@static struct workqueue_struct *irqfd_cleanup_wq; +bool __attribute__((weak)) +kvm_arch_irqfd_allowed (struct kvm *kvm,struct kvm_irqfd *args) +{ +return true; +} +static void irqfd_inject (struct work_struct *work) {
virt/kvm/eventfd.c kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args) · line 311
Quoted 9 lines
virt/kvm/eventfd.c +3
-0@@ -297,6 +303,9 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args)if (!kvm_arch_intc_initialized (kvm))return -EAGAIN ; +if (!kvm_arch_irqfd_allowed (kvm, args)) +return -EINVAL ; + irqfd =kzalloc (sizeof (*irqfd),GFP_KERNEL_ACCOUNT );if (!irqfd)return -ENOMEM ;
Queued, thanks.
Paolo