On Mon, Jun 09, 2014 at 11:14:31AM +0200, Christoffer Dall wrote:
> On Mon, Jun 09, 2014 at 11:02:57AM +0200, Andrew Jones wrote:
> > 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.
> >
>
> So I think the tradeoff here is very small, it's super easy to fix it so
> you can support physical addresses > 32 bits in this part of the code.
> The difficult thing comes when you start dereferencing them, but since
> we may add virtual addresses at some point too, I really think adding a
> trivial ioremap function and start using that will make our lives much
> easier down the line:
>
> /* We don't support virtual addresses yet, trivial identity implementation */
> void __iomem *ioremap(phys_addr_t phys_addr)
> {
> assert(sizeof(long) == 8 || !(base >> 32));
> return (void *)(unsigned long)phys_addr;
> }
>
> But I can submit that patch after this code goes in if you think it's
> too much of a headache at this point?
Nah, I'll steal your idea and do it now :-)
>
> Thanks,
> -Christoffer
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
