diff mbox series

[v8,10/17] vfio-user: run vfio-user context

Message ID 1f71b01f49b461a41130ac9d19355344c3752f7d.1650379269.git.jag.raman@oracle.com
State New
Headers show
Series vfio-user server in QEMU | expand

Commit Message

Jag Raman April 19, 2022, 8:44 p.m. UTC 
Setup a handler to run vfio-user context. The context is driven by
messages to the file descriptor associated with it - get the fd for
the context and hook up the handler with it

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>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 qapi/misc.json            | 23 ++++++++++
 hw/remote/vfio-user-obj.c | 95 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 117 insertions(+), 1 deletion(-)

Comments

Markus Armbruster April 21, 2022, 2:59 p.m. UTC | #1 
Jagannathan Raman <jag.raman@oracle.com> writes:

> Setup a handler to run vfio-user context. The context is driven by
> messages to the file descriptor associated with it - get the fd for
> the context and hook up the handler with it
>
> 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>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  qapi/misc.json            | 23 ++++++++++
>  hw/remote/vfio-user-obj.c | 95 ++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 117 insertions(+), 1 deletion(-)
>
> diff --git a/qapi/misc.json b/qapi/misc.json
> index b83cc39029..f3cc4a4854 100644
> --- a/qapi/misc.json
> +++ b/qapi/misc.json
> @@ -553,3 +553,26 @@
>  ##
>  { 'event': 'RTC_CHANGE',
>    'data': { 'offset': 'int', 'qom-path': 'str' } }
> +
> +##
> +# @VFU_CLIENT_HANGUP:
> +#
> +# Emitted when the client of a TYPE_VFIO_USER_SERVER closes the
> +# communication channel
> +#
> +# @id: ID of the TYPE_VFIO_USER_SERVER object
> +#
> +# @device: ID of attached PCI device

Is this the ID set with -device id=... and such?

> +#
> +# Since: 7.1
> +#
> +# Example:
> +#
> +# <- { "event": "VFU_CLIENT_HANGUP",
> +#      "data": { "id": "vfu1",
> +#                "device": "lsi1" },
> +#      "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
> +#
> +##
> +{ 'event': 'VFU_CLIENT_HANGUP',
> +  'data': { 'id': 'str', 'device': 'str' } }
Jag Raman April 21, 2022, 5:52 p.m. UTC | #2 
> On Apr 21, 2022, at 10:59 AM, Markus Armbruster <armbru@redhat.com> wrote:
> 
> Jagannathan Raman <jag.raman@oracle.com> writes:
> 
>> Setup a handler to run vfio-user context. The context is driven by
>> messages to the file descriptor associated with it - get the fd for
>> the context and hook up the handler with it
>> 
>> 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>
>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>> ---
>> qapi/misc.json            | 23 ++++++++++
>> hw/remote/vfio-user-obj.c | 95 ++++++++++++++++++++++++++++++++++++++-
>> 2 files changed, 117 insertions(+), 1 deletion(-)
>> 
>> diff --git a/qapi/misc.json b/qapi/misc.json
>> index b83cc39029..f3cc4a4854 100644
>> --- a/qapi/misc.json
>> +++ b/qapi/misc.json
>> @@ -553,3 +553,26 @@
>> ##
>> { 'event': 'RTC_CHANGE',
>>   'data': { 'offset': 'int', 'qom-path': 'str' } }
>> +
>> +##
>> +# @VFU_CLIENT_HANGUP:
>> +#
>> +# Emitted when the client of a TYPE_VFIO_USER_SERVER closes the
>> +# communication channel
>> +#
>> +# @id: ID of the TYPE_VFIO_USER_SERVER object
>> +#
>> +# @device: ID of attached PCI device
> 
> Is this the ID set with -device id=... and such?

Yes, that is correct. It’s the ID set with the “-device id=…” option/

Thank you!
--
Jag

> 
>> +#
>> +# Since: 7.1
>> +#
>> +# Example:
>> +#
>> +# <- { "event": "VFU_CLIENT_HANGUP",
>> +#      "data": { "id": "vfu1",
>> +#                "device": "lsi1" },
>> +#      "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
>> +#
>> +##
>> +{ 'event': 'VFU_CLIENT_HANGUP',
>> +  'data': { 'id': 'str', 'device': 'str' } }
>
Markus Armbruster April 22, 2022, 5:14 a.m. UTC | #3 
Jag Raman <jag.raman@oracle.com> writes:

>> On Apr 21, 2022, at 10:59 AM, Markus Armbruster <armbru@redhat.com> wrote:
>> 
>> Jagannathan Raman <jag.raman@oracle.com> writes:
>> 
>>> Setup a handler to run vfio-user context. The context is driven by
>>> messages to the file descriptor associated with it - get the fd for
>>> the context and hook up the handler with it
>>> 
>>> 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>
>>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>>> ---
>>> qapi/misc.json            | 23 ++++++++++
>>> hw/remote/vfio-user-obj.c | 95 ++++++++++++++++++++++++++++++++++++++-
>>> 2 files changed, 117 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/qapi/misc.json b/qapi/misc.json
>>> index b83cc39029..f3cc4a4854 100644
>>> --- a/qapi/misc.json
>>> +++ b/qapi/misc.json
>>> @@ -553,3 +553,26 @@
>>> ##
>>> { 'event': 'RTC_CHANGE',
>>>   'data': { 'offset': 'int', 'qom-path': 'str' } }
>>> +
>>> +##
>>> +# @VFU_CLIENT_HANGUP:
>>> +#
>>> +# Emitted when the client of a TYPE_VFIO_USER_SERVER closes the
>>> +# communication channel
>>> +#
>>> +# @id: ID of the TYPE_VFIO_USER_SERVER object
>>> +#
>>> +# @device: ID of attached PCI device
>> 
>> Is this the ID set with -device id=... and such?
>
> Yes, that is correct. It’s the ID set with the “-device id=…” option/

What happens when the device was added *without* id=...?  DeviceState
member @id is null then.

I figure we need to make @device optional here, present if the device
has an ID.  I recommend to also add a member @qom-path, like we did for
MEMORY_DEVICE_SIZE_CHANGE in commit d89dd28f0e2.
Jag Raman April 22, 2022, 2:18 p.m. UTC | #4 
> On Apr 22, 2022, at 1:14 AM, Markus Armbruster <armbru@redhat.com> wrote:
> 
> Jag Raman <jag.raman@oracle.com> writes:
> 
>>> On Apr 21, 2022, at 10:59 AM, Markus Armbruster <armbru@redhat.com> wrote:
>>> 
>>> Jagannathan Raman <jag.raman@oracle.com> writes:
>>> 
>>>> Setup a handler to run vfio-user context. The context is driven by
>>>> messages to the file descriptor associated with it - get the fd for
>>>> the context and hook up the handler with it
>>>> 
>>>> 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>
>>>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>>>> ---
>>>> qapi/misc.json            | 23 ++++++++++
>>>> hw/remote/vfio-user-obj.c | 95 ++++++++++++++++++++++++++++++++++++++-
>>>> 2 files changed, 117 insertions(+), 1 deletion(-)
>>>> 
>>>> diff --git a/qapi/misc.json b/qapi/misc.json
>>>> index b83cc39029..f3cc4a4854 100644
>>>> --- a/qapi/misc.json
>>>> +++ b/qapi/misc.json
>>>> @@ -553,3 +553,26 @@
>>>> ##
>>>> { 'event': 'RTC_CHANGE',
>>>>  'data': { 'offset': 'int', 'qom-path': 'str' } }
>>>> +
>>>> +##
>>>> +# @VFU_CLIENT_HANGUP:
>>>> +#
>>>> +# Emitted when the client of a TYPE_VFIO_USER_SERVER closes the
>>>> +# communication channel
>>>> +#
>>>> +# @id: ID of the TYPE_VFIO_USER_SERVER object
>>>> +#
>>>> +# @device: ID of attached PCI device
>>> 
>>> Is this the ID set with -device id=... and such?
>> 
>> Yes, that is correct. It’s the ID set with the “-device id=…” option/
> 
> What happens when the device was added *without* id=...?  DeviceState
> member @id is null then.

I’m sorry, the @id member in the event is the same as DeviceState->id, but
it’s not directly derived from it. It derived from  VfuObject->device, which is
a property of TYPE_VFU_OBJECT that the user sets.

This will not be NULL, as  TYPE_VFU_OBJECT will not initialize without it.

> 
> I figure we need to make @device optional here, present if the device
> has an ID.  I recommend to also add a member @qom-path, like we did for
> MEMORY_DEVICE_SIZE_CHANGE in commit d89dd28f0e2.

OK, will add it.

Thank you!
--
Jag

>
diff mbox series

Patch

diff --git a/qapi/misc.json b/qapi/misc.json
index b83cc39029..f3cc4a4854 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -553,3 +553,26 @@ 
 ##
 { 'event': 'RTC_CHANGE',
   'data': { 'offset': 'int', 'qom-path': 'str' } }
+
+##
+# @VFU_CLIENT_HANGUP:
+#
+# Emitted when the client of a TYPE_VFIO_USER_SERVER closes the
+# communication channel
+#
+# @id: ID of the TYPE_VFIO_USER_SERVER object
+#
+# @device: ID of attached PCI device
+#
+# Since: 7.1
+#
+# Example:
+#
+# <- { "event": "VFU_CLIENT_HANGUP",
+#      "data": { "id": "vfu1",
+#                "device": "lsi1" },
+#      "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
+#
+##
+{ 'event': 'VFU_CLIENT_HANGUP',
+  'data': { 'id': 'str', 'device': 'str' } }
diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c
index 15f6fe3a1a..06d99a8698 100644
--- a/hw/remote/vfio-user-obj.c
+++ b/hw/remote/vfio-user-obj.c
@@ -27,6 +27,9 @@ 
  *
  * device - id of a device on the server, a required option. PCI devices
  *          alone are supported presently.
+ *
+ * notes - x-vfio-user-server could block IO and monitor during the
+ *         initialization phase.
  */
 
 #include "qemu/osdep.h"
@@ -41,11 +44,14 @@ 
 #include "hw/remote/machine.h"
 #include "qapi/error.h"
 #include "qapi/qapi-visit-sockets.h"
+#include "qapi/qapi-events-misc.h"
 #include "qemu/notify.h"
+#include "qemu/thread.h"
 #include "sysemu/sysemu.h"
 #include "libvfio-user.h"
 #include "hw/qdev-core.h"
 #include "hw/pci/pci.h"
+#include "qemu/timer.h"
 
 #define TYPE_VFU_OBJECT "x-vfio-user-server"
 OBJECT_DECLARE_TYPE(VfuObject, VfuObjectClass, VFU_OBJECT)
@@ -87,6 +93,8 @@  struct VfuObject {
     PCIDevice *pci_dev;
 
     Error *unplug_blocker;
+
+    int vfu_poll_fd;
 };
 
 static void vfu_object_init_ctx(VfuObject *o, Error **errp);
@@ -165,6 +173,69 @@  static void vfu_object_set_device(Object *obj, const char *str, Error **errp)
     vfu_object_init_ctx(o, errp);
 }
 
+static void vfu_object_ctx_run(void *opaque)
+{
+    VfuObject *o = opaque;
+    const char *id = NULL;
+    int ret = -1;
+
+    while (ret != 0) {
+        ret = vfu_run_ctx(o->vfu_ctx);
+        if (ret < 0) {
+            if (errno == EINTR) {
+                continue;
+            } else if (errno == ENOTCONN) {
+                id = object_get_canonical_path_component(OBJECT(o));
+                qapi_event_send_vfu_client_hangup(id, o->device);
+                qemu_set_fd_handler(o->vfu_poll_fd, NULL, NULL, NULL);
+                o->vfu_poll_fd = -1;
+                object_unparent(OBJECT(o));
+                break;
+            } else {
+                VFU_OBJECT_ERROR(o, "vfu: Failed to run device %s - %s",
+                                 o->device, strerror(errno));
+                break;
+            }
+        }
+    }
+}
+
+static void vfu_object_attach_ctx(void *opaque)
+{
+    VfuObject *o = opaque;
+    GPollFD pfds[1];
+    int ret;
+
+    qemu_set_fd_handler(o->vfu_poll_fd, NULL, NULL, NULL);
+
+    pfds[0].fd = o->vfu_poll_fd;
+    pfds[0].events = G_IO_IN | G_IO_HUP | G_IO_ERR;
+
+retry_attach:
+    ret = vfu_attach_ctx(o->vfu_ctx);
+    if (ret < 0 && (errno == EAGAIN || errno == EWOULDBLOCK)) {
+        /**
+         * vfu_object_attach_ctx can block QEMU's main loop
+         * during attach - the monitor and other IO
+         * could be unresponsive during this time.
+         */
+        (void)qemu_poll_ns(pfds, 1, 500 * (int64_t)SCALE_MS);
+        goto retry_attach;
+    } else if (ret < 0) {
+        VFU_OBJECT_ERROR(o, "vfu: Failed to attach device %s to context - %s",
+                         o->device, strerror(errno));
+        return;
+    }
+
+    o->vfu_poll_fd = vfu_get_poll_fd(o->vfu_ctx);
+    if (o->vfu_poll_fd < 0) {
+        VFU_OBJECT_ERROR(o, "vfu: Failed to get poll fd %s", o->device);
+        return;
+    }
+
+    qemu_set_fd_handler(o->vfu_poll_fd, vfu_object_ctx_run, NULL, o);
+}
+
 /*
  * TYPE_VFU_OBJECT depends on the availability of the 'socket' and 'device'
  * properties. It also depends on devices instantiated in QEMU. These
@@ -203,7 +274,8 @@  static void vfu_object_init_ctx(VfuObject *o, Error **errp)
         return;
     }
 
-    o->vfu_ctx = vfu_create_ctx(VFU_TRANS_SOCK, o->socket->u.q_unix.path, 0,
+    o->vfu_ctx = vfu_create_ctx(VFU_TRANS_SOCK, o->socket->u.q_unix.path,
+                                LIBVFIO_USER_FLAG_ATTACH_NB,
                                 o, VFU_DEV_TYPE_PCI);
     if (o->vfu_ctx == NULL) {
         error_setg(errp, "vfu: Failed to create context - %s", strerror(errno));
@@ -242,6 +314,21 @@  static void vfu_object_init_ctx(VfuObject *o, Error **errp)
                TYPE_VFU_OBJECT, o->device);
     qdev_add_unplug_blocker(DEVICE(o->pci_dev), o->unplug_blocker);
 
+    ret = vfu_realize_ctx(o->vfu_ctx);
+    if (ret < 0) {
+        error_setg(errp, "vfu: Failed to realize device %s- %s",
+                   o->device, strerror(errno));
+        goto fail;
+    }
+
+    o->vfu_poll_fd = vfu_get_poll_fd(o->vfu_ctx);
+    if (o->vfu_poll_fd < 0) {
+        error_setg(errp, "vfu: Failed to get poll fd %s", o->device);
+        goto fail;
+    }
+
+    qemu_set_fd_handler(o->vfu_poll_fd, vfu_object_attach_ctx, NULL, o);
+
     return;
 
 fail:
@@ -276,6 +363,7 @@  static void vfu_object_init(Object *obj)
         qemu_add_machine_init_done_notifier(&o->machine_done);
     }
 
+    o->vfu_poll_fd = -1;
 }
 
 static void vfu_object_finalize(Object *obj)
@@ -289,6 +377,11 @@  static void vfu_object_finalize(Object *obj)
 
     o->socket = NULL;
 
+    if (o->vfu_poll_fd != -1) {
+        qemu_set_fd_handler(o->vfu_poll_fd, NULL, NULL, NULL);
+        o->vfu_poll_fd = -1;
+    }
+
     if (o->vfu_ctx) {
         vfu_destroy_ctx(o->vfu_ctx);
         o->vfu_ctx = NULL;