Re: [PATCH v16 QEMU 05/16] vfio: Add migration region initialization and finalize function

2020-05-19 Thread Dr. David Alan Gilbert
* Kirti Wankhede (kwankh...@nvidia.com) wrote:
> 
> 
> On 3/26/2020 11:22 PM, Dr. David Alan Gilbert wrote:
> > * Kirti Wankhede (kwankh...@nvidia.com) wrote:
> > > - Migration functions are implemented for VFIO_DEVICE_TYPE_PCI device in 
> > > this
> > >patch series.
> > > - VFIO device supports migration or not is decided based of migration 
> > > region
> > >query. If migration region query is successful and migration region
> > >initialization is successful then migration is supported else 
> > > migration is
> > >blocked.
> > > 
> > > Signed-off-by: Kirti Wankhede 
> > > Reviewed-by: Neo Jia 
> > > ---
> > >   hw/vfio/Makefile.objs |   2 +-
> > >   hw/vfio/migration.c   | 138 
> > > ++
> > >   hw/vfio/trace-events  |   3 +
> > >   include/hw/vfio/vfio-common.h |   9 +++
> > >   4 files changed, 151 insertions(+), 1 deletion(-)
> > >   create mode 100644 hw/vfio/migration.c
> > > 
> > > diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
> > > index 9bb1c09e8477..8b296c889ed9 100644
> > > --- a/hw/vfio/Makefile.objs
> > > +++ b/hw/vfio/Makefile.objs
> > > @@ -1,4 +1,4 @@
> > > -obj-y += common.o spapr.o
> > > +obj-y += common.o spapr.o migration.o
> > >   obj-$(CONFIG_VFIO_PCI) += pci.o pci-quirks.o display.o
> > >   obj-$(CONFIG_VFIO_CCW) += ccw.o
> > >   obj-$(CONFIG_VFIO_PLATFORM) += platform.o
> > > diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> > > new file mode 100644
> > > index ..a078dcf1dd8f
> > > --- /dev/null
> > > +++ b/hw/vfio/migration.c
> > > @@ -0,0 +1,138 @@
> > > +/*
> > > + * Migration support for VFIO devices
> > > + *
> > > + * Copyright NVIDIA, Inc. 2019
> > 
> > Time flies by...
> > 
> > > + *
> > > + * This work is licensed under the terms of the GNU GPL, version 2. See
> > > + * the COPYING file in the top-level directory.
> > 
> > Are you sure you want this to be V2 only? Most code added to qemu now is
> > v2 or later.
> > 
> 
> I kept it same as in files vfio-pci and hw/vfio/common.c
> 
> Should it be different? Can you give some reference what it should be?

It's OK to be v2; there's a statement in LICENSE saying hw/vfio is
acceptable for that; most other new code we try and make
'version 2 or later' - if you want it as 2 that is fine though.

Dave

> Thanks,
> Kirti
> 
> > > + */
> > > +
> > > +#include "qemu/osdep.h"
> > > +#include 
> > > +
> > > +#include "hw/vfio/vfio-common.h"
> > > +#include "cpu.h"
> > > +#include "migration/migration.h"
> > > +#include "migration/qemu-file.h"
> > > +#include "migration/register.h"
> > > +#include "migration/blocker.h"
> > > +#include "migration/misc.h"
> > > +#include "qapi/error.h"
> > > +#include "exec/ramlist.h"
> > > +#include "exec/ram_addr.h"
> > > +#include "pci.h"
> > > +#include "trace.h"
> > > +
> > > +static void vfio_migration_region_exit(VFIODevice *vbasedev)
> > > +{
> > > +VFIOMigration *migration = vbasedev->migration;
> > > +
> > > +if (!migration) {
> > > +return;
> > > +}
> > > +
> > > +if (migration->region.size) {
> > > +vfio_region_exit(>region);
> > > +vfio_region_finalize(>region);
> > > +}
> > > +}
> > > +
> > > +static int vfio_migration_region_init(VFIODevice *vbasedev, int index)
> > > +{
> > > +VFIOMigration *migration = vbasedev->migration;
> > > +Object *obj = NULL;
> > > +int ret = -EINVAL;
> > > +
> > > +if (!vbasedev->ops->vfio_get_object) {
> > > +return ret;
> > > +}
> > > +
> > > +obj = vbasedev->ops->vfio_get_object(vbasedev);
> > > +if (!obj) {
> > > +return ret;
> > > +}
> > > +
> > > +ret = vfio_region_setup(obj, vbasedev, >region, index,
> > > +"migration");
> > > +if (ret) {
> > > +error_report("%s: Failed to setup VFIO migration region %d: %s",
> > > + vbasedev->name, index, strerror(-ret));
> > > +goto err;
> > > +}
> > > +
> > > +if (!migration->region.size) {
> > > +ret = -EINVAL;
> > > +error_report("%s: Invalid region size of VFIO migration region 
> > > %d: %s",
> > > + vbasedev->name, index, strerror(-ret));
> > > +goto err;
> > > +}
> > > +
> > > +return 0;
> > > +
> > > +err:
> > > +vfio_migration_region_exit(vbasedev);
> > > +return ret;
> > > +}
> > > +
> > > +static int vfio_migration_init(VFIODevice *vbasedev,
> > > +   struct vfio_region_info *info)
> > > +{
> > > +int ret;
> > > +
> > > +vbasedev->migration = g_new0(VFIOMigration, 1);
> > > +
> > > +ret = vfio_migration_region_init(vbasedev, info->index);
> > > +if (ret) {
> > > +error_report("%s: Failed to initialise migration region",
> > > + vbasedev->name);
> > > +g_free(vbasedev->migration);
> > > +vbasedev->migration = NULL;
> > > +return ret;
> > > +}
> > > +
> > > +return 0;
> > 

Re: [PATCH v16 QEMU 05/16] vfio: Add migration region initialization and finalize function

2020-05-04 Thread Kirti Wankhede




On 3/26/2020 11:22 PM, Dr. David Alan Gilbert wrote:

* Kirti Wankhede (kwankh...@nvidia.com) wrote:

- Migration functions are implemented for VFIO_DEVICE_TYPE_PCI device in this
   patch series.
- VFIO device supports migration or not is decided based of migration region
   query. If migration region query is successful and migration region
   initialization is successful then migration is supported else migration is
   blocked.

Signed-off-by: Kirti Wankhede 
Reviewed-by: Neo Jia 
---
  hw/vfio/Makefile.objs |   2 +-
  hw/vfio/migration.c   | 138 ++
  hw/vfio/trace-events  |   3 +
  include/hw/vfio/vfio-common.h |   9 +++
  4 files changed, 151 insertions(+), 1 deletion(-)
  create mode 100644 hw/vfio/migration.c

diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
index 9bb1c09e8477..8b296c889ed9 100644
--- a/hw/vfio/Makefile.objs
+++ b/hw/vfio/Makefile.objs
@@ -1,4 +1,4 @@
-obj-y += common.o spapr.o
+obj-y += common.o spapr.o migration.o
  obj-$(CONFIG_VFIO_PCI) += pci.o pci-quirks.o display.o
  obj-$(CONFIG_VFIO_CCW) += ccw.o
  obj-$(CONFIG_VFIO_PLATFORM) += platform.o
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
new file mode 100644
index ..a078dcf1dd8f
--- /dev/null
+++ b/hw/vfio/migration.c
@@ -0,0 +1,138 @@
+/*
+ * Migration support for VFIO devices
+ *
+ * Copyright NVIDIA, Inc. 2019


Time flies by...


+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.


Are you sure you want this to be V2 only? Most code added to qemu now is
v2 or later.



I kept it same as in files vfio-pci and hw/vfio/common.c

Should it be different? Can you give some reference what it should be?

Thanks,
Kirti


+ */
+
+#include "qemu/osdep.h"
+#include 
+
+#include "hw/vfio/vfio-common.h"
+#include "cpu.h"
+#include "migration/migration.h"
+#include "migration/qemu-file.h"
+#include "migration/register.h"
+#include "migration/blocker.h"
+#include "migration/misc.h"
+#include "qapi/error.h"
+#include "exec/ramlist.h"
+#include "exec/ram_addr.h"
+#include "pci.h"
+#include "trace.h"
+
+static void vfio_migration_region_exit(VFIODevice *vbasedev)
+{
+VFIOMigration *migration = vbasedev->migration;
+
+if (!migration) {
+return;
+}
+
+if (migration->region.size) {
+vfio_region_exit(>region);
+vfio_region_finalize(>region);
+}
+}
+
+static int vfio_migration_region_init(VFIODevice *vbasedev, int index)
+{
+VFIOMigration *migration = vbasedev->migration;
+Object *obj = NULL;
+int ret = -EINVAL;
+
+if (!vbasedev->ops->vfio_get_object) {
+return ret;
+}
+
+obj = vbasedev->ops->vfio_get_object(vbasedev);
+if (!obj) {
+return ret;
+}
+
+ret = vfio_region_setup(obj, vbasedev, >region, index,
+"migration");
+if (ret) {
+error_report("%s: Failed to setup VFIO migration region %d: %s",
+ vbasedev->name, index, strerror(-ret));
+goto err;
+}
+
+if (!migration->region.size) {
+ret = -EINVAL;
+error_report("%s: Invalid region size of VFIO migration region %d: %s",
+ vbasedev->name, index, strerror(-ret));
+goto err;
+}
+
+return 0;
+
+err:
+vfio_migration_region_exit(vbasedev);
+return ret;
+}
+
+static int vfio_migration_init(VFIODevice *vbasedev,
+   struct vfio_region_info *info)
+{
+int ret;
+
+vbasedev->migration = g_new0(VFIOMigration, 1);
+
+ret = vfio_migration_region_init(vbasedev, info->index);
+if (ret) {
+error_report("%s: Failed to initialise migration region",
+ vbasedev->name);
+g_free(vbasedev->migration);
+vbasedev->migration = NULL;
+return ret;
+}
+
+return 0;
+}
+
+/* -- */
+
+int vfio_migration_probe(VFIODevice *vbasedev, Error **errp)
+{
+struct vfio_region_info *info;
+Error *local_err = NULL;
+int ret;
+
+ret = vfio_get_dev_region_info(vbasedev, VFIO_REGION_TYPE_MIGRATION,
+   VFIO_REGION_SUBTYPE_MIGRATION, );
+if (ret) {
+goto add_blocker;
+}
+
+ret = vfio_migration_init(vbasedev, info);
+if (ret) {
+goto add_blocker;
+}
+
+trace_vfio_migration_probe(vbasedev->name, info->index);
+return 0;
+
+add_blocker:
+error_setg(>migration_blocker,
+   "VFIO device doesn't support migration");
+ret = migrate_add_blocker(vbasedev->migration_blocker, _err);
+if (local_err) {
+error_propagate(errp, local_err);
+error_free(vbasedev->migration_blocker);
+}
+return ret;
+}
+
+void vfio_migration_finalize(VFIODevice *vbasedev)
+{
+if (vbasedev->migration_blocker) {
+migrate_del_blocker(vbasedev->migration_blocker);

Re: [PATCH v16 QEMU 05/16] vfio: Add migration region initialization and finalize function

2020-03-26 Thread Dr. David Alan Gilbert
* Kirti Wankhede (kwankh...@nvidia.com) wrote:
> - Migration functions are implemented for VFIO_DEVICE_TYPE_PCI device in this
>   patch series.
> - VFIO device supports migration or not is decided based of migration region
>   query. If migration region query is successful and migration region
>   initialization is successful then migration is supported else migration is
>   blocked.
> 
> Signed-off-by: Kirti Wankhede 
> Reviewed-by: Neo Jia 
> ---
>  hw/vfio/Makefile.objs |   2 +-
>  hw/vfio/migration.c   | 138 
> ++
>  hw/vfio/trace-events  |   3 +
>  include/hw/vfio/vfio-common.h |   9 +++
>  4 files changed, 151 insertions(+), 1 deletion(-)
>  create mode 100644 hw/vfio/migration.c
> 
> diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
> index 9bb1c09e8477..8b296c889ed9 100644
> --- a/hw/vfio/Makefile.objs
> +++ b/hw/vfio/Makefile.objs
> @@ -1,4 +1,4 @@
> -obj-y += common.o spapr.o
> +obj-y += common.o spapr.o migration.o
>  obj-$(CONFIG_VFIO_PCI) += pci.o pci-quirks.o display.o
>  obj-$(CONFIG_VFIO_CCW) += ccw.o
>  obj-$(CONFIG_VFIO_PLATFORM) += platform.o
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> new file mode 100644
> index ..a078dcf1dd8f
> --- /dev/null
> +++ b/hw/vfio/migration.c
> @@ -0,0 +1,138 @@
> +/*
> + * Migration support for VFIO devices
> + *
> + * Copyright NVIDIA, Inc. 2019

Time flies by...

> + *
> + * This work is licensed under the terms of the GNU GPL, version 2. See
> + * the COPYING file in the top-level directory.

Are you sure you want this to be V2 only? Most code added to qemu now is
v2 or later.

> + */
> +
> +#include "qemu/osdep.h"
> +#include 
> +
> +#include "hw/vfio/vfio-common.h"
> +#include "cpu.h"
> +#include "migration/migration.h"
> +#include "migration/qemu-file.h"
> +#include "migration/register.h"
> +#include "migration/blocker.h"
> +#include "migration/misc.h"
> +#include "qapi/error.h"
> +#include "exec/ramlist.h"
> +#include "exec/ram_addr.h"
> +#include "pci.h"
> +#include "trace.h"
> +
> +static void vfio_migration_region_exit(VFIODevice *vbasedev)
> +{
> +VFIOMigration *migration = vbasedev->migration;
> +
> +if (!migration) {
> +return;
> +}
> +
> +if (migration->region.size) {
> +vfio_region_exit(>region);
> +vfio_region_finalize(>region);
> +}
> +}
> +
> +static int vfio_migration_region_init(VFIODevice *vbasedev, int index)
> +{
> +VFIOMigration *migration = vbasedev->migration;
> +Object *obj = NULL;
> +int ret = -EINVAL;
> +
> +if (!vbasedev->ops->vfio_get_object) {
> +return ret;
> +}
> +
> +obj = vbasedev->ops->vfio_get_object(vbasedev);
> +if (!obj) {
> +return ret;
> +}
> +
> +ret = vfio_region_setup(obj, vbasedev, >region, index,
> +"migration");
> +if (ret) {
> +error_report("%s: Failed to setup VFIO migration region %d: %s",
> + vbasedev->name, index, strerror(-ret));
> +goto err;
> +}
> +
> +if (!migration->region.size) {
> +ret = -EINVAL;
> +error_report("%s: Invalid region size of VFIO migration region %d: 
> %s",
> + vbasedev->name, index, strerror(-ret));
> +goto err;
> +}
> +
> +return 0;
> +
> +err:
> +vfio_migration_region_exit(vbasedev);
> +return ret;
> +}
> +
> +static int vfio_migration_init(VFIODevice *vbasedev,
> +   struct vfio_region_info *info)
> +{
> +int ret;
> +
> +vbasedev->migration = g_new0(VFIOMigration, 1);
> +
> +ret = vfio_migration_region_init(vbasedev, info->index);
> +if (ret) {
> +error_report("%s: Failed to initialise migration region",
> + vbasedev->name);
> +g_free(vbasedev->migration);
> +vbasedev->migration = NULL;
> +return ret;
> +}
> +
> +return 0;
> +}
> +
> +/* -- */
> +
> +int vfio_migration_probe(VFIODevice *vbasedev, Error **errp)
> +{
> +struct vfio_region_info *info;
> +Error *local_err = NULL;
> +int ret;
> +
> +ret = vfio_get_dev_region_info(vbasedev, VFIO_REGION_TYPE_MIGRATION,
> +   VFIO_REGION_SUBTYPE_MIGRATION, );
> +if (ret) {
> +goto add_blocker;
> +}
> +
> +ret = vfio_migration_init(vbasedev, info);
> +if (ret) {
> +goto add_blocker;
> +}
> +
> +trace_vfio_migration_probe(vbasedev->name, info->index);
> +return 0;
> +
> +add_blocker:
> +error_setg(>migration_blocker,
> +   "VFIO device doesn't support migration");
> +ret = migrate_add_blocker(vbasedev->migration_blocker, _err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +error_free(vbasedev->migration_blocker);
> +}
> +return ret;
> +}
> +
> +void vfio_migration_finalize(VFIODevice 

[PATCH v16 QEMU 05/16] vfio: Add migration region initialization and finalize function

2020-03-24 Thread Kirti Wankhede
- Migration functions are implemented for VFIO_DEVICE_TYPE_PCI device in this
  patch series.
- VFIO device supports migration or not is decided based of migration region
  query. If migration region query is successful and migration region
  initialization is successful then migration is supported else migration is
  blocked.

Signed-off-by: Kirti Wankhede 
Reviewed-by: Neo Jia 
---
 hw/vfio/Makefile.objs |   2 +-
 hw/vfio/migration.c   | 138 ++
 hw/vfio/trace-events  |   3 +
 include/hw/vfio/vfio-common.h |   9 +++
 4 files changed, 151 insertions(+), 1 deletion(-)
 create mode 100644 hw/vfio/migration.c

diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
index 9bb1c09e8477..8b296c889ed9 100644
--- a/hw/vfio/Makefile.objs
+++ b/hw/vfio/Makefile.objs
@@ -1,4 +1,4 @@
-obj-y += common.o spapr.o
+obj-y += common.o spapr.o migration.o
 obj-$(CONFIG_VFIO_PCI) += pci.o pci-quirks.o display.o
 obj-$(CONFIG_VFIO_CCW) += ccw.o
 obj-$(CONFIG_VFIO_PLATFORM) += platform.o
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
new file mode 100644
index ..a078dcf1dd8f
--- /dev/null
+++ b/hw/vfio/migration.c
@@ -0,0 +1,138 @@
+/*
+ * Migration support for VFIO devices
+ *
+ * Copyright NVIDIA, Inc. 2019
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include 
+
+#include "hw/vfio/vfio-common.h"
+#include "cpu.h"
+#include "migration/migration.h"
+#include "migration/qemu-file.h"
+#include "migration/register.h"
+#include "migration/blocker.h"
+#include "migration/misc.h"
+#include "qapi/error.h"
+#include "exec/ramlist.h"
+#include "exec/ram_addr.h"
+#include "pci.h"
+#include "trace.h"
+
+static void vfio_migration_region_exit(VFIODevice *vbasedev)
+{
+VFIOMigration *migration = vbasedev->migration;
+
+if (!migration) {
+return;
+}
+
+if (migration->region.size) {
+vfio_region_exit(>region);
+vfio_region_finalize(>region);
+}
+}
+
+static int vfio_migration_region_init(VFIODevice *vbasedev, int index)
+{
+VFIOMigration *migration = vbasedev->migration;
+Object *obj = NULL;
+int ret = -EINVAL;
+
+if (!vbasedev->ops->vfio_get_object) {
+return ret;
+}
+
+obj = vbasedev->ops->vfio_get_object(vbasedev);
+if (!obj) {
+return ret;
+}
+
+ret = vfio_region_setup(obj, vbasedev, >region, index,
+"migration");
+if (ret) {
+error_report("%s: Failed to setup VFIO migration region %d: %s",
+ vbasedev->name, index, strerror(-ret));
+goto err;
+}
+
+if (!migration->region.size) {
+ret = -EINVAL;
+error_report("%s: Invalid region size of VFIO migration region %d: %s",
+ vbasedev->name, index, strerror(-ret));
+goto err;
+}
+
+return 0;
+
+err:
+vfio_migration_region_exit(vbasedev);
+return ret;
+}
+
+static int vfio_migration_init(VFIODevice *vbasedev,
+   struct vfio_region_info *info)
+{
+int ret;
+
+vbasedev->migration = g_new0(VFIOMigration, 1);
+
+ret = vfio_migration_region_init(vbasedev, info->index);
+if (ret) {
+error_report("%s: Failed to initialise migration region",
+ vbasedev->name);
+g_free(vbasedev->migration);
+vbasedev->migration = NULL;
+return ret;
+}
+
+return 0;
+}
+
+/* -- */
+
+int vfio_migration_probe(VFIODevice *vbasedev, Error **errp)
+{
+struct vfio_region_info *info;
+Error *local_err = NULL;
+int ret;
+
+ret = vfio_get_dev_region_info(vbasedev, VFIO_REGION_TYPE_MIGRATION,
+   VFIO_REGION_SUBTYPE_MIGRATION, );
+if (ret) {
+goto add_blocker;
+}
+
+ret = vfio_migration_init(vbasedev, info);
+if (ret) {
+goto add_blocker;
+}
+
+trace_vfio_migration_probe(vbasedev->name, info->index);
+return 0;
+
+add_blocker:
+error_setg(>migration_blocker,
+   "VFIO device doesn't support migration");
+ret = migrate_add_blocker(vbasedev->migration_blocker, _err);
+if (local_err) {
+error_propagate(errp, local_err);
+error_free(vbasedev->migration_blocker);
+}
+return ret;
+}
+
+void vfio_migration_finalize(VFIODevice *vbasedev)
+{
+if (vbasedev->migration_blocker) {
+migrate_del_blocker(vbasedev->migration_blocker);
+error_free(vbasedev->migration_blocker);
+}
+
+vfio_migration_region_exit(vbasedev);
+g_free(vbasedev->migration);
+}
diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
index 8cdc27946cb8..191a726a1312 100644
--- a/hw/vfio/trace-events
+++ b/hw/vfio/trace-events
@@ -143,3 +143,6 @@ vfio_display_edid_link_up(void) ""