On Wed, Dec 02, 2009 at 07:50:33PM +0100, Juan Quintela wrote: > "Michael S. Tsirkin" <m...@redhat.com> wrote: > > On Wed, Dec 02, 2009 at 07:22:11PM +0100, Juan Quintela wrote: > >> "Michael S. Tsirkin" <m...@redhat.com> wrote: > >> > On Wed, Dec 02, 2009 at 01:04:13PM +0100, Juan Quintela wrote: > >> >> diff --git a/hw/virtio.c b/hw/virtio.c > >> >> index c136005..b565bf9 100644 > >> >> --- a/hw/virtio.c > >> >> +++ b/hw/virtio.c > >> >> @@ -643,8 +643,10 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f) > >> >> qemu_put_be32(f, vdev->vq[i].vring.num); > >> >> qemu_put_be64(f, vdev->vq[i].pa); > >> >> qemu_put_be16s(f, &vdev->vq[i].last_avail_idx); > >> >> - if (vdev->binding->save_queue) > >> >> - vdev->binding->save_queue(vdev->binding_opaque, i, f); > >> >> + if (vdev->type == VIRTIO_PCI && > >> >> + virtio_pci_msix_present(vdev->binding_opaque)) { > >> >> + qemu_put_be16s(f, &vdev->vq[i].vector); > >> >> + }
Hmm, I just noticed that this also assumes that vector # fits in 16 bit. correct for PCI, but might be better not assume this in virtio.c > >> >> } > >> >> } > >> >> > >> > > >> > I think this will break build on systems without PCI > >> > because virtio_pci.c is not linked in there. > >> > Correct? > >> > >> It compiles for syborg (it has pci). There are no other users. > >> > >> > Making generic virtio.c depend on virtio_pci.c looks > >> > wrong in any case. We have bindings to > >> > resolve exactly this dependency problem. > >> > >> There is no way that you throw "this" blob to vmstate and it will know > >> what to do there. if it is needed, we can create an empty > >> virtio_pci_msix_present() function for !CONFIG_PCI. > >> > >> Later, Juan. > > > > That's not the idea. virtio knows nothing about msix etc. > > this is patently false :) I will agree if you would have done > s/kwnows/shouldn't know/ :) > > int nvectors; > > this is a field of VirtIODevice. > and there is another one in virtio-pci. > (master)$ grep -c nvectors hw/virtio.c > 0 > (master)$ > vectors are the abstraction that we use. > And you can see know what I mean by incestuous relation between virtio > <-> virtio-pci. To make things more interesting, it becomes a threesome > with msix :( These are just callbacks, no reason to call them names :) > > This belongs > > in the binding. If you want to know the number of vectors, please put > > something like ->get_nvectors in the binding and call that to figure out > > whether virtio has multivector. > > We could do it whatever. The big problem here is that virtio devices > are (normally) virtio devices and pci devices. There is no way that > would fit it well with qemu. > > There are two options: > > - having virtio contain callbacks from virtio-pci. > - having virtio-pci contain a virtio device. Second one sounds sane. virtio provides functions, virtio-pci calls them. > One is bad and the other is worse :( > > Will take a look at creating that get_nvectors() helper. > > Later, Juan.