[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH 15/41] virtio: remove save/load_queue for virtio
From: |
Michael S. Tsirkin |
Subject: |
[Qemu-devel] Re: [PATCH 15/41] virtio: remove save/load_queue for virtio |
Date: |
Wed, 2 Dec 2009 20:57:23 +0200 |
User-agent: |
Mutt/1.5.19 (2009-01-05) |
On Wed, Dec 02, 2009 at 07:50:33PM +0100, Juan Quintela wrote:
> "Michael S. Tsirkin" <address@hidden> wrote:
> > On Wed, Dec 02, 2009 at 07:22:11PM +0100, Juan Quintela wrote:
> >> "Michael S. Tsirkin" <address@hidden> 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.
- [Qemu-devel] Re: [PATCH 11/41] virtio: Introduce type field to distingish between PCI and Syborg, (continued)
[Qemu-devel] [PATCH 16/41] virtio: Add num_pci_queues field, Juan Quintela, 2009/12/02
[Qemu-devel] [PATCH 17/41] virtio: split virtio_post_load() from virtio_load(), Juan Quintela, 2009/12/02
[Qemu-devel] [PATCH 18/41] virtio: change config_len type to int32_t, Juan Quintela, 2009/12/02
[Qemu-devel] [PATCH 21/41] virtio: port to vmstate, Juan Quintela, 2009/12/02
[Qemu-devel] [PATCH 19/41] virtio: use the right types for VirtQueue elements, Juan Quintela, 2009/12/02