Re: [Qemu-devel] [PATCH] virtio: Introduce virtio-testdev
On Tue, Oct 15, 2013 at 4:08 PM, Gleb Natapov wrote: > On Tue, Oct 15, 2013 at 11:14:12AM +0100, Peter Maydell wrote: >> On 15 October 2013 10:47, Anup Patel wrote: >> > On Tue, Oct 15, 2013 at 2:06 PM, Andrew Jones wrote: >> >> I'm not opposed to it, but at the moment I'm not sure how we would >> >> utilize it within kvm-unit-tests. Maybe it would be useful for another >> >> application though? So maybe we can add it as an add-on patch at the >> >> time we come up with its use case? >> > >> > I suggested it because we have "machvirt" machine in QEMU for >> > KVM ARM/ARM64 which has only VirtIO devices. In "machvirt", we >> > don't have mechanism to reset the system because none of the >> > VirtIO devices have such a mechanism. Now since you are introducing >> > a "testdev", we can have a RESET command in VirtIO and implement >> > VirtIO REBOOT driver in Linux kernel to use it. >> > >> > Currently, due to no RESET support in "machvirt" we are not able >> > to reboot Guest Linux from Guest console. >> >> We shouldn't be abusing a device intended for testing to >> provide necessary features. If we need guest reboot support >> (which seems like an obvious thing to want) then we need >> to implement a sensible mechanism for it. >> > Definitely. Test device is only for use by unit tests. It does not normally > present and may contain hacks not suitable for human consumption. Got it. > > -- > Gleb. Thanks, Anup
Re: [Qemu-devel] [PATCH] virtio: Introduce virtio-testdev
On Tue, Oct 15, 2013 at 11:14:12AM +0100, Peter Maydell wrote: > On 15 October 2013 10:47, Anup Patel wrote: > > On Tue, Oct 15, 2013 at 2:06 PM, Andrew Jones wrote: > >> I'm not opposed to it, but at the moment I'm not sure how we would > >> utilize it within kvm-unit-tests. Maybe it would be useful for another > >> application though? So maybe we can add it as an add-on patch at the > >> time we come up with its use case? > > > > I suggested it because we have "machvirt" machine in QEMU for > > KVM ARM/ARM64 which has only VirtIO devices. In "machvirt", we > > don't have mechanism to reset the system because none of the > > VirtIO devices have such a mechanism. Now since you are introducing > > a "testdev", we can have a RESET command in VirtIO and implement > > VirtIO REBOOT driver in Linux kernel to use it. > > > > Currently, due to no RESET support in "machvirt" we are not able > > to reboot Guest Linux from Guest console. > > We shouldn't be abusing a device intended for testing to > provide necessary features. If we need guest reboot support > (which seems like an obvious thing to want) then we need > to implement a sensible mechanism for it. > Definitely. Test device is only for use by unit tests. It does not normally present and may contain hacks not suitable for human consumption. -- Gleb.
Re: [Qemu-devel] [PATCH] virtio: Introduce virtio-testdev
On 15 October 2013 10:47, Anup Patel wrote: > On Tue, Oct 15, 2013 at 2:06 PM, Andrew Jones wrote: >> I'm not opposed to it, but at the moment I'm not sure how we would >> utilize it within kvm-unit-tests. Maybe it would be useful for another >> application though? So maybe we can add it as an add-on patch at the >> time we come up with its use case? > > I suggested it because we have "machvirt" machine in QEMU for > KVM ARM/ARM64 which has only VirtIO devices. In "machvirt", we > don't have mechanism to reset the system because none of the > VirtIO devices have such a mechanism. Now since you are introducing > a "testdev", we can have a RESET command in VirtIO and implement > VirtIO REBOOT driver in Linux kernel to use it. > > Currently, due to no RESET support in "machvirt" we are not able > to reboot Guest Linux from Guest console. We shouldn't be abusing a device intended for testing to provide necessary features. If we need guest reboot support (which seems like an obvious thing to want) then we need to implement a sensible mechanism for it. -- PMM
Re: [Qemu-devel] [PATCH] virtio: Introduce virtio-testdev
On Tue, Oct 15, 2013 at 3:17 PM, Anup Patel wrote: > On Tue, Oct 15, 2013 at 2:06 PM, Andrew Jones wrote: >> On Tue, Oct 15, 2013 at 12:26:10PM +0530, Anup Patel wrote: >>> Hi Andrew, >>> >>> On Mon, Oct 14, 2013 at 9:29 PM, Andrew Jones wrote: >>> > This is a virtio version of hw/misc/debugexit and should evolve into a >>> > virtio version of pc-testdev. pc-testdev uses the PC's ISA bus, whereas >>> > this testdev can be plugged into a virtio-mmio transport, which is >>> > needed for kvm-unit-tests/arm. virtio-testdev uses the virtio device >>> > config space as a communication channel, and implements an RTAS-like >>> > protocol through it allowing guests to execute commands. Only three >>> > commands are currently implemented; >>> > 1) VERSION: for version compatibility checks >>> > 2) CLEAR: set all the config space back to zero >>> > 3) EXIT:exit() from qemu with a status code >>> >>> How about adding RESET command to reset the VM? >>> >> >> Hi Anup, >> >> I'm not opposed to it, but at the moment I'm not sure how we would >> utilize it within kvm-unit-tests. Maybe it would be useful for another >> application though? So maybe we can add it as an add-on patch at the >> time we come up with its use case? > > I suggested it because we have "machvirt" machine in QEMU for > KVM ARM/ARM64 which has only VirtIO devices. In "machvirt", we > don't have mechanism to reset the system because none of the > VirtIO devices have such a mechanism. Now since you are introducing > a "testdev", we can have a RESET command in VirtIO and implement > VirtIO REBOOT driver in Linux kernel to use it. > > Currently, due to no RESET support in "machvirt" we are not able > to reboot Guest Linux from Guest console. The RESET support will not fit here only if "testdev" is supposed to be used for kvm-unit-tests only. Anyways, its just a suggestion. > >> >> Thanks for the review! >> >> drew > > Thanks, > Anup Regards, Anup
Re: [Qemu-devel] [PATCH] virtio: Introduce virtio-testdev
On Tue, Oct 15, 2013 at 2:06 PM, Andrew Jones wrote: > On Tue, Oct 15, 2013 at 12:26:10PM +0530, Anup Patel wrote: >> Hi Andrew, >> >> On Mon, Oct 14, 2013 at 9:29 PM, Andrew Jones wrote: >> > This is a virtio version of hw/misc/debugexit and should evolve into a >> > virtio version of pc-testdev. pc-testdev uses the PC's ISA bus, whereas >> > this testdev can be plugged into a virtio-mmio transport, which is >> > needed for kvm-unit-tests/arm. virtio-testdev uses the virtio device >> > config space as a communication channel, and implements an RTAS-like >> > protocol through it allowing guests to execute commands. Only three >> > commands are currently implemented; >> > 1) VERSION: for version compatibility checks >> > 2) CLEAR: set all the config space back to zero >> > 3) EXIT:exit() from qemu with a status code >> >> How about adding RESET command to reset the VM? >> > > Hi Anup, > > I'm not opposed to it, but at the moment I'm not sure how we would > utilize it within kvm-unit-tests. Maybe it would be useful for another > application though? So maybe we can add it as an add-on patch at the > time we come up with its use case? I suggested it because we have "machvirt" machine in QEMU for KVM ARM/ARM64 which has only VirtIO devices. In "machvirt", we don't have mechanism to reset the system because none of the VirtIO devices have such a mechanism. Now since you are introducing a "testdev", we can have a RESET command in VirtIO and implement VirtIO REBOOT driver in Linux kernel to use it. Currently, due to no RESET support in "machvirt" we are not able to reboot Guest Linux from Guest console. > > Thanks for the review! > > drew Thanks, Anup
Re: [Qemu-devel] [PATCH] virtio: Introduce virtio-testdev
On Tue, Oct 15, 2013 at 12:26:10PM +0530, Anup Patel wrote: > Hi Andrew, > > On Mon, Oct 14, 2013 at 9:29 PM, Andrew Jones wrote: > > This is a virtio version of hw/misc/debugexit and should evolve into a > > virtio version of pc-testdev. pc-testdev uses the PC's ISA bus, whereas > > this testdev can be plugged into a virtio-mmio transport, which is > > needed for kvm-unit-tests/arm. virtio-testdev uses the virtio device > > config space as a communication channel, and implements an RTAS-like > > protocol through it allowing guests to execute commands. Only three > > commands are currently implemented; > > 1) VERSION: for version compatibility checks > > 2) CLEAR: set all the config space back to zero > > 3) EXIT:exit() from qemu with a status code > > How about adding RESET command to reset the VM? > Hi Anup, I'm not opposed to it, but at the moment I'm not sure how we would utilize it within kvm-unit-tests. Maybe it would be useful for another application though? So maybe we can add it as an add-on patch at the time we come up with its use case? Thanks for the review! drew
Re: [Qemu-devel] [PATCH] virtio: Introduce virtio-testdev
Hi Andrew,
On Mon, Oct 14, 2013 at 9:29 PM, Andrew Jones wrote:
> This is a virtio version of hw/misc/debugexit and should evolve into a
> virtio version of pc-testdev. pc-testdev uses the PC's ISA bus, whereas
> this testdev can be plugged into a virtio-mmio transport, which is
> needed for kvm-unit-tests/arm. virtio-testdev uses the virtio device
> config space as a communication channel, and implements an RTAS-like
> protocol through it allowing guests to execute commands. Only three
> commands are currently implemented;
> 1) VERSION: for version compatibility checks
> 2) CLEAR: set all the config space back to zero
> 3) EXIT:exit() from qemu with a status code
How about adding RESET command to reset the VM?
Regards,
Anup
>
> Note, the protocol also requires all data passing through the config
> space to be in little-endian.
>
> See [1] for an example of a driver for this device.
>
> [1]
> https://github.com/rhdrjones/kvm-unit-tests/blob/ff8df5378ffccfbdf25fe79241837e61eb2258e1/lib/virtio-testdev.c
>
> Signed-off-by: Andrew Jones
> ---
> default-configs/arm-softmmu.mak | 2 +
> hw/virtio/Makefile.objs | 1 +
> hw/virtio/virtio-testdev.c | 117
>
> 3 files changed, 120 insertions(+)
> create mode 100644 hw/virtio/virtio-testdev.c
>
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index ac0815d66310f..56f8086e61974 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -80,3 +80,5 @@ CONFIG_VERSATILE_PCI=y
> CONFIG_VERSATILE_I2C=y
>
> CONFIG_SDHCI=y
> +
> +CONFIG_VIRTIO_TESTDEV=y
> diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
> index 1ba53d9cc316c..b3d16d522f54b 100644
> --- a/hw/virtio/Makefile.objs
> +++ b/hw/virtio/Makefile.objs
> @@ -3,6 +3,7 @@ common-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o
> common-obj-y += virtio-bus.o
> common-obj-y += virtio-mmio.o
> common-obj-$(CONFIG_VIRTIO_BLK_DATA_PLANE) += dataplane/
> +common-obj-$(CONFIG_VIRTIO_TESTDEV) += virtio-testdev.o
>
> obj-y += virtio.o virtio-balloon.o
> obj-$(CONFIG_LINUX) += vhost.o
> diff --git a/hw/virtio/virtio-testdev.c b/hw/virtio/virtio-testdev.c
> new file mode 100644
> index 0..d6852d563702e
> --- /dev/null
> +++ b/hw/virtio/virtio-testdev.c
> @@ -0,0 +1,117 @@
> +#include "hw/virtio/virtio-bus.h"
> +
> +#define VIRTIO_ID_TESTDEV 0x
> +
> +#define TYPE_VIRTIO_TESTDEV "virtio-testdev"
> +#define VIRTIO_TESTDEV(obj) \
> +OBJECT_CHECK(VirtIOTestdev, (obj), TYPE_VIRTIO_TESTDEV)
> +
> +#define TESTDEV_MAJOR_VER 1
> +#define TESTDEV_MINOR_VER 1
> +
> +#define CONFIG_SIZE 0x100
> +
> +enum {
> +VERSION = 1,
> +CLEAR,
> +EXIT,
> +};
> +
> +enum { TOKEN, NARGS, NRETS, ARG1, ARG2, ARG3, ARG4, };
> +
> +#define RET1(nargs) (ARG1 + (nargs) + 0)
> +#define RET2(nargs) (ARG1 + (nargs) + 1)
> +#define RET3(nargs) (ARG1 + (nargs) + 2)
> +#define RET4(nargs) (ARG1 + (nargs) + 3)
> +
> +#define calc_len(nargs, nrets) ((3 + (nargs) + (nrets)) * 4)
> +
> +typedef struct VirtIOTestdev {
> +VirtIODevice parent_obj;
> +uint8_t config[CONFIG_SIZE];
> +size_t len; /* currently used bytes */
> +} VirtIOTestdev;
> +
> +static void virtio_testdev_get_config(VirtIODevice *vdev, uint8_t
> *config_data)
> +{
> +VirtIOTestdev *dev = VIRTIO_TESTDEV(vdev);
> +memcpy(config_data, &dev->config[0], dev->len);
> +}
> +
> +static void virtio_testdev_set_config(VirtIODevice *vdev,
> + const uint8_t *config_data)
> +{
> +VirtIOTestdev *dev = VIRTIO_TESTDEV(vdev);
> +uint32_t *c32 = (uint32_t *)&dev->config[0];
> +uint32_t token, nargs, nrets;
> +
> +memcpy(c32, config_data, 32); /* assume write is in first 32 bytes,
> + we can grab more later, if needed */
> +token = le32_to_cpu(c32[TOKEN]);
> +nargs = le32_to_cpu(c32[NARGS]);
> +nrets = le32_to_cpu(c32[NRETS]);
> +
> +if (!token) {
> +return;
> +}
> +
> +switch (token) {
> +case VERSION:
> +c32[RET1(nargs)] =
> +cpu_to_le32((TESTDEV_MAJOR_VER << 16) | TESTDEV_MINOR_VER);
> +break;
> +case CLEAR:
> +memset(c32, 0, CONFIG_SIZE);
> +break;
> +case EXIT:
> +exit((le32_to_cpu(c32[ARG1]) << 1) | 1);
> +default:
> +break;
> +}
> +
> +c32[TOKEN] = 0;
> +dev->len = calc_len(nargs, nrets);
> +}
> +
> +static uint32_t virtio_testdev_get_features(VirtIODevice *vdev, uint32_t f)
> +{
> +return f;
> +}
> +
> +static int virtio_testdev_init(VirtIODevice *vdev)
> +{
> +virtio_init(vdev, "virtio-testdev", VIRTIO_ID_TESTDEV, CONFIG_SIZE);
> +return 0;
> +}
> +
> +static int virtio_testdev_exit(DeviceState *qdev)
> +{
> +virtio_cleanup(VIRTIO_DEVICE(qdev));
> +return 0;
> +}
> +
> +static void virtio_testdev_class_init(ObjectClass *klass, void *data)
> +{
> +DeviceClass *dc = DEVICE_CLASS(kla
[Qemu-devel] [PATCH] virtio: Introduce virtio-testdev
This is a virtio version of hw/misc/debugexit and should evolve into a
virtio version of pc-testdev. pc-testdev uses the PC's ISA bus, whereas
this testdev can be plugged into a virtio-mmio transport, which is
needed for kvm-unit-tests/arm. virtio-testdev uses the virtio device
config space as a communication channel, and implements an RTAS-like
protocol through it allowing guests to execute commands. Only three
commands are currently implemented;
1) VERSION: for version compatibility checks
2) CLEAR: set all the config space back to zero
3) EXIT:exit() from qemu with a status code
Note, the protocol also requires all data passing through the config
space to be in little-endian.
See [1] for an example of a driver for this device.
[1]
https://github.com/rhdrjones/kvm-unit-tests/blob/ff8df5378ffccfbdf25fe79241837e61eb2258e1/lib/virtio-testdev.c
Signed-off-by: Andrew Jones
---
default-configs/arm-softmmu.mak | 2 +
hw/virtio/Makefile.objs | 1 +
hw/virtio/virtio-testdev.c | 117
3 files changed, 120 insertions(+)
create mode 100644 hw/virtio/virtio-testdev.c
diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index ac0815d66310f..56f8086e61974 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -80,3 +80,5 @@ CONFIG_VERSATILE_PCI=y
CONFIG_VERSATILE_I2C=y
CONFIG_SDHCI=y
+
+CONFIG_VIRTIO_TESTDEV=y
diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
index 1ba53d9cc316c..b3d16d522f54b 100644
--- a/hw/virtio/Makefile.objs
+++ b/hw/virtio/Makefile.objs
@@ -3,6 +3,7 @@ common-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o
common-obj-y += virtio-bus.o
common-obj-y += virtio-mmio.o
common-obj-$(CONFIG_VIRTIO_BLK_DATA_PLANE) += dataplane/
+common-obj-$(CONFIG_VIRTIO_TESTDEV) += virtio-testdev.o
obj-y += virtio.o virtio-balloon.o
obj-$(CONFIG_LINUX) += vhost.o
diff --git a/hw/virtio/virtio-testdev.c b/hw/virtio/virtio-testdev.c
new file mode 100644
index 0..d6852d563702e
--- /dev/null
+++ b/hw/virtio/virtio-testdev.c
@@ -0,0 +1,117 @@
+#include "hw/virtio/virtio-bus.h"
+
+#define VIRTIO_ID_TESTDEV 0x
+
+#define TYPE_VIRTIO_TESTDEV "virtio-testdev"
+#define VIRTIO_TESTDEV(obj) \
+OBJECT_CHECK(VirtIOTestdev, (obj), TYPE_VIRTIO_TESTDEV)
+
+#define TESTDEV_MAJOR_VER 1
+#define TESTDEV_MINOR_VER 1
+
+#define CONFIG_SIZE 0x100
+
+enum {
+VERSION = 1,
+CLEAR,
+EXIT,
+};
+
+enum { TOKEN, NARGS, NRETS, ARG1, ARG2, ARG3, ARG4, };
+
+#define RET1(nargs) (ARG1 + (nargs) + 0)
+#define RET2(nargs) (ARG1 + (nargs) + 1)
+#define RET3(nargs) (ARG1 + (nargs) + 2)
+#define RET4(nargs) (ARG1 + (nargs) + 3)
+
+#define calc_len(nargs, nrets) ((3 + (nargs) + (nrets)) * 4)
+
+typedef struct VirtIOTestdev {
+VirtIODevice parent_obj;
+uint8_t config[CONFIG_SIZE];
+size_t len; /* currently used bytes */
+} VirtIOTestdev;
+
+static void virtio_testdev_get_config(VirtIODevice *vdev, uint8_t *config_data)
+{
+VirtIOTestdev *dev = VIRTIO_TESTDEV(vdev);
+memcpy(config_data, &dev->config[0], dev->len);
+}
+
+static void virtio_testdev_set_config(VirtIODevice *vdev,
+ const uint8_t *config_data)
+{
+VirtIOTestdev *dev = VIRTIO_TESTDEV(vdev);
+uint32_t *c32 = (uint32_t *)&dev->config[0];
+uint32_t token, nargs, nrets;
+
+memcpy(c32, config_data, 32); /* assume write is in first 32 bytes,
+ we can grab more later, if needed */
+token = le32_to_cpu(c32[TOKEN]);
+nargs = le32_to_cpu(c32[NARGS]);
+nrets = le32_to_cpu(c32[NRETS]);
+
+if (!token) {
+return;
+}
+
+switch (token) {
+case VERSION:
+c32[RET1(nargs)] =
+cpu_to_le32((TESTDEV_MAJOR_VER << 16) | TESTDEV_MINOR_VER);
+break;
+case CLEAR:
+memset(c32, 0, CONFIG_SIZE);
+break;
+case EXIT:
+exit((le32_to_cpu(c32[ARG1]) << 1) | 1);
+default:
+break;
+}
+
+c32[TOKEN] = 0;
+dev->len = calc_len(nargs, nrets);
+}
+
+static uint32_t virtio_testdev_get_features(VirtIODevice *vdev, uint32_t f)
+{
+return f;
+}
+
+static int virtio_testdev_init(VirtIODevice *vdev)
+{
+virtio_init(vdev, "virtio-testdev", VIRTIO_ID_TESTDEV, CONFIG_SIZE);
+return 0;
+}
+
+static int virtio_testdev_exit(DeviceState *qdev)
+{
+virtio_cleanup(VIRTIO_DEVICE(qdev));
+return 0;
+}
+
+static void virtio_testdev_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+dc->exit = virtio_testdev_exit;
+set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+vdc->init = virtio_testdev_init;
+vdc->get_config = virtio_testdev_get_config;
+vdc->set_config = virtio_testdev_set_config;
+vdc->get_features = virtio_testdev_get_features;
+}
+
+static const TypeInfo virtio_testdev_info = {
+.name = TYPE_VIRTIO_TESTDE
