| Message ID | 1446466878-32837-2-git-send-email-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2015-11-02 13:21+0100, Paolo Bonzini: > We do not want to do too much work in atomic context, in particular > not walking all the VCPUs of the virtual machine. So we want > to distinguish the architecture-specific injection function for irqfd > from kvm_set_msi. Since it's still empty, reuse the newly added > kvm_arch_set_irq and rename it to kvm_arch_set_irq_inatomic. kvm/queue uses kvm_arch_set_irq since b7184313f4b9 ("kvm/x86: Hyper-V synthetic interrupt controller"). Is synic going to be dropped before this patch is merged? -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/11/2015 15:59, Radim Krcmar wrote: > > We do not want to do too much work in atomic context, in particular > > not walking all the VCPUs of the virtual machine. So we want > > to distinguish the architecture-specific injection function for irqfd > > from kvm_set_msi. Since it's still empty, reuse the newly added > > kvm_arch_set_irq and rename it to kvm_arch_set_irq_inatomic. > > kvm/queue uses kvm_arch_set_irq since b7184313f4b9 ("kvm/x86: Hyper-V > synthetic interrupt controller"). > > Is synic going to be dropped before this patch is merged? Yes. Both because the Virtuozzo people confirmed that kvm_arch_set_irq isn't needed for synic, and because synic is currently broken with APICv. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2015-11-02 17:08+0100, Paolo Bonzini: > On 02/11/2015 15:59, Radim Krcmar wrote: >>> We do not want to do too much work in atomic context, in particular >>> not walking all the VCPUs of the virtual machine. So we want >>> to distinguish the architecture-specific injection function for irqfd >>> from kvm_set_msi. Since it's still empty, reuse the newly added >>> kvm_arch_set_irq and rename it to kvm_arch_set_irq_inatomic. >> >> kvm/queue uses kvm_arch_set_irq since b7184313f4b9 ("kvm/x86: Hyper-V >> synthetic interrupt controller"). >> >> Is synic going to be dropped before this patch is merged? > > Yes. Both because the Virtuozzo people confirmed that kvm_arch_set_irq > isn't needed for synic, and because synic is currently broken with APICv. Thanks. (We can add direct delivery for |online vcpus| < X if performance with low number of VCPUs happens to regress because of the schedule_work,) Reviewed-by: Radim Kr?má? <rkrcmar@redhat.com> [2/3] and [3/3] are good too. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/11/2015 18:01, Radim Krcmar wrote: >> > Yes. Both because the Virtuozzo people confirmed that kvm_arch_set_irq >> > isn't needed for synic, and because synic is currently broken with APICv. > Thanks. > > (We can add direct delivery for |online vcpus| < X if performance with > low number of VCPUs happens to regress because of the schedule_work,) Yeah, we will change the VFIO interrupt handler to non-threaded, which will do more or less the same (what you lose now through schedule_work, you will recoup by doing things directly in the ISR). Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c index 8f4499c7ffc1..26d830c1b7af 100644 --- a/arch/x86/kvm/irq_comm.c +++ b/arch/x86/kvm/irq_comm.c @@ -124,12 +124,16 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e, } -static int kvm_set_msi_inatomic(struct kvm_kernel_irq_routing_entry *e, - struct kvm *kvm) +int kvm_arch_set_irq_inatomic(struct kvm_kernel_irq_routing_entry *e, + struct kvm *kvm, int irq_source_id, int level, + bool line_status) { struct kvm_lapic_irq irq; int r; + if (unlikely(e->type != KVM_IRQ_ROUTING_MSI)) + return -EWOULDBLOCK; + kvm_set_msi_irq(e, &irq); if (kvm_irq_delivery_to_apic_fast(kvm, NULL, &irq, &r, NULL)) @@ -165,10 +169,8 @@ int kvm_set_irq_inatomic(struct kvm *kvm, int irq_source_id, u32 irq, int level) idx = srcu_read_lock(&kvm->irq_srcu); if (kvm_irq_map_gsi(kvm, entries, irq) > 0) { e = &entries[0]; - if (likely(e->type == KVM_IRQ_ROUTING_MSI)) - ret = kvm_set_msi_inatomic(e, kvm); - else - ret = -EWOULDBLOCK; + ret = kvm_arch_set_irq_inatomic(e, kvm, irq_source_id, + irq, level); } srcu_read_unlock(&kvm->irq_srcu, idx); return ret; diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index eba9caebc9c1..42fb9e089fc9 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -828,10 +828,9 @@ int kvm_set_irq(struct kvm *kvm, int irq_source_id, u32 irq, int level, int kvm_set_irq_inatomic(struct kvm *kvm, int irq_source_id, u32 irq, int level); int kvm_set_msi(struct kvm_kernel_irq_routing_entry *irq_entry, struct kvm *kvm, int irq_source_id, int level, bool line_status); - -int kvm_arch_set_irq(struct kvm_kernel_irq_routing_entry *irq, struct kvm *kvm, - int irq_source_id, int level, bool line_status); - +int kvm_arch_set_irq_inatomic(struct kvm_kernel_irq_routing_entry *e, + struct kvm *kvm, int irq_source_id, + int level, bool line_status); bool kvm_irq_has_notifier(struct kvm *kvm, unsigned irqchip, unsigned pin); void kvm_notify_acked_gsi(struct kvm *kvm, int gsi); void kvm_notify_acked_irq(struct kvm *kvm, unsigned irqchip, unsigned pin); diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c index e29fd2640709..46dbc0a7dfc1 100644 --- a/virt/kvm/eventfd.c +++ b/virt/kvm/eventfd.c @@ -171,7 +171,7 @@ irqfd_deactivate(struct kvm_kernel_irqfd *irqfd) queue_work(irqfd_cleanup_wq, &irqfd->shutdown); } -int __attribute__((weak)) kvm_arch_set_irq( +int __attribute__((weak)) kvm_arch_set_irq_inatomic( struct kvm_kernel_irq_routing_entry *irq, struct kvm *kvm, int irq_source_id, int level, @@ -201,12 +201,9 @@ irqfd_wakeup(wait_queue_t *wait, unsigned mode, int sync, void *key) irq = irqfd->irq_entry; } while (read_seqcount_retry(&irqfd->irq_entry_sc, seq)); /* An event has been signaled, inject an interrupt */ - if (irq.type == KVM_IRQ_ROUTING_MSI) - kvm_set_msi(&irq, kvm, KVM_USERSPACE_IRQ_SOURCE_ID, 1, - false); - else if (kvm_arch_set_irq(&irq, kvm, - KVM_USERSPACE_IRQ_SOURCE_ID, 1, - false) == -EWOULDBLOCK) + if (kvm_arch_set_irq_inatomic(&irq, kvm, + KVM_USERSPACE_IRQ_SOURCE_ID, 1, + false) == -EWOULDBLOCK) schedule_work(&irqfd->inject); srcu_read_unlock(&kvm->irq_srcu, idx); }
We do not want to do too much work in atomic context, in particular not walking all the VCPUs of the virtual machine. So we want to distinguish the architecture-specific injection function for irqfd from kvm_set_msi. Since it's still empty, reuse the newly added kvm_arch_set_irq and rename it to kvm_arch_set_irq_inatomic. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- arch/x86/kvm/irq_comm.c | 14 ++++++++------ include/linux/kvm_host.h | 7 +++---- virt/kvm/eventfd.c | 11 ++++------- 3 files changed, 15 insertions(+), 17 deletions(-)