Message-ID: <E959C4978C3B6342920538CF579893F002677105@SHSMSX104.ccr.corp.intel.com>
Date:	Thu, 6 Aug 2015 02:12:26 +0000
From:	"Wu, Feng" <feng.wu@...el.com>
To:	Alex Williamson <alex.williamson@...hat.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>
CC:	"eric.auger@...com" <eric.auger@...com>,
	"eric.auger@...aro.org" <eric.auger@...aro.org>,
	"joro@...tes.org" <joro@...tes.org>,
	"avi.kivity@...il.com" <avi.kivity@...il.com>,
	"pbonzini@...hat.com" <pbonzini@...hat.com>,
	"Wu, Feng" <feng.wu@...el.com>
Subject: RE: [PATCH v3] virt: IRQ bypass manager



> -----Original Message-----
> From: Alex Williamson [mailto:alex.williamson@...hat.com]
> Sent: Thursday, August 06, 2015 6:08 AM
> To: linux-kernel@...r.kernel.org; kvm@...r.kernel.org
> Cc: eric.auger@...com; eric.auger@...aro.org; joro@...tes.org;
> avi.kivity@...il.com; pbonzini@...hat.com; Wu, Feng
> Subject: [PATCH v3] virt: IRQ bypass manager
> 
> When a physical I/O device is assigned to a virtual machine through
> facilities like VFIO and KVM, the interrupt for the device generally
> bounces through the host system before being injected into the VM.
> However, hardware technologies exist that often allow the host to be
> bypassed for some of these scenarios.  Intel Posted Interrupts allow
> the specified physical edge interrupts to be directly injected into a
> guest when delivered to a physical processor while the vCPU is
> running.  ARM IRQ Forwarding allows forwarded physical interrupts to
> be directly deactivated by the guest.
> 
> The IRQ bypass manager here is meant to provide the shim to connect
> interrupt producers, generally the host physical device driver, with
> interrupt consumers, generally the hypervisor, in order to configure
> these bypass mechanism.  To do this, we base the connection on a
> shared, opaque token.  For KVM-VFIO this is expected to be an
> eventfd_ctx since this is the connection we already use to connect an
> eventfd to an irqfd on the in-kernel path.  When a producer and
> consumer with matching tokens is found, callbacks via both registered
> participants allow the bypass facilities to be automatically enabled.
> 
> Signed-off-by: Alex Williamson <alex.williamson@...hat.com>
> ---
> 
> v3: Fix list_for_each_entry issue noted by Eric
> 
> Do we want to revisit whether add and del are required callbacks?
> Maybe they should only be required for the consumer?  If Feng already
> doesn't require them for Intel PI, let's not impose stub callback
> requirements.

Since both irq_bypass_register_producer() and irq_bypass_register_consumer()
call __connect(), which then calls add_consumer() and add_producer(), only providing
the add/del callbacks for 'struct irq_bypass_consumer' is fine to me. Thanks!

BTW, could you please have a look at the common part patches v4 sent by Eric:
[PATCH v4 0/5] KVM: irqfd consumer based on IRQ bypass manager

If it looks fine to your guys, we can continue for the arch specific work, thank you!

Thanks,
Feng

> 
>  MAINTAINERS               |    7 +
>  include/linux/irqbypass.h |   90 ++++++++++++++++
>  virt/lib/Kconfig          |    2
>  virt/lib/Makefile         |    1
>  virt/lib/irqbypass.c      |  256
> +++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 356 insertions(+)
>  create mode 100644 include/linux/irqbypass.h
>  create mode 100644 virt/lib/Kconfig
>  create mode 100644 virt/lib/Makefile
>  create mode 100644 virt/lib/irqbypass.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a9ae6c1..10c8b2f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10963,6 +10963,13 @@ L:	netdev@...r.kernel.org
>  S:	Maintained
>  F:	drivers/net/ethernet/via/via-velocity.*
> 
> +VIRT LIB
> +M:	Alex Williamson <alex.williamson@...hat.com>
> +M:	Paolo Bonzini <pbonzini@...hat.com>
> +L:	kvm@...r.kernel.org
> +S:	Supported
> +F:	virt/lib/
> +
>  VIVID VIRTUAL VIDEO DRIVER
>  M:	Hans Verkuil <hverkuil@...all.nl>
>  L:	linux-media@...r.kernel.org
> diff --git a/include/linux/irqbypass.h b/include/linux/irqbypass.h
> new file mode 100644
> index 0000000..fde7b64
> --- /dev/null
> +++ b/include/linux/irqbypass.h
> @@ -0,0 +1,90 @@
> +/*
> + * IRQ offload/bypass manager
> + *
> + * Copyright (C) 2015 Red Hat, Inc.
> + * Copyright (c) 2015 Linaro Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#ifndef IRQBYPASS_H
> +#define IRQBYPASS_H
> +
> +#include <linux/list.h>
> +
> +struct irq_bypass_consumer;
> +
> +/*
> + * Theory of operation
> + *
> + * The IRQ bypass manager is a simple set of lists and callbacks that allows
> + * IRQ producers (ex. physical interrupt sources) to be matched to IRQ
> + * consumers (ex. virtualization hardware that allows IRQ bypass or offload)
> + * via a shared token (ex. eventfd_ctx).  Producers and consumers register
> + * independently.  When a token match is found, the optional @stop callback
> + * will be called for each participant.  The pair will then be connected via
> + * the @add_* callbacks, and finally the optional @start callback will allow
> + * any final coordination.  When either participant is unregistered, the
> + * process is repeated using the @del_* callbacks in place of the @add_*
> + * callbacks.  Match tokens must be unique per producer/consumer, 1:N
> pairings
> + * are not supported.
> + */
> +
> +/**
> + * struct irq_bypass_producer - IRQ bypass producer definition
> + * @node: IRQ bypass manager private list management
> + * @token: opaque token to match between producer and consumer
> + * @irq: Linux IRQ number for the producer device
> + * @add_consumer: Connect the IRQ producer to an IRQ consumer
> + * @del_consumer: Disconnect the IRQ producer from an IRQ consumer
> + * @stop: Perform any quiesce operations necessary prior to add/del
> (optional)
> + * @start: Perform any startup operations necessary after add/del (optional)
> + *
> + * The IRQ bypass producer structure represents an interrupt source for
> + * participation in possible host bypass, for instance an interrupt vector
> + * for a physical device assigned to a VM.
> + */
> +struct irq_bypass_producer {
> +	struct list_head node;
> +	void *token;
> +	int irq;
> +	int (*add_consumer)(struct irq_bypass_producer *,
> +			    struct irq_bypass_consumer *);
> +	void (*del_consumer)(struct irq_bypass_producer *,
> +			     struct irq_bypass_consumer *);
> +	void (*stop)(struct irq_bypass_producer *);
> +	void (*start)(struct irq_bypass_producer *);
> +};
> +
> +/**
> + * struct irq_bypass_consumer - IRQ bypass consumer definition
> + * @node: IRQ bypass manager private list management
> + * @token: opaque token to match between producer and consumer
> + * @add_producer: Connect the IRQ consumer to an IRQ producer
> + * @del_producer: Disconnect the IRQ consumer from an IRQ producer
> + * @stop: Perform any quiesce operations necessary prior to add/del
> (optional)
> + * @start: Perform any startup operations necessary after add/del (optional)
> + *
> + * The IRQ bypass consumer structure represents an interrupt sink for
> + * participation in possible host bypass, for instance a hypervisor may
> + * support offloads to allow bypassing the host entirely or offload
> + * portions of the interrupt handling to the VM.
> + */
> +struct irq_bypass_consumer {
> +	struct list_head node;
> +	void *token;
> +	int (*add_producer)(struct irq_bypass_consumer *,
> +			    struct irq_bypass_producer *);
> +	void (*del_producer)(struct irq_bypass_consumer *,
> +			     struct irq_bypass_producer *);
> +	void (*stop)(struct irq_bypass_consumer *);
> +	void (*start)(struct irq_bypass_consumer *);
> +};
> +
> +int irq_bypass_register_producer(struct irq_bypass_producer *);
> +void irq_bypass_unregister_producer(struct irq_bypass_producer *);
> +int irq_bypass_register_consumer(struct irq_bypass_consumer *);
> +void irq_bypass_unregister_consumer(struct irq_bypass_consumer *);
> +
> +#endif /* IRQBYPASS_H */
> diff --git a/virt/lib/Kconfig b/virt/lib/Kconfig
> new file mode 100644
> index 0000000..89a414f
> --- /dev/null
> +++ b/virt/lib/Kconfig
> @@ -0,0 +1,2 @@
> +config IRQ_BYPASS_MANAGER
> +	tristate
> diff --git a/virt/lib/Makefile b/virt/lib/Makefile
> new file mode 100644
> index 0000000..901228d
> --- /dev/null
> +++ b/virt/lib/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_IRQ_BYPASS_MANAGER) += irqbypass.o
> diff --git a/virt/lib/irqbypass.c b/virt/lib/irqbypass.c
> new file mode 100644
> index 0000000..98a4141
> --- /dev/null
> +++ b/virt/lib/irqbypass.c
> @@ -0,0 +1,256 @@
> +/*
> + * IRQ offload/bypass manager
> + *
> + * Copyright (C) 2015 Red Hat, Inc.
> + * Copyright (c) 2015 Linaro Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * Various virtualization hardware acceleration techniques allow bypassing or
> + * offloading interrupts received from devices around the host kernel.
> Posted
> + * Interrupts on Intel VT-d systems can allow interrupts to be received
> + * directly by a virtual machine.  ARM IRQ Forwarding allows forwarded
> physical
> + * interrupts to be directly deactivated by the guest.  This manager allows
> + * interrupt producers and consumers to find each other to enable this sort of
> + * bypass.
> + */
> +
> +#include <linux/irqbypass.h>
> +#include <linux/list.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("IRQ bypass manager utility module");
> +
> +static LIST_HEAD(producers);
> +static LIST_HEAD(consumers);
> +static DEFINE_MUTEX(lock);
> +
> +/* @lock must be held when calling connect */
> +static int __connect(struct irq_bypass_producer *prod,
> +		     struct irq_bypass_consumer *cons)
> +{
> +	int ret;
> +
> +	if (prod->stop)
> +		prod->stop(prod);
> +	if (cons->stop)
> +		cons->stop(cons);
> +
> +	ret = prod->add_consumer(prod, cons);
> +	if (!ret) {
> +		ret = cons->add_producer(cons, prod);
> +		if (ret)
> +			prod->del_consumer(prod, cons);
> +	}
> +
> +	if (cons->start)
> +		cons->start(cons);
> +	if (prod->start)
> +		prod->start(prod);
> +
> +	return ret;
> +}
> +
> +/* @lock must be held when calling disconnect */
> +static void __disconnect(struct irq_bypass_producer *prod,
> +			 struct irq_bypass_consumer *cons)
> +{
> +	if (prod->stop)
> +		prod->stop(prod);
> +	if (cons->stop)
> +		cons->stop(cons);
> +
> +	cons->del_producer(cons, prod);
> +	prod->del_consumer(prod, cons);
> +
> +	if (cons->start)
> +		cons->start(cons);
> +	if (prod->start)
> +		prod->start(prod);
> +}
> +
> +/**
> + * irq_bypass_register_producer - register IRQ bypass producer
> + * @producer: pointer to producer structure
> + *
> + * Add the provided IRQ producer to the list of producers and connect
> + * with any matching token found on the IRQ consumers list.
> + */
> +int irq_bypass_register_producer(struct irq_bypass_producer *producer)
> +{
> +	struct irq_bypass_producer *tmp;
> +	struct irq_bypass_consumer *consumer;
> +
> +	if (!producer->add_consumer || !producer->del_consumer)
> +		return -EINVAL;
> +
> +	might_sleep();
> +
> +	if (!try_module_get(THIS_MODULE))
> +		return -ENODEV;
> +
> +	mutex_lock(&lock);
> +
> +	list_for_each_entry(tmp, &producers, node) {
> +		if (tmp->token == producer->token) {
> +			mutex_unlock(&lock);
> +			module_put(THIS_MODULE);
> +			return -EBUSY;
> +		}
> +	}
> +
> +	list_for_each_entry(consumer, &consumers, node) {
> +		if (consumer->token == producer->token) {
> +			int ret = __connect(producer, consumer);
> +			if (ret) {
> +				mutex_unlock(&lock);
> +				module_put(THIS_MODULE);
> +				return ret;
> +			}
> +			break;
> +		}
> +	}
> +
> +	list_add(&producer->node, &producers);
> +
> +	mutex_unlock(&lock);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(irq_bypass_register_producer);
> +
> +/**
> + * irq_bypass_unregister_producer - unregister IRQ bypass producer
> + * @producer: pointer to producer structure
> + *
> + * Remove a previously registered IRQ producer from the list of producers
> + * and disconnect it from any connected IRQ consumer.
> + */
> +void irq_bypass_unregister_producer(struct irq_bypass_producer *producer)
> +{
> +	struct irq_bypass_producer *tmp;
> +	struct irq_bypass_consumer *consumer;
> +
> +	might_sleep();
> +
> +	if (!try_module_get(THIS_MODULE))
> +		return; /* nothing in the list anyway */
> +
> +	mutex_lock(&lock);
> +
> +	list_for_each_entry(tmp, &producers, node) {
> +		if (tmp->token != producer->token)
> +			continue;
> +
> +		list_for_each_entry(consumer, &consumers, node) {
> +			if (consumer->token == producer->token) {
> +				__disconnect(producer, consumer);
> +				break;
> +			}
> +		}
> +
> +		list_del(&producer->node);
> +		module_put(THIS_MODULE);
> +		break;
> +	}
> +
> +	mutex_unlock(&lock);
> +
> +	module_put(THIS_MODULE);
> +}
> +EXPORT_SYMBOL_GPL(irq_bypass_unregister_producer);
> +
> +/**
> + * irq_bypass_register_consumer - register IRQ bypass consumer
> + * @consumer: pointer to consumer structure
> + *
> + * Add the provided IRQ consumer to the list of consumers and connect
> + * with any matching token found on the IRQ producer list.
> + */
> +int irq_bypass_register_consumer(struct irq_bypass_consumer *consumer)
> +{
> +	struct irq_bypass_consumer *tmp;
> +	struct irq_bypass_producer *producer;
> +
> +	if (!consumer->add_producer || !consumer->del_producer)
> +		return -EINVAL;
> +
> +	might_sleep();
> +
> +	if (!try_module_get(THIS_MODULE))
> +		return -ENODEV;
> +
> +	mutex_lock(&lock);
> +
> +	list_for_each_entry(tmp, &consumers, node) {
> +		if (tmp->token == consumer->token) {
> +			mutex_unlock(&lock);
> +			module_put(THIS_MODULE);
> +			return -EBUSY;
> +		}
> +	}
> +
> +	list_for_each_entry(producer, &producers, node) {
> +		if (producer->token == consumer->token) {
> +			int ret = __connect(producer, consumer);
> +			if (ret) {
> +				mutex_unlock(&lock);
> +				module_put(THIS_MODULE);
> +				return ret;
> +			}
> +			break;
> +		}
> +	}
> +
> +	list_add(&consumer->node, &consumers);
> +
> +	mutex_unlock(&lock);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(irq_bypass_register_consumer);
> +
> +/**
> + * irq_bypass_unregister_consumer - unregister IRQ bypass consumer
> + * @consumer: pointer to consumer structure
> + *
> + * Remove a previously registered IRQ consumer from the list of consumers
> + * and disconnect it from any connected IRQ producer.
> + */
> +void irq_bypass_unregister_consumer(struct irq_bypass_consumer
> *consumer)
> +{
> +	struct irq_bypass_consumer *tmp;
> +	struct irq_bypass_producer *producer;
> +
> +	might_sleep();
> +
> +	if (!try_module_get(THIS_MODULE))
> +		return; /* nothing in the list anyway */
> +
> +	mutex_lock(&lock);
> +
> +	list_for_each_entry(tmp, &consumers, node) {
> +		if (tmp->token != consumer->token)
> +			continue;
> +
> +		list_for_each_entry(producer, &producers, node) {
> +			if (producer->token == consumer->token) {
> +				__disconnect(producer, consumer);
> +				break;
> +			}
> +		}
> +
> +		list_del(&consumer->node);
> +		module_put(THIS_MODULE);
> +		break;
> +	}
> +
> +	mutex_unlock(&lock);
> +
> +	module_put(THIS_MODULE);
> +}
> +EXPORT_SYMBOL_GPL(irq_bypass_unregister_consumer);

Powered by blists - more mailing lists