Re: [PATCH v4 08/14] vfio-user: handle PCI config space accesses

[Stefan Hajnoczi]

On Thu, Dec 16, 2021 at 11:47:40AM +, John Levon wrote:
> On Thu, Dec 16, 2021 at 11:30:20AM +, Stefan Hajnoczi wrote:
> 
> > > +ret = vfu_setup_region(o->vfu_ctx, VFU_PCI_DEV_CFG_REGION_IDX,
> > > +   pci_config_size(o->pci_dev), 
> > > _object_cfg_access,
> > > +   VFU_REGION_FLAG_RW | 
> > > VFU_REGION_FLAG_ALWAYS_CB,
> > > +   NULL, 0, -1, 0);
> > 
> > Thanos or John: QEMU's pci_host_config_read/write_common() works with
> > host-endian values. I don't know which endianness libvfio-user's region
> > callbacks expect. Does this patch look endian-safe to you (e.g. will it
> > work on a big-endian host)?
> 
> https://github.com/nutanix/libvfio-user/issues/615

Thanks!

Stefan


signature.asc
Description: PGP signature

Re: [PATCH v4 08/14] vfio-user: handle PCI config space accesses

[John Levon]

On Thu, Dec 16, 2021 at 11:30:20AM +, Stefan Hajnoczi wrote:

> > +ret = vfu_setup_region(o->vfu_ctx, VFU_PCI_DEV_CFG_REGION_IDX,
> > +   pci_config_size(o->pci_dev), 
> > _object_cfg_access,
> > +   VFU_REGION_FLAG_RW | VFU_REGION_FLAG_ALWAYS_CB,
> > +   NULL, 0, -1, 0);
> 
> Thanos or John: QEMU's pci_host_config_read/write_common() works with
> host-endian values. I don't know which endianness libvfio-user's region
> callbacks expect. Does this patch look endian-safe to you (e.g. will it
> work on a big-endian host)?

https://github.com/nutanix/libvfio-user/issues/615

regards
john

Re: [PATCH v4 08/14] vfio-user: handle PCI config space accesses

[Stefan Hajnoczi]

On Wed, Dec 15, 2021 at 10:35:32AM -0500, Jagannathan Raman wrote:
> Define and register handlers for PCI config space accesses
> 
> Signed-off-by: Elena Ufimtseva 
> Signed-off-by: John G Johnson 
> Signed-off-by: Jagannathan Raman 
> ---
>  hw/remote/vfio-user-obj.c | 45 +++
>  hw/remote/trace-events|  2 ++
>  2 files changed, 47 insertions(+)
> 
> diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c
> index a01a0ad185..c6d0c675b7 100644
> --- a/hw/remote/vfio-user-obj.c
> +++ b/hw/remote/vfio-user-obj.c
> @@ -43,6 +43,7 @@
>  #include "qapi/qapi-visit-sockets.h"
>  #include "qemu/notify.h"
>  #include "qemu/thread.h"
> +#include "qemu/main-loop.h"
>  #include "sysemu/sysemu.h"
>  #include "libvfio-user.h"
>  #include "hw/qdev-core.h"
> @@ -174,6 +175,39 @@ retry_attach:
>  qemu_set_fd_handler(o->vfu_poll_fd, vfu_object_ctx_run, NULL, o);
>  }
>  
> +static ssize_t vfu_object_cfg_access(vfu_ctx_t *vfu_ctx, char * const buf,
> + size_t count, loff_t offset,
> + const bool is_write)
> +{
> +VfuObject *o = vfu_get_private(vfu_ctx);
> +uint32_t pci_access_width = sizeof(uint32_t);
> +size_t bytes = count;
> +uint32_t val = 0;
> +char *ptr = buf;
> +int len;
> +
> +while (bytes > 0) {
> +len = (bytes > pci_access_width) ? pci_access_width : bytes;
> +if (is_write) {
> +memcpy(, ptr, len);
> +pci_host_config_write_common(o->pci_dev, offset,
> + pci_config_size(o->pci_dev),
> + val, len);
> +trace_vfu_cfg_write(offset, val);
> +} else {
> +val = pci_host_config_read_common(o->pci_dev, offset,
> +  pci_config_size(o->pci_dev), 
> len);
> +memcpy(ptr, , len);
> +trace_vfu_cfg_read(offset, val);
> +}
> +offset += len;
> +ptr += len;
> +bytes -= len;
> +}
> +
> +return count;
> +}
> +
>  /*
>   * TYPE_VFU_OBJECT depends on the availability of the 'socket' and 'device'
>   * properties. It also depends on devices instantiated in QEMU. These
> @@ -244,6 +278,17 @@ static void vfu_object_init_ctx(VfuObject *o, Error 
> **errp)
>  goto fail;
>  }
>  
> +ret = vfu_setup_region(o->vfu_ctx, VFU_PCI_DEV_CFG_REGION_IDX,
> +   pci_config_size(o->pci_dev), 
> _object_cfg_access,
> +   VFU_REGION_FLAG_RW | VFU_REGION_FLAG_ALWAYS_CB,
> +   NULL, 0, -1, 0);

Thanos or John: QEMU's pci_host_config_read/write_common() works with
host-endian values. I don't know which endianness libvfio-user's region
callbacks expect. Does this patch look endian-safe to you (e.g. will it
work on a big-endian host)?

Thanks,
Stefan


signature.asc
Description: PGP signature

[PATCH v4 08/14] vfio-user: handle PCI config space accesses

[Jagannathan Raman]

Define and register handlers for PCI config space accesses

Signed-off-by: Elena Ufimtseva 
Signed-off-by: John G Johnson 
Signed-off-by: Jagannathan Raman 
---
 hw/remote/vfio-user-obj.c | 45 +++
 hw/remote/trace-events|  2 ++
 2 files changed, 47 insertions(+)

diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c
index a01a0ad185..c6d0c675b7 100644
--- a/hw/remote/vfio-user-obj.c
+++ b/hw/remote/vfio-user-obj.c
@@ -43,6 +43,7 @@
 #include "qapi/qapi-visit-sockets.h"
 #include "qemu/notify.h"
 #include "qemu/thread.h"
+#include "qemu/main-loop.h"
 #include "sysemu/sysemu.h"
 #include "libvfio-user.h"
 #include "hw/qdev-core.h"
@@ -174,6 +175,39 @@ retry_attach:
 qemu_set_fd_handler(o->vfu_poll_fd, vfu_object_ctx_run, NULL, o);
 }
 
+static ssize_t vfu_object_cfg_access(vfu_ctx_t *vfu_ctx, char * const buf,
+ size_t count, loff_t offset,
+ const bool is_write)
+{
+VfuObject *o = vfu_get_private(vfu_ctx);
+uint32_t pci_access_width = sizeof(uint32_t);
+size_t bytes = count;
+uint32_t val = 0;
+char *ptr = buf;
+int len;
+
+while (bytes > 0) {
+len = (bytes > pci_access_width) ? pci_access_width : bytes;
+if (is_write) {
+memcpy(, ptr, len);
+pci_host_config_write_common(o->pci_dev, offset,
+ pci_config_size(o->pci_dev),
+ val, len);
+trace_vfu_cfg_write(offset, val);
+} else {
+val = pci_host_config_read_common(o->pci_dev, offset,
+  pci_config_size(o->pci_dev), 
len);
+memcpy(ptr, , len);
+trace_vfu_cfg_read(offset, val);
+}
+offset += len;
+ptr += len;
+bytes -= len;
+}
+
+return count;
+}
+
 /*
  * TYPE_VFU_OBJECT depends on the availability of the 'socket' and 'device'
  * properties. It also depends on devices instantiated in QEMU. These
@@ -244,6 +278,17 @@ static void vfu_object_init_ctx(VfuObject *o, Error **errp)
 goto fail;
 }
 
+ret = vfu_setup_region(o->vfu_ctx, VFU_PCI_DEV_CFG_REGION_IDX,
+   pci_config_size(o->pci_dev), _object_cfg_access,
+   VFU_REGION_FLAG_RW | VFU_REGION_FLAG_ALWAYS_CB,
+   NULL, 0, -1, 0);
+if (ret < 0) {
+error_setg(errp,
+   "vfu: Failed to setup config space handlers for %s- %s",
+   o->device, strerror(errno));
+goto fail;
+}
+
 ret = vfu_realize_ctx(o->vfu_ctx);
 if (ret < 0) {
 error_setg(errp, "vfu: Failed to realize device %s- %s",
diff --git a/hw/remote/trace-events b/hw/remote/trace-events
index 7da12f0d96..2ef7884346 100644
--- a/hw/remote/trace-events
+++ b/hw/remote/trace-events
@@ -5,3 +5,5 @@ mpqemu_recv_io_error(int cmd, int size, int nfds) "failed to 
receive %d size %d,
 
 # vfio-user-obj.c
 vfu_prop(const char *prop, const char *val) "vfu: setting %s as %s"
+vfu_cfg_read(uint32_t offset, uint32_t val) "vfu: cfg: 0x%u -> 0x%x"
+vfu_cfg_write(uint32_t offset, uint32_t val) "vfu: cfg: 0x%u <- 0x%x"
-- 
2.20.1