Messages in this thread | | | Subject | Re: [PATCH v5 02/10] vfio/fsl-mc: Scan DPRC objects on vfio-fsl-mc driver bind | From | Diana Craciun OSS <> | Date | Mon, 5 Oct 2020 20:24:00 +0300 |
| |
On 10/2/2020 8:24 PM, Alex Williamson wrote: > On Tue, 29 Sep 2020 12:03:31 +0300 > Diana Craciun <diana.craciun@oss.nxp.com> wrote: > >> The DPRC (Data Path Resource Container) device is a bus device and has >> child devices attached to it. When the vfio-fsl-mc driver is probed >> the DPRC is scanned and the child devices discovered and initialized. >> >> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com> >> Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com> >> --- >> drivers/vfio/fsl-mc/vfio_fsl_mc.c | 90 +++++++++++++++++++++++ >> drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 1 + >> 2 files changed, 91 insertions(+) >> >> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c >> index a7a483a1e90b..ba44d6d01cc9 100644 >> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c >> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c >> @@ -15,6 +15,8 @@ >> >> #include "vfio_fsl_mc_private.h" >> >> +static struct fsl_mc_driver vfio_fsl_mc_driver; >> + >> static int vfio_fsl_mc_open(void *device_data) >> { >> if (!try_module_get(THIS_MODULE)) >> @@ -84,6 +86,79 @@ static const struct vfio_device_ops vfio_fsl_mc_ops = { >> .mmap = vfio_fsl_mc_mmap, >> }; >> >> +static int vfio_fsl_mc_bus_notifier(struct notifier_block *nb, >> + unsigned long action, void *data) >> +{ >> + struct vfio_fsl_mc_device *vdev = container_of(nb, >> + struct vfio_fsl_mc_device, nb); >> + struct device *dev = data; >> + struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev); >> + struct fsl_mc_device *mc_cont = to_fsl_mc_device(mc_dev->dev.parent); >> + >> + if (action == BUS_NOTIFY_ADD_DEVICE && >> + vdev->mc_dev == mc_cont) { >> + mc_dev->driver_override = kasprintf(GFP_KERNEL, "%s", >> + vfio_fsl_mc_ops.name); >> + if (!mc_dev->driver_override) >> + dev_warn(dev, "VFIO_FSL_MC: Setting driver override for device in dprc %s failed\n", >> + dev_name(&mc_cont->dev)); >> + else >> + dev_info(dev, "VFIO_FSL_MC: Setting driver override for device in dprc %s\n", >> + dev_name(&mc_cont->dev)); > > Nit, some whitespace inconsistencies on the second line of each of > these. I can fixup on commit if we don't find anything else worth a > respin. > >> + } else if (action == BUS_NOTIFY_BOUND_DRIVER && >> + vdev->mc_dev == mc_cont) { >> + struct fsl_mc_driver *mc_drv = to_fsl_mc_driver(dev->driver); >> + >> + if (mc_drv && mc_drv != &vfio_fsl_mc_driver) >> + dev_warn(dev, "VFIO_FSL_MC: Object %s bound to driver %s while DPRC bound to vfio-fsl-mc\n", >> + dev_name(dev), mc_drv->driver.name); >> + } >> + >> + return 0; >> +} >> + >> +static int vfio_fsl_mc_init_device(struct vfio_fsl_mc_device *vdev) >> +{ >> + struct fsl_mc_device *mc_dev = vdev->mc_dev; >> + int ret; >> + >> + /* Non-dprc devices share mc_io from parent */ >> + if (!is_fsl_mc_bus_dprc(mc_dev)) { >> + struct fsl_mc_device *mc_cont = to_fsl_mc_device(mc_dev->dev.parent); >> + >> + mc_dev->mc_io = mc_cont->mc_io; >> + return 0; >> + } >> + >> + vdev->nb.notifier_call = vfio_fsl_mc_bus_notifier; >> + ret = bus_register_notifier(&fsl_mc_bus_type, &vdev->nb); >> + if (ret) >> + return ret; >> + >> + /* open DPRC, allocate a MC portal */ >> + ret = dprc_setup(mc_dev); >> + if (ret) { >> + dev_err(&mc_dev->dev, "VFIO_FSL_MC: Failed to setup DPRC (%d)\n", ret); >> + goto out_nc_unreg; >> + } >> + >> + ret = dprc_scan_container(mc_dev, false); >> + if (ret) { >> + dev_err(&mc_dev->dev, "VFIO_FSL_MC: Container scanning failed (%d)\n", ret); >> + goto out_dprc_cleanup; >> + } > > If I understand this correctly, we've setup the bus notifier to write > the driver override as each sub-devices appear on the bus from this > scan. When non-dprc devices are removed below, it appears we remove all > their sub-devices. Is there a chance here that an error from the scan > leaves residual sub-devices, ie. should we proceed the below > dprc_cleanup() with a call to dprc_remove_devices() to be certain none > remain? Thanks, > > Alex
Right, we should call dprc_remove_devices as well. I will respin another version.
> > >> + >> + return 0; >> + >> +out_dprc_cleanup: >> + dprc_cleanup(mc_dev); >> +out_nc_unreg: >> + bus_unregister_notifier(&fsl_mc_bus_type, &vdev->nb); >> + vdev->nb.notifier_call = NULL; >> + >> + return ret; >> +} >> + >> static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev) >> { >> struct iommu_group *group; >> @@ -110,8 +185,15 @@ static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev) >> dev_err(dev, "VFIO_FSL_MC: Failed to add to vfio group\n"); >> goto out_group_put; >> } >> + >> + ret = vfio_fsl_mc_init_device(vdev); >> + if (ret) >> + goto out_group_dev; >> + >> return 0; >> >> +out_group_dev: >> + vfio_del_group_dev(dev); >> out_group_put: >> vfio_iommu_group_put(group, dev); >> return ret; >> @@ -126,6 +208,14 @@ static int vfio_fsl_mc_remove(struct fsl_mc_device *mc_dev) >> if (!vdev) >> return -EINVAL; >> >> + if (is_fsl_mc_bus_dprc(mc_dev)) { >> + dprc_remove_devices(mc_dev, NULL, 0); >> + dprc_cleanup(mc_dev); >> + } >> + >> + if (vdev->nb.notifier_call) >> + bus_unregister_notifier(&fsl_mc_bus_type, &vdev->nb); >> + >> vfio_iommu_group_put(mc_dev->dev.iommu_group, dev); >> >> return 0; >> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h >> index e79cc116f6b8..37d61eaa58c8 100644 >> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h >> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h >> @@ -9,6 +9,7 @@ >> >> struct vfio_fsl_mc_device { >> struct fsl_mc_device *mc_dev; >> + struct notifier_block nb; >> }; >> >> #endif /* VFIO_FSL_MC_PRIVATE_H */ >
| |