On 05/05/19 10:56, Peter Xu wrote:
> 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.
>
> CC: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> CC: Radim Krčmář <rkrcmar@xxxxxxxxxx>
> CC: Alex Williamson <alex.williamson@xxxxxxxxxx>
> CC: Eduardo Habkost <ehabkost@xxxxxxxxxx>
> Signed-off-by: Peter Xu <peterx@xxxxxxxxxx>
> ---
> arch/x86/kvm/irq.c | 7 +++++++
> arch/x86/kvm/irq.h | 1 +
> virt/kvm/eventfd.c | 9 +++++++++
> 3 files changed, 17 insertions(+)
>
> diff --git a/arch/x86/kvm/irq.c b/arch/x86/kvm/irq.c
> index faa264822cee..007bc654f928 100644
> --- a/arch/x86/kvm/irq.c
> +++ b/arch/x86/kvm/irq.c
> @@ -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);
> +}
> diff --git a/arch/x86/kvm/irq.h b/arch/x86/kvm/irq.h
> index d5005cc26521..fd210cdd4983 100644
> --- a/arch/x86/kvm/irq.h
> +++ b/arch/x86/kvm/irq.h
> @@ -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);
> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> index 001aeda4c154..3972a9564c76 100644
> --- a/virt/kvm/eventfd.c
> +++ b/virt/kvm/eventfd.c
> @@ -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;
>
Queued, thanks.
Paolo
