Re: [PATCH 4/4] iommu: Add recoverable fault reporting
On 24/05/2019 19:14, Jacob Pan wrote: > On Thu, 23 May 2019 19:06:13 +0100 >> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c >> index d546f7baa0d4..b09b3707f0e4 100644 >> --- a/drivers/iommu/iommu.c >> +++ b/drivers/iommu/iommu.c >> @@ -872,7 +872,14 @@ >> EXPORT_SYMBOL_GPL(iommu_group_unregister_notifier); >> * @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. The handler should return 0 on >> success. >> + * fault event and data as argument. The handler should return 0 on >> success. If >> + * the fault is recoverable (IOMMU_FAULT_PAGE_REQ), the handler >> should also >> + * complete the fault by calling iommu_page_response() with one of >> the following > nit, in case of injecting into the guest, handler does not have to call > iommu_page_response() directly. True, I'll think of a better wording. Maybe just s/handler/consumer/ although we didn't define consumer anywhere >> IOMMU_PAGE_RESP_PASID_VALID : 0; + >> +ret = domain->ops->page_response(dev, msg, >> evt->iommu_private); > I guess here you could drop iommu_private in favor of prm such that > drivers such as vt-d can recover private data as needed? Yes, will change this Thanks, Jean
Re: [PATCH 4/4] iommu: Add recoverable fault reporting
On Thu, 23 May 2019 19:06:13 +0100 Jean-Philippe Brucker wrote: > Some IOMMU hardware features, for example PCI PRI and Arm SMMU Stall, > enable recoverable I/O page faults. Allow IOMMU drivers to report PRI > Page Requests and Stall events through the new fault reporting API. > The consumer of the fault can be either an I/O page fault handler in > the host, or a guest OS. > > Once handled, the fault must be completed by sending a page response > back to the IOMMU. Add an iommu_page_response() function to complete > a page fault. > > Signed-off-by: Jacob Pan > Signed-off-by: Jean-Philippe Brucker > --- > drivers/iommu/iommu.c | 95 > +- include/linux/iommu.h | > 19 include/uapi/linux/iommu.h | 34 ++ > 3 files changed, 146 insertions(+), 2 deletions(-) > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > index d546f7baa0d4..b09b3707f0e4 100644 > --- a/drivers/iommu/iommu.c > +++ b/drivers/iommu/iommu.c > @@ -872,7 +872,14 @@ > EXPORT_SYMBOL_GPL(iommu_group_unregister_notifier); > * @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. The handler should return 0 on > success. > + * fault event and data as argument. The handler should return 0 on > success. If > + * the fault is recoverable (IOMMU_FAULT_PAGE_REQ), the handler > should also > + * complete the fault by calling iommu_page_response() with one of > the following nit, in case of injecting into the guest, handler does not have to call iommu_page_response() directly. > + * response code: > + * - IOMMU_PAGE_RESP_SUCCESS: retry the translation > + * - IOMMU_PAGE_RESP_INVALID: terminate the fault > + * - IOMMU_PAGE_RESP_FAILURE: terminate the fault and stop reporting > + * page faults if possible. > * > * Return 0 if the fault handler was installed successfully, or an > error. */ > @@ -907,6 +914,8 @@ int iommu_register_device_fault_handler(struct > device *dev, } > param->fault_param->handler = handler; > param->fault_param->data = data; > + mutex_init(>fault_param->lock); > + INIT_LIST_HEAD(>fault_param->faults); > > done_unlock: > mutex_unlock(>lock); > @@ -937,6 +946,12 @@ int iommu_unregister_device_fault_handler(struct > device *dev) if (!param->fault_param) > goto unlock; > > + /* we cannot unregister handler if there are pending faults > */ > + if (!list_empty(>fault_param->faults)) { > + ret = -EBUSY; > + goto unlock; > + } > + > kfree(param->fault_param); > param->fault_param = NULL; > put_device(dev); > @@ -953,13 +968,15 @@ > EXPORT_SYMBOL_GPL(iommu_unregister_device_fault_handler); > * @evt: fault event data > * > * Called by IOMMU drivers when a fault is detected, typically in a > threaded IRQ > - * handler. > + * handler. When this function fails and the fault is recoverable, > it is the > + * caller's responsibility to complete the fault. > * > * Return 0 on success, or an error. > */ > int iommu_report_device_fault(struct device *dev, struct > iommu_fault_event *evt) { > struct iommu_param *param = dev->iommu_param; > + struct iommu_fault_event *evt_pending = NULL; > struct iommu_fault_param *fparam; > int ret = 0; > > @@ -974,7 +991,27 @@ int iommu_report_device_fault(struct device > *dev, struct iommu_fault_event *evt) ret = -EINVAL; > goto done_unlock; > } > + > + if (evt->fault.type == IOMMU_FAULT_PAGE_REQ && > + (evt->fault.prm.flags & > IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE)) { > + evt_pending = kmemdup(evt, sizeof(struct > iommu_fault_event), > + GFP_KERNEL); > + if (!evt_pending) { > + ret = -ENOMEM; > + goto done_unlock; > + } > + mutex_lock(>lock); > + list_add_tail(_pending->list, >faults); > + mutex_unlock(>lock); > + } > + > ret = fparam->handler(evt, fparam->data); > + if (ret && evt_pending) { > + mutex_lock(>lock); > + list_del(_pending->list); > + mutex_unlock(>lock); > + kfree(evt_pending); > + } > done_unlock: > mutex_unlock(>lock); > return ret; > @@ -1515,6 +1552,60 @@ int iommu_attach_device(struct iommu_domain > *domain, struct device *dev) } > EXPORT_SYMBOL_GPL(iommu_attach_device); > > +int iommu_page_response(struct device *dev, > + struct iommu_page_response *msg) > +{ > + bool pasid_valid; > + int ret = -EINVAL; > + struct iommu_fault_event *evt; > + struct iommu_fault_page_request *prm; > + struct iommu_param *param = dev->iommu_param; > + struct iommu_domain *domain = iommu_get_domain_for_dev(dev); > + > + if (!domain || !domain->ops->page_response) > +
[PATCH 4/4] iommu: Add recoverable fault reporting
Some IOMMU hardware features, for example PCI PRI and Arm SMMU Stall, enable recoverable I/O page faults. Allow IOMMU drivers to report PRI Page Requests and Stall events through the new fault reporting API. The consumer of the fault can be either an I/O page fault handler in the host, or a guest OS. Once handled, the fault must be completed by sending a page response back to the IOMMU. Add an iommu_page_response() function to complete a page fault. Signed-off-by: Jacob Pan Signed-off-by: Jean-Philippe Brucker --- drivers/iommu/iommu.c | 95 +- include/linux/iommu.h | 19 include/uapi/linux/iommu.h | 34 ++ 3 files changed, 146 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index d546f7baa0d4..b09b3707f0e4 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -872,7 +872,14 @@ EXPORT_SYMBOL_GPL(iommu_group_unregister_notifier); * @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. The handler should return 0 on success. + * fault event and data as argument. The handler should return 0 on success. If + * the fault is recoverable (IOMMU_FAULT_PAGE_REQ), the handler should also + * complete the fault by calling iommu_page_response() with one of the following + * response code: + * - IOMMU_PAGE_RESP_SUCCESS: retry the translation + * - IOMMU_PAGE_RESP_INVALID: terminate the fault + * - IOMMU_PAGE_RESP_FAILURE: terminate the fault and stop reporting + * page faults if possible. * * Return 0 if the fault handler was installed successfully, or an error. */ @@ -907,6 +914,8 @@ int iommu_register_device_fault_handler(struct device *dev, } param->fault_param->handler = handler; param->fault_param->data = data; + mutex_init(>fault_param->lock); + INIT_LIST_HEAD(>fault_param->faults); done_unlock: mutex_unlock(>lock); @@ -937,6 +946,12 @@ int iommu_unregister_device_fault_handler(struct device *dev) if (!param->fault_param) goto unlock; + /* we cannot unregister handler if there are pending faults */ + if (!list_empty(>fault_param->faults)) { + ret = -EBUSY; + goto unlock; + } + kfree(param->fault_param); param->fault_param = NULL; put_device(dev); @@ -953,13 +968,15 @@ EXPORT_SYMBOL_GPL(iommu_unregister_device_fault_handler); * @evt: fault event data * * Called by IOMMU drivers when a fault is detected, typically in a threaded IRQ - * handler. + * handler. When this function fails and the fault is recoverable, it is the + * caller's responsibility to complete the fault. * * Return 0 on success, or an error. */ int iommu_report_device_fault(struct device *dev, struct iommu_fault_event *evt) { struct iommu_param *param = dev->iommu_param; + struct iommu_fault_event *evt_pending = NULL; struct iommu_fault_param *fparam; int ret = 0; @@ -974,7 +991,27 @@ int iommu_report_device_fault(struct device *dev, struct iommu_fault_event *evt) ret = -EINVAL; goto done_unlock; } + + if (evt->fault.type == IOMMU_FAULT_PAGE_REQ && + (evt->fault.prm.flags & IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE)) { + evt_pending = kmemdup(evt, sizeof(struct iommu_fault_event), + GFP_KERNEL); + if (!evt_pending) { + ret = -ENOMEM; + goto done_unlock; + } + mutex_lock(>lock); + list_add_tail(_pending->list, >faults); + mutex_unlock(>lock); + } + ret = fparam->handler(evt, fparam->data); + if (ret && evt_pending) { + mutex_lock(>lock); + list_del(_pending->list); + mutex_unlock(>lock); + kfree(evt_pending); + } done_unlock: mutex_unlock(>lock); return ret; @@ -1515,6 +1552,60 @@ int iommu_attach_device(struct iommu_domain *domain, struct device *dev) } EXPORT_SYMBOL_GPL(iommu_attach_device); +int iommu_page_response(struct device *dev, + struct iommu_page_response *msg) +{ + bool pasid_valid; + int ret = -EINVAL; + struct iommu_fault_event *evt; + struct iommu_fault_page_request *prm; + struct iommu_param *param = dev->iommu_param; + struct iommu_domain *domain = iommu_get_domain_for_dev(dev); + + if (!domain || !domain->ops->page_response) + return -ENODEV; + + /* +* Device iommu_param should have been allocated when device is +* added to its iommu_group. +*/ + if (!param || !param->fault_param) + return -EINVAL; + + /* Only send response if there is a fault