| Message ID | 20210309062207.505-2-lushenming@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add IOPF support for VFIO passthrough | expand |
Hi Shenming, On 3/9/21 2:22 PM, Shenming Lu wrote: > This patch follows the discussion here: > > https://lore.kernel.org/linux-acpi/YAaxjmJW+ZMvrhac@myrica/ > > In order to support more scenarios of using IOPF (mainly consider > the nested extension), besides keeping IOMMU_DEV_FEAT_IOPF as a > general capability for whether delivering faults through the IOMMU, > we extend iommu_register_fault_handler() with flags and introduce > IOPF_REPORT_FLAT and IOPF_REPORT_NESTED to describe the page fault > reporting capability under a specific configuration. > IOPF_REPORT_NESTED needs additional info to indicate which level/stage > is concerned since the fault client may be interested in only one > level. > > Signed-off-by: Shenming Lu <lushenming@huawei.com> > --- > .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 3 +- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 11 ++-- > drivers/iommu/io-pgfault.c | 4 -- > drivers/iommu/iommu.c | 56 ++++++++++++++++++- > include/linux/iommu.h | 21 ++++++- > include/uapi/linux/iommu.h | 3 + > 6 files changed, 85 insertions(+), 13 deletions(-) > > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c > index ee66d1f4cb81..5de9432349d4 100644 > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c > @@ -482,7 +482,8 @@ static int arm_smmu_master_sva_enable_iopf(struct arm_smmu_master *master) > if (ret) > return ret; > > - ret = iommu_register_device_fault_handler(dev, iommu_queue_iopf, dev); > + ret = iommu_register_device_fault_handler(dev, iommu_queue_iopf, > + IOPF_REPORT_FLAT, dev); > if (ret) { > iopf_queue_remove_device(master->smmu->evtq.iopf, dev); > return ret; > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > index 363744df8d51..f40529d0075d 100644 > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > @@ -1447,10 +1447,6 @@ static int arm_smmu_handle_evt(struct arm_smmu_device *smmu, u64 *evt) > return -EOPNOTSUPP; > } > > - /* Stage-2 is always pinned at the moment */ > - if (evt[1] & EVTQ_1_S2) > - return -EFAULT; > - > if (evt[1] & EVTQ_1_RnW) > perm |= IOMMU_FAULT_PERM_READ; > else > @@ -1468,13 +1464,18 @@ static int arm_smmu_handle_evt(struct arm_smmu_device *smmu, u64 *evt) > .flags = IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE, > .grpid = FIELD_GET(EVTQ_1_STAG, evt[1]), > .perm = perm, > - .addr = FIELD_GET(EVTQ_2_ADDR, evt[2]), > }; > > if (ssid_valid) { > flt->prm.flags |= IOMMU_FAULT_PAGE_REQUEST_PASID_VALID; > flt->prm.pasid = FIELD_GET(EVTQ_0_SSID, evt[0]); > } > + > + if (evt[1] & EVTQ_1_S2) { > + flt->prm.flags |= IOMMU_FAULT_PAGE_REQUEST_L2; > + flt->prm.addr = FIELD_GET(EVTQ_3_IPA, evt[3]); > + } else > + flt->prm.addr = FIELD_GET(EVTQ_2_ADDR, evt[2]); > } else { > flt->type = IOMMU_FAULT_DMA_UNRECOV; > flt->event = (struct iommu_fault_unrecoverable) { > diff --git a/drivers/iommu/io-pgfault.c b/drivers/iommu/io-pgfault.c > index 1df8c1dcae77..abf16e06bcf5 100644 > --- a/drivers/iommu/io-pgfault.c > +++ b/drivers/iommu/io-pgfault.c > @@ -195,10 +195,6 @@ int iommu_queue_iopf(struct iommu_fault *fault, void *cookie) > > lockdep_assert_held(¶m->lock); > > - if (fault->type != IOMMU_FAULT_PAGE_REQ) > - /* Not a recoverable page fault */ > - return -EOPNOTSUPP; > - Any reasons why do you want to remove this check? > /* > * As long as we're holding param->lock, the queue can't be unlinked > * from the device and therefore cannot disappear. > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > index d0b0a15dba84..cb1d93b00f7d 100644 > --- a/drivers/iommu/iommu.c > +++ b/drivers/iommu/iommu.c > @@ -1056,6 +1056,40 @@ int iommu_group_unregister_notifier(struct iommu_group *group, > } > EXPORT_SYMBOL_GPL(iommu_group_unregister_notifier); > > +/* > + * iommu_update_device_fault_handler - Update the device fault handler via flags > + * @dev: the device > + * @mask: bits(not set) to clear > + * @set: bits to set > + * > + * Update the device fault handler installed by > + * iommu_register_device_fault_handler(). > + * > + * Return 0 on success, or an error. > + */ > +int iommu_update_device_fault_handler(struct device *dev, u32 mask, u32 set) > +{ > + struct dev_iommu *param = dev->iommu; > + int ret = 0; > + > + if (!param) > + return -EINVAL; > + > + mutex_lock(¶m->lock); > + > + if (param->fault_param) { > + ret = -EINVAL; > + goto out_unlock; > + } > + > + param->fault_param->flags = (param->fault_param->flags & mask) | set; > + > +out_unlock: > + mutex_unlock(¶m->lock); > + return ret; > +} > +EXPORT_SYMBOL_GPL(iommu_update_device_fault_handler); When and why will this API be used? Why not registering the fault handling capabilities of a device driver only once during probe()? > + > /** > * iommu_register_device_fault_handler() - Register a device fault handler > * @dev: the device > @@ -1076,11 +1110,14 @@ EXPORT_SYMBOL_GPL(iommu_group_unregister_notifier); > */ > int iommu_register_device_fault_handler(struct device *dev, > iommu_dev_fault_handler_t handler, > - void *data) > + u32 flags, void *data) > { > struct dev_iommu *param = dev->iommu; > int ret = 0; > > + if (flags & IOPF_REPORT_FLAT && flags & IOPF_REPORT_NESTED) > + return -EINVAL; > + > if (!param) > return -EINVAL; > > @@ -1099,6 +1136,7 @@ int iommu_register_device_fault_handler(struct device *dev, > goto done_unlock; > } > param->fault_param->handler = handler; > + param->fault_param->flags = flags; > param->fault_param->data = data; > mutex_init(¶m->fault_param->lock); > INIT_LIST_HEAD(¶m->fault_param->faults); > @@ -1177,6 +1215,22 @@ int iommu_report_device_fault(struct device *dev, struct iommu_fault_event *evt) > goto done_unlock; > } > > + /* The unrecoverable fault reporting is not supported at the moment. */ > + if (evt->fault.type != IOMMU_FAULT_PAGE_REQ) > + return -EOPNOTSUPP; Any reasons why do you want to disable reporting an unrecoverable fault? > + > + if (evt->fault.type == IOMMU_FAULT_PAGE_REQ) { > + if (fparam->flags & IOPF_REPORT_NESTED) { > + if (evt->fault.prm.flags & IOMMU_FAULT_PAGE_REQUEST_L2 && > + !(fparam->flags & IOPF_REPORT_NESTED_L2_CONCERNED)) > + return -EOPNOTSUPP; > + if (!(evt->fault.prm.flags & IOMMU_FAULT_PAGE_REQUEST_L2) && > + !(fparam->flags & IOPF_REPORT_NESTED_L1_CONCERNED)) > + return -EOPNOTSUPP; > + } else if (!(fparam->flags & IOPF_REPORT_FLAT)) > + return -EOPNOTSUPP; > + } > + > 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), > diff --git a/include/linux/iommu.h b/include/linux/iommu.h > index 86d688c4418f..f03d761e8310 100644 > --- a/include/linux/iommu.h > +++ b/include/linux/iommu.h > @@ -352,12 +352,21 @@ struct iommu_fault_event { > /** > * struct iommu_fault_param - per-device IOMMU fault data > * @handler: Callback function to handle IOMMU faults at device level > + * @flags: Indicates whether the fault reporting is available under a > + * specific configuration (1st/2nd-level-only(FLAT), or nested). > + * IOPF_REPORT_NESTED needs to additionally know which level/stage > + * is concerned. If IOPF_REPORT_NESTED only is not valid why do you want to define it? > * @data: handler private data > * @faults: holds the pending faults which needs response > * @lock: protect pending faults list > */ > struct iommu_fault_param { > iommu_dev_fault_handler_t handler; > +#define IOPF_REPORT_FLAT (1 << 0) > +#define IOPF_REPORT_NESTED (1 << 1) > +#define IOPF_REPORT_NESTED_L1_CONCERNED (1 << 2) > +#define IOPF_REPORT_NESTED_L2_CONCERNED (1 << 3) > + u32 flags; > void *data; > struct list_head faults; > struct mutex lock; > @@ -509,9 +518,11 @@ 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_update_device_fault_handler(struct device *dev, > + u32 mask, u32 set); > extern int iommu_register_device_fault_handler(struct device *dev, > iommu_dev_fault_handler_t handler, > - void *data); > + u32 flags, void *data); > > extern int iommu_unregister_device_fault_handler(struct device *dev); > > @@ -873,10 +884,16 @@ static inline int iommu_group_unregister_notifier(struct iommu_group *group, > return 0; > } > > +static inline int iommu_update_device_fault_handler(struct device *dev, > + u32 mask, u32 set) > +{ > + return -ENODEV; > +} > + > static inline > int iommu_register_device_fault_handler(struct device *dev, > iommu_dev_fault_handler_t handler, > - void *data) > + u32 flags, void *data) > { > return -ENODEV; > } > diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h > index e1d9e75f2c94..0ce0dfb7713e 100644 > --- a/include/uapi/linux/iommu.h > +++ b/include/uapi/linux/iommu.h > @@ -85,6 +85,8 @@ struct iommu_fault_unrecoverable { > * When IOMMU_FAULT_PAGE_RESPONSE_NEEDS_PASID is set, the page response > * must have the same PASID value as the page request. When it is clear, > * the page response should not have a PASID. > + * If IOMMU_FAULT_PAGE_REQUEST_L2 is set, the fault occurred at the > + * second level/stage, otherwise, occurred at the first level. > * @pasid: Process Address Space ID > * @grpid: Page Request Group Index > * @perm: requested page permissions (IOMMU_FAULT_PERM_* values) > @@ -96,6 +98,7 @@ struct iommu_fault_page_request { > #define IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE (1 << 1) > #define IOMMU_FAULT_PAGE_REQUEST_PRIV_DATA (1 << 2) > #define IOMMU_FAULT_PAGE_RESPONSE_NEEDS_PASID (1 << 3) > +#define IOMMU_FAULT_PAGE_REQUEST_L2 (1 << 4) > __u32 flags; > __u32 pasid; > __u32 grpid; > Best regards, baolu
Hi Baolu, On 2021/3/10 10:09, Lu Baolu wrote: > Hi Shenming, > > On 3/9/21 2:22 PM, Shenming Lu wrote: >> This patch follows the discussion here: >> >> https://lore.kernel.org/linux-acpi/YAaxjmJW+ZMvrhac@myrica/ >> >> In order to support more scenarios of using IOPF (mainly consider >> the nested extension), besides keeping IOMMU_DEV_FEAT_IOPF as a >> general capability for whether delivering faults through the IOMMU, >> we extend iommu_register_fault_handler() with flags and introduce >> IOPF_REPORT_FLAT and IOPF_REPORT_NESTED to describe the page fault >> reporting capability under a specific configuration. >> IOPF_REPORT_NESTED needs additional info to indicate which level/stage >> is concerned since the fault client may be interested in only one >> level. >> >> Signed-off-by: Shenming Lu <lushenming@huawei.com> >> --- >> .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 3 +- >> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 11 ++-- >> drivers/iommu/io-pgfault.c | 4 -- >> drivers/iommu/iommu.c | 56 ++++++++++++++++++- >> include/linux/iommu.h | 21 ++++++- >> include/uapi/linux/iommu.h | 3 + >> 6 files changed, 85 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c >> index ee66d1f4cb81..5de9432349d4 100644 >> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c >> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c >> @@ -482,7 +482,8 @@ static int arm_smmu_master_sva_enable_iopf(struct arm_smmu_master *master) >> if (ret) >> return ret; >> - ret = iommu_register_device_fault_handler(dev, iommu_queue_iopf, dev); >> + ret = iommu_register_device_fault_handler(dev, iommu_queue_iopf, >> + IOPF_REPORT_FLAT, dev); >> if (ret) { >> iopf_queue_remove_device(master->smmu->evtq.iopf, dev); >> return ret; >> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c >> index 363744df8d51..f40529d0075d 100644 >> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c >> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c >> @@ -1447,10 +1447,6 @@ static int arm_smmu_handle_evt(struct arm_smmu_device *smmu, u64 *evt) >> return -EOPNOTSUPP; >> } >> - /* Stage-2 is always pinned at the moment */ >> - if (evt[1] & EVTQ_1_S2) >> - return -EFAULT; >> - >> if (evt[1] & EVTQ_1_RnW) >> perm |= IOMMU_FAULT_PERM_READ; >> else >> @@ -1468,13 +1464,18 @@ static int arm_smmu_handle_evt(struct arm_smmu_device *smmu, u64 *evt) >> .flags = IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE, >> .grpid = FIELD_GET(EVTQ_1_STAG, evt[1]), >> .perm = perm, >> - .addr = FIELD_GET(EVTQ_2_ADDR, evt[2]), >> }; >> if (ssid_valid) { >> flt->prm.flags |= IOMMU_FAULT_PAGE_REQUEST_PASID_VALID; >> flt->prm.pasid = FIELD_GET(EVTQ_0_SSID, evt[0]); >> } >> + >> + if (evt[1] & EVTQ_1_S2) { >> + flt->prm.flags |= IOMMU_FAULT_PAGE_REQUEST_L2; >> + flt->prm.addr = FIELD_GET(EVTQ_3_IPA, evt[3]); >> + } else >> + flt->prm.addr = FIELD_GET(EVTQ_2_ADDR, evt[2]); >> } else { >> flt->type = IOMMU_FAULT_DMA_UNRECOV; >> flt->event = (struct iommu_fault_unrecoverable) { >> diff --git a/drivers/iommu/io-pgfault.c b/drivers/iommu/io-pgfault.c >> index 1df8c1dcae77..abf16e06bcf5 100644 >> --- a/drivers/iommu/io-pgfault.c >> +++ b/drivers/iommu/io-pgfault.c >> @@ -195,10 +195,6 @@ int iommu_queue_iopf(struct iommu_fault *fault, void *cookie) >> lockdep_assert_held(¶m->lock); >> - if (fault->type != IOMMU_FAULT_PAGE_REQ) >> - /* Not a recoverable page fault */ >> - return -EOPNOTSUPP; >> - > > Any reasons why do you want to remove this check? My thought was to make the reporting cap more detailed: IOPF_REPORT_ is only for recoverable page faults (IOMMU_FAULT_PAGE_REQ), and we may add UNRECOV_FAULT_REPORT_ later for unrecoverable faults (IOMMU_FAULT_DMA_UNRECOV)... > >> /* >> * As long as we're holding param->lock, the queue can't be unlinked >> * from the device and therefore cannot disappear. >> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c >> index d0b0a15dba84..cb1d93b00f7d 100644 >> --- a/drivers/iommu/iommu.c >> +++ b/drivers/iommu/iommu.c >> @@ -1056,6 +1056,40 @@ int iommu_group_unregister_notifier(struct iommu_group *group, >> } >> EXPORT_SYMBOL_GPL(iommu_group_unregister_notifier); >> +/* >> + * iommu_update_device_fault_handler - Update the device fault handler via flags >> + * @dev: the device >> + * @mask: bits(not set) to clear >> + * @set: bits to set >> + * >> + * Update the device fault handler installed by >> + * iommu_register_device_fault_handler(). >> + * >> + * Return 0 on success, or an error. >> + */ >> +int iommu_update_device_fault_handler(struct device *dev, u32 mask, u32 set) >> +{ >> + struct dev_iommu *param = dev->iommu; >> + int ret = 0; >> + >> + if (!param) >> + return -EINVAL; >> + >> + mutex_lock(¶m->lock); >> + >> + if (param->fault_param) { >> + ret = -EINVAL; >> + goto out_unlock; >> + } >> + >> + param->fault_param->flags = (param->fault_param->flags & mask) | set; >> + >> +out_unlock: >> + mutex_unlock(¶m->lock); >> + return ret; >> +} >> +EXPORT_SYMBOL_GPL(iommu_update_device_fault_handler); > > When and why will this API be used? Why not registering the fault > handling capabilities of a device driver only once during probe()? In VFIO, stage 2 IOPF might be enabled via an ioctl from the userspace (in this series), while stage 1 IOPF is enabled from vfio_pci_enable() [1] for nested mode, they are configured separately, and currently each device can only have one iommu dev fault handler. So I choose to add a update interface for the second one. [1] https://patchwork.kernel.org/project/kvm/patch/20210223210625.604517-6-eric.auger@redhat.com/ > >> + >> /** >> * iommu_register_device_fault_handler() - Register a device fault handler >> * @dev: the device >> @@ -1076,11 +1110,14 @@ EXPORT_SYMBOL_GPL(iommu_group_unregister_notifier); >> */ >> int iommu_register_device_fault_handler(struct device *dev, >> iommu_dev_fault_handler_t handler, >> - void *data) >> + u32 flags, void *data) >> { >> struct dev_iommu *param = dev->iommu; >> int ret = 0; >> + if (flags & IOPF_REPORT_FLAT && flags & IOPF_REPORT_NESTED) >> + return -EINVAL; >> + >> if (!param) >> return -EINVAL; >> @@ -1099,6 +1136,7 @@ int iommu_register_device_fault_handler(struct device *dev, >> goto done_unlock; >> } >> param->fault_param->handler = handler; >> + param->fault_param->flags = flags; >> param->fault_param->data = data; >> mutex_init(¶m->fault_param->lock); >> INIT_LIST_HEAD(¶m->fault_param->faults); >> @@ -1177,6 +1215,22 @@ int iommu_report_device_fault(struct device *dev, struct iommu_fault_event *evt) >> goto done_unlock; >> } >> + /* The unrecoverable fault reporting is not supported at the moment. */ >> + if (evt->fault.type != IOMMU_FAULT_PAGE_REQ) >> + return -EOPNOTSUPP; > > Any reasons why do you want to disable reporting an unrecoverable fault? When I add UNRECOV_FAULT_REPORT_ (mentioned above), I will enable the unrecoverable fault reporting if the fault client concerns it. Sorry for this. :-) > >> + >> + if (evt->fault.type == IOMMU_FAULT_PAGE_REQ) { >> + if (fparam->flags & IOPF_REPORT_NESTED) { >> + if (evt->fault.prm.flags & IOMMU_FAULT_PAGE_REQUEST_L2 && >> + !(fparam->flags & IOPF_REPORT_NESTED_L2_CONCERNED)) >> + return -EOPNOTSUPP; >> + if (!(evt->fault.prm.flags & IOMMU_FAULT_PAGE_REQUEST_L2) && >> + !(fparam->flags & IOPF_REPORT_NESTED_L1_CONCERNED)) >> + return -EOPNOTSUPP; >> + } else if (!(fparam->flags & IOPF_REPORT_FLAT)) >> + return -EOPNOTSUPP; >> + } >> + >> 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), >> diff --git a/include/linux/iommu.h b/include/linux/iommu.h >> index 86d688c4418f..f03d761e8310 100644 >> --- a/include/linux/iommu.h >> +++ b/include/linux/iommu.h >> @@ -352,12 +352,21 @@ struct iommu_fault_event { >> /** >> * struct iommu_fault_param - per-device IOMMU fault data >> * @handler: Callback function to handle IOMMU faults at device level >> + * @flags: Indicates whether the fault reporting is available under a >> + * specific configuration (1st/2nd-level-only(FLAT), or nested). >> + * IOPF_REPORT_NESTED needs to additionally know which level/stage >> + * is concerned. > > If IOPF_REPORT_NESTED only is not valid why do you want to define it? Yeah, it seems that IOPF_REPORT_NESTED is unnecessary, IOPF_REPORT_NESTED_L1 + IOPF_REPORT_NESTED_L2 is enough... Thanks for your comments! Shenming > >> * @data: handler private data >> * @faults: holds the pending faults which needs response >> * @lock: protect pending faults list >> */ >> struct iommu_fault_param { >> iommu_dev_fault_handler_t handler; >> +#define IOPF_REPORT_FLAT (1 << 0) >> +#define IOPF_REPORT_NESTED (1 << 1) >> +#define IOPF_REPORT_NESTED_L1_CONCERNED (1 << 2) >> +#define IOPF_REPORT_NESTED_L2_CONCERNED (1 << 3) >> + u32 flags; >> void *data; >> struct list_head faults; >> struct mutex lock; >> @@ -509,9 +518,11 @@ 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_update_device_fault_handler(struct device *dev, >> + u32 mask, u32 set); >> extern int iommu_register_device_fault_handler(struct device *dev, >> iommu_dev_fault_handler_t handler, >> - void *data); >> + u32 flags, void *data); >> extern int iommu_unregister_device_fault_handler(struct device *dev); >> @@ -873,10 +884,16 @@ static inline int iommu_group_unregister_notifier(struct iommu_group *group, >> return 0; >> } >> +static inline int iommu_update_device_fault_handler(struct device *dev, >> + u32 mask, u32 set) >> +{ >> + return -ENODEV; >> +} >> + >> static inline >> int iommu_register_device_fault_handler(struct device *dev, >> iommu_dev_fault_handler_t handler, >> - void *data) >> + u32 flags, void *data) >> { >> return -ENODEV; >> } >> diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h >> index e1d9e75f2c94..0ce0dfb7713e 100644 >> --- a/include/uapi/linux/iommu.h >> +++ b/include/uapi/linux/iommu.h >> @@ -85,6 +85,8 @@ struct iommu_fault_unrecoverable { >> * When IOMMU_FAULT_PAGE_RESPONSE_NEEDS_PASID is set, the page response >> * must have the same PASID value as the page request. When it is clear, >> * the page response should not have a PASID. >> + * If IOMMU_FAULT_PAGE_REQUEST_L2 is set, the fault occurred at the >> + * second level/stage, otherwise, occurred at the first level. >> * @pasid: Process Address Space ID >> * @grpid: Page Request Group Index >> * @perm: requested page permissions (IOMMU_FAULT_PERM_* values) >> @@ -96,6 +98,7 @@ struct iommu_fault_page_request { >> #define IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE (1 << 1) >> #define IOMMU_FAULT_PAGE_REQUEST_PRIV_DATA (1 << 2) >> #define IOMMU_FAULT_PAGE_RESPONSE_NEEDS_PASID (1 << 3) >> +#define IOMMU_FAULT_PAGE_REQUEST_L2 (1 << 4) >> __u32 flags; >> __u32 pasid; >> __u32 grpid; >> > > Best regards, > baolu > .
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c index ee66d1f4cb81..5de9432349d4 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c @@ -482,7 +482,8 @@ static int arm_smmu_master_sva_enable_iopf(struct arm_smmu_master *master) if (ret) return ret; - ret = iommu_register_device_fault_handler(dev, iommu_queue_iopf, dev); + ret = iommu_register_device_fault_handler(dev, iommu_queue_iopf, + IOPF_REPORT_FLAT, dev); if (ret) { iopf_queue_remove_device(master->smmu->evtq.iopf, dev); return ret; diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c index 363744df8d51..f40529d0075d 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c @@ -1447,10 +1447,6 @@ static int arm_smmu_handle_evt(struct arm_smmu_device *smmu, u64 *evt) return -EOPNOTSUPP; } - /* Stage-2 is always pinned at the moment */ - if (evt[1] & EVTQ_1_S2) - return -EFAULT; - if (evt[1] & EVTQ_1_RnW) perm |= IOMMU_FAULT_PERM_READ; else @@ -1468,13 +1464,18 @@ static int arm_smmu_handle_evt(struct arm_smmu_device *smmu, u64 *evt) .flags = IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE, .grpid = FIELD_GET(EVTQ_1_STAG, evt[1]), .perm = perm, - .addr = FIELD_GET(EVTQ_2_ADDR, evt[2]), }; if (ssid_valid) { flt->prm.flags |= IOMMU_FAULT_PAGE_REQUEST_PASID_VALID; flt->prm.pasid = FIELD_GET(EVTQ_0_SSID, evt[0]); } + + if (evt[1] & EVTQ_1_S2) { + flt->prm.flags |= IOMMU_FAULT_PAGE_REQUEST_L2; + flt->prm.addr = FIELD_GET(EVTQ_3_IPA, evt[3]); + } else + flt->prm.addr = FIELD_GET(EVTQ_2_ADDR, evt[2]); } else { flt->type = IOMMU_FAULT_DMA_UNRECOV; flt->event = (struct iommu_fault_unrecoverable) { diff --git a/drivers/iommu/io-pgfault.c b/drivers/iommu/io-pgfault.c index 1df8c1dcae77..abf16e06bcf5 100644 --- a/drivers/iommu/io-pgfault.c +++ b/drivers/iommu/io-pgfault.c @@ -195,10 +195,6 @@ int iommu_queue_iopf(struct iommu_fault *fault, void *cookie) lockdep_assert_held(¶m->lock); - if (fault->type != IOMMU_FAULT_PAGE_REQ) - /* Not a recoverable page fault */ - return -EOPNOTSUPP; - /* * As long as we're holding param->lock, the queue can't be unlinked * from the device and therefore cannot disappear. diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index d0b0a15dba84..cb1d93b00f7d 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -1056,6 +1056,40 @@ int iommu_group_unregister_notifier(struct iommu_group *group, } EXPORT_SYMBOL_GPL(iommu_group_unregister_notifier); +/* + * iommu_update_device_fault_handler - Update the device fault handler via flags + * @dev: the device + * @mask: bits(not set) to clear + * @set: bits to set + * + * Update the device fault handler installed by + * iommu_register_device_fault_handler(). + * + * Return 0 on success, or an error. + */ +int iommu_update_device_fault_handler(struct device *dev, u32 mask, u32 set) +{ + struct dev_iommu *param = dev->iommu; + int ret = 0; + + if (!param) + return -EINVAL; + + mutex_lock(¶m->lock); + + if (param->fault_param) { + ret = -EINVAL; + goto out_unlock; + } + + param->fault_param->flags = (param->fault_param->flags & mask) | set; + +out_unlock: + mutex_unlock(¶m->lock); + return ret; +} +EXPORT_SYMBOL_GPL(iommu_update_device_fault_handler); + /** * iommu_register_device_fault_handler() - Register a device fault handler * @dev: the device @@ -1076,11 +1110,14 @@ EXPORT_SYMBOL_GPL(iommu_group_unregister_notifier); */ int iommu_register_device_fault_handler(struct device *dev, iommu_dev_fault_handler_t handler, - void *data) + u32 flags, void *data) { struct dev_iommu *param = dev->iommu; int ret = 0; + if (flags & IOPF_REPORT_FLAT && flags & IOPF_REPORT_NESTED) + return -EINVAL; + if (!param) return -EINVAL; @@ -1099,6 +1136,7 @@ int iommu_register_device_fault_handler(struct device *dev, goto done_unlock; } param->fault_param->handler = handler; + param->fault_param->flags = flags; param->fault_param->data = data; mutex_init(¶m->fault_param->lock); INIT_LIST_HEAD(¶m->fault_param->faults); @@ -1177,6 +1215,22 @@ int iommu_report_device_fault(struct device *dev, struct iommu_fault_event *evt) goto done_unlock; } + /* The unrecoverable fault reporting is not supported at the moment. */ + if (evt->fault.type != IOMMU_FAULT_PAGE_REQ) + return -EOPNOTSUPP; + + if (evt->fault.type == IOMMU_FAULT_PAGE_REQ) { + if (fparam->flags & IOPF_REPORT_NESTED) { + if (evt->fault.prm.flags & IOMMU_FAULT_PAGE_REQUEST_L2 && + !(fparam->flags & IOPF_REPORT_NESTED_L2_CONCERNED)) + return -EOPNOTSUPP; + if (!(evt->fault.prm.flags & IOMMU_FAULT_PAGE_REQUEST_L2) && + !(fparam->flags & IOPF_REPORT_NESTED_L1_CONCERNED)) + return -EOPNOTSUPP; + } else if (!(fparam->flags & IOPF_REPORT_FLAT)) + return -EOPNOTSUPP; + } + 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), diff --git a/include/linux/iommu.h b/include/linux/iommu.h index 86d688c4418f..f03d761e8310 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -352,12 +352,21 @@ struct iommu_fault_event { /** * struct iommu_fault_param - per-device IOMMU fault data * @handler: Callback function to handle IOMMU faults at device level + * @flags: Indicates whether the fault reporting is available under a + * specific configuration (1st/2nd-level-only(FLAT), or nested). + * IOPF_REPORT_NESTED needs to additionally know which level/stage + * is concerned. * @data: handler private data * @faults: holds the pending faults which needs response * @lock: protect pending faults list */ struct iommu_fault_param { iommu_dev_fault_handler_t handler; +#define IOPF_REPORT_FLAT (1 << 0) +#define IOPF_REPORT_NESTED (1 << 1) +#define IOPF_REPORT_NESTED_L1_CONCERNED (1 << 2) +#define IOPF_REPORT_NESTED_L2_CONCERNED (1 << 3) + u32 flags; void *data; struct list_head faults; struct mutex lock; @@ -509,9 +518,11 @@ 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_update_device_fault_handler(struct device *dev, + u32 mask, u32 set); extern int iommu_register_device_fault_handler(struct device *dev, iommu_dev_fault_handler_t handler, - void *data); + u32 flags, void *data); extern int iommu_unregister_device_fault_handler(struct device *dev); @@ -873,10 +884,16 @@ static inline int iommu_group_unregister_notifier(struct iommu_group *group, return 0; } +static inline int iommu_update_device_fault_handler(struct device *dev, + u32 mask, u32 set) +{ + return -ENODEV; +} + static inline int iommu_register_device_fault_handler(struct device *dev, iommu_dev_fault_handler_t handler, - void *data) + u32 flags, void *data) { return -ENODEV; } diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h index e1d9e75f2c94..0ce0dfb7713e 100644 --- a/include/uapi/linux/iommu.h +++ b/include/uapi/linux/iommu.h @@ -85,6 +85,8 @@ struct iommu_fault_unrecoverable { * When IOMMU_FAULT_PAGE_RESPONSE_NEEDS_PASID is set, the page response * must have the same PASID value as the page request. When it is clear, * the page response should not have a PASID. + * If IOMMU_FAULT_PAGE_REQUEST_L2 is set, the fault occurred at the + * second level/stage, otherwise, occurred at the first level. * @pasid: Process Address Space ID * @grpid: Page Request Group Index * @perm: requested page permissions (IOMMU_FAULT_PERM_* values) @@ -96,6 +98,7 @@ struct iommu_fault_page_request { #define IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE (1 << 1) #define IOMMU_FAULT_PAGE_REQUEST_PRIV_DATA (1 << 2) #define IOMMU_FAULT_PAGE_RESPONSE_NEEDS_PASID (1 << 3) +#define IOMMU_FAULT_PAGE_REQUEST_L2 (1 << 4) __u32 flags; __u32 pasid; __u32 grpid;
This patch follows the discussion here: https://lore.kernel.org/linux-acpi/YAaxjmJW+ZMvrhac@myrica/ In order to support more scenarios of using IOPF (mainly consider the nested extension), besides keeping IOMMU_DEV_FEAT_IOPF as a general capability for whether delivering faults through the IOMMU, we extend iommu_register_fault_handler() with flags and introduce IOPF_REPORT_FLAT and IOPF_REPORT_NESTED to describe the page fault reporting capability under a specific configuration. IOPF_REPORT_NESTED needs additional info to indicate which level/stage is concerned since the fault client may be interested in only one level. Signed-off-by: Shenming Lu <lushenming@huawei.com> --- .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 3 +- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 11 ++-- drivers/iommu/io-pgfault.c | 4 -- drivers/iommu/iommu.c | 56 ++++++++++++++++++- include/linux/iommu.h | 21 ++++++- include/uapi/linux/iommu.h | 3 + 6 files changed, 85 insertions(+), 13 deletions(-)