Patchwork [v3,4/5] KVM: ioeventfd for virtio-ccw devices.

login
register
mail settings
Submitter Cornelia Huck
Date Feb. 25, 2013, 3:27 p.m.
Message ID <1361806070-62465-5-git-send-email-cornelia.huck@de.ibm.com>
Download mbox | patch
Permalink /patch/222940/
State New
Headers show

Comments

Cornelia Huck - Feb. 25, 2013, 3:27 p.m.
Enhance KVM_IOEVENTFD with a new flag that allows to attach to virtio-ccw
devices on s390 via the KVM_VIRTIO_CCW_NOTIFY_BUS.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 Documentation/virtual/kvm/api.txt |  8 ++++++++
 include/uapi/linux/kvm.h          |  3 +++
 virt/kvm/eventfd.c                | 14 ++++++++++++--
 3 files changed, 23 insertions(+), 2 deletions(-)
Michael S. Tsirkin - Feb. 26, 2013, 10:55 a.m.
On Mon, Feb 25, 2013 at 04:27:49PM +0100, Cornelia Huck wrote:
> Enhance KVM_IOEVENTFD with a new flag that allows to attach to virtio-ccw
> devices on s390 via the KVM_VIRTIO_CCW_NOTIFY_BUS.
> 
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> ---
>  Documentation/virtual/kvm/api.txt |  8 ++++++++
>  include/uapi/linux/kvm.h          |  3 +++
>  virt/kvm/eventfd.c                | 14 ++++++++++++--
>  3 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index c2534c3..86232d6 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -1468,15 +1468,23 @@ struct kvm_ioeventfd {
>  	__u8  pad[36];
>  };
>  
> +For the special case of virtio-ccw devices on s390, the ioevent is matched
> +to a subchannel/virtqueue tuple instead.
> +
>  The following flags are defined:
>  
>  #define KVM_IOEVENTFD_FLAG_DATAMATCH (1 << kvm_ioeventfd_flag_nr_datamatch)
>  #define KVM_IOEVENTFD_FLAG_PIO       (1 << kvm_ioeventfd_flag_nr_pio)
>  #define KVM_IOEVENTFD_FLAG_DEASSIGN  (1 << kvm_ioeventfd_flag_nr_deassign)
> +#define KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY \
> +	(1 << kvm_ioeventfd_flag_nr_virtio_ccw_notify)
>  
>  If datamatch flag is set, the event will be signaled only if the written value
>  to the registered address is equal to datamatch in struct kvm_ioeventfd.
>  
> +For virtio-ccw devices, addr contains the subchannel id and datamatch the
> +virtqueue index.
> +
>  
>  4.60 KVM_DIRTY_TLB
>  
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 9a2db57..8f3e5ae 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -448,12 +448,15 @@ enum {
>  	kvm_ioeventfd_flag_nr_datamatch,
>  	kvm_ioeventfd_flag_nr_pio,
>  	kvm_ioeventfd_flag_nr_deassign,
> +	kvm_ioeventfd_flag_nr_virtio_ccw_notify,
>  	kvm_ioeventfd_flag_nr_max,
>  };
>  
>  #define KVM_IOEVENTFD_FLAG_DATAMATCH (1 << kvm_ioeventfd_flag_nr_datamatch)
>  #define KVM_IOEVENTFD_FLAG_PIO       (1 << kvm_ioeventfd_flag_nr_pio)
>  #define KVM_IOEVENTFD_FLAG_DEASSIGN  (1 << kvm_ioeventfd_flag_nr_deassign)
> +#define KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY \
> +	(1 << kvm_ioeventfd_flag_nr_virtio_ccw_notify)
>  
>  #define KVM_IOEVENTFD_VALID_FLAG_MASK  ((1 << kvm_ioeventfd_flag_nr_max) - 1)
>  
> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> index f0ced1a..8de3cd7 100644
> --- a/virt/kvm/eventfd.c
> +++ b/virt/kvm/eventfd.c
> @@ -679,11 +679,16 @@ static int
>  kvm_assign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
>  {
>  	int                       pio = args->flags & KVM_IOEVENTFD_FLAG_PIO;
> -	enum kvm_bus              bus_idx = pio ? KVM_PIO_BUS : KVM_MMIO_BUS;
> +	int                       ccw;
> +	enum kvm_bus              bus_idx;
>  	struct _ioeventfd        *p;
>  	struct eventfd_ctx       *eventfd;
>  	int                       ret;
>  
> +	ccw = args->flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY;
> +	bus_idx = pio ? KVM_PIO_BUS :
> +		ccw ? KVM_VIRTIO_CCW_NOTIFY_BUS :
> +		KVM_MMIO_BUS;

May be better to rewrite using if/else.

>  	/* must be natural-word sized */
>  	switch (args->len) {
>  	case 1:
> @@ -759,11 +764,16 @@ static int
>  kvm_deassign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
>  {
>  	int                       pio = args->flags & KVM_IOEVENTFD_FLAG_PIO;
> -	enum kvm_bus              bus_idx = pio ? KVM_PIO_BUS : KVM_MMIO_BUS;
> +	int                       ccw;
> +	enum kvm_bus              bus_idx;
>  	struct _ioeventfd        *p, *tmp;
>  	struct eventfd_ctx       *eventfd;
>  	int                       ret = -ENOENT;
>  
> +	ccw = args->flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY;
> +	bus_idx = pio ? KVM_PIO_BUS :
> +		ccw ? KVM_VIRTIO_CCW_NOTIFY_BUS :
> +		KVM_MMIO_BUS;
>  	eventfd = eventfd_ctx_fdget(args->fd);
>  	if (IS_ERR(eventfd))
>  		return PTR_ERR(eventfd);
> -- 
> 1.7.12.4
Cornelia Huck - Feb. 26, 2013, 11:55 a.m.
On Tue, 26 Feb 2013 12:55:36 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Feb 25, 2013 at 04:27:49PM +0100, Cornelia Huck wrote:
> > Enhance KVM_IOEVENTFD with a new flag that allows to attach to virtio-ccw
> > devices on s390 via the KVM_VIRTIO_CCW_NOTIFY_BUS.
> > 
> > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> > ---
> >  Documentation/virtual/kvm/api.txt |  8 ++++++++
> >  include/uapi/linux/kvm.h          |  3 +++
> >  virt/kvm/eventfd.c                | 14 ++++++++++++--
> >  3 files changed, 23 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> > index c2534c3..86232d6 100644
> > --- a/Documentation/virtual/kvm/api.txt
> > +++ b/Documentation/virtual/kvm/api.txt
> > @@ -1468,15 +1468,23 @@ struct kvm_ioeventfd {
> >  	__u8  pad[36];
> >  };
> >  
> > +For the special case of virtio-ccw devices on s390, the ioevent is matched
> > +to a subchannel/virtqueue tuple instead.
> > +
> >  The following flags are defined:
> >  
> >  #define KVM_IOEVENTFD_FLAG_DATAMATCH (1 << kvm_ioeventfd_flag_nr_datamatch)
> >  #define KVM_IOEVENTFD_FLAG_PIO       (1 << kvm_ioeventfd_flag_nr_pio)
> >  #define KVM_IOEVENTFD_FLAG_DEASSIGN  (1 << kvm_ioeventfd_flag_nr_deassign)
> > +#define KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY \
> > +	(1 << kvm_ioeventfd_flag_nr_virtio_ccw_notify)
> >  
> >  If datamatch flag is set, the event will be signaled only if the written value
> >  to the registered address is equal to datamatch in struct kvm_ioeventfd.
> >  
> > +For virtio-ccw devices, addr contains the subchannel id and datamatch the
> > +virtqueue index.
> > +
> >  
> >  4.60 KVM_DIRTY_TLB
> >  
> > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> > index 9a2db57..8f3e5ae 100644
> > --- a/include/uapi/linux/kvm.h
> > +++ b/include/uapi/linux/kvm.h
> > @@ -448,12 +448,15 @@ enum {
> >  	kvm_ioeventfd_flag_nr_datamatch,
> >  	kvm_ioeventfd_flag_nr_pio,
> >  	kvm_ioeventfd_flag_nr_deassign,
> > +	kvm_ioeventfd_flag_nr_virtio_ccw_notify,
> >  	kvm_ioeventfd_flag_nr_max,
> >  };
> >  
> >  #define KVM_IOEVENTFD_FLAG_DATAMATCH (1 << kvm_ioeventfd_flag_nr_datamatch)
> >  #define KVM_IOEVENTFD_FLAG_PIO       (1 << kvm_ioeventfd_flag_nr_pio)
> >  #define KVM_IOEVENTFD_FLAG_DEASSIGN  (1 << kvm_ioeventfd_flag_nr_deassign)
> > +#define KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY \
> > +	(1 << kvm_ioeventfd_flag_nr_virtio_ccw_notify)
> >  
> >  #define KVM_IOEVENTFD_VALID_FLAG_MASK  ((1 << kvm_ioeventfd_flag_nr_max) - 1)
> >  
> > diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> > index f0ced1a..8de3cd7 100644
> > --- a/virt/kvm/eventfd.c
> > +++ b/virt/kvm/eventfd.c
> > @@ -679,11 +679,16 @@ static int
> >  kvm_assign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
> >  {
> >  	int                       pio = args->flags & KVM_IOEVENTFD_FLAG_PIO;
> > -	enum kvm_bus              bus_idx = pio ? KVM_PIO_BUS : KVM_MMIO_BUS;
> > +	int                       ccw;
> > +	enum kvm_bus              bus_idx;
> >  	struct _ioeventfd        *p;
> >  	struct eventfd_ctx       *eventfd;
> >  	int                       ret;
> >  
> > +	ccw = args->flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY;
> > +	bus_idx = pio ? KVM_PIO_BUS :
> > +		ccw ? KVM_VIRTIO_CCW_NOTIFY_BUS :
> > +		KVM_MMIO_BUS;
> 
> May be better to rewrite using if/else.

Matter of taste, I guess. But I can do that.

> 
> >  	/* must be natural-word sized */
> >  	switch (args->len) {
> >  	case 1:
> > @@ -759,11 +764,16 @@ static int
> >  kvm_deassign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
> >  {
> >  	int                       pio = args->flags & KVM_IOEVENTFD_FLAG_PIO;
> > -	enum kvm_bus              bus_idx = pio ? KVM_PIO_BUS : KVM_MMIO_BUS;
> > +	int                       ccw;
> > +	enum kvm_bus              bus_idx;
> >  	struct _ioeventfd        *p, *tmp;
> >  	struct eventfd_ctx       *eventfd;
> >  	int                       ret = -ENOENT;
> >  
> > +	ccw = args->flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY;
> > +	bus_idx = pio ? KVM_PIO_BUS :
> > +		ccw ? KVM_VIRTIO_CCW_NOTIFY_BUS :
> > +		KVM_MMIO_BUS;
> >  	eventfd = eventfd_ctx_fdget(args->fd);
> >  	if (IS_ERR(eventfd))
> >  		return PTR_ERR(eventfd);
> > -- 
> > 1.7.12.4
>
Stefan Hajnoczi - Feb. 26, 2013, 2:19 p.m.
On Mon, Feb 25, 2013 at 04:27:49PM +0100, Cornelia Huck wrote:
> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> index f0ced1a..8de3cd7 100644
> --- a/virt/kvm/eventfd.c
> +++ b/virt/kvm/eventfd.c
> @@ -679,11 +679,16 @@ static int
>  kvm_assign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
>  {
>  	int                       pio = args->flags & KVM_IOEVENTFD_FLAG_PIO;
> -	enum kvm_bus              bus_idx = pio ? KVM_PIO_BUS : KVM_MMIO_BUS;
> +	int                       ccw;
> +	enum kvm_bus              bus_idx;
>  	struct _ioeventfd        *p;
>  	struct eventfd_ctx       *eventfd;
>  	int                       ret;
>  
> +	ccw = args->flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY;
> +	bus_idx = pio ? KVM_PIO_BUS :
> +		ccw ? KVM_VIRTIO_CCW_NOTIFY_BUS :
> +		KVM_MMIO_BUS;
>  	/* must be natural-word sized */
>  	switch (args->len) {
>  	case 1:
> @@ -759,11 +764,16 @@ static int
>  kvm_deassign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
>  {
>  	int                       pio = args->flags & KVM_IOEVENTFD_FLAG_PIO;
> -	enum kvm_bus              bus_idx = pio ? KVM_PIO_BUS : KVM_MMIO_BUS;
> +	int                       ccw;
> +	enum kvm_bus              bus_idx;
>  	struct _ioeventfd        *p, *tmp;
>  	struct eventfd_ctx       *eventfd;
>  	int                       ret = -ENOENT;
>  
> +	ccw = args->flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY;
> +	bus_idx = pio ? KVM_PIO_BUS :
> +		ccw ? KVM_VIRTIO_CCW_NOTIFY_BUS :
> +		KVM_MMIO_BUS;

This is getting pretty convoluted.  Drop of pio and ccw local variables,
replace ?: with an if statement:

if (args->flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY)
	bus_idx = KVM_VIRTIO_CCW_NOTIFY_BUS;
else if (args->flags & KVM_IOEVENTFD_FLAG_PIO)
	bus_idx = KVM_PIO_BUS;
else
	bus_idx = KVM_MMIO_BUS;
Stefan Hajnoczi - Feb. 26, 2013, 2:20 p.m.
On Tue, Feb 26, 2013 at 12:55:36PM +0200, Michael S. Tsirkin wrote:
> On Mon, Feb 25, 2013 at 04:27:49PM +0100, Cornelia Huck wrote:
> > diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> > index f0ced1a..8de3cd7 100644
> > --- a/virt/kvm/eventfd.c
> > +++ b/virt/kvm/eventfd.c
> > @@ -679,11 +679,16 @@ static int
> >  kvm_assign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
> >  {
> >  	int                       pio = args->flags & KVM_IOEVENTFD_FLAG_PIO;
> > -	enum kvm_bus              bus_idx = pio ? KVM_PIO_BUS : KVM_MMIO_BUS;
> > +	int                       ccw;
> > +	enum kvm_bus              bus_idx;
> >  	struct _ioeventfd        *p;
> >  	struct eventfd_ctx       *eventfd;
> >  	int                       ret;
> >  
> > +	ccw = args->flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY;
> > +	bus_idx = pio ? KVM_PIO_BUS :
> > +		ccw ? KVM_VIRTIO_CCW_NOTIFY_BUS :
> > +		KVM_MMIO_BUS;
> 
> May be better to rewrite using if/else.

Saw this after sending my comment.  I agree with Michael, an if
statement allows you to drop the locals and capture the bus_idx
conversion in a single place (it could even be a static function to save
duplicating the code in both functions that use it).

Stefan
Cornelia Huck - Feb. 26, 2013, 2:47 p.m.
On Tue, 26 Feb 2013 15:20:52 +0100
Stefan Hajnoczi <stefanha@gmail.com> wrote:

> On Tue, Feb 26, 2013 at 12:55:36PM +0200, Michael S. Tsirkin wrote:
> > On Mon, Feb 25, 2013 at 04:27:49PM +0100, Cornelia Huck wrote:
> > > diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> > > index f0ced1a..8de3cd7 100644
> > > --- a/virt/kvm/eventfd.c
> > > +++ b/virt/kvm/eventfd.c
> > > @@ -679,11 +679,16 @@ static int
> > >  kvm_assign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
> > >  {
> > >  	int                       pio = args->flags & KVM_IOEVENTFD_FLAG_PIO;
> > > -	enum kvm_bus              bus_idx = pio ? KVM_PIO_BUS : KVM_MMIO_BUS;
> > > +	int                       ccw;
> > > +	enum kvm_bus              bus_idx;
> > >  	struct _ioeventfd        *p;
> > >  	struct eventfd_ctx       *eventfd;
> > >  	int                       ret;
> > >  
> > > +	ccw = args->flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY;
> > > +	bus_idx = pio ? KVM_PIO_BUS :
> > > +		ccw ? KVM_VIRTIO_CCW_NOTIFY_BUS :
> > > +		KVM_MMIO_BUS;
> > 
> > May be better to rewrite using if/else.
> 
> Saw this after sending my comment.  I agree with Michael, an if
> statement allows you to drop the locals and capture the bus_idx
> conversion in a single place (it could even be a static function to save
> duplicating the code in both functions that use it).
> 
> Stefan
> 

Introducing a helper function sounds reasonable.

Patch

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index c2534c3..86232d6 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -1468,15 +1468,23 @@  struct kvm_ioeventfd {
 	__u8  pad[36];
 };
 
+For the special case of virtio-ccw devices on s390, the ioevent is matched
+to a subchannel/virtqueue tuple instead.
+
 The following flags are defined:
 
 #define KVM_IOEVENTFD_FLAG_DATAMATCH (1 << kvm_ioeventfd_flag_nr_datamatch)
 #define KVM_IOEVENTFD_FLAG_PIO       (1 << kvm_ioeventfd_flag_nr_pio)
 #define KVM_IOEVENTFD_FLAG_DEASSIGN  (1 << kvm_ioeventfd_flag_nr_deassign)
+#define KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY \
+	(1 << kvm_ioeventfd_flag_nr_virtio_ccw_notify)
 
 If datamatch flag is set, the event will be signaled only if the written value
 to the registered address is equal to datamatch in struct kvm_ioeventfd.
 
+For virtio-ccw devices, addr contains the subchannel id and datamatch the
+virtqueue index.
+
 
 4.60 KVM_DIRTY_TLB
 
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 9a2db57..8f3e5ae 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -448,12 +448,15 @@  enum {
 	kvm_ioeventfd_flag_nr_datamatch,
 	kvm_ioeventfd_flag_nr_pio,
 	kvm_ioeventfd_flag_nr_deassign,
+	kvm_ioeventfd_flag_nr_virtio_ccw_notify,
 	kvm_ioeventfd_flag_nr_max,
 };
 
 #define KVM_IOEVENTFD_FLAG_DATAMATCH (1 << kvm_ioeventfd_flag_nr_datamatch)
 #define KVM_IOEVENTFD_FLAG_PIO       (1 << kvm_ioeventfd_flag_nr_pio)
 #define KVM_IOEVENTFD_FLAG_DEASSIGN  (1 << kvm_ioeventfd_flag_nr_deassign)
+#define KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY \
+	(1 << kvm_ioeventfd_flag_nr_virtio_ccw_notify)
 
 #define KVM_IOEVENTFD_VALID_FLAG_MASK  ((1 << kvm_ioeventfd_flag_nr_max) - 1)
 
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index f0ced1a..8de3cd7 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -679,11 +679,16 @@  static int
 kvm_assign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
 {
 	int                       pio = args->flags & KVM_IOEVENTFD_FLAG_PIO;
-	enum kvm_bus              bus_idx = pio ? KVM_PIO_BUS : KVM_MMIO_BUS;
+	int                       ccw;
+	enum kvm_bus              bus_idx;
 	struct _ioeventfd        *p;
 	struct eventfd_ctx       *eventfd;
 	int                       ret;
 
+	ccw = args->flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY;
+	bus_idx = pio ? KVM_PIO_BUS :
+		ccw ? KVM_VIRTIO_CCW_NOTIFY_BUS :
+		KVM_MMIO_BUS;
 	/* must be natural-word sized */
 	switch (args->len) {
 	case 1:
@@ -759,11 +764,16 @@  static int
 kvm_deassign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
 {
 	int                       pio = args->flags & KVM_IOEVENTFD_FLAG_PIO;
-	enum kvm_bus              bus_idx = pio ? KVM_PIO_BUS : KVM_MMIO_BUS;
+	int                       ccw;
+	enum kvm_bus              bus_idx;
 	struct _ioeventfd        *p, *tmp;
 	struct eventfd_ctx       *eventfd;
 	int                       ret = -ENOENT;
 
+	ccw = args->flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY;
+	bus_idx = pio ? KVM_PIO_BUS :
+		ccw ? KVM_VIRTIO_CCW_NOTIFY_BUS :
+		KVM_MMIO_BUS;
 	eventfd = eventfd_ctx_fdget(args->fd);
 	if (IS_ERR(eventfd))
 		return PTR_ERR(eventfd);