Re: [PATCH 8/9] Introduce virtio-testdev

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Dec 04, 2013 at 05:42:56PM +0100, Andrew Jones wrote:
> virtio-testdev is a communication channel to qemu through virtio that
> can be used by test code to send commands. The only command currently
> implemented is EXIT, which allows the test code to exit with a given
> status code.
> 
> Signed-off-by: Andrew Jones <drjones@xxxxxxxxxx>
> 
> ---
> v2:
>  - use readl/writel framework (libio) [Christoffer Dall]
>  - keep the virtio abstraction in virtio-testdev [Alexander Graf]
> ---
>  lib/libcflat.h       |   3 ++
>  lib/virtio-testdev.c | 139 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/virtio-testdev.h |   9 ++++
>  lib/virtio.c         |  70 ++++++++++++++++++++++++++
>  lib/virtio.h         |  74 +++++++++++++++++++++++++++
>  5 files changed, 295 insertions(+)
>  create mode 100644 lib/virtio-testdev.c
>  create mode 100644 lib/virtio-testdev.h
>  create mode 100644 lib/virtio.c
>  create mode 100644 lib/virtio.h
> 
> diff --git a/lib/libcflat.h b/lib/libcflat.h
> index a1be635ab4ee9..197b703e53b46 100644
> --- a/lib/libcflat.h
> +++ b/lib/libcflat.h
> @@ -56,6 +56,9 @@ extern long atol(const char *ptr);
>  #define ARRAY_SIZE(_a)  (sizeof(_a)/sizeof((_a)[0]))
>  
>  #define offsetof(TYPE, MEMBER) __builtin_offsetof (TYPE, MEMBER)
> +#define container_of(ptr, type, member) ({				\
> +		const typeof( ((type *)0)->member ) *__mptr = (ptr);	\
> +		(type *)( (char *)__mptr - offsetof(type,member) );})
>  
>  #define NULL ((void *)0UL)
>  #include "errno.h"
> diff --git a/lib/virtio-testdev.c b/lib/virtio-testdev.c
> new file mode 100644
> index 0000000000000..93731a3565b81
> --- /dev/null
> +++ b/lib/virtio-testdev.c
> @@ -0,0 +1,139 @@
> +#include "libcflat.h"
> +#include "virtio.h"
> +
> +#define TESTDEV_MAJOR_VER 1
> +#define TESTDEV_MINOR_VER 1
> +
> +#define VIRTIO_ID_TESTDEV 0xffff
> +
> +#define CONFIG_SIZE 64
> +
> +enum {
> +	VERSION = 1,
> +	CLEAR,
> +	EXIT,
> +};
> +
> +#define TOKEN_OFFSET		0x0
> +#define NARGS_OFFSET		0x4
> +#define NRETS_OFFSET		0x8
> +#define ARG_OFFSET(n)		(0xc + (n) * 4)
> +#define __RET_OFFSET(nargs, n)	(ARG_OFFSET(nargs) + (n) * 4)
> +
> +static struct virtio_dev *testdev;
> +
> +static u32 testdev_readl(unsigned offset)
> +{
> +	if (offset > (CONFIG_SIZE - 4)) {
> +		printf("%s: offset 0x%x to big!\n", __func__, offset);
> +		exit(EINVAL);
> +	}
> +
> +	return virtio_config_readl(testdev, offset);
> +}
> +
> +static void testdev_writel(unsigned offset, u32 val)
> +{
> +	if (offset > (CONFIG_SIZE - 4)) {
> +		printf("%s: offset 0x%x to big!\n", __func__, offset);
> +		exit(EINVAL);
> +	}
> +
> +	virtio_config_writel(testdev, offset, val);
> +}
> +
> +/*
> + * We have to write all args; nargs, nrets, ... first to
> + * avoid executing token's operation until all args are in

the token's ?

> + * place. Then issue the op, and then read the rets. Reading

rets?  return values?

> + * the rets (or just sanity checking by reading token) will
> + * read a zero into qemu's copy of the token, which allows us
> + * to prepare additional ops without re-executing the last one.
> + */
> +void virtio_testdev(u32 token, u32 nargs, u32 nrets, ...)
> +{
> +	va_list va;
> +	unsigned off;
> +	u32 n;
> +
> +	if (!testdev)
> +		return;
> +
> +	testdev_writel(NARGS_OFFSET, nargs);
> +	testdev_writel(NRETS_OFFSET, nrets);
> +
> +	va_start(va, nrets);
> +
> +	off = ARG_OFFSET(0);
> +	n = nargs;
> +	while (n--) {
> +		testdev_writel(off, va_arg(va, unsigned));
> +		off += 4;
> +	}
> +
> +	/* this runs the op, but then resets token to zero */
> +	testdev_writel(TOKEN_OFFSET, token);
> +
> +	/* sanity check */
> +	if (testdev_readl(TOKEN_OFFSET) != 0) {
> +		printf("virtio-testdev token should always read as zero!\n");
> +		halt(EIO);
> +	}
> +
> +	off = __RET_OFFSET(nargs, 0);
> +	n = nrets;
> +	while (n--) {
> +		u32 *r = va_arg(va, unsigned *);
> +		*r = testdev_readl(off);
> +		off += 4;
> +	}
> +
> +	va_end(va);
> +}
> +
> +void virtio_testdev_version(u32 *version)
> +{
> +	virtio_testdev(VERSION, 0, 1, version);
> +}
> +
> +void virtio_testdev_clear(void)
> +{
> +	virtio_testdev(CLEAR, 0, 0);
> +}
> +
> +void virtio_testdev_exit(int code)
> +{
> +	virtio_testdev(EXIT, 1, 0, code);
> +}
> +
> +void virtio_testdev_init(void)
> +{
> +	u16 major, minor;
> +	u32 version;
> +
> +	testdev = virtio_bind(VIRTIO_ID_TESTDEV);
> +	if (testdev == NULL) {
> +		printf("Can't find virtio-testdev. "
> +		       "Is '-device virtio-testdev' "
> +		       "on the qemu command line?\n");
> +		halt(ENODEV);
> +	}
> +
> +	virtio_testdev_version(&version);
> +	major = version >> 16;
> +	minor = version & 0xffff;
> +
> +	if (major != TESTDEV_MAJOR_VER || minor < TESTDEV_MINOR_VER) {
> +		char *u = "qemu";
> +		if (major > TESTDEV_MAJOR_VER)
> +			u = "kvm-unit-tests";
> +		printf("Incompatible version of virtio-testdev: major = %d, "
> +		       "minor = %d\n", major, minor);
> +		printf("Update %s.\n", u);
> +		exit(ENODEV);
> +	}
> +
> +	if (minor > TESTDEV_MINOR_VER)
> +		printf("testdev has new features. "
> +		       "An update of kvm-unit-tests may be possible.\n");
> +}
> diff --git a/lib/virtio-testdev.h b/lib/virtio-testdev.h
> new file mode 100644
> index 0000000000000..c10bbfb591d0b
> --- /dev/null
> +++ b/lib/virtio-testdev.h
> @@ -0,0 +1,9 @@
> +#ifndef _VIRTIO_TESTDEV_H_
> +#define _VIRTIO_TESTDEV_H_
> +#include "libcflat.h"
> +
> +extern void virtio_testdev_init(void);
> +extern void virtio_testdev_version(u32 *version);
> +extern void virtio_testdev_clear(void);
> +extern void virtio_testdev_exit(int code);
> +#endif
> diff --git a/lib/virtio.c b/lib/virtio.c
> new file mode 100644
> index 0000000000000..c5ad1c85ff9a1
> --- /dev/null
> +++ b/lib/virtio.c
> @@ -0,0 +1,70 @@
> +#include "libcflat.h"
> +#include "libio.h"
> +#include "iomaps.h"
> +#include "heap.h"
> +#include "virtio.h"
> +
> +#define to_virtio_mmio_dev(vdev) \
> +	container_of(vdev, struct virtio_mmio_dev, vdev)

argh, macro pain, can you rename the parameter to _vdev or vdev_ptr or
something like that?

> +
> +static void vm_get(struct virtio_dev *vdev, unsigned offset,
> +		   void *buf, unsigned len)
> +{
> +	struct virtio_mmio_dev *vmdev = to_virtio_mmio_dev(vdev);
> +	void *addr = vmdev->base + VIRTIO_MMIO_CONFIG + offset;
> +	read_len(addr, buf, len);
> +}
> +
> +static void vm_set(struct virtio_dev *vdev, unsigned offset,
> +		   const void *buf, unsigned len)
> +{
> +	struct virtio_mmio_dev *vmdev = to_virtio_mmio_dev(vdev);
> +	void *addr = vmdev->base + VIRTIO_MMIO_CONFIG + offset;
> +	write_len(addr, buf, len);
> +}
> +
> +static struct virtio_dev *virtio_mmio_bind(const struct iomap *m, u32 device)
> +{
> +	struct virtio_mmio_dev *vmdev;
> +	void *page;
> +	u32 devid, i;
> +
> +	page = alloc_page();
> +	vmdev = page;
> +	vmdev->vdev.config = page + sizeof(struct virtio_mmio_dev);
> +
> +	vmdev->vdev.id.device = device;
> +	vmdev->vdev.id.vendor = -1;
> +	vmdev->vdev.config->get = vm_get;
> +	vmdev->vdev.config->set = vm_set;
> +
> +	device &= 0xffff;

what is this mask again?  Is this to correspond to the virtio PCI device
ID specs, but then shouldn't we also check the range, seems strange to
me to just ignore upper-bits garbage?

> +
> +	for (i = 0; i < m->nr; ++i) {
> +		vmdev->base = compat_ptr(m->addrs[i]);
> +		devid = readl(vmdev->base + VIRTIO_MMIO_DEVICE_ID);
> +		if ((devid & 0xffff) == device)
> +			break;
> +	}
> +
> +	if (i >= m->nr) {

this can just be 'if (i == m->nr)' right?

> +		printf("%s: Can't find device 0x%x.\n", __func__, device);

I would ditch the '.'

> +		free_page(page);
> +		return NULL;
> +	}
> +
> +	return &vmdev->vdev;
> +}
> +
> +struct virtio_dev *virtio_bind(u32 device)
> +{
> +	const struct iomap *m;
> +
> +	/* currently we only support virtio-mmio */
> +	m = iomaps_find_compatible("virtio,mmio");
> +	if (!m) {
> +		printf("%s: No virtio-mmio transports found!\n", __func__);
> +		return NULL;
> +	}
> +	return virtio_mmio_bind(m, device);
> +}
> diff --git a/lib/virtio.h b/lib/virtio.h
> new file mode 100644
> index 0000000000000..c30a8dcd105cd
> --- /dev/null
> +++ b/lib/virtio.h
> @@ -0,0 +1,74 @@
> +#ifndef _VIRTIO_H_
> +#define _VIRTIO_H_
> +#include "libcflat.h"
> +
> +#define VIRTIO_MMIO_DEVICE_ID	0x008
> +#define VIRTIO_MMIO_CONFIG	0x100
> +
> +struct virtio_devid {
> +	u32 device;
> +	u32 vendor;
> +};
> +
> +struct virtio_dev {
> +	struct virtio_devid id;
> +	struct virtio_conf_ops *config;
> +};
> +
> +struct virtio_conf_ops {
> +	void (*get)(struct virtio_dev *vdev, unsigned offset,
> +		    void *buf, unsigned len);
> +	void (*set)(struct virtio_dev *vdev, unsigned offset,
> +		    const void *buf, unsigned len);
> +};
> +
> +struct virtio_mmio_dev {
> +	struct virtio_dev vdev;
> +	void *base;
> +};
> +
> +static inline u8
> +virtio_config_readb(struct virtio_dev *vdev, unsigned offset)
> +{
> +	u8 val;
> +	vdev->config->get(vdev, offset, &val, 1);
> +	return val;
> +}
> +
> +static inline u16
> +virtio_config_readw(struct virtio_dev *vdev, unsigned offset)
> +{
> +	u16 val;
> +	vdev->config->get(vdev, offset, &val, 2);
> +	return val;
> +}
> +
> +static inline u32
> +virtio_config_readl(struct virtio_dev *vdev, unsigned offset)
> +{
> +	u32 val;
> +	vdev->config->get(vdev, offset, &val, 4);
> +	return val;
> +}
> +
> +static inline void
> +virtio_config_writeb(struct virtio_dev *vdev, unsigned offset, u8 val)
> +{
> +	vdev->config->set(vdev, offset, &val, 1);
> +}
> +
> +static inline void
> +virtio_config_writew(struct virtio_dev *vdev, unsigned offset, u16 val)
> +{
> +	vdev->config->set(vdev, offset, &val, 2);
> +}
> +
> +static inline void
> +virtio_config_writel(struct virtio_dev *vdev, unsigned offset, u32 val)
> +{
> +	vdev->config->set(vdev, offset, &val, 4);
> +}
> +
> +extern struct virtio_dev *virtio_bind(u32 device);
> +
> +#endif
> -- 
> 1.8.1.4
> 
> _______________________________________________
> kvmarm mailing list
> kvmarm@xxxxxxxxxxxxxxxxxxxxx
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm

-- 
Christoffer
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux