| Message ID | 20100111171649.GB11936@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 01/11/2010 11:16 AM, Michael S. Tsirkin wrote: > vhost needs physical addresses for > ring so expose that structure. > > Signed-off-by: Michael S. Tsirkin<mst@redhat.com> > I think accessor functions might make more sense. Regards, Anthony Liguori
On Tue, Jan 12, 2010 at 04:32:44PM -0600, Anthony Liguori wrote: > On 01/11/2010 11:16 AM, Michael S. Tsirkin wrote: >> vhost needs physical addresses for >> ring so expose that structure. >> >> Signed-off-by: Michael S. Tsirkin<mst@redhat.com> >> > > I think accessor functions might make more sense. > > Regards, > > Anthony Liguori Well, take a look: typedef struct VRing { unsigned int num; target_phys_addr_t desc; target_phys_addr_t avail; target_phys_addr_t used; } VRing; struct VirtQueue { VRing vring; target_phys_addr_t pa; uint16_t last_avail_idx; int inuse; uint16_t vector; void (*handle_output)(VirtIODevice *vdev, VirtQueue *vq); VirtIODevice *vdev; EventNotifier guest_notifier; EventNotifier host_notifier; } (Notifiers are added by patches I will shortly post). We need at least: unsigned int num; target_phys_addr_t desc; target_phys_addr_t avail; target_phys_addr_t used; VRing vring; target_phys_addr_t pa; uint16_t vector; VirtIODevice *vdev; EventNotifier guest_notifier; EventNotifier host_notifier; We do not need: uint16_t last_avail_idx; int inuse; void (*handle_output)(VirtIODevice *vdev, VirtQueue *vq); (last_avail_idx will be needed if we ever want to move a running guest from kernel to userspace or back). IOW, most of VirtQueue needs to be exposed. So - do we really want accessors?
On 01/25/2010 01:10 PM, Michael S. Tsirkin wrote: > On Tue, Jan 12, 2010 at 04:32:44PM -0600, Anthony Liguori wrote: > >> On 01/11/2010 11:16 AM, Michael S. Tsirkin wrote: >> >>> vhost needs physical addresses for >>> ring so expose that structure. >>> >>> Signed-off-by: Michael S. Tsirkin<mst@redhat.com> >>> >>> >> I think accessor functions might make more sense. >> >> Regards, >> >> Anthony Liguori >> > Well, take a look: > > typedef struct VRing > { > unsigned int num; > target_phys_addr_t desc; > target_phys_addr_t avail; > target_phys_addr_t used; > } VRing; > > struct VirtQueue > { > VRing vring; > target_phys_addr_t pa; > uint16_t last_avail_idx; > int inuse; > uint16_t vector; > void (*handle_output)(VirtIODevice *vdev, VirtQueue *vq); > VirtIODevice *vdev; > EventNotifier guest_notifier; > EventNotifier host_notifier; > } > > (Notifiers are added by patches I will shortly post). > We need at least: > > unsigned int num; > target_phys_addr_t desc; > target_phys_addr_t avail; > target_phys_addr_t used; > VRing vring; > target_phys_addr_t pa; > You need num and the physical address of the ring. The location of desc/available/used are all fixed offsets rom the start of the ring (by spec). > uint16_t vector; > VirtIODevice *vdev; > EventNotifier guest_notifier; > EventNotifier host_notifier; > guest_notifier and host_notifier appear to be new. I don't see them in your previous patchset. I can't see why you need vdev. The backlink is there only for handle_output because handle_output is passed a VirtQueue. I also don't see you using vector in this patch series. So at least from what you posted in the RFC, you just need to know the ring pa and num which would be totally reasonable to have an accessor for. Regards, Anthony Liguori
diff --git a/hw/virtio.c b/hw/virtio.c index 3c609ce..8e3c9ad 100644 --- a/hw/virtio.c +++ b/hw/virtio.c @@ -49,24 +49,6 @@ typedef struct VRingUsed VRingUsedElem ring[0]; } VRingUsed; -typedef struct VRing -{ - unsigned int num; - target_phys_addr_t desc; - target_phys_addr_t avail; - target_phys_addr_t used; -} VRing; - -struct VirtQueue -{ - VRing vring; - target_phys_addr_t pa; - uint16_t last_avail_idx; - int inuse; - uint16_t vector; - void (*handle_output)(VirtIODevice *vdev, VirtQueue *vq); -}; - #define VIRTIO_PCI_QUEUE_MAX 16 /* virt queue functions */ diff --git a/hw/virtio.h b/hw/virtio.h index 3994cc9..ca840e1 100644 --- a/hw/virtio.h +++ b/hw/virtio.h @@ -180,5 +180,22 @@ void virtio_net_exit(VirtIODevice *vdev); DEFINE_PROP_BIT("indirect_desc", _state, _field, \ VIRTIO_RING_F_INDIRECT_DESC, true) +typedef struct VRing +{ + unsigned int num; + target_phys_addr_t desc; + target_phys_addr_t avail; + target_phys_addr_t used; +} VRing; + +struct VirtQueue +{ + VRing vring; + target_phys_addr_t pa; + uint16_t last_avail_idx; + int inuse; + uint16_t vector; + void (*handle_output)(VirtIODevice *vdev, VirtQueue *vq); +}; #endif
vhost needs physical addresses for ring so expose that structure. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- hw/virtio.c | 18 ------------------ hw/virtio.h | 17 +++++++++++++++++ 2 files changed, 17 insertions(+), 18 deletions(-)