kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH kvmtool 00/13] Implement reset of virtio devices
@ 2018-12-04 11:08 Julien Thierry
  2018-12-04 11:08 ` [PATCH kvmtool 01/13] ioeventfd: Fix removal of ioeventfd Julien Thierry
                   ` (13 more replies)
  0 siblings, 14 replies; 17+ messages in thread
From: Julien Thierry @ 2018-12-04 11:08 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: will.deacon

Hi,

This series was developped by Jean-Philippe and is needed for a series
I'll be posting shortly after to load firmwares on arm kvmtool.

Currently, when a guest tries to reset a device, a lot of ressources
aren't reset (threads keep running, virtio queue keep their state, etc).

When the guest only does the reset to initialize the device and there
were no previous users, there is no noticeable issue. But when a guest
has a firmare + Linux, if the firmware uses a virtio device, Linux will
fail to probe that device.

This series aim to properly reset the virtio resources when the guests
requests it.

Reset of net vhost is unsupported for now.

Patch 1 is a bug fix on ioeventfd
Patch 2-6 provide the core support so devices can implement their reset
Patch 7-13 implements the reset for the various virtio devices

Thanks,

Julien

-->

Jean-Philippe Brucker (13):
  ioeventfd: Fix removal of ioeventfd
  virtio: Implement notify_status
  virtio: Add get_vq_count() callback
  virtio: Add get_vq() callback
  virtio: Add exit_vq() callback
  virtio: Add reset() callback
  net/uip: Add exit function
  virtio/net: Clean virtqueue state
  virtio/net: Implement device and virtqueue reset
  virtio/blk: Reset virtqueue
  threadpool: Add cancel() function
  virtio/p9: Implement reset
  virtio/console: Implement reset

 include/kvm/threadpool.h  |   2 +
 include/kvm/uip.h         |   6 ++
 include/kvm/virtio-mmio.h |   1 +
 include/kvm/virtio-pci.h  |   1 +
 include/kvm/virtio.h      |  30 ++++++-
 ioeventfd.c               |   6 +-
 net/uip/core.c            |  17 ++++
 net/uip/dhcp.c            |   6 ++
 net/uip/tcp.c             |  54 ++++++++++---
 net/uip/udp.c             |  41 ++++++++--
 util/threadpool.c         |  25 +++++-
 virtio/9p.c               |  33 +++++++-
 virtio/balloon.c          |  17 +++-
 virtio/blk.c              |  84 ++++++++++++++------
 virtio/console.c          |  49 ++++++++----
 virtio/core.c             |  42 ++++++++++
 virtio/mmio.c             |  52 +++++++++----
 virtio/net.c              | 195 +++++++++++++++++++++++++++++++++-------------
 virtio/pci.c              |  49 ++++++++----
 virtio/rng.c              |  12 ++-
 virtio/scsi.c             |  17 +++-
 21 files changed, 582 insertions(+), 157 deletions(-)

--
1.9.1

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH kvmtool 01/13] ioeventfd: Fix removal of ioeventfd
  2018-12-04 11:08 [PATCH kvmtool 00/13] Implement reset of virtio devices Julien Thierry
@ 2018-12-04 11:08 ` Julien Thierry
  2018-12-04 11:08 ` [PATCH kvmtool 02/13] virtio: Implement notify_status Julien Thierry
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Julien Thierry @ 2018-12-04 11:08 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: will.deacon

From: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>

Fix three bugs that prevent removal of ioeventfds in KVM. Store the
flags in the right structure, check the datamatch parameter, and pass
the fd to KVM.

Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Signed-off-by: Julien Thierry <julien.thierry@arm.com>
---
 ioeventfd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/ioeventfd.c b/ioeventfd.c
index 14453b8..3ae8267 100644
--- a/ioeventfd.c
+++ b/ioeventfd.c
@@ -172,7 +172,7 @@ int ioeventfd__add_event(struct ioevent *ioevent, int flags)
 		}
 	}
 
-	ioevent->flags = kvm_ioevent.flags;
+	new_ioevent->flags = kvm_ioevent.flags;
 	list_add_tail(&new_ioevent->list, &used_ioevents);
 
 	return 0;
@@ -192,7 +192,8 @@ int ioeventfd__del_event(u64 addr, u64 datamatch)
 		return -ENOSYS;
 
 	list_for_each_entry(ioevent, &used_ioevents, list) {
-		if (ioevent->io_addr == addr) {
+		if (ioevent->io_addr == addr &&
+		    ioevent->datamatch == datamatch) {
 			found = 1;
 			break;
 		}
@@ -202,6 +203,7 @@ int ioeventfd__del_event(u64 addr, u64 datamatch)
 		return -ENOENT;
 
 	kvm_ioevent = (struct kvm_ioeventfd) {
+		.fd			= ioevent->fd,
 		.addr			= ioevent->io_addr,
 		.len			= ioevent->io_len,
 		.datamatch		= ioevent->datamatch,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH kvmtool 02/13] virtio: Implement notify_status
  2018-12-04 11:08 [PATCH kvmtool 00/13] Implement reset of virtio devices Julien Thierry
  2018-12-04 11:08 ` [PATCH kvmtool 01/13] ioeventfd: Fix removal of ioeventfd Julien Thierry
@ 2018-12-04 11:08 ` Julien Thierry
  2018-12-04 11:08 ` [PATCH kvmtool 03/13] virtio: Add get_vq_count() callback Julien Thierry
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Julien Thierry @ 2018-12-04 11:08 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: will.deacon

From: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>

Modern virtio require proper status handling and reset. A "notify_status"
callback is already present in the virtio ops, but isn't implemented by
any device. Instead they currently use "set_guest_feature" to reset the
device and deal with endianess. This isn't sufficient for proper device
reset, so add the notify_status callback to all devices that need it.

To add useful hints like "start" and "stop", extend the status variable to
32-bits.

Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Signed-off-by: Julien Thierry <julien.thierry@arm.com>
---
 include/kvm/virtio.h | 21 ++++++++++++++++++++-
 virtio/9p.c          |  5 +++++
 virtio/balloon.c     |  5 +++++
 virtio/blk.c         |  5 +++++
 virtio/console.c     |  5 +++++
 virtio/core.c        | 23 +++++++++++++++++++++++
 virtio/mmio.c        |  3 +--
 virtio/net.c         | 10 ++++++++++
 virtio/pci.c         |  3 +--
 virtio/scsi.c        |  5 +++++
 10 files changed, 80 insertions(+), 5 deletions(-)

diff --git a/include/kvm/virtio.h b/include/kvm/virtio.h
index db758b1..742aa9e 100644
--- a/include/kvm/virtio.h
+++ b/include/kvm/virtio.h
@@ -7,6 +7,7 @@
 #include <linux/virtio_pci.h>
 
 #include <linux/types.h>
+#include <linux/virtio_config.h>
 #include <sys/uio.h>
 
 #include "kvm/barrier.h"
@@ -27,6 +28,21 @@
 #define VIRTIO_ENDIAN_HOST VIRTIO_ENDIAN_BE
 #endif
 
+/* Reserved status bits */
+#define VIRTIO_CONFIG_S_MASK \
+	(VIRTIO_CONFIG_S_ACKNOWLEDGE |	\
+	 VIRTIO_CONFIG_S_DRIVER |	\
+	 VIRTIO_CONFIG_S_DRIVER_OK |	\
+	 VIRTIO_CONFIG_S_FEATURES_OK |	\
+	 VIRTIO_CONFIG_S_NEEDS_RESET |	\
+	 VIRTIO_CONFIG_S_FAILED)
+
+/* Kvmtool status bits */
+/* Start the device */
+#define VIRTIO__STATUS_START		(1 << 8)
+/* Stop the device */
+#define VIRTIO__STATUS_STOP		(1 << 9)
+
 struct virt_queue {
 	struct vring	vring;
 	u32		pfn;
@@ -155,6 +171,7 @@ struct virtio_device {
 	struct virtio_ops	*ops;
 	u16			endian;
 	u32			features;
+	u32			status;
 };
 
 struct virtio_ops {
@@ -171,7 +188,7 @@ struct virtio_ops {
 	void (*notify_vq_eventfd)(struct kvm *kvm, void *dev, u32 vq, u32 efd);
 	int (*signal_vq)(struct kvm *kvm, struct virtio_device *vdev, u32 queueid);
 	int (*signal_config)(struct kvm *kvm, struct virtio_device *vdev);
-	void (*notify_status)(struct kvm *kvm, void *dev, u8 status);
+	void (*notify_status)(struct kvm *kvm, void *dev, u32 status);
 	int (*init)(struct kvm *kvm, void *dev, struct virtio_device *vdev,
 		    int device_id, int subsys_id, int class);
 	int (*exit)(struct kvm *kvm, struct virtio_device *vdev);
@@ -197,5 +214,7 @@ static inline void virtio_init_device_vq(struct virtio_device *vdev,
 
 void virtio_set_guest_features(struct kvm *kvm, struct virtio_device *vdev,
 			       void *dev, u32 features);
+void virtio_notify_status(struct kvm *kvm, struct virtio_device *vdev,
+			  void *dev, u8 status);
 
 #endif /* KVM__VIRTIO_H */
diff --git a/virtio/9p.c b/virtio/9p.c
index 69fdc4b..4b93b4c 100644
--- a/virtio/9p.c
+++ b/virtio/9p.c
@@ -1382,6 +1382,10 @@ static void set_guest_features(struct kvm *kvm, void *dev, u32 features)
 	conf->tag_len = virtio_host_to_guest_u16(&p9dev->vdev, conf->tag_len);
 }
 
+static void notify_status(struct kvm *kvm, void *dev, u32 status)
+{
+}
+
 static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
 		   u32 pfn)
 {
@@ -1441,6 +1445,7 @@ struct virtio_ops p9_dev_virtio_ops = {
 	.get_host_features	= get_host_features,
 	.set_guest_features	= set_guest_features,
 	.init_vq		= init_vq,
+	.notify_status		= notify_status,
 	.notify_vq		= notify_vq,
 	.get_pfn_vq		= get_pfn_vq,
 	.get_size_vq		= get_size_vq,
diff --git a/virtio/balloon.c b/virtio/balloon.c
index 9564aa3..871d6e0 100644
--- a/virtio/balloon.c
+++ b/virtio/balloon.c
@@ -193,6 +193,10 @@ static void set_guest_features(struct kvm *kvm, void *dev, u32 features)
 	bdev->features = features;
 }
 
+static void notify_status(struct kvm *kvm, void *dev, u32 status)
+{
+}
+
 static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
 		   u32 pfn)
 {
@@ -244,6 +248,7 @@ struct virtio_ops bln_dev_virtio_ops = {
 	.get_host_features	= get_host_features,
 	.set_guest_features	= set_guest_features,
 	.init_vq		= init_vq,
+	.notify_status		= notify_status,
 	.notify_vq		= notify_vq,
 	.get_pfn_vq		= get_pfn_vq,
 	.get_size_vq		= get_size_vq,
diff --git a/virtio/blk.c b/virtio/blk.c
index c485e4f..db9f4cc 100644
--- a/virtio/blk.c
+++ b/virtio/blk.c
@@ -174,6 +174,10 @@ static void set_guest_features(struct kvm *kvm, void *dev, u32 features)
 	conf->opt_io_size = virtio_host_to_guest_u32(&bdev->vdev, conf->opt_io_size);
 }
 
+static void notify_status(struct kvm *kvm, void *dev, u32 status)
+{
+}
+
 static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
 		   u32 pfn)
 {
@@ -249,6 +253,7 @@ static struct virtio_ops blk_dev_virtio_ops = {
 	.get_host_features	= get_host_features,
 	.set_guest_features	= set_guest_features,
 	.init_vq		= init_vq,
+	.notify_status		= notify_status,
 	.notify_vq		= notify_vq,
 	.get_pfn_vq		= get_pfn_vq,
 	.get_size_vq		= get_size_vq,
diff --git a/virtio/console.c b/virtio/console.c
index dc0a9c4..b9df5c9 100644
--- a/virtio/console.c
+++ b/virtio/console.c
@@ -140,6 +140,10 @@ static void set_guest_features(struct kvm *kvm, void *dev, u32 features)
 	conf->max_nr_ports = virtio_host_to_guest_u32(&cdev->vdev, conf->max_nr_ports);
 }
 
+static void notify_status(struct kvm *kvm, void *dev, u32 status)
+{
+}
+
 static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
 		   u32 pfn)
 {
@@ -203,6 +207,7 @@ static struct virtio_ops con_dev_virtio_ops = {
 	.get_host_features	= get_host_features,
 	.set_guest_features	= set_guest_features,
 	.init_vq		= init_vq,
+	.notify_status		= notify_status,
 	.notify_vq		= notify_vq,
 	.get_pfn_vq		= get_pfn_vq,
 	.get_size_vq		= get_size_vq,
diff --git a/virtio/core.c b/virtio/core.c
index 0e2646c..9114f27 100644
--- a/virtio/core.c
+++ b/virtio/core.c
@@ -214,6 +214,29 @@ void virtio_set_guest_features(struct kvm *kvm, struct virtio_device *vdev,
 	vdev->ops->set_guest_features(kvm, dev, features);
 }
 
+void virtio_notify_status(struct kvm *kvm, struct virtio_device *vdev,
+			  void *dev, u8 status)
+{
+	u32 ext_status = status;
+
+	vdev->status &= ~VIRTIO_CONFIG_S_MASK;
+	vdev->status |= status;
+
+	/* Add a few hints to help devices */
+	if ((status & VIRTIO_CONFIG_S_DRIVER_OK) &&
+	    !(vdev->status & VIRTIO__STATUS_START)) {
+		vdev->status |= VIRTIO__STATUS_START;
+		ext_status |= VIRTIO__STATUS_START;
+
+	} else if (!status && (vdev->status & VIRTIO__STATUS_START)) {
+		vdev->status &= ~VIRTIO__STATUS_START;
+		ext_status |= VIRTIO__STATUS_STOP;
+	}
+
+	if (vdev->ops->notify_status)
+		vdev->ops->notify_status(kvm, dev, ext_status);
+}
+
 int virtio_init(struct kvm *kvm, void *dev, struct virtio_device *vdev,
 		struct virtio_ops *ops, enum virtio_trans trans,
 		int device_id, int subsys_id, int class)
diff --git a/virtio/mmio.c b/virtio/mmio.c
index 8c0f1cc..7a78fef 100644
--- a/virtio/mmio.c
+++ b/virtio/mmio.c
@@ -162,8 +162,7 @@ static void virtio_mmio_config_out(struct kvm_cpu *vcpu,
 		vmmio->hdr.status = ioport__read32(data);
 		if (!vmmio->hdr.status) /* Sample endianness on reset */
 			vdev->endian = kvm_cpu__get_endianness(vcpu);
-		if (vdev->ops->notify_status)
-			vdev->ops->notify_status(kvm, vmmio->dev, vmmio->hdr.status);
+		virtio_notify_status(kvm, vdev, vmmio->dev, vmmio->hdr.status);
 		break;
 	case VIRTIO_MMIO_GUEST_FEATURES:
 		if (vmmio->hdr.guest_features_sel == 0) {
diff --git a/virtio/net.c b/virtio/net.c
index f95258c..619b545 100644
--- a/virtio/net.c
+++ b/virtio/net.c
@@ -518,7 +518,10 @@ static void set_guest_features(struct kvm *kvm, void *dev, u32 features)
 	conf->status = virtio_host_to_guest_u16(&ndev->vdev, conf->status);
 	conf->max_virtqueue_pairs = virtio_host_to_guest_u16(&ndev->vdev,
 							     conf->max_virtqueue_pairs);
+}
 
+static void virtio_net_start(struct net_dev *ndev)
+{
 	if (ndev->mode == NET_MODE_TAP) {
 		if (!virtio_net__tap_init(ndev))
 			die_perror("TAP device initialized failed because");
@@ -534,6 +537,12 @@ static void set_guest_features(struct kvm *kvm, void *dev, u32 features)
 	}
 }
 
+static void notify_status(struct kvm *kvm, void *dev, u32 status)
+{
+	if (status & VIRTIO__STATUS_START)
+		virtio_net_start(dev);
+}
+
 static bool is_ctrl_vq(struct net_dev *ndev, u32 vq)
 {
 	return vq == (u32)(ndev->queue_pairs * 2);
@@ -683,6 +692,7 @@ static struct virtio_ops net_dev_virtio_ops = {
 	.notify_vq		= notify_vq,
 	.notify_vq_gsi		= notify_vq_gsi,
 	.notify_vq_eventfd	= notify_vq_eventfd,
+	.notify_status		= notify_status,
 };
 
 static void virtio_net__vhost_init(struct kvm *kvm, struct net_dev *ndev)
diff --git a/virtio/pci.c b/virtio/pci.c
index 5a5fc6e..fdeee69 100644
--- a/virtio/pci.c
+++ b/virtio/pci.c
@@ -285,8 +285,7 @@ static bool virtio_pci__io_out(struct ioport *ioport, struct kvm_cpu *vcpu, u16
 		vpci->status = ioport__read8(data);
 		if (!vpci->status) /* Sample endianness on reset */
 			vdev->endian = kvm_cpu__get_endianness(vcpu);
-		if (vdev->ops->notify_status)
-			vdev->ops->notify_status(kvm, vpci->dev, vpci->status);
+		virtio_notify_status(kvm, vdev, vpci->dev, vpci->status);
 		break;
 	default:
 		ret = virtio_pci__specific_io_out(kvm, vdev, port, data, size, offset);
diff --git a/virtio/scsi.c b/virtio/scsi.c
index a429ac8..788bfa2 100644
--- a/virtio/scsi.c
+++ b/virtio/scsi.c
@@ -50,6 +50,10 @@ static void set_guest_features(struct kvm *kvm, void *dev, u32 features)
 	sdev->features = features;
 }
 
+static void notify_status(struct kvm *kvm, void *dev, u32 status)
+{
+}
+
 static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
 		   u32 pfn)
 {
@@ -171,6 +175,7 @@ static struct virtio_ops scsi_dev_virtio_ops = {
 	.get_pfn_vq		= get_pfn_vq,
 	.get_size_vq		= get_size_vq,
 	.set_size_vq		= set_size_vq,
+	.notify_status		= notify_status,
 	.notify_vq		= notify_vq,
 	.notify_vq_gsi		= notify_vq_gsi,
 	.notify_vq_eventfd	= notify_vq_eventfd,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH kvmtool 03/13] virtio: Add get_vq_count() callback
  2018-12-04 11:08 [PATCH kvmtool 00/13] Implement reset of virtio devices Julien Thierry
  2018-12-04 11:08 ` [PATCH kvmtool 01/13] ioeventfd: Fix removal of ioeventfd Julien Thierry
  2018-12-04 11:08 ` [PATCH kvmtool 02/13] virtio: Implement notify_status Julien Thierry
@ 2018-12-04 11:08 ` Julien Thierry
  2018-12-04 11:08 ` [PATCH kvmtool 04/13] virtio: Add get_vq() callback Julien Thierry
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Julien Thierry @ 2018-12-04 11:08 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: will.deacon

From: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>

Modern virtio requires devices to report how many queues they support. Add
an operation to query all devices about their capacities.

Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Signed-off-by: Julien Thierry <julien.thierry@arm.com>
---
 include/kvm/virtio.h | 1 +
 virtio/9p.c          | 6 ++++++
 virtio/balloon.c     | 6 ++++++
 virtio/blk.c         | 6 ++++++
 virtio/console.c     | 6 ++++++
 virtio/net.c         | 8 ++++++++
 virtio/rng.c         | 6 ++++++
 virtio/scsi.c        | 6 ++++++
 8 files changed, 45 insertions(+)

diff --git a/include/kvm/virtio.h b/include/kvm/virtio.h
index 742aa9e..8b6dbd0 100644
--- a/include/kvm/virtio.h
+++ b/include/kvm/virtio.h
@@ -178,6 +178,7 @@ struct virtio_ops {
 	u8 *(*get_config)(struct kvm *kvm, void *dev);
 	u32 (*get_host_features)(struct kvm *kvm, void *dev);
 	void (*set_guest_features)(struct kvm *kvm, void *dev, u32 features);
+	int (*get_vq_count)(struct kvm *kvm, void *dev);
 	int (*init_vq)(struct kvm *kvm, void *dev, u32 vq, u32 page_size,
 		       u32 align, u32 pfn);
 	int (*notify_vq)(struct kvm *kvm, void *dev, u32 vq);
diff --git a/virtio/9p.c b/virtio/9p.c
index 4b93b4c..94f7a8f 100644
--- a/virtio/9p.c
+++ b/virtio/9p.c
@@ -1440,6 +1440,11 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
+static int get_vq_count(struct kvm *kvm, void *dev)
+{
+	return NUM_VIRT_QUEUES;
+}
+
 struct virtio_ops p9_dev_virtio_ops = {
 	.get_config		= get_config,
 	.get_host_features	= get_host_features,
@@ -1450,6 +1455,7 @@ struct virtio_ops p9_dev_virtio_ops = {
 	.get_pfn_vq		= get_pfn_vq,
 	.get_size_vq		= get_size_vq,
 	.set_size_vq		= set_size_vq,
+	.get_vq_count		= get_vq_count,
 };
 
 int virtio_9p_rootdir_parser(const struct option *opt, const char *arg, int unset)
diff --git a/virtio/balloon.c b/virtio/balloon.c
index 871d6e0..2c2e24a 100644
--- a/virtio/balloon.c
+++ b/virtio/balloon.c
@@ -243,6 +243,11 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
+static int get_vq_count(struct kvm *kvm, void *dev)
+{
+	return NUM_VIRT_QUEUES;
+}
+
 struct virtio_ops bln_dev_virtio_ops = {
 	.get_config		= get_config,
 	.get_host_features	= get_host_features,
@@ -253,6 +258,7 @@ struct virtio_ops bln_dev_virtio_ops = {
 	.get_pfn_vq		= get_pfn_vq,
 	.get_size_vq		= get_size_vq,
 	.set_size_vq            = set_size_vq,
+	.get_vq_count		= get_vq_count,
 };
 
 int virtio_bln__init(struct kvm *kvm)
diff --git a/virtio/blk.c b/virtio/blk.c
index db9f4cc..6502b8c 100644
--- a/virtio/blk.c
+++ b/virtio/blk.c
@@ -248,10 +248,16 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
+static int get_vq_count(struct kvm *kvm, void *dev)
+{
+	return NUM_VIRT_QUEUES;
+}
+
 static struct virtio_ops blk_dev_virtio_ops = {
 	.get_config		= get_config,
 	.get_host_features	= get_host_features,
 	.set_guest_features	= set_guest_features,
+	.get_vq_count		= get_vq_count,
 	.init_vq		= init_vq,
 	.notify_status		= notify_status,
 	.notify_vq		= notify_vq,
diff --git a/virtio/console.c b/virtio/console.c
index b9df5c9..c96bc11 100644
--- a/virtio/console.c
+++ b/virtio/console.c
@@ -202,10 +202,16 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
+static int get_vq_count(struct kvm *kvm, void *dev)
+{
+	return VIRTIO_CONSOLE_NUM_QUEUES;
+}
+
 static struct virtio_ops con_dev_virtio_ops = {
 	.get_config		= get_config,
 	.get_host_features	= get_host_features,
 	.set_guest_features	= set_guest_features,
+	.get_vq_count		= get_vq_count,
 	.init_vq		= init_vq,
 	.notify_status		= notify_status,
 	.notify_vq		= notify_vq,
diff --git a/virtio/net.c b/virtio/net.c
index 619b545..3b08aea 100644
--- a/virtio/net.c
+++ b/virtio/net.c
@@ -681,10 +681,18 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
+static int get_vq_count(struct kvm *kvm, void *dev)
+{
+	struct net_dev *ndev = dev;
+
+	return ndev->queue_pairs * 2 + 1;
+}
+
 static struct virtio_ops net_dev_virtio_ops = {
 	.get_config		= get_config,
 	.get_host_features	= get_host_features,
 	.set_guest_features	= set_guest_features,
+	.get_vq_count		= get_vq_count,
 	.init_vq		= init_vq,
 	.get_pfn_vq		= get_pfn_vq,
 	.get_size_vq		= get_size_vq,
diff --git a/virtio/rng.c b/virtio/rng.c
index 9b9e128..fc0e320 100644
--- a/virtio/rng.c
+++ b/virtio/rng.c
@@ -141,6 +141,11 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
+static int get_vq_count(struct kvm *kvm, void *dev)
+{
+	return NUM_VIRT_QUEUES;
+}
+
 static struct virtio_ops rng_dev_virtio_ops = {
 	.get_config		= get_config,
 	.get_host_features	= get_host_features,
@@ -150,6 +155,7 @@ static struct virtio_ops rng_dev_virtio_ops = {
 	.get_pfn_vq		= get_pfn_vq,
 	.get_size_vq		= get_size_vq,
 	.set_size_vq		= set_size_vq,
+	.get_vq_count		= get_vq_count,
 };
 
 int virtio_rng__init(struct kvm *kvm)
diff --git a/virtio/scsi.c b/virtio/scsi.c
index 788bfa2..e21263c 100644
--- a/virtio/scsi.c
+++ b/virtio/scsi.c
@@ -167,6 +167,11 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
+static int get_vq_count(struct kvm *kvm, void *dev)
+{
+	return NUM_VIRT_QUEUES;
+}
+
 static struct virtio_ops scsi_dev_virtio_ops = {
 	.get_config		= get_config,
 	.get_host_features	= get_host_features,
@@ -179,6 +184,7 @@ static struct virtio_ops scsi_dev_virtio_ops = {
 	.notify_vq		= notify_vq,
 	.notify_vq_gsi		= notify_vq_gsi,
 	.notify_vq_eventfd	= notify_vq_eventfd,
+	.get_vq_count		= get_vq_count,
 };
 
 static void virtio_scsi_vhost_init(struct kvm *kvm, struct scsi_dev *sdev)
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH kvmtool 04/13] virtio: Add get_vq() callback
  2018-12-04 11:08 [PATCH kvmtool 00/13] Implement reset of virtio devices Julien Thierry
                   ` (2 preceding siblings ...)
  2018-12-04 11:08 ` [PATCH kvmtool 03/13] virtio: Add get_vq_count() callback Julien Thierry
@ 2018-12-04 11:08 ` Julien Thierry
  2018-12-04 11:08 ` [PATCH kvmtool 05/13] virtio: Add exit_vq() callback Julien Thierry
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Julien Thierry @ 2018-12-04 11:08 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: will.deacon

From: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>

To ease future changes to the core, replace get_pfn_vq() with get_vq().
This way adding new generic operation on virtqueues won't require
modifying every virtio device.

Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Signed-off-by: Julien Thierry <julien.thierry@arm.com>
---
 include/kvm/virtio.h | 2 +-
 virtio/9p.c          | 6 +++---
 virtio/balloon.c     | 6 +++---
 virtio/blk.c         | 6 +++---
 virtio/console.c     | 6 +++---
 virtio/mmio.c        | 7 ++++---
 virtio/net.c         | 6 +++---
 virtio/pci.c         | 5 +++--
 virtio/rng.c         | 6 +++---
 virtio/scsi.c        | 6 +++---
 10 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/include/kvm/virtio.h b/include/kvm/virtio.h
index 8b6dbd0..fd04d21 100644
--- a/include/kvm/virtio.h
+++ b/include/kvm/virtio.h
@@ -182,7 +182,7 @@ struct virtio_ops {
 	int (*init_vq)(struct kvm *kvm, void *dev, u32 vq, u32 page_size,
 		       u32 align, u32 pfn);
 	int (*notify_vq)(struct kvm *kvm, void *dev, u32 vq);
-	int (*get_pfn_vq)(struct kvm *kvm, void *dev, u32 vq);
+	struct virt_queue *(*get_vq)(struct kvm *kvm, void *dev, u32 vq);
 	int (*get_size_vq)(struct kvm *kvm, void *dev, u32 vq);
 	int (*set_size_vq)(struct kvm *kvm, void *dev, u32 vq, int size);
 	void (*notify_vq_gsi)(struct kvm *kvm, void *dev, u32 vq, u32 gsi);
diff --git a/virtio/9p.c b/virtio/9p.c
index 94f7a8f..d9f45cf 100644
--- a/virtio/9p.c
+++ b/virtio/9p.c
@@ -1422,11 +1422,11 @@ static int notify_vq(struct kvm *kvm, void *dev, u32 vq)
 	return 0;
 }
 
-static int get_pfn_vq(struct kvm *kvm, void *dev, u32 vq)
+static struct virt_queue *get_vq(struct kvm *kvm, void *dev, u32 vq)
 {
 	struct p9_dev *p9dev = dev;
 
-	return p9dev->vqs[vq].pfn;
+	return &p9dev->vqs[vq];
 }
 
 static int get_size_vq(struct kvm *kvm, void *dev, u32 vq)
@@ -1452,7 +1452,7 @@ struct virtio_ops p9_dev_virtio_ops = {
 	.init_vq		= init_vq,
 	.notify_status		= notify_status,
 	.notify_vq		= notify_vq,
-	.get_pfn_vq		= get_pfn_vq,
+	.get_vq			= get_vq,
 	.get_size_vq		= get_size_vq,
 	.set_size_vq		= set_size_vq,
 	.get_vq_count		= get_vq_count,
diff --git a/virtio/balloon.c b/virtio/balloon.c
index 2c2e24a..15a9a46 100644
--- a/virtio/balloon.c
+++ b/virtio/balloon.c
@@ -225,11 +225,11 @@ static int notify_vq(struct kvm *kvm, void *dev, u32 vq)
 	return 0;
 }
 
-static int get_pfn_vq(struct kvm *kvm, void *dev, u32 vq)
+static struct virt_queue *get_vq(struct kvm *kvm, void *dev, u32 vq)
 {
 	struct bln_dev *bdev = dev;
 
-	return bdev->vqs[vq].pfn;
+	return &bdev->vqs[vq];
 }
 
 static int get_size_vq(struct kvm *kvm, void *dev, u32 vq)
@@ -255,7 +255,7 @@ struct virtio_ops bln_dev_virtio_ops = {
 	.init_vq		= init_vq,
 	.notify_status		= notify_status,
 	.notify_vq		= notify_vq,
-	.get_pfn_vq		= get_pfn_vq,
+	.get_vq			= get_vq,
 	.get_size_vq		= get_size_vq,
 	.set_size_vq            = set_size_vq,
 	.get_vq_count		= get_vq_count,
diff --git a/virtio/blk.c b/virtio/blk.c
index 6502b8c..6a6b3b7 100644
--- a/virtio/blk.c
+++ b/virtio/blk.c
@@ -229,11 +229,11 @@ static int notify_vq(struct kvm *kvm, void *dev, u32 vq)
 	return 0;
 }
 
-static int get_pfn_vq(struct kvm *kvm, void *dev, u32 vq)
+static struct virt_queue *get_vq(struct kvm *kvm, void *dev, u32 vq)
 {
 	struct blk_dev *bdev = dev;
 
-	return bdev->vqs[vq].pfn;
+	return &bdev->vqs[vq];
 }
 
 static int get_size_vq(struct kvm *kvm, void *dev, u32 vq)
@@ -261,7 +261,7 @@ static struct virtio_ops blk_dev_virtio_ops = {
 	.init_vq		= init_vq,
 	.notify_status		= notify_status,
 	.notify_vq		= notify_vq,
-	.get_pfn_vq		= get_pfn_vq,
+	.get_vq			= get_vq,
 	.get_size_vq		= get_size_vq,
 	.set_size_vq		= set_size_vq,
 };
diff --git a/virtio/console.c b/virtio/console.c
index c96bc11..d2b312c 100644
--- a/virtio/console.c
+++ b/virtio/console.c
@@ -184,11 +184,11 @@ static int notify_vq(struct kvm *kvm, void *dev, u32 vq)
 	return 0;
 }
 
-static int get_pfn_vq(struct kvm *kvm, void *dev, u32 vq)
+static struct virt_queue *get_vq(struct kvm *kvm, void *dev, u32 vq)
 {
 	struct con_dev *cdev = dev;
 
-	return cdev->vqs[vq].pfn;
+	return &cdev->vqs[vq];
 }
 
 static int get_size_vq(struct kvm *kvm, void *dev, u32 vq)
@@ -215,7 +215,7 @@ static struct virtio_ops con_dev_virtio_ops = {
 	.init_vq		= init_vq,
 	.notify_status		= notify_status,
 	.notify_vq		= notify_vq,
-	.get_pfn_vq		= get_pfn_vq,
+	.get_vq			= get_vq,
 	.get_size_vq		= get_size_vq,
 	.set_size_vq		= set_size_vq,
 };
diff --git a/virtio/mmio.c b/virtio/mmio.c
index 7a78fef..70f767e 100644
--- a/virtio/mmio.c
+++ b/virtio/mmio.c
@@ -111,6 +111,7 @@ static void virtio_mmio_config_in(struct kvm_cpu *vcpu,
 				  struct virtio_device *vdev)
 {
 	struct virtio_mmio *vmmio = vdev->virtio;
+	struct virt_queue *vq;
 	u32 val = 0;
 
 	switch (addr) {
@@ -129,9 +130,9 @@ static void virtio_mmio_config_in(struct kvm_cpu *vcpu,
 		ioport__write32(data, val);
 		break;
 	case VIRTIO_MMIO_QUEUE_PFN:
-		val = vdev->ops->get_pfn_vq(vmmio->kvm, vmmio->dev,
-					    vmmio->hdr.queue_sel);
-		ioport__write32(data, val);
+		vq = vdev->ops->get_vq(vmmio->kvm, vmmio->dev,
+				       vmmio->hdr.queue_sel);
+		ioport__write32(data, vq->pfn);
 		break;
 	case VIRTIO_MMIO_QUEUE_NUM_MAX:
 		val = vdev->ops->get_size_vq(vmmio->kvm, vmmio->dev,
diff --git a/virtio/net.c b/virtio/net.c
index 3b08aea..d65d04e 100644
--- a/virtio/net.c
+++ b/virtio/net.c
@@ -662,11 +662,11 @@ static int notify_vq(struct kvm *kvm, void *dev, u32 vq)
 	return 0;
 }
 
-static int get_pfn_vq(struct kvm *kvm, void *dev, u32 vq)
+static struct virt_queue *get_vq(struct kvm *kvm, void *dev, u32 vq)
 {
 	struct net_dev *ndev = dev;
 
-	return ndev->vqs[vq].pfn;
+	return &ndev->vqs[vq];
 }
 
 static int get_size_vq(struct kvm *kvm, void *dev, u32 vq)
@@ -694,7 +694,7 @@ static struct virtio_ops net_dev_virtio_ops = {
 	.set_guest_features	= set_guest_features,
 	.get_vq_count		= get_vq_count,
 	.init_vq		= init_vq,
-	.get_pfn_vq		= get_pfn_vq,
+	.get_vq			= get_vq,
 	.get_size_vq		= get_size_vq,
 	.set_size_vq		= set_size_vq,
 	.notify_vq		= notify_vq,
diff --git a/virtio/pci.c b/virtio/pci.c
index fdeee69..8add770 100644
--- a/virtio/pci.c
+++ b/virtio/pci.c
@@ -113,6 +113,7 @@ static bool virtio_pci__io_in(struct ioport *ioport, struct kvm_cpu *vcpu, u16 p
 	bool ret = true;
 	struct virtio_device *vdev;
 	struct virtio_pci *vpci;
+	struct virt_queue *vq;
 	struct kvm *kvm;
 	u32 val;
 
@@ -127,8 +128,8 @@ static bool virtio_pci__io_in(struct ioport *ioport, struct kvm_cpu *vcpu, u16 p
 		ioport__write32(data, val);
 		break;
 	case VIRTIO_PCI_QUEUE_PFN:
-		val = vdev->ops->get_pfn_vq(kvm, vpci->dev, vpci->queue_selector);
-		ioport__write32(data, val);
+		vq = vdev->ops->get_vq(kvm, vpci->dev, vpci->queue_selector);
+		ioport__write32(data, vq->pfn);
 		break;
 	case VIRTIO_PCI_QUEUE_NUM:
 		val = vdev->ops->get_size_vq(kvm, vpci->dev, vpci->queue_selector);
diff --git a/virtio/rng.c b/virtio/rng.c
index fc0e320..9dd757b 100644
--- a/virtio/rng.c
+++ b/virtio/rng.c
@@ -123,11 +123,11 @@ static int notify_vq(struct kvm *kvm, void *dev, u32 vq)
 	return 0;
 }
 
-static int get_pfn_vq(struct kvm *kvm, void *dev, u32 vq)
+static struct virt_queue *get_vq(struct kvm *kvm, void *dev, u32 vq)
 {
 	struct rng_dev *rdev = dev;
 
-	return rdev->vqs[vq].pfn;
+	return &rdev->vqs[vq];
 }
 
 static int get_size_vq(struct kvm *kvm, void *dev, u32 vq)
@@ -152,7 +152,7 @@ static struct virtio_ops rng_dev_virtio_ops = {
 	.set_guest_features	= set_guest_features,
 	.init_vq		= init_vq,
 	.notify_vq		= notify_vq,
-	.get_pfn_vq		= get_pfn_vq,
+	.get_vq			= get_vq,
 	.get_size_vq		= get_size_vq,
 	.set_size_vq		= set_size_vq,
 	.get_vq_count		= get_vq_count,
diff --git a/virtio/scsi.c b/virtio/scsi.c
index e21263c..c8400b6 100644
--- a/virtio/scsi.c
+++ b/virtio/scsi.c
@@ -150,11 +150,11 @@ static int notify_vq(struct kvm *kvm, void *dev, u32 vq)
 	return 0;
 }
 
-static int get_pfn_vq(struct kvm *kvm, void *dev, u32 vq)
+static struct virt_queue *get_vq(struct kvm *kvm, void *dev, u32 vq)
 {
 	struct scsi_dev *sdev = dev;
 
-	return sdev->vqs[vq].pfn;
+	return &sdev->vqs[vq];
 }
 
 static int get_size_vq(struct kvm *kvm, void *dev, u32 vq)
@@ -177,7 +177,7 @@ static struct virtio_ops scsi_dev_virtio_ops = {
 	.get_host_features	= get_host_features,
 	.set_guest_features	= set_guest_features,
 	.init_vq		= init_vq,
-	.get_pfn_vq		= get_pfn_vq,
+	.get_vq			= get_vq,
 	.get_size_vq		= get_size_vq,
 	.set_size_vq		= set_size_vq,
 	.notify_status		= notify_status,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH kvmtool 05/13] virtio: Add exit_vq() callback
  2018-12-04 11:08 [PATCH kvmtool 00/13] Implement reset of virtio devices Julien Thierry
                   ` (3 preceding siblings ...)
  2018-12-04 11:08 ` [PATCH kvmtool 04/13] virtio: Add get_vq() callback Julien Thierry
@ 2018-12-04 11:08 ` Julien Thierry
  2018-12-04 11:08 ` [PATCH kvmtool 06/13] virtio: Add reset() callback Julien Thierry
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Julien Thierry @ 2018-12-04 11:08 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: will.deacon

From: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>

Virtio allows to reset individual virtqueues. For legacy devices, it's
done by writing an address of 0 into the PFN register. Modern devices have
an "enable" register. Add an exit_vq() callback to all devices. A lot more
work is required by each device to clean up their virtqueue state, and by
the core to reset things like MSI routes and ioeventfds.

Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Signed-off-by: Julien Thierry <julien.thierry@arm.com>
---
 include/kvm/virtio.h |  5 +++++
 virtio/core.c        | 10 ++++++++++
 virtio/mmio.c        | 26 ++++++++++++++++++++------
 virtio/pci.c         | 23 +++++++++++++++++++----
 4 files changed, 54 insertions(+), 10 deletions(-)

diff --git a/include/kvm/virtio.h b/include/kvm/virtio.h
index fd04d21..20399f4 100644
--- a/include/kvm/virtio.h
+++ b/include/kvm/virtio.h
@@ -52,6 +52,7 @@ struct virt_queue {
 	u16		last_used_signalled;
 	u16		endian;
 	bool		use_event_idx;
+	bool		enabled;
 };
 
 /*
@@ -181,6 +182,7 @@ struct virtio_ops {
 	int (*get_vq_count)(struct kvm *kvm, void *dev);
 	int (*init_vq)(struct kvm *kvm, void *dev, u32 vq, u32 page_size,
 		       u32 align, u32 pfn);
+	void (*exit_vq)(struct kvm *kvm, void *dev, u32 vq);
 	int (*notify_vq)(struct kvm *kvm, void *dev, u32 vq);
 	struct virt_queue *(*get_vq)(struct kvm *kvm, void *dev, u32 vq);
 	int (*get_size_vq)(struct kvm *kvm, void *dev, u32 vq);
@@ -211,8 +213,11 @@ static inline void virtio_init_device_vq(struct virtio_device *vdev,
 {
 	vq->endian = vdev->endian;
 	vq->use_event_idx = (vdev->features & VIRTIO_RING_F_EVENT_IDX);
+	vq->enabled = true;
 }
 
+void virtio_exit_vq(struct kvm *kvm, struct virtio_device *vdev, void *dev,
+		    int num);
 void virtio_set_guest_features(struct kvm *kvm, struct virtio_device *vdev,
 			       void *dev, u32 features);
 void virtio_notify_status(struct kvm *kvm, struct virtio_device *vdev,
diff --git a/virtio/core.c b/virtio/core.c
index 9114f27..67d4114 100644
--- a/virtio/core.c
+++ b/virtio/core.c
@@ -166,6 +166,16 @@ u16 virt_queue__get_inout_iov(struct kvm *kvm, struct virt_queue *queue,
 	return head;
 }
 
+void virtio_exit_vq(struct kvm *kvm, struct virtio_device *vdev,
+			   void *dev, int num)
+{
+	struct virt_queue *vq = vdev->ops->get_vq(kvm, dev, num);
+
+	if (vq->enabled && vdev->ops->exit_vq)
+		vdev->ops->exit_vq(kvm, dev, num);
+	memset(vq, 0, sizeof(*vq));
+}
+
 int virtio__get_dev_specific_field(int offset, bool msix, u32 *config_off)
 {
 	if (msix) {
diff --git a/virtio/mmio.c b/virtio/mmio.c
index 70f767e..4b8c20e 100644
--- a/virtio/mmio.c
+++ b/virtio/mmio.c
@@ -79,6 +79,15 @@ int virtio_mmio_signal_vq(struct kvm *kvm, struct virtio_device *vdev, u32 vq)
 	return 0;
 }
 
+static void virtio_mmio_exit_vq(struct kvm *kvm, struct virtio_device *vdev,
+				int vq)
+{
+	struct virtio_mmio *vmmio = vdev->virtio;
+
+	ioeventfd__del_event(vmmio->addr + VIRTIO_MMIO_QUEUE_NOTIFY, vq);
+	virtio_exit_vq(kvm, vdev, vmmio->dev, vq);
+}
+
 int virtio_mmio_signal_config(struct kvm *kvm, struct virtio_device *vdev)
 {
 	struct virtio_mmio *vmmio = vdev->virtio;
@@ -188,12 +197,17 @@ static void virtio_mmio_config_out(struct kvm_cpu *vcpu,
 		break;
 	case VIRTIO_MMIO_QUEUE_PFN:
 		val = ioport__read32(data);
-		virtio_mmio_init_ioeventfd(vmmio->kvm, vdev, vmmio->hdr.queue_sel);
-		vdev->ops->init_vq(vmmio->kvm, vmmio->dev,
-				   vmmio->hdr.queue_sel,
-				   vmmio->hdr.guest_page_size,
-				   vmmio->hdr.queue_align,
-				   val);
+		if (val) {
+			virtio_mmio_init_ioeventfd(vmmio->kvm, vdev,
+						   vmmio->hdr.queue_sel);
+			vdev->ops->init_vq(vmmio->kvm, vmmio->dev,
+					   vmmio->hdr.queue_sel,
+					   vmmio->hdr.guest_page_size,
+					   vmmio->hdr.queue_align,
+					   val);
+		} else {
+			virtio_mmio_exit_vq(kvm, vdev, vmmio->hdr.queue_sel);
+		}
 		break;
 	case VIRTIO_MMIO_QUEUE_NOTIFY:
 		val = ioport__read32(data);
diff --git a/virtio/pci.c b/virtio/pci.c
index 8add770..2da2d3f 100644
--- a/virtio/pci.c
+++ b/virtio/pci.c
@@ -72,6 +72,16 @@ free_ioport_evt:
 	return r;
 }
 
+static void virtio_pci_exit_vq(struct kvm *kvm, struct virtio_device *vdev,
+			       int vq)
+{
+	struct virtio_pci *vpci = vdev->virtio;
+
+	ioeventfd__del_event(vpci->port_addr + VIRTIO_PCI_QUEUE_NOTIFY, vq);
+	ioeventfd__del_event(vpci->mmio_addr + VIRTIO_PCI_QUEUE_NOTIFY, vq);
+	virtio_exit_vq(kvm, vdev, vpci->dev, vq);
+}
+
 static inline bool virtio_pci__msix_enabled(struct virtio_pci *vpci)
 {
 	return vpci->pci_hdr.msix.ctrl & cpu_to_le16(PCI_MSIX_FLAGS_ENABLE);
@@ -270,10 +280,15 @@ static bool virtio_pci__io_out(struct ioport *ioport, struct kvm_cpu *vcpu, u16
 		break;
 	case VIRTIO_PCI_QUEUE_PFN:
 		val = ioport__read32(data);
-		virtio_pci__init_ioeventfd(kvm, vdev, vpci->queue_selector);
-		vdev->ops->init_vq(kvm, vpci->dev, vpci->queue_selector,
-				   1 << VIRTIO_PCI_QUEUE_ADDR_SHIFT,
-				   VIRTIO_PCI_VRING_ALIGN, val);
+		if (val) {
+			virtio_pci__init_ioeventfd(kvm, vdev,
+						   vpci->queue_selector);
+			vdev->ops->init_vq(kvm, vpci->dev, vpci->queue_selector,
+					   1 << VIRTIO_PCI_QUEUE_ADDR_SHIFT,
+					   VIRTIO_PCI_VRING_ALIGN, val);
+		} else {
+			virtio_pci_exit_vq(kvm, vdev, vpci->queue_selector);
+		}
 		break;
 	case VIRTIO_PCI_QUEUE_SEL:
 		vpci->queue_selector = ioport__read16(data);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH kvmtool 06/13] virtio: Add reset() callback
  2018-12-04 11:08 [PATCH kvmtool 00/13] Implement reset of virtio devices Julien Thierry
                   ` (4 preceding siblings ...)
  2018-12-04 11:08 ` [PATCH kvmtool 05/13] virtio: Add exit_vq() callback Julien Thierry
@ 2018-12-04 11:08 ` Julien Thierry
  2018-12-04 11:08 ` [PATCH kvmtool 07/13] net/uip: Add exit function Julien Thierry
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Julien Thierry @ 2018-12-04 11:08 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: will.deacon

From: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>

When the guest writes a status of 0, the device should be reset. Add a
reset() callback for the transport layer to reset all queues and their
ioeventfd.

Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Signed-off-by: Julien Thierry <julien.thierry@arm.com>
---
 include/kvm/virtio-mmio.h |  1 +
 include/kvm/virtio-pci.h  |  1 +
 include/kvm/virtio.h      |  1 +
 virtio/core.c             |  9 +++++++++
 virtio/mmio.c             | 16 ++++++++++++----
 virtio/pci.c              | 20 +++++++++++++-------
 6 files changed, 37 insertions(+), 11 deletions(-)

diff --git a/include/kvm/virtio-mmio.h b/include/kvm/virtio-mmio.h
index 835f421..0528947 100644
--- a/include/kvm/virtio-mmio.h
+++ b/include/kvm/virtio-mmio.h
@@ -54,6 +54,7 @@ struct virtio_mmio {
 int virtio_mmio_signal_vq(struct kvm *kvm, struct virtio_device *vdev, u32 vq);
 int virtio_mmio_signal_config(struct kvm *kvm, struct virtio_device *vdev);
 int virtio_mmio_exit(struct kvm *kvm, struct virtio_device *vdev);
+int virtio_mmio_reset(struct kvm *kvm, struct virtio_device *vdev);
 int virtio_mmio_init(struct kvm *kvm, void *dev, struct virtio_device *vdev,
 		      int device_id, int subsys_id, int class);
 void virtio_mmio_assign_irq(struct device_header *dev_hdr);
diff --git a/include/kvm/virtio-pci.h b/include/kvm/virtio-pci.h
index b70cadd..278a259 100644
--- a/include/kvm/virtio-pci.h
+++ b/include/kvm/virtio-pci.h
@@ -55,6 +55,7 @@ struct virtio_pci {
 int virtio_pci__signal_vq(struct kvm *kvm, struct virtio_device *vdev, u32 vq);
 int virtio_pci__signal_config(struct kvm *kvm, struct virtio_device *vdev);
 int virtio_pci__exit(struct kvm *kvm, struct virtio_device *vdev);
+int virtio_pci__reset(struct kvm *kvm, struct virtio_device *vdev);
 int virtio_pci__init(struct kvm *kvm, void *dev, struct virtio_device *vdev,
 		     int device_id, int subsys_id, int class);
 
diff --git a/include/kvm/virtio.h b/include/kvm/virtio.h
index 20399f4..5678695 100644
--- a/include/kvm/virtio.h
+++ b/include/kvm/virtio.h
@@ -195,6 +195,7 @@ struct virtio_ops {
 	int (*init)(struct kvm *kvm, void *dev, struct virtio_device *vdev,
 		    int device_id, int subsys_id, int class);
 	int (*exit)(struct kvm *kvm, struct virtio_device *vdev);
+	int (*reset)(struct kvm *kvm, struct virtio_device *vdev);
 };
 
 int virtio_init(struct kvm *kvm, void *dev, struct virtio_device *vdev,
diff --git a/virtio/core.c b/virtio/core.c
index 67d4114..e10ec36 100644
--- a/virtio/core.c
+++ b/virtio/core.c
@@ -241,6 +241,13 @@ void virtio_notify_status(struct kvm *kvm, struct virtio_device *vdev,
 	} else if (!status && (vdev->status & VIRTIO__STATUS_START)) {
 		vdev->status &= ~VIRTIO__STATUS_START;
 		ext_status |= VIRTIO__STATUS_STOP;
+
+		/*
+		 * Reset virtqueues and stop all traffic now, so that the device
+		 * can safely reset the backend in notify_status().
+		 */
+		if (ext_status & VIRTIO__STATUS_STOP)
+			vdev->ops->reset(kvm, vdev);
 	}
 
 	if (vdev->ops->notify_status)
@@ -264,6 +271,7 @@ int virtio_init(struct kvm *kvm, void *dev, struct virtio_device *vdev,
 		vdev->ops->signal_config	= virtio_pci__signal_config;
 		vdev->ops->init			= virtio_pci__init;
 		vdev->ops->exit			= virtio_pci__exit;
+		vdev->ops->reset		= virtio_pci__reset;
 		vdev->ops->init(kvm, dev, vdev, device_id, subsys_id, class);
 		break;
 	case VIRTIO_MMIO:
@@ -276,6 +284,7 @@ int virtio_init(struct kvm *kvm, void *dev, struct virtio_device *vdev,
 		vdev->ops->signal_config	= virtio_mmio_signal_config;
 		vdev->ops->init			= virtio_mmio_init;
 		vdev->ops->exit			= virtio_mmio_exit;
+		vdev->ops->reset		= virtio_mmio_reset;
 		vdev->ops->init(kvm, dev, vdev, device_id, subsys_id, class);
 		break;
 	default:
diff --git a/virtio/mmio.c b/virtio/mmio.c
index 4b8c20e..eff147a 100644
--- a/virtio/mmio.c
+++ b/virtio/mmio.c
@@ -326,15 +326,23 @@ int virtio_mmio_init(struct kvm *kvm, void *dev, struct virtio_device *vdev,
 	return 0;
 }
 
+int virtio_mmio_reset(struct kvm *kvm, struct virtio_device *vdev)
+{
+	int vq;
+	struct virtio_mmio *vmmio = vdev->virtio;
+
+	for (vq = 0; vq < vdev->ops->get_vq_count(kvm, vmmio->dev); vq++)
+		virtio_mmio_exit_vq(kvm, vdev, vq);
+
+	return 0;
+}
+
 int virtio_mmio_exit(struct kvm *kvm, struct virtio_device *vdev)
 {
 	struct virtio_mmio *vmmio = vdev->virtio;
-	int i;
 
+	virtio_mmio_reset(kvm, vdev);
 	kvm__deregister_mmio(kvm, vmmio->addr);
 
-	for (i = 0; i < VIRTIO_MMIO_MAX_VQ; i++)
-		ioeventfd__del_event(vmmio->addr + VIRTIO_MMIO_QUEUE_NOTIFY, i);
-
 	return 0;
 }
diff --git a/virtio/pci.c b/virtio/pci.c
index 2da2d3f..99653ca 100644
--- a/virtio/pci.c
+++ b/virtio/pci.c
@@ -77,8 +77,8 @@ static void virtio_pci_exit_vq(struct kvm *kvm, struct virtio_device *vdev,
 {
 	struct virtio_pci *vpci = vdev->virtio;
 
-	ioeventfd__del_event(vpci->port_addr + VIRTIO_PCI_QUEUE_NOTIFY, vq);
 	ioeventfd__del_event(vpci->mmio_addr + VIRTIO_PCI_QUEUE_NOTIFY, vq);
+	ioeventfd__del_event(vpci->port_addr + VIRTIO_PCI_QUEUE_NOTIFY, vq);
 	virtio_exit_vq(kvm, vdev, vpci->dev, vq);
 }
 
@@ -522,19 +522,25 @@ free_ioport:
 	return r;
 }
 
+int virtio_pci__reset(struct kvm *kvm, struct virtio_device *vdev)
+{
+	int vq;
+	struct virtio_pci *vpci = vdev->virtio;
+
+	for (vq = 0; vq < vdev->ops->get_vq_count(kvm, vpci->dev); vq++)
+		virtio_pci_exit_vq(kvm, vdev, vq);
+
+	return 0;
+}
+
 int virtio_pci__exit(struct kvm *kvm, struct virtio_device *vdev)
 {
 	struct virtio_pci *vpci = vdev->virtio;
-	int i;
 
+	virtio_pci__reset(kvm, vdev);
 	kvm__deregister_mmio(kvm, vpci->mmio_addr);
 	kvm__deregister_mmio(kvm, vpci->msix_io_block);
 	ioport__unregister(kvm, vpci->port_addr);
 
-	for (i = 0; i < VIRTIO_PCI_MAX_VQ; i++) {
-		ioeventfd__del_event(vpci->port_addr + VIRTIO_PCI_QUEUE_NOTIFY, i);
-		ioeventfd__del_event(vpci->mmio_addr + VIRTIO_PCI_QUEUE_NOTIFY, i);
-	}
-
 	return 0;
 }
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH kvmtool 07/13] net/uip: Add exit function
  2018-12-04 11:08 [PATCH kvmtool 00/13] Implement reset of virtio devices Julien Thierry
                   ` (5 preceding siblings ...)
  2018-12-04 11:08 ` [PATCH kvmtool 06/13] virtio: Add reset() callback Julien Thierry
@ 2018-12-04 11:08 ` Julien Thierry
  2018-12-04 11:08 ` [PATCH kvmtool 08/13] virtio/net: Clean virtqueue state Julien Thierry
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Julien Thierry @ 2018-12-04 11:08 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: will.deacon

From: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>

When resetting the virtio-net queues, the UIP state needs to be reset as
well. Stop all threads (one per TCP stream and UDP connection) and free
memory.

Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Signed-off-by: Julien Thierry <julien.thierry@arm.com>
---
 include/kvm/uip.h |  6 ++++++
 net/uip/core.c    | 17 +++++++++++++++++
 net/uip/dhcp.c    |  6 ++++++
 net/uip/tcp.c     | 54 ++++++++++++++++++++++++++++++++++++++++++++----------
 net/uip/udp.c     | 41 +++++++++++++++++++++++++++++++++++------
 5 files changed, 108 insertions(+), 16 deletions(-)

diff --git a/include/kvm/uip.h b/include/kvm/uip.h
index a9a8d85..efa508a 100644
--- a/include/kvm/uip.h
+++ b/include/kvm/uip.h
@@ -196,6 +196,7 @@ struct uip_info {
 	struct list_head buf_head;
 	struct mutex buf_lock;
 	pthread_t udp_thread;
+	u8 *udp_buf;
 	int udp_epollfd;
 	int buf_free_nr;
 	int buf_used_nr;
@@ -249,6 +250,7 @@ struct uip_tcp_socket {
 	int read_done;
 	u32 dip, sip;
 	u8 *payload;
+	u8 *buf;
 	int fd;
 };
 
@@ -336,6 +338,9 @@ int uip_tx(struct iovec *iov, u16 out, struct uip_info *info);
 int uip_rx(struct iovec *iov, u16 in, struct uip_info *info);
 void uip_static_init(struct uip_info *info);
 int uip_init(struct uip_info *info);
+void uip_exit(struct uip_info *info);
+void uip_tcp_exit(struct uip_info *info);
+void uip_udp_exit(struct uip_info *info);
 
 int uip_tx_do_ipv4_udp_dhcp(struct uip_tx_arg *arg);
 int uip_tx_do_ipv4_icmp(struct uip_tx_arg *arg);
@@ -359,4 +364,5 @@ int uip_udp_make_pkg(struct uip_info *info, struct uip_udp_socket *sk, struct ui
 bool uip_udp_is_dhcp(struct uip_udp *udp);
 
 int uip_dhcp_get_dns(struct uip_info *info);
+void uip_dhcp_exit(struct uip_info *info);
 #endif /* KVM__UIP_H */
diff --git a/net/uip/core.c b/net/uip/core.c
index e860f3a..977b9b0 100644
--- a/net/uip/core.c
+++ b/net/uip/core.c
@@ -154,3 +154,20 @@ int uip_init(struct uip_info *info)
 
 	return 0;
 }
+
+void uip_exit(struct uip_info *info)
+{
+	struct uip_buf *buf, *next;
+
+	uip_udp_exit(info);
+	uip_tcp_exit(info);
+	uip_dhcp_exit(info);
+
+	list_for_each_entry_safe(buf, next, &info->buf_head, list) {
+		free(buf->vnet);
+		free(buf->eth);
+		list_del(&buf->list);
+		free(buf);
+	}
+	uip_static_init(info);
+}
diff --git a/net/uip/dhcp.c b/net/uip/dhcp.c
index b17d352..8f01300 100644
--- a/net/uip/dhcp.c
+++ b/net/uip/dhcp.c
@@ -200,3 +200,9 @@ int uip_tx_do_ipv4_udp_dhcp(struct uip_tx_arg *arg)
 
 	return 0;
 }
+
+void uip_dhcp_exit(struct uip_info *info)
+{
+	free(info->domain_name);
+	info->domain_name = NULL;
+}
diff --git a/net/uip/tcp.c b/net/uip/tcp.c
index 3c30ade..8e0ad52 100644
--- a/net/uip/tcp.c
+++ b/net/uip/tcp.c
@@ -18,6 +18,7 @@ static int uip_tcp_socket_close(struct uip_tcp_socket *sk, int how)
 		list_del(&sk->list);
 		mutex_unlock(sk->lock);
 
+		free(sk->buf);
 		free(sk);
 	}
 
@@ -94,6 +95,24 @@ static struct uip_tcp_socket *uip_tcp_socket_alloc(struct uip_tx_arg *arg, u32 s
 	return sk;
 }
 
+/* Caller holds the sk lock */
+static void uip_tcp_socket_free(struct uip_tcp_socket *sk)
+{
+	/*
+	 * Here we assume that the virtqueues are already inactive so we don't
+	 * race with uip_tx_do_ipv4_tcp. We are racing with
+	 * uip_tcp_socket_thread though, but holding the sk lock ensures that it
+	 * cannot free data concurrently.
+	 */
+	if (sk->thread) {
+		pthread_cancel(sk->thread);
+		pthread_join(sk->thread, NULL);
+	}
+
+	sk->write_done = sk->read_done = 1;
+	uip_tcp_socket_close(sk, SHUT_RDWR);
+}
+
 static int uip_tcp_payload_send(struct uip_tcp_socket *sk, u8 flag, u16 payload_len)
 {
 	struct uip_info *info;
@@ -175,20 +194,16 @@ static void *uip_tcp_socket_thread(void *p)
 {
 	struct uip_tcp_socket *sk;
 	int len, left, ret;
-	u8 *payload, *pos;
+	u8 *pos;
 
 	kvm__set_thread_name("uip-tcp");
 
 	sk = p;
 
-	payload = malloc(UIP_MAX_TCP_PAYLOAD);
-	if (!payload)
-		goto out;
-
 	while (1) {
-		pos = payload;
+		pos = sk->buf;
 
-		ret = read(sk->fd, payload, UIP_MAX_TCP_PAYLOAD);
+		ret = read(sk->fd, sk->buf, UIP_MAX_TCP_PAYLOAD);
 
 		if (ret <= 0 || ret > UIP_MAX_TCP_PAYLOAD)
 			goto out;
@@ -224,7 +239,6 @@ out:
 
 	sk->read_done = 1;
 
-	free(payload);
 	pthread_exit(NULL);
 
 	return NULL;
@@ -232,8 +246,18 @@ out:
 
 static int uip_tcp_socket_receive(struct uip_tcp_socket *sk)
 {
-	if (sk->thread == 0)
-		return pthread_create(&sk->thread, NULL, uip_tcp_socket_thread, (void *)sk);
+	int ret;
+
+	if (sk->thread == 0) {
+		sk->buf = malloc(UIP_MAX_TCP_PAYLOAD);
+		if (!sk->buf)
+			return -ENOMEM;
+		ret = pthread_create(&sk->thread, NULL, uip_tcp_socket_thread,
+				     (void *)sk);
+		if (ret)
+			free(sk->buf);
+		return ret;
+	}
 
 	return 0;
 }
@@ -346,3 +370,13 @@ int uip_tx_do_ipv4_tcp(struct uip_tx_arg *arg)
 out:
 	return 0;
 }
+
+void uip_tcp_exit(struct uip_info *info)
+{
+	struct uip_tcp_socket *sk, *next;
+
+	mutex_lock(&info->tcp_socket_lock);
+	list_for_each_entry_safe(sk, next, &info->tcp_socket_head, list)
+		uip_tcp_socket_free(sk);
+	mutex_unlock(&info->tcp_socket_lock);
+}
diff --git a/net/uip/udp.c b/net/uip/udp.c
index dd288c5..d2580d0 100644
--- a/net/uip/udp.c
+++ b/net/uip/udp.c
@@ -164,10 +164,7 @@ static void *uip_udp_socket_thread(void *p)
 	kvm__set_thread_name("uip-udp");
 
 	info = p;
-
-	do {
-		payload = malloc(UIP_MAX_UDP_PAYLOAD);
-	} while (!payload);
+	payload = info->udp_buf;
 
 	while (1) {
 		nfds = epoll_wait(info->udp_epollfd, events, UIP_UDP_MAX_EVENTS, -1);
@@ -196,7 +193,11 @@ static void *uip_udp_socket_thread(void *p)
 		}
 	}
 
-	free(payload);
+	mutex_lock(&info->udp_socket_lock);
+	free(info->udp_buf);
+	info->udp_buf = NULL;
+	mutex_unlock(&info->udp_socket_lock);
+
 	pthread_exit(NULL);
 	return NULL;
 }
@@ -232,8 +233,36 @@ int uip_tx_do_ipv4_udp(struct uip_tx_arg *arg)
 	if (ret)
 		return -1;
 
-	if (!info->udp_thread)
+	if (!info->udp_thread) {
+		info->udp_buf = malloc(UIP_MAX_UDP_PAYLOAD);
+		if (!info->udp_buf)
+			return -1;
+
 		pthread_create(&info->udp_thread, NULL, uip_udp_socket_thread, (void *)info);
+	}
 
 	return 0;
 }
+
+void uip_udp_exit(struct uip_info *info)
+{
+	struct uip_udp_socket *sk, *next;
+
+	mutex_lock(&info->udp_socket_lock);
+	if (info->udp_thread) {
+		pthread_cancel(info->udp_thread);
+		pthread_join(info->udp_thread, NULL);
+		info->udp_thread = 0;
+		free(info->udp_buf);
+	}
+	if (info->udp_epollfd > 0) {
+		close(info->udp_epollfd);
+		info->udp_epollfd = 0;
+	}
+
+	list_for_each_entry_safe(sk, next, &info->udp_socket_head, list) {
+		close(sk->fd);
+		free(sk);
+	}
+	mutex_unlock(&info->udp_socket_lock);
+}
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH kvmtool 08/13] virtio/net: Clean virtqueue state
  2018-12-04 11:08 [PATCH kvmtool 00/13] Implement reset of virtio devices Julien Thierry
                   ` (6 preceding siblings ...)
  2018-12-04 11:08 ` [PATCH kvmtool 07/13] net/uip: Add exit function Julien Thierry
@ 2018-12-04 11:08 ` Julien Thierry
  2018-12-04 11:08 ` [PATCH kvmtool 09/13] virtio/net: Implement device and virtqueue reset Julien Thierry
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Julien Thierry @ 2018-12-04 11:08 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: will.deacon

From: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>

Currently the virtqueue state is mixed with the netdev state. Move it to a
separate structure.

Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Signed-off-by: Julien Thierry <julien.thierry@arm.com>
---
 virtio/net.c | 110 +++++++++++++++++++++++++++++++----------------------------
 1 file changed, 57 insertions(+), 53 deletions(-)

diff --git a/virtio/net.c b/virtio/net.c
index d65d04e..ef8e226 100644
--- a/virtio/net.c
+++ b/virtio/net.c
@@ -36,18 +36,23 @@ struct net_dev_operations {
 	int (*tx)(struct iovec *iov, u16 in, struct net_dev *ndev);
 };
 
+struct net_dev_queue {
+	int				id;
+	struct net_dev			*ndev;
+	struct virt_queue		vq;
+	pthread_t			thread;
+	struct mutex			lock;
+	pthread_cond_t			cond;
+};
+
 struct net_dev {
 	struct mutex			mutex;
 	struct virtio_device		vdev;
 	struct list_head		list;
 
-	struct virt_queue		vqs[VIRTIO_NET_NUM_QUEUES * 2 + 1];
+	struct net_dev_queue		queues[VIRTIO_NET_NUM_QUEUES * 2 + 1];
 	struct virtio_net_config	config;
-	u32				features, rx_vqs, tx_vqs, queue_pairs;
-
-	pthread_t			io_thread[VIRTIO_NET_NUM_QUEUES * 2 + 1];
-	struct mutex			io_lock[VIRTIO_NET_NUM_QUEUES * 2 + 1];
-	pthread_cond_t			io_cond[VIRTIO_NET_NUM_QUEUES * 2 + 1];
+	u32				features, queue_pairs;
 
 	int				vhost_fd;
 	int				tap_fd;
@@ -92,28 +97,22 @@ static void virtio_net_fix_rx_hdr(struct virtio_net_hdr *hdr, struct net_dev *nd
 static void *virtio_net_rx_thread(void *p)
 {
 	struct iovec iov[VIRTIO_NET_QUEUE_SIZE];
-	struct virt_queue *vq;
+	struct net_dev_queue *queue = p;
+	struct virt_queue *vq = &queue->vq;
+	struct net_dev *ndev = queue->ndev;
 	struct kvm *kvm;
-	struct net_dev *ndev = p;
 	u16 out, in;
 	u16 head;
 	int len, copied;
-	u32 id;
-
-	mutex_lock(&ndev->mutex);
-	id = ndev->rx_vqs++ * 2;
-	mutex_unlock(&ndev->mutex);
 
 	kvm__set_thread_name("virtio-net-rx");
 
 	kvm = ndev->kvm;
-	vq = &ndev->vqs[id];
-
 	while (1) {
-		mutex_lock(&ndev->io_lock[id]);
+		mutex_lock(&queue->lock);
 		if (!virt_queue__available(vq))
-			pthread_cond_wait(&ndev->io_cond[id], &ndev->io_lock[id].mutex);
-		mutex_unlock(&ndev->io_lock[id]);
+			pthread_cond_wait(&queue->cond, &queue->lock.mutex);
+		mutex_unlock(&queue->lock);
 
 		while (virt_queue__available(vq)) {
 			unsigned char buffer[MAX_PACKET_SIZE + sizeof(struct virtio_net_hdr_mrg_rxbuf)];
@@ -127,7 +126,7 @@ static void *virtio_net_rx_thread(void *p)
 			len = ndev->ops->rx(&dummy_iov, 1, ndev);
 			if (len < 0) {
 				pr_warning("%s: rx on vq %u failed (%d), exiting thread\n",
-						__func__, id, len);
+						__func__, queue->id, len);
 				goto out_err;
 			}
 
@@ -155,7 +154,7 @@ static void *virtio_net_rx_thread(void *p)
 
 			/* We should interrupt guest right now, otherwise latency is huge. */
 			if (virtio_queue__should_signal(vq))
-				ndev->vdev.ops->signal_vq(kvm, &ndev->vdev, id);
+				ndev->vdev.ops->signal_vq(kvm, &ndev->vdev, queue->id);
 		}
 	}
 
@@ -168,28 +167,23 @@ out_err:
 static void *virtio_net_tx_thread(void *p)
 {
 	struct iovec iov[VIRTIO_NET_QUEUE_SIZE];
-	struct virt_queue *vq;
+	struct net_dev_queue *queue = p;
+	struct virt_queue *vq = &queue->vq;
+	struct net_dev *ndev = queue->ndev;
 	struct kvm *kvm;
-	struct net_dev *ndev = p;
 	u16 out, in;
 	u16 head;
 	int len;
-	u32 id;
-
-	mutex_lock(&ndev->mutex);
-	id = ndev->tx_vqs++ * 2 + 1;
-	mutex_unlock(&ndev->mutex);
 
 	kvm__set_thread_name("virtio-net-tx");
 
 	kvm = ndev->kvm;
-	vq = &ndev->vqs[id];
 
 	while (1) {
-		mutex_lock(&ndev->io_lock[id]);
+		mutex_lock(&queue->lock);
 		if (!virt_queue__available(vq))
-			pthread_cond_wait(&ndev->io_cond[id], &ndev->io_lock[id].mutex);
-		mutex_unlock(&ndev->io_lock[id]);
+			pthread_cond_wait(&queue->cond, &queue->lock.mutex);
+		mutex_unlock(&queue->lock);
 
 		while (virt_queue__available(vq)) {
 			struct virtio_net_hdr *hdr;
@@ -199,7 +193,7 @@ static void *virtio_net_tx_thread(void *p)
 			len = ndev->ops->tx(iov, out, ndev);
 			if (len < 0) {
 				pr_warning("%s: tx on vq %u failed (%d)\n",
-						__func__, id, errno);
+						__func__, queue->id, errno);
 				goto out_err;
 			}
 
@@ -207,7 +201,7 @@ static void *virtio_net_tx_thread(void *p)
 		}
 
 		if (virtio_queue__should_signal(vq))
-			ndev->vdev.ops->signal_vq(kvm, &ndev->vdev, id);
+			ndev->vdev.ops->signal_vq(kvm, &ndev->vdev, queue->id);
 	}
 
 out_err:
@@ -224,24 +218,24 @@ static virtio_net_ctrl_ack virtio_net_handle_mq(struct kvm* kvm, struct net_dev
 static void *virtio_net_ctrl_thread(void *p)
 {
 	struct iovec iov[VIRTIO_NET_QUEUE_SIZE];
+	struct net_dev_queue *queue = p;
+	struct virt_queue *vq = &queue->vq;
+	struct net_dev *ndev = queue->ndev;
 	u16 out, in, head;
-	struct net_dev *ndev = p;
 	struct kvm *kvm = ndev->kvm;
-	u32 id = ndev->queue_pairs * 2;
-	struct virt_queue *vq = &ndev->vqs[id];
 	struct virtio_net_ctrl_hdr *ctrl;
 	virtio_net_ctrl_ack *ack;
 
 	kvm__set_thread_name("virtio-net-ctrl");
 
 	while (1) {
-		mutex_lock(&ndev->io_lock[id]);
+		mutex_lock(&queue->lock);
 		if (!virt_queue__available(vq))
-			pthread_cond_wait(&ndev->io_cond[id], &ndev->io_lock[id].mutex);
-		mutex_unlock(&ndev->io_lock[id]);
+			pthread_cond_wait(&queue->cond, &queue->lock.mutex);
+		mutex_unlock(&queue->lock);
 
 		while (virt_queue__available(vq)) {
-			head = virt_queue__get_iov(&ndev->vqs[id], iov, &out, &in, kvm);
+			head = virt_queue__get_iov(vq, iov, &out, &in, kvm);
 			ctrl = iov[0].iov_base;
 			ack = iov[out].iov_base;
 
@@ -253,11 +247,11 @@ static void *virtio_net_ctrl_thread(void *p)
 				*ack = VIRTIO_NET_ERR;
 				break;
 			}
-			virt_queue__set_used_elem(&ndev->vqs[id], head, iov[out].iov_len);
+			virt_queue__set_used_elem(vq, head, iov[out].iov_len);
 		}
 
-		if (virtio_queue__should_signal(&ndev->vqs[id]))
-			ndev->vdev.ops->signal_vq(kvm, &ndev->vdev, id);
+		if (virtio_queue__should_signal(vq))
+			ndev->vdev.ops->signal_vq(kvm, &ndev->vdev, queue->id);
 	}
 
 	pthread_exit(NULL);
@@ -267,14 +261,16 @@ static void *virtio_net_ctrl_thread(void *p)
 
 static void virtio_net_handle_callback(struct kvm *kvm, struct net_dev *ndev, int queue)
 {
+	struct net_dev_queue *net_queue = &ndev->queues[queue];
+
 	if ((u32)queue >= (ndev->queue_pairs * 2 + 1)) {
 		pr_warning("Unknown queue index %u", queue);
 		return;
 	}
 
-	mutex_lock(&ndev->io_lock[queue]);
-	pthread_cond_signal(&ndev->io_cond[queue]);
-	mutex_unlock(&ndev->io_lock[queue]);
+	mutex_lock(&net_queue->lock);
+	pthread_cond_signal(&net_queue->cond);
+	mutex_unlock(&net_queue->lock);
 }
 
 static int virtio_net_request_tap(struct net_dev *ndev, struct ifreq *ifr,
@@ -552,6 +548,7 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
 		   u32 pfn)
 {
 	struct vhost_vring_state state = { .index = vq };
+	struct net_dev_queue *net_queue;
 	struct vhost_vring_addr addr;
 	struct net_dev *ndev = dev;
 	struct virt_queue *queue;
@@ -560,24 +557,30 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
 
 	compat__remove_message(compat_id);
 
-	queue		= &ndev->vqs[vq];
+	net_queue	= &ndev->queues[vq];
+	net_queue->id	= vq;
+	net_queue->ndev	= ndev;
+	queue		= &net_queue->vq;
 	queue->pfn	= pfn;
 	p		= virtio_get_vq(kvm, queue->pfn, page_size);
 
 	vring_init(&queue->vring, VIRTIO_NET_QUEUE_SIZE, p, align);
 	virtio_init_device_vq(&ndev->vdev, queue);
 
-	mutex_init(&ndev->io_lock[vq]);
-	pthread_cond_init(&ndev->io_cond[vq], NULL);
+	mutex_init(&net_queue->lock);
+	pthread_cond_init(&net_queue->cond, NULL);
 	if (is_ctrl_vq(ndev, vq)) {
-		pthread_create(&ndev->io_thread[vq], NULL, virtio_net_ctrl_thread, ndev);
+		pthread_create(&net_queue->thread, NULL, virtio_net_ctrl_thread,
+			       net_queue);
 
 		return 0;
 	} else if (ndev->vhost_fd == 0 ) {
 		if (vq & 1)
-			pthread_create(&ndev->io_thread[vq], NULL, virtio_net_tx_thread, ndev);
+			pthread_create(&net_queue->thread, NULL,
+				       virtio_net_tx_thread, net_queue);
 		else
-			pthread_create(&ndev->io_thread[vq], NULL, virtio_net_rx_thread, ndev);
+			pthread_create(&net_queue->thread, NULL,
+				       virtio_net_rx_thread, net_queue);
 
 		return 0;
 	}
@@ -611,6 +614,7 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
 static void notify_vq_gsi(struct kvm *kvm, void *dev, u32 vq, u32 gsi)
 {
 	struct net_dev *ndev = dev;
+	struct net_dev_queue *queue = &ndev->queues[vq];
 	struct vhost_vring_file file;
 	int r;
 
@@ -666,7 +670,7 @@ static struct virt_queue *get_vq(struct kvm *kvm, void *dev, u32 vq)
 {
 	struct net_dev *ndev = dev;
 
-	return &ndev->vqs[vq];
+	return &ndev->queues[vq].vq;
 }
 
 static int get_size_vq(struct kvm *kvm, void *dev, u32 vq)
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH kvmtool 09/13] virtio/net: Implement device and virtqueue reset
  2018-12-04 11:08 [PATCH kvmtool 00/13] Implement reset of virtio devices Julien Thierry
                   ` (7 preceding siblings ...)
  2018-12-04 11:08 ` [PATCH kvmtool 08/13] virtio/net: Clean virtqueue state Julien Thierry
@ 2018-12-04 11:08 ` Julien Thierry
  2018-12-04 11:08 ` [PATCH kvmtool 10/13] virtio/blk: Reset virtqueue Julien Thierry
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Julien Thierry @ 2018-12-04 11:08 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: will.deacon

From: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>

On exit_vq(), clean all resources allocated for the queue. When the device
is reset, clean the backend.

Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Signed-off-by: Julien Thierry <julien.thierry@arm.com>
---
 virtio/net.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/virtio/net.c b/virtio/net.c
index ef8e226..a05f4cd 100644
--- a/virtio/net.c
+++ b/virtio/net.c
@@ -43,6 +43,8 @@ struct net_dev_queue {
 	pthread_t			thread;
 	struct mutex			lock;
 	pthread_cond_t			cond;
+	int				gsi;
+	int				irqfd;
 };
 
 struct net_dev {
@@ -361,6 +363,23 @@ fail:
 	return 0;
 }
 
+static void virtio_net__tap_exit(struct net_dev *ndev)
+{
+	int sock;
+	struct ifreq ifr;
+
+	if (ndev->params->tapif)
+		return;
+
+	sock = socket(AF_INET, SOCK_STREAM, 0);
+	strncpy(ifr.ifr_name, ndev->tap_name, sizeof(ndev->tap_name));
+	ioctl(sock, SIOCGIFFLAGS, &ifr);
+	ifr.ifr_flags &= ~(IFF_UP | IFF_RUNNING);
+	if (ioctl(sock, SIOCGIFFLAGS, &ifr) < 0)
+		pr_warning("Count not bring tap device down");
+	close(sock);
+}
+
 static bool virtio_net__tap_create(struct net_dev *ndev)
 {
 	int offload;
@@ -533,10 +552,21 @@ static void virtio_net_start(struct net_dev *ndev)
 	}
 }
 
+static void virtio_net_stop(struct net_dev *ndev)
+{
+	/* Undo whatever start() did */
+	if (ndev->mode == NET_MODE_TAP)
+		virtio_net__tap_exit(ndev);
+	else
+		uip_exit(&ndev->info);
+}
+
 static void notify_status(struct kvm *kvm, void *dev, u32 status)
 {
 	if (status & VIRTIO__STATUS_START)
 		virtio_net_start(dev);
+	else if (status & VIRTIO__STATUS_STOP)
+		virtio_net_stop(dev);
 }
 
 static bool is_ctrl_vq(struct net_dev *ndev, u32 vq)
@@ -611,6 +641,35 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
 	return 0;
 }
 
+static void exit_vq(struct kvm *kvm, void *dev, u32 vq)
+{
+	struct net_dev *ndev = dev;
+	struct net_dev_queue *queue = &ndev->queues[vq];
+
+	if (!is_ctrl_vq(ndev, vq) && queue->gsi) {
+		irq__del_irqfd(kvm, queue->gsi, queue->irqfd);
+		close(queue->irqfd);
+		queue->gsi = queue->irqfd = 0;
+	}
+
+	/*
+	 * TODO: vhost reset owner. It's the only way to cleanly stop vhost, but
+	 * we can't restart it at the moment.
+	 */
+	if (ndev->vhost_fd && !is_ctrl_vq(ndev, vq)) {
+		pr_warning("Cannot reset VHOST queue");
+		ioctl(ndev->vhost_fd, VHOST_RESET_OWNER);
+		return;
+	}
+
+	/*
+	 * Threads are waiting on cancellation points (readv or
+	 * pthread_cond_wait) and should stop gracefully.
+	 */
+	pthread_cancel(queue->thread);
+	pthread_join(queue->thread, NULL);
+}
+
 static void notify_vq_gsi(struct kvm *kvm, void *dev, u32 vq, u32 gsi)
 {
 	struct net_dev *ndev = dev;
@@ -630,6 +689,9 @@ static void notify_vq_gsi(struct kvm *kvm, void *dev, u32 vq, u32 gsi)
 	if (r < 0)
 		die_perror("KVM_IRQFD failed");
 
+	queue->irqfd = file.fd;
+	queue->gsi = gsi;
+
 	r = ioctl(ndev->vhost_fd, VHOST_SET_VRING_CALL, &file);
 	if (r < 0)
 		die_perror("VHOST_SET_VRING_CALL failed");
@@ -698,6 +760,7 @@ static struct virtio_ops net_dev_virtio_ops = {
 	.set_guest_features	= set_guest_features,
 	.get_vq_count		= get_vq_count,
 	.init_vq		= init_vq,
+	.exit_vq		= exit_vq,
 	.get_vq			= get_vq,
 	.get_size_vq		= get_size_vq,
 	.set_size_vq		= set_size_vq,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH kvmtool 10/13] virtio/blk: Reset virtqueue
  2018-12-04 11:08 [PATCH kvmtool 00/13] Implement reset of virtio devices Julien Thierry
                   ` (8 preceding siblings ...)
  2018-12-04 11:08 ` [PATCH kvmtool 09/13] virtio/net: Implement device and virtqueue reset Julien Thierry
@ 2018-12-04 11:08 ` Julien Thierry
  2018-12-04 11:08 ` [PATCH kvmtool 11/13] threadpool: Add cancel() function Julien Thierry
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Julien Thierry @ 2018-12-04 11:08 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: will.deacon

From: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>

Move pthread creation to init_vq, and kill the thread in exit_vq.
Initialize the virtqueue states at runtime.

All in-flight I/O is canceled with the virtqueue pthreads, except for AIO
threads, but after reading the code I'm not sure if AIO has ever worked
anyway.

Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Signed-off-by: Julien Thierry <julien.thierry@arm.com>
---
 virtio/blk.c | 67 ++++++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 45 insertions(+), 22 deletions(-)

diff --git a/virtio/blk.c b/virtio/blk.c
index 6a6b3b7..a57df2e 100644
--- a/virtio/blk.c
+++ b/virtio/blk.c
@@ -178,9 +178,29 @@ static void notify_status(struct kvm *kvm, void *dev, u32 status)
 {
 }
 
+static void *virtio_blk_thread(void *dev)
+{
+	struct blk_dev *bdev = dev;
+	u64 data;
+	int r;
+
+	kvm__set_thread_name("virtio-blk-io");
+
+	while (1) {
+		r = read(bdev->io_efd, &data, sizeof(u64));
+		if (r < 0)
+			continue;
+		virtio_blk_do_io(bdev->kvm, &bdev->vqs[0], bdev);
+	}
+
+	pthread_exit(NULL);
+	return NULL;
+}
+
 static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
 		   u32 pfn)
 {
+	unsigned int i;
 	struct blk_dev *bdev = dev;
 	struct virt_queue *queue;
 	void *p;
@@ -194,26 +214,37 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
 	vring_init(&queue->vring, VIRTIO_BLK_QUEUE_SIZE, p, align);
 	virtio_init_device_vq(&bdev->vdev, queue);
 
+	if (vq != 0)
+		return 0;
+
+	for (i = 0; i < ARRAY_SIZE(bdev->reqs); i++) {
+		bdev->reqs[i] = (struct blk_dev_req) {
+			.bdev = bdev,
+			.kvm = kvm,
+		};
+	}
+
+	mutex_init(&bdev->mutex);
+	bdev->io_efd = eventfd(0, 0);
+	if (bdev->io_efd < 0)
+		return -errno;
+
+	if (pthread_create(&bdev->io_thread, NULL, virtio_blk_thread, bdev))
+		return -errno;
+
 	return 0;
 }
 
-static void *virtio_blk_thread(void *dev)
+static void exit_vq(struct kvm *kvm, void *dev, u32 vq)
 {
 	struct blk_dev *bdev = dev;
-	u64 data;
-	int r;
 
-	kvm__set_thread_name("virtio-blk-io");
+	if (vq != 0)
+		return;
 
-	while (1) {
-		r = read(bdev->io_efd, &data, sizeof(u64));
-		if (r < 0)
-			continue;
-		virtio_blk_do_io(bdev->kvm, &bdev->vqs[0], bdev);
-	}
-
-	pthread_exit(NULL);
-	return NULL;
+	close(bdev->io_efd);
+	pthread_cancel(bdev->io_thread);
+	pthread_join(bdev->io_thread, NULL);
 }
 
 static int notify_vq(struct kvm *kvm, void *dev, u32 vq)
@@ -259,6 +290,7 @@ static struct virtio_ops blk_dev_virtio_ops = {
 	.set_guest_features	= set_guest_features,
 	.get_vq_count		= get_vq_count,
 	.init_vq		= init_vq,
+	.exit_vq		= exit_vq,
 	.notify_status		= notify_status,
 	.notify_vq		= notify_vq,
 	.get_vq			= get_vq,
@@ -269,7 +301,6 @@ static struct virtio_ops blk_dev_virtio_ops = {
 static int virtio_blk__init_one(struct kvm *kvm, struct disk_image *disk)
 {
 	struct blk_dev *bdev;
-	unsigned int i;
 
 	if (!disk)
 		return -EINVAL;
@@ -279,13 +310,11 @@ static int virtio_blk__init_one(struct kvm *kvm, struct disk_image *disk)
 		return -ENOMEM;
 
 	*bdev = (struct blk_dev) {
-		.mutex			= MUTEX_INITIALIZER,
 		.disk			= disk,
 		.blk_config		= (struct virtio_blk_config) {
 			.capacity	= disk->size / SECTOR_SIZE,
 			.seg_max	= DISK_SEG_MAX,
 		},
-		.io_efd			= eventfd(0, 0),
 		.kvm			= kvm,
 	};
 
@@ -295,14 +324,8 @@ static int virtio_blk__init_one(struct kvm *kvm, struct disk_image *disk)
 
 	list_add_tail(&bdev->list, &bdevs);
 
-	for (i = 0; i < ARRAY_SIZE(bdev->reqs); i++) {
-		bdev->reqs[i].bdev = bdev;
-		bdev->reqs[i].kvm = kvm;
-	}
-
 	disk_image__set_callback(bdev->disk, virtio_blk_complete);
 
-	pthread_create(&bdev->io_thread, NULL, virtio_blk_thread, bdev);
 	if (compat_id == -1)
 		compat_id = virtio_compat_add_message("virtio-blk", "CONFIG_VIRTIO_BLK");
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH kvmtool 11/13] threadpool: Add cancel() function
  2018-12-04 11:08 [PATCH kvmtool 00/13] Implement reset of virtio devices Julien Thierry
                   ` (9 preceding siblings ...)
  2018-12-04 11:08 ` [PATCH kvmtool 10/13] virtio/blk: Reset virtqueue Julien Thierry
@ 2018-12-04 11:08 ` Julien Thierry
  2018-12-04 11:08 ` [PATCH kvmtool 12/13] virtio/p9: Implement reset Julien Thierry
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Julien Thierry @ 2018-12-04 11:08 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: will.deacon

From: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>

When resetting a virtqueue, it is often necessary to make sure that the
associated threadpool job isn't running anymore. Add a function to
cancel a job.

A threadpool job has three states: idle, queued and running. A job is
queued when it is in the job list. It is running when it is out the
list, but its signal count is greater than zero. It is idle when it is
both out of the list and its signal count is zero. The cancel() function
simply waits for the job to be idle. It is up to the caller to make sure
that the job isn't queued concurrently.

Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Signed-off-by: Julien Thierry <julien.thierry@arm.com>
---
 include/kvm/threadpool.h |  2 ++
 util/threadpool.c        | 25 ++++++++++++++++++++++++-
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/include/kvm/threadpool.h b/include/kvm/threadpool.h
index bacb243..880487a 100644
--- a/include/kvm/threadpool.h
+++ b/include/kvm/threadpool.h
@@ -28,11 +28,13 @@ static inline void thread_pool__init_job(struct thread_pool__job *job, struct kv
 		.data		= data,
 		.mutex		= MUTEX_INITIALIZER,
 	};
+	INIT_LIST_HEAD(&job->queue);
 }
 
 int thread_pool__init(struct kvm *kvm);
 int thread_pool__exit(struct kvm *kvm);
 
 void thread_pool__do_job(struct thread_pool__job *job);
+void thread_pool__cancel_job(struct thread_pool__job *job);
 
 #endif
diff --git a/util/threadpool.c b/util/threadpool.c
index e64aa26..1dc3bf7 100644
--- a/util/threadpool.c
+++ b/util/threadpool.c
@@ -25,7 +25,7 @@ static struct thread_pool__job *thread_pool__job_pop_locked(void)
 		return NULL;
 
 	job = list_first_entry(&head, struct thread_pool__job, queue);
-	list_del(&job->queue);
+	list_del_init(&job->queue);
 
 	return job;
 }
@@ -173,3 +173,26 @@ void thread_pool__do_job(struct thread_pool__job *job)
 	pthread_cond_signal(&job_cond);
 	mutex_unlock(&job_mutex);
 }
+
+void thread_pool__cancel_job(struct thread_pool__job *job)
+{
+	bool running;
+
+	/*
+	 * If the job is queued but not running, remove it. Otherwise, wait for
+	 * the signalcount to drop to 0, indicating that it has finished
+	 * running. We assume that nobody is queueing this job -
+	 * thread_pool__do_job() isn't called - while this function is running.
+	 */
+	do {
+		mutex_lock(&job_mutex);
+		if (list_empty(&job->queue)) {
+			running = job->signalcount > 0;
+		} else {
+			list_del_init(&job->queue);
+			job->signalcount = 0;
+			running = false;
+		}
+		mutex_unlock(&job_mutex);
+	} while (running);
+}
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH kvmtool 12/13] virtio/p9: Implement reset
  2018-12-04 11:08 [PATCH kvmtool 00/13] Implement reset of virtio devices Julien Thierry
                   ` (10 preceding siblings ...)
  2018-12-04 11:08 ` [PATCH kvmtool 11/13] threadpool: Add cancel() function Julien Thierry
@ 2018-12-04 11:08 ` Julien Thierry
  2018-12-04 11:08 ` [PATCH kvmtool 13/13] virtio/console: " Julien Thierry
  2018-12-05  8:21 ` [PATCH kvmtool 00/13] Implement reset of virtio devices Gerd Hoffmann
  13 siblings, 0 replies; 17+ messages in thread
From: Julien Thierry @ 2018-12-04 11:08 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: will.deacon

From: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>

The p9 reset cancels all running jobs and closes any open fid.

Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Signed-off-by: Julien Thierry <julien.thierry@arm.com>
---
 virtio/9p.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/virtio/9p.c b/virtio/9p.c
index d9f45cf..6bae403 100644
--- a/virtio/9p.c
+++ b/virtio/9p.c
@@ -1384,6 +1384,14 @@ static void set_guest_features(struct kvm *kvm, void *dev, u32 features)
 
 static void notify_status(struct kvm *kvm, void *dev, u32 status)
 {
+	struct p9_dev *p9dev = dev;
+	struct p9_fid *pfid, *next;
+
+	if (!(status & VIRTIO__STATUS_STOP))
+		return;
+
+	rbtree_postorder_for_each_entry_safe(pfid, next, &p9dev->fids, node)
+		close_fid(p9dev, pfid->fid);
 }
 
 static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
@@ -1413,6 +1421,13 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
 	return 0;
 }
 
+static void exit_vq(struct kvm *kvm, void *dev, u32 vq)
+{
+	struct p9_dev *p9dev = dev;
+
+	thread_pool__cancel_job(&p9dev->jobs[vq].job_id);
+}
+
 static int notify_vq(struct kvm *kvm, void *dev, u32 vq)
 {
 	struct p9_dev *p9dev = dev;
@@ -1450,6 +1465,7 @@ struct virtio_ops p9_dev_virtio_ops = {
 	.get_host_features	= get_host_features,
 	.set_guest_features	= set_guest_features,
 	.init_vq		= init_vq,
+	.exit_vq		= exit_vq,
 	.notify_status		= notify_status,
 	.notify_vq		= notify_vq,
 	.get_vq			= get_vq,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH kvmtool 13/13] virtio/console: Implement reset
  2018-12-04 11:08 [PATCH kvmtool 00/13] Implement reset of virtio devices Julien Thierry
                   ` (11 preceding siblings ...)
  2018-12-04 11:08 ` [PATCH kvmtool 12/13] virtio/p9: Implement reset Julien Thierry
@ 2018-12-04 11:08 ` Julien Thierry
  2018-12-05  8:21 ` [PATCH kvmtool 00/13] Implement reset of virtio devices Gerd Hoffmann
  13 siblings, 0 replies; 17+ messages in thread
From: Julien Thierry @ 2018-12-04 11:08 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: will.deacon

From: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>

The virtio-console reset cancels all running jobs.

Unfortunately we don't have a good way to prevent the term polling thread
from getting in the way, read invalid data during reset and cause a
segfault. To prevent this, move all handling of the Rx queue in the
threadpool job.

Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Signed-off-by: Julien Thierry <julien.thierry@arm.com>
---
 virtio/console.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/virtio/console.c b/virtio/console.c
index d2b312c..f1be025 100644
--- a/virtio/console.c
+++ b/virtio/console.c
@@ -35,8 +35,6 @@ struct con_dev {
 	struct virt_queue		vqs[VIRTIO_CONSOLE_NUM_QUEUES];
 	struct virtio_console_config	config;
 	u32				features;
-
-	pthread_cond_t			poll_cond;
 	int				vq_ready;
 
 	struct thread_pool__job		jobs[VIRTIO_CONSOLE_NUM_QUEUES];
@@ -44,7 +42,6 @@ struct con_dev {
 
 static struct con_dev cdev = {
 	.mutex				= MUTEX_INITIALIZER,
-	.poll_cond			= PTHREAD_COND_INITIALIZER,
 
 	.vq_ready			= 0,
 
@@ -68,16 +65,10 @@ static void virtio_console__inject_interrupt_callback(struct kvm *kvm, void *par
 	u16 head;
 	int len;
 
-	if (kvm->cfg.active_console != CONSOLE_VIRTIO)
-		return;
-
 	mutex_lock(&cdev.mutex);
 
 	vq = param;
 
-	if (!cdev.vq_ready)
-		pthread_cond_wait(&cdev.poll_cond, &cdev.mutex.mutex);
-
 	if (term_readable(0) && virt_queue__available(vq)) {
 		head = virt_queue__get_iov(vq, iov, &out, &in, kvm);
 		len = term_getc_iov(kvm, iov, in, 0);
@@ -90,8 +81,13 @@ static void virtio_console__inject_interrupt_callback(struct kvm *kvm, void *par
 
 void virtio_console__inject_interrupt(struct kvm *kvm)
 {
-	virtio_console__inject_interrupt_callback(kvm,
-					&cdev.vqs[VIRTIO_CONSOLE_RX_QUEUE]);
+	if (kvm->cfg.active_console != CONSOLE_VIRTIO)
+		return;
+
+	mutex_lock(&cdev.mutex);
+	if (cdev.vq_ready)
+		thread_pool__do_job(&cdev.jobs[VIRTIO_CONSOLE_RX_QUEUE]);
+	mutex_unlock(&cdev.mutex);
 }
 
 static void virtio_console_handle_callback(struct kvm *kvm, void *param)
@@ -168,13 +164,24 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
 		/* Tell the waiting poll thread that we're ready to go */
 		mutex_lock(&cdev.mutex);
 		cdev.vq_ready = 1;
-		pthread_cond_signal(&cdev.poll_cond);
 		mutex_unlock(&cdev.mutex);
 	}
 
 	return 0;
 }
 
+static void exit_vq(struct kvm *kvm, void *dev, u32 vq)
+{
+	if (vq == VIRTIO_CONSOLE_RX_QUEUE) {
+		mutex_lock(&cdev.mutex);
+		cdev.vq_ready = 0;
+		mutex_unlock(&cdev.mutex);
+		thread_pool__cancel_job(&cdev.jobs[vq]);
+	} else if (vq == VIRTIO_CONSOLE_TX_QUEUE) {
+		thread_pool__cancel_job(&cdev.jobs[vq]);
+	}
+}
+
 static int notify_vq(struct kvm *kvm, void *dev, u32 vq)
 {
 	struct con_dev *cdev = dev;
@@ -213,6 +220,7 @@ static struct virtio_ops con_dev_virtio_ops = {
 	.set_guest_features	= set_guest_features,
 	.get_vq_count		= get_vq_count,
 	.init_vq		= init_vq,
+	.exit_vq		= exit_vq,
 	.notify_status		= notify_status,
 	.notify_vq		= notify_vq,
 	.get_vq			= get_vq,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH kvmtool 00/13] Implement reset of virtio devices
  2018-12-04 11:08 [PATCH kvmtool 00/13] Implement reset of virtio devices Julien Thierry
                   ` (12 preceding siblings ...)
  2018-12-04 11:08 ` [PATCH kvmtool 13/13] virtio/console: " Julien Thierry
@ 2018-12-05  8:21 ` Gerd Hoffmann
  2018-12-05  9:10   ` Julien Thierry
  13 siblings, 1 reply; 17+ messages in thread
From: Gerd Hoffmann @ 2018-12-05  8:21 UTC (permalink / raw)
  To: Julien Thierry; +Cc: will.deacon, kvmarm, kvm

On Tue, Dec 04, 2018 at 11:08:33AM +0000, Julien Thierry wrote:
> Hi,
> 
> This series was developped by Jean-Philippe and is needed for a series
> I'll be posting shortly after to load firmwares on arm kvmtool.
> 
> Currently, when a guest tries to reset a device, a lot of ressources
> aren't reset (threads keep running, virtio queue keep their state, etc).
> 
> When the guest only does the reset to initialize the device and there
> were no previous users, there is no noticeable issue. But when a guest
> has a firmare + Linux, if the firmware uses a virtio device, Linux will
> fail to probe that device.
> 
> This series aim to properly reset the virtio resources when the guests
> requests it.
> 
> Reset of net vhost is unsupported for now.
> 
> Patch 1 is a bug fix on ioeventfd
> Patch 2-6 provide the core support so devices can implement their reset
> Patch 7-13 implements the reset for the various virtio devices

Fails to build:

kraxel@sirius ~/projects/kvmtool (virtio)# make 
Makefile:323: Skipping optional libraries: vncserver
  CC       virtio/core.o
In file included from virtio/core.c:8:0:
virtio/core.c: In function ‘virtio_notify_status’:
include/kvm/virtio.h:37:3: error: ‘VIRTIO_CONFIG_S_NEEDS_RESET’
undeclared (first use in this function)
   VIRTIO_CONFIG_S_NEEDS_RESET | \
   ^
virtio/core.c:232:19: note: in expansion of macro ‘VIRTIO_CONFIG_S_MASK’
  vdev->status &= ~VIRTIO_CONFIG_S_MASK;

Yes, it is an older machine (rhel-7), VIRTIO_CONFIG_S_NEEDS_RESET isn't
in the system headers (IIRC this bit was added with virtio-1.0).

cheers,
  Gerd

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH kvmtool 00/13] Implement reset of virtio devices
  2018-12-05  8:21 ` [PATCH kvmtool 00/13] Implement reset of virtio devices Gerd Hoffmann
@ 2018-12-05  9:10   ` Julien Thierry
  2018-12-05 10:09     ` Gerd Hoffmann
  0 siblings, 1 reply; 17+ messages in thread
From: Julien Thierry @ 2018-12-05  9:10 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: will.deacon, kvmarm, kvm



On 05/12/18 08:21, Gerd Hoffmann wrote:
> On Tue, Dec 04, 2018 at 11:08:33AM +0000, Julien Thierry wrote:
>> Hi,
>>
>> This series was developped by Jean-Philippe and is needed for a series
>> I'll be posting shortly after to load firmwares on arm kvmtool.
>>
>> Currently, when a guest tries to reset a device, a lot of ressources
>> aren't reset (threads keep running, virtio queue keep their state, etc).
>>
>> When the guest only does the reset to initialize the device and there
>> were no previous users, there is no noticeable issue. But when a guest
>> has a firmare + Linux, if the firmware uses a virtio device, Linux will
>> fail to probe that device.
>>
>> This series aim to properly reset the virtio resources when the guests
>> requests it.
>>
>> Reset of net vhost is unsupported for now.
>>
>> Patch 1 is a bug fix on ioeventfd
>> Patch 2-6 provide the core support so devices can implement their reset
>> Patch 7-13 implements the reset for the various virtio devices
> 
> Fails to build:
> 
> kraxel@sirius ~/projects/kvmtool (virtio)# make 
> Makefile:323: Skipping optional libraries: vncserver
>   CC       virtio/core.o
> In file included from virtio/core.c:8:0:
> virtio/core.c: In function ‘virtio_notify_status’:
> include/kvm/virtio.h:37:3: error: ‘VIRTIO_CONFIG_S_NEEDS_RESET’
> undeclared (first use in this function)
>    VIRTIO_CONFIG_S_NEEDS_RESET | \
>    ^
> virtio/core.c:232:19: note: in expansion of macro ‘VIRTIO_CONFIG_S_MASK’
>   vdev->status &= ~VIRTIO_CONFIG_S_MASK;
> 
> Yes, it is an older machine (rhel-7), VIRTIO_CONFIG_S_NEEDS_RESET isn't
> in the system headers (IIRC this bit was added with virtio-1.0).
> 

Thanks for reporting the issue. I think this series was initially
developed with the intent of supporting virtio-1.0 but then reset and
virtio-1.0 were split into two series.

I think here I can just remove that bit from the config mask while
virtio-1.0 is not supported, and things should work.

Thanks,

-- 
Julien Thierry
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH kvmtool 00/13] Implement reset of virtio devices
  2018-12-05  9:10   ` Julien Thierry
@ 2018-12-05 10:09     ` Gerd Hoffmann
  0 siblings, 0 replies; 17+ messages in thread
From: Gerd Hoffmann @ 2018-12-05 10:09 UTC (permalink / raw)
  To: Julien Thierry; +Cc: will.deacon, kvmarm, kvm

On Wed, Dec 05, 2018 at 09:10:33AM +0000, Julien Thierry wrote:
> 
> 
> On 05/12/18 08:21, Gerd Hoffmann wrote:
> > On Tue, Dec 04, 2018 at 11:08:33AM +0000, Julien Thierry wrote:
> >> Hi,
> >>
> >> This series was developped by Jean-Philippe and is needed for a series
> >> I'll be posting shortly after to load firmwares on arm kvmtool.
> >>
> >> Currently, when a guest tries to reset a device, a lot of ressources
> >> aren't reset (threads keep running, virtio queue keep their state, etc).
> >>
> >> When the guest only does the reset to initialize the device and there
> >> were no previous users, there is no noticeable issue. But when a guest
> >> has a firmare + Linux, if the firmware uses a virtio device, Linux will
> >> fail to probe that device.
> >>
> >> This series aim to properly reset the virtio resources when the guests
> >> requests it.
> >>
> >> Reset of net vhost is unsupported for now.
> >>
> >> Patch 1 is a bug fix on ioeventfd
> >> Patch 2-6 provide the core support so devices can implement their reset
> >> Patch 7-13 implements the reset for the various virtio devices
> > 
> > Fails to build:
> > 
> > kraxel@sirius ~/projects/kvmtool (virtio)# make 
> > Makefile:323: Skipping optional libraries: vncserver
> >   CC       virtio/core.o
> > In file included from virtio/core.c:8:0:
> > virtio/core.c: In function ‘virtio_notify_status’:
> > include/kvm/virtio.h:37:3: error: ‘VIRTIO_CONFIG_S_NEEDS_RESET’
> > undeclared (first use in this function)
> >    VIRTIO_CONFIG_S_NEEDS_RESET | \
> >    ^
> > virtio/core.c:232:19: note: in expansion of macro ‘VIRTIO_CONFIG_S_MASK’
> >   vdev->status &= ~VIRTIO_CONFIG_S_MASK;
> > 
> > Yes, it is an older machine (rhel-7), VIRTIO_CONFIG_S_NEEDS_RESET isn't
> > in the system headers (IIRC this bit was added with virtio-1.0).
> > 
> 
> Thanks for reporting the issue. I think this series was initially
> developed with the intent of supporting virtio-1.0 but then reset and
> virtio-1.0 were split into two series.
> 
> I think here I can just remove that bit from the config mask while
> virtio-1.0 is not supported, and things should work.

Fixed that using ...

+#ifndef VIRTIO_CONFIG_S_NEEDS_RESET
+# define VIRTIO_CONFIG_S_NEEDS_RESET 0x40
+#endif

... in include/kvm/virtio.h

With that in place firmware boot works for x86 too.

Well, to be exact, sort of starts working.  There is still the problem
that kvmtool tweaks the linux kernel command line to compensate for
incomplete emulation.  So when booting a kernel from the disk image via
firmware and boot loader you have to apply the same tweaks to the boot
loader config.  At minimum pci=conf1 is required, otherwise the kernel
doesn't find any pci devices ...

cheers,
  Gerd

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2018-12-05 10:09 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-04 11:08 [PATCH kvmtool 00/13] Implement reset of virtio devices Julien Thierry
2018-12-04 11:08 ` [PATCH kvmtool 01/13] ioeventfd: Fix removal of ioeventfd Julien Thierry
2018-12-04 11:08 ` [PATCH kvmtool 02/13] virtio: Implement notify_status Julien Thierry
2018-12-04 11:08 ` [PATCH kvmtool 03/13] virtio: Add get_vq_count() callback Julien Thierry
2018-12-04 11:08 ` [PATCH kvmtool 04/13] virtio: Add get_vq() callback Julien Thierry
2018-12-04 11:08 ` [PATCH kvmtool 05/13] virtio: Add exit_vq() callback Julien Thierry
2018-12-04 11:08 ` [PATCH kvmtool 06/13] virtio: Add reset() callback Julien Thierry
2018-12-04 11:08 ` [PATCH kvmtool 07/13] net/uip: Add exit function Julien Thierry
2018-12-04 11:08 ` [PATCH kvmtool 08/13] virtio/net: Clean virtqueue state Julien Thierry
2018-12-04 11:08 ` [PATCH kvmtool 09/13] virtio/net: Implement device and virtqueue reset Julien Thierry
2018-12-04 11:08 ` [PATCH kvmtool 10/13] virtio/blk: Reset virtqueue Julien Thierry
2018-12-04 11:08 ` [PATCH kvmtool 11/13] threadpool: Add cancel() function Julien Thierry
2018-12-04 11:08 ` [PATCH kvmtool 12/13] virtio/p9: Implement reset Julien Thierry
2018-12-04 11:08 ` [PATCH kvmtool 13/13] virtio/console: " Julien Thierry
2018-12-05  8:21 ` [PATCH kvmtool 00/13] Implement reset of virtio devices Gerd Hoffmann
2018-12-05  9:10   ` Julien Thierry
2018-12-05 10:09     ` Gerd Hoffmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).