Emit VFIO migration QAPI event when a VFIO device changes its migration
state. This can be used by management applications to get updates on the
current state of the VFIO device for their own purposes.
A new per VFIO device capability, "migration-events", is added so events
can be enabled only for the required devices. It is disabled by default.
Signed-off-by: Avihai Horon <avihaih@nvidia.com>
---
include/hw/vfio/vfio-common.h | 1 +
hw/vfio/migration.c | 56 +++++++++++++++++++++++++++++++++--
hw/vfio/pci.c | 2 ++
3 files changed, 56 insertions(+), 3 deletions(-)
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index b9da6c08ef..3ec5f2425e 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -115,6 +115,7 @@ typedef struct VFIODevice {
bool no_mmap;
bool ram_block_discard_allowed;
OnOffAuto enable_migration;
+ bool migration_events;
VFIODeviceOps *ops;
unsigned int num_irqs;
unsigned int num_regions;
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 06ae40969b..5a359c4c78 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -24,6 +24,7 @@
#include "migration/register.h"
#include "migration/blocker.h"
#include "qapi/error.h"
+#include "qapi/qapi-events-vfio.h"
#include "exec/ramlist.h"
#include "exec/ram_addr.h"
#include "pci.h"
@@ -80,6 +81,55 @@ static const char *mig_state_to_str(enum vfio_device_mig_state state)
}
}
+static VfioMigrationState
+mig_state_to_qapi_state(enum vfio_device_mig_state state)
+{
+ switch (state) {
+ case VFIO_DEVICE_STATE_STOP:
+ return QAPI_VFIO_MIGRATION_STATE_STOP;
+ case VFIO_DEVICE_STATE_RUNNING:
+ return QAPI_VFIO_MIGRATION_STATE_RUNNING;
+ case VFIO_DEVICE_STATE_STOP_COPY:
+ return QAPI_VFIO_MIGRATION_STATE_STOP_COPY;
+ case VFIO_DEVICE_STATE_RESUMING:
+ return QAPI_VFIO_MIGRATION_STATE_RESUMING;
+ case VFIO_DEVICE_STATE_RUNNING_P2P:
+ return QAPI_VFIO_MIGRATION_STATE_RUNNING_P2P;
+ case VFIO_DEVICE_STATE_PRE_COPY:
+ return QAPI_VFIO_MIGRATION_STATE_PRE_COPY;
+ case VFIO_DEVICE_STATE_PRE_COPY_P2P:
+ return QAPI_VFIO_MIGRATION_STATE_PRE_COPY_P2P;
+ default:
+ g_assert_not_reached();
+ }
+}
+
+static void vfio_migration_send_event(VFIODevice *vbasedev)
+{
+ VFIOMigration *migration = vbasedev->migration;
+ DeviceState *dev = vbasedev->dev;
+ g_autofree char *qom_path = NULL;
+ Object *obj;
+
+ if (!vbasedev->migration_events) {
+ return;
+ }
+
+ obj = vbasedev->ops->vfio_get_object(vbasedev);
+ qom_path = object_get_canonical_path(obj);
+
+ qapi_event_send_vfio_migration(
+ dev->id, qom_path, mig_state_to_qapi_state(migration->device_state));
+}
+
+static void set_state(VFIODevice *vbasedev, enum vfio_device_mig_state state)
+{
+ VFIOMigration *migration = vbasedev->migration;
+
+ migration->device_state = state;
+ vfio_migration_send_event(vbasedev);
+}
+
static int vfio_migration_set_state(VFIODevice *vbasedev,
enum vfio_device_mig_state new_state,
enum vfio_device_mig_state recover_state)
@@ -125,12 +175,12 @@ static int vfio_migration_set_state(VFIODevice *vbasedev,
goto reset_device;
}
- migration->device_state = recover_state;
+ set_state(vbasedev, recover_state);
return ret;
}
- migration->device_state = new_state;
+ set_state(vbasedev, new_state);
if (mig_state->data_fd != -1) {
if (migration->data_fd != -1) {
/*
@@ -156,7 +206,7 @@ reset_device:
strerror(errno));
}
- migration->device_state = VFIO_DEVICE_STATE_RUNNING;
+ set_state(vbasedev, VFIO_DEVICE_STATE_RUNNING);
return ret;
}
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 64780d1b79..8840602c50 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3362,6 +3362,8 @@ static Property vfio_pci_dev_properties[] = {
VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT, false),
DEFINE_PROP_ON_OFF_AUTO("enable-migration", VFIOPCIDevice,
vbasedev.enable_migration, ON_OFF_AUTO_AUTO),
+ DEFINE_PROP_BOOL("migration-events", VFIOPCIDevice,
+ vbasedev.migration_events, false),
DEFINE_PROP_BOOL("x-no-mmap", VFIOPCIDevice, vbasedev.no_mmap, false),
DEFINE_PROP_BOOL("x-balloon-allowed", VFIOPCIDevice,
vbasedev.ram_block_discard_allowed, false),
--
2.26.3On 5/9/24 11:09, Avihai Horon wrote:
> Emit VFIO migration QAPI event when a VFIO device changes its migration
> state. This can be used by management applications to get updates on the
> current state of the VFIO device for their own purposes.
>
> A new per VFIO device capability, "migration-events", is added so events
> can be enabled only for the required devices. It is disabled by default.
>
> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
> ---
> include/hw/vfio/vfio-common.h | 1 +
> hw/vfio/migration.c | 56 +++++++++++++++++++++++++++++++++--
> hw/vfio/pci.c | 2 ++
> 3 files changed, 56 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index b9da6c08ef..3ec5f2425e 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -115,6 +115,7 @@ typedef struct VFIODevice {
> bool no_mmap;
> bool ram_block_discard_allowed;
> OnOffAuto enable_migration;
> + bool migration_events;
> VFIODeviceOps *ops;
> unsigned int num_irqs;
> unsigned int num_regions;
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> index 06ae40969b..5a359c4c78 100644
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
> @@ -24,6 +24,7 @@
> #include "migration/register.h"
> #include "migration/blocker.h"
> #include "qapi/error.h"
> +#include "qapi/qapi-events-vfio.h"
> #include "exec/ramlist.h"
> #include "exec/ram_addr.h"
> #include "pci.h"
> @@ -80,6 +81,55 @@ static const char *mig_state_to_str(enum vfio_device_mig_state state)
> }
> }
>
> +static VfioMigrationState
> +mig_state_to_qapi_state(enum vfio_device_mig_state state)
> +{
> + switch (state) {
> + case VFIO_DEVICE_STATE_STOP:
> + return QAPI_VFIO_MIGRATION_STATE_STOP;
> + case VFIO_DEVICE_STATE_RUNNING:
> + return QAPI_VFIO_MIGRATION_STATE_RUNNING;
> + case VFIO_DEVICE_STATE_STOP_COPY:
> + return QAPI_VFIO_MIGRATION_STATE_STOP_COPY;
> + case VFIO_DEVICE_STATE_RESUMING:
> + return QAPI_VFIO_MIGRATION_STATE_RESUMING;
> + case VFIO_DEVICE_STATE_RUNNING_P2P:
> + return QAPI_VFIO_MIGRATION_STATE_RUNNING_P2P;
> + case VFIO_DEVICE_STATE_PRE_COPY:
> + return QAPI_VFIO_MIGRATION_STATE_PRE_COPY;
> + case VFIO_DEVICE_STATE_PRE_COPY_P2P:
> + return QAPI_VFIO_MIGRATION_STATE_PRE_COPY_P2P;
> + default:
> + g_assert_not_reached();
> + }
> +}
> +
> +static void vfio_migration_send_event(VFIODevice *vbasedev)
> +{
> + VFIOMigration *migration = vbasedev->migration;
> + DeviceState *dev = vbasedev->dev;
> + g_autofree char *qom_path = NULL;
> + Object *obj;
> +
> + if (!vbasedev->migration_events) {
> + return;
> + }
I would add an assert on vbasedev->ops->vfio_get_object
> + obj = vbasedev->ops->vfio_get_object(vbasedev);
and another assert on obj.
> + qom_path = object_get_canonical_path(obj);
> +
> + qapi_event_send_vfio_migration(
> + dev->id, qom_path, mig_state_to_qapi_state(migration->device_state));
> +}
> +
> +static void set_state(VFIODevice *vbasedev, enum vfio_device_mig_state state)
to avoid the conflict with vfio_migration_set_state(), let's call it :
vfio_migration_set_device_state() ? We want a 'vfio_migration_' prefix.
Thanks,
C.
> +{
> + VFIOMigration *migration = vbasedev->migration;
> +
> + migration->device_state = state;
> + vfio_migration_send_event(vbasedev);
> +}
> +
> static int vfio_migration_set_state(VFIODevice *vbasedev,
> enum vfio_device_mig_state new_state,
> enum vfio_device_mig_state recover_state)
> @@ -125,12 +175,12 @@ static int vfio_migration_set_state(VFIODevice *vbasedev,
> goto reset_device;
> }
>
> - migration->device_state = recover_state;
> + set_state(vbasedev, recover_state);
>
> return ret;
> }
>
> - migration->device_state = new_state;
> + set_state(vbasedev, new_state);
> if (mig_state->data_fd != -1) {
> if (migration->data_fd != -1) {
> /*
> @@ -156,7 +206,7 @@ reset_device:
> strerror(errno));
> }
>
> - migration->device_state = VFIO_DEVICE_STATE_RUNNING;
> + set_state(vbasedev, VFIO_DEVICE_STATE_RUNNING);
>
> return ret;
> }
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 64780d1b79..8840602c50 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -3362,6 +3362,8 @@ static Property vfio_pci_dev_properties[] = {
> VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT, false),
> DEFINE_PROP_ON_OFF_AUTO("enable-migration", VFIOPCIDevice,
> vbasedev.enable_migration, ON_OFF_AUTO_AUTO),
> + DEFINE_PROP_BOOL("migration-events", VFIOPCIDevice,
> + vbasedev.migration_events, false),
> DEFINE_PROP_BOOL("x-no-mmap", VFIOPCIDevice, vbasedev.no_mmap, false),
> DEFINE_PROP_BOOL("x-balloon-allowed", VFIOPCIDevice,
> vbasedev.ram_block_discard_allowed, false),
On 13/05/2024 17:01, Cédric Le Goater wrote:
> External email: Use caution opening links or attachments
>
>
> On 5/9/24 11:09, Avihai Horon wrote:
>> Emit VFIO migration QAPI event when a VFIO device changes its migration
>> state. This can be used by management applications to get updates on the
>> current state of the VFIO device for their own purposes.
>>
>> A new per VFIO device capability, "migration-events", is added so events
>> can be enabled only for the required devices. It is disabled by default.
>>
>> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
>> ---
>> include/hw/vfio/vfio-common.h | 1 +
>> hw/vfio/migration.c | 56 +++++++++++++++++++++++++++++++++--
>> hw/vfio/pci.c | 2 ++
>> 3 files changed, 56 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/hw/vfio/vfio-common.h
>> b/include/hw/vfio/vfio-common.h
>> index b9da6c08ef..3ec5f2425e 100644
>> --- a/include/hw/vfio/vfio-common.h
>> +++ b/include/hw/vfio/vfio-common.h
>> @@ -115,6 +115,7 @@ typedef struct VFIODevice {
>> bool no_mmap;
>> bool ram_block_discard_allowed;
>> OnOffAuto enable_migration;
>> + bool migration_events;
>> VFIODeviceOps *ops;
>> unsigned int num_irqs;
>> unsigned int num_regions;
>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>> index 06ae40969b..5a359c4c78 100644
>> --- a/hw/vfio/migration.c
>> +++ b/hw/vfio/migration.c
>> @@ -24,6 +24,7 @@
>> #include "migration/register.h"
>> #include "migration/blocker.h"
>> #include "qapi/error.h"
>> +#include "qapi/qapi-events-vfio.h"
>> #include "exec/ramlist.h"
>> #include "exec/ram_addr.h"
>> #include "pci.h"
>> @@ -80,6 +81,55 @@ static const char *mig_state_to_str(enum
>> vfio_device_mig_state state)
>> }
>> }
>>
>> +static VfioMigrationState
>> +mig_state_to_qapi_state(enum vfio_device_mig_state state)
>> +{
>> + switch (state) {
>> + case VFIO_DEVICE_STATE_STOP:
>> + return QAPI_VFIO_MIGRATION_STATE_STOP;
>> + case VFIO_DEVICE_STATE_RUNNING:
>> + return QAPI_VFIO_MIGRATION_STATE_RUNNING;
>> + case VFIO_DEVICE_STATE_STOP_COPY:
>> + return QAPI_VFIO_MIGRATION_STATE_STOP_COPY;
>> + case VFIO_DEVICE_STATE_RESUMING:
>> + return QAPI_VFIO_MIGRATION_STATE_RESUMING;
>> + case VFIO_DEVICE_STATE_RUNNING_P2P:
>> + return QAPI_VFIO_MIGRATION_STATE_RUNNING_P2P;
>> + case VFIO_DEVICE_STATE_PRE_COPY:
>> + return QAPI_VFIO_MIGRATION_STATE_PRE_COPY;
>> + case VFIO_DEVICE_STATE_PRE_COPY_P2P:
>> + return QAPI_VFIO_MIGRATION_STATE_PRE_COPY_P2P;
>> + default:
>> + g_assert_not_reached();
>> + }
>> +}
>> +
>> +static void vfio_migration_send_event(VFIODevice *vbasedev)
>> +{
>> + VFIOMigration *migration = vbasedev->migration;
>> + DeviceState *dev = vbasedev->dev;
>> + g_autofree char *qom_path = NULL;
>> + Object *obj;
>> +
>> + if (!vbasedev->migration_events) {
>> + return;
>> + }
>
> I would add an assert on vbasedev->ops->vfio_get_object
>
>> + obj = vbasedev->ops->vfio_get_object(vbasedev);
>
> and another assert on obj.
vfio_migration_init() already checks these:
if (!vbasedev->ops->vfio_get_object) {
return -EINVAL;
}
obj = vbasedev->ops->vfio_get_object(vbasedev);
if (!obj) {
return -EINVAL;
}
Do you think these checks in migration init are enough?
>
>> + qom_path = object_get_canonical_path(obj);
>> +
>> + qapi_event_send_vfio_migration(
>> + dev->id, qom_path,
>> mig_state_to_qapi_state(migration->device_state));
>> +}
>> +
>> +static void set_state(VFIODevice *vbasedev, enum
>> vfio_device_mig_state state)
>
> to avoid the conflict with vfio_migration_set_state(), let's call it :
> vfio_migration_set_device_state() ? We want a 'vfio_migration_' prefix.
Sure, I will rename to that.
Thanks.
>
>
> Thanks,
>
> C.
>
>
>
>
>> +{
>> + VFIOMigration *migration = vbasedev->migration;
>> +
>> + migration->device_state = state;
>> + vfio_migration_send_event(vbasedev);
>> +}
>> +
>> static int vfio_migration_set_state(VFIODevice *vbasedev,
>> enum vfio_device_mig_state
>> new_state,
>> enum vfio_device_mig_state
>> recover_state)
>> @@ -125,12 +175,12 @@ static int vfio_migration_set_state(VFIODevice
>> *vbasedev,
>> goto reset_device;
>> }
>>
>> - migration->device_state = recover_state;
>> + set_state(vbasedev, recover_state);
>>
>> return ret;
>> }
>>
>> - migration->device_state = new_state;
>> + set_state(vbasedev, new_state);
>> if (mig_state->data_fd != -1) {
>> if (migration->data_fd != -1) {
>> /*
>> @@ -156,7 +206,7 @@ reset_device:
>> strerror(errno));
>> }
>>
>> - migration->device_state = VFIO_DEVICE_STATE_RUNNING;
>> + set_state(vbasedev, VFIO_DEVICE_STATE_RUNNING);
>>
>> return ret;
>> }
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index 64780d1b79..8840602c50 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -3362,6 +3362,8 @@ static Property vfio_pci_dev_properties[] = {
>> VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT, false),
>> DEFINE_PROP_ON_OFF_AUTO("enable-migration", VFIOPCIDevice,
>> vbasedev.enable_migration,
>> ON_OFF_AUTO_AUTO),
>> + DEFINE_PROP_BOOL("migration-events", VFIOPCIDevice,
>> + vbasedev.migration_events, false),
>> DEFINE_PROP_BOOL("x-no-mmap", VFIOPCIDevice, vbasedev.no_mmap,
>> false),
>> DEFINE_PROP_BOOL("x-balloon-allowed", VFIOPCIDevice,
>> vbasedev.ram_block_discard_allowed, false),
>
On 5/13/24 16:34, Avihai Horon wrote:
>
> On 13/05/2024 17:01, Cédric Le Goater wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 5/9/24 11:09, Avihai Horon wrote:
>>> Emit VFIO migration QAPI event when a VFIO device changes its migration
>>> state. This can be used by management applications to get updates on the
>>> current state of the VFIO device for their own purposes.
>>>
>>> A new per VFIO device capability, "migration-events", is added so events
>>> can be enabled only for the required devices. It is disabled by default.
>>>
>>> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
>>> ---
>>> include/hw/vfio/vfio-common.h | 1 +
>>> hw/vfio/migration.c | 56 +++++++++++++++++++++++++++++++++--
>>> hw/vfio/pci.c | 2 ++
>>> 3 files changed, 56 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
>>> index b9da6c08ef..3ec5f2425e 100644
>>> --- a/include/hw/vfio/vfio-common.h
>>> +++ b/include/hw/vfio/vfio-common.h
>>> @@ -115,6 +115,7 @@ typedef struct VFIODevice {
>>> bool no_mmap;
>>> bool ram_block_discard_allowed;
>>> OnOffAuto enable_migration;
>>> + bool migration_events;
>>> VFIODeviceOps *ops;
>>> unsigned int num_irqs;
>>> unsigned int num_regions;
>>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>>> index 06ae40969b..5a359c4c78 100644
>>> --- a/hw/vfio/migration.c
>>> +++ b/hw/vfio/migration.c
>>> @@ -24,6 +24,7 @@
>>> #include "migration/register.h"
>>> #include "migration/blocker.h"
>>> #include "qapi/error.h"
>>> +#include "qapi/qapi-events-vfio.h"
>>> #include "exec/ramlist.h"
>>> #include "exec/ram_addr.h"
>>> #include "pci.h"
>>> @@ -80,6 +81,55 @@ static const char *mig_state_to_str(enum vfio_device_mig_state state)
>>> }
>>> }
>>>
>>> +static VfioMigrationState
>>> +mig_state_to_qapi_state(enum vfio_device_mig_state state)
>>> +{
>>> + switch (state) {
>>> + case VFIO_DEVICE_STATE_STOP:
>>> + return QAPI_VFIO_MIGRATION_STATE_STOP;
>>> + case VFIO_DEVICE_STATE_RUNNING:
>>> + return QAPI_VFIO_MIGRATION_STATE_RUNNING;
>>> + case VFIO_DEVICE_STATE_STOP_COPY:
>>> + return QAPI_VFIO_MIGRATION_STATE_STOP_COPY;
>>> + case VFIO_DEVICE_STATE_RESUMING:
>>> + return QAPI_VFIO_MIGRATION_STATE_RESUMING;
>>> + case VFIO_DEVICE_STATE_RUNNING_P2P:
>>> + return QAPI_VFIO_MIGRATION_STATE_RUNNING_P2P;
>>> + case VFIO_DEVICE_STATE_PRE_COPY:
>>> + return QAPI_VFIO_MIGRATION_STATE_PRE_COPY;
>>> + case VFIO_DEVICE_STATE_PRE_COPY_P2P:
>>> + return QAPI_VFIO_MIGRATION_STATE_PRE_COPY_P2P;
>>> + default:
>>> + g_assert_not_reached();
>>> + }
>>> +}
>>> +
>>> +static void vfio_migration_send_event(VFIODevice *vbasedev)
>>> +{
>>> + VFIOMigration *migration = vbasedev->migration;
>>> + DeviceState *dev = vbasedev->dev;
>>> + g_autofree char *qom_path = NULL;
>>> + Object *obj;
>>> +
>>> + if (!vbasedev->migration_events) {
>>> + return;
>>> + }
>>
>> I would add an assert on vbasedev->ops->vfio_get_object
>>
>>> + obj = vbasedev->ops->vfio_get_object(vbasedev);
>>
>> and another assert on obj.
>
> vfio_migration_init() already checks these:
>
> if (!vbasedev->ops->vfio_get_object) {
> return -EINVAL;
> }
>
> obj = vbasedev->ops->vfio_get_object(vbasedev);
> if (!obj) {
> return -EINVAL;
> }
>
> Do you think these checks in migration init are enough?
I am sure they are today. These extra asserts are to avoid issues if
the code is moved around or if anyone finds inspiration by reading
vfio_migration_send_event().
Thanks,
C.
>
>>
>>> + qom_path = object_get_canonical_path(obj);
>>> +
>>> + qapi_event_send_vfio_migration(
>>> + dev->id, qom_path, mig_state_to_qapi_state(migration->device_state));
>>> +}
>>> +
>>> +static void set_state(VFIODevice *vbasedev, enum vfio_device_mig_state state)
>>
>> to avoid the conflict with vfio_migration_set_state(), let's call it :
>> vfio_migration_set_device_state() ? We want a 'vfio_migration_' prefix.
>
> Sure, I will rename to that.
>
> Thanks.
>
>>
>>
>> Thanks,
>>
>> C.
>>
>>
>>
>>
>>> +{
>>> + VFIOMigration *migration = vbasedev->migration;
>>> +
>>> + migration->device_state = state;
>>> + vfio_migration_send_event(vbasedev);
>>> +}
>>> +
>>> static int vfio_migration_set_state(VFIODevice *vbasedev,
>>> enum vfio_device_mig_state new_state,
>>> enum vfio_device_mig_state recover_state)
>>> @@ -125,12 +175,12 @@ static int vfio_migration_set_state(VFIODevice *vbasedev,
>>> goto reset_device;
>>> }
>>>
>>> - migration->device_state = recover_state;
>>> + set_state(vbasedev, recover_state);
>>>
>>> return ret;
>>> }
>>>
>>> - migration->device_state = new_state;
>>> + set_state(vbasedev, new_state);
>>> if (mig_state->data_fd != -1) {
>>> if (migration->data_fd != -1) {
>>> /*
>>> @@ -156,7 +206,7 @@ reset_device:
>>> strerror(errno));
>>> }
>>>
>>> - migration->device_state = VFIO_DEVICE_STATE_RUNNING;
>>> + set_state(vbasedev, VFIO_DEVICE_STATE_RUNNING);
>>>
>>> return ret;
>>> }
>>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>>> index 64780d1b79..8840602c50 100644
>>> --- a/hw/vfio/pci.c
>>> +++ b/hw/vfio/pci.c
>>> @@ -3362,6 +3362,8 @@ static Property vfio_pci_dev_properties[] = {
>>> VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT, false),
>>> DEFINE_PROP_ON_OFF_AUTO("enable-migration", VFIOPCIDevice,
>>> vbasedev.enable_migration, ON_OFF_AUTO_AUTO),
>>> + DEFINE_PROP_BOOL("migration-events", VFIOPCIDevice,
>>> + vbasedev.migration_events, false),
>>> DEFINE_PROP_BOOL("x-no-mmap", VFIOPCIDevice, vbasedev.no_mmap, false),
>>> DEFINE_PROP_BOOL("x-balloon-allowed", VFIOPCIDevice,
>>> vbasedev.ram_block_discard_allowed, false),
>>
>
On 13/05/2024 17:43, Cédric Le Goater wrote:
> External email: Use caution opening links or attachments
>
>
> On 5/13/24 16:34, Avihai Horon wrote:
>>
>> On 13/05/2024 17:01, Cédric Le Goater wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On 5/9/24 11:09, Avihai Horon wrote:
>>>> Emit VFIO migration QAPI event when a VFIO device changes its
>>>> migration
>>>> state. This can be used by management applications to get updates
>>>> on the
>>>> current state of the VFIO device for their own purposes.
>>>>
>>>> A new per VFIO device capability, "migration-events", is added so
>>>> events
>>>> can be enabled only for the required devices. It is disabled by
>>>> default.
>>>>
>>>> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
>>>> ---
>>>> include/hw/vfio/vfio-common.h | 1 +
>>>> hw/vfio/migration.c | 56
>>>> +++++++++++++++++++++++++++++++++--
>>>> hw/vfio/pci.c | 2 ++
>>>> 3 files changed, 56 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/include/hw/vfio/vfio-common.h
>>>> b/include/hw/vfio/vfio-common.h
>>>> index b9da6c08ef..3ec5f2425e 100644
>>>> --- a/include/hw/vfio/vfio-common.h
>>>> +++ b/include/hw/vfio/vfio-common.h
>>>> @@ -115,6 +115,7 @@ typedef struct VFIODevice {
>>>> bool no_mmap;
>>>> bool ram_block_discard_allowed;
>>>> OnOffAuto enable_migration;
>>>> + bool migration_events;
>>>> VFIODeviceOps *ops;
>>>> unsigned int num_irqs;
>>>> unsigned int num_regions;
>>>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>>>> index 06ae40969b..5a359c4c78 100644
>>>> --- a/hw/vfio/migration.c
>>>> +++ b/hw/vfio/migration.c
>>>> @@ -24,6 +24,7 @@
>>>> #include "migration/register.h"
>>>> #include "migration/blocker.h"
>>>> #include "qapi/error.h"
>>>> +#include "qapi/qapi-events-vfio.h"
>>>> #include "exec/ramlist.h"
>>>> #include "exec/ram_addr.h"
>>>> #include "pci.h"
>>>> @@ -80,6 +81,55 @@ static const char *mig_state_to_str(enum
>>>> vfio_device_mig_state state)
>>>> }
>>>> }
>>>>
>>>> +static VfioMigrationState
>>>> +mig_state_to_qapi_state(enum vfio_device_mig_state state)
>>>> +{
>>>> + switch (state) {
>>>> + case VFIO_DEVICE_STATE_STOP:
>>>> + return QAPI_VFIO_MIGRATION_STATE_STOP;
>>>> + case VFIO_DEVICE_STATE_RUNNING:
>>>> + return QAPI_VFIO_MIGRATION_STATE_RUNNING;
>>>> + case VFIO_DEVICE_STATE_STOP_COPY:
>>>> + return QAPI_VFIO_MIGRATION_STATE_STOP_COPY;
>>>> + case VFIO_DEVICE_STATE_RESUMING:
>>>> + return QAPI_VFIO_MIGRATION_STATE_RESUMING;
>>>> + case VFIO_DEVICE_STATE_RUNNING_P2P:
>>>> + return QAPI_VFIO_MIGRATION_STATE_RUNNING_P2P;
>>>> + case VFIO_DEVICE_STATE_PRE_COPY:
>>>> + return QAPI_VFIO_MIGRATION_STATE_PRE_COPY;
>>>> + case VFIO_DEVICE_STATE_PRE_COPY_P2P:
>>>> + return QAPI_VFIO_MIGRATION_STATE_PRE_COPY_P2P;
>>>> + default:
>>>> + g_assert_not_reached();
>>>> + }
>>>> +}
>>>> +
>>>> +static void vfio_migration_send_event(VFIODevice *vbasedev)
>>>> +{
>>>> + VFIOMigration *migration = vbasedev->migration;
>>>> + DeviceState *dev = vbasedev->dev;
>>>> + g_autofree char *qom_path = NULL;
>>>> + Object *obj;
>>>> +
>>>> + if (!vbasedev->migration_events) {
>>>> + return;
>>>> + }
>>>
>>> I would add an assert on vbasedev->ops->vfio_get_object
>>>
>>>> + obj = vbasedev->ops->vfio_get_object(vbasedev);
>>>
>>> and another assert on obj.
>>
>> vfio_migration_init() already checks these:
>>
>> if (!vbasedev->ops->vfio_get_object) {
>> return -EINVAL;
>> }
>>
>> obj = vbasedev->ops->vfio_get_object(vbasedev);
>> if (!obj) {
>> return -EINVAL;
>> }
>>
>> Do you think these checks in migration init are enough?
>
> I am sure they are today. These extra asserts are to avoid issues if
> the code is moved around or if anyone finds inspiration by reading
> vfio_migration_send_event().
>
Ah, I see your point.
I will add the asserts then.
Thanks.
> Thanks,
>
> C.
>
>
>
>>
>>>
>>>> + qom_path = object_get_canonical_path(obj);
>>>> +
>>>> + qapi_event_send_vfio_migration(
>>>> + dev->id, qom_path,
>>>> mig_state_to_qapi_state(migration->device_state));
>>>> +}
>>>> +
>>>> +static void set_state(VFIODevice *vbasedev, enum
>>>> vfio_device_mig_state state)
>>>
>>> to avoid the conflict with vfio_migration_set_state(), let's call it :
>>> vfio_migration_set_device_state() ? We want a 'vfio_migration_' prefix.
>>
>> Sure, I will rename to that.
>>
>> Thanks.
>>
>>>
>>>
>>> Thanks,
>>>
>>> C.
>>>
>>>
>>>
>>>
>>>> +{
>>>> + VFIOMigration *migration = vbasedev->migration;
>>>> +
>>>> + migration->device_state = state;
>>>> + vfio_migration_send_event(vbasedev);
>>>> +}
>>>> +
>>>> static int vfio_migration_set_state(VFIODevice *vbasedev,
>>>> enum vfio_device_mig_state
>>>> new_state,
>>>> enum vfio_device_mig_state
>>>> recover_state)
>>>> @@ -125,12 +175,12 @@ static int
>>>> vfio_migration_set_state(VFIODevice *vbasedev,
>>>> goto reset_device;
>>>> }
>>>>
>>>> - migration->device_state = recover_state;
>>>> + set_state(vbasedev, recover_state);
>>>>
>>>> return ret;
>>>> }
>>>>
>>>> - migration->device_state = new_state;
>>>> + set_state(vbasedev, new_state);
>>>> if (mig_state->data_fd != -1) {
>>>> if (migration->data_fd != -1) {
>>>> /*
>>>> @@ -156,7 +206,7 @@ reset_device:
>>>> strerror(errno));
>>>> }
>>>>
>>>> - migration->device_state = VFIO_DEVICE_STATE_RUNNING;
>>>> + set_state(vbasedev, VFIO_DEVICE_STATE_RUNNING);
>>>>
>>>> return ret;
>>>> }
>>>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>>>> index 64780d1b79..8840602c50 100644
>>>> --- a/hw/vfio/pci.c
>>>> +++ b/hw/vfio/pci.c
>>>> @@ -3362,6 +3362,8 @@ static Property vfio_pci_dev_properties[] = {
>>>> VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT, false),
>>>> DEFINE_PROP_ON_OFF_AUTO("enable-migration", VFIOPCIDevice,
>>>> vbasedev.enable_migration,
>>>> ON_OFF_AUTO_AUTO),
>>>> + DEFINE_PROP_BOOL("migration-events", VFIOPCIDevice,
>>>> + vbasedev.migration_events, false),
>>>> DEFINE_PROP_BOOL("x-no-mmap", VFIOPCIDevice,
>>>> vbasedev.no_mmap, false),
>>>> DEFINE_PROP_BOOL("x-balloon-allowed", VFIOPCIDevice,
>>>> vbasedev.ram_block_discard_allowed, false),
>>>
>>
>
© 2016 - 2024 Red Hat, Inc.