Re: [RFC PATCH v2 5/5] vhost_user: Implement mem_read/mem_write handlers

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3122 bytes --]

On Wed, Sep 04, 2024 at 03:01:06PM +0200, Albert Esteve wrote:
> On Thu, Jul 11, 2024 at 10:55 AM Stefan Hajnoczi <stefanha@redhat.com>
> wrote:
> 
> > On Fri, Jun 28, 2024 at 04:57:10PM +0200, Albert Esteve wrote:
> > > Implement function handlers for memory read and write
> > > operations.
> > >
> > > Signed-off-by: Albert Esteve <aesteve@redhat.com>
> > > ---
> > >  hw/virtio/vhost-user.c | 34 ++++++++++++++++++++++++++++++----
> > >  1 file changed, 30 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > > index 18cacb2d68..79becbc87b 100644
> > > --- a/hw/virtio/vhost-user.c
> > > +++ b/hw/virtio/vhost-user.c
> > > @@ -1884,16 +1884,42 @@ static int
> > >  vhost_user_backend_handle_mem_read(struct vhost_dev *dev,
> > >                                     VhostUserMemRWMsg *mem_rw)
> > >  {
> > > -    /* TODO */
> > > -    return -EPERM;
> > > +    ram_addr_t offset;
> > > +    int fd;
> > > +    MemoryRegion *mr;
> > > +
> > > +    mr = vhost_user_get_mr_data(mem_rw->guest_address, &offset, &fd);
> > > +
> > > +    if (!mr) {
> > > +        error_report("Failed to get memory region with address %"
> > PRIx64,
> > > +                     mem_rw->guest_address);
> > > +        return -EFAULT;
> > > +    }
> > > +
> > > +    memcpy(mem_rw->data, memory_region_get_ram_ptr(mr) + offset,
> > mem_rw->size);
> >
> > Don't try to write this from scratch. Use address_space_read/write(). It
> > supports corner cases like crossing MemoryRegions.
> >
> 
> I am having issues getting the address space from the vhost_dev struct to
> feed
> address_spave_read/write() function with the first parameter. But I found
> mr->ops.
> Would something like this perhaps be enough?
> 
> ```
>     mr->ops->read_with_attrs(mr->opaque, mem_rw->guest_address,
>                              &mem_rw->data, mem_rw->size,
>                              MEMTXATTRS_UNSPECIFIED);
> ```

You can use dev->vdev->dma_as to get the AddressSpace for
address_space_read/write():

  struct vhost_dev {
      VirtIODevice *vdev;
  
  struct VirtIODevice
  {
      ...
      AddressSpace *dma_as;

> 
> 
> >
> > > +
> > > +    return 0;
> > >  }
> > >
> > >  static int
> > >  vhost_user_backend_handle_mem_write(struct vhost_dev *dev,
> > >                                     VhostUserMemRWMsg *mem_rw)
> > >  {
> > > -    /* TODO */
> > > -    return -EPERM;
> > > +    ram_addr_t offset;
> > > +    int fd;
> > > +    MemoryRegion *mr;
> > > +
> > > +    mr = vhost_user_get_mr_data(mem_rw->guest_address, &offset, &fd);
> > > +
> > > +    if (!mr) {
> > > +        error_report("Failed to get memory region with address %"
> > PRIx64,
> > > +                     mem_rw->guest_address);
> > > +        return -EFAULT;
> > > +    }
> > > +
> > > +    memcpy(memory_region_get_ram_ptr(mr) + offset, mem_rw->data,
> > mem_rw->size);
> > > +
> > > +    return 0;
> > >  }
> > >
> > >  static void close_backend_channel(struct vhost_user *u)
> > > --
> > > 2.45.2
> > >
> >

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]