On Fri, Apr 17, 2020 at 04:44:50PM +0800, Cornelia Huck wrote:

On Mon, 13 Apr 2020 01:52:01 -0400 Yan Zhao <yan.y.zhao@intel.com> wrote:

...

This patchset introduces a migration_version attribute under sysfs of VFIO Mediated devices.

This migration_version attribute is used to check migration compatibility between two mdev devices.

Currently, it has two locations: (1) under mdev_type node, which can be used even before device creation, but only for mdev devices of the same mdev type. (2) under mdev device node, which can only be used after the mdev devices are created, but the src and target mdev devices are not necessarily be of the same mdev type (The second location is newly added in v5, in order to keep consistent with the migration_version node for migratable pass-though devices)

What is the relationship between those two attributes?

(1) is for mdev devices specifically, and (2) is provided to keep the same sysfs interface as with non-mdev cases. so (2) is for both mdev devices and non-mdev devices. in future, if we enable vfio-pci vendor ops, (i.e. a non-mdev device is binding to vfio-pci, but is able to register migration region and do migration transactions from a vendor provided affiliate driver), the vendor driver would export (2) directly, under device node. It is not able to provide (1) as there're no mdev devices involved.

Is existence (and compatibility) of (1) a pre-req for possible existence (and compatibility) of (2)?

no. (2) does not reply on (1).

Does userspace need to check (1) or can it completely rely on (2), if it so chooses?

I think it can completely reply on (2) if compatibility check before mdev creation is not required.

If devices with a different mdev type are indeed compatible, it seems userspace can only find out after the devices have actually been created, as (1) does not apply? yes, I think so.

One of my worries is that the existence of an attribute with the same name in two similar locations might lead to confusion. But maybe it isn't a problem.

Yes, I have the same feeling. but as (2) is for sysfs interface consistency, to make it transparent to userspace tools like libvirt, I guess the same name is necessary? Thanks Yan

...

Patch 1 defines migration_version attribute for the first location in Documentation/vfio-mediated-device.txt

Patch 2 uses GVT as an example for patch 1 to show how to expose migration_version attribute and check migration compatibility in vendor driver.

Patch 3 defines migration_version attribute for the second location in Documentation/vfio-mediated-device.txt

Patch 4 uses GVT as an example for patch 3 to show how to expose migration_version attribute and check migration compatibility in vendor driver.

(The previous "Reviewed-by" and "Acked-by" for patch 1 and patch 2 are kept in v5, as there are only small changes to commit messages of the two patches.)

v5: added patch 2 and 4 for mdev device part of migration_version attribute.

v4: 1. fixed indentation/spell errors, reworded several error messages 2. added a missing memory free for error handling in patch 2

v3: 1. renamed version to migration_version 2. let errno to be freely defined by vendor driver 3. let checking mdev_type be prerequisite of migration compatibility check 4. reworded most part of patch 1 5. print detailed error log in patch 2 and generate migration_version string at init time

v2: 1. renamed patched 1 2. made definition of device version string completely private to vendor driver 3. reverted changes to sample mdev drivers 4. described intent and usage of version attribute more clearly.

Yan Zhao (4): vfio/mdev: add migration_version attribute for mdev (under mdev_type node) drm/i915/gvt: export migration_version to mdev sysfs (under mdev_type node) vfio/mdev: add migration_version attribute for mdev (under mdev device node) drm/i915/gvt: export migration_version to mdev sysfs (under mdev device node)

.../driver-api/vfio-mediated-device.rst | 183 ++++++++++++++++++ drivers/gpu/drm/i915/gvt/Makefile | 2 +- drivers/gpu/drm/i915/gvt/gvt.c | 39 ++++ drivers/gpu/drm/i915/gvt/gvt.h | 7 + drivers/gpu/drm/i915/gvt/kvmgt.c | 55 ++++++ drivers/gpu/drm/i915/gvt/migration_version.c | 170 ++++++++++++++++ drivers/gpu/drm/i915/gvt/vgpu.c | 13 +- 7 files changed, 466 insertions(+), 3 deletions(-) create mode 100644 drivers/gpu/drm/i915/gvt/migration_version.c

On Fri, Apr 17, 2020 at 07:24:57PM +0800, Cornelia Huck wrote:

On Fri, 17 Apr 2020 05:52:02 -0400 Yan Zhao <yan.y.zhao@intel.com> wrote:

...

On Fri, Apr 17, 2020 at 04:44:50PM +0800, Cornelia Huck wrote:

...

On Mon, 13 Apr 2020 01:52:01 -0400 Yan Zhao <yan.y.zhao@intel.com> wrote:

...

This patchset introduces a migration_version attribute under sysfs of VFIO Mediated devices.

This migration_version attribute is used to check migration compatibility between two mdev devices.

Currently, it has two locations: (1) under mdev_type node, which can be used even before device creation, but only for mdev devices of the same mdev type. (2) under mdev device node, which can only be used after the mdev devices are created, but the src and target mdev devices are not necessarily be of the same mdev type (The second location is newly added in v5, in order to keep consistent with the migration_version node for migratable pass-though devices)

What is the relationship between those two attributes?

(1) is for mdev devices specifically, and (2) is provided to keep the same sysfs interface as with non-mdev cases. so (2) is for both mdev devices and non-mdev devices.

in future, if we enable vfio-pci vendor ops, (i.e. a non-mdev device is binding to vfio-pci, but is able to register migration region and do migration transactions from a vendor provided affiliate driver), the vendor driver would export (2) directly, under device node. It is not able to provide (1) as there're no mdev devices involved.

Ok, creating an alternate attribute for non-mdev devices makes sense. However, wouldn't that rather be a case (3)? The change here only refers to mdev devices.

as you pointed below, (3) and (2) serve the same purpose. and I think a possible usage is to migrate between a non-mdev device and an mdev device. so I think it's better for them both to use (2) rather than creating (3).

...

...

Is existence (and compatibility) of (1) a pre-req for possible existence (and compatibility) of (2)?

no. (2) does not reply on (1).

Hm. Non-existence of (1) seems to imply "this type does not support migration". If an mdev created for such a type suddenly does support migration, it feels a bit odd.

yes. but I think if the condition happens, it should be reported a bug to vendor driver. should I add a line in the doc like "vendor driver should ensure that the migration compatibility from migration_version under mdev_type should be consistent with that from migration_version under device node" ?

(It obviously cannot be a prereq for what I called (3) above.)

...

...

Does userspace need to check (1) or can it completely rely on (2), if it so chooses?

I think it can completely reply on (2) if compatibility check before mdev creation is not required.

...

If devices with a different mdev type are indeed compatible, it seems userspace can only find out after the devices have actually been created, as (1) does not apply? yes, I think so.

How useful would it be for userspace to even look at (1) in that case? It only knows if things have a chance of working if it actually goes ahead and creates devices.

hmm, is it useful for userspace to test the migration_version under mdev type before it knows what mdev device to generate ? like when the userspace wants to migrate an mdev device in src vm, but it has not created target vm and the target mdev device.

...

...

One of my worries is that the existence of an attribute with the same name in two similar locations might lead to confusion. But maybe it isn't a problem.

Yes, I have the same feeling. but as (2) is for sysfs interface consistency, to make it transparent to userspace tools like libvirt, I guess the same name is necessary?

What do we actually need here, I wonder? (1) and (2) seem to serve slightly different purposes, while (2) and what I called (3) have the same purpose. Is it important to userspace that (1) and (2) have the same name? so change (1) to migration_type_version and (2) to migration_instance_version? But as they are under different locations, could that location imply enough information?

Thanks Yan

On Tue, Apr 21, 2020 at 06:56:00AM +0800, Alex Williamson wrote:

On Sun, 19 Apr 2020 21:24:57 -0400 Yan Zhao <yan.y.zhao@intel.com> wrote:

...

On Fri, Apr 17, 2020 at 07:24:57PM +0800, Cornelia Huck wrote:

...

On Fri, 17 Apr 2020 05:52:02 -0400 Yan Zhao <yan.y.zhao@intel.com> wrote:

...

On Fri, Apr 17, 2020 at 04:44:50PM +0800, Cornelia Huck wrote:

...

On Mon, 13 Apr 2020 01:52:01 -0400 Yan Zhao <yan.y.zhao@intel.com> wrote:

...

This patchset introduces a migration_version attribute under sysfs of VFIO Mediated devices.

This migration_version attribute is used to check migration compatibility between two mdev devices.

Currently, it has two locations: (1) under mdev_type node, which can be used even before device creation, but only for mdev devices of the same mdev type. (2) under mdev device node, which can only be used after the mdev devices are created, but the src and target mdev devices are not necessarily be of the same mdev type (The second location is newly added in v5, in order to keep consistent with the migration_version node for migratable pass-though devices)

What is the relationship between those two attributes?

(1) is for mdev devices specifically, and (2) is provided to keep the same sysfs interface as with non-mdev cases. so (2) is for both mdev devices and non-mdev devices.

in future, if we enable vfio-pci vendor ops, (i.e. a non-mdev device is binding to vfio-pci, but is able to register migration region and do migration transactions from a vendor provided affiliate driver), the vendor driver would export (2) directly, under device node. It is not able to provide (1) as there're no mdev devices involved.

Ok, creating an alternate attribute for non-mdev devices makes sense. However, wouldn't that rather be a case (3)? The change here only refers to mdev devices.

as you pointed below, (3) and (2) serve the same purpose. and I think a possible usage is to migrate between a non-mdev device and an mdev device. so I think it's better for them both to use (2) rather than creating (3).

An mdev type is meant to define a software compatible interface, so in the case of mdev->mdev migration, doesn't migrating to a different type fail the most basic of compatibility tests that we expect userspace to perform? IOW, if two mdev types are migration compatible, it seems a prerequisite to that is that they provide the same software interface, which means they should be the same mdev type.

In the hybrid cases of mdev->phys or phys->mdev, how does a management tool begin to even guess what might be compatible? Are we expecting libvirt to probe ever device with this attribute in the system? Is there going to be a new class hierarchy created to enumerate all possible migrate-able devices?

I agree that there was a gap in the previous proposal for non-mdev devices, but I think this bring a lot of questions that we need to puzzle through and libvirt will need to re-evaluate how they might decide to pick a migration target device. For example, I'm sure libvirt would reject any policy decisions regarding picking a physical device versus an mdev device. Had we previously left it that only a layer above libvirt would select a target device and libvirt only tests compatibility to that target device? I'm not sure if there's a layer above libvirt would select a target device. but if there is such a layer (even it's human), we need to

We also need to consider that this expands the namespace. If we no longer require matching types as the first level of comparison, then vendor migration strings can theoretically collide. How do we coordinate that can't happen? Thanks, yes, it's indeed a problem. could only allowing migration beteen devices from the same vendor be a good

yes, management tool needs to guess and test migration compatible between two devices. But I think it's not the problem only for mdev->phys or phys->mdev. even for mdev->mdev, management tool needs to first assume that the two mdevs have the same type of parent devices (e.g.their pciids are equal). otherwise, it's still enumerating possibilities. on the other hand, for two mdevs, mdev1 from pdev1, its mdev_type is 1/2 of pdev1; mdev2 from pdev2, its mdev_type is 1/4 of pdev2; if pdev2 is exactly 2 times of pdev1, why not allow migration between mdev1 <-> mdev2. provide an interface for them to know whether their decision is suitable for migration. The migration_version interface provides a potential to allow mdev->phys migration, even libvirt may currently reject it. prerequisite? Thanks Yan

...

...

...

...

Is existence (and compatibility) of (1) a pre-req for possible existence (and compatibility) of (2)?

no. (2) does not reply on (1).

Hm. Non-existence of (1) seems to imply "this type does not support migration". If an mdev created for such a type suddenly does support migration, it feels a bit odd.

yes. but I think if the condition happens, it should be reported a bug to vendor driver. should I add a line in the doc like "vendor driver should ensure that the migration compatibility from migration_version under mdev_type should be consistent with that from migration_version under device node" ?

...

(It obviously cannot be a prereq for what I called (3) above.)

...

...

Does userspace need to check (1) or can it completely rely on (2), if it so chooses?

I think it can completely reply on (2) if compatibility check before mdev creation is not required.

...

If devices with a different mdev type are indeed compatible, it seems userspace can only find out after the devices have actually been created, as (1) does not apply? yes, I think so.

How useful would it be for userspace to even look at (1) in that case? It only knows if things have a chance of working if it actually goes ahead and creates devices.

hmm, is it useful for userspace to test the migration_version under mdev type before it knows what mdev device to generate ? like when the userspace wants to migrate an mdev device in src vm, but it has not created target vm and the target mdev device.

...

...

...

One of my worries is that the existence of an attribute with the same name in two similar locations might lead to confusion. But maybe it isn't a problem.

Yes, I have the same feeling. but as (2) is for sysfs interface consistency, to make it transparent to userspace tools like libvirt, I guess the same name is necessary?

What do we actually need here, I wonder? (1) and (2) seem to serve slightly different purposes, while (2) and what I called (3) have the same purpose. Is it important to userspace that (1) and (2) have the same name? so change (1) to migration_type_version and (2) to migration_instance_version? But as they are under different locations, could that location imply enough information?

Thanks Yan

...

From: Yan Zhao Sent: Tuesday, April 21, 2020 10:37 AM

...

On Sun, 19 Apr 2020 21:24:57 -0400 Yan Zhao <yan.y.zhao@intel.com> wrote:

...

On Fri, Apr 17, 2020 at 07:24:57PM +0800, Cornelia Huck wrote:

...

On Fri, 17 Apr 2020 05:52:02 -0400 Yan Zhao <yan.y.zhao@intel.com> wrote:

...

On Fri, Apr 17, 2020 at 04:44:50PM +0800, Cornelia Huck wrote: > On Mon, 13 Apr 2020 01:52:01 -0400 > Yan Zhao <yan.y.zhao@intel.com> wrote: > > > This patchset introduces a migration_version attribute under sysfs of VFIO > > Mediated devices. > > > > This migration_version attribute is used to check migration compatibility > > between two mdev devices. > > > > Currently, it has two locations: > > (1) under mdev_type node, > > which can be used even before device creation, but only for mdev > > devices of the same mdev type. > > (2) under mdev device node, > > which can only be used after the mdev devices are created, but

On Tue, Apr 21, 2020 at 06:56:00AM +0800, Alex Williamson wrote: the src

...

...

...

...

> > and target mdev devices are not necessarily be of the same mdev type > > (The second location is newly added in v5, in order to keep consistent > > with the migration_version node for migratable pass-though devices) > > What is the relationship between those two attributes? > (1) is for mdev devices specifically, and (2) is provided to keep the same sysfs interface as with non-mdev cases. so (2) is for both mdev devices and non-mdev devices.

in future, if we enable vfio-pci vendor ops, (i.e. a non-mdev device is binding to vfio-pci, but is able to register migration region and do migration transactions from a vendor provided affiliate driver), the vendor driver would export (2) directly, under device node. It is not able to provide (1) as there're no mdev devices involved.

Ok, creating an alternate attribute for non-mdev devices makes sense. However, wouldn't that rather be a case (3)? The change here only refers to mdev devices.

as you pointed below, (3) and (2) serve the same purpose. and I think a possible usage is to migrate between a non-mdev device and an mdev device. so I think it's better for them both to use (2) rather than creating (3).

An mdev type is meant to define a software compatible interface, so in the case of mdev->mdev migration, doesn't migrating to a different type fail the most basic of compatibility tests that we expect userspace to perform? IOW, if two mdev types are migration compatible, it seems a prerequisite to that is that they provide the same software interface, which means they should be the same mdev type.

In the hybrid cases of mdev->phys or phys->mdev, how does a management tool begin to even guess what might be compatible? Are we expecting libvirt to probe ever device with this attribute in the system? Is there going to be a new class hierarchy created to enumerate all possible migrate-able devices?

yes, management tool needs to guess and test migration compatible between two devices. But I think it's not the problem only for mdev->phys or phys->mdev. even for mdev->mdev, management tool needs to first assume that the two mdevs have the same type of parent devices (e.g.their pciids are equal). otherwise, it's still enumerating possibilities.

on the other hand, for two mdevs, mdev1 from pdev1, its mdev_type is 1/2 of pdev1; mdev2 from pdev2, its mdev_type is 1/4 of pdev2; if pdev2 is exactly 2 times of pdev1, why not allow migration between mdev1 <-> mdev2.

How could the manage tool figure out that 1/2 of pdev1 is equivalent to 1/4 of pdev2? If we really want to allow such thing happen, the best choice is to report the same mdev type on both pdev1 and pdev2. I think that's exactly the value of this migration_version interface.

On Tue, Apr 21, 2020 at 08:08:49PM +0800, Tian, Kevin wrote: the management tool can take advantage of this interface to know if two devices are migration compatible, no matter they are mdevs, non-mdevs, or mix. as I know, (please correct me if not right), current libvirt still requires manually generating mdev devices, and it just duplicates src vm configuration to the target vm. for libvirt, currently it's always phys->phys and mdev->mdev (and of the same mdev type). But it does not justify that hybrid cases should not be allowed. otherwise, why do we need to introduce this migration_version interface and leave the judgement of migration compatibility to vendor driver? why not simply set the criteria to something like "pciids of parent devices are equal, and mdev types are equal" ?

btw mdev<->phys just brings trouble to upper stack as Alex pointed out. could you help me understand why it will bring trouble to upper stack?

I think it just needs to read src migration_version under src dev node, and test it in target migration version under target dev node. after all, through this interface we just help the upper layer knowing available options through reading and testing, and they decide to use it or not.

Can we simplify the requirement by allowing only mdev<->mdev and phys<->phys migration? If an customer does want to migrate between a mdev and phys, he could wrap physical device into a wrapped mdev instance (with the same type as the source mdev) instead of using vendor ops. Doing so does add some burden but if mdev<->phys is not dominant usage then such tradeoff might be worthywhile...

If the interfaces for phys<->phys and mdev<->mdev are consistent, it makes no difference to phys<->mdev, right? I think the vendor string for a mdev device is something like: "Parent PCIID + mdev type + software version", and that for a phys device is something like: "PCIID + software version". as long as we don't migrate between devices from different vendors, it's easy for vendor driver to tell if a phys device is migration compatible to a mdev device according it supports it or not. Thanks Yan

...

...

I agree that there was a gap in the previous proposal for non-mdev devices, but I think this bring a lot of questions that we need to puzzle through and libvirt will need to re-evaluate how they might decide to pick a migration target device. For example, I'm sure libvirt would reject any policy decisions regarding picking a physical device versus an mdev device. Had we previously left it that only a layer above libvirt would select a target device and libvirt only tests compatibility to that target device? I'm not sure if there's a layer above libvirt would select a target device. but if there is such a layer (even it's human), we need to provide an interface for them to know whether their decision is suitable for migration. The migration_version interface provides a potential to allow mdev->phys migration, even libvirt may currently reject it.

...

We also need to consider that this expands the namespace. If we no longer require matching types as the first level of comparison, then vendor migration strings can theoretically collide. How do we coordinate that can't happen? Thanks, yes, it's indeed a problem. could only allowing migration beteen devices from the same vendor be a good prerequisite?

Thanks Yan

...

...

...

...

> Is existence (and compatibility) of (1) a pre-req for possible > existence (and compatibility) of (2)? > no. (2) does not reply on (1).

Hm. Non-existence of (1) seems to imply "this type does not support migration". If an mdev created for such a type suddenly does support migration, it feels a bit odd.

yes. but I think if the condition happens, it should be reported a bug to vendor driver. should I add a line in the doc like "vendor driver should ensure that the migration compatibility from migration_version under mdev_type should

be

...

...

consistent with that from migration_version under device node" ?

...

(It obviously cannot be a prereq for what I called (3) above.)

...

> Does userspace need to check (1) or can it completely rely on (2), if > it so chooses? > I think it can completely reply on (2) if compatibility check before mdev creation is not required.

> If devices with a different mdev type are indeed compatible, it

seems

...

> userspace can only find out after the devices have actually been > created, as (1) does not apply? yes, I think so.

How useful would it be for userspace to even look at (1) in that case? It only knows if things have a chance of working if it actually goes ahead and creates devices.

hmm, is it useful for userspace to test the migration_version under mdev type before it knows what mdev device to generate ? like when the userspace wants to migrate an mdev device in src vm, but it has not created target vm and the target mdev device.

...

...

> One of my worries is that the existence of an attribute with the

same

...

> name in two similar locations might lead to confusion. But maybe it > isn't a problem. > Yes, I have the same feeling. but as (2) is for sysfs interface consistency, to make it transparent to userspace tools like libvirt, I guess the same name is necessary?

What do we actually need here, I wonder? (1) and (2) seem to serve slightly different purposes, while (2) and what I called (3) have the same purpose. Is it important to userspace that (1) and (2) have the same name? so change (1) to migration_type_version and (2) to migration_instance_version? But as they are under different locations, could that location imply enough information?

Thanks Yan

_______________________________________________ intel-gvt-dev mailing list intel-gvt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

On Sat, Apr 25, 2020 at 03:10:49AM +0800, Dr. David Alan Gilbert wrote:

* Yan Zhao (yan.y.zhao@intel.com) wrote:

...

...

...

From: Yan Zhao Sent: Tuesday, April 21, 2020 10:37 AM

...

On Sun, 19 Apr 2020 21:24:57 -0400 Yan Zhao <yan.y.zhao@intel.com> wrote:

...

On Fri, Apr 17, 2020 at 07:24:57PM +0800, Cornelia Huck wrote: > On Fri, 17 Apr 2020 05:52:02 -0400 > Yan Zhao <yan.y.zhao@intel.com> wrote: > > > On Fri, Apr 17, 2020 at 04:44:50PM +0800, Cornelia Huck wrote: > > > On Mon, 13 Apr 2020 01:52:01 -0400 > > > Yan Zhao <yan.y.zhao@intel.com> wrote: > > > > > > > This patchset introduces a migration_version attribute under sysfs of VFIO > > > > Mediated devices. > > > > > > > > This migration_version attribute is used to check migration compatibility > > > > between two mdev devices. > > > > > > > > Currently, it has two locations: > > > > (1) under mdev_type node, > > > > which can be used even before device creation, but only for mdev > > > > devices of the same mdev type. > > > > (2) under mdev device node, > > > > which can only be used after the mdev devices are created, but

On Tue, Apr 21, 2020 at 06:56:00AM +0800, Alex Williamson wrote: the src

...

...

> > > > and target mdev devices are not necessarily be of the same mdev type > > > > (The second location is newly added in v5, in order to keep consistent > > > > with the migration_version node for migratable pass-though devices) > > > > > > What is the relationship between those two attributes? > > > > > (1) is for mdev devices specifically, and (2) is provided to keep the same > > sysfs interface as with non-mdev cases. so (2) is for both mdev devices and > > non-mdev devices. > > > > in future, if we enable vfio-pci vendor ops, (i.e. a non-mdev device > > is binding to vfio-pci, but is able to register migration region and do > > migration transactions from a vendor provided affiliate driver), > > the vendor driver would export (2) directly, under device node. > > It is not able to provide (1) as there're no mdev devices involved. > > Ok, creating an alternate attribute for non-mdev devices makes sense. > However, wouldn't that rather be a case (3)? The change here only > refers to mdev devices. > as you pointed below, (3) and (2) serve the same purpose. and I think a possible usage is to migrate between a non-mdev device and an mdev device. so I think it's better for them both to use (2) rather than creating (3).

An mdev type is meant to define a software compatible interface, so in the case of mdev->mdev migration, doesn't migrating to a different type fail the most basic of compatibility tests that we expect userspace to perform? IOW, if two mdev types are migration compatible, it seems a prerequisite to that is that they provide the same software interface, which means they should be the same mdev type.

In the hybrid cases of mdev->phys or phys->mdev, how does a management tool begin to even guess what might be compatible? Are we expecting libvirt to probe ever device with this attribute in the system? Is there going to be a new class hierarchy created to enumerate all possible migrate-able devices?

yes, management tool needs to guess and test migration compatible between two devices. But I think it's not the problem only for mdev->phys or phys->mdev. even for mdev->mdev, management tool needs to first assume that the two mdevs have the same type of parent devices (e.g.their pciids are equal). otherwise, it's still enumerating possibilities.

on the other hand, for two mdevs, mdev1 from pdev1, its mdev_type is 1/2 of pdev1; mdev2 from pdev2, its mdev_type is 1/4 of pdev2; if pdev2 is exactly 2 times of pdev1, why not allow migration between mdev1 <-> mdev2.

How could the manage tool figure out that 1/2 of pdev1 is equivalent to 1/4 of pdev2? If we really want to allow such thing happen, the best choice is to report the same mdev type on both pdev1 and pdev2. I think that's exactly the value of this migration_version interface.

On Tue, Apr 21, 2020 at 08:08:49PM +0800, Tian, Kevin wrote: the management tool can take advantage of this interface to know if two devices are migration compatible, no matter they are mdevs, non-mdevs, or mix.

as I know, (please correct me if not right), current libvirt still requires manually generating mdev devices, and it just duplicates src vm configuration to the target vm. for libvirt, currently it's always phys->phys and mdev->mdev (and of the same mdev type). But it does not justify that hybrid cases should not be allowed. otherwise, why do we need to introduce this migration_version interface and leave the judgement of migration compatibility to vendor driver? why not simply set the criteria to something like "pciids of parent devices are equal, and mdev types are equal" ?

...

btw mdev<->phys just brings trouble to upper stack as Alex pointed out. could you help me understand why it will bring trouble to upper stack?

I think it just needs to read src migration_version under src dev node, and test it in target migration version under target dev node.

after all, through this interface we just help the upper layer knowing available options through reading and testing, and they decide to use it or not.

...

Can we simplify the requirement by allowing only mdev<->mdev and phys<->phys migration? If an customer does want to migrate between a mdev and phys, he could wrap physical device into a wrapped mdev instance (with the same type as the source mdev) instead of using vendor ops. Doing so does add some burden but if mdev<->phys is not dominant usage then such tradeoff might be worthywhile...

If the interfaces for phys<->phys and mdev<->mdev are consistent, it makes no difference to phys<->mdev, right? I think the vendor string for a mdev device is something like: "Parent PCIID + mdev type + software version", and that for a phys device is something like: "PCIID + software version". as long as we don't migrate between devices from different vendors, it's easy for vendor driver to tell if a phys device is migration compatible to a mdev device according it supports it or not.

It surprises me that the PCIID matching is a requirement; I'd assumed with this clever mdev name setup that you could migrate between two different models in a series, or to a newer model, as long as they both supported the same mdev view.

hi Dave the migration_version string is transparent to userspace, and is completely defined by vendor driver. I put it there just as an example of how vendor driver may implement it. e.g. the src migration_version string is "src PCIID + src software version", then when this string is write to target migration_version node, the vendor driver in the target device will compare it with its own device info and software version. If different models are allowed, the write just succeeds even PCIIDs in src and target are different. so, it is the vendor driver to define whether two devices are able to migrate, no matter their PCIIDs, mdev types, software versions..., which provides vendor driver full flexibility. do you think it's good? Thanks Yan

...

Thanks Yan

...

...

...

I agree that there was a gap in the previous proposal for non-mdev devices, but I think this bring a lot of questions that we need to puzzle through and libvirt will need to re-evaluate how they might decide to pick a migration target device. For example, I'm sure libvirt would reject any policy decisions regarding picking a physical device versus an mdev device. Had we previously left it that only a layer above libvirt would select a target device and libvirt only tests compatibility to that target device? I'm not sure if there's a layer above libvirt would select a target device. but if there is such a layer (even it's human), we need to provide an interface for them to know whether their decision is suitable for migration. The migration_version interface provides a potential to allow mdev->phys migration, even libvirt may currently reject it.

...

We also need to consider that this expands the namespace. If we no longer require matching types as the first level of comparison, then vendor migration strings can theoretically collide. How do we coordinate that can't happen? Thanks, yes, it's indeed a problem. could only allowing migration beteen devices from the same vendor be a good prerequisite?

Thanks Yan

...

...

> > > Is existence (and compatibility) of (1) a pre-req for possible > > > existence (and compatibility) of (2)? > > > > > no. (2) does not reply on (1). > > Hm. Non-existence of (1) seems to imply "this type does not support > migration". If an mdev created for such a type suddenly does support > migration, it feels a bit odd. > yes. but I think if the condition happens, it should be reported a bug to vendor driver. should I add a line in the doc like "vendor driver should ensure that the migration compatibility from migration_version under mdev_type should

be

...

...

consistent with that from migration_version under device node" ?

> (It obviously cannot be a prereq for what I called (3) above.) > > > > > > Does userspace need to check (1) or can it completely rely on (2), if > > > it so chooses? > > > > > I think it can completely reply on (2) if compatibility check before > > mdev creation is not required. > > > > > If devices with a different mdev type are indeed compatible, it seems > > > userspace can only find out after the devices have actually been > > > created, as (1) does not apply? > > yes, I think so. > > How useful would it be for userspace to even look at (1) in that case? > It only knows if things have a chance of working if it actually goes > ahead and creates devices. > hmm, is it useful for userspace to test the migration_version under mdev type before it knows what mdev device to generate ? like when the userspace wants to migrate an mdev device in src vm, but it has not created target vm and the target mdev device.

> > > > > One of my worries is that the existence of an attribute with the same > > > name in two similar locations might lead to confusion. But maybe it > > > isn't a problem. > > > > > Yes, I have the same feeling. but as (2) is for sysfs interface > > consistency, to make it transparent to userspace tools like libvirt, > > I guess the same name is necessary? > > What do we actually need here, I wonder? (1) and (2) seem to serve > slightly different purposes, while (2) and what I called (3) have the > same purpose. Is it important to userspace that (1) and (2) have the > same name? so change (1) to migration_type_version and (2) to migration_instance_version? But as they are under different locations, could that location imply enough information?

Thanks Yan

_______________________________________________ intel-gvt-dev mailing list intel-gvt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

-- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

On Mon, Apr 27, 2020 at 11:37:43PM +0800, Dr. David Alan Gilbert wrote:

* Yan Zhao (yan.y.zhao@intel.com) wrote:

...

On Sat, Apr 25, 2020 at 03:10:49AM +0800, Dr. David Alan Gilbert wrote:

...

* Yan Zhao (yan.y.zhao@intel.com) wrote:

...

...

...

From: Yan Zhao Sent: Tuesday, April 21, 2020 10:37 AM

On Tue, Apr 21, 2020 at 06:56:00AM +0800, Alex Williamson wrote: > On Sun, 19 Apr 2020 21:24:57 -0400 > Yan Zhao <yan.y.zhao@intel.com> wrote: > > > On Fri, Apr 17, 2020 at 07:24:57PM +0800, Cornelia Huck wrote: > > > On Fri, 17 Apr 2020 05:52:02 -0400 > > > Yan Zhao <yan.y.zhao@intel.com> wrote: > > > > > > > On Fri, Apr 17, 2020 at 04:44:50PM +0800, Cornelia Huck wrote: > > > > > On Mon, 13 Apr 2020 01:52:01 -0400 > > > > > Yan Zhao <yan.y.zhao@intel.com> wrote: > > > > > > > > > > > This patchset introduces a migration_version attribute under sysfs of VFIO > > > > > > Mediated devices. > > > > > > > > > > > > This migration_version attribute is used to check migration compatibility > > > > > > between two mdev devices. > > > > > > > > > > > > Currently, it has two locations: > > > > > > (1) under mdev_type node, > > > > > > which can be used even before device creation, but only for mdev > > > > > > devices of the same mdev type. > > > > > > (2) under mdev device node, > > > > > > which can only be used after the mdev devices are created, but the src > > > > > > and target mdev devices are not necessarily be of the same mdev type > > > > > > (The second location is newly added in v5, in order to keep consistent > > > > > > with the migration_version node for migratable pass-though devices) > > > > > > > > > > What is the relationship between those two attributes? > > > > > > > > > (1) is for mdev devices specifically, and (2) is provided to keep the same > > > > sysfs interface as with non-mdev cases. so (2) is for both mdev devices and > > > > non-mdev devices. > > > > > > > > in future, if we enable vfio-pci vendor ops, (i.e. a non-mdev device > > > > is binding to vfio-pci, but is able to register migration region and do > > > > migration transactions from a vendor provided affiliate driver), > > > > the vendor driver would export (2) directly, under device node. > > > > It is not able to provide (1) as there're no mdev devices involved. > > > > > > Ok, creating an alternate attribute for non-mdev devices makes sense. > > > However, wouldn't that rather be a case (3)? The change here only > > > refers to mdev devices. > > > > > as you pointed below, (3) and (2) serve the same purpose. > > and I think a possible usage is to migrate between a non-mdev device and > > an mdev device. so I think it's better for them both to use (2) rather > > than creating (3). > > An mdev type is meant to define a software compatible interface, so in > the case of mdev->mdev migration, doesn't migrating to a different type > fail the most basic of compatibility tests that we expect userspace to > perform? IOW, if two mdev types are migration compatible, it seems a > prerequisite to that is that they provide the same software interface, > which means they should be the same mdev type. > > In the hybrid cases of mdev->phys or phys->mdev, how does a management > tool begin to even guess what might be compatible? Are we expecting > libvirt to probe ever device with this attribute in the system? Is > there going to be a new class hierarchy created to enumerate all > possible migrate-able devices? > yes, management tool needs to guess and test migration compatible between two devices. But I think it's not the problem only for mdev->phys or phys->mdev. even for mdev->mdev, management tool needs to first assume that the two mdevs have the same type of parent devices (e.g.their pciids are equal). otherwise, it's still enumerating possibilities.

on the other hand, for two mdevs, mdev1 from pdev1, its mdev_type is 1/2 of pdev1; mdev2 from pdev2, its mdev_type is 1/4 of pdev2; if pdev2 is exactly 2 times of pdev1, why not allow migration between mdev1 <-> mdev2.

How could the manage tool figure out that 1/2 of pdev1 is equivalent to 1/4 of pdev2? If we really want to allow such thing happen, the best choice is to report the same mdev type on both pdev1 and pdev2. I think that's exactly the value of this migration_version interface.

On Tue, Apr 21, 2020 at 08:08:49PM +0800, Tian, Kevin wrote: the management tool can take advantage of this interface to know if two devices are migration compatible, no matter they are mdevs, non-mdevs, or mix.

as I know, (please correct me if not right), current libvirt still requires manually generating mdev devices, and it just duplicates src vm configuration to the target vm. for libvirt, currently it's always phys->phys and mdev->mdev (and of the same mdev type). But it does not justify that hybrid cases should not be allowed. otherwise, why do we need to introduce this migration_version interface and leave the judgement of migration compatibility to vendor driver? why not simply set the criteria to something like "pciids of parent devices are equal, and mdev types are equal" ?

...

btw mdev<->phys just brings trouble to upper stack as Alex pointed out. could you help me understand why it will bring trouble to upper stack?

I think it just needs to read src migration_version under src dev node, and test it in target migration version under target dev node.

after all, through this interface we just help the upper layer knowing available options through reading and testing, and they decide to use it or not.

...

Can we simplify the requirement by allowing only mdev<->mdev and phys<->phys migration? If an customer does want to migrate between a mdev and phys, he could wrap physical device into a wrapped mdev instance (with the same type as the source mdev) instead of using vendor ops. Doing so does add some burden but if mdev<->phys is not dominant usage then such tradeoff might be worthywhile...

If the interfaces for phys<->phys and mdev<->mdev are consistent, it makes no difference to phys<->mdev, right? I think the vendor string for a mdev device is something like: "Parent PCIID + mdev type + software version", and that for a phys device is something like: "PCIID + software version". as long as we don't migrate between devices from different vendors, it's easy for vendor driver to tell if a phys device is migration compatible to a mdev device according it supports it or not.

It surprises me that the PCIID matching is a requirement; I'd assumed with this clever mdev name setup that you could migrate between two different models in a series, or to a newer model, as long as they both supported the same mdev view.

hi Dave the migration_version string is transparent to userspace, and is completely defined by vendor driver. I put it there just as an example of how vendor driver may implement it. e.g. the src migration_version string is "src PCIID + src software version", then when this string is write to target migration_version node, the vendor driver in the target device will compare it with its own device info and software version. If different models are allowed, the write just succeeds even PCIIDs in src and target are different.

so, it is the vendor driver to define whether two devices are able to migrate, no matter their PCIIDs, mdev types, software versions..., which provides vendor driver full flexibility.

do you think it's good?

Yeh that's OK; I guess it's going to need to have a big table in their with all the PCIIDs in. The alternative would be to abstract it a little; e.g. to say it's an Intel-gpu-core-v4 and then it would be less worried about the exact clock speed etc - but yes you might be right htat PCIIDs might be best for checking for quirks.

glad that you are agreed with it:) I think the vendor driver still can choose a way to abstract a little (e.g. Intel-gpu-core-v4...) if they think it's better. In that case, the migration_string would be something like "Intel-gpu-core-v4 + instance number + software version". IOW, they can choose anything they think appropriate to identify migration compatibility of a device. But Alex is right, we have to prevent namespace overlapping. So I think we need to ensure src and target devices are from the same vendors. or, any other ideas? Thanks Yan

...

...

...

...

...

> I agree that there was a gap in the previous proposal for non-mdev > devices, but I think this bring a lot of questions that we need to > puzzle through and libvirt will need to re-evaluate how they might > decide to pick a migration target device. For example, I'm sure > libvirt would reject any policy decisions regarding picking a physical > device versus an mdev device. Had we previously left it that only a > layer above libvirt would select a target device and libvirt only tests > compatibility to that target device? I'm not sure if there's a layer above libvirt would select a target device. but if there is such a layer (even it's human), we need to provide an interface for them to know whether their decision is suitable for migration. The migration_version interface provides a potential to allow mdev->phys migration, even libvirt may currently reject it.

> We also need to consider that this expands the namespace. If we no > longer require matching types as the first level of comparison, then > vendor migration strings can theoretically collide. How do we > coordinate that can't happen? Thanks, yes, it's indeed a problem. could only allowing migration beteen devices from the same vendor be a good prerequisite?

Thanks Yan > > > > > > Is existence (and compatibility) of (1) a pre-req for possible > > > > > existence (and compatibility) of (2)? > > > > > > > > > no. (2) does not reply on (1). > > > > > > Hm. Non-existence of (1) seems to imply "this type does not support > > > migration". If an mdev created for such a type suddenly does support > > > migration, it feels a bit odd. > > > > > yes. but I think if the condition happens, it should be reported a bug > > to vendor driver. > > should I add a line in the doc like "vendor driver should ensure that the > > migration compatibility from migration_version under mdev_type should be > > consistent with that from migration_version under device node" ? > > > > > (It obviously cannot be a prereq for what I called (3) above.) > > > > > > > > > > > > Does userspace need to check (1) or can it completely rely on (2), if > > > > > it so chooses? > > > > > > > > > I think it can completely reply on (2) if compatibility check before > > > > mdev creation is not required. > > > > > > > > > If devices with a different mdev type are indeed compatible, it seems > > > > > userspace can only find out after the devices have actually been > > > > > created, as (1) does not apply? > > > > yes, I think so. > > > > > > How useful would it be for userspace to even look at (1) in that case? > > > It only knows if things have a chance of working if it actually goes > > > ahead and creates devices. > > > > > hmm, is it useful for userspace to test the migration_version under mdev > > type before it knows what mdev device to generate ? > > like when the userspace wants to migrate an mdev device in src vm, > > but it has not created target vm and the target mdev device. > > > > > > > > > > > One of my worries is that the existence of an attribute with the same > > > > > name in two similar locations might lead to confusion. But maybe it > > > > > isn't a problem. > > > > > > > > > Yes, I have the same feeling. but as (2) is for sysfs interface > > > > consistency, to make it transparent to userspace tools like libvirt, > > > > I guess the same name is necessary? > > > > > > What do we actually need here, I wonder? (1) and (2) seem to serve > > > slightly different purposes, while (2) and what I called (3) have the > > > same purpose. Is it important to userspace that (1) and (2) have the > > > same name? > > so change (1) to migration_type_version and (2) to > > migration_instance_version? > > But as they are under different locations, could that location imply > > enough information? > > > > > > Thanks > > Yan > > > > > _______________________________________________ intel-gvt-dev mailing list intel-gvt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

-- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

-- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

On Tue, Apr 28, 2020 at 10:14:37PM +0800, Dr. David Alan Gilbert wrote:

* Yan Zhao (yan.y.zhao@intel.com) wrote:

...

On Mon, Apr 27, 2020 at 11:37:43PM +0800, Dr. David Alan Gilbert wrote:

...

* Yan Zhao (yan.y.zhao@intel.com) wrote:

...

On Sat, Apr 25, 2020 at 03:10:49AM +0800, Dr. David Alan Gilbert wrote:

...

* Yan Zhao (yan.y.zhao@intel.com) wrote:

...

On Tue, Apr 21, 2020 at 08:08:49PM +0800, Tian, Kevin wrote: > > From: Yan Zhao > > Sent: Tuesday, April 21, 2020 10:37 AM > > > > On Tue, Apr 21, 2020 at 06:56:00AM +0800, Alex Williamson wrote: > > > On Sun, 19 Apr 2020 21:24:57 -0400 > > > Yan Zhao <yan.y.zhao@intel.com> wrote: > > > > > > > On Fri, Apr 17, 2020 at 07:24:57PM +0800, Cornelia Huck wrote: > > > > > On Fri, 17 Apr 2020 05:52:02 -0400 > > > > > Yan Zhao <yan.y.zhao@intel.com> wrote: > > > > > > > > > > > On Fri, Apr 17, 2020 at 04:44:50PM +0800, Cornelia Huck wrote: > > > > > > > On Mon, 13 Apr 2020 01:52:01 -0400 > > > > > > > Yan Zhao <yan.y.zhao@intel.com> wrote: > > > > > > > > > > > > > > > This patchset introduces a migration_version attribute under sysfs > > of VFIO > > > > > > > > Mediated devices. > > > > > > > > > > > > > > > > This migration_version attribute is used to check migration > > compatibility > > > > > > > > between two mdev devices. > > > > > > > > > > > > > > > > Currently, it has two locations: > > > > > > > > (1) under mdev_type node, > > > > > > > > which can be used even before device creation, but only for > > mdev > > > > > > > > devices of the same mdev type. > > > > > > > > (2) under mdev device node, > > > > > > > > which can only be used after the mdev devices are created, but > > the src > > > > > > > > and target mdev devices are not necessarily be of the same > > mdev type > > > > > > > > (The second location is newly added in v5, in order to keep > > consistent > > > > > > > > with the migration_version node for migratable pass-though > > devices) > > > > > > > > > > > > > > What is the relationship between those two attributes? > > > > > > > > > > > > > (1) is for mdev devices specifically, and (2) is provided to keep the > > same > > > > > > sysfs interface as with non-mdev cases. so (2) is for both mdev > > devices and > > > > > > non-mdev devices. > > > > > > > > > > > > in future, if we enable vfio-pci vendor ops, (i.e. a non-mdev device > > > > > > is binding to vfio-pci, but is able to register migration region and do > > > > > > migration transactions from a vendor provided affiliate driver), > > > > > > the vendor driver would export (2) directly, under device node. > > > > > > It is not able to provide (1) as there're no mdev devices involved. > > > > > > > > > > Ok, creating an alternate attribute for non-mdev devices makes sense. > > > > > However, wouldn't that rather be a case (3)? The change here only > > > > > refers to mdev devices. > > > > > > > > > as you pointed below, (3) and (2) serve the same purpose. > > > > and I think a possible usage is to migrate between a non-mdev device and > > > > an mdev device. so I think it's better for them both to use (2) rather > > > > than creating (3). > > > > > > An mdev type is meant to define a software compatible interface, so in > > > the case of mdev->mdev migration, doesn't migrating to a different type > > > fail the most basic of compatibility tests that we expect userspace to > > > perform? IOW, if two mdev types are migration compatible, it seems a > > > prerequisite to that is that they provide the same software interface, > > > which means they should be the same mdev type. > > > > > > In the hybrid cases of mdev->phys or phys->mdev, how does a > > management > > > tool begin to even guess what might be compatible? Are we expecting > > > libvirt to probe ever device with this attribute in the system? Is > > > there going to be a new class hierarchy created to enumerate all > > > possible migrate-able devices? > > > > > yes, management tool needs to guess and test migration compatible > > between two devices. But I think it's not the problem only for > > mdev->phys or phys->mdev. even for mdev->mdev, management tool needs > > to > > first assume that the two mdevs have the same type of parent devices > > (e.g.their pciids are equal). otherwise, it's still enumerating > > possibilities. > > > > on the other hand, for two mdevs, > > mdev1 from pdev1, its mdev_type is 1/2 of pdev1; > > mdev2 from pdev2, its mdev_type is 1/4 of pdev2; > > if pdev2 is exactly 2 times of pdev1, why not allow migration between > > mdev1 <-> mdev2. > > How could the manage tool figure out that 1/2 of pdev1 is equivalent > to 1/4 of pdev2? If we really want to allow such thing happen, the best > choice is to report the same mdev type on both pdev1 and pdev2. I think that's exactly the value of this migration_version interface. the management tool can take advantage of this interface to know if two devices are migration compatible, no matter they are mdevs, non-mdevs, or mix.

as I know, (please correct me if not right), current libvirt still requires manually generating mdev devices, and it just duplicates src vm configuration to the target vm. for libvirt, currently it's always phys->phys and mdev->mdev (and of the same mdev type). But it does not justify that hybrid cases should not be allowed. otherwise, why do we need to introduce this migration_version interface and leave the judgement of migration compatibility to vendor driver? why not simply set the criteria to something like "pciids of parent devices are equal, and mdev types are equal" ?

> btw mdev<->phys just brings trouble to upper stack as Alex pointed out. could you help me understand why it will bring trouble to upper stack?

I think it just needs to read src migration_version under src dev node, and test it in target migration version under target dev node.

after all, through this interface we just help the upper layer knowing available options through reading and testing, and they decide to use it or not.

> Can we simplify the requirement by allowing only mdev<->mdev and > phys<->phys migration? If an customer does want to migrate between a > mdev and phys, he could wrap physical device into a wrapped mdev > instance (with the same type as the source mdev) instead of using vendor > ops. Doing so does add some burden but if mdev<->phys is not dominant > usage then such tradeoff might be worthywhile... > If the interfaces for phys<->phys and mdev<->mdev are consistent, it makes no difference to phys<->mdev, right? I think the vendor string for a mdev device is something like: "Parent PCIID + mdev type + software version", and that for a phys device is something like: "PCIID + software version". as long as we don't migrate between devices from different vendors, it's easy for vendor driver to tell if a phys device is migration compatible to a mdev device according it supports it or not.

It surprises me that the PCIID matching is a requirement; I'd assumed with this clever mdev name setup that you could migrate between two different models in a series, or to a newer model, as long as they both supported the same mdev view.

hi Dave the migration_version string is transparent to userspace, and is completely defined by vendor driver. I put it there just as an example of how vendor driver may implement it. e.g. the src migration_version string is "src PCIID + src software version", then when this string is write to target migration_version node, the vendor driver in the target device will compare it with its own device info and software version. If different models are allowed, the write just succeeds even PCIIDs in src and target are different.

so, it is the vendor driver to define whether two devices are able to migrate, no matter their PCIIDs, mdev types, software versions..., which provides vendor driver full flexibility.

do you think it's good?

Yeh that's OK; I guess it's going to need to have a big table in their with all the PCIIDs in. The alternative would be to abstract it a little; e.g. to say it's an Intel-gpu-core-v4 and then it would be less worried about the exact clock speed etc - but yes you might be right htat PCIIDs might be best for checking for quirks.

glad that you are agreed with it:) I think the vendor driver still can choose a way to abstract a little (e.g. Intel-gpu-core-v4...) if they think it's better. In that case, the migration_string would be something like "Intel-gpu-core-v4 + instance number + software version". IOW, they can choose anything they think appropriate to identify migration compatibility of a device. But Alex is right, we have to prevent namespace overlapping. So I think we need to ensure src and target devices are from the same vendors. or, any other ideas?

That's why I kept the 'Intel' in that example; or PCI vendor ID; I was Yes, it's a good idea! could we add a line in the doc saying that it is the vendor driver to add a unique string to avoid namespace

collision?

only really trying to say that within one vendors range there are often a lot of PCI-IDs that have really minor variations. Yes. I also prefer to include PCI-IDs. BTW, sometimes even the same PCI-ID does not guarantee two devices are of no difference or are migration compatible. for example, two local NVMe devices may have the same PCI-ID but are configured to two different remote NVMe devices. the vendor driver needs to add extra info besides PCI-IDs then.

Thanks Yan

...

...

...

...

...

> > > > > > > I agree that there was a gap in the previous proposal for non-mdev > > > devices, but I think this bring a lot of questions that we need to > > > puzzle through and libvirt will need to re-evaluate how they might > > > decide to pick a migration target device. For example, I'm sure > > > libvirt would reject any policy decisions regarding picking a physical > > > device versus an mdev device. Had we previously left it that only a > > > layer above libvirt would select a target device and libvirt only tests > > > compatibility to that target device? > > I'm not sure if there's a layer above libvirt would select a target > > device. but if there is such a layer (even it's human), we need to > > provide an interface for them to know whether their decision is suitable > > for migration. The migration_version interface provides a potential to > > allow mdev->phys migration, even libvirt may currently reject it. > > > > > > > We also need to consider that this expands the namespace. If we no > > > longer require matching types as the first level of comparison, then > > > vendor migration strings can theoretically collide. How do we > > > coordinate that can't happen? Thanks, > > yes, it's indeed a problem. > > could only allowing migration beteen devices from the same vendor be a > > good > > prerequisite? > > > > Thanks > > Yan > > > > > > > > > > Is existence (and compatibility) of (1) a pre-req for possible > > > > > > > existence (and compatibility) of (2)? > > > > > > > > > > > > > no. (2) does not reply on (1). > > > > > > > > > > Hm. Non-existence of (1) seems to imply "this type does not support > > > > > migration". If an mdev created for such a type suddenly does support > > > > > migration, it feels a bit odd. > > > > > > > > > yes. but I think if the condition happens, it should be reported a bug > > > > to vendor driver. > > > > should I add a line in the doc like "vendor driver should ensure that the > > > > migration compatibility from migration_version under mdev_type should > > be > > > > consistent with that from migration_version under device node" ? > > > > > > > > > (It obviously cannot be a prereq for what I called (3) above.) > > > > > > > > > > > > > > > > > > Does userspace need to check (1) or can it completely rely on (2), if > > > > > > > it so chooses? > > > > > > > > > > > > > I think it can completely reply on (2) if compatibility check before > > > > > > mdev creation is not required. > > > > > > > > > > > > > If devices with a different mdev type are indeed compatible, it > > seems > > > > > > > userspace can only find out after the devices have actually been > > > > > > > created, as (1) does not apply? > > > > > > yes, I think so. > > > > > > > > > > How useful would it be for userspace to even look at (1) in that case? > > > > > It only knows if things have a chance of working if it actually goes > > > > > ahead and creates devices. > > > > > > > > > hmm, is it useful for userspace to test the migration_version under mdev > > > > type before it knows what mdev device to generate ? > > > > like when the userspace wants to migrate an mdev device in src vm, > > > > but it has not created target vm and the target mdev device. > > > > > > > > > > > > > > > > > One of my worries is that the existence of an attribute with the > > same > > > > > > > name in two similar locations might lead to confusion. But maybe it > > > > > > > isn't a problem. > > > > > > > > > > > > > Yes, I have the same feeling. but as (2) is for sysfs interface > > > > > > consistency, to make it transparent to userspace tools like libvirt, > > > > > > I guess the same name is necessary? > > > > > > > > > > What do we actually need here, I wonder? (1) and (2) seem to serve > > > > > slightly different purposes, while (2) and what I called (3) have the > > > > > same purpose. Is it important to userspace that (1) and (2) have the > > > > > same name? > > > > so change (1) to migration_type_version and (2) to > > > > migration_instance_version? > > > > But as they are under different locations, could that location imply > > > > enough information? > > > > > > > > > > > > Thanks > > > > Yan > > > > > > > > > > > > > _______________________________________________ > > intel-gvt-dev mailing list > > intel-gvt-dev@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

-- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

-- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

-- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

On Wed, Apr 29, 2020 at 04:22:01PM +0800, Dr. David Alan Gilbert wrote:

* Yan Zhao (yan.y.zhao@intel.com) wrote:

...

On Tue, Apr 28, 2020 at 10:14:37PM +0800, Dr. David Alan Gilbert wrote:

...

* Yan Zhao (yan.y.zhao@intel.com) wrote:

...

On Mon, Apr 27, 2020 at 11:37:43PM +0800, Dr. David Alan Gilbert wrote:

...

* Yan Zhao (yan.y.zhao@intel.com) wrote:

...

On Sat, Apr 25, 2020 at 03:10:49AM +0800, Dr. David Alan Gilbert wrote: > * Yan Zhao (yan.y.zhao@intel.com) wrote: > > On Tue, Apr 21, 2020 at 08:08:49PM +0800, Tian, Kevin wrote: > > > > From: Yan Zhao > > > > Sent: Tuesday, April 21, 2020 10:37 AM > > > > > > > > On Tue, Apr 21, 2020 at 06:56:00AM +0800, Alex Williamson wrote: > > > > > On Sun, 19 Apr 2020 21:24:57 -0400 > > > > > Yan Zhao <yan.y.zhao@intel.com> wrote: > > > > > > > > > > > On Fri, Apr 17, 2020 at 07:24:57PM +0800, Cornelia Huck wrote: > > > > > > > On Fri, 17 Apr 2020 05:52:02 -0400 > > > > > > > Yan Zhao <yan.y.zhao@intel.com> wrote: > > > > > > > > > > > > > > > On Fri, Apr 17, 2020 at 04:44:50PM +0800, Cornelia Huck wrote: > > > > > > > > > On Mon, 13 Apr 2020 01:52:01 -0400 > > > > > > > > > Yan Zhao <yan.y.zhao@intel.com> wrote: > > > > > > > > > > > > > > > > > > > This patchset introduces a migration_version attribute under sysfs > > > > of VFIO > > > > > > > > > > Mediated devices. > > > > > > > > > > > > > > > > > > > > This migration_version attribute is used to check migration > > > > compatibility > > > > > > > > > > between two mdev devices. > > > > > > > > > > > > > > > > > > > > Currently, it has two locations: > > > > > > > > > > (1) under mdev_type node, > > > > > > > > > > which can be used even before device creation, but only for > > > > mdev > > > > > > > > > > devices of the same mdev type. > > > > > > > > > > (2) under mdev device node, > > > > > > > > > > which can only be used after the mdev devices are created, but > > > > the src > > > > > > > > > > and target mdev devices are not necessarily be of the same > > > > mdev type > > > > > > > > > > (The second location is newly added in v5, in order to keep > > > > consistent > > > > > > > > > > with the migration_version node for migratable pass-though > > > > devices) > > > > > > > > > > > > > > > > > > What is the relationship between those two attributes? > > > > > > > > > > > > > > > > > (1) is for mdev devices specifically, and (2) is provided to keep the > > > > same > > > > > > > > sysfs interface as with non-mdev cases. so (2) is for both mdev > > > > devices and > > > > > > > > non-mdev devices. > > > > > > > > > > > > > > > > in future, if we enable vfio-pci vendor ops, (i.e. a non-mdev device > > > > > > > > is binding to vfio-pci, but is able to register migration region and do > > > > > > > > migration transactions from a vendor provided affiliate driver), > > > > > > > > the vendor driver would export (2) directly, under device node. > > > > > > > > It is not able to provide (1) as there're no mdev devices involved. > > > > > > > > > > > > > > Ok, creating an alternate attribute for non-mdev devices makes sense. > > > > > > > However, wouldn't that rather be a case (3)? The change here only > > > > > > > refers to mdev devices. > > > > > > > > > > > > > as you pointed below, (3) and (2) serve the same purpose. > > > > > > and I think a possible usage is to migrate between a non-mdev device and > > > > > > an mdev device. so I think it's better for them both to use (2) rather > > > > > > than creating (3). > > > > > > > > > > An mdev type is meant to define a software compatible interface, so in > > > > > the case of mdev->mdev migration, doesn't migrating to a different type > > > > > fail the most basic of compatibility tests that we expect userspace to > > > > > perform? IOW, if two mdev types are migration compatible, it seems a > > > > > prerequisite to that is that they provide the same software interface, > > > > > which means they should be the same mdev type. > > > > > > > > > > In the hybrid cases of mdev->phys or phys->mdev, how does a > > > > management > > > > > tool begin to even guess what might be compatible? Are we expecting > > > > > libvirt to probe ever device with this attribute in the system? Is > > > > > there going to be a new class hierarchy created to enumerate all > > > > > possible migrate-able devices? > > > > > > > > > yes, management tool needs to guess and test migration compatible > > > > between two devices. But I think it's not the problem only for > > > > mdev->phys or phys->mdev. even for mdev->mdev, management tool needs > > > > to > > > > first assume that the two mdevs have the same type of parent devices > > > > (e.g.their pciids are equal). otherwise, it's still enumerating > > > > possibilities. > > > > > > > > on the other hand, for two mdevs, > > > > mdev1 from pdev1, its mdev_type is 1/2 of pdev1; > > > > mdev2 from pdev2, its mdev_type is 1/4 of pdev2; > > > > if pdev2 is exactly 2 times of pdev1, why not allow migration between > > > > mdev1 <-> mdev2. > > > > > > How could the manage tool figure out that 1/2 of pdev1 is equivalent > > > to 1/4 of pdev2? If we really want to allow such thing happen, the best > > > choice is to report the same mdev type on both pdev1 and pdev2. > > I think that's exactly the value of this migration_version interface. > > the management tool can take advantage of this interface to know if two > > devices are migration compatible, no matter they are mdevs, non-mdevs, > > or mix. > > > > as I know, (please correct me if not right), current libvirt still > > requires manually generating mdev devices, and it just duplicates src vm > > configuration to the target vm. > > for libvirt, currently it's always phys->phys and mdev->mdev (and of the > > same mdev type). > > But it does not justify that hybrid cases should not be allowed. otherwise, > > why do we need to introduce this migration_version interface and leave > > the judgement of migration compatibility to vendor driver? why not simply > > set the criteria to something like "pciids of parent devices are equal, > > and mdev types are equal" ? > > > > > > > btw mdev<->phys just brings trouble to upper stack as Alex pointed out. > > could you help me understand why it will bring trouble to upper stack? > > > > I think it just needs to read src migration_version under src dev node, > > and test it in target migration version under target dev node. > > > > after all, through this interface we just help the upper layer > > knowing available options through reading and testing, and they decide > > to use it or not. > > > > > Can we simplify the requirement by allowing only mdev<->mdev and > > > phys<->phys migration? If an customer does want to migrate between a > > > mdev and phys, he could wrap physical device into a wrapped mdev > > > instance (with the same type as the source mdev) instead of using vendor > > > ops. Doing so does add some burden but if mdev<->phys is not dominant > > > usage then such tradeoff might be worthywhile... > > > > > If the interfaces for phys<->phys and mdev<->mdev are consistent, it makes no > > difference to phys<->mdev, right? > > I think the vendor string for a mdev device is something like: > > "Parent PCIID + mdev type + software version", and > > that for a phys device is something like: > > "PCIID + software version". > > as long as we don't migrate between devices from different vendors, it's > > easy for vendor driver to tell if a phys device is migration compatible > > to a mdev device according it supports it or not. > > It surprises me that the PCIID matching is a requirement; I'd assumed > with this clever mdev name setup that you could migrate between two > different models in a series, or to a newer model, as long as they > both supported the same mdev view. > hi Dave the migration_version string is transparent to userspace, and is completely defined by vendor driver. I put it there just as an example of how vendor driver may implement it. e.g. the src migration_version string is "src PCIID + src software version", then when this string is write to target migration_version node, the vendor driver in the target device will compare it with its own device info and software version. If different models are allowed, the write just succeeds even PCIIDs in src and target are different.

so, it is the vendor driver to define whether two devices are able to migrate, no matter their PCIIDs, mdev types, software versions..., which provides vendor driver full flexibility.

do you think it's good?

Yeh that's OK; I guess it's going to need to have a big table in their with all the PCIIDs in. The alternative would be to abstract it a little; e.g. to say it's an Intel-gpu-core-v4 and then it would be less worried about the exact clock speed etc - but yes you might be right htat PCIIDs might be best for checking for quirks.

glad that you are agreed with it:) I think the vendor driver still can choose a way to abstract a little (e.g. Intel-gpu-core-v4...) if they think it's better. In that case, the migration_string would be something like "Intel-gpu-core-v4 + instance number + software version". IOW, they can choose anything they think appropriate to identify migration compatibility of a device. But Alex is right, we have to prevent namespace overlapping. So I think we need to ensure src and target devices are from the same vendors. or, any other ideas?

That's why I kept the 'Intel' in that example; or PCI vendor ID; I was Yes, it's a good idea! could we add a line in the doc saying that it is the vendor driver to add a unique string to avoid namespace

collision?

So why don't we split the difference; lets say that it should start with the hex PCI Vendor ID.

The problem is for mdev devices, if the parent devices are not PCI devices, they don't have PCI vendor IDs. Thanks Yan

...

...

only really trying to say that within one vendors range there are often a lot of PCI-IDs that have really minor variations. Yes. I also prefer to include PCI-IDs. BTW, sometimes even the same PCI-ID does not guarantee two devices are of no difference or are migration compatible. for example, two local NVMe devices may have the same PCI-ID but are configured to two different remote NVMe devices. the vendor driver needs to add extra info besides PCI-IDs then.

Ah, yes that's an interesting example.

Dave

...

...

...

...

...

> > > > > > > > > > > > > I agree that there was a gap in the previous proposal for non-mdev > > > > > devices, but I think this bring a lot of questions that we need to > > > > > puzzle through and libvirt will need to re-evaluate how they might > > > > > decide to pick a migration target device. For example, I'm sure > > > > > libvirt would reject any policy decisions regarding picking a physical > > > > > device versus an mdev device. Had we previously left it that only a > > > > > layer above libvirt would select a target device and libvirt only tests > > > > > compatibility to that target device? > > > > I'm not sure if there's a layer above libvirt would select a target > > > > device. but if there is such a layer (even it's human), we need to > > > > provide an interface for them to know whether their decision is suitable > > > > for migration. The migration_version interface provides a potential to > > > > allow mdev->phys migration, even libvirt may currently reject it. > > > > > > > > > > > > > We also need to consider that this expands the namespace. If we no > > > > > longer require matching types as the first level of comparison, then > > > > > vendor migration strings can theoretically collide. How do we > > > > > coordinate that can't happen? Thanks, > > > > yes, it's indeed a problem. > > > > could only allowing migration beteen devices from the same vendor be a > > > > good > > > > prerequisite? > > > > > > > > Thanks > > > > Yan > > > > > > > > > > > > > > Is existence (and compatibility) of (1) a pre-req for possible > > > > > > > > > existence (and compatibility) of (2)? > > > > > > > > > > > > > > > > > no. (2) does not reply on (1). > > > > > > > > > > > > > > Hm. Non-existence of (1) seems to imply "this type does not support > > > > > > > migration". If an mdev created for such a type suddenly does support > > > > > > > migration, it feels a bit odd. > > > > > > > > > > > > > yes. but I think if the condition happens, it should be reported a bug > > > > > > to vendor driver. > > > > > > should I add a line in the doc like "vendor driver should ensure that the > > > > > > migration compatibility from migration_version under mdev_type should > > > > be > > > > > > consistent with that from migration_version under device node" ? > > > > > > > > > > > > > (It obviously cannot be a prereq for what I called (3) above.) > > > > > > > > > > > > > > > > > > > > > > > > Does userspace need to check (1) or can it completely rely on (2), if > > > > > > > > > it so chooses? > > > > > > > > > > > > > > > > > I think it can completely reply on (2) if compatibility check before > > > > > > > > mdev creation is not required. > > > > > > > > > > > > > > > > > If devices with a different mdev type are indeed compatible, it > > > > seems > > > > > > > > > userspace can only find out after the devices have actually been > > > > > > > > > created, as (1) does not apply? > > > > > > > > yes, I think so. > > > > > > > > > > > > > > How useful would it be for userspace to even look at (1) in that case? > > > > > > > It only knows if things have a chance of working if it actually goes > > > > > > > ahead and creates devices. > > > > > > > > > > > > > hmm, is it useful for userspace to test the migration_version under mdev > > > > > > type before it knows what mdev device to generate ? > > > > > > like when the userspace wants to migrate an mdev device in src vm, > > > > > > but it has not created target vm and the target mdev device. > > > > > > > > > > > > > > > > > > > > > > > One of my worries is that the existence of an attribute with the > > > > same > > > > > > > > > name in two similar locations might lead to confusion. But maybe it > > > > > > > > > isn't a problem. > > > > > > > > > > > > > > > > > Yes, I have the same feeling. but as (2) is for sysfs interface > > > > > > > > consistency, to make it transparent to userspace tools like libvirt, > > > > > > > > I guess the same name is necessary? > > > > > > > > > > > > > > What do we actually need here, I wonder? (1) and (2) seem to serve > > > > > > > slightly different purposes, while (2) and what I called (3) have the > > > > > > > same purpose. Is it important to userspace that (1) and (2) have the > > > > > > > same name? > > > > > > so change (1) to migration_type_version and (2) to > > > > > > migration_instance_version? > > > > > > But as they are under different locations, could that location imply > > > > > > enough information? > > > > > > > > > > > > > > > > > > Thanks > > > > > > Yan > > > > > > > > > > > > > > > > > > > > > _______________________________________________ > > > > intel-gvt-dev mailing list > > > > intel-gvt-dev@lists.freedesktop.org > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev > > > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK >

-- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

-- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

-- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

On Wed, Apr 29, 2020 at 05:48:44PM +0800, Dr. David Alan Gilbert wrote: <snip>

...

...

...

...

...

> > > > > > > An mdev type is meant to define a software compatible interface, so in > > > > > > > the case of mdev->mdev migration, doesn't migrating to a different type > > > > > > > fail the most basic of compatibility tests that we expect userspace to > > > > > > > perform? IOW, if two mdev types are migration compatible, it seems a > > > > > > > prerequisite to that is that they provide the same software interface, > > > > > > > which means they should be the same mdev type. > > > > > > > > > > > > > > In the hybrid cases of mdev->phys or phys->mdev, how does a > > > > > > management > > > > > > > tool begin to even guess what might be compatible? Are we expecting > > > > > > > libvirt to probe ever device with this attribute in the system? Is > > > > > > > there going to be a new class hierarchy created to enumerate all > > > > > > > possible migrate-able devices? > > > > > > > > > > > > > yes, management tool needs to guess and test migration compatible > > > > > > between two devices. But I think it's not the problem only for > > > > > > mdev->phys or phys->mdev. even for mdev->mdev, management tool needs > > > > > > to > > > > > > first assume that the two mdevs have the same type of parent devices > > > > > > (e.g.their pciids are equal). otherwise, it's still enumerating > > > > > > possibilities. > > > > > > > > > > > > on the other hand, for two mdevs, > > > > > > mdev1 from pdev1, its mdev_type is 1/2 of pdev1; > > > > > > mdev2 from pdev2, its mdev_type is 1/4 of pdev2; > > > > > > if pdev2 is exactly 2 times of pdev1, why not allow migration between > > > > > > mdev1 <-> mdev2. > > > > > > > > > > How could the manage tool figure out that 1/2 of pdev1 is equivalent > > > > > to 1/4 of pdev2? If we really want to allow such thing happen, the best > > > > > choice is to report the same mdev type on both pdev1 and pdev2. > > > > I think that's exactly the value of this migration_version interface. > > > > the management tool can take advantage of this interface to know if two > > > > devices are migration compatible, no matter they are mdevs, non-mdevs, > > > > or mix. > > > > > > > > as I know, (please correct me if not right), current libvirt still > > > > requires manually generating mdev devices, and it just duplicates src vm > > > > configuration to the target vm. > > > > for libvirt, currently it's always phys->phys and mdev->mdev (and of the > > > > same mdev type). > > > > But it does not justify that hybrid cases should not be allowed. otherwise, > > > > why do we need to introduce this migration_version interface and leave > > > > the judgement of migration compatibility to vendor driver? why not simply > > > > set the criteria to something like "pciids of parent devices are equal, > > > > and mdev types are equal" ? > > > > > > > > > > > > > btw mdev<->phys just brings trouble to upper stack as Alex pointed out. > > > > could you help me understand why it will bring trouble to upper stack? > > > > > > > > I think it just needs to read src migration_version under src dev node, > > > > and test it in target migration version under target dev node. > > > > > > > > after all, through this interface we just help the upper layer > > > > knowing available options through reading and testing, and they decide > > > > to use it or not. > > > > > > > > > Can we simplify the requirement by allowing only mdev<->mdev and > > > > > phys<->phys migration? If an customer does want to migrate between a > > > > > mdev and phys, he could wrap physical device into a wrapped mdev > > > > > instance (with the same type as the source mdev) instead of using vendor > > > > > ops. Doing so does add some burden but if mdev<->phys is not dominant > > > > > usage then such tradeoff might be worthywhile... > > > > > > > > > If the interfaces for phys<->phys and mdev<->mdev are consistent, it makes no > > > > difference to phys<->mdev, right? > > > > I think the vendor string for a mdev device is something like: > > > > "Parent PCIID + mdev type + software version", and > > > > that for a phys device is something like: > > > > "PCIID + software version". > > > > as long as we don't migrate between devices from different vendors, it's > > > > easy for vendor driver to tell if a phys device is migration compatible > > > > to a mdev device according it supports it or not. > > > > > > It surprises me that the PCIID matching is a requirement; I'd assumed > > > with this clever mdev name setup that you could migrate between two > > > different models in a series, or to a newer model, as long as they > > > both supported the same mdev view. > > > > > hi Dave > > the migration_version string is transparent to userspace, and is > > completely defined by vendor driver. > > I put it there just as an example of how vendor driver may implement it. > > e.g. > > the src migration_version string is "src PCIID + src software version", > > then when this string is write to target migration_version node, > > the vendor driver in the target device will compare it with its own > > device info and software version. > > If different models are allowed, the write just succeeds even > > PCIIDs in src and target are different. > > > > so, it is the vendor driver to define whether two devices are able to > > migrate, no matter their PCIIDs, mdev types, software versions..., which > > provides vendor driver full flexibility. > > > > do you think it's good? > > Yeh that's OK; I guess it's going to need to have a big table in their > with all the PCIIDs in. > The alternative would be to abstract it a little; e.g. to say it's > an Intel-gpu-core-v4 and then it would be less worried about the exact > clock speed etc - but yes you might be right htat PCIIDs might be best > for checking for quirks. > glad that you are agreed with it:) I think the vendor driver still can choose a way to abstract a little (e.g. Intel-gpu-core-v4...) if they think it's better. In that case, the migration_string would be something like "Intel-gpu-core-v4 + instance number + software version". IOW, they can choose anything they think appropriate to identify migration compatibility of a device. But Alex is right, we have to prevent namespace overlapping. So I think we need to ensure src and target devices are from the same vendors. or, any other ideas?

That's why I kept the 'Intel' in that example; or PCI vendor ID; I was Yes, it's a good idea! could we add a line in the doc saying that it is the vendor driver to add a unique string to avoid namespace

collision?

So why don't we split the difference; lets say that it should start with the hex PCI Vendor ID.

The problem is for mdev devices, if the parent devices are not PCI devices, they don't have PCI vendor IDs.

Hmm it would be best not to invent a whole new way of giving unique idenitifiers for vendors if we can.

what about leveraging the flags in vfio device info ? #define VFIO_DEVICE_FLAGS_RESET (1 << 0) /* Device supports reset */ #define VFIO_DEVICE_FLAGS_PCI (1 << 1) /* vfio-pci device */ #define VFIO_DEVICE_FLAGS_PLATFORM (1 << 2) /* vfio-platform device */ #define VFIO_DEVICE_FLAGS_AMBA (1 << 3) /* vfio-amba device */ #define VFIO_DEVICE_FLAGS_CCW (1 << 4) /* vfio-ccw device */ #define VFIO_DEVICE_FLAGS_AP (1 << 5) /* vfio-ap device */ Then for migration_version string, The first 64 bits are for device type, the second 64 bits are for device id. e.g. for PCI devices, it could be VFIO_DEVICE_FLAGS_PCI + PCI ID. Currently in the doc, we only define PCI devices to use PCI ID as the second 64 bits. In future, if other types of devices want to support migration, they can define their own parts of device id. e.g. use ACPI ID as the second 64-bit... sounds good? Thanks Yan

...

...

...

...

only really trying to say that within one vendors range there are often a lot of PCI-IDs that have really minor variations. Yes. I also prefer to include PCI-IDs. BTW, sometimes even the same PCI-ID does not guarantee two devices are of no difference or are migration compatible. for example, two local NVMe devices may have the same PCI-ID but are configured to two different remote NVMe devices. the vendor driver needs to add extra info besides PCI-IDs then.

Ah, yes that's an interesting example.

Dave

...

...

...

> > > > > > > > > > > > > > > > > > > I agree that there was a gap in the previous proposal for non-mdev > > > > > > > devices, but I think this bring a lot of questions that we need to > > > > > > > puzzle through and libvirt will need to re-evaluate how they might > > > > > > > decide to pick a migration target device. For example, I'm sure > > > > > > > libvirt would reject any policy decisions regarding picking a physical > > > > > > > device versus an mdev device. Had we previously left it that only a > > > > > > > layer above libvirt would select a target device and libvirt only tests > > > > > > > compatibility to that target device? > > > > > > I'm not sure if there's a layer above libvirt would select a target > > > > > > device. but if there is such a layer (even it's human), we need to > > > > > > provide an interface for them to know whether their decision is suitable > > > > > > for migration. The migration_version interface provides a potential to > > > > > > allow mdev->phys migration, even libvirt may currently reject it. > > > > > > > > > > > > > > > > > > > We also need to consider that this expands the namespace. If we no > > > > > > > longer require matching types as the first level of comparison, then > > > > > > > vendor migration strings can theoretically collide. How do we > > > > > > > coordinate that can't happen? Thanks, > > > > > > yes, it's indeed a problem. > > > > > > could only allowing migration beteen devices from the same vendor be a > > > > > > good > > > > > > prerequisite? > > > > > > > > > > > > Thanks > > > > > > Yan > > > > > > >

<snip>

On Tue, Jun 02, 2020 at 04:55:27PM -0600, Alex Williamson wrote:

On Wed, 29 Apr 2020 20:39:50 -0400 Yan Zhao <yan.y.zhao@intel.com> wrote:

...

On Wed, Apr 29, 2020 at 05:48:44PM +0800, Dr. David Alan Gilbert wrote: <snip>

...

...

...

...

> > > > > > > > > An mdev type is meant to define a software compatible interface, so in > > > > > > > > > the case of mdev->mdev migration, doesn't migrating to a different type > > > > > > > > > fail the most basic of compatibility tests that we expect userspace to > > > > > > > > > perform? IOW, if two mdev types are migration compatible, it seems a > > > > > > > > > prerequisite to that is that they provide the same software interface, > > > > > > > > > which means they should be the same mdev type. > > > > > > > > > > > > > > > > > > In the hybrid cases of mdev->phys or phys->mdev, how does a > > > > > > > > management > > > > > > > > > tool begin to even guess what might be compatible? Are we expecting > > > > > > > > > libvirt to probe ever device with this attribute in the system? Is > > > > > > > > > there going to be a new class hierarchy created to enumerate all > > > > > > > > > possible migrate-able devices? > > > > > > > > > > > > > > > > > yes, management tool needs to guess and test migration compatible > > > > > > > > between two devices. But I think it's not the problem only for > > > > > > > > mdev->phys or phys->mdev. even for mdev->mdev, management tool needs > > > > > > > > to > > > > > > > > first assume that the two mdevs have the same type of parent devices > > > > > > > > (e.g.their pciids are equal). otherwise, it's still enumerating > > > > > > > > possibilities. > > > > > > > > > > > > > > > > on the other hand, for two mdevs, > > > > > > > > mdev1 from pdev1, its mdev_type is 1/2 of pdev1; > > > > > > > > mdev2 from pdev2, its mdev_type is 1/4 of pdev2; > > > > > > > > if pdev2 is exactly 2 times of pdev1, why not allow migration between > > > > > > > > mdev1 <-> mdev2. > > > > > > > > > > > > > > How could the manage tool figure out that 1/2 of pdev1 is equivalent > > > > > > > to 1/4 of pdev2? If we really want to allow such thing happen, the best > > > > > > > choice is to report the same mdev type on both pdev1 and pdev2. > > > > > > I think that's exactly the value of this migration_version interface. > > > > > > the management tool can take advantage of this interface to know if two > > > > > > devices are migration compatible, no matter they are mdevs, non-mdevs, > > > > > > or mix. > > > > > > > > > > > > as I know, (please correct me if not right), current libvirt still > > > > > > requires manually generating mdev devices, and it just duplicates src vm > > > > > > configuration to the target vm. > > > > > > for libvirt, currently it's always phys->phys and mdev->mdev (and of the > > > > > > same mdev type). > > > > > > But it does not justify that hybrid cases should not be allowed. otherwise, > > > > > > why do we need to introduce this migration_version interface and leave > > > > > > the judgement of migration compatibility to vendor driver? why not simply > > > > > > set the criteria to something like "pciids of parent devices are equal, > > > > > > and mdev types are equal" ? > > > > > > > > > > > > > > > > > > > btw mdev<->phys just brings trouble to upper stack as Alex pointed out. > > > > > > could you help me understand why it will bring trouble to upper stack? > > > > > > > > > > > > I think it just needs to read src migration_version under src dev node, > > > > > > and test it in target migration version under target dev node. > > > > > > > > > > > > after all, through this interface we just help the upper layer > > > > > > knowing available options through reading and testing, and they decide > > > > > > to use it or not. > > > > > > > > > > > > > Can we simplify the requirement by allowing only mdev<->mdev and > > > > > > > phys<->phys migration? If an customer does want to migrate between a > > > > > > > mdev and phys, he could wrap physical device into a wrapped mdev > > > > > > > instance (with the same type as the source mdev) instead of using vendor > > > > > > > ops. Doing so does add some burden but if mdev<->phys is not dominant > > > > > > > usage then such tradeoff might be worthywhile... > > > > > > > > > > > > > If the interfaces for phys<->phys and mdev<->mdev are consistent, it makes no > > > > > > difference to phys<->mdev, right? > > > > > > I think the vendor string for a mdev device is something like: > > > > > > "Parent PCIID + mdev type + software version", and > > > > > > that for a phys device is something like: > > > > > > "PCIID + software version". > > > > > > as long as we don't migrate between devices from different vendors, it's > > > > > > easy for vendor driver to tell if a phys device is migration compatible > > > > > > to a mdev device according it supports it or not. > > > > > > > > > > It surprises me that the PCIID matching is a requirement; I'd assumed > > > > > with this clever mdev name setup that you could migrate between two > > > > > different models in a series, or to a newer model, as long as they > > > > > both supported the same mdev view. > > > > > > > > > hi Dave > > > > the migration_version string is transparent to userspace, and is > > > > completely defined by vendor driver. > > > > I put it there just as an example of how vendor driver may implement it. > > > > e.g. > > > > the src migration_version string is "src PCIID + src software version", > > > > then when this string is write to target migration_version node, > > > > the vendor driver in the target device will compare it with its own > > > > device info and software version. > > > > If different models are allowed, the write just succeeds even > > > > PCIIDs in src and target are different. > > > > > > > > so, it is the vendor driver to define whether two devices are able to > > > > migrate, no matter their PCIIDs, mdev types, software versions..., which > > > > provides vendor driver full flexibility. > > > > > > > > do you think it's good? > > > > > > Yeh that's OK; I guess it's going to need to have a big table in their > > > with all the PCIIDs in. > > > The alternative would be to abstract it a little; e.g. to say it's > > > an Intel-gpu-core-v4 and then it would be less worried about the exact > > > clock speed etc - but yes you might be right htat PCIIDs might be best > > > for checking for quirks. > > > > > glad that you are agreed with it:) > > I think the vendor driver still can choose a way to abstract a little > > (e.g. Intel-gpu-core-v4...) if they think it's better. In that case, the > > migration_string would be something like "Intel-gpu-core-v4 + instance > > number + software version". > > IOW, they can choose anything they think appropriate to identify migration > > compatibility of a device. > > But Alex is right, we have to prevent namespace overlapping. So I think > > we need to ensure src and target devices are from the same vendors. > > or, any other ideas? > > That's why I kept the 'Intel' in that example; or PCI vendor ID; I was Yes, it's a good idea! could we add a line in the doc saying that it is the vendor driver to add a unique string to avoid namespace collision?

So why don't we split the difference; lets say that it should start with the hex PCI Vendor ID.

The problem is for mdev devices, if the parent devices are not PCI devices, they don't have PCI vendor IDs.

Hmm it would be best not to invent a whole new way of giving unique idenitifiers for vendors if we can.

what about leveraging the flags in vfio device info ?

#define VFIO_DEVICE_FLAGS_RESET (1 << 0) /* Device supports reset */ #define VFIO_DEVICE_FLAGS_PCI (1 << 1) /* vfio-pci device */ #define VFIO_DEVICE_FLAGS_PLATFORM (1 << 2) /* vfio-platform device */ #define VFIO_DEVICE_FLAGS_AMBA (1 << 3) /* vfio-amba device */ #define VFIO_DEVICE_FLAGS_CCW (1 << 4) /* vfio-ccw device */ #define VFIO_DEVICE_FLAGS_AP (1 << 5) /* vfio-ap device */

Then for migration_version string, The first 64 bits are for device type, the second 64 bits are for device id. e.g. for PCI devices, it could be VFIO_DEVICE_FLAGS_PCI + PCI ID.

Currently in the doc, we only define PCI devices to use PCI ID as the second 64 bits. In future, if other types of devices want to support migration, they can define their own parts of device id. e.g. use ACPI ID as the second 64-bit...

sounds good?

[dead thread resurrection alert]

Not really. We're deep into territory that we were trying to avoid. We had previously defined the version string as opaque (not transparent) specifically because we did not want userspace to make assumptions about compatibility based on the content of the string. It was 100% left to the vendor driver to determine compatibility. The mdev type was the full extent of the first level filter that userspace could use to narrow the set of potentially compatible devices. If we remove that due to physical device migration support, I'm not sure how we simplify the problem for userspace.

We need to step away from PCI IDs and parent devices. We're not designing a solution that only works for PCI, there's no guarantee that parent devices are similar or even from the same vendor.

Does the mdev type sufficiently solve the problem for mdev devices? If so, then what can we learn from it and how can we apply an equivalence to physical devices? For example, should a vfio bus driver (vfio-pci or vfio-mdev) expose vfio_migration_type and vfio_migration_version attributes under the device in sysfs where the _type provides the first level, user transparent, matching string (ex. mdev type for mdev devices) while the _version provides the user opaque, vendor known compatibility test?

This pushes the problem out to the drivers where we can perhaps incorporate the module name to avoid collisions. For example Yan's vendor extension proposal makes use of vfio-pci with extension modules loaded via an alias incorporating the PCI vendor and device ID. So vfio-pci might use a type of "vfio-pci:$ALIAS".

It's still a bit messy that someone needs to go evaluate all these types between devices that exist and mdev devices that might exist if created, but I don't have any good ideas to resolve that (maybe a new class hierarchy?). Thanks,

hi Alex yes, with the same mdev_type, user still has to enumerate all parent devices and test between the supported mdev_types to know whether two mdev devices are compatible. maybe this is not a problem? in reality, it is the administrator that specifies two devices and the management tool feedbacks compatibility result. management tool is not required to pre-test and setup the compatibility map beforehand. If so, then the only problem left is namespace collision. given that the migration_version nodes is exported by vendor driver, maybe it can also embed its module name in the migration version string, like "i915" in "i915-GVTg_V5_8", as you suggested above. with module name as the first mandatory field in version string and skipping the enumeration/testing problem, we can happyly unify migration across mdev and phys devices. e.g. it is possible to migrate between VFs in sriov and mdevs in siov to achieve backwards compatibility. Thanks Yan

On Tue, Jun 02, 2020 at 09:55:28PM -0600, Alex Williamson wrote:

On Tue, 2 Jun 2020 23:19:48 -0400 Yan Zhao <yan.y.zhao@intel.com> wrote:

...

On Tue, Jun 02, 2020 at 04:55:27PM -0600, Alex Williamson wrote:

...

On Wed, 29 Apr 2020 20:39:50 -0400 Yan Zhao <yan.y.zhao@intel.com> wrote:

...

On Wed, Apr 29, 2020 at 05:48:44PM +0800, Dr. David Alan Gilbert wrote: <snip>

...

...

> > > > > > > > > > > An mdev type is meant to define a software compatible interface, so in > > > > > > > > > > > the case of mdev->mdev migration, doesn't migrating to a different type > > > > > > > > > > > fail the most basic of compatibility tests that we expect userspace to > > > > > > > > > > > perform? IOW, if two mdev types are migration compatible, it seems a > > > > > > > > > > > prerequisite to that is that they provide the same software interface, > > > > > > > > > > > which means they should be the same mdev type. > > > > > > > > > > > > > > > > > > > > > > In the hybrid cases of mdev->phys or phys->mdev, how does a > > > > > > > > > > management > > > > > > > > > > > tool begin to even guess what might be compatible? Are we expecting > > > > > > > > > > > libvirt to probe ever device with this attribute in the system? Is > > > > > > > > > > > there going to be a new class hierarchy created to enumerate all > > > > > > > > > > > possible migrate-able devices? > > > > > > > > > > > > > > > > > > > > > yes, management tool needs to guess and test migration compatible > > > > > > > > > > between two devices. But I think it's not the problem only for > > > > > > > > > > mdev->phys or phys->mdev. even for mdev->mdev, management tool needs > > > > > > > > > > to > > > > > > > > > > first assume that the two mdevs have the same type of parent devices > > > > > > > > > > (e.g.their pciids are equal). otherwise, it's still enumerating > > > > > > > > > > possibilities. > > > > > > > > > > > > > > > > > > > > on the other hand, for two mdevs, > > > > > > > > > > mdev1 from pdev1, its mdev_type is 1/2 of pdev1; > > > > > > > > > > mdev2 from pdev2, its mdev_type is 1/4 of pdev2; > > > > > > > > > > if pdev2 is exactly 2 times of pdev1, why not allow migration between > > > > > > > > > > mdev1 <-> mdev2. > > > > > > > > > > > > > > > > > > How could the manage tool figure out that 1/2 of pdev1 is equivalent > > > > > > > > > to 1/4 of pdev2? If we really want to allow such thing happen, the best > > > > > > > > > choice is to report the same mdev type on both pdev1 and pdev2. > > > > > > > > I think that's exactly the value of this migration_version interface. > > > > > > > > the management tool can take advantage of this interface to know if two > > > > > > > > devices are migration compatible, no matter they are mdevs, non-mdevs, > > > > > > > > or mix. > > > > > > > > > > > > > > > > as I know, (please correct me if not right), current libvirt still > > > > > > > > requires manually generating mdev devices, and it just duplicates src vm > > > > > > > > configuration to the target vm. > > > > > > > > for libvirt, currently it's always phys->phys and mdev->mdev (and of the > > > > > > > > same mdev type). > > > > > > > > But it does not justify that hybrid cases should not be allowed. otherwise, > > > > > > > > why do we need to introduce this migration_version interface and leave > > > > > > > > the judgement of migration compatibility to vendor driver? why not simply > > > > > > > > set the criteria to something like "pciids of parent devices are equal, > > > > > > > > and mdev types are equal" ? > > > > > > > > > > > > > > > > > > > > > > > > > btw mdev<->phys just brings trouble to upper stack as Alex pointed out. > > > > > > > > could you help me understand why it will bring trouble to upper stack? > > > > > > > > > > > > > > > > I think it just needs to read src migration_version under src dev node, > > > > > > > > and test it in target migration version under target dev node. > > > > > > > > > > > > > > > > after all, through this interface we just help the upper layer > > > > > > > > knowing available options through reading and testing, and they decide > > > > > > > > to use it or not. > > > > > > > > > > > > > > > > > Can we simplify the requirement by allowing only mdev<->mdev and > > > > > > > > > phys<->phys migration? If an customer does want to migrate between a > > > > > > > > > mdev and phys, he could wrap physical device into a wrapped mdev > > > > > > > > > instance (with the same type as the source mdev) instead of using vendor > > > > > > > > > ops. Doing so does add some burden but if mdev<->phys is not dominant > > > > > > > > > usage then such tradeoff might be worthywhile... > > > > > > > > > > > > > > > > > If the interfaces for phys<->phys and mdev<->mdev are consistent, it makes no > > > > > > > > difference to phys<->mdev, right? > > > > > > > > I think the vendor string for a mdev device is something like: > > > > > > > > "Parent PCIID + mdev type + software version", and > > > > > > > > that for a phys device is something like: > > > > > > > > "PCIID + software version". > > > > > > > > as long as we don't migrate between devices from different vendors, it's > > > > > > > > easy for vendor driver to tell if a phys device is migration compatible > > > > > > > > to a mdev device according it supports it or not. > > > > > > > > > > > > > > It surprises me that the PCIID matching is a requirement; I'd assumed > > > > > > > with this clever mdev name setup that you could migrate between two > > > > > > > different models in a series, or to a newer model, as long as they > > > > > > > both supported the same mdev view. > > > > > > > > > > > > > hi Dave > > > > > > the migration_version string is transparent to userspace, and is > > > > > > completely defined by vendor driver. > > > > > > I put it there just as an example of how vendor driver may implement it. > > > > > > e.g. > > > > > > the src migration_version string is "src PCIID + src software version", > > > > > > then when this string is write to target migration_version node, > > > > > > the vendor driver in the target device will compare it with its own > > > > > > device info and software version. > > > > > > If different models are allowed, the write just succeeds even > > > > > > PCIIDs in src and target are different. > > > > > > > > > > > > so, it is the vendor driver to define whether two devices are able to > > > > > > migrate, no matter their PCIIDs, mdev types, software versions..., which > > > > > > provides vendor driver full flexibility. > > > > > > > > > > > > do you think it's good? > > > > > > > > > > Yeh that's OK; I guess it's going to need to have a big table in their > > > > > with all the PCIIDs in. > > > > > The alternative would be to abstract it a little; e.g. to say it's > > > > > an Intel-gpu-core-v4 and then it would be less worried about the exact > > > > > clock speed etc - but yes you might be right htat PCIIDs might be best > > > > > for checking for quirks. > > > > > > > > > glad that you are agreed with it:) > > > > I think the vendor driver still can choose a way to abstract a little > > > > (e.g. Intel-gpu-core-v4...) if they think it's better. In that case, the > > > > migration_string would be something like "Intel-gpu-core-v4 + instance > > > > number + software version". > > > > IOW, they can choose anything they think appropriate to identify migration > > > > compatibility of a device. > > > > But Alex is right, we have to prevent namespace overlapping. So I think > > > > we need to ensure src and target devices are from the same vendors. > > > > or, any other ideas? > > > > > > That's why I kept the 'Intel' in that example; or PCI vendor ID; I was > > Yes, it's a good idea! > > could we add a line in the doc saying that > > it is the vendor driver to add a unique string to avoid namespace > > collision? > > So why don't we split the difference; lets say that it should start with > the hex PCI Vendor ID. > The problem is for mdev devices, if the parent devices are not PCI devices, they don't have PCI vendor IDs.

Hmm it would be best not to invent a whole new way of giving unique idenitifiers for vendors if we can.

what about leveraging the flags in vfio device info ?

#define VFIO_DEVICE_FLAGS_RESET (1 << 0) /* Device supports reset */ #define VFIO_DEVICE_FLAGS_PCI (1 << 1) /* vfio-pci device */ #define VFIO_DEVICE_FLAGS_PLATFORM (1 << 2) /* vfio-platform device */ #define VFIO_DEVICE_FLAGS_AMBA (1 << 3) /* vfio-amba device */ #define VFIO_DEVICE_FLAGS_CCW (1 << 4) /* vfio-ccw device */ #define VFIO_DEVICE_FLAGS_AP (1 << 5) /* vfio-ap device */

Then for migration_version string, The first 64 bits are for device type, the second 64 bits are for device id. e.g. for PCI devices, it could be VFIO_DEVICE_FLAGS_PCI + PCI ID.

Currently in the doc, we only define PCI devices to use PCI ID as the second 64 bits. In future, if other types of devices want to support migration, they can define their own parts of device id. e.g. use ACPI ID as the second 64-bit...

sounds good?

[dead thread resurrection alert]

Not really. We're deep into territory that we were trying to avoid. We had previously defined the version string as opaque (not transparent) specifically because we did not want userspace to make assumptions about compatibility based on the content of the string. It was 100% left to the vendor driver to determine compatibility. The mdev type was the full extent of the first level filter that userspace could use to narrow the set of potentially compatible devices. If we remove that due to physical device migration support, I'm not sure how we simplify the problem for userspace.

We need to step away from PCI IDs and parent devices. We're not designing a solution that only works for PCI, there's no guarantee that parent devices are similar or even from the same vendor.

Does the mdev type sufficiently solve the problem for mdev devices? If so, then what can we learn from it and how can we apply an equivalence to physical devices? For example, should a vfio bus driver (vfio-pci or vfio-mdev) expose vfio_migration_type and vfio_migration_version attributes under the device in sysfs where the _type provides the first level, user transparent, matching string (ex. mdev type for mdev devices) while the _version provides the user opaque, vendor known compatibility test?

This pushes the problem out to the drivers where we can perhaps incorporate the module name to avoid collisions. For example Yan's vendor extension proposal makes use of vfio-pci with extension modules loaded via an alias incorporating the PCI vendor and device ID. So vfio-pci might use a type of "vfio-pci:$ALIAS".

It's still a bit messy that someone needs to go evaluate all these types between devices that exist and mdev devices that might exist if created, but I don't have any good ideas to resolve that (maybe a new class hierarchy?). Thanks,

hi Alex

yes, with the same mdev_type, user still has to enumerate all parent devices and test between the supported mdev_types to know whether two mdev devices are compatible. maybe this is not a problem? in reality, it is the administrator that specifies two devices and the management tool feedbacks compatibility result. management tool is not required to pre-test and setup the compatibility map beforehand.

That's exactly the purpose of this interface though is to give the management tools some indication that a migration has a chance of working.

...

If so, then the only problem left is namespace collision. given that the migration_version nodes is exported by vendor driver, maybe it can also embed its module name in the migration version string, like "i915" in "i915-GVTg_V5_8", as you suggested above.

No, we've already decided that the version string is opaque, the user is not to attempt to infer anything from it. That's why I've suggested another attribute in sysfs that does present type information that a user can compare. Thanks,

Alex

ok. got it. one more thing I want to confirm is that do you think it's a necessary restriction that "The mdev devices are of the same type" ? could mdev and phys devices both expose "vfio_migration_type" and "vfio_migration_version" under device sysfs so that it may not be confined in mdev_type? (e.g. when aggregator is enabled, though two mdevs are of the same mdev_type, they are not actually compatible; and two mdevs are compatible though their mdev_type is not equal.) for mdev devices, we could still expose vfio_migration_version attribute under mdev_type for detection before mdev generated. Thanks Yan

...

with module name as the first mandatory field in version string and skipping the enumeration/testing problem, we can happyly unify migration across mdev and phys devices. e.g. it is possible to migrate between VFs in sriov and mdevs in siov to achieve backwards compatibility.

Thanks Yan

...

* Alex Williamson (alex.williamson@redhat.com) wrote:

On Wed, 3 Jun 2020 01:24:43 -0400 Yan Zhao <yan.y.zhao@intel.com> wrote:

...

On Tue, Jun 02, 2020 at 09:55:28PM -0600, Alex Williamson wrote:

...

On Tue, 2 Jun 2020 23:19:48 -0400 Yan Zhao <yan.y.zhao@intel.com> wrote:

...

On Tue, Jun 02, 2020 at 04:55:27PM -0600, Alex Williamson wrote:

...

On Wed, 29 Apr 2020 20:39:50 -0400 Yan Zhao <yan.y.zhao@intel.com> wrote:

...

On Wed, Apr 29, 2020 at 05:48:44PM +0800, Dr. David Alan Gilbert wrote: <snip> > > > > > > > > > > > > > An mdev type is meant to define a software compatible interface, so in > > > > > > > > > > > > > the case of mdev->mdev migration, doesn't migrating to a different type > > > > > > > > > > > > > fail the most basic of compatibility tests that we expect userspace to > > > > > > > > > > > > > perform? IOW, if two mdev types are migration compatible, it seems a > > > > > > > > > > > > > prerequisite to that is that they provide the same software interface, > > > > > > > > > > > > > which means they should be the same mdev type. > > > > > > > > > > > > > > > > > > > > > > > > > > In the hybrid cases of mdev->phys or phys->mdev, how does a > > > > > > > > > > > > management > > > > > > > > > > > > > tool begin to even guess what might be compatible? Are we expecting > > > > > > > > > > > > > libvirt to probe ever device with this attribute in the system? Is > > > > > > > > > > > > > there going to be a new class hierarchy created to enumerate all > > > > > > > > > > > > > possible migrate-able devices? > > > > > > > > > > > > > > > > > > > > > > > > > yes, management tool needs to guess and test migration compatible > > > > > > > > > > > > between two devices. But I think it's not the problem only for > > > > > > > > > > > > mdev->phys or phys->mdev. even for mdev->mdev, management tool needs > > > > > > > > > > > > to > > > > > > > > > > > > first assume that the two mdevs have the same type of parent devices > > > > > > > > > > > > (e.g.their pciids are equal). otherwise, it's still enumerating > > > > > > > > > > > > possibilities. > > > > > > > > > > > > > > > > > > > > > > > > on the other hand, for two mdevs, > > > > > > > > > > > > mdev1 from pdev1, its mdev_type is 1/2 of pdev1; > > > > > > > > > > > > mdev2 from pdev2, its mdev_type is 1/4 of pdev2; > > > > > > > > > > > > if pdev2 is exactly 2 times of pdev1, why not allow migration between > > > > > > > > > > > > mdev1 <-> mdev2. > > > > > > > > > > > > > > > > > > > > > > How could the manage tool figure out that 1/2 of pdev1 is equivalent > > > > > > > > > > > to 1/4 of pdev2? If we really want to allow such thing happen, the best > > > > > > > > > > > choice is to report the same mdev type on both pdev1 and pdev2. > > > > > > > > > > I think that's exactly the value of this migration_version interface. > > > > > > > > > > the management tool can take advantage of this interface to know if two > > > > > > > > > > devices are migration compatible, no matter they are mdevs, non-mdevs, > > > > > > > > > > or mix. > > > > > > > > > > > > > > > > > > > > as I know, (please correct me if not right), current libvirt still > > > > > > > > > > requires manually generating mdev devices, and it just duplicates src vm > > > > > > > > > > configuration to the target vm. > > > > > > > > > > for libvirt, currently it's always phys->phys and mdev->mdev (and of the > > > > > > > > > > same mdev type). > > > > > > > > > > But it does not justify that hybrid cases should not be allowed. otherwise, > > > > > > > > > > why do we need to introduce this migration_version interface and leave > > > > > > > > > > the judgement of migration compatibility to vendor driver? why not simply > > > > > > > > > > set the criteria to something like "pciids of parent devices are equal, > > > > > > > > > > and mdev types are equal" ? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > btw mdev<->phys just brings trouble to upper stack as Alex pointed out. > > > > > > > > > > could you help me understand why it will bring trouble to upper stack? > > > > > > > > > > > > > > > > > > > > I think it just needs to read src migration_version under src dev node, > > > > > > > > > > and test it in target migration version under target dev node. > > > > > > > > > > > > > > > > > > > > after all, through this interface we just help the upper layer > > > > > > > > > > knowing available options through reading and testing, and they decide > > > > > > > > > > to use it or not. > > > > > > > > > > > > > > > > > > > > > Can we simplify the requirement by allowing only mdev<->mdev and > > > > > > > > > > > phys<->phys migration? If an customer does want to migrate between a > > > > > > > > > > > mdev and phys, he could wrap physical device into a wrapped mdev > > > > > > > > > > > instance (with the same type as the source mdev) instead of using vendor > > > > > > > > > > > ops. Doing so does add some burden but if mdev<->phys is not dominant > > > > > > > > > > > usage then such tradeoff might be worthywhile... > > > > > > > > > > > > > > > > > > > > > If the interfaces for phys<->phys and mdev<->mdev are consistent, it makes no > > > > > > > > > > difference to phys<->mdev, right? > > > > > > > > > > I think the vendor string for a mdev device is something like: > > > > > > > > > > "Parent PCIID + mdev type + software version", and > > > > > > > > > > that for a phys device is something like: > > > > > > > > > > "PCIID + software version". > > > > > > > > > > as long as we don't migrate between devices from different vendors, it's > > > > > > > > > > easy for vendor driver to tell if a phys device is migration compatible > > > > > > > > > > to a mdev device according it supports it or not. > > > > > > > > > > > > > > > > > > It surprises me that the PCIID matching is a requirement; I'd assumed > > > > > > > > > with this clever mdev name setup that you could migrate between two > > > > > > > > > different models in a series, or to a newer model, as long as they > > > > > > > > > both supported the same mdev view. > > > > > > > > > > > > > > > > > hi Dave > > > > > > > > the migration_version string is transparent to userspace, and is > > > > > > > > completely defined by vendor driver. > > > > > > > > I put it there just as an example of how vendor driver may implement it. > > > > > > > > e.g. > > > > > > > > the src migration_version string is "src PCIID + src software version", > > > > > > > > then when this string is write to target migration_version node, > > > > > > > > the vendor driver in the target device will compare it with its own > > > > > > > > device info and software version. > > > > > > > > If different models are allowed, the write just succeeds even > > > > > > > > PCIIDs in src and target are different. > > > > > > > > > > > > > > > > so, it is the vendor driver to define whether two devices are able to > > > > > > > > migrate, no matter their PCIIDs, mdev types, software versions..., which > > > > > > > > provides vendor driver full flexibility. > > > > > > > > > > > > > > > > do you think it's good? > > > > > > > > > > > > > > Yeh that's OK; I guess it's going to need to have a big table in their > > > > > > > with all the PCIIDs in. > > > > > > > The alternative would be to abstract it a little; e.g. to say it's > > > > > > > an Intel-gpu-core-v4 and then it would be less worried about the exact > > > > > > > clock speed etc - but yes you might be right htat PCIIDs might be best > > > > > > > for checking for quirks. > > > > > > > > > > > > > glad that you are agreed with it:) > > > > > > I think the vendor driver still can choose a way to abstract a little > > > > > > (e.g. Intel-gpu-core-v4...) if they think it's better. In that case, the > > > > > > migration_string would be something like "Intel-gpu-core-v4 + instance > > > > > > number + software version". > > > > > > IOW, they can choose anything they think appropriate to identify migration > > > > > > compatibility of a device. > > > > > > But Alex is right, we have to prevent namespace overlapping. So I think > > > > > > we need to ensure src and target devices are from the same vendors. > > > > > > or, any other ideas? > > > > > > > > > > That's why I kept the 'Intel' in that example; or PCI vendor ID; I was > > > > Yes, it's a good idea! > > > > could we add a line in the doc saying that > > > > it is the vendor driver to add a unique string to avoid namespace > > > > collision? > > > > > > So why don't we split the difference; lets say that it should start with > > > the hex PCI Vendor ID. > > > > > The problem is for mdev devices, if the parent devices are not PCI devices, > > they don't have PCI vendor IDs. > > Hmm it would be best not to invent a whole new way of giving unique > idenitifiers for vendors if we can. > what about leveraging the flags in vfio device info ?

#define VFIO_DEVICE_FLAGS_RESET (1 << 0) /* Device supports reset */ #define VFIO_DEVICE_FLAGS_PCI (1 << 1) /* vfio-pci device */ #define VFIO_DEVICE_FLAGS_PLATFORM (1 << 2) /* vfio-platform device */ #define VFIO_DEVICE_FLAGS_AMBA (1 << 3) /* vfio-amba device */ #define VFIO_DEVICE_FLAGS_CCW (1 << 4) /* vfio-ccw device */ #define VFIO_DEVICE_FLAGS_AP (1 << 5) /* vfio-ap device */

Then for migration_version string, The first 64 bits are for device type, the second 64 bits are for device id. e.g. for PCI devices, it could be VFIO_DEVICE_FLAGS_PCI + PCI ID.

Currently in the doc, we only define PCI devices to use PCI ID as the second 64 bits. In future, if other types of devices want to support migration, they can define their own parts of device id. e.g. use ACPI ID as the second 64-bit...

sounds good?

[dead thread resurrection alert]

Not really. We're deep into territory that we were trying to avoid. We had previously defined the version string as opaque (not transparent) specifically because we did not want userspace to make assumptions about compatibility based on the content of the string. It was 100% left to the vendor driver to determine compatibility. The mdev type was the full extent of the first level filter that userspace could use to narrow the set of potentially compatible devices. If we remove that due to physical device migration support, I'm not sure how we simplify the problem for userspace.

We need to step away from PCI IDs and parent devices. We're not designing a solution that only works for PCI, there's no guarantee that parent devices are similar or even from the same vendor.

Does the mdev type sufficiently solve the problem for mdev devices? If so, then what can we learn from it and how can we apply an equivalence to physical devices? For example, should a vfio bus driver (vfio-pci or vfio-mdev) expose vfio_migration_type and vfio_migration_version attributes under the device in sysfs where the _type provides the first level, user transparent, matching string (ex. mdev type for mdev devices) while the _version provides the user opaque, vendor known compatibility test?

This pushes the problem out to the drivers where we can perhaps incorporate the module name to avoid collisions. For example Yan's vendor extension proposal makes use of vfio-pci with extension modules loaded via an alias incorporating the PCI vendor and device ID. So vfio-pci might use a type of "vfio-pci:$ALIAS".

It's still a bit messy that someone needs to go evaluate all these types between devices that exist and mdev devices that might exist if created, but I don't have any good ideas to resolve that (maybe a new class hierarchy?). Thanks,

hi Alex

yes, with the same mdev_type, user still has to enumerate all parent devices and test between the supported mdev_types to know whether two mdev devices are compatible. maybe this is not a problem? in reality, it is the administrator that specifies two devices and the management tool feedbacks compatibility result. management tool is not required to pre-test and setup the compatibility map beforehand.

That's exactly the purpose of this interface though is to give the management tools some indication that a migration has a chance of working.

...

If so, then the only problem left is namespace collision. given that the migration_version nodes is exported by vendor driver, maybe it can also embed its module name in the migration version string, like "i915" in "i915-GVTg_V5_8", as you suggested above.

No, we've already decided that the version string is opaque, the user is not to attempt to infer anything from it. That's why I've suggested another attribute in sysfs that does present type information that a user can compare. Thanks,

Alex

ok. got it. one more thing I want to confirm is that do you think it's a necessary restriction that "The mdev devices are of the same type" ? could mdev and phys devices both expose "vfio_migration_type" and "vfio_migration_version" under device sysfs so that it may not be confined in mdev_type? (e.g. when aggregator is enabled, though two mdevs are of the same mdev_type, they are not actually compatible; and two mdevs are compatible though their mdev_type is not equal.)

for mdev devices, we could still expose vfio_migration_version attribute under mdev_type for detection before mdev generated.

I tried to simplify the problem a bit, but we keep going backwards. If the requirement is that potentially any source device can migrate to any target device and we cannot provide any means other than writing an opaque source string into a version attribute on the target and evaluating the result to determine compatibility, then we're requiring userspace to do an exhaustive search to find a potential match. That sucks.

Why is the mechanism a 'write and test' why isn't it a 'write and ask'? i.e. the destination tells the driver what type it's received from the source, and the driver replies with a set of compatible configurations (in some preferred order). It's also not clear to me why the name has to be that opaque; I agree it's only got to be understood by the driver but that doesn't seem to be a reason for the driver to make it purposely obfuscated. I wouldn't expect a user to be able to parse it necessarily; but would expect something that would be useful for an error message. Dave

We don't have an agreed proposal for aggregation and even this exhaustive search mechanism doesn't solve that problem, ex. the target type may be able to support a compatible aggregation, but the user might find after they've created the device that their aggregation was wrong and the resulting device doesn't even match the version compatibility of the parent type. We're arguing our way into an unsolvable problem and unless we can simplify it, I'm afraid there's no solution, we're just going to have a bad interface for the user to test compatibility, which is not really acceptable. Thanks,

Alex

-- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

* Alex Williamson (alex.williamson@redhat.com) wrote:

On Fri, 5 Jun 2020 11:22:24 +0100 "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:

...

* Alex Williamson (alex.williamson@redhat.com) wrote:

...

On Wed, 3 Jun 2020 01:24:43 -0400 Yan Zhao <yan.y.zhao@intel.com> wrote:

...

On Tue, Jun 02, 2020 at 09:55:28PM -0600, Alex Williamson wrote:

...

On Tue, 2 Jun 2020 23:19:48 -0400 Yan Zhao <yan.y.zhao@intel.com> wrote:

...

On Tue, Jun 02, 2020 at 04:55:27PM -0600, Alex Williamson wrote: > On Wed, 29 Apr 2020 20:39:50 -0400 > Yan Zhao <yan.y.zhao@intel.com> wrote: > > > On Wed, Apr 29, 2020 at 05:48:44PM +0800, Dr. David Alan Gilbert wrote: > > <snip> > > > > > > > > > > > > > > > An mdev type is meant to define a software compatible interface, so in > > > > > > > > > > > > > > > the case of mdev->mdev migration, doesn't migrating to a different type > > > > > > > > > > > > > > > fail the most basic of compatibility tests that we expect userspace to > > > > > > > > > > > > > > > perform? IOW, if two mdev types are migration compatible, it seems a > > > > > > > > > > > > > > > prerequisite to that is that they provide the same software interface, > > > > > > > > > > > > > > > which means they should be the same mdev type. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > In the hybrid cases of mdev->phys or phys->mdev, how does a > > > > > > > > > > > > > > management > > > > > > > > > > > > > > > tool begin to even guess what might be compatible? Are we expecting > > > > > > > > > > > > > > > libvirt to probe ever device with this attribute in the system? Is > > > > > > > > > > > > > > > there going to be a new class hierarchy created to enumerate all > > > > > > > > > > > > > > > possible migrate-able devices? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > yes, management tool needs to guess and test migration compatible > > > > > > > > > > > > > > between two devices. But I think it's not the problem only for > > > > > > > > > > > > > > mdev->phys or phys->mdev. even for mdev->mdev, management tool needs > > > > > > > > > > > > > > to > > > > > > > > > > > > > > first assume that the two mdevs have the same type of parent devices > > > > > > > > > > > > > > (e.g.their pciids are equal). otherwise, it's still enumerating > > > > > > > > > > > > > > possibilities. > > > > > > > > > > > > > > > > > > > > > > > > > > > > on the other hand, for two mdevs, > > > > > > > > > > > > > > mdev1 from pdev1, its mdev_type is 1/2 of pdev1; > > > > > > > > > > > > > > mdev2 from pdev2, its mdev_type is 1/4 of pdev2; > > > > > > > > > > > > > > if pdev2 is exactly 2 times of pdev1, why not allow migration between > > > > > > > > > > > > > > mdev1 <-> mdev2. > > > > > > > > > > > > > > > > > > > > > > > > > > How could the manage tool figure out that 1/2 of pdev1 is equivalent > > > > > > > > > > > > > to 1/4 of pdev2? If we really want to allow such thing happen, the best > > > > > > > > > > > > > choice is to report the same mdev type on both pdev1 and pdev2. > > > > > > > > > > > > I think that's exactly the value of this migration_version interface. > > > > > > > > > > > > the management tool can take advantage of this interface to know if two > > > > > > > > > > > > devices are migration compatible, no matter they are mdevs, non-mdevs, > > > > > > > > > > > > or mix. > > > > > > > > > > > > > > > > > > > > > > > > as I know, (please correct me if not right), current libvirt still > > > > > > > > > > > > requires manually generating mdev devices, and it just duplicates src vm > > > > > > > > > > > > configuration to the target vm. > > > > > > > > > > > > for libvirt, currently it's always phys->phys and mdev->mdev (and of the > > > > > > > > > > > > same mdev type). > > > > > > > > > > > > But it does not justify that hybrid cases should not be allowed. otherwise, > > > > > > > > > > > > why do we need to introduce this migration_version interface and leave > > > > > > > > > > > > the judgement of migration compatibility to vendor driver? why not simply > > > > > > > > > > > > set the criteria to something like "pciids of parent devices are equal, > > > > > > > > > > > > and mdev types are equal" ? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > btw mdev<->phys just brings trouble to upper stack as Alex pointed out. > > > > > > > > > > > > could you help me understand why it will bring trouble to upper stack? > > > > > > > > > > > > > > > > > > > > > > > > I think it just needs to read src migration_version under src dev node, > > > > > > > > > > > > and test it in target migration version under target dev node. > > > > > > > > > > > > > > > > > > > > > > > > after all, through this interface we just help the upper layer > > > > > > > > > > > > knowing available options through reading and testing, and they decide > > > > > > > > > > > > to use it or not. > > > > > > > > > > > > > > > > > > > > > > > > > Can we simplify the requirement by allowing only mdev<->mdev and > > > > > > > > > > > > > phys<->phys migration? If an customer does want to migrate between a > > > > > > > > > > > > > mdev and phys, he could wrap physical device into a wrapped mdev > > > > > > > > > > > > > instance (with the same type as the source mdev) instead of using vendor > > > > > > > > > > > > > ops. Doing so does add some burden but if mdev<->phys is not dominant > > > > > > > > > > > > > usage then such tradeoff might be worthywhile... > > > > > > > > > > > > > > > > > > > > > > > > > If the interfaces for phys<->phys and mdev<->mdev are consistent, it makes no > > > > > > > > > > > > difference to phys<->mdev, right? > > > > > > > > > > > > I think the vendor string for a mdev device is something like: > > > > > > > > > > > > "Parent PCIID + mdev type + software version", and > > > > > > > > > > > > that for a phys device is something like: > > > > > > > > > > > > "PCIID + software version". > > > > > > > > > > > > as long as we don't migrate between devices from different vendors, it's > > > > > > > > > > > > easy for vendor driver to tell if a phys device is migration compatible > > > > > > > > > > > > to a mdev device according it supports it or not. > > > > > > > > > > > > > > > > > > > > > > It surprises me that the PCIID matching is a requirement; I'd assumed > > > > > > > > > > > with this clever mdev name setup that you could migrate between two > > > > > > > > > > > different models in a series, or to a newer model, as long as they > > > > > > > > > > > both supported the same mdev view. > > > > > > > > > > > > > > > > > > > > > hi Dave > > > > > > > > > > the migration_version string is transparent to userspace, and is > > > > > > > > > > completely defined by vendor driver. > > > > > > > > > > I put it there just as an example of how vendor driver may implement it. > > > > > > > > > > e.g. > > > > > > > > > > the src migration_version string is "src PCIID + src software version", > > > > > > > > > > then when this string is write to target migration_version node, > > > > > > > > > > the vendor driver in the target device will compare it with its own > > > > > > > > > > device info and software version. > > > > > > > > > > If different models are allowed, the write just succeeds even > > > > > > > > > > PCIIDs in src and target are different. > > > > > > > > > > > > > > > > > > > > so, it is the vendor driver to define whether two devices are able to > > > > > > > > > > migrate, no matter their PCIIDs, mdev types, software versions..., which > > > > > > > > > > provides vendor driver full flexibility. > > > > > > > > > > > > > > > > > > > > do you think it's good? > > > > > > > > > > > > > > > > > > Yeh that's OK; I guess it's going to need to have a big table in their > > > > > > > > > with all the PCIIDs in. > > > > > > > > > The alternative would be to abstract it a little; e.g. to say it's > > > > > > > > > an Intel-gpu-core-v4 and then it would be less worried about the exact > > > > > > > > > clock speed etc - but yes you might be right htat PCIIDs might be best > > > > > > > > > for checking for quirks. > > > > > > > > > > > > > > > > > glad that you are agreed with it:) > > > > > > > > I think the vendor driver still can choose a way to abstract a little > > > > > > > > (e.g. Intel-gpu-core-v4...) if they think it's better. In that case, the > > > > > > > > migration_string would be something like "Intel-gpu-core-v4 + instance > > > > > > > > number + software version". > > > > > > > > IOW, they can choose anything they think appropriate to identify migration > > > > > > > > compatibility of a device. > > > > > > > > But Alex is right, we have to prevent namespace overlapping. So I think > > > > > > > > we need to ensure src and target devices are from the same vendors. > > > > > > > > or, any other ideas? > > > > > > > > > > > > > > That's why I kept the 'Intel' in that example; or PCI vendor ID; I was > > > > > > Yes, it's a good idea! > > > > > > could we add a line in the doc saying that > > > > > > it is the vendor driver to add a unique string to avoid namespace > > > > > > collision? > > > > > > > > > > So why don't we split the difference; lets say that it should start with > > > > > the hex PCI Vendor ID. > > > > > > > > > The problem is for mdev devices, if the parent devices are not PCI devices, > > > > they don't have PCI vendor IDs. > > > > > > Hmm it would be best not to invent a whole new way of giving unique > > > idenitifiers for vendors if we can. > > > > > what about leveraging the flags in vfio device info ? > > > > #define VFIO_DEVICE_FLAGS_RESET (1 << 0) /* Device supports reset */ > > #define VFIO_DEVICE_FLAGS_PCI (1 << 1) /* vfio-pci device */ > > #define VFIO_DEVICE_FLAGS_PLATFORM (1 << 2) /* vfio-platform device */ > > #define VFIO_DEVICE_FLAGS_AMBA (1 << 3) /* vfio-amba device */ > > #define VFIO_DEVICE_FLAGS_CCW (1 << 4) /* vfio-ccw device */ > > #define VFIO_DEVICE_FLAGS_AP (1 << 5) /* vfio-ap device */ > > > > Then for migration_version string, > > The first 64 bits are for device type, the second 64 bits are for device id. > > e.g. > > for PCI devices, it could be > > VFIO_DEVICE_FLAGS_PCI + PCI ID. > > > > Currently in the doc, we only define PCI devices to use PCI ID as the second > > 64 bits. In future, if other types of devices want to support migration, > > they can define their own parts of device id. e.g. use ACPI ID as the > > second 64-bit... > > > > sounds good? > > [dead thread resurrection alert] > > Not really. We're deep into territory that we were trying to avoid. > We had previously defined the version string as opaque (not > transparent) specifically because we did not want userspace to make > assumptions about compatibility based on the content of the string. It > was 100% left to the vendor driver to determine compatibility. The > mdev type was the full extent of the first level filter that userspace > could use to narrow the set of potentially compatible devices. If we > remove that due to physical device migration support, I'm not sure how > we simplify the problem for userspace. > > We need to step away from PCI IDs and parent devices. We're not > designing a solution that only works for PCI, there's no guarantee that > parent devices are similar or even from the same vendor. > > Does the mdev type sufficiently solve the problem for mdev devices? If > so, then what can we learn from it and how can we apply an equivalence > to physical devices? For example, should a vfio bus driver (vfio-pci > or vfio-mdev) expose vfio_migration_type and vfio_migration_version > attributes under the device in sysfs where the _type provides the first > level, user transparent, matching string (ex. mdev type for mdev > devices) while the _version provides the user opaque, vendor known > compatibility test? > > This pushes the problem out to the drivers where we can perhaps > incorporate the module name to avoid collisions. For example Yan's > vendor extension proposal makes use of vfio-pci with extension modules > loaded via an alias incorporating the PCI vendor and device ID. So > vfio-pci might use a type of "vfio-pci:$ALIAS". > > It's still a bit messy that someone needs to go evaluate all these > types between devices that exist and mdev devices that might exist if > created, but I don't have any good ideas to resolve that (maybe a new > class hierarchy?). Thanks,

hi Alex

yes, with the same mdev_type, user still has to enumerate all parent devices and test between the supported mdev_types to know whether two mdev devices are compatible. maybe this is not a problem? in reality, it is the administrator that specifies two devices and the management tool feedbacks compatibility result. management tool is not required to pre-test and setup the compatibility map beforehand.

That's exactly the purpose of this interface though is to give the management tools some indication that a migration has a chance of working.

...

If so, then the only problem left is namespace collision. given that the migration_version nodes is exported by vendor driver, maybe it can also embed its module name in the migration version string, like "i915" in "i915-GVTg_V5_8", as you suggested above.

No, we've already decided that the version string is opaque, the user is not to attempt to infer anything from it. That's why I've suggested another attribute in sysfs that does present type information that a user can compare. Thanks,

Alex

ok. got it. one more thing I want to confirm is that do you think it's a necessary restriction that "The mdev devices are of the same type" ? could mdev and phys devices both expose "vfio_migration_type" and "vfio_migration_version" under device sysfs so that it may not be confined in mdev_type? (e.g. when aggregator is enabled, though two mdevs are of the same mdev_type, they are not actually compatible; and two mdevs are compatible though their mdev_type is not equal.)

for mdev devices, we could still expose vfio_migration_version attribute under mdev_type for detection before mdev generated.

I tried to simplify the problem a bit, but we keep going backwards. If the requirement is that potentially any source device can migrate to any target device and we cannot provide any means other than writing an opaque source string into a version attribute on the target and evaluating the result to determine compatibility, then we're requiring userspace to do an exhaustive search to find a potential match. That sucks.

Why is the mechanism a 'write and test' why isn't it a 'write and ask'? i.e. the destination tells the driver what type it's received from the source, and the driver replies with a set of compatible configurations (in some preferred order).

A 'write and ask' interface would imply some sort of session in order to not be racy with concurrent users. More likely this would imply an ioctl interface, which I don't think we have in sysfs. Where do we host this ioctl?

Or one fd? f=open() write(f, "The ID I want") do { read(f, ...) -> The IDs we're offering that are compatible } while (!eof)

...

It's also not clear to me why the name has to be that opaque; I agree it's only got to be understood by the driver but that doesn't seem to be a reason for the driver to make it purposely obfuscated. I wouldn't expect a user to be able to parse it necessarily; but would expect something that would be useful for an error message.

If the name is not opaque, then we're going to rat hole on the format and the fields and evolving that format for every feature a vendor decides they want the user to be able to parse out of the version string. Then we require a full specification of the string in order that it be parsed according to a standard such that we don't break users inferring features in subtly different ways.

This is a lot like the problems with mdev description attributes, libvirt complains they can't use description because there's no standard formatting, but even with two vendors describing the same class of device we don't have an agreed set of things to expose in the description attribute. Thanks,

I'm not suggesting anything in anyway machine parsable; just something human readable that you can present in a menu/choice/configuration/error message. The text would be down to the vendor, and I'd suggest it start with the vendor name just as a disambiguator and to make it obvious when we get it grossly wrong. Dave

Alex -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

On Tue, 9 Jun 2020 20:37:31 -0400 Yan Zhao <yan.y.zhao@intel.com> wrote:

On Fri, Jun 05, 2020 at 03:39:50PM +0100, Dr. David Alan Gilbert wrote:

...

...

...

...

I tried to simplify the problem a bit, but we keep going backwards. If the requirement is that potentially any source device can migrate to any target device and we cannot provide any means other than writing an opaque source string into a version attribute on the target and evaluating the result to determine compatibility, then we're requiring userspace to do an exhaustive search to find a potential match. That sucks.

hi Alex and Dave, do you think it's good for us to put aside physical devices and mdev aggregation for the moment, and use Alex's original idea that

+ Userspace should regard two mdev devices compatible when ALL of below + conditions are met: + (0) The mdev devices are of the same type + (1) success when reading from migration_version attribute of one mdev device. + (2) success when writing migration_version string of one mdev device to + migration_version attribute of the other mdev device.

I think Pandora's box is already opened, if we can't articulate how this solution would evolve to support features that we know are coming, why should we proceed with this approach? We've already seen interest in breaking rule (0) in this thread, so we can't focus the solution on mdev devices. Maybe the best we can do is to compare one instance of a device to another instance of a device, without any capability to predict compatibility prior to creating devices, in the case on mdev. The string would need to include not only the device and vendor driver compatibility, but also anything that has modified the state of the device, such as creation time or post-creation time configuration. The user is left on their own for creating a compatible device, or filtering devices to determine which might be, or which might generate, compatible devices. It's not much of a solution, I wonder if anyone would even use it.

and what about adding another sysfs attribute for vendors to put recommended migration compatible device type. e.g. #cat /sys/bus/pci/devices/0000:00:02.0/mdev_supported_types/i915-GVTg_V5_8/migration_compatible_devices parent id: 8086 591d mdev_type: i915-GVTg_V5_8

vendors are free to define the format and conent of this migration_compatible_devices and it's even not to be a full list.

before libvirt or user to do live migration, they have to read and test migration_version attributes of src/target devices to check migration compatibility.

AFAICT, free-form, vendor defined attributes are useless to libvirt. Vendors could already put this information in the description attribute and have it ignored by userspace tools due to the lack of defined format. It's also not clear what value this provides when it's necessarily incomplete, a driver written today cannot know what future drivers might be compatible with its migration data. Thanks, Alex

[meta-comment] On 4/29/20 4:35 AM, Yan Zhao wrote:

On Wed, Apr 29, 2020 at 04:22:01PM +0800, Dr. David Alan Gilbert wrote: [...]

...

...

...

...

...

>>>>>>>>>>> This patchset introduces a migration_version attribute under sysfs >>>>> of VFIO >>>>>>>>>>> Mediated devices.

Hmm, several pages with up to 16 levels of quoting, with editors making the lines ragged, all before I get to the real meat of the email. Remember, it's okay to trim content,...

...

So why don't we split the difference; lets say that it should start with the hex PCI Vendor ID.

The problem is for mdev devices, if the parent devices are not PCI devices, they don't have PCI vendor IDs.

...to just what you are replying to. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org