> From: Jacob Pan <[email protected]> > Sent: Thursday, March 26, 2020 1:55 AM > > IOASID users fit into the publisher-subscriber pattern, a system wide > blocking notifier chain can be used to inform subscribers of state > changes. Notifier mechanism also abstracts publisher from knowing the > private context each subcriber may have. > > This patch adds APIs and a global notifier chain, a further optimization > might be per set notifier for ioasid_set aware users. > > Usage example: > KVM register notifier block such that it can keep its guest-host PASID > translation table in sync with any IOASID updates. > > VFIO publish IOASID change by performing alloc/free, bind/unbind > operations. > > IOMMU driver gets notified when IOASID is freed by VFIO or core mm code > such that PASID context can be cleaned up.
above example looks mixed. You have KVM registers the notifier but finally having IOMMU driver to get notified... 😊 > > Signed-off-by: Liu Yi L <[email protected]> > Signed-off-by: Jacob Pan <[email protected]> > --- > drivers/iommu/ioasid.c | 61 > ++++++++++++++++++++++++++++++++++++++++++++++++++ > include/linux/ioasid.h | 40 +++++++++++++++++++++++++++++++++ > 2 files changed, 101 insertions(+) > > diff --git a/drivers/iommu/ioasid.c b/drivers/iommu/ioasid.c > index 8612fe6477dc..27dce2cb5af2 100644 > --- a/drivers/iommu/ioasid.c > +++ b/drivers/iommu/ioasid.c > @@ -11,6 +11,22 @@ > #include <linux/xarray.h> > > static DEFINE_XARRAY_ALLOC(ioasid_sets); > +/* > + * An IOASID could have multiple consumers. When a status change occurs, > + * this notifier chain is used to keep them in sync. Each consumer of the > + * IOASID service must register notifier block early to ensure no events > + * are missed. > + * > + * This is a publisher-subscriber pattern where publisher can change the > + * state of each IOASID, e.g. alloc/free, bind IOASID to a device and mm. > + * On the other hand, subscribers gets notified for the state change and > + * keep local states in sync. > + * > + * Currently, the notifier is global. A further optimization could be per > + * IOASID set notifier chain. > + */ > +static BLOCKING_NOTIFIER_HEAD(ioasid_chain); > + > /** > * struct ioasid_set_data - Meta data about ioasid_set > * > @@ -408,6 +424,7 @@ static void ioasid_free_locked(ioasid_t ioasid) > { > struct ioasid_data *ioasid_data; > struct ioasid_set_data *sdata; > + struct ioasid_nb_args args; > > ioasid_data = xa_load(&active_allocator->xa, ioasid); > if (!ioasid_data) { > @@ -415,6 +432,13 @@ static void ioasid_free_locked(ioasid_t ioasid) > return; > } > > + args.id = ioasid; > + args.sid = ioasid_data->sdata->sid; > + args.pdata = ioasid_data->private; > + args.set_token = ioasid_data->sdata->token; > + > + /* Notify all users that this IOASID is being freed */ > + blocking_notifier_call_chain(&ioasid_chain, IOASID_FREE, &args); > active_allocator->ops->free(ioasid, active_allocator->ops->pdata); > /* Custom allocator needs additional steps to free the xa element */ > if (active_allocator->flags & IOASID_ALLOCATOR_CUSTOM) { > @@ -624,6 +648,43 @@ int ioasid_find_sid(ioasid_t ioasid) > } > EXPORT_SYMBOL_GPL(ioasid_find_sid); > > +int ioasid_add_notifier(struct notifier_block *nb) > +{ > + return blocking_notifier_chain_register(&ioasid_chain, nb); > +} > +EXPORT_SYMBOL_GPL(ioasid_add_notifier); > + > +void ioasid_remove_notifier(struct notifier_block *nb) > +{ > + blocking_notifier_chain_unregister(&ioasid_chain, nb); > +} > +EXPORT_SYMBOL_GPL(ioasid_remove_notifier); register/unregister > + > +int ioasid_notify(ioasid_t ioasid, enum ioasid_notify_val cmd) add a comment on when this function should be used? > +{ > + struct ioasid_data *ioasid_data; > + struct ioasid_nb_args args; > + int ret = 0; > + > + mutex_lock(&ioasid_allocator_lock); > + ioasid_data = xa_load(&active_allocator->xa, ioasid); > + if (!ioasid_data) { > + pr_err("Trying to free unknown IOASID %u\n", ioasid); why is it fixed to 'free'? > + mutex_unlock(&ioasid_allocator_lock); > + return -EINVAL; > + } > + > + args.id = ioasid; > + args.sid = ioasid_data->sdata->sid; > + args.pdata = ioasid_data->private; why no token info as did in ioasid_free? > + > + ret = blocking_notifier_call_chain(&ioasid_chain, cmd, &args); > + mutex_unlock(&ioasid_allocator_lock); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(ioasid_notify); > + > MODULE_AUTHOR("Jean-Philippe Brucker <jean- > [email protected]>"); > MODULE_AUTHOR("Jacob Pan <[email protected]>"); > MODULE_DESCRIPTION("IO Address Space ID (IOASID) allocator"); > diff --git a/include/linux/ioasid.h b/include/linux/ioasid.h > index e19c0ad93bd7..32d032913828 100644 > --- a/include/linux/ioasid.h > +++ b/include/linux/ioasid.h > @@ -4,6 +4,7 @@ > > #include <linux/types.h> > #include <linux/errno.h> > +#include <linux/notifier.h> > > #define INVALID_IOASID ((ioasid_t)-1) > #define INVALID_IOASID_SET (-1) > @@ -30,6 +31,27 @@ struct ioasid_allocator_ops { > void *pdata; > }; > > +/* Notification data when IOASID status changed */ > +enum ioasid_notify_val { > + IOASID_ALLOC = 1, > + IOASID_FREE, > + IOASID_BIND, > + IOASID_UNBIND, > +}; Curious why IOASID_ALLOC is not notified aumatically within ioasid_alloc similar to ioasid_free, while leaving to the publisher? BIND/UNBIND is a publisher thing but a bit strange to see ALLOC/FREE with different policy here. > + > +/** > + * struct ioasid_nb_args - Argument provided by IOASID core when notifier > + * is called. > + * @id: the IOASID being notified > + * @sid: the IOASID set @id belongs to > + * @pdata: the private data attached to the IOASID > + */ > +struct ioasid_nb_args { > + ioasid_t id; > + int sid; > + struct ioasid_set *set_token; > + void *pdata; > +}; > /* Shared IOASID set for reserved for host system use */ > extern int system_ioasid_sid; > > @@ -43,11 +65,15 @@ void *ioasid_find(int sid, ioasid_t ioasid, bool > (*getter)(void *)); > int ioasid_register_allocator(struct ioasid_allocator_ops *allocator); > void ioasid_unregister_allocator(struct ioasid_allocator_ops *allocator); > int ioasid_attach_data(ioasid_t ioasid, void *data); > +int ioasid_add_notifier(struct notifier_block *nb); > +void ioasid_remove_notifier(struct notifier_block *nb); > void ioasid_install_capacity(ioasid_t total); > int ioasid_alloc_system_set(int quota); > int ioasid_alloc_set(struct ioasid_set *token, ioasid_t quota, int *sid); > void ioasid_free_set(int sid, bool destroy_set); > int ioasid_find_sid(ioasid_t ioasid); > +int ioasid_notify(ioasid_t id, enum ioasid_notify_val cmd); > + > #else /* !CONFIG_IOASID */ > static inline ioasid_t ioasid_alloc(int sid, ioasid_t min, > ioasid_t max, void *private) > @@ -73,6 +99,20 @@ static inline void *ioasid_find(int sid, ioasid_t ioasid, > bool (*getter)(void *) > return NULL; > } > > +static inline int ioasid_add_notifier(struct notifier_block *nb) > +{ > + return -ENOTSUPP; > +} > + > +static inline void ioasid_remove_notifier(struct notifier_block *nb) > +{ > +} > + > +int ioasid_notify(ioasid_t ioasid, enum ioasid_notify_val cmd) > +{ > + return -ENOTSUPP; > +} > + > static inline int ioasid_register_allocator(struct ioasid_allocator_ops > *allocator) > { > return -ENOTSUPP; > -- > 2.7.4 _______________________________________________ iommu mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/iommu
