1. Patchew
  2. QEMU
  3. [RFC PATCH v2 0/5] vhost-user: Add SHMEM_MAP/UNMAP requests
  4. View patch

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

Albert Esteve posted 5 patches 2 months, 2 weeks ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>
There is a newer version of this series
[RFC PATCH v2 5/5] vhost_user: Implement mem_read/mem_write handlers
Posted by Albert Esteve 2 months, 2 weeks ago 
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);
+
+    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
Re: [RFC PATCH v2 5/5] vhost_user: Implement mem_read/mem_write handlers
Posted by Stefan Hajnoczi 2 months, 1 week ago 
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.

> +
> +    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
>
Re: [RFC PATCH v2 5/5] vhost_user: Implement mem_read/mem_write handlers
Posted by Albert Esteve 1 week, 5 days ago 
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);
```


>
> > +
> > +    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
> >
>
Re: [RFC PATCH v2 5/5] vhost_user: Implement mem_read/mem_write handlers
Posted by Stefan Hajnoczi 1 week, 4 days ago 
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
> > >
> >
Re: [RFC PATCH v2 5/5] vhost_user: Implement mem_read/mem_write handlers
Posted by Albert Esteve 6 days, 12 hours ago 
On Thu, Sep 5, 2024 at 9:18 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:

> 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():
>

Oof, I see, thanks!
I still struggle a bit with the structs relationships...


>
>   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
> > > >
> > >
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.