On Fri, Jul 11, 2014 at 11:08:28AM +0200, Andrew Jones wrote:
> On Fri, Jul 11, 2014 at 10:32:51AM +0200, Paolo Bonzini wrote:
> > Il 11/07/2014 10:19, Andrew Jones ha scritto:
> > >+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_device *(*device_bind)(u32 devid);
> > >+};
> > >+
> > >+static struct virtio_device *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,
> > >+ },
> > >+},
> > >+};
> > >+
> >
> > If you put this in lib/virtio.c, it is overengineered. It would make sense
> > if something else provided virtio_bind_busses[][].
>
> yeah, I acknowledged in the v4 changelog that it was unnecessary at the
> moment, but it's there to reduce the ugly hardcodeding that this simplified
> version currently needs, and who knows what the future shall bring.
>
> >
> > I suggest that you drop it and split this file in four:
> >
> > lib/virtio.c
> > lib/virtio-mmio.c
>
> This split already crossed my mind.
I made this split.
>
> > lib/virtio-mmio-dt.c
>
> I'm not sure we need this one, but OK.
I opted not to make this one. To me it seems reasonable to keep virtio-mmio
and virtio-mmio-dt code together, if for nothing else just to lean more
on the reduce file count side of the trade-off.
>
> > lib/arm/virtio.c
> >
> > where virtio_bind is in lib/arm/virtio.c.
> >
>
> Well, virtio_bind will still need to be in lib/virtio.c, but just as
> a wrapper to arch_virtio_bind. And, I'm inclined to keep virtio_bind_busses
> in arm's arch_virtio_bind.
>
I dropped the idea of depending on arch-specific implementations being
necessary. As the whole reason to get rid of my table is to simplify
things, then I think all we need for now is
virtio_bind
virtio_mmio_bind
virtio_mmio_dt_bind
to maintain abstractions and give room for expansions (virtio-pci).
drew
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
