On Sun, 17 Mar 2019 18:22:13 +0100
Eric Auger <[email protected]> wrote:

> From: Jacob Pan <[email protected]>
> 
> Traditionally, device specific faults are detected and handled within
> their own device drivers. When IOMMU is enabled, faults such as DMA
> related transactions are detected by IOMMU. There is no generic
> reporting mechanism to report faults back to the in-kernel device
> driver or the guest OS in case of assigned devices.
> 
> This patch introduces a registration API for device specific fault
> handlers. This differs from the existing iommu_set_fault_handler/
> report_iommu_fault infrastructures in several ways:
> - it allows to report more sophisticated fault events (both
>   unrecoverable faults and page request faults) due to the nature
>   of the iommu_fault struct
> - it is device specific and not domain specific.
> 
> The current iommu_report_device_fault() implementation only handles
> the "shoot and forget" unrecoverable fault case. Handling of page
> request faults or stalled faults will come later.
> 
> Signed-off-by: Jacob Pan <[email protected]>
> Signed-off-by: Ashok Raj <[email protected]>
> Signed-off-by: Jean-Philippe Brucker <[email protected]>
> Signed-off-by: Eric Auger <[email protected]>
> 
> ---
> 
> v4 -> v5:
> - remove stuff related to recoverable faults
> ---
>  drivers/iommu/iommu.c | 134 +++++++++++++++++++++++++++++++++++++++++-
>  include/linux/iommu.h |  36 +++++++++++-
>  2 files changed, 168 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 33a982e33716..56d5bf68de53 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -648,6 +648,13 @@ int iommu_group_add_device(struct iommu_group *group, 
> struct device *dev)
>               goto err_free_name;
>       }
>  
> +     dev->iommu_param = kzalloc(sizeof(*dev->iommu_param), GFP_KERNEL);
> +     if (!dev->iommu_param) {
> +             ret = -ENOMEM;
> +             goto err_free_name;
> +     }
> +     mutex_init(&dev->iommu_param->lock);
> +
>       kobject_get(group->devices_kobj);
>  
>       dev->iommu_group = group;
> @@ -678,6 +685,7 @@ int iommu_group_add_device(struct iommu_group *group, 
> struct device *dev)
>       mutex_unlock(&group->mutex);
>       dev->iommu_group = NULL;
>       kobject_put(group->devices_kobj);
> +     kfree(dev->iommu_param);
>  err_free_name:
>       kfree(device->name);
>  err_remove_link:
> @@ -724,7 +732,7 @@ void iommu_group_remove_device(struct device *dev)
>       sysfs_remove_link(&dev->kobj, "iommu_group");
>  
>       trace_remove_device_from_group(group->id, dev);
> -
> +     kfree(dev->iommu_param);
>       kfree(device->name);
>       kfree(device);
>       dev->iommu_group = NULL;
> @@ -858,6 +866,130 @@ int iommu_group_unregister_notifier(struct iommu_group 
> *group,
>  }
>  EXPORT_SYMBOL_GPL(iommu_group_unregister_notifier);
>  
> +/**
> + * iommu_register_device_fault_handler() - Register a device fault handler
> + * @dev: the device
> + * @handler: the fault handler
> + * @data: private data passed as argument to the handler
> + *
> + * When an IOMMU fault event is received, this handler gets called with the
> + * fault event and data as argument.
> + *
> + * Return 0 if the fault handler was installed successfully, or an error.
> + */
> +int iommu_register_device_fault_handler(struct device *dev,
> +                                     iommu_dev_fault_handler_t handler,
> +                                     void *data)
> +{
> +     struct iommu_param *param = dev->iommu_param;
> +     int ret = 0;
> +
> +     /*
> +      * Device iommu_param should have been allocated when device is
> +      * added to its iommu_group.
> +      */
> +     if (!param)
> +             return -EINVAL;
> +
> +     mutex_lock(&param->lock);
> +     /* Only allow one fault handler registered for each device */
> +     if (param->fault_param) {
> +             ret = -EBUSY;
> +             goto done_unlock;
> +     }
> +
> +     get_device(dev);
> +     param->fault_param =
> +             kzalloc(sizeof(struct iommu_fault_param), GFP_KERNEL);
> +     if (!param->fault_param) {
> +             put_device(dev);
> +             ret = -ENOMEM;
> +             goto done_unlock;
> +     }
> +     mutex_init(&param->fault_param->lock);
> +     param->fault_param->handler = handler;
> +     param->fault_param->data = data;
> +     INIT_LIST_HEAD(&param->fault_param->faults);
> +
> +done_unlock:
> +     mutex_unlock(&param->lock);
> +
> +     return ret;
> +}
> +EXPORT_SYMBOL_GPL(iommu_register_device_fault_handler);
> +
> +/**
> + * iommu_unregister_device_fault_handler() - Unregister the device fault 
> handler
> + * @dev: the device
> + *
> + * Remove the device fault handler installed with
> + * iommu_register_device_fault_handler().
> + *
> + * Return 0 on success, or an error.
> + */
> +int iommu_unregister_device_fault_handler(struct device *dev)
> +{
> +     struct iommu_param *param = dev->iommu_param;
> +     int ret = 0;
> +
> +     if (!param)
> +             return -EINVAL;
> +
> +     mutex_lock(&param->lock);
> +
> +     if (!param->fault_param)
> +             goto unlock;
> +
> +     /* we cannot unregister handler if there are pending faults */
> +     if (!list_empty(&param->fault_param->faults)) {
> +             ret = -EBUSY;
> +             goto unlock;
> +     }
> +
> +     kfree(param->fault_param);
> +     param->fault_param = NULL;
> +     put_device(dev);
> +unlock:
> +     mutex_unlock(&param->lock);
> +
> +     return ret;
> +}
> +EXPORT_SYMBOL_GPL(iommu_unregister_device_fault_handler);
> +
> +
> +/**
> + * iommu_report_device_fault() - Report fault event to device
> + * @dev: the device
> + * @evt: fault event data
> + *
> + * Called by IOMMU model specific drivers when fault is detected, typically
> + * in a threaded IRQ handler.
> + *
> + * Return 0 on success, or an error.
> + */
> +int iommu_report_device_fault(struct device *dev, struct iommu_fault_event 
> *evt)
> +{
> +     struct iommu_fault_param *fparam;
> +     int ret = 0;
> +


Nit, for consistency with above functions, it'd be useful to have:

struct iommu_param *param = dev->iommu_param;

It's not as obvious as it could be that we're using the same mutex here
as in the register/unregister above.  Thanks,

Alex

> +     /* iommu_param is allocated when device is added to group */
> +     if (!dev->iommu_param || !evt)
> +             return -EINVAL;
> +     /* we only report device fault if there is a handler registered */
> +     mutex_lock(&dev->iommu_param->lock);
> +     if (!dev->iommu_param->fault_param ||
> +             !dev->iommu_param->fault_param->handler) {
> +             ret = -EINVAL;
> +             goto done_unlock;
> +     }
> +     fparam = dev->iommu_param->fault_param;
> +     ret = fparam->handler(evt, fparam->data);
> +done_unlock:
> +     mutex_unlock(&dev->iommu_param->lock);
> +     return ret;
> +}
> +EXPORT_SYMBOL_GPL(iommu_report_device_fault);
> +
>  /**
>   * iommu_group_id - Return ID for a group
>   * @group: the group to ID
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index c6f398f7e6e0..aeb4b615cb44 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -257,11 +257,13 @@ struct iommu_device {
>   * unrecoverable faults such as DMA or IRQ remapping faults.
>   *
>   * @fault: fault descriptor
> + * @list pending fault event list, used for tracking responses
>   * @iommu_private: used by the IOMMU driver for storing fault-specific
>   *                 data. Users should not modify this field before
>   *                 sending the fault response.
>   */
>  struct iommu_fault_event {
> +     struct list_head list;
>       struct iommu_fault fault;
>       u64 iommu_private;
>  };
> @@ -270,10 +272,13 @@ struct iommu_fault_event {
>   * struct iommu_fault_param - per-device IOMMU fault data
>   * @dev_fault_handler: Callback function to handle IOMMU faults at device 
> level
>   * @data: handler private data
> - *
> + * @faults: holds the pending faults which needs response, e.g. page 
> response.
> + * @lock: protect pending PRQ event list
>   */
>  struct iommu_fault_param {
>       iommu_dev_fault_handler_t handler;
> +     struct list_head faults;
> +     struct mutex lock;
>       void *data;
>  };
>  
> @@ -287,6 +292,7 @@ struct iommu_fault_param {
>   *   struct iommu_fwspec     *iommu_fwspec;
>   */
>  struct iommu_param {
> +     struct mutex lock;
>       struct iommu_fault_param *fault_param;
>  };
>  
> @@ -379,6 +385,15 @@ extern int iommu_group_register_notifier(struct 
> iommu_group *group,
>                                        struct notifier_block *nb);
>  extern int iommu_group_unregister_notifier(struct iommu_group *group,
>                                          struct notifier_block *nb);
> +extern int iommu_register_device_fault_handler(struct device *dev,
> +                                     iommu_dev_fault_handler_t handler,
> +                                     void *data);
> +
> +extern int iommu_unregister_device_fault_handler(struct device *dev);
> +
> +extern int iommu_report_device_fault(struct device *dev,
> +                                  struct iommu_fault_event *evt);
> +
>  extern int iommu_group_id(struct iommu_group *group);
>  extern struct iommu_group *iommu_group_get_for_dev(struct device *dev);
>  extern struct iommu_domain *iommu_group_default_domain(struct iommu_group *);
> @@ -659,6 +674,25 @@ static inline int iommu_group_unregister_notifier(struct 
> iommu_group *group,
>       return 0;
>  }
>  
> +static inline
> +int iommu_register_device_fault_handler(struct device *dev,
> +                                     iommu_dev_fault_handler_t handler,
> +                                     void *data)
> +{
> +     return -ENODEV;
> +}
> +
> +static inline int iommu_unregister_device_fault_handler(struct device *dev)
> +{
> +     return 0;
> +}
> +
> +static inline
> +int iommu_report_device_fault(struct device *dev, struct iommu_fault_event 
> *evt)
> +{
> +     return -ENODEV;
> +}
> +
>  static inline int iommu_group_id(struct iommu_group *group)
>  {
>       return -ENODEV;

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to