Re: [PATCH 12/17] Introduce virtio-testdev

Christoffer Dall
2014-02-02T02:27:34Z

On Tue, Jan 21, 2014 at 05:21:58PM +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@redhat.com>
>
> ---
> v3:
> - switched from iomaps to devicetree
> - fixed Christoffer's nits
> - don't call halt() explicitly, let exit() fall back to it
> - vm_{get,set} now {read,write} byte by byte like the kernel
> 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 | 126 ++++++++++++++++++++++++++++++++++++++++++++++
> lib/virtio.h | 74 +++++++++++++++++++++++++++
> 5 files changed, 351 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 fdaaf2a8ab31d..84b8783bacfdc 100644
> --- a/lib/libcflat.h
> +++ b/lib/libcflat.h
> @@ -60,6 +60,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 __unused __attribute__((__unused__))
> #define NULL ((void *)0UL)
> diff --git a/lib/virtio-testdev.c b/lib/virtio-testdev.c
> new file mode 100644
> index 0000000000000..d4a69ede12a7a
> --- /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
> + * the token's operation until all args are in place. Then issue the op,
> + * and then read the return values. Reading the return values (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");
> + exit(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");
> + exit(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..8abc8b2399a08
> --- /dev/null
> +++ b/lib/virtio.c
> @@ -0,0 +1,126 @@
> +#include "libcflat.h"
> +#include "devicetree.h"
> +#include "libio.h"
> +#include "heap.h"
> +#include "virtio.h"
> +
> +struct virtio_dt_device_data {
> + u32 device;
> + struct dt_bus *bus;
> + int node;
> + /* cached variables */
> + u32 address_cells, size_cells;
> +};
> +
> +static void virtio_dt_device_data_init(struct virtio_dt_device_data *data,
> + u32 device, struct dt_bus *bus)
> +{
> + memset(data, 0, sizeof(struct virtio_dt_device_data));
> + data->device = device;
> + data->bus = bus;
> +}
> +
> +static int vm_dt_bus_match(const struct dt_bus *bus, int nodeoffset);
> +static struct virtio_dev *vm_bind(struct virtio_dt_device_data *data);
> +
> +struct virtio_dev *virtio_bind(u32 device)
> +{
> + struct virtio_dt_device_data data;
> + struct dt_bus bus;
> + const char *compatible;
> +
> + /*
> + * currently we only support virtio-mmio
> + */
> + compatible = "virtio,mmio";
> + dt_bus_init_defaults(&bus, compatible);
> + bus.match = vm_dt_bus_match;
> + bus.private = (void *)&data;
> + virtio_dt_device_data_init(&data, device, &bus);
> +
> + data.node = dt_bus_find_device_compatible(&bus, compatible);
> + if (data.node < 0) {
> + printf("virtio bind for device id 0x%x failed, "
> + "fdt_error: %s\n", device, dt_strerror(data.node));
> + return NULL;
> + }
> +
> + return vm_bind(&data);
> +}
> +
> +static int virtio_dt_bus_translate_reg(int nodeoffset,
> + const struct dt_bus *bus,
> + int regidx, void **addr,
> + size_t *size)

so this is really virtio_dt_bus_get_addr right?

> +{
> + struct virtio_dt_device_data *data =
> + (struct virtio_dt_device_data *)bus->private;
> +
> + return __dt_bus_translate_reg(nodeoffset, bus, regidx,
> + &data->address_cells, &data->size_cells, addr, size);
> +}
> +
> +/******************************************************
> + * virtio_mmio
> + ******************************************************/
> +
> +static int vm_dt_bus_match(const struct dt_bus *bus, int nodeoffset)
> +{
> + struct virtio_dt_device_data *data =
> + (struct virtio_dt_device_data *)bus->private;
> + void *addr;
> + int ret;
> +
> + ret = virtio_dt_bus_translate_reg(nodeoffset, bus, 0, &addr, NULL);
> + if (ret < 0) {
> + printf("can't get reg! fdt_error: %s\n", dt_strerror(ret));
> + return ret;
> + }
> +
> + return readl(addr + VIRTIO_MMIO_DEVICE_ID) == data->device;
> +}
> +
> +#define to_virtio_mmio_dev(vdev_ptr) \
> + container_of(vdev_ptr, struct virtio_mmio_dev, vdev)
> +
> +static void vm_get(struct virtio_dev *vdev, unsigned offset,
> + void *buf, unsigned len)
> +{
> + struct virtio_mmio_dev *vmdev = to_virtio_mmio_dev(vdev);
> + u8 *p = buf;
> + unsigned i;
> +
> + for (i = 0; i < len; ++i)
> + p[i] = readb(vmdev->base + VIRTIO_MMIO_CONFIG + offset + i);
> +}
> +
> +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);
> + const u8 *p = buf;
> + unsigned i;
> +
> + for (i = 0; i < len; ++i)
> + writeb(p[i], vmdev->base + VIRTIO_MMIO_CONFIG + offset + i);
> +}
> +
> +static struct virtio_dev *vm_bind(struct virtio_dt_device_data *data)
> +{
> + struct virtio_mmio_dev *vmdev;
> + void *page;
> +
> + page = alloc_page();
> + vmdev = page;
> + vmdev->vdev.config = page + sizeof(struct virtio_mmio_dev);
> +
> + vmdev->vdev.id.device = data->device;
> + vmdev->vdev.id.vendor = -1;
> + vmdev->vdev.config->get = vm_get;
> + vmdev->vdev.config->set = vm_set;
> +
> + (void)virtio_dt_bus_translate_reg(data->node, data->bus, 0,
> + &vmdev->base, NULL);

didn't you already do this in vm_dt_bus_mach() ?

> +
> + return &vmdev->vdev;
> +}
> 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
>

otherwise:

Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>