Hi,
The optional PRE_COPY state support for migration feature is introduced
by this series here[1](Thanks for that!). This is something HiSilicon
ACC driver can make use for early dev compatibility checks and have
made an attempt in the past here[2].
Adding this will speed up reporting the compatibility error early in
the migration process without the need to wait for complete data
transfer. This is sanity tested on a HiSilicon Platform.
Please take a look.
Regards,
Shameer
1.https://lore.kernel.org/all/[email protected]/
2.https://lore.kernel.org/kvm/[email protected]/
Shameer Kolothum (4):
hisi_acc_vfio_pci: Add support for precopy IOCTL
hisi_acc_vfio_pci: Introduce support for PRE_COPY state transitions
hisi_acc_vfio_pci: Move the dev compatibility tests for early check
hisi_acc_vfio_pci: Enable PRE_COPY flag
.../vfio/pci/hisilicon/hisi_acc_vfio_pci.c | 147 ++++++++++++++++--
.../vfio/pci/hisilicon/hisi_acc_vfio_pci.h | 2 +
2 files changed, 133 insertions(+), 16 deletions(-)
--
2.34.1
The saving_migf is open in PRE_COPY state if it is supported and reads
initial device match data. hisi_acc_vf_stop_copy() is refactored to
make use of common code.
Signed-off-by: Shameer Kolothum <[email protected]>
---
.../vfio/pci/hisilicon/hisi_acc_vfio_pci.c | 74 ++++++++++++++++++-
1 file changed, 71 insertions(+), 3 deletions(-)
diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
index f3b74a06edb6..c8658636a84c 100644
--- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
+++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
@@ -863,7 +863,7 @@ static const struct file_operations hisi_acc_vf_save_fops = {
};
static struct hisi_acc_vf_migration_file *
-hisi_acc_vf_stop_copy(struct hisi_acc_vf_core_device *hisi_acc_vdev)
+hisi_acc_open_saving_migf(struct hisi_acc_vf_core_device *hisi_acc_vdev)
{
struct hisi_acc_vf_migration_file *migf;
int ret;
@@ -885,7 +885,7 @@ hisi_acc_vf_stop_copy(struct hisi_acc_vf_core_device *hisi_acc_vdev)
mutex_init(&migf->lock);
migf->hisi_acc_vdev = hisi_acc_vdev;
- ret = vf_qm_state_save(hisi_acc_vdev, migf);
+ ret = vf_qm_get_match_data(hisi_acc_vdev, &migf->vf_data);
if (ret) {
fput(migf->filp);
return ERR_PTR(ret);
@@ -894,6 +894,44 @@ hisi_acc_vf_stop_copy(struct hisi_acc_vf_core_device *hisi_acc_vdev)
return migf;
}
+static struct hisi_acc_vf_migration_file *
+hisi_acc_vf_pre_copy(struct hisi_acc_vf_core_device *hisi_acc_vdev)
+{
+ struct hisi_acc_vf_migration_file *migf;
+
+ migf = hisi_acc_open_saving_migf(hisi_acc_vdev);
+ if (IS_ERR(migf))
+ return migf;
+
+ migf->total_length = QM_MATCH_SIZE;
+ return migf;
+}
+
+static struct hisi_acc_vf_migration_file *
+hisi_acc_vf_stop_copy(struct hisi_acc_vf_core_device *hisi_acc_vdev, bool open)
+{
+ int ret;
+ struct hisi_acc_vf_migration_file *migf = NULL;
+
+ if (open) {
+ /*
+ * Userspace didn't use PRECOPY support. Hence saving_migf
+ * is not opened yet.
+ */
+ migf = hisi_acc_open_saving_migf(hisi_acc_vdev);
+ if (IS_ERR(migf))
+ return migf;
+ } else {
+ migf = hisi_acc_vdev->saving_migf;
+ }
+
+ ret = vf_qm_state_save(hisi_acc_vdev, migf);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return open ? migf : NULL;
+}
+
static int hisi_acc_vf_stop_device(struct hisi_acc_vf_core_device *hisi_acc_vdev)
{
struct device *dev = &hisi_acc_vdev->vf_dev->dev;
@@ -921,6 +959,31 @@ hisi_acc_vf_set_device_state(struct hisi_acc_vf_core_device *hisi_acc_vdev,
u32 cur = hisi_acc_vdev->mig_state;
int ret;
+ if (cur == VFIO_DEVICE_STATE_RUNNING && new == VFIO_DEVICE_STATE_PRE_COPY) {
+ struct hisi_acc_vf_migration_file *migf;
+
+ migf = hisi_acc_vf_pre_copy(hisi_acc_vdev);
+ if (IS_ERR(migf))
+ return ERR_CAST(migf);
+ get_file(migf->filp);
+ hisi_acc_vdev->saving_migf = migf;
+ return migf->filp;
+ }
+
+ if (cur == VFIO_DEVICE_STATE_PRE_COPY && new == VFIO_DEVICE_STATE_STOP_COPY) {
+ struct hisi_acc_vf_migration_file *migf;
+
+ ret = hisi_acc_vf_stop_device(hisi_acc_vdev);
+ if (ret)
+ return ERR_PTR(ret);
+
+ migf = hisi_acc_vf_stop_copy(hisi_acc_vdev, false);
+ if (IS_ERR(migf))
+ return ERR_CAST(migf);
+
+ return NULL;
+ }
+
if (cur == VFIO_DEVICE_STATE_RUNNING && new == VFIO_DEVICE_STATE_STOP) {
ret = hisi_acc_vf_stop_device(hisi_acc_vdev);
if (ret)
@@ -931,7 +994,7 @@ hisi_acc_vf_set_device_state(struct hisi_acc_vf_core_device *hisi_acc_vdev,
if (cur == VFIO_DEVICE_STATE_STOP && new == VFIO_DEVICE_STATE_STOP_COPY) {
struct hisi_acc_vf_migration_file *migf;
- migf = hisi_acc_vf_stop_copy(hisi_acc_vdev);
+ migf = hisi_acc_vf_stop_copy(hisi_acc_vdev, true);
if (IS_ERR(migf))
return ERR_CAST(migf);
get_file(migf->filp);
@@ -963,6 +1026,11 @@ hisi_acc_vf_set_device_state(struct hisi_acc_vf_core_device *hisi_acc_vdev,
return NULL;
}
+ if (cur == VFIO_DEVICE_STATE_PRE_COPY && new == VFIO_DEVICE_STATE_RUNNING) {
+ hisi_acc_vf_disable_fds(hisi_acc_vdev);
+ return NULL;
+ }
+
if (cur == VFIO_DEVICE_STATE_STOP && new == VFIO_DEVICE_STATE_RUNNING) {
hisi_acc_vf_start_device(hisi_acc_vdev);
return NULL;
--
2.34.1
Now that we have everything to support the PRE_COPY state,
enable it.
Signed-off-by: Shameer Kolothum <[email protected]>
---
drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
index 9a51f41e1d2a..51941bb4f31f 100644
--- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
+++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
@@ -1351,7 +1351,7 @@ static int hisi_acc_vfio_pci_migrn_init_dev(struct vfio_device *core_vdev)
hisi_acc_vdev->vf_dev = pdev;
mutex_init(&hisi_acc_vdev->state_mutex);
- core_vdev->migration_flags = VFIO_MIGRATION_STOP_COPY;
+ core_vdev->migration_flags = VFIO_MIGRATION_STOP_COPY | VFIO_MIGRATION_PRE_COPY;
core_vdev->mig_ops = &hisi_acc_vfio_pci_migrn_state_ops;
return vfio_pci_core_init_dev(core_vdev);
--
2.34.1
Instead of waiting till data transfer is complete to perform dev
compatibility, do it as soon as we have enough data to perform the
check. This will be useful when we enable the support for PRE_COPY.
Signed-off-by: Shameer Kolothum <[email protected]>
---
.../vfio/pci/hisilicon/hisi_acc_vfio_pci.c | 19 +++++++------------
.../vfio/pci/hisilicon/hisi_acc_vfio_pci.h | 1 +
2 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
index c8658636a84c..9a51f41e1d2a 100644
--- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
+++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
@@ -360,8 +360,8 @@ static int vf_qm_check_match(struct hisi_acc_vf_core_device *hisi_acc_vdev,
u32 que_iso_state;
int ret;
- if (migf->total_length < QM_MATCH_SIZE)
- return -EINVAL;
+ if (migf->total_length < QM_MATCH_SIZE || hisi_acc_vdev->match_done)
+ return 0;
if (vf_data->acc_magic != ACC_DEV_MAGIC) {
dev_err(dev, "failed to match ACC_DEV_MAGIC\n");
@@ -406,6 +406,7 @@ static int vf_qm_check_match(struct hisi_acc_vf_core_device *hisi_acc_vdev,
}
hisi_acc_vdev->vf_qm_state = vf_data->vf_qm_state;
+ hisi_acc_vdev->match_done = true;
return 0;
}
@@ -493,10 +494,6 @@ static int vf_qm_state_save(struct hisi_acc_vf_core_device *hisi_acc_vdev,
struct device *dev = &vf_qm->pdev->dev;
int ret;
- ret = vf_qm_get_match_data(hisi_acc_vdev, vf_data);
- if (ret)
- return ret;
-
if (unlikely(qm_wait_dev_not_ready(vf_qm))) {
/* Update state and return with match data */
vf_data->vf_qm_state = QM_NOT_READY;
@@ -673,12 +670,6 @@ static int hisi_acc_vf_load_state(struct hisi_acc_vf_core_device *hisi_acc_vdev)
struct hisi_acc_vf_migration_file *migf = hisi_acc_vdev->resuming_migf;
int ret;
- /* Check dev compatibility */
- ret = vf_qm_check_match(hisi_acc_vdev, migf);
- if (ret) {
- dev_err(dev, "failed to match the VF!\n");
- return ret;
- }
/* Recover data to VF */
ret = vf_qm_load_data(hisi_acc_vdev, migf);
if (ret) {
@@ -732,6 +723,10 @@ static ssize_t hisi_acc_vf_resume_write(struct file *filp, const char __user *bu
*pos += len;
done = len;
migf->total_length += len;
+
+ ret = vf_qm_check_match(migf->hisi_acc_vdev, migf);
+ if (ret)
+ done = -EFAULT;
out_unlock:
mutex_unlock(&migf->lock);
return done;
diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h
index 11d51345f5b5..dcabfeec6ca1 100644
--- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h
+++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h
@@ -98,6 +98,7 @@ struct hisi_acc_vf_migration_file {
struct hisi_acc_vf_core_device {
struct vfio_pci_core_device core_device;
+ u8 match_done:1;
u8 deferred_reset:1;
/* For migration state */
struct mutex state_mutex;
--
2.34.1
Hi Shameer,
I love your patch! Yet something to improve:
[auto build test ERROR on awilliam-vfio/for-linus]
[also build test ERROR on linus/master v6.1-rc7 next-20221202]
[cannot apply to awilliam-vfio/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Shameer-Kolothum/hisi_acc_vfio_pci-Add-PRE_COPY-migration-feature-support/20221123-193542
base: https://github.com/awilliam/linux-vfio.git for-linus
patch link: https://lore.kernel.org/r/20221123113236.896-5-shameerali.kolothum.thodi%40huawei.com
patch subject: [PATCH 4/4] hisi_acc_vfio_pci: Enable PRE_COPY flag
config: arm64-allyesconfig
compiler: aarch64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/0a0337a53f36cbe7185cefb1365816476e10bdf7
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Shameer-Kolothum/hisi_acc_vfio_pci-Add-PRE_COPY-migration-feature-support/20221123-193542
git checkout 0a0337a53f36cbe7185cefb1365816476e10bdf7
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c: In function 'hisi_acc_vf_precopy_ioctl':
drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c:772:34: error: storage size of 'info' isn't known
772 | struct vfio_precopy_info info;
| ^~~~
drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c:776:20: error: 'VFIO_MIG_GET_PRECOPY_INFO' undeclared (first use in this function)
776 | if (cmd != VFIO_MIG_GET_PRECOPY_INFO)
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c:776:20: note: each undeclared identifier is reported only once for each function it appears in
In file included from include/uapi/linux/posix_types.h:5,
from include/uapi/linux/types.h:14,
from include/linux/types.h:6,
from include/linux/kasan-checks.h:5,
from include/asm-generic/rwonce.h:26,
from arch/arm64/include/asm/rwonce.h:71,
from include/linux/compiler.h:246,
from include/linux/dev_printk.h:14,
from include/linux/device.h:15,
from drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c:6:
include/linux/stddef.h:16:33: error: invalid use of undefined type 'struct vfio_precopy_info'
16 | #define offsetof(TYPE, MEMBER) __builtin_offsetof(TYPE, MEMBER)
| ^~~~~~~~~~~~~~~~~~
include/linux/stddef.h:33:10: note: in expansion of macro 'offsetof'
33 | (offsetof(TYPE, MEMBER) + sizeof_field(TYPE, MEMBER))
| ^~~~~~~~
drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c:779:17: note: in expansion of macro 'offsetofend'
779 | minsz = offsetofend(struct vfio_precopy_info, dirty_bytes);
| ^~~~~~~~~~~
include/linux/stddef.h:24:55: error: invalid use of undefined type 'struct vfio_precopy_info'
24 | #define sizeof_field(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER))
| ^~
include/linux/stddef.h:33:35: note: in expansion of macro 'sizeof_field'
33 | (offsetof(TYPE, MEMBER) + sizeof_field(TYPE, MEMBER))
| ^~~~~~~~~~~~
drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c:779:17: note: in expansion of macro 'offsetofend'
779 | minsz = offsetofend(struct vfio_precopy_info, dirty_bytes);
| ^~~~~~~~~~~
drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c:787:41: error: 'VFIO_DEVICE_STATE_PRE_COPY' undeclared (first use in this function); did you mean 'VFIO_DEVICE_STATE_STOP_COPY'?
787 | if (hisi_acc_vdev->mig_state != VFIO_DEVICE_STATE_PRE_COPY) {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
| VFIO_DEVICE_STATE_STOP_COPY
drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c:772:34: warning: unused variable 'info' [-Wunused-variable]
772 | struct vfio_precopy_info info;
| ^~~~
drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c: In function 'hisi_acc_vf_set_device_state':
drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c:957:56: error: 'VFIO_DEVICE_STATE_PRE_COPY' undeclared (first use in this function); did you mean 'VFIO_DEVICE_STATE_STOP_COPY'?
957 | if (cur == VFIO_DEVICE_STATE_RUNNING && new == VFIO_DEVICE_STATE_PRE_COPY) {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
| VFIO_DEVICE_STATE_STOP_COPY
drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c: In function 'hisi_acc_vfio_pci_migrn_init_dev':
>> drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c:1345:65: error: 'VFIO_MIGRATION_PRE_COPY' undeclared (first use in this function); did you mean 'VFIO_MIGRATION_STOP_COPY'?
1345 | core_vdev->migration_flags = VFIO_MIGRATION_STOP_COPY | VFIO_MIGRATION_PRE_COPY;
| ^~~~~~~~~~~~~~~~~~~~~~~
| VFIO_MIGRATION_STOP_COPY
vim +1345 drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
1332
1333 static int hisi_acc_vfio_pci_migrn_init_dev(struct vfio_device *core_vdev)
1334 {
1335 struct hisi_acc_vf_core_device *hisi_acc_vdev = container_of(core_vdev,
1336 struct hisi_acc_vf_core_device, core_device.vdev);
1337 struct pci_dev *pdev = to_pci_dev(core_vdev->dev);
1338 struct hisi_qm *pf_qm = hisi_acc_get_pf_qm(pdev);
1339
1340 hisi_acc_vdev->vf_id = pci_iov_vf_id(pdev) + 1;
1341 hisi_acc_vdev->pf_qm = pf_qm;
1342 hisi_acc_vdev->vf_dev = pdev;
1343 mutex_init(&hisi_acc_vdev->state_mutex);
1344
> 1345 core_vdev->migration_flags = VFIO_MIGRATION_STOP_COPY | VFIO_MIGRATION_PRE_COPY;
1346 core_vdev->mig_ops = &hisi_acc_vfio_pci_migrn_state_ops;
1347
1348 return vfio_pci_core_init_dev(core_vdev);
1349 }
1350
--
0-DAY CI Kernel Test Service
https://01.org/lkp
On Wed, 23 Nov 2022 11:32:32 +0000
Shameer Kolothum <[email protected]> wrote:
> Hi,
>
> The optional PRE_COPY state support for migration feature is introduced
> by this series here[1](Thanks for that!). This is something HiSilicon
> ACC driver can make use for early dev compatibility checks and have
> made an attempt in the past here[2].
>
> Adding this will speed up reporting the compatibility error early in
> the migration process without the need to wait for complete data
> transfer. This is sanity tested on a HiSilicon Platform.
>
> Please take a look.
>
> Regards,
> Shameer
>
> 1.https://lore.kernel.org/all/[email protected]/
> 2.https://lore.kernel.org/kvm/[email protected]/
>
> Shameer Kolothum (4):
> hisi_acc_vfio_pci: Add support for precopy IOCTL
> hisi_acc_vfio_pci: Introduce support for PRE_COPY state transitions
> hisi_acc_vfio_pci: Move the dev compatibility tests for early check
> hisi_acc_vfio_pci: Enable PRE_COPY flag
>
> .../vfio/pci/hisilicon/hisi_acc_vfio_pci.c | 147 ++++++++++++++++--
> .../vfio/pci/hisilicon/hisi_acc_vfio_pci.h | 2 +
> 2 files changed, 133 insertions(+), 16 deletions(-)
Applied to vfio next branch for v6.2. Thanks,
Alex