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@xxxxxxxxx> 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 > > >
