| Message ID | 1f71b01f49b461a41130ac9d19355344c3752f7d.1650379269.git.jag.raman@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vfio-user server in QEMU | expand |
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' } }
> 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' } } >
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.
> 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 --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;