> From: Shameerali Kolothum Thodi > <shameerali.kolothum.thodi@xxxxxxxxxx> > Sent: Tuesday, March 8, 2022 4:53 PM > > > + } 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); > > > + } > > > > This logic looks weird. Earlier you state that the migration control region > must > > be hidden from the userspace as a security requirement, but above logic > reads > > like if the driver fails to initialize migration support then we just fall back to > the > > default ops which grants the user the full access to the entire MMIO bar. > > As I explained previously the risk of exposing migration BAR is only limited to > migration > use case. So if for some reason we can't get the migration working, we > default to the > generic vfio-pci like behavior. Yes, as long as there is guarantee that exposing such region doesn't allow the guest to escape any configuration enforced by the PF driver. > > > > > > + } else { > > > + vfio_pci_core_init_device(&hisi_acc_vdev->core_device, pdev, > > > + &hisi_acc_vfio_pci_ops); > > > + } > > > > If the hardware itself doesn't support the migration capability, can we just > > move it out of the id table and let vfio-pci to drive it? > > > But the above is just like vfio-pci driving it, right? > Yes, earlier I thought this driver may only want to drive devices which support migration capability. But my opinion on this is not strong and having it drive all acc devices is not harmful anyway... Thanks Kevin