| Message ID | 69a46c72e43ed086840be462eef731167d90a9d8.1735933254.git.nicolinc@nvidia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | iommufd: Add vIOMMU infrastructure (Part-3: vEVENTQ) | expand |
On 1/4/25 03:43, Nicolin Chen wrote: > diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h > index 0a08aa82e7cc..55e3d5a14cca 100644 > --- a/include/uapi/linux/iommufd.h > +++ b/include/uapi/linux/iommufd.h > @@ -1016,9 +1016,24 @@ struct iommu_ioas_change_process { > /** > * enum iommu_veventq_type - Virtual Event Queue Type > * @IOMMU_VEVENTQ_TYPE_DEFAULT: Reserved for future use > + * @IOMMU_VEVENTQ_TYPE_ARM_SMMUV3: ARM SMMUv3 Virtual Event Queue > */ > enum iommu_veventq_type { > IOMMU_VEVENTQ_TYPE_DEFAULT = 0, > + IOMMU_VEVENTQ_TYPE_ARM_SMMUV3 = 1, > +}; > + > +/** > + * struct iommu_vevent_arm_smmuv3 - ARM SMMUv3 Virtual Event > + * (IOMMU_VEVENTQ_TYPE_ARM_SMMUV3) > + * @evt: 256-bit ARM SMMUv3 Event record, little-endian. > + * (Refer to "7.3 Event records" in SMMUv3 HW Spec) > + * > + * StreamID field reports a virtual device ID. To receive a virtual event for a > + * device, a vDEVICE must be allocated via IOMMU_VDEVICE_ALLOC. > + */ > +struct iommu_vevent_arm_smmuv3 { > + __aligned_le64 evt[4]; > }; Nit: I think it would be more readable to add a check in the vevent reporting helper. diff --git a/drivers/iommu/iommufd/driver.c b/drivers/iommu/iommufd/driver.c index 77c34f8791ef..ccada0ada5ff 100644 --- a/drivers/iommu/iommufd/driver.c +++ b/drivers/iommu/iommufd/driver.c @@ -86,6 +86,9 @@ int iommufd_viommu_report_event(struct iommufd_viommu *viommu, if (WARN_ON_ONCE(!data_len || !event_data)) return -EINVAL; + if (WARN_ON_ONCE(type != IOMMU_VEVENTQ_TYPE_ARM_SMMUV3)) + return -EINVAL; + down_read(&viommu->veventqs_rwsem); veventq = iommufd_viommu_find_veventq(viommu, type); Or perhaps the compiler could automatically make a warning if the @type is not one of those values in enum iommu_veventq_type? Others look good to me. Thanks, baolu
On Mon, Jan 06, 2025 at 11:01:32AM +0800, Baolu Lu wrote: > On 1/4/25 03:43, Nicolin Chen wrote: > > diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h > > index 0a08aa82e7cc..55e3d5a14cca 100644 > > --- a/include/uapi/linux/iommufd.h > > +++ b/include/uapi/linux/iommufd.h > > @@ -1016,9 +1016,24 @@ struct iommu_ioas_change_process { > > /** > > * enum iommu_veventq_type - Virtual Event Queue Type > > * @IOMMU_VEVENTQ_TYPE_DEFAULT: Reserved for future use > > + * @IOMMU_VEVENTQ_TYPE_ARM_SMMUV3: ARM SMMUv3 Virtual Event Queue > > */ > > enum iommu_veventq_type { > > IOMMU_VEVENTQ_TYPE_DEFAULT = 0, > > + IOMMU_VEVENTQ_TYPE_ARM_SMMUV3 = 1, > > +}; > > + > > +/** > > + * struct iommu_vevent_arm_smmuv3 - ARM SMMUv3 Virtual Event > > + * (IOMMU_VEVENTQ_TYPE_ARM_SMMUV3) > > + * @evt: 256-bit ARM SMMUv3 Event record, little-endian. > > + * (Refer to "7.3 Event records" in SMMUv3 HW Spec) > > + * > > + * StreamID field reports a virtual device ID. To receive a virtual event for a > > + * device, a vDEVICE must be allocated via IOMMU_VDEVICE_ALLOC. > > + */ > > +struct iommu_vevent_arm_smmuv3 { > > + __aligned_le64 evt[4]; > > }; > > Nit: I think it would be more readable to add a check in the vevent > reporting helper. > > diff --git a/drivers/iommu/iommufd/driver.c b/drivers/iommu/iommufd/driver.c > index 77c34f8791ef..ccada0ada5ff 100644 > --- a/drivers/iommu/iommufd/driver.c > +++ b/drivers/iommu/iommufd/driver.c > @@ -86,6 +86,9 @@ int iommufd_viommu_report_event(struct iommufd_viommu > *viommu, > if (WARN_ON_ONCE(!data_len || !event_data)) > return -EINVAL; > > + if (WARN_ON_ONCE(type != IOMMU_VEVENTQ_TYPE_ARM_SMMUV3)) > + return -EINVAL; > + Hmm, that's a good point I think. > down_read(&viommu->veventqs_rwsem); > > veventq = iommufd_viommu_find_veventq(viommu, type); ^ | We actually have been missing a type validation entirely, so the type could have been rejected by this function. Perhaps we should add a static list of supported types to struct iommufd_viommu_ops for drivers to report so that then the core could reject from the first place during a vEVENTQ allocation. > Or perhaps the compiler could automatically make a warning if the @type > is not one of those values in enum iommu_veventq_type? Just gave that a try. Mine doesn't give any warning. Not sure if needs to be some "-W" augment though.. > Others look good to me. Thanks for the review! Nicolin
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h index 4435ad7db776..d24c3d8ee397 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h @@ -1066,6 +1066,7 @@ struct iommufd_viommu *arm_vsmmu_alloc(struct device *dev, int arm_smmu_attach_prepare_vmaster(struct arm_smmu_attach_state *state, struct iommu_domain *domain); void arm_smmu_attach_commit_vmaster(struct arm_smmu_attach_state *state); +int arm_vmaster_report_event(struct arm_smmu_vmaster *vmaster, u64 *evt); #else #define arm_smmu_hw_info NULL #define arm_vsmmu_alloc NULL @@ -1081,6 +1082,12 @@ static inline void arm_smmu_attach_commit_vmaster(struct arm_smmu_attach_state *state) { } + +static inline int arm_vmaster_report_event(struct arm_smmu_vmaster *vmaster, + u64 *evt) +{ + return -EOPNOTSUPP; +} #endif /* CONFIG_ARM_SMMU_V3_IOMMUFD */ #endif /* _ARM_SMMU_V3_H */ diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h index 0a08aa82e7cc..55e3d5a14cca 100644 --- a/include/uapi/linux/iommufd.h +++ b/include/uapi/linux/iommufd.h @@ -1016,9 +1016,24 @@ struct iommu_ioas_change_process { /** * enum iommu_veventq_type - Virtual Event Queue Type * @IOMMU_VEVENTQ_TYPE_DEFAULT: Reserved for future use + * @IOMMU_VEVENTQ_TYPE_ARM_SMMUV3: ARM SMMUv3 Virtual Event Queue */ enum iommu_veventq_type { IOMMU_VEVENTQ_TYPE_DEFAULT = 0, + IOMMU_VEVENTQ_TYPE_ARM_SMMUV3 = 1, +}; + +/** + * struct iommu_vevent_arm_smmuv3 - ARM SMMUv3 Virtual Event + * (IOMMU_VEVENTQ_TYPE_ARM_SMMUV3) + * @evt: 256-bit ARM SMMUv3 Event record, little-endian. + * (Refer to "7.3 Event records" in SMMUv3 HW Spec) + * + * StreamID field reports a virtual device ID. To receive a virtual event for a + * device, a vDEVICE must be allocated via IOMMU_VDEVICE_ALLOC. + */ +struct iommu_vevent_arm_smmuv3 { + __aligned_le64 evt[4]; }; /** diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c index 2b6253ef0e8f..f88fd2f5cae8 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c @@ -447,4 +447,20 @@ struct iommufd_viommu *arm_vsmmu_alloc(struct device *dev, return &vsmmu->core; } +int arm_vmaster_report_event(struct arm_smmu_vmaster *vmaster, u64 *evt) +{ + struct iommu_vevent_arm_smmuv3 vevt = + *(struct iommu_vevent_arm_smmuv3 *)evt; + + vevt.evt[0] &= ~EVTQ_0_SID; + vevt.evt[0] |= FIELD_PREP(EVTQ_0_SID, vmaster->vsid); + + vevt.evt[0] = cpu_to_le64(vevt.evt[0]); + vevt.evt[1] = cpu_to_le64(vevt.evt[1]); + + return iommufd_viommu_report_event(&vmaster->vsmmu->core, + IOMMU_VEVENTQ_TYPE_ARM_SMMUV3, &vevt, + sizeof(vevt)); +} + MODULE_IMPORT_NS("IOMMUFD"); 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 686c171dd273..59fbc342a095 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c @@ -1812,8 +1812,8 @@ static void arm_smmu_decode_event(struct arm_smmu_device *smmu, u64 *raw, mutex_unlock(&smmu->streams_mutex); } -static int arm_smmu_handle_event(struct arm_smmu_device *smmu, - struct arm_smmu_event *event) +static int arm_smmu_handle_event(struct arm_smmu_device *smmu, u64 *evt, + struct arm_smmu_event *event) { int ret = 0; u32 perm = 0; @@ -1831,31 +1831,30 @@ static int arm_smmu_handle_event(struct arm_smmu_device *smmu, return -EOPNOTSUPP; } - if (!event->stall) - return -EOPNOTSUPP; - - if (event->read) - perm |= IOMMU_FAULT_PERM_READ; - else - perm |= IOMMU_FAULT_PERM_WRITE; + if (event->stall) { + if (event->read) + perm |= IOMMU_FAULT_PERM_READ; + else + perm |= IOMMU_FAULT_PERM_WRITE; - if (event->instruction) - perm |= IOMMU_FAULT_PERM_EXEC; + if (event->instruction) + perm |= IOMMU_FAULT_PERM_EXEC; - if (event->privileged) - perm |= IOMMU_FAULT_PERM_PRIV; + if (event->privileged) + perm |= IOMMU_FAULT_PERM_PRIV; - flt->type = IOMMU_FAULT_PAGE_REQ; - flt->prm = (struct iommu_fault_page_request) { - .flags = IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE, - .grpid = event->stag, - .perm = perm, - .addr = event->iova, - }; + flt->type = IOMMU_FAULT_PAGE_REQ; + flt->prm = (struct iommu_fault_page_request){ + .flags = IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE, + .grpid = event->stag, + .perm = perm, + .addr = event->iova, + }; - if (event->ssv) { - flt->prm.flags |= IOMMU_FAULT_PAGE_REQUEST_PASID_VALID; - flt->prm.pasid = event->ssid; + if (event->ssv) { + flt->prm.flags |= IOMMU_FAULT_PAGE_REQUEST_PASID_VALID; + flt->prm.pasid = event->ssid; + } } mutex_lock(&smmu->streams_mutex); @@ -1865,7 +1864,16 @@ static int arm_smmu_handle_event(struct arm_smmu_device *smmu, goto out_unlock; } - ret = iommu_report_device_fault(master->dev, &fault_evt); + if (event->stall) { + ret = iommu_report_device_fault(master->dev, &fault_evt); + } else { + down_read(&master->vmaster_rwsem); + if (master->vmaster && !event->s2) + ret = arm_vmaster_report_event(master->vmaster, evt); + else + ret = -EFAULT; /* Unhandled events should be pinned */ + up_read(&master->vmaster_rwsem); + } out_unlock: mutex_unlock(&smmu->streams_mutex); return ret; @@ -1943,7 +1951,7 @@ static irqreturn_t arm_smmu_evtq_thread(int irq, void *dev) do { while (!queue_remove_raw(q, evt)) { arm_smmu_decode_event(smmu, evt, &event); - if (arm_smmu_handle_event(smmu, &event)) + if (arm_smmu_handle_event(smmu, evt, &event)) arm_smmu_dump_event(smmu, evt, &event, &rs); put_device(event.dev);
Aside from the IOPF framework, iommufd provides an additional pathway to report hardware events, via the vEVENTQ of vIOMMU infrastructure. Define an iommu_vevent_arm_smmuv3 uAPI structure, and report stage-1 events in the threaded IRQ handler. Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> --- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 7 +++ include/uapi/linux/iommufd.h | 15 +++++ .../arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 16 +++++ drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 58 +++++++++++-------- 4 files changed, 71 insertions(+), 25 deletions(-)