RE: [RFC v4 12/21] vfio-user: region read/write

2022-01-26 Thread Thanos Makatos


> -Original Message-
> From: Qemu-devel  bounces+thanos.makatos=nutanix@nongnu.org> On Behalf Of John
> Johnson
> Sent: 12 January 2022 00:44
> To: qemu-devel@nongnu.org
> Subject: [RFC v4 12/21] vfio-user: region read/write
> 
> Add support for posted writes on remote devices
> 
> Signed-off-by: Elena Ufimtseva 
> Signed-off-by: John G Johnson 
> Signed-off-by: Jagannathan Raman 
> ---
>  hw/vfio/pci.h |   1 +
>  hw/vfio/user-protocol.h   |  12 +
>  hw/vfio/user.h|   1 +
>  include/hw/vfio/vfio-common.h |   7 +--
>  hw/vfio/common.c  |  10 +++-
>  hw/vfio/pci.c |   9 +++-
>  hw/vfio/user.c| 109
> ++
>  7 files changed, 143 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
> index ec9f345..643ff75 100644
> --- a/hw/vfio/pci.h
> +++ b/hw/vfio/pci.h
> @@ -194,6 +194,7 @@ struct VFIOUserPCIDevice {
>  VFIOPCIDevice device;
>  char *sock_name;
>  bool send_queued;   /* all sends are queued */
> +bool no_post;   /* all regions write are sync */
>  };
> 
>  /* Use uin32_t for vendor & device so PCI_ANY_ID expands and cannot match
> hw */
> diff --git a/hw/vfio/user-protocol.h b/hw/vfio/user-protocol.h
> index caa523a..b1ea55f 100644
> --- a/hw/vfio/user-protocol.h
> +++ b/hw/vfio/user-protocol.h
> @@ -120,4 +120,16 @@ typedef struct {
>  uint64_t offset;
>  } VFIOUserRegionInfo;
> 
> +/*
> + * VFIO_USER_REGION_READ
> + * VFIO_USER_REGION_WRITE
> + */
> +typedef struct {
> +VFIOUserHdr hdr;
> +uint64_t offset;
> +uint32_t region;
> +uint32_t count;
> +char data[];
> +} VFIOUserRegionRW;
> +
>  #endif /* VFIO_USER_PROTOCOL_H */
> diff --git a/hw/vfio/user.h b/hw/vfio/user.h
> index 19edd84..f2098f2 100644
> --- a/hw/vfio/user.h
> +++ b/hw/vfio/user.h
> @@ -75,6 +75,7 @@ typedef struct VFIOProxy {
>  /* VFIOProxy flags */
>  #define VFIO_PROXY_CLIENT0x1
>  #define VFIO_PROXY_FORCE_QUEUED  0x4
> +#define VFIO_PROXY_NO_POST   0x8
> 
>  VFIOProxy *vfio_user_connect_dev(SocketAddress *addr, Error **errp);
>  void vfio_user_disconnect(VFIOProxy *proxy);
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 2552557..4118b8a 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -57,6 +57,7 @@ typedef struct VFIORegion {
>  VFIOMmap *mmaps;
>  uint8_t nr; /* cache the region number for debug */
>  int fd; /* fd to mmap() region */
> +bool post_wr; /* writes can be posted */
>  } VFIORegion;
> 
>  typedef struct VFIOMigration {
> @@ -180,7 +181,7 @@ struct VFIODevIO {
>  int (*region_read)(VFIODevice *vdev, uint8_t nr, off_t off, uint32_t 
> size,
> void *data);
>  int (*region_write)(VFIODevice *vdev, uint8_t nr, off_t off, uint32_t 
> size,
> -void *data);
> +void *data, bool post);
>  };
> 
>  #define VDEV_GET_INFO(vdev, info) \
> @@ -193,8 +194,8 @@ struct VFIODevIO {
>  ((vdev)->io_ops->set_irqs((vdev), (irqs)))
>  #define VDEV_REGION_READ(vdev, nr, off, size, data) \
>  ((vdev)->io_ops->region_read((vdev), (nr), (off), (size), (data)))
> -#define VDEV_REGION_WRITE(vdev, nr, off, size, data) \
> -((vdev)->io_ops->region_write((vdev), (nr), (off), (size), (data)))
> +#define VDEV_REGION_WRITE(vdev, nr, off, size, data, post) \
> +((vdev)->io_ops->region_write((vdev), (nr), (off), (size), (data), 
> (post)))
> 
>  struct VFIOContIO {
>  int (*dma_map)(VFIOContainer *container,
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index a50bf4b..83cc5ec 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -213,6 +213,7 @@ void vfio_region_write(void *opaque, hwaddr addr,
>  uint32_t dword;
>  uint64_t qword;
>  } buf;
> +bool post = region->post_wr;
>  int ret;
> 
>  switch (size) {
> @@ -233,7 +234,11 @@ void vfio_region_write(void *opaque, hwaddr addr,
>  break;
>  }
> 
> -ret = VDEV_REGION_WRITE(vbasedev, region->nr, addr, size, );
> +/* read-after-write hazard if guest can directly access region */
> +if (region->nr_mmaps) {
> +post = false;
> +}
> +ret = VDEV_REGION_WRITE(vbasedev, region->nr, addr, size, , post);
>  if (ret != size) {
>  const char *err = ret < 0 ? strerror(-ret) : "short write";
> 
> @@ -1555,6 +1560,7 @@ int vfio_region_setup(Object *obj, VFIODevice
> *vbasedev, VFIORe

[RFC v4 12/21] vfio-user: region read/write

2022-01-11 Thread John Johnson
Add support for posted writes on remote devices

Signed-off-by: Elena Ufimtseva 
Signed-off-by: John G Johnson 
Signed-off-by: Jagannathan Raman 
---
 hw/vfio/pci.h |   1 +
 hw/vfio/user-protocol.h   |  12 +
 hw/vfio/user.h|   1 +
 include/hw/vfio/vfio-common.h |   7 +--
 hw/vfio/common.c  |  10 +++-
 hw/vfio/pci.c |   9 +++-
 hw/vfio/user.c| 109 ++
 7 files changed, 143 insertions(+), 6 deletions(-)

diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index ec9f345..643ff75 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -194,6 +194,7 @@ struct VFIOUserPCIDevice {
 VFIOPCIDevice device;
 char *sock_name;
 bool send_queued;   /* all sends are queued */
+bool no_post;   /* all regions write are sync */
 };
 
 /* Use uin32_t for vendor & device so PCI_ANY_ID expands and cannot match hw */
diff --git a/hw/vfio/user-protocol.h b/hw/vfio/user-protocol.h
index caa523a..b1ea55f 100644
--- a/hw/vfio/user-protocol.h
+++ b/hw/vfio/user-protocol.h
@@ -120,4 +120,16 @@ typedef struct {
 uint64_t offset;
 } VFIOUserRegionInfo;
 
+/*
+ * VFIO_USER_REGION_READ
+ * VFIO_USER_REGION_WRITE
+ */
+typedef struct {
+VFIOUserHdr hdr;
+uint64_t offset;
+uint32_t region;
+uint32_t count;
+char data[];
+} VFIOUserRegionRW;
+
 #endif /* VFIO_USER_PROTOCOL_H */
diff --git a/hw/vfio/user.h b/hw/vfio/user.h
index 19edd84..f2098f2 100644
--- a/hw/vfio/user.h
+++ b/hw/vfio/user.h
@@ -75,6 +75,7 @@ typedef struct VFIOProxy {
 /* VFIOProxy flags */
 #define VFIO_PROXY_CLIENT0x1
 #define VFIO_PROXY_FORCE_QUEUED  0x4
+#define VFIO_PROXY_NO_POST   0x8
 
 VFIOProxy *vfio_user_connect_dev(SocketAddress *addr, Error **errp);
 void vfio_user_disconnect(VFIOProxy *proxy);
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 2552557..4118b8a 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -57,6 +57,7 @@ typedef struct VFIORegion {
 VFIOMmap *mmaps;
 uint8_t nr; /* cache the region number for debug */
 int fd; /* fd to mmap() region */
+bool post_wr; /* writes can be posted */
 } VFIORegion;
 
 typedef struct VFIOMigration {
@@ -180,7 +181,7 @@ struct VFIODevIO {
 int (*region_read)(VFIODevice *vdev, uint8_t nr, off_t off, uint32_t size,
void *data);
 int (*region_write)(VFIODevice *vdev, uint8_t nr, off_t off, uint32_t size,
-void *data);
+void *data, bool post);
 };
 
 #define VDEV_GET_INFO(vdev, info) \
@@ -193,8 +194,8 @@ struct VFIODevIO {
 ((vdev)->io_ops->set_irqs((vdev), (irqs)))
 #define VDEV_REGION_READ(vdev, nr, off, size, data) \
 ((vdev)->io_ops->region_read((vdev), (nr), (off), (size), (data)))
-#define VDEV_REGION_WRITE(vdev, nr, off, size, data) \
-((vdev)->io_ops->region_write((vdev), (nr), (off), (size), (data)))
+#define VDEV_REGION_WRITE(vdev, nr, off, size, data, post) \
+((vdev)->io_ops->region_write((vdev), (nr), (off), (size), (data), (post)))
 
 struct VFIOContIO {
 int (*dma_map)(VFIOContainer *container,
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index a50bf4b..83cc5ec 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -213,6 +213,7 @@ void vfio_region_write(void *opaque, hwaddr addr,
 uint32_t dword;
 uint64_t qword;
 } buf;
+bool post = region->post_wr;
 int ret;
 
 switch (size) {
@@ -233,7 +234,11 @@ void vfio_region_write(void *opaque, hwaddr addr,
 break;
 }
 
-ret = VDEV_REGION_WRITE(vbasedev, region->nr, addr, size, );
+/* read-after-write hazard if guest can directly access region */
+if (region->nr_mmaps) {
+post = false;
+}
+ret = VDEV_REGION_WRITE(vbasedev, region->nr, addr, size, , post);
 if (ret != size) {
 const char *err = ret < 0 ? strerror(-ret) : "short write";
 
@@ -1555,6 +1560,7 @@ int vfio_region_setup(Object *obj, VFIODevice *vbasedev, 
VFIORegion *region,
 region->size = info->size;
 region->fd_offset = info->offset;
 region->nr = index;
+region->post_wr = false;
 if (vbasedev->regfds != NULL) {
 region->fd = vbasedev->regfds[index];
 } else {
@@ -2689,7 +2695,7 @@ static int vfio_io_region_read(VFIODevice *vbasedev, 
uint8_t index, off_t off,
 }
 
 static int vfio_io_region_write(VFIODevice *vbasedev, uint8_t index, off_t off,
-uint32_t size, void *data)
+uint32_t size, void *data, bool post)
 {
 struct vfio_region_info *info = vbasedev->regions[index];
 int ret;
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 6f85853..a4fd5e2 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -51,7 +51,7 @@
  (size), (data))
 #define VDEV_CONFIG_WRITE(vbasedev, off, size, data) \
 VDEV_REGION_WRITE((vbasedev), VFIO_PCI_CONFIG_REGION_INDEX, (off),