hw/vfio/common.c | 32 +++++++++ hw/vfio/pci.c | 164 +++++++++++++++++++++++++++++++++++++++++- hw/vfio/pci.h | 1 + include/hw/vfio/vfio-common.h | 1 + linux-headers/linux/vfio.h | 26 ++++++- 5 files changed, 220 insertions(+), 4 deletions(-)
Summary
This series RFC would like to introduce the live migration capability
to vfio_mdev device.
As currently vfio_mdev device don't support migration, we introduce
a new vfio subtype region VFIO_REGION_SUBTYPE_INTEL_IGD_DEVICE_STATE
for Intel vGPU device, during the vfio device initialization, the mdev
device will be set to migratable if the new region exist.
The intention to add the new region is using it for vfio_mdev device
status save and restore during the migration. The access to this region
will be trapped and forward to the vfio_mdev device driver. And we use
the first byte in the new region to control the running state of mdev
device.
Meanwhile we add one new ioctl VFIO_IOMMU_GET_DIRTY_BITMAP to help do
the mdev device dirty page synchronization.
So the vfio_mdev device migration sequence would be
Source VM side:
start migration
|
V
get the cpu state change callback, write to the
subregion's first byte to stop the mdev device
|
V
quary the dirty page bitmap from iommu container
and add into qemu dirty list for synchronization
|
V
save the deivce status into Qemufile which is
read from the vfio device subregion
Target VM side:
restore the mdev device after get the
saved status context from Qemufile
|
V
get the cpu state change callback
write to subregion's first byte to
start the mdev device to put it in
running status
|
V
finish migration
V1->V2:
Per Alex's suggestion:
1. use device subtype region instead of VFIO PCI fixed region.
2. remove unnecessary ioctl, use the first byte of subregion to
control the running state of mdev device.
3. for dirty page synchronization, implement the interface with
VFIOContainer instead of vfio pci device.
Yulei Zhang (4):
vfio: introduce a new VFIO sub region for mdev device migration
support
vfio: Add vm status change callback to stop/restart the mdev device
vfio: Add struct vfio_vmstate_info to introduce put/get callback
funtion for vfio device status save/restore
vifo: introduce new VFIO ioctl VFIO_IOMMU_GET_DIRTY_BITMAP
hw/vfio/common.c | 32 +++++++++
hw/vfio/pci.c | 164 +++++++++++++++++++++++++++++++++++++++++-
hw/vfio/pci.h | 1 +
include/hw/vfio/vfio-common.h | 1 +
linux-headers/linux/vfio.h | 26 ++++++-
5 files changed, 220 insertions(+), 4 deletions(-)
--
2.7.4
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [Qemu-devel] [RFC V2 0/4] vfio: Introduce Live migration capability to vfio_mdev device
Message-id: 1494316727-15518-1-git-send-email-yulei.zhang@intel.com
Type: series
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
git config --local diff.renamelimit 0
git config --local diff.renames True
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
e72290dd77 vifo: introduce new VFIO ioctl VFIO_IOMMU_GET_DIRTY_BITMAP
e63ec9785b vfio: Add struct vfio_vmstate_info to introduce put/get callback funtion for vfio device status save/restore
196e200d3c vfio: Add vm status change callback to stop/restart the mdev device
bd9ff9bed2 vfio: introduce a new VFIO sub region for mdev device migration support
=== OUTPUT BEGIN ===
Checking PATCH 1/4: vfio: introduce a new VFIO sub region for mdev device migration support...
WARNING: line over 80 characters
#34: FILE: hw/vfio/pci.c:2822:
+ memcpy(&vdev->device_state, device_state, sizeof(struct vfio_region_info));
ERROR: code indent should never use tabs
#34: FILE: hw/vfio/pci.c:2822:
+^Imemcpy(&vdev->device_state, device_state, sizeof(struct vfio_region_info));$
ERROR: code indent should never use tabs
#35: FILE: hw/vfio/pci.c:2823:
+^Ig_free(device_state);$
ERROR: initializer for struct VMStateDescription should normally be const
#47: FILE: hw/vfio/pci.c:3008:
+static VMStateDescription vfio_pci_vmstate = {
total: 3 errors, 1 warnings, 51 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 2/4: vfio: Add vm status change callback to stop/restart the mdev device...
WARNING: line over 80 characters
#49: FILE: hw/vfio/pci.c:2975:
+ if (pwrite(vdev->vbasedev.fd, &dev_state, sz, vdev->device_state.offset) != sz) {
ERROR: Error messages should not contain newlines
#50: FILE: hw/vfio/pci.c:2976:
+ error_report("vfio: Failed to %s device\n", running ? "start" : "stop");
total: 1 errors, 1 warnings, 53 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 3/4: vfio: Add struct vfio_vmstate_info to introduce put/get callback funtion for vfio device status save/restore...
WARNING: line over 80 characters
#33: FILE: hw/vfio/pci.c:2983:
+static int vfio_device_put(QEMUFile *f, void *pv, size_t size, VMStateField *field,
ERROR: spaces required around that '*' (ctx:VxV)
#44: FILE: hw/vfio/pci.c:2994:
+ bar_cfg = pci_default_read_config(pdev, PCI_BASE_ADDRESS_0 + i*4, 4);
^
WARNING: line over 80 characters
#51: FILE: hw/vfio/pci.c:3001:
+ msi_lo = pci_default_read_config(pdev, pdev->msi_cap + PCI_MSI_ADDRESS_LO, 4);
WARNING: line over 80 characters
#55: FILE: hw/vfio/pci.c:3005:
+ msi_hi = pci_default_read_config(pdev, pdev->msi_cap + PCI_MSI_ADDRESS_HI, 4);
WARNING: line over 80 characters
#60: FILE: hw/vfio/pci.c:3010:
+ pdev->msi_cap + (msi_64bit ? PCI_MSI_DATA_64 : PCI_MSI_DATA_32), 2);
ERROR: Error messages should not contain newlines
#65: FILE: hw/vfio/pci.c:3015:
+ error_report("vfio: Failed to allocate memory for migrate\n");
ERROR: Error messages should not contain newlines
#71: FILE: hw/vfio/pci.c:3021:
+ error_report("vfio: Failed to read Device State Region\n");
ERROR: braces {} are necessary for all arms of this statement
#78: FILE: hw/vfio/pci.c:3028:
+ if (buf)
[...]
ERROR: g_free(NULL) is safe this check is probably not required
#79: FILE: hw/vfio/pci.c:3029:
+ if (buf)
+ g_free(buf);
WARNING: line over 80 characters
#84: FILE: hw/vfio/pci.c:3034:
+static int vfio_device_get(QEMUFile *f, void *pv, size_t size, VMStateField *field)
ERROR: spaces required around that '*' (ctx:VxV)
#99: FILE: hw/vfio/pci.c:3049:
+ vfio_pci_write_config(pdev, PCI_BASE_ADDRESS_0 + i*4, bar_cfg, 4);
^
WARNING: line over 80 characters
#117: FILE: hw/vfio/pci.c:3067:
+ vfio_pci_write_config(pdev, pdev->msi_cap + PCI_MSI_ADDRESS_HI, msi_hi, 4);
WARNING: line over 80 characters
#121: FILE: hw/vfio/pci.c:3071:
+ pdev->msi_cap + (msi_64bit ? PCI_MSI_DATA_64 : PCI_MSI_DATA_32),
ERROR: Error messages should not contain newlines
#129: FILE: hw/vfio/pci.c:3079:
+ error_report("vfio: Failed to allocate memory for migrate\n");
ERROR: Error messages should not contain newlines
#136: FILE: hw/vfio/pci.c:3086:
+ error_report("vfio: Failed to write Device State Region\n");
ERROR: braces {} are necessary for all arms of this statement
#140: FILE: hw/vfio/pci.c:3090:
+ if (buf)
[...]
ERROR: code indent should never use tabs
#141: FILE: hw/vfio/pci.c:3091:
+^Ig_free(buf);$
ERROR: g_free(NULL) is safe this check is probably not required
#141: FILE: hw/vfio/pci.c:3091:
+ if (buf)
+ g_free(buf);
ERROR: initializer for struct VMStateInfo should normally be const
#152: FILE: hw/vfio/pci.c:3139:
+static VMStateInfo vfio_vmstate_info = {
total: 12 errors, 7 warnings, 155 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 4/4: vifo: introduce new VFIO ioctl VFIO_IOMMU_GET_DIRTY_BITMAP...
ERROR: braces {} are necessary for all arms of this statement
#37: FILE: hw/vfio/common.c:635:
+ if (vbasedev->device_state == VFIO_DEVICE_START)
[...]
ERROR: code indent should never use tabs
#38: FILE: hw/vfio/common.c:636:
+^I^Ireturn;$
WARNING: line over 80 characters
#44: FILE: hw/vfio/common.c:642:
+ unsigned long bitmap_size = (BITS_TO_LONGS(page_nr) + 1) * sizeof(unsigned long);
ERROR: Error messages should not contain newlines
#50: FILE: hw/vfio/common.c:648:
+ error_report("vfio: Failed to fetch dirty pages for migration\n");
ERROR: line over 90 characters
#54: FILE: hw/vfio/common.c:652:
+ cpu_physical_memory_set_dirty_lebitmap((unsigned long*)&d->dirty_bitmap, d->start_addr, d->page_nr);
ERROR: "(foo*)" should be "(foo *)"
#54: FILE: hw/vfio/common.c:652:
+ cpu_physical_memory_set_dirty_lebitmap((unsigned long*)&d->dirty_bitmap, d->start_addr, d->page_nr);
total: 5 errors, 1 warnings, 67 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
> From: Zhang, Yulei > Sent: Tuesday, May 9, 2017 3:59 PM > > Summary > > This series RFC would like to introduce the live migration capability > to vfio_mdev device. > > As currently vfio_mdev device don't support migration, we introduce > a new vfio subtype region > VFIO_REGION_SUBTYPE_INTEL_IGD_DEVICE_STATE > for Intel vGPU device, during the vfio device initialization, the mdev > device will be set to migratable if the new region exist. Looking at your series, there is really nothing specific to vGPU or even Intel vGPU regarding to device state save/restore... > > The intention to add the new region is using it for vfio_mdev device > status save and restore during the migration. The access to this region > will be trapped and forward to the vfio_mdev device driver. And we use > the first byte in the new region to control the running state of mdev > device. > > Meanwhile we add one new ioctl VFIO_IOMMU_GET_DIRTY_BITMAP to help > do > the mdev device dirty page synchronization. > > So the vfio_mdev device migration sequence would be > Source VM side: > start migration > | > V > get the cpu state change callback, write to the > subregion's first byte to stop the mdev device > | > V > quary the dirty page bitmap from iommu container > and add into qemu dirty list for synchronization > | > V > save the deivce status into Qemufile which is > read from the vfio device subregion > > Target VM side: > restore the mdev device after get the > saved status context from Qemufile > | > V > get the cpu state change callback > write to subregion's first byte to > start the mdev device to put it in > running status > | > V > finish migration > > V1->V2: > Per Alex's suggestion: > 1. use device subtype region instead of VFIO PCI fixed region. > 2. remove unnecessary ioctl, use the first byte of subregion to > control the running state of mdev device. > 3. for dirty page synchronization, implement the interface with > VFIOContainer instead of vfio pci device. > > Yulei Zhang (4): > vfio: introduce a new VFIO sub region for mdev device migration > support > vfio: Add vm status change callback to stop/restart the mdev device > vfio: Add struct vfio_vmstate_info to introduce put/get callback > funtion for vfio device status save/restore > vifo: introduce new VFIO ioctl VFIO_IOMMU_GET_DIRTY_BITMAP > > hw/vfio/common.c | 32 +++++++++ > hw/vfio/pci.c | 164 > +++++++++++++++++++++++++++++++++++++++++- > hw/vfio/pci.h | 1 + > include/hw/vfio/vfio-common.h | 1 + > linux-headers/linux/vfio.h | 26 ++++++- > 5 files changed, 220 insertions(+), 4 deletions(-) > > -- > 2.7.4
> -----Original Message----- > From: Tian, Kevin > Sent: Monday, July 31, 2017 2:55 PM > To: Zhang, Yulei <yulei.zhang@intel.com>; qemu-devel@nongnu.org > Cc: joonas.lahtinen@linux.intel.com; zhenyuw@linux.intel.com; Zheng, Xiao > <xiao.zheng@intel.com>; Wang, Zhi A <zhi.a.wang@intel.com>; > alex.williamson@redhat.com > Subject: RE: [Qemu-devel][RFC V2 0/4] vfio: Introduce Live migration > capability to vfio_mdev device > > > From: Zhang, Yulei > > Sent: Tuesday, May 9, 2017 3:59 PM > > > > Summary > > > > This series RFC would like to introduce the live migration capability > > to vfio_mdev device. > > > > As currently vfio_mdev device don't support migration, we introduce a > > new vfio subtype region > VFIO_REGION_SUBTYPE_INTEL_IGD_DEVICE_STATE > > for Intel vGPU device, during the vfio device initialization, the mdev > > device will be set to migratable if the new region exist. > > Looking at your series, there is really nothing specific to vGPU or even Intel > vGPU regarding to device state save/restore... > You are right, we may define a new subtype for it as it is common region for vfio device state save/restore. > > > > The intention to add the new region is using it for vfio_mdev device > > status save and restore during the migration. The access to this > > region will be trapped and forward to the vfio_mdev device driver. And > > we use the first byte in the new region to control the running state > > of mdev device. > > > > Meanwhile we add one new ioctl VFIO_IOMMU_GET_DIRTY_BITMAP to > help do > > the mdev device dirty page synchronization. > > > > So the vfio_mdev device migration sequence would be Source VM side: > > start migration > > | > > V > > get the cpu state change callback, write to the > > subregion's first byte to stop the mdev device > > | > > V > > quary the dirty page bitmap from iommu container > > and add into qemu dirty list for synchronization > > | > > V > > save the deivce status into Qemufile which is > > read from the vfio device subregion > > > > Target VM side: > > restore the mdev device after get the > > saved status context from Qemufile > > | > > V > > get the cpu state change callback > > write to subregion's first byte to > > start the mdev device to put it in > > running status > > | > > V > > finish migration > > > > V1->V2: > > Per Alex's suggestion: > > 1. use device subtype region instead of VFIO PCI fixed region. > > 2. remove unnecessary ioctl, use the first byte of subregion to > > control the running state of mdev device. > > 3. for dirty page synchronization, implement the interface with > > VFIOContainer instead of vfio pci device. > > > > Yulei Zhang (4): > > vfio: introduce a new VFIO sub region for mdev device migration > > support > > vfio: Add vm status change callback to stop/restart the mdev device > > vfio: Add struct vfio_vmstate_info to introduce put/get callback > > funtion for vfio device status save/restore > > vifo: introduce new VFIO ioctl VFIO_IOMMU_GET_DIRTY_BITMAP > > > > hw/vfio/common.c | 32 +++++++++ > > hw/vfio/pci.c | 164 > > +++++++++++++++++++++++++++++++++++++++++- > > hw/vfio/pci.h | 1 + > > include/hw/vfio/vfio-common.h | 1 + > > linux-headers/linux/vfio.h | 26 ++++++- > > 5 files changed, 220 insertions(+), 4 deletions(-) > > > > -- > > 2.7.4
© 2016 - 2024 Red Hat, Inc.