> -----Original Message----- > From: Kevin Tian [mailto:kevin.tian@xxxxxxxxx] > Sent: 27 August 2022 18:10 > To: Zhenyu Wang <zhenyuw@xxxxxxxxxxxxxxx>; Zhi Wang > <zhi.a.wang@xxxxxxxxx>; Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>; Joonas > Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>; Rodrigo Vivi > <rodrigo.vivi@xxxxxxxxx>; Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>; > David Airlie <airlied@xxxxxxxx>; Daniel Vetter <daniel@xxxxxxxx>; Eric Farman > <farman@xxxxxxxxxxxxx>; Matthew Rosato <mjrosato@xxxxxxxxxxxxx>; > Halil Pasic <pasic@xxxxxxxxxxxxx>; Vineeth Vijayan > <vneethv@xxxxxxxxxxxxx>; Peter Oberparleiter <oberpar@xxxxxxxxxxxxx>; > Heiko Carstens <hca@xxxxxxxxxxxxx>; Vasily Gorbik <gor@xxxxxxxxxxxxx>; > Alexander Gordeev <agordeev@xxxxxxxxxxxxx>; Christian Borntraeger > <borntraeger@xxxxxxxxxxxxx>; Sven Schnelle <svens@xxxxxxxxxxxxx>; Tony > Krowiak <akrowiak@xxxxxxxxxxxxx>; Jason Herne <jjherne@xxxxxxxxxxxxx>; > Harald Freudenberger <freude@xxxxxxxxxxxxx>; Diana Craciun > <diana.craciun@xxxxxxxxxxx>; Alex Williamson > <alex.williamson@xxxxxxxxxx>; Cornelia Huck <cohuck@xxxxxxxxxx>; > liulongfang <liulongfang@xxxxxxxxxx>; Shameerali Kolothum Thodi > <shameerali.kolothum.thodi@xxxxxxxxxx>; Jason Gunthorpe > <jgg@xxxxxxxx>; Yishai Hadas <yishaih@xxxxxxxxxx>; Kevin Tian > <kevin.tian@xxxxxxxxx>; Eric Auger <eric.auger@xxxxxxxxxx>; Kirti > Wankhede <kwankhede@xxxxxxxxxx>; Leon Romanovsky <leon@xxxxxxxxxx>; > Abhishek Sahu <abhsahu@xxxxxxxxxx>; intel-gvt-dev@xxxxxxxxxxxxxxxxxxxxx; > intel-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx; > linux-kernel@xxxxxxxxxxxxxxx; linux-s390@xxxxxxxxxxxxxxx; > kvm@xxxxxxxxxxxxxxx > Cc: Yi Liu <yi.l.liu@xxxxxxxxx> > Subject: [PATCH 04/15] vfio/hisi_acc: Use the new device life cycle helpers > > From: Yi Liu <yi.l.liu@xxxxxxxxx> > > Tidy up @probe so all migration specific initialization logic is moved > to migration specific @init callback. > > Remove vfio_pci_core_{un}init_device() given no user now. This looks fine to me and the probe() is indeed much cleaner now. Reviewed-by: Shameer Kolothum <shameerali.kolothum.thodi@xxxxxxxxxx> Thanks, Shameer > Signed-off-by: Yi Liu <yi.l.liu@xxxxxxxxx> > Signed-off-by: Kevin Tian <kevin.tian@xxxxxxxxx> > --- > .../vfio/pci/hisilicon/hisi_acc_vfio_pci.c | 80 +++++++++---------- > drivers/vfio/pci/vfio_pci_core.c | 30 ------- > include/linux/vfio_pci_core.h | 4 - > 3 files changed, 37 insertions(+), 77 deletions(-) > > diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c > b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c > index ea762e28c1cc..f06f9a799128 100644 > --- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c > +++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c > @@ -1213,8 +1213,28 @@ static const struct vfio_migration_ops > hisi_acc_vfio_pci_migrn_state_ops = { > .migration_get_state = hisi_acc_vfio_pci_get_device_state, > }; > > +int hisi_acc_vfio_pci_migrn_init_dev(struct vfio_device *core_vdev) > +{ > + struct hisi_acc_vf_core_device *hisi_acc_vdev = > container_of(core_vdev, > + struct hisi_acc_vf_core_device, core_device.vdev); > + struct pci_dev *pdev = to_pci_dev(core_vdev->dev); > + struct hisi_qm *pf_qm = hisi_acc_get_pf_qm(pdev); > + > + hisi_acc_vdev->vf_id = pci_iov_vf_id(pdev) + 1; > + hisi_acc_vdev->pf_qm = pf_qm; > + hisi_acc_vdev->vf_dev = pdev; > + mutex_init(&hisi_acc_vdev->state_mutex); > + > + core_vdev->migration_flags = VFIO_MIGRATION_STOP_COPY; > + core_vdev->mig_ops = &hisi_acc_vfio_pci_migrn_state_ops; > + > + return vfio_pci_core_init_dev(core_vdev); > +} > + > static const struct vfio_device_ops hisi_acc_vfio_pci_migrn_ops = { > .name = "hisi-acc-vfio-pci-migration", > + .init = hisi_acc_vfio_pci_migrn_init_dev, > + .release = vfio_pci_core_release_dev, > .open_device = hisi_acc_vfio_pci_open_device, > .close_device = hisi_acc_vfio_pci_close_device, > .ioctl = hisi_acc_vfio_pci_ioctl, > @@ -1228,6 +1248,8 @@ static const struct vfio_device_ops > hisi_acc_vfio_pci_migrn_ops = { > > static const struct vfio_device_ops hisi_acc_vfio_pci_ops = { > .name = "hisi-acc-vfio-pci", > + .init = vfio_pci_core_init_dev, > + .release = vfio_pci_core_release_dev, > .open_device = hisi_acc_vfio_pci_open_device, > .close_device = vfio_pci_core_close_device, > .ioctl = vfio_pci_core_ioctl, > @@ -1239,63 +1261,36 @@ static const struct vfio_device_ops > hisi_acc_vfio_pci_ops = { > .match = vfio_pci_core_match, > }; > > -static int > -hisi_acc_vfio_pci_migrn_init(struct hisi_acc_vf_core_device *hisi_acc_vdev, > - struct pci_dev *pdev, struct hisi_qm *pf_qm) > -{ > - int vf_id; > - > - vf_id = pci_iov_vf_id(pdev); > - if (vf_id < 0) > - return vf_id; > - > - hisi_acc_vdev->vf_id = vf_id + 1; > - hisi_acc_vdev->core_device.vdev.migration_flags = > - VFIO_MIGRATION_STOP_COPY; > - hisi_acc_vdev->pf_qm = pf_qm; > - hisi_acc_vdev->vf_dev = pdev; > - mutex_init(&hisi_acc_vdev->state_mutex); > - > - return 0; > -} > - > static int hisi_acc_vfio_pci_probe(struct pci_dev *pdev, const struct > pci_device_id *id) > { > struct hisi_acc_vf_core_device *hisi_acc_vdev; > + const struct vfio_device_ops *ops = &hisi_acc_vfio_pci_ops; > struct hisi_qm *pf_qm; > + int vf_id; > int ret; > > - hisi_acc_vdev = kzalloc(sizeof(*hisi_acc_vdev), GFP_KERNEL); > - if (!hisi_acc_vdev) > - return -ENOMEM; > - > pf_qm = hisi_acc_get_pf_qm(pdev); > if (pf_qm && pf_qm->ver >= QM_HW_V3) { > - ret = hisi_acc_vfio_pci_migrn_init(hisi_acc_vdev, pdev, pf_qm); > - 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 { > + vf_id = pci_iov_vf_id(pdev); > + if (vf_id >= 0) > + ops = &hisi_acc_vfio_pci_migrn_ops; > + else > pci_warn(pdev, "migration support failed, continue with > generic interface\n"); > - vfio_pci_core_init_device(&hisi_acc_vdev->core_device, pdev, > - &hisi_acc_vfio_pci_ops); > - } > - } else { > - vfio_pci_core_init_device(&hisi_acc_vdev->core_device, pdev, > - &hisi_acc_vfio_pci_ops); > } > > + hisi_acc_vdev = vfio_alloc_device(hisi_acc_vf_core_device, > + core_device.vdev, &pdev->dev, ops); > + if (IS_ERR(hisi_acc_vdev)) > + return PTR_ERR(hisi_acc_vdev); > + > dev_set_drvdata(&pdev->dev, &hisi_acc_vdev->core_device); > ret = vfio_pci_core_register_device(&hisi_acc_vdev->core_device); > if (ret) > - goto out_free; > + goto out_put_vdev; > return 0; > > -out_free: > - vfio_pci_core_uninit_device(&hisi_acc_vdev->core_device); > - kfree(hisi_acc_vdev); > +out_put_vdev: > + vfio_put_device(&hisi_acc_vdev->core_device.vdev); > return ret; > } > > @@ -1304,8 +1299,7 @@ static void hisi_acc_vfio_pci_remove(struct > pci_dev *pdev) > struct hisi_acc_vf_core_device *hisi_acc_vdev = > hssi_acc_drvdata(pdev); > > vfio_pci_core_unregister_device(&hisi_acc_vdev->core_device); > - vfio_pci_core_uninit_device(&hisi_acc_vdev->core_device); > - kfree(hisi_acc_vdev); > + vfio_put_device(&hisi_acc_vdev->core_device.vdev); > } > > static const struct pci_device_id hisi_acc_vfio_pci_table[] = { > diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c > index 708b61d1b364..f29d780e327e 100644 > --- a/drivers/vfio/pci/vfio_pci_core.c > +++ b/drivers/vfio/pci/vfio_pci_core.c > @@ -1860,36 +1860,6 @@ void vfio_pci_core_release_dev(struct > vfio_device *core_vdev) > } > EXPORT_SYMBOL_GPL(vfio_pci_core_release_dev); > > -void vfio_pci_core_init_device(struct vfio_pci_core_device *vdev, > - struct pci_dev *pdev, > - const struct vfio_device_ops *vfio_pci_ops) > -{ > - vfio_init_group_dev(&vdev->vdev, &pdev->dev, vfio_pci_ops); > - vdev->pdev = pdev; > - vdev->irq_type = VFIO_PCI_NUM_IRQS; > - mutex_init(&vdev->igate); > - spin_lock_init(&vdev->irqlock); > - mutex_init(&vdev->ioeventfds_lock); > - INIT_LIST_HEAD(&vdev->dummy_resources_list); > - INIT_LIST_HEAD(&vdev->ioeventfds_list); > - mutex_init(&vdev->vma_lock); > - INIT_LIST_HEAD(&vdev->vma_list); > - INIT_LIST_HEAD(&vdev->sriov_pfs_item); > - init_rwsem(&vdev->memory_lock); > -} > -EXPORT_SYMBOL_GPL(vfio_pci_core_init_device); > - > -void vfio_pci_core_uninit_device(struct vfio_pci_core_device *vdev) > -{ > - mutex_destroy(&vdev->igate); > - mutex_destroy(&vdev->ioeventfds_lock); > - mutex_destroy(&vdev->vma_lock); > - vfio_uninit_group_dev(&vdev->vdev); > - kfree(vdev->region); > - kfree(vdev->pm_save); > -} > -EXPORT_SYMBOL_GPL(vfio_pci_core_uninit_device); > - > int vfio_pci_core_register_device(struct vfio_pci_core_device *vdev) > { > struct pci_dev *pdev = vdev->pdev; > diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h > index 98c8c66e2400..9f10ff34b2ba 100644 > --- a/include/linux/vfio_pci_core.h > +++ b/include/linux/vfio_pci_core.h > @@ -230,13 +230,9 @@ static inline void vfio_pci_zdev_close_device(struct > vfio_pci_core_device *vdev) > void vfio_pci_core_set_params(bool nointxmask, bool is_disable_vga, > bool is_disable_idle_d3); > void vfio_pci_core_close_device(struct vfio_device *core_vdev); > -void vfio_pci_core_init_device(struct vfio_pci_core_device *vdev, > - struct pci_dev *pdev, > - const struct vfio_device_ops *vfio_pci_ops); > int vfio_pci_core_init_dev(struct vfio_device *core_vdev); > void vfio_pci_core_release_dev(struct vfio_device *core_vdev); > int vfio_pci_core_register_device(struct vfio_pci_core_device *vdev); > -void vfio_pci_core_uninit_device(struct vfio_pci_core_device *vdev); > void vfio_pci_core_unregister_device(struct vfio_pci_core_device *vdev); > extern const struct pci_error_handlers vfio_pci_core_err_handlers; > int vfio_pci_core_sriov_configure(struct vfio_pci_core_device *vdev, > -- > 2.21.3