[PATCH v3 0/3] kvm: KVM_IRQFD cleanup, docs, sanitize flags

[PATCH v3 0/3] kvm: KVM_IRQFD cleanup, docs, sanitize flags

[Alex Williamson]

Before we start fiddling with what we can and can't add to KVM_IRQFD
we need to figure out if anyone has been sloppy in their use of the
ioctl flags.  This series has a minor cleanup to pass the struct
kvm_irqfd to seup functions rather than individual parameters, making
it more consistent with ioeventfd, adds API documentation for this
ioctl, and sanitizes the flags.  If anyone screams, we may have to
revert this last patch.  Thanks,

Alex

---

Alex Williamson (3):
      kvm: Sanitize KVM_IRQFD flags
      kvm: Add missing KVM_IRQFD API documentation
      kvm: Pass kvm_irqfd to functions


 Documentation/virtual/kvm/api.txt |   16 ++++++++++++++++
 include/linux/kvm_host.h          |    4 ++--
 virt/kvm/eventfd.c                |   23 +++++++++++++----------
 virt/kvm/kvm_main.c               |    2 +-
 4 files changed, 32 insertions(+), 13 deletions(-)

[PATCH v3 1/3] kvm: Pass kvm_irqfd to functions

[Alex Williamson]

Prune this down to just the struct kvm_irqfd so we can avoid
changing function definition for every flag or field we use.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---

 include/linux/kvm_host.h |    4 ++--
 virt/kvm/eventfd.c       |   20 ++++++++++----------
 virt/kvm/kvm_main.c      |    2 +-
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 27ac8a4..ae3b426 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -824,7 +824,7 @@ static inline void kvm_free_irq_routing(struct kvm *kvm) {}
 #ifdef CONFIG_HAVE_KVM_EVENTFD
 
 void kvm_eventfd_init(struct kvm *kvm);
-int kvm_irqfd(struct kvm *kvm, int fd, int gsi, int flags);
+int kvm_irqfd(struct kvm *kvm, struct kvm_irqfd *args);
 void kvm_irqfd_release(struct kvm *kvm);
 void kvm_irq_routing_update(struct kvm *, struct kvm_irq_routing_table *);
 int kvm_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args);
@@ -833,7 +833,7 @@ int kvm_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args);
 
 static inline void kvm_eventfd_init(struct kvm *kvm) {}
 
-static inline int kvm_irqfd(struct kvm *kvm, int fd, int gsi, int flags)
+static inline int kvm_irqfd(struct kvm *kvm, struct kvm_irqfd *args)
 {
 	return -EINVAL;
 }
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index f59c1e8..c307c24 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -198,7 +198,7 @@ static void irqfd_update(struct kvm *kvm, struct _irqfd *irqfd,
 }
 
 static int
-kvm_irqfd_assign(struct kvm *kvm, int fd, int gsi)
+kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args)
 {
 	struct kvm_irq_routing_table *irq_rt;
 	struct _irqfd *irqfd, *tmp;
@@ -212,12 +212,12 @@ kvm_irqfd_assign(struct kvm *kvm, int fd, int gsi)
 		return -ENOMEM;
 
 	irqfd->kvm = kvm;
-	irqfd->gsi = gsi;
+	irqfd->gsi = args->gsi;
 	INIT_LIST_HEAD(&irqfd->list);
 	INIT_WORK(&irqfd->inject, irqfd_inject);
 	INIT_WORK(&irqfd->shutdown, irqfd_shutdown);
 
-	file = eventfd_fget(fd);
+	file = eventfd_fget(args->fd);
 	if (IS_ERR(file)) {
 		ret = PTR_ERR(file);
 		goto fail;
@@ -298,19 +298,19 @@ kvm_eventfd_init(struct kvm *kvm)
  * shutdown any irqfd's that match fd+gsi
  */
 static int
-kvm_irqfd_deassign(struct kvm *kvm, int fd, int gsi)
+kvm_irqfd_deassign(struct kvm *kvm, struct kvm_irqfd *args)
 {
 	struct _irqfd *irqfd, *tmp;
 	struct eventfd_ctx *eventfd;
 
-	eventfd = eventfd_ctx_fdget(fd);
+	eventfd = eventfd_ctx_fdget(args->fd);
 	if (IS_ERR(eventfd))
 		return PTR_ERR(eventfd);
 
 	spin_lock_irq(&kvm->irqfds.lock);
 
 	list_for_each_entry_safe(irqfd, tmp, &kvm->irqfds.items, list) {
-		if (irqfd->eventfd == eventfd && irqfd->gsi == gsi) {
+		if (irqfd->eventfd == eventfd && irqfd->gsi == args->gsi) {
 			/*
 			 * This rcu_assign_pointer is needed for when
 			 * another thread calls kvm_irq_routing_update before
@@ -338,12 +338,12 @@ kvm_irqfd_deassign(struct kvm *kvm, int fd, int gsi)
 }
 
 int
-kvm_irqfd(struct kvm *kvm, int fd, int gsi, int flags)
+kvm_irqfd(struct kvm *kvm, struct kvm_irqfd *args)
 {
-	if (flags & KVM_IRQFD_FLAG_DEASSIGN)
-		return kvm_irqfd_deassign(kvm, fd, gsi);
+	if (args->flags & KVM_IRQFD_FLAG_DEASSIGN)
+		return kvm_irqfd_deassign(kvm, args);
 
-	return kvm_irqfd_assign(kvm, fd, gsi);
+	return kvm_irqfd_assign(kvm, args);
 }
 
 /*
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 02cb440..b4ad14cc 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2059,7 +2059,7 @@ static long kvm_vm_ioctl(struct file *filp,
 		r = -EFAULT;
 		if (copy_from_user(&data, argp, sizeof data))
 			goto out;
-		r = kvm_irqfd(kvm, data.fd, data.gsi, data.flags);
+		r = kvm_irqfd(kvm, &data);
 		break;
 	}
 	case KVM_IOEVENTFD: {

[PATCH v3 2/3] kvm: Add missing KVM_IRQFD API documentation

[Alex Williamson]

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---

 Documentation/virtual/kvm/api.txt |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 310fe50..100acde 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -1965,6 +1965,22 @@ return the hash table order in the parameter.  (If the guest is using
 the virtualized real-mode area (VRMA) facility, the kernel will
 re-create the VMRA HPTEs on the next KVM_RUN of any vcpu.)
 
+4.76 KVM_IRQFD
+
+Capability: KVM_CAP_IRQFD
+Architectures: x86
+Type: vm ioctl
+Parameters: struct kvm_irqfd (in)
+Returns: 0 on success, -1 on error
+
+Allows setting an eventfd to directly trigger a guest interrupt.
+kvm_irqfd.fd specifies the file descriptor to use as the eventfd and
+kvm_irqfd.gsi specifies the irqchip pin toggled by this event.  When
+an event is triggered on the eventfd, an interrupt is injected into
+the guest using the specified gsi pin.  The irqfd is removed using
+the KVM_IRQFD_FLAG_DEASSIGN flag, specifying both kvm_irqfd.fd
+and kvm_irqfd.gsi.
+
 
 5. The kvm_run structure
 ------------------------

[PATCH v3 3/3] kvm: Sanitize KVM_IRQFD flags

[Alex Williamson]

We only know of one so far.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

 virt/kvm/eventfd.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index c307c24..7d7e2aa 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -340,6 +340,9 @@ kvm_irqfd_deassign(struct kvm *kvm, struct kvm_irqfd *args)
 int
 kvm_irqfd(struct kvm *kvm, struct kvm_irqfd *args)
 {
+	if (args->flags & ~KVM_IRQFD_FLAG_DEASSIGN)
+		return -EINVAL;
+
 	if (args->flags & KVM_IRQFD_FLAG_DEASSIGN)
 		return kvm_irqfd_deassign(kvm, args);
 

Re: [PATCH v3 3/3] kvm: Sanitize KVM_IRQFD flags

[Michael S. Tsirkin]

On Fri, Jun 29, 2012 at 09:56:24AM -0600, Alex Williamson wrote:
> We only know of one so far.
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>

BTW should we sanitize padding as well?

> ---
> 
>  virt/kvm/eventfd.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> index c307c24..7d7e2aa 100644
> --- a/virt/kvm/eventfd.c
> +++ b/virt/kvm/eventfd.c
> @@ -340,6 +340,9 @@ kvm_irqfd_deassign(struct kvm *kvm, struct kvm_irqfd *args)
>  int
>  kvm_irqfd(struct kvm *kvm, struct kvm_irqfd *args)
>  {
> +	if (args->flags & ~KVM_IRQFD_FLAG_DEASSIGN)
> +		return -EINVAL;
> +
>  	if (args->flags & KVM_IRQFD_FLAG_DEASSIGN)
>  		return kvm_irqfd_deassign(kvm, args);
>  
> 
> --
> 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/

Re: [PATCH v3 3/3] kvm: Sanitize KVM_IRQFD flags

[Alex Williamson]

On Sun, 2012-07-01 at 11:26 +0300, Michael S. Tsirkin wrote:
> On Fri, Jun 29, 2012 at 09:56:24AM -0600, Alex Williamson wrote:
> > We only know of one so far.
> > 
> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> 
> BTW should we sanitize padding as well?

I imagine that to make use of a field in the padding we'll set a bit in
the flags to indicate that the field is present/valid.  That way we
don't care about garbage in the padding as long as we can sanitize the
flags.  Thanks,

Alex

> > ---
> > 
> >  virt/kvm/eventfd.c |    3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> > index c307c24..7d7e2aa 100644
> > --- a/virt/kvm/eventfd.c
> > +++ b/virt/kvm/eventfd.c
> > @@ -340,6 +340,9 @@ kvm_irqfd_deassign(struct kvm *kvm, struct kvm_irqfd *args)
> >  int
> >  kvm_irqfd(struct kvm *kvm, struct kvm_irqfd *args)
> >  {
> > +	if (args->flags & ~KVM_IRQFD_FLAG_DEASSIGN)
> > +		return -EINVAL;
> > +
> >  	if (args->flags & KVM_IRQFD_FLAG_DEASSIGN)
> >  		return kvm_irqfd_deassign(kvm, args);
> >  
> > 
> > --
> > 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/

Re: [PATCH v3 0/3] kvm: KVM_IRQFD cleanup, docs, sanitize flags

[Alex Williamson]

On Fri, 2012-06-29 at 09:56 -0600, Alex Williamson wrote:
> Before we start fiddling with what we can and can't add to KVM_IRQFD
> we need to figure out if anyone has been sloppy in their use of the
> ioctl flags.  This series has a minor cleanup to pass the struct
> kvm_irqfd to seup functions rather than individual parameters, making
> it more consistent with ioeventfd, adds API documentation for this
> ioctl, and sanitizes the flags.  If anyone screams, we may have to
> revert this last patch.  Thanks,

Avi, what do you think about trying to get this in for 3.5 to test
whether we break anyone?  Then we can aim for 3.6 for level irqfd &
eoifd support.  Thanks,

Alex

> ---
> 
> Alex Williamson (3):
>       kvm: Sanitize KVM_IRQFD flags
>       kvm: Add missing KVM_IRQFD API documentation
>       kvm: Pass kvm_irqfd to functions
> 
> 
>  Documentation/virtual/kvm/api.txt |   16 ++++++++++++++++
>  include/linux/kvm_host.h          |    4 ++--
>  virt/kvm/eventfd.c                |   23 +++++++++++++----------
>  virt/kvm/kvm_main.c               |    2 +-
>  4 files changed, 32 insertions(+), 13 deletions(-)

Re: [PATCH v3 0/3] kvm: KVM_IRQFD cleanup, docs, sanitize flags

[Avi Kivity]

On 07/02/2012 06:51 PM, Alex Williamson wrote:
> On Fri, 2012-06-29 at 09:56 -0600, Alex Williamson wrote:
>> Before we start fiddling with what we can and can't add to KVM_IRQFD
>> we need to figure out if anyone has been sloppy in their use of the
>> ioctl flags.  This series has a minor cleanup to pass the struct
>> kvm_irqfd to seup functions rather than individual parameters, making
>> it more consistent with ioeventfd, adds API documentation for this
>> ioctl, and sanitizes the flags.  If anyone screams, we may have to
>> revert this last patch.  Thanks,
> 
> Avi, what do you think about trying to get this in for 3.5 to test
> whether we break anyone?  Then we can aim for 3.6 for level irqfd &
> eoifd support.

Makes sense.  Marcelo?


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

Re: [PATCH v3 0/3] kvm: KVM_IRQFD cleanup, docs, sanitize flags

[Marcelo Tosatti]

On Mon, Jul 02, 2012 at 06:52:46PM +0300, Avi Kivity wrote:
> On 07/02/2012 06:51 PM, Alex Williamson wrote:
> > On Fri, 2012-06-29 at 09:56 -0600, Alex Williamson wrote:
> >> Before we start fiddling with what we can and can't add to KVM_IRQFD
> >> we need to figure out if anyone has been sloppy in their use of the
> >> ioctl flags.  This series has a minor cleanup to pass the struct
> >> kvm_irqfd to seup functions rather than individual parameters, making
> >> it more consistent with ioeventfd, adds API documentation for this
> >> ioctl, and sanitizes the flags.  If anyone screams, we may have to
> >> revert this last patch.  Thanks,
> > 
> > Avi, what do you think about trying to get this in for 3.5 to test
> > whether we break anyone?  Then we can aim for 3.6 for level irqfd &
> > eoifd support.
> 
> Makes sense.  Marcelo?

Applied to master, thanks.