On Fri, Jun 06, 2014 at 08:39:18PM +0200, Christoffer Dall wrote:
> On Thu, Apr 10, 2014 at 06:56:52PM +0200, Andrew Jones wrote:
> > Support the bare minimum of virtio to enable access to the virtio-mmio
> > config space of a device. Currently this implementation must use a
> > device tree to find the device.
> >
> > Signed-off-by: Andrew Jones <drjones@xxxxxxxxxx>
> > ---
> > v4:
> > - split from the virtio-testdev patch
> > - search a table to "discover" that the device must be DT/virtio-mmio,
> > which doesn't change anything, but looks less hacky than comments
> > saying the device must be DT/virtio-mmio...
> > - manage own pool of virtio-mmio pre-allocated device structures in
> > order to avoid needing access to the heap
> > ---
> > lib/libcflat.h | 3 ++
> > lib/virtio.c | 166 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > lib/virtio.h | 89 +++++++++++++++++++++++++++++++
> > 3 files changed, 258 insertions(+)
> > create mode 100644 lib/virtio.c
> > create mode 100644 lib/virtio.h
> >
> > diff --git a/lib/libcflat.h b/lib/libcflat.h
> > index c7c31be1cc8e5..5bb66d01dfc53 100644
> > --- a/lib/libcflat.h
> > +++ b/lib/libcflat.h
> > @@ -62,6 +62,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)
> >
> > diff --git a/lib/virtio.c b/lib/virtio.c
> > new file mode 100644
> > index 0000000000000..e7161ff591e4c
> > --- /dev/null
> > +++ b/lib/virtio.c
> > @@ -0,0 +1,166 @@
> > +/*
> > + * Copyright (C) 2014, Red Hat Inc, Andrew Jones <drjones@xxxxxxxxxx>
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.
> > + */
> > +#include "libcflat.h"
> > +#include "devicetree.h"
> > +#include "asm/spinlock.h"
> > +#include "asm/io.h"
> > +#include "virtio.h"
> > +
> > +enum virtio_hwdesc_type {
> > + VIRTIO_HWDESC_TYPE_DT = 0, /* device tree */
> > + NR_VIRTIO_HWDESC_TYPES,
> > +};
> > +
> > +enum virtio_bus_type {
> > + VIRTIO_BUS_TYPE_MMIO = 0, /* virtio-mmio */
> > + NR_VIRTIO_BUS_TYPES,
> > +};
> > +
> > +struct virtio_bind_bus {
> > + bool (*hwdesc_probe)(void);
> > + struct virtio_dev *(*device_bind)(u32 devid);
> > +};
> > +
> > +static struct virtio_dev *vm_dt_device_bind(u32 devid);
> > +
> > +static struct virtio_bind_bus
> > +virtio_bind_busses[NR_VIRTIO_HWDESC_TYPES][NR_VIRTIO_BUS_TYPES] = {
> > +
> > +[VIRTIO_HWDESC_TYPE_DT] = {
> > +
> > + [VIRTIO_BUS_TYPE_MMIO] = {
> > + .hwdesc_probe = dt_available,
> > + .device_bind = vm_dt_device_bind,
> > + },
> > +},
> > +};
> > +
> > +struct virtio_dev *virtio_bind(u32 devid)
> > +{
> > + struct virtio_bind_bus *bus;
> > + struct virtio_dev *dev;
> > + int i, j;
> > +
> > + for (i = 0; i < NR_VIRTIO_HWDESC_TYPES; ++i) {
> > + for (j = 0; j < NR_VIRTIO_BUS_TYPES; ++j) {
> > +
> > + bus = &virtio_bind_busses[i][j];
> > +
> > + if (!bus->hwdesc_probe())
> > + continue;
> > +
> > + dev = bus->device_bind(devid);
> > + if (dev)
> > + return dev;
> > + }
> > + }
> > +
> > + return NULL;
> > +}
> > +
> > +/******************************************************
> > + * virtio-mmio support (config space only)
> > + ******************************************************/
> > +
> > +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);
> > +}
> > +
> > +#define NR_VM_DEVICES 32
> > +static struct spinlock vm_lock;
> > +static struct virtio_mmio_dev vm_devs[NR_VM_DEVICES];
> > +static struct virtio_conf_ops vm_confs[NR_VM_DEVICES];
> > +static int nr_vm_devs;
> > +
> > +static struct virtio_mmio_dev *vm_new_device(u32 devid)
> > +{
> > + struct virtio_mmio_dev *vmdev;
> > +
> > + if (nr_vm_devs >= NR_VM_DEVICES)
> > + return NULL;
> > +
> > + spin_lock(&vm_lock);
> > + vmdev = &vm_devs[nr_vm_devs];
> > + vmdev->vdev.config = &vm_confs[nr_vm_devs];
>
> this seems a bit weird, will we ever share vm_confs between vm_devs or
> change the vm_conf pointer later on? Could vm_conf just be part of the
> vm_dev struct and you could just have a virtio_mmio_conf_init() to set
> the get/set functions?
I can do that, but it'll require a typedef for struct virtio_dev to
appease the compiler.
>
> > + ++nr_vm_devs;
> > + spin_unlock(&vm_lock);
> > +
> > + vmdev->vdev.id.device = devid;
> > + vmdev->vdev.id.vendor = -1;
> > + vmdev->vdev.config->get = vm_get;
> > + vmdev->vdev.config->set = vm_set;
> > +
> > + return vmdev;
> > +}
> > +
> > +/******************************************************
> > + * virtio-mmio device tree support
> > + ******************************************************/
> > +
> > +struct vm_dt_info {
> > + u32 devid;
> > + void *base;
>
> won't work for LPAE and 32-bit arm systems, can't you use
> dt_device_bind_node?
>
> I guess that requires you to have an ioremap function, but it sure seems
> cleaner to give up there if you don't have proper memory mapping
> features yet....
Right. I was just skipping past that complexity for now, as mach-virt's
DT is currently "nice" (32-bit only addrs). I agree we need to handle
LPAE addrs on arm eventually though.
>
> > +};
> > +
> > +static int vm_dt_match(const struct dt_device *dev, int fdtnode)
> > +{
> > + struct vm_dt_info *info = (struct vm_dt_info *)dev->info;
> > + dt_pbus_addr_t base;
> > +
> > + dt_device_bind_node((struct dt_device *)dev, fdtnode);
> > +
> > + assert(dt_pbus_get_baseaddr(dev, &base) == 0);
> > + assert(sizeof(long) == 8 || !(base >> 32));
>
> again, this assert will break with >32bit LPAE addresses on 32-bit arm systems.
>
> so, not sure if it's an assert, or if you should test for it and print
> a more benign error for these types of devices.
It's supposed to be a crash-and-burn assert, but a message stating
something like >32-bit addrs on arm is not yet supported would certainly be
nicer. Of course that message would only make it out at this point if our uart
address guess is correct.
>
> > +
> > + info->base = (void *)(unsigned long)base;
>
> why do you need to cast it twice?
Same as above (as we still don't support >32-bit addrs on arm). This keeps
the compiler from complaining when casting a dt_pbus_addr_t (u64) to void*
on 32-bit.
>
> > +
> > + return readl(info->base + VIRTIO_MMIO_DEVICE_ID) == info->devid;
> > +}
> > +
> > +static struct virtio_dev *vm_dt_device_bind(u32 devid)
> > +{
> > + struct virtio_mmio_dev *vmdev;
> > + struct dt_device dt_dev;
> > + struct dt_bus dt_bus;
> > + struct vm_dt_info info;
> > + int node;
> > +
> > + dt_bus_init_defaults(&dt_bus);
> > + dt_bus.match = vm_dt_match;
> > +
> > + info.devid = devid;
> > +
> > + dt_device_init(&dt_dev, &dt_bus, &info);
> > +
> > + node = dt_device_find_compatible(&dt_dev, "virtio,mmio");
> > + assert(node >= 0 || node == -FDT_ERR_NOTFOUND);
> > +
> > + if (node == -FDT_ERR_NOTFOUND)
> > + return NULL;
> > +
> > + vmdev = vm_new_device(devid);
> > + vmdev->base = info.base;
> > +
> > + return &vmdev->vdev;
> > +}
> > diff --git a/lib/virtio.h b/lib/virtio.h
> > new file mode 100644
> > index 0000000000000..163b9912c4e05
> > --- /dev/null
> > +++ b/lib/virtio.h
> > @@ -0,0 +1,89 @@
> > +#ifndef _VIRTIO_H_
> > +#define _VIRTIO_H_
> > +/*
> > + * A minimal implementation of virtio for virtio-mmio config space
> > + * access.
> > + *
> > + * Copyright (C) 2014, Red Hat Inc, Andrew Jones <drjones@xxxxxxxxxx>
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.
> > + */
> > +#include "libcflat.h"
> > +
> > +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);
> > +};
> > +
> > +extern struct virtio_dev *virtio_bind(u32 devid);
> > +
> > +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);
> > +}
> > +
> > +/******************************************************
> > + * virtio-mmio
> > + ******************************************************/
> > +
> > +#define VIRTIO_MMIO_DEVICE_ID 0x008
> > +#define VIRTIO_MMIO_CONFIG 0x100
> > +
> > +#define to_virtio_mmio_dev(vdev_ptr) \
> > + container_of(vdev_ptr, struct virtio_mmio_dev, vdev)
> > +
> > +struct virtio_mmio_dev {
> > + struct virtio_dev vdev;
> > + void *base;
> > +};
> > +
> > +#endif /* _VIRTIO_H_ */
> > --
> > 1.8.1.4
> >
>
> -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
