Messages in this thread | | | Date | Fri, 14 Apr 2023 09:24:43 -0300 | From | Jason Gunthorpe <> | Subject | Re: [PATCH v10 3/5] hisi_acc_vfio_pci: register debugfs for hisilicon migration driver |
| |
On Sat, Apr 08, 2023 at 03:42:22PM +0800, Longfang Liu wrote: > +static int hisi_acc_vf_debug_restore(struct seq_file *seq, void *data) > +{ > + struct device *vf_dev = seq->private; > + struct vfio_pci_core_device *core_device = dev_get_drvdata(vf_dev); > + struct vfio_device *vdev = &core_device->vdev; > + struct hisi_acc_vf_core_device *hisi_acc_vdev = hisi_acc_get_vf_dev(vdev); > + struct hisi_acc_vf_migration_file *migf = hisi_acc_vdev->debug_migf; > + int ret; > + > + ret = hisi_acc_vf_debug_check(seq, vdev); > + if (ret) > + goto restore_err; > + > + ret = vf_qm_state_save(hisi_acc_vdev, migf); > + if (ret) { > + seq_printf(seq, "%s\n", "failed to save device data!"); > + goto restore_err; > + } > + > + ret = vf_qm_check_match(hisi_acc_vdev, migf); > + if (ret) { > + seq_printf(seq, "%s\n", "failed to match the VF!"); > + goto restore_err; > + } > + > + ret = vf_qm_load_data(hisi_acc_vdev, migf); > + if (ret) { > + seq_printf(seq, "%s\n", "failed to recover the VF!"); > + goto restore_err; > + } > + > + vf_qm_fun_reset(&hisi_acc_vdev->vf_qm); > + seq_printf(seq, "%s\n", "successful to resume device data!"); > + > +restore_err: > + return 0; > +}
This is basically an in-kernel self test, it should be protected with some kind of VFIO selftest kconfig.
Though, I wonder why we need it??? Can't you write a trivial userspace program under tools/testing to do this sequence with the ioctls?
> +static int hisi_acc_vf_debug_save(struct seq_file *seq, void *data) > +{ > + struct device *vf_dev = seq->private; > + struct vfio_pci_core_device *core_device = dev_get_drvdata(vf_dev); > + struct vfio_device *vdev = &core_device->vdev; > + struct hisi_acc_vf_core_device *hisi_acc_vdev = hisi_acc_get_vf_dev(vdev); > + struct hisi_acc_vf_migration_file *migf = hisi_acc_vdev->debug_migf; > + int ret; > + > + ret = hisi_acc_vf_debug_check(seq, vdev); > + if (ret) > + goto save_err; > + > + ret = vf_qm_state_save(hisi_acc_vdev, migf); > + if (ret) { > + seq_printf(seq, "%s\n", "failed to save device data!"); > + goto save_err; > + } > + seq_printf(seq, "%s\n", "successful to save device data!"); > + > +save_err: > + return 0; > +}
Same kind of commen there, this is a selftest, why does it need a special kernel interface?
.. and so on..
I thought the non-selftesty bits were OK, maybe split the patch to match progress
Jason
| |