diff mbox series

[v1,10/24] vfio-user: get device info

Message ID d0fcc3415ab22bf66bbd86c1d69d4dade8c023bb.1667542066.git.john.g.johnson@oracle.com (mailing list archive)
State New, archived
Headers show
Series vfio-user client | expand

Commit Message

John Johnson Nov. 8, 2022, 11:13 p.m. UTC 
Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
Signed-off-by: John G Johnson <john.g.johnson@oracle.com>
Signed-off-by: Jagannathan Raman <jag.raman@oracle.com>
---
 hw/vfio/pci.c           | 15 ++++++++++++++
 hw/vfio/user-protocol.h | 13 ++++++++++++
 hw/vfio/user.c          | 55 +++++++++++++++++++++++++++++++++++++++++++++++++
 hw/vfio/user.h          |  2 ++
 4 files changed, 85 insertions(+)

Comments

John Levon Dec. 9, 2022, 3:57 p.m. UTC | #1 
On Tue, Nov 08, 2022 at 03:13:32PM -0800, John Johnson wrote:

> +/*
> + * VFIO_USER_DEVICE_GET_INFO
> + * imported from struct_device_info
> + */
> +typedef struct {
> +    VFIOUserHdr hdr;
> +    uint32_t argsz;
> +    uint32_t flags;
> +    uint32_t num_regions;
> +    uint32_t num_irqs;
> +    uint32_t cap_offset;
> +} VFIOUserDeviceInfo;

The server doesn't have or populate cap_offset - and this isn't in the protocol
either.

Looks good otherwise

regards
john
John Johnson Dec. 12, 2022, 8:28 p.m. UTC | #2 
> On Dec 9, 2022, at 7:57 AM, John Levon <levon@movementarian.org> wrote:
> 
> On Tue, Nov 08, 2022 at 03:13:32PM -0800, John Johnson wrote:
> 
>> +/*
>> + * VFIO_USER_DEVICE_GET_INFO
>> + * imported from struct_device_info
>> + */
>> +typedef struct {
>> +    VFIOUserHdr hdr;
>> +    uint32_t argsz;
>> +    uint32_t flags;
>> +    uint32_t num_regions;
>> +    uint32_t num_irqs;
>> +    uint32_t cap_offset;
>> +} VFIOUserDeviceInfo;
> 
> The server doesn't have or populate cap_offset - and this isn't in the protocol
> either.
> 

	I can just delete it from VFIOUserDeviceInfo

					JJ
Cédric Le Goater Dec. 13, 2022, 2:06 p.m. UTC | #3 
On 11/9/22 00:13, John Johnson wrote:
> Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
> Signed-off-by: John G Johnson <john.g.johnson@oracle.com>
> Signed-off-by: Jagannathan Raman <jag.raman@oracle.com>
> ---
>   hw/vfio/pci.c           | 15 ++++++++++++++
>   hw/vfio/user-protocol.h | 13 ++++++++++++
>   hw/vfio/user.c          | 55 +++++++++++++++++++++++++++++++++++++++++++++++++
>   hw/vfio/user.h          |  2 ++
>   4 files changed, 85 insertions(+)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index b2534b3..2e0e41d 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -3465,6 +3465,8 @@ static void vfio_user_pci_realize(PCIDevice *pdev, Error **errp)
>       VFIODevice *vbasedev = &vdev->vbasedev;
>       SocketAddress addr;
>       VFIOProxy *proxy;
> +    struct vfio_device_info info;
> +    int ret;
>       Error *err = NULL;
>   
>       /*
> @@ -3503,6 +3505,19 @@ static void vfio_user_pci_realize(PCIDevice *pdev, Error **errp)
>       vbasedev->ops = &vfio_user_pci_ops;
>       vbasedev->type = VFIO_DEVICE_TYPE_PCI;
>       vbasedev->dev = DEVICE(vdev);
> +    vbasedev->io_ops = &vfio_dev_io_sock;
> +
> +    ret = VDEV_GET_INFO(vbasedev, &info);

We are dealing with a "vfio-user-pci" device since this is being
called from the realize handler.

Is it really useful to go through the ops abstraction layer here ?
Can't we simply call vfio_user_get_info() ?  and propagate the
underlying Error while we're at it.
  
> +    if (ret) {
> +        error_setg_errno(errp, -ret, "get info failure");
> +        goto error;
> +    }
> +
> +    vfio_populate_device(vdev, &err);
> +    if (err) {
> +        error_propagate(errp, err);
> +        goto error;
> +    }
>   
>       return;
>   
> diff --git a/hw/vfio/user-protocol.h b/hw/vfio/user-protocol.h
> index 5de5b20..43912a5 100644
> --- a/hw/vfio/user-protocol.h
> +++ b/hw/vfio/user-protocol.h
> @@ -113,4 +113,17 @@ typedef struct {
>    */
>   #define VFIO_USER_DEF_MAX_BITMAP (256 * 1024 * 1024)
>   
> +/*
> + * VFIO_USER_DEVICE_GET_INFO
> + * imported from struct_device_info

from struct vfio_device_info

Thanks,

C.

> + */
> +typedef struct {
> +    VFIOUserHdr hdr;
> +    uint32_t argsz;
> +    uint32_t flags;
> +    uint32_t num_regions;
> +    uint32_t num_irqs;
> +    uint32_t cap_offset;
> +} VFIOUserDeviceInfo;
> +
>   #endif /* VFIO_USER_PROTOCOL_H */
> diff --git a/hw/vfio/user.c b/hw/vfio/user.c
> index 31bcc93..7873534 100644
> --- a/hw/vfio/user.c
> +++ b/hw/vfio/user.c
> @@ -31,6 +31,14 @@
>   #include "qapi/qmp/qbool.h"
>   #include "user.h"
>   
> +
> +/*
> + * These are to defend against a malign server trying
> + * to force us to run out of memory.
> + */
> +#define VFIO_USER_MAX_REGIONS   100
> +#define VFIO_USER_MAX_IRQS      50
> +
>   static int wait_time = 5000;   /* wait up to 5 sec for busy servers */
>   static IOThread *vfio_user_iothread;
>   
> @@ -1075,3 +1083,50 @@ int vfio_user_validate_version(VFIOProxy *proxy, Error **errp)
>   
>       return 0;
>   }
> +
> +static int vfio_user_get_info(VFIOProxy *proxy, struct vfio_device_info *info)
> +{
> +    VFIOUserDeviceInfo msg;
> +
> +    memset(&msg, 0, sizeof(msg));
> +    vfio_user_request_msg(&msg.hdr, VFIO_USER_DEVICE_GET_INFO, sizeof(msg), 0);
> +    msg.argsz = sizeof(struct vfio_device_info);
> +
> +    vfio_user_send_wait(proxy, &msg.hdr, NULL, 0, false);
> +    if (msg.hdr.flags & VFIO_USER_ERROR) {
> +        return -msg.hdr.error_reply;
> +    }
> +
> +    memcpy(info, &msg.argsz, sizeof(*info));
> +    return 0;
> +}
> +
> +
> +/*
> + * Socket-based io_ops
> + */
> +
> +static int vfio_user_io_get_info(VFIODevice *vbasedev,
> +                                 struct vfio_device_info *info)
> +{
> +    int ret;
> +
> +    ret = vfio_user_get_info(vbasedev->proxy, info);
> +    if (ret) {
> +        return ret;
> +    }
> +
> +    /* defend against a malicious server */
> +    if (info->num_regions > VFIO_USER_MAX_REGIONS ||
> +        info->num_irqs > VFIO_USER_MAX_IRQS) {
> +        error_printf("vfio_user_get_info: invalid reply\n");
> +        return -EINVAL;
> +    }
> +
> +    return 0;
> +}
> +
> +VFIODevIO vfio_dev_io_sock = {
> +    .get_info = vfio_user_io_get_info,
> +};
> +
> diff --git a/hw/vfio/user.h b/hw/vfio/user.h
> index 8ce3cd9..2547cf6 100644
> --- a/hw/vfio/user.h
> +++ b/hw/vfio/user.h
> @@ -92,4 +92,6 @@ void vfio_user_set_handler(VFIODevice *vbasedev,
>                              void *reqarg);
>   int vfio_user_validate_version(VFIOProxy *proxy, Error **errp);
>   
> +extern VFIODevIO vfio_dev_io_sock;
> +
>   #endif /* VFIO_USER_H */
diff mbox series

Patch

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index b2534b3..2e0e41d 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3465,6 +3465,8 @@  static void vfio_user_pci_realize(PCIDevice *pdev, Error **errp)
     VFIODevice *vbasedev = &vdev->vbasedev;
     SocketAddress addr;
     VFIOProxy *proxy;
+    struct vfio_device_info info;
+    int ret;
     Error *err = NULL;
 
     /*
@@ -3503,6 +3505,19 @@  static void vfio_user_pci_realize(PCIDevice *pdev, Error **errp)
     vbasedev->ops = &vfio_user_pci_ops;
     vbasedev->type = VFIO_DEVICE_TYPE_PCI;
     vbasedev->dev = DEVICE(vdev);
+    vbasedev->io_ops = &vfio_dev_io_sock;
+
+    ret = VDEV_GET_INFO(vbasedev, &info);
+    if (ret) {
+        error_setg_errno(errp, -ret, "get info failure");
+        goto error;
+    }
+
+    vfio_populate_device(vdev, &err);
+    if (err) {
+        error_propagate(errp, err);
+        goto error;
+    }
 
     return;
 
diff --git a/hw/vfio/user-protocol.h b/hw/vfio/user-protocol.h
index 5de5b20..43912a5 100644
--- a/hw/vfio/user-protocol.h
+++ b/hw/vfio/user-protocol.h
@@ -113,4 +113,17 @@  typedef struct {
  */
 #define VFIO_USER_DEF_MAX_BITMAP (256 * 1024 * 1024)
 
+/*
+ * VFIO_USER_DEVICE_GET_INFO
+ * imported from struct_device_info
+ */
+typedef struct {
+    VFIOUserHdr hdr;
+    uint32_t argsz;
+    uint32_t flags;
+    uint32_t num_regions;
+    uint32_t num_irqs;
+    uint32_t cap_offset;
+} VFIOUserDeviceInfo;
+
 #endif /* VFIO_USER_PROTOCOL_H */
diff --git a/hw/vfio/user.c b/hw/vfio/user.c
index 31bcc93..7873534 100644
--- a/hw/vfio/user.c
+++ b/hw/vfio/user.c
@@ -31,6 +31,14 @@ 
 #include "qapi/qmp/qbool.h"
 #include "user.h"
 
+
+/*
+ * These are to defend against a malign server trying
+ * to force us to run out of memory.
+ */
+#define VFIO_USER_MAX_REGIONS   100
+#define VFIO_USER_MAX_IRQS      50
+
 static int wait_time = 5000;   /* wait up to 5 sec for busy servers */
 static IOThread *vfio_user_iothread;
 
@@ -1075,3 +1083,50 @@  int vfio_user_validate_version(VFIOProxy *proxy, Error **errp)
 
     return 0;
 }
+
+static int vfio_user_get_info(VFIOProxy *proxy, struct vfio_device_info *info)
+{
+    VFIOUserDeviceInfo msg;
+
+    memset(&msg, 0, sizeof(msg));
+    vfio_user_request_msg(&msg.hdr, VFIO_USER_DEVICE_GET_INFO, sizeof(msg), 0);
+    msg.argsz = sizeof(struct vfio_device_info);
+
+    vfio_user_send_wait(proxy, &msg.hdr, NULL, 0, false);
+    if (msg.hdr.flags & VFIO_USER_ERROR) {
+        return -msg.hdr.error_reply;
+    }
+
+    memcpy(info, &msg.argsz, sizeof(*info));
+    return 0;
+}
+
+
+/*
+ * Socket-based io_ops
+ */
+
+static int vfio_user_io_get_info(VFIODevice *vbasedev,
+                                 struct vfio_device_info *info)
+{
+    int ret;
+
+    ret = vfio_user_get_info(vbasedev->proxy, info);
+    if (ret) {
+        return ret;
+    }
+
+    /* defend against a malicious server */
+    if (info->num_regions > VFIO_USER_MAX_REGIONS ||
+        info->num_irqs > VFIO_USER_MAX_IRQS) {
+        error_printf("vfio_user_get_info: invalid reply\n");
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
+VFIODevIO vfio_dev_io_sock = {
+    .get_info = vfio_user_io_get_info,
+};
+
diff --git a/hw/vfio/user.h b/hw/vfio/user.h
index 8ce3cd9..2547cf6 100644
--- a/hw/vfio/user.h
+++ b/hw/vfio/user.h
@@ -92,4 +92,6 @@  void vfio_user_set_handler(VFIODevice *vbasedev,
                            void *reqarg);
 int vfio_user_validate_version(VFIOProxy *proxy, Error **errp);
 
+extern VFIODevIO vfio_dev_io_sock;
+
 #endif /* VFIO_USER_H */