Re: [PATCHv6 05/13] iommu/core: add ops->{bound,unbind}_driver()

[Hiroshi Doyu <hdoyu@nvidia.com>]

On Mon, 25 Nov 2013 14:49:37 +0100
Hiroshi Doyu <hdoyu@nvidia.com> wrote:

> Hi Joerg,
> 
> Do you have some time to review this patch along with the following ones?
> 
>   [PATCHv6 02/13] iommu/of: introduce a global iommu device list
>   http://lists.linuxfoundation.org/pipermail/iommu/2013-November/007050.html
> 
>   [PATCHv6 03/13] iommu/of: check if dependee iommu is ready or not
>   http://lists.linuxfoundation.org/pipermail/iommu/2013-November/007051.html

Any chance to get some feedback on them?

> With those patches, now I'm trying to populate iommu master devices
> after an IOMMU device is populated. Originally [PATCHv6 02/13] was
> proposed by Thierry. I'm a bit afraid of adding new IOMMU API as
> this, but I think that this new {bound,unbind}_driver() is the right
> timiing that depeding devices are populated instead of add_device()
> since, even after add_device, a device won't be populated. I'm not so
> sure how this affects on the existing IOMMUs.
> 
> It would be nice if you give some feedback on this.
> 
> On Thu, 21 Nov 2013 14:40:41 +0100
> Hiroshi Doyu <hdoyu@nvidia.com> wrote:
> 
> > ops->{bound,unbind}_driver() functions are called at
> > BUS_NOTIFY_{BOUND,UNBIND}_DRIVER respectively.
> > 
> > This is necessary to control the device population order. IOMMU master
> > devices depend on an IOMMU device instanciation. IOMMU master devices
> > can be registered to an IOMMU only after it's successfully
> > populated. This IOMMU registration is done via
> > ops->bound_driver(). Currently this population can be deferred if
> > depending IOMMU device hasn't yet been populated in driver core. This
> > cannot be done via ops->add_device() since after add_device() device's
> > population/instanciation can be still deferred via probe().
> > 
> > Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> > ---
> > v6:
> > New for v6.
> > ---
> >  drivers/iommu/iommu.c | 13 +++++++++++--
> >  include/linux/iommu.h |  4 ++++
> >  2 files changed, 15 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > index e5555fc..5469d36 100644
> > --- a/drivers/iommu/iommu.c
> > +++ b/drivers/iommu/iommu.c
> > @@ -540,14 +540,23 @@ static int iommu_bus_notifier(struct notifier_block *nb,
> >  	 * ADD/DEL call into iommu driver ops if provided, which may
> >  	 * result in ADD/DEL notifiers to group->notifier
> >  	 */
> > -	if (action == BUS_NOTIFY_ADD_DEVICE) {
> > +	switch (action) {
> > +	case BUS_NOTIFY_ADD_DEVICE:
> >  		if (ops->add_device)
> >  			return ops->add_device(dev);
> > -	} else if (action == BUS_NOTIFY_DEL_DEVICE) {
> > +	case BUS_NOTIFY_DEL_DEVICE:
> >  		if (ops->remove_device && dev->iommu_group) {
> >  			ops->remove_device(dev);
> >  			return 0;
> >  		}
> > +	case BUS_NOTIFY_BOUND_DRIVER:
> > +		if (ops->bound_driver)
> > +			ops->bound_driver(dev);
> > +		break;
> > +	case BUS_NOTIFY_UNBIND_DRIVER:
> > +		if (ops->unbind_driver)
> > +			ops->unbind_driver(dev);
> > +		break;
> >  	}
> >  
> >  	/*
> > diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> > index a444c79..a0e92be 100644
> > --- a/include/linux/iommu.h
> > +++ b/include/linux/iommu.h
> > @@ -96,6 +96,8 @@ enum iommu_attr {
> >   * @domain_has_cap: domain capabilities query
> >   * @add_device: add device to iommu grouping
> >   * @remove_device: remove device from iommu grouping
> > + * @bound_driver: called at BUS_NOTIFY_BOUND_DRIVER
> > + * @unbind_driver: called at BUS_NOTIFY_UNBIND_DRIVER
> >   * @domain_get_attr: Query domain attributes
> >   * @domain_set_attr: Change domain attributes
> >   * @pgsize_bitmap: bitmap of supported page sizes
> > @@ -114,6 +116,8 @@ struct iommu_ops {
> >  			      unsigned long cap);
> >  	int (*add_device)(struct device *dev);
> >  	void (*remove_device)(struct device *dev);
> > +	int (*bound_driver)(struct device *dev);
> > +	void (*unbind_driver)(struct device *dev);
> >  	int (*device_group)(struct device *dev, unsigned int *groupid);
> >  	int (*domain_get_attr)(struct iommu_domain *domain,
> >  			       enum iommu_attr attr, void *data);
> > -- 
> > 1.8.1.5
> > 
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu