> -----Original Message----- > From: Yishai Hadas [mailto:yishaih@xxxxxxxxxx] > Sent: 22 May 2022 16:04 > To: kernel test robot <lkp@xxxxxxxxx>; alex.williamson@xxxxxxxxxx; > jgg@xxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; Shameerali Kolothum Thodi > <shameerali.kolothum.thodi@xxxxxxxxxx> > Cc: kbuild-all@xxxxxxxxxxxx; maorg@xxxxxxxxxx; cohuck@xxxxxxxxxx; > kevin.tian@xxxxxxxxx; liulongfang <liulongfang@xxxxxxxxxx> > Subject: Re: [PATCH] vfio: Split migration ops from main device ops > > > Alex, > This patch requires some extra handling in hisi driver to fix and > encapsulate the migration specific handling in a single function per op, > which at the end will call the matching vfio_pci_core_xxx function. > This won't fit to current kernel cycle as the merge window is almost > here, however your feedback on the below approach would be appreciated. > > Shameerali, > Can you please review the below and send me some matching code in your > driver ? I may put as part of V1, unless that you prefer that I'll do. Ok. I think for HiSilicon driver you require the below changes instead, --x--- diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c index 4def43f5f7b6..b42964f5d7ee 100644 --- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c +++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c @@ -1185,7 +1185,7 @@ static int hisi_acc_vfio_pci_open_device(struct vfio_device *core_vdev) if (ret) return ret; - if (core_vdev->ops->migration_set_state) { + if (core_vdev->mig_ops) { ret = hisi_acc_vf_qm_init(hisi_acc_vdev); if (ret) { vfio_pci_core_disable(vdev); @@ -1208,6 +1208,11 @@ static void hisi_acc_vfio_pci_close_device(struct vfio_device *core_vdev) vfio_pci_core_close_device(core_vdev); } +static const struct vfio_migration_ops hisi_acc_vfio_pci_migrn_state_ops = { + .migration_set_state = hisi_acc_vfio_pci_set_device_state, + .migration_get_state = hisi_acc_vfio_pci_get_device_state, +}; + static const struct vfio_device_ops hisi_acc_vfio_pci_migrn_ops = { .name = "hisi-acc-vfio-pci-migration", .open_device = hisi_acc_vfio_pci_open_device, @@ -1219,8 +1224,6 @@ static const struct vfio_device_ops hisi_acc_vfio_pci_migrn_ops = { .mmap = hisi_acc_vfio_pci_mmap, .request = vfio_pci_core_request, .match = vfio_pci_core_match, - .migration_set_state = hisi_acc_vfio_pci_set_device_state, - .migration_get_state = hisi_acc_vfio_pci_get_device_state, }; static const struct vfio_device_ops hisi_acc_vfio_pci_ops = { @@ -1272,6 +1275,8 @@ static int hisi_acc_vfio_pci_probe(struct pci_dev *pdev, const struct pci_device if (!ret) { vfio_pci_core_init_device(&hisi_acc_vdev->core_device, pdev, &hisi_acc_vfio_pci_migrn_ops); + hisi_acc_vdev->core_device.vdev.mig_ops = + &hisi_acc_vfio_pci_migrn_state_ops; } else { pci_warn(pdev, "migration support failed, continue with generic interface\n"); vfio_pci_core_init_device(&hisi_acc_vdev->core_device, pdev, --x--- Thanks, Shameer > On 22/05/2022 17:21, kernel test robot wrote: > > Hi Yishai, > > > > I love your patch! Yet something to improve: > > > > [auto build test ERROR on awilliam-vfio/next] > > [cannot apply to v5.18-rc7] > > [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] > > > > url: > https://github.com/intel-lab-lkp/linux/commits/Yishai-Hadas/vfio-Split-migrati > on-ops-from-main-device-ops/20220522-174959 > > base: https://github.com/awilliam/linux-vfio.git next > > config: arm64-allyesconfig > (https://download.01.org/0day-ci/archive/20220522/202205222209.5JkbCw > Da-lkp@xxxxxxxxx/config) > > compiler: aarch64-linux-gcc (GCC) 11.3.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/f9fa522b20c805dbbb0907b0f9 > 0b2b7f1d260218 > > git remote add linux-review https://github.com/intel-lab-lkp/linux > > git fetch --no-tags linux-review > Yishai-Hadas/vfio-Split-migration-ops-from-main-device-ops/20220522-17495 > 9 > > git checkout f9fa522b20c805dbbb0907b0f90b2b7f1d260218 > > # save the config file > > mkdir build_dir && cp config build_dir/.config > > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.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 <lkp@xxxxxxxxx> > > > > All errors (new ones prefixed by >>): > > > > drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c: In function > 'hisi_acc_vfio_pci_open_device': > >>> drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c:1188:27: error: 'const struct > vfio_device_ops' has no member named 'migration_set_state' > > 1188 | if (core_vdev->ops->migration_set_state) { > > | ^~ > > At top level: > > drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c:1201:13: warning: > 'hisi_acc_vfio_pci_close_device' defined but not used [-Wunused-function] > > 1201 | static void hisi_acc_vfio_pci_close_device(struct vfio_device > *core_vdev) > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c:1138:13: warning: > 'hisi_acc_vfio_pci_ioctl' defined but not used [-Wunused-function] > > 1138 | static long hisi_acc_vfio_pci_ioctl(struct vfio_device > *core_vdev, unsigned int cmd, > > | ^~~~~~~~~~~~~~~~~~~~~~~ > > drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c:1124:16: warning: > 'hisi_acc_vfio_pci_read' defined but not used [-Wunused-function] > > 1124 | static ssize_t hisi_acc_vfio_pci_read(struct vfio_device > *core_vdev, > > | ^~~~~~~~~~~~~~~~~~~~~~ > > drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c:1110:16: warning: > 'hisi_acc_vfio_pci_write' defined but not used [-Wunused-function] > > 1110 | static ssize_t hisi_acc_vfio_pci_write(struct vfio_device > *core_vdev, > > | ^~~~~~~~~~~~~~~~~~~~~~~ > > drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c:1086:12: warning: > 'hisi_acc_vfio_pci_mmap' defined but not used [-Wunused-function] > > 1086 | static int hisi_acc_vfio_pci_mmap(struct vfio_device > *core_vdev, > > | ^~~~~~~~~~~~~~~~~~~~~~ > > > > > > vim +1188 drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c > > > > 6abdce51af1a21 Shameer Kolothum 2022-03-08 1176 > > ee3a5b2359e0e5 Shameer Kolothum 2022-03-08 1177 static int > hisi_acc_vfio_pci_open_device(struct vfio_device *core_vdev) > > ee3a5b2359e0e5 Shameer Kolothum 2022-03-08 1178 { > > b0eed085903e77 Longfang Liu 2022-03-08 1179 struct > hisi_acc_vf_core_device *hisi_acc_vdev = container_of(core_vdev, > > b0eed085903e77 Longfang Liu 2022-03-08 1180 > struct hisi_acc_vf_core_device, core_device.vdev); > > b0eed085903e77 Longfang Liu 2022-03-08 1181 struct > vfio_pci_core_device *vdev = &hisi_acc_vdev->core_device; > > ee3a5b2359e0e5 Shameer Kolothum 2022-03-08 1182 int ret; > > ee3a5b2359e0e5 Shameer Kolothum 2022-03-08 1183 > > ee3a5b2359e0e5 Shameer Kolothum 2022-03-08 1184 ret = > vfio_pci_core_enable(vdev); > > ee3a5b2359e0e5 Shameer Kolothum 2022-03-08 1185 if (ret) > > ee3a5b2359e0e5 Shameer Kolothum 2022-03-08 1186 return > ret; > > ee3a5b2359e0e5 Shameer Kolothum 2022-03-08 1187 > > b0eed085903e77 Longfang Liu 2022-03-08 @1188 if > (core_vdev->ops->migration_set_state) { > > b0eed085903e77 Longfang Liu 2022-03-08 1189 ret = > hisi_acc_vf_qm_init(hisi_acc_vdev); > > b0eed085903e77 Longfang Liu 2022-03-08 1190 if (ret) { > > b0eed085903e77 Longfang Liu 2022-03-08 1191 > vfio_pci_core_disable(vdev); > > b0eed085903e77 Longfang Liu 2022-03-08 1192 > return ret; > > b0eed085903e77 Longfang Liu 2022-03-08 1193 } > > b0eed085903e77 Longfang Liu 2022-03-08 1194 > hisi_acc_vdev->mig_state = VFIO_DEVICE_STATE_RUNNING; > > b0eed085903e77 Longfang Liu 2022-03-08 1195 } > > ee3a5b2359e0e5 Shameer Kolothum 2022-03-08 1196 > > b0eed085903e77 Longfang Liu 2022-03-08 1197 > vfio_pci_core_finish_enable(vdev); > > ee3a5b2359e0e5 Shameer Kolothum 2022-03-08 1198 return 0; > > ee3a5b2359e0e5 Shameer Kolothum 2022-03-08 1199 } > > ee3a5b2359e0e5 Shameer Kolothum 2022-03-08 1200 > >