| Message ID | 20240628145710.1516121-6-aesteve@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vhost-user: Add SHMEM_MAP/UNMAP requests | expand |
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 >
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 > > >
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 > > > > >
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 > > > > > > > >
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)
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(-)