Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1260593 > unrolled thread

[PATCH 1/3] KVM: x86: merge kvm_arch_set_irq with kvm_set_msi_inatomic

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 1/3] KVM: x86: merge kvm_arch_set_irq with kvm_set_msi_inatomic Paolo Bonzini <pbonzini@redhat.com> - 2015-11-02 13:30 +0100
    Re: [PATCH 1/3] KVM: x86: merge kvm_arch_set_irq with  kvm_set_msi_inatomic Radim Krcmar <rkrcmar@redhat.com> - 2015-11-02 16:00 +0100
      Re: [PATCH 1/3] KVM: x86: merge kvm_arch_set_irq with  kvm_set_msi_inatomic Paolo Bonzini <pbonzini@redhat.com> - 2015-11-02 17:10 +0100
        Re: [PATCH 1/3] KVM: x86: merge kvm_arch_set_irq with  kvm_set_msi_inatomic Radim Krcmar <rkrcmar@redhat.com> - 2015-11-02 18:10 +0100
          Re: [PATCH 1/3] KVM: x86: merge kvm_arch_set_irq with  kvm_set_msi_inatomic Paolo Bonzini <pbonzini@redhat.com> - 2015-11-02 18:10 +0100

#1260593 — [PATCH 1/3] KVM: x86: merge kvm_arch_set_irq with kvm_set_msi_inatomic

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(-)

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);
 	}
-- 
1.8.3.1


--
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/

[toc] | [next] | [standalone]

#1260698 — Re: [PATCH 1/3] KVM: x86: merge kvm_arch_set_irq with kvm_set_msi_inatomic

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 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/

[toc] | [prev] | [next] | [standalone]

#1260753 — Re: [PATCH 1/3] KVM: x86: merge kvm_arch_set_irq with kvm_set_msi_inatomic

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 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/

[toc] | [prev] | [next] | [standalone]

#1260797 — Re: [PATCH 1/3] KVM: x86: merge kvm_arch_set_irq with kvm_set_msi_inatomic

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 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/

[toc] | [prev] | [next] | [standalone]

#1260800 — Re: [PATCH 1/3] KVM: x86: merge kvm_arch_set_irq with kvm_set_msi_inatomic

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 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/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web