On Mon, Feb 03, 2014 at 04:44:31PM +0100, Andrew Jones wrote:
> On Sat, Feb 01, 2014 at 06:27:37PM -0800, Christoffer Dall wrote:
> > 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 <drjo...@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?
>
> right. Actually, we could probably always assume regidx will be zero,
> remove that param, and then rename this.
>
> >
> > > +{
> > > + 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() ?
>
> yes, but we didn't save the result, so we need to go back to the
> node's reg property again and get it. At least we know which node
> to go to and {address,size}_cells are cached. Hmm, we might as
> well add an addr field to virtio_dt_device_data, allowing match
> to save it there. I'll do that.
>
on second read, either way will work. Perhaps just a comment explaining
why we read out data twice would work otherwise. Up to you.
> >
> > > +
> > > + 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.d...@linaro.org>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
