[PATCHv2 RFC] kvm: deliver msix interrupts from irq handler

[PATCHv2 RFC] kvm: deliver msix interrupts from irq handler

[Michael S. Tsirkin]

We can deliver certain interrupts, notably MSIX,
from atomic context.  Add a new API kvm_set_irq_inatomic,
that does exactly that, and use it to implement
an irq handler for msi.

This reduces the pressure on scheduler in case
where host and guest irq share a host cpu.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Untested.
Changes from v1:
Tried to address comments from v1.
Not all suggestions from v1 were addressed yet, but run
out of time and weekend is starting.
Posting so Jan can test if he likes.
---
 include/linux/kvm_host.h |  1 +
 virt/kvm/assigned-dev.c  | 31 +++++++++++++++++++++++++++++--
 virt/kvm/irq_comm.c      | 38 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index c446435..908e1ac 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -616,6 +616,7 @@ void kvm_get_intr_delivery_bitmask(struct kvm_ioapic *ioapic,
 				   unsigned long *deliver_bitmask);
 #endif
 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);
 void kvm_notify_acked_irq(struct kvm *kvm, unsigned irqchip, unsigned pin);
diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c
index 01f572c..c5c332c 100644
--- a/virt/kvm/assigned-dev.c
+++ b/virt/kvm/assigned-dev.c
@@ -117,6 +117,23 @@ static irqreturn_t kvm_assigned_dev_thread_msi(int irq, void *dev_id)
 #endif
 
 #ifdef __KVM_HAVE_MSIX
+static irqreturn_t kvm_assigned_dev_msix(int irq, void *dev_id)
+{
+	struct kvm_assigned_dev_kernel *assigned_dev = dev_id;
+	int index = find_index_from_host_irq(assigned_dev, irq);
+	u32 vector;
+	int ret = 0;
+
+	if (index >= 0) {
+		vector = assigned_dev->guest_msix_entries[index].vector;
+		ret = kvm_set_irq_inatomic(assigned_dev->kvm,
+					   assigned_dev->irq_source_id,
+					   vector, 1);
+	}
+
+	return unlikely(ret == -EWOULDBLOCK) ? IRQ_WAKE_THREAD : IRQ_HANDLED;
+}
+
 static irqreturn_t kvm_assigned_dev_thread_msix(int irq, void *dev_id)
 {
 	struct kvm_assigned_dev_kernel *assigned_dev = dev_id;
@@ -334,6 +351,15 @@ static int assigned_device_enable_host_intx(struct kvm *kvm,
 }
 
 #ifdef __KVM_HAVE_MSI
+static irqreturn_t kvm_assigned_dev_msi(int irq, void *dev_id)
+{
+	struct kvm_assigned_dev_kernel *assigned_dev = dev_id;
+	int ret = kvm_set_irq_inatomic(assigned_dev->kvm,
+				       assigned_dev->irq_source_id,
+				       assigned_dev->guest_irq, 1);
+	return unlikely(ret == -EWOULDBLOCK) ? IRQ_WAKE_THREAD : IRQ_HANDLED;
+}
+
 static int assigned_device_enable_host_msi(struct kvm *kvm,
 					   struct kvm_assigned_dev_kernel *dev)
 {
@@ -346,7 +372,7 @@ static int assigned_device_enable_host_msi(struct kvm *kvm,
 	}
 
 	dev->host_irq = dev->dev->irq;
-	if (request_threaded_irq(dev->host_irq, NULL,
+	if (request_threaded_irq(dev->host_irq, kvm_assigned_dev_msi,
 				 kvm_assigned_dev_thread_msi, 0,
 				 dev->irq_name, dev)) {
 		pci_disable_msi(dev->dev);
@@ -374,7 +400,8 @@ static int assigned_device_enable_host_msix(struct kvm *kvm,
 
 	for (i = 0; i < dev->entries_nr; i++) {
 		r = request_threaded_irq(dev->host_msix_entries[i].vector,
-					 NULL, kvm_assigned_dev_thread_msix,
+					 kvm_assigned_dev_msix,
+					 kvm_assigned_dev_thread_msix,
 					 0, dev->irq_name, dev);
 		if (r)
 			goto err;
diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
index a6a0365..7c73bb4 100644
--- a/virt/kvm/irq_comm.c
+++ b/virt/kvm/irq_comm.c
@@ -190,6 +190,44 @@ int kvm_set_irq(struct kvm *kvm, int irq_source_id, u32 irq, int level)
 	return ret;
 }
 
+/*
+ * Deliver an IRQ in an atomic context if we can, or return a failure,
+ * user can retry in a process context.
+ * Return value:
+ *  -EWOULDBLOCK - Can't deliver in atomic context: retry in a process context.
+ *  Other values - No need to retry.
+ */
+int kvm_set_irq_inatomic(struct kvm *kvm, int irq_source_id, u32 irq, int level)
+{
+	struct kvm_kernel_irq_routing_entry *e;
+	int ret = -EINVAL;
+	struct kvm_irq_routing_table *irq_rt;
+	struct hlist_node *n;
+
+	trace_kvm_set_irq(irq, level, irq_source_id);
+
+	/*
+	 * We know MSI are safe in interrupt context. They are also
+	 * easy as there's a single routing entry for these GSIs.
+	 * So only handle MSI in an atomic context, for now.
+	 *
+	 * This shares some code with kvm_set_irq: this
+	 * version is optimized for a single entry MSI only case.
+	 */
+	rcu_read_lock();
+	irq_rt = rcu_dereference(kvm->irq_routing);
+	if (irq < irq_rt->nr_rt_entries)
+		hlist_for_each_entry(e, n, &irq_rt->map[irq], link) {
+			if (likely(e->type == KVM_IRQ_ROUTING_MSI))
+				ret = kvm_set_msi(e, kvm, irq_source_id, level);
+			else
+				ret = -EWOULDBLOCK;
+			break;
+		}
+	rcu_read_unlock();
+	return ret;
+}
+
 void kvm_notify_acked_irq(struct kvm *kvm, unsigned irqchip, unsigned pin)
 {
 	struct kvm_irq_ack_notifier *kian;
-- 
MST

Re: [PATCHv2 RFC] kvm: deliver msix interrupts from irq handler

[Avi Kivity]

On 06/08/2012 07:01 PM, Michael S. Tsirkin wrote:
> We can deliver certain interrupts, notably MSIX,
> from atomic context.  Add a new API kvm_set_irq_inatomic,
> that does exactly that, and use it to implement
> an irq handler for msi.
> 
> This reduces the pressure on scheduler in case
> where host and guest irq share a host cpu.
> 

Looks nice.

>  
> +/*
> + * Deliver an IRQ in an atomic context if we can, or return a failure,
> + * user can retry in a process context.
> + * Return value:
> + *  -EWOULDBLOCK - Can't deliver in atomic context: retry in a process context.
> + *  Other values - No need to retry.
> + */
> +int kvm_set_irq_inatomic(struct kvm *kvm, int irq_source_id, u32 irq, int level)
> +{
> +	struct kvm_kernel_irq_routing_entry *e;
> +	int ret = -EINVAL;
> +	struct kvm_irq_routing_table *irq_rt;
> +	struct hlist_node *n;
> +
> +	trace_kvm_set_irq(irq, level, irq_source_id);
> +
> +	/*
> +	 * We know MSI are safe in interrupt context. They are also
> +	 * easy as there's a single routing entry for these GSIs.
> +	 * So only handle MSI in an atomic context, for now.
> +	 *
> +	 * This shares some code with kvm_set_irq: this
> +	 * version is optimized for a single entry MSI only case.
> +	 */
> +	rcu_read_lock();
> +	irq_rt = rcu_dereference(kvm->irq_routing);
> +	if (irq < irq_rt->nr_rt_entries)
> +		hlist_for_each_entry(e, n, &irq_rt->map[irq], link) {
> +			if (likely(e->type == KVM_IRQ_ROUTING_MSI))
> +				ret = kvm_set_msi(e, kvm, irq_source_id, level);
> +			else
> +				ret = -EWOULDBLOCK;
> +			break;
> +		}
> +	rcu_read_unlock();
> +	return ret;
> +}
> +

kvm_set_msi() contains a loop over all vcpus to match the APIC ID (or to
broadcast).  I'd rather see a cache that contains the vcpu pointer and
vector number (assumes delivery mode of fixed and not the other junk),
if the cache is filled fire away, otherwise slow path to a thread and
fill the cache if possible.

-- 
error compiling committee.c: too many arguments to function