Re: [Qemu-devel] [RFC 3/7] vhost: New memory update functions

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Igor Mammedov (imammedo@redhat.com) wrote:
> On Wed, 29 Nov 2017 18:50:22 +0000
> "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> 
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > vhost_update_mem will replace the existing update mechanism.
> > They make use of the Flatview we have now to make the update simpler.
> > This commit just adds the basic structure.
> see below suggestion on dropping vhost_region_add/vhost_region_del
> 
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> >  hw/virtio/vhost.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 44 insertions(+)
> > 
> > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> > index 938253d1e8..c959a59fb3 100644
> > --- a/hw/virtio/vhost.c
> > +++ b/hw/virtio/vhost.c
> > @@ -629,6 +629,43 @@ static void vhost_begin(MemoryListener *listener)
> >      dev->mem_changed_start_addr = -1;
> >  }
> >  
> > +struct vhost_update_mem_tmp {
> > +    struct vhost_dev   *dev;
> > +    uint32_t nregions;
> > +    struct vhost_memory_region *regions;
> > +};
> > +
> > +/* Called for each MRS from vhost_update_mem */
> > +static int vhost_update_mem_cb(MemoryRegionSection *mrs, void *opaque)
> > +{
> > +    if (!vhost_section(mrs)) {
> > +        return 0;
> > +    }
> > +
> it's possible to move section tracking from vhost_region_add() to here
> 
>     ++dev->n_mem_sections;                                                       
>     dev->mem_sections = g_renew(MemoryRegionSection, dev->mem_sections,          
>                                 dev->n_mem_sections);                            
>     ;[dev->n_mem_sections - 1] = *section;                       
>     memory_region_ref(section->mr);  
> ...
> 
> > +static int vhost_update_mem(struct vhost_dev *dev)
> > +{
> > +    int res;
> > +    struct vhost_update_mem_tmp vtmp;
> > +    vtmp.regions = 0;
> > +    vtmp.nregions = 0;
> > +    vtmp.dev = dev;
>        
>  iterate over dev->mem_sections
>     memory_region_unref(section->mr);
>  g_free(dev->mem_sections)
>  dev->mem_sections = NULL;
> 
> with this you won't need vhost_region_add/del callbacks anymore

Yes, with the addition of a:
   dev->n_mem_sections = 0;

in the deletion bit, that seems to work nicely.  Added.

Thanks,

Dave

> 
> > +
> > +    res = address_space_iterate(&address_space_memory,
> > +                                vhost_update_mem_cb, &vtmp);
> > +    if (res) {
> > +        goto out;
> > +    }
> > +
> > +    /* TODO */
> > +out:
> > +    g_free(vtmp.regions);
> > +    return res;
> > +}
> > +
> >  static void vhost_commit(MemoryListener *listener)
> >  {
> >      struct vhost_dev *dev = container_of(listener, struct vhost_dev,
> > @@ -648,6 +685,10 @@ static void vhost_commit(MemoryListener *listener)
> >          return;
> >      }
> >  
> > +    if (vhost_update_mem(dev)) {
> > +        return;
> > +    }
> > +
> >      if (dev->started) {
> >          start_addr = dev->mem_changed_start_addr;
> >          size = dev->mem_changed_end_addr - dev->mem_changed_start_addr + 1;
> > @@ -1523,6 +1564,9 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
> >          goto fail_features;
> >      }
> >  
> > +    if (vhost_update_mem(hdev)) {
> > +        goto fail_mem;
> > +    }
> >      if (vhost_dev_has_iommu(hdev)) {
> >          memory_listener_register(&hdev->iommu_listener, vdev->dma_as);
> >      }
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK