| Message ID | 03c01be90e53f743a91b6c1376c408404b891867.1736237481.git.nicolinc@nvidia.com |
|---|---|
| State | New |
| Headers | show |
| Series | iommufd: Add vIOMMU infrastructure (Part-3: vEVENTQ) | expand |
Hi Nicolin, kernel test robot noticed the following build warnings: [auto build test WARNING on e94dc6ddda8dd3770879a132d577accd2cce25f9] url: https://github.com/intel-lab-lkp/linux/commits/Nicolin-Chen/iommufd-Keep-OBJ-IOCTL-lists-in-an-alphabetical-order/20250108-011247 base: e94dc6ddda8dd3770879a132d577accd2cce25f9 patch link: https://lore.kernel.org/r/03c01be90e53f743a91b6c1376c408404b891867.1736237481.git.nicolinc%40nvidia.com patch subject: [PATCH v5 14/14] iommu/arm-smmu-v3: Report events that belong to devices attached to vIOMMU config: arm64-randconfig-r131-20250109 (https://download.01.org/0day-ci/archive/20250109/202501091822.4ocbIobQ-lkp@intel.com/config) compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff) reproduce: (https://download.01.org/0day-ci/archive/20250109/202501091822.4ocbIobQ-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202501091822.4ocbIobQ-lkp@intel.com/ sparse warnings: (new ones prefixed by >>) >> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c:461:21: sparse: sparse: invalid assignment: &= drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c:461:21: sparse: left side has type restricted __le64 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c:461:21: sparse: right side has type unsigned long long drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c:462:21: sparse: sparse: invalid assignment: |= drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c:462:21: sparse: left side has type restricted __le64 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c:462:21: sparse: right side has type unsigned long long >> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c:464:23: sparse: sparse: cast from restricted __le64 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c:465:23: sparse: sparse: cast from restricted __le64 vim +461 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c 455 456 int arm_vmaster_report_event(struct arm_smmu_vmaster *vmaster, u64 *evt) 457 { 458 struct iommu_vevent_arm_smmuv3 vevt = 459 *(struct iommu_vevent_arm_smmuv3 *)evt; 460 > 461 vevt.evt[0] &= ~EVTQ_0_SID; 462 vevt.evt[0] |= FIELD_PREP(EVTQ_0_SID, vmaster->vsid); 463 > 464 vevt.evt[0] = cpu_to_le64(vevt.evt[0]); 465 vevt.evt[1] = cpu_to_le64(vevt.evt[1]); 466 467 return iommufd_viommu_report_event(&vmaster->vsmmu->core, 468 IOMMU_VEVENTQ_TYPE_ARM_SMMUV3, &vevt, 469 sizeof(vevt)); 470 } 471
On Mon, Jan 13, 2025 at 11:01:10AM -0800, Nicolin Chen wrote: > +/* This is basically iommu_vevent_arm_smmuv3 in u64 for conversion */ > +struct arm_vsmmu_evt { > + union { > + u64 evt[EVTQ_ENT_DWORDS]; > + struct iommu_vevent_arm_smmuv3 uevt; > + }; > +}; This doesn't seem right, don't make unions like this > int arm_vmaster_report_event(struct arm_smmu_vmaster *vmaster, u64 *evt) > { > - struct iommu_vevent_arm_smmuv3 vevt = > - *(struct iommu_vevent_arm_smmuv3 *)evt; evt is clearly not a iommu_vevent_arm_smmuv3 since it has the wrong endianess? It should stay in its own type. struct struct iommu_vevent_arm_smmuv3 uevt; uet.evt[0] = cpu_to_le64((evt[0] & ~EVTQ_0_SID) | FIELD_PREP(EVTQ_0_SID, vmaster->vsid)); for (i = 1; i != EVTQ_ENT_DWORDS; i++) uet.evt[i] = cpu_to_le64(evt[i]); Jason
On Mon, Jan 13, 2025 at 11:15:21AM -0800, Nicolin Chen wrote: > On Mon, Jan 13, 2025 at 03:06:41PM -0400, Jason Gunthorpe wrote: > > On Mon, Jan 13, 2025 at 11:01:10AM -0800, Nicolin Chen wrote: > > > > > +/* This is basically iommu_vevent_arm_smmuv3 in u64 for conversion */ > > > +struct arm_vsmmu_evt { > > > + union { > > > + u64 evt[EVTQ_ENT_DWORDS]; > > > + struct iommu_vevent_arm_smmuv3 uevt; > > > + }; > > > +}; > > > > This doesn't seem right, don't make unions like this > > This is copied from the invalidate union though... Any reason why > this is no longer good for evt v.s. cmd? In that case it was trying to be really optimal and copy in place, here we don't care so much.. Jason
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..82b4513e56f3 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 @@ -389,9 +389,15 @@ static int arm_vsmmu_cache_invalidate(struct iommufd_viommu *viommu, return ret; } +static bool arm_vsmmu_supports_veventq(unsigned int type) +{ + return type == IOMMU_VEVENTQ_TYPE_ARM_SMMUV3; +} + static const struct iommufd_viommu_ops arm_vsmmu_ops = { .alloc_domain_nested = arm_vsmmu_alloc_domain_nested, .cache_invalidate = arm_vsmmu_cache_invalidate, + .supports_veventq = arm_vsmmu_supports_veventq, }; struct iommufd_viommu *arm_vsmmu_alloc(struct device *dev, @@ -447,4 +453,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 | 22 +++++++ drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 58 +++++++++++-------- 4 files changed, 77 insertions(+), 25 deletions(-)