diff options
| -rw-r--r-- | drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 197 | ||||
| -rw-r--r-- | drivers/iommu/generic_pt/iommu_pt.h | 30 | ||||
| -rw-r--r-- | drivers/iommu/intel/nested.c | 54 | ||||
| -rw-r--r-- | drivers/iommu/iommufd/device.c | 8 | ||||
| -rw-r--r-- | drivers/iommu/iommufd/eventq.c | 4 | ||||
| -rw-r--r-- | drivers/iommu/iommufd/hw_pagetable.c | 25 | ||||
| -rw-r--r-- | drivers/iommu/iommufd/io_pagetable.c | 5 | ||||
| -rw-r--r-- | drivers/iommu/iommufd/iova_bitmap.c | 26 | ||||
| -rw-r--r-- | drivers/iommu/iommufd/pages.c | 13 | ||||
| -rw-r--r-- | drivers/iommu/iommufd/selftest.c | 153 | ||||
| -rw-r--r-- | include/linux/iommu.h | 6 | ||||
| -rw-r--r-- | include/linux/iommufd.h | 2 | ||||
| -rw-r--r-- | include/uapi/linux/iommufd.h | 12 | ||||
| -rw-r--r-- | tools/testing/selftests/iommu/iommufd.c | 24 | ||||
| -rw-r--r-- | tools/testing/selftests/iommu/iommufd_fail_nth.c | 14 |
15 files changed, 375 insertions, 198 deletions
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 25982bdbcbd9..ab1078a97d80 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 @@ -322,6 +322,106 @@ struct arm_vsmmu_invalidation_cmd { }; }; +/* Reject the range field values that the spec defines as Reserved */ +static int arm_vsmmu_validate_range(struct arm_smmu_device *smmu, u64 data[2]) +{ + bool range = !!(data[0] & (CMDQ_TLBI_0_NUM | CMDQ_TLBI_0_SCALE)); + u8 ttl = FIELD_GET(CMDQ_TLBI_1_TTL, data[1]); + u8 tg = FIELD_GET(CMDQ_TLBI_1_TG, data[1]); + + /* NUM, SCALE and TTL are RES0 when TG == 0 */ + if (!tg) + return (range || ttl) ? -EIO : 0; + /* TTL == 0b01 with a 16KB TG requires SMMU_IDR5.DS */ + if (tg == 2 && ttl == 1 && !(smmu->features & ARM_SMMU_FEAT_DS)) + return -EIO; + /* NUM == 0, SCALE == 0 with TTL == 0 is a reserved combination */ + if (!range && !ttl) + return -EIO; + return 0; +} + +static int arm_vsmmu_validate_user_cmd(struct arm_vsmmu *vsmmu, u64 data[2]) +{ + struct arm_smmu_device *smmu = vsmmu->smmu; + u64 allowed[2] = { CMDQ_0_OP }; + + /* Collect the fields userspace is allowed to set for each opcode */ + switch (data[0] & CMDQ_0_OP) { + case CMDQ_OP_TLBI_NSNH_ALL: + break; + case CMDQ_OP_TLBI_NH_VA: + /* An SMMU with 8-bit ASIDs treats the upper 8 bits as RES0 */ + allowed[0] |= FIELD_PREP(CMDQ_TLBI_0_ASID, + GENMASK(smmu->asid_bits - 1, 0)); + fallthrough; + case CMDQ_OP_TLBI_NH_VAA: + /* NUM/SCALE/TG/TTL are range fields gated on FEAT_RANGE_INV */ + if (smmu->features & ARM_SMMU_FEAT_RANGE_INV) { + if (arm_vsmmu_validate_range(smmu, data)) + return -EIO; + allowed[0] |= CMDQ_TLBI_0_NUM | CMDQ_TLBI_0_SCALE; + allowed[1] |= CMDQ_TLBI_1_TG | CMDQ_TLBI_1_TTL; + /* SCALE bit 25 (values above 31) is RES0 without DS */ + if (!(smmu->features & ARM_SMMU_FEAT_DS)) + allowed[0] &= ~FIELD_PREP(CMDQ_TLBI_0_SCALE, + BIT(5)); + } + allowed[0] |= CMDQ_TLBI_0_VMID; + allowed[1] |= CMDQ_TLBI_1_LEAF | CMDQ_TLBI_1_VA_MASK; + break; + case CMDQ_OP_TLBI_NH_ASID: + /* An SMMU with 8-bit ASIDs treats the upper 8 bits as RES0 */ + allowed[0] |= FIELD_PREP(CMDQ_TLBI_0_ASID, + GENMASK(smmu->asid_bits - 1, 0)); + fallthrough; + case CMDQ_OP_TLBI_NH_ALL: + allowed[0] |= CMDQ_TLBI_0_VMID; + break; + case CMDQ_OP_ATC_INV: + /* ATC_INV is illegal unless the SMMU implements ATS */ + if (!(smmu->features & ARM_SMMU_FEAT_ATS)) + return -EIO; + /* A Size above 52 (invalidate-all) may raise a CERROR_ILL */ + if (FIELD_GET(CMDQ_ATC_1_SIZE, data[1]) > ATC_INV_SIZE_ALL) + return -EIO; + /* + * SSV/SSID/Global need substream support. SSID and Global are + * IGNORED (not RES0) when SSV == 0, so they need no SSV check. + */ + if (smmu->ssid_bits) + allowed[0] |= CMDQ_0_SSV | CMDQ_ATC_0_SSID | + CMDQ_ATC_0_GLOBAL; + allowed[0] |= CMDQ_ATC_0_SID; + allowed[1] |= CMDQ_ATC_1_SIZE | CMDQ_ATC_1_ADDR_MASK; + break; + case CMDQ_OP_CFGI_CD: + /* No SSV for CFGI_CD; SSID requires substream support */ + if (smmu->ssid_bits) + allowed[0] |= CMDQ_CFGI_0_SSID; + allowed[1] |= CMDQ_CFGI_1_LEAF; + fallthrough; + case CMDQ_OP_CFGI_CD_ALL: + allowed[0] |= CMDQ_CFGI_0_SID; + break; + default: + return -EIO; + } + + /* + * Reject any other bit, e.g. a RES0 bit or a Secure bit, before the + * command reaches the trusted main cmdq, so a guest cannot wedge the + * shared queue for every device with a CERROR_ILL. + * + * By contrast, an out-of-range address or ID value does not need a + * check: the spec defines it as CONSTRAINED UNPREDICTABLE, which is + * scoped to the guest itself and does not raise a CERROR_ILL. + */ + if ((data[0] & ~allowed[0]) || (data[1] & ~allowed[1])) + return -EIO; + return 0; +} + /* * Convert, in place, the raw invalidation command into an internal format that * can be passed to arm_smmu_cmdq_issue_cmdlist(). Internally commands are @@ -332,33 +432,40 @@ struct arm_vsmmu_invalidation_cmd { static int arm_vsmmu_convert_user_cmd(struct arm_vsmmu *vsmmu, struct arm_vsmmu_invalidation_cmd *cmd) { + u64 *data = cmd->cmd.data; + int ret; + /* Commands are le64 stored in u64 */ - cmd->cmd.data[0] = le64_to_cpu(cmd->ucmd.cmd[0]); - cmd->cmd.data[1] = le64_to_cpu(cmd->ucmd.cmd[1]); + data[0] = le64_to_cpu(cmd->ucmd.cmd[0]); + data[1] = le64_to_cpu(cmd->ucmd.cmd[1]); - switch (cmd->cmd.data[0] & CMDQ_0_OP) { + ret = arm_vsmmu_validate_user_cmd(vsmmu, data); + if (ret) + return ret; + + switch (data[0] & CMDQ_0_OP) { case CMDQ_OP_TLBI_NSNH_ALL: /* Convert to NH_ALL */ - cmd->cmd.data[0] = CMDQ_OP_TLBI_NH_ALL | - FIELD_PREP(CMDQ_TLBI_0_VMID, vsmmu->vmid); - cmd->cmd.data[1] = 0; + data[0] = CMDQ_OP_TLBI_NH_ALL | + FIELD_PREP(CMDQ_TLBI_0_VMID, vsmmu->vmid); + data[1] = 0; break; case CMDQ_OP_TLBI_NH_VA: case CMDQ_OP_TLBI_NH_VAA: case CMDQ_OP_TLBI_NH_ALL: case CMDQ_OP_TLBI_NH_ASID: - cmd->cmd.data[0] &= ~CMDQ_TLBI_0_VMID; - cmd->cmd.data[0] |= FIELD_PREP(CMDQ_TLBI_0_VMID, vsmmu->vmid); + data[0] &= ~CMDQ_TLBI_0_VMID; + data[0] |= FIELD_PREP(CMDQ_TLBI_0_VMID, vsmmu->vmid); break; case CMDQ_OP_ATC_INV: case CMDQ_OP_CFGI_CD: case CMDQ_OP_CFGI_CD_ALL: { - u32 sid, vsid = FIELD_GET(CMDQ_CFGI_0_SID, cmd->cmd.data[0]); + u32 sid, vsid = FIELD_GET(CMDQ_CFGI_0_SID, data[0]); if (arm_vsmmu_vsid_to_sid(vsmmu, vsid, &sid)) return -EIO; - cmd->cmd.data[0] &= ~CMDQ_CFGI_0_SID; - cmd->cmd.data[0] |= FIELD_PREP(CMDQ_CFGI_0_SID, sid); + data[0] &= ~CMDQ_CFGI_0_SID; + data[0] |= FIELD_PREP(CMDQ_CFGI_0_SID, sid); break; } default: @@ -371,49 +478,57 @@ int arm_vsmmu_cache_invalidate(struct iommufd_viommu *viommu, struct iommu_user_data_array *array) { struct arm_vsmmu *vsmmu = container_of(viommu, struct arm_vsmmu, core); + struct arm_vsmmu_invalidation_cmd cmds[CMDQ_BATCH_ENTRIES - 1]; struct arm_smmu_device *smmu = vsmmu->smmu; - struct arm_vsmmu_invalidation_cmd *last; - struct arm_vsmmu_invalidation_cmd *cmds; - struct arm_vsmmu_invalidation_cmd *cur; - struct arm_vsmmu_invalidation_cmd *end; + struct iommu_user_data_array batch = { + .type = array->type, + .uptr = array->uptr, + .entry_len = array->entry_len, + }; + u32 processed = 0; int ret; - - cmds = kzalloc_objs(*cmds, array->entry_num); - if (!cmds) - return -ENOMEM; - cur = cmds; - end = cmds + array->entry_num; + u32 i; static_assert(sizeof(*cmds) == 2 * sizeof(u64)); + + if (array->type != IOMMU_VIOMMU_INVALIDATE_DATA_ARM_SMMUV3) { + ret = -EINVAL; + goto out; + } + + /* A zero-length array only probes the type, validated above */ + if (!array->entry_num) + return 0; + + /* + * The core re-invokes this op for the remaining requests, so copy one + * cmdq batch worth of commands into a fixed on-stack buffer rather than + * allocating for the whole array. + */ + batch.entry_num = min_t(u32, array->entry_num, ARRAY_SIZE(cmds)); ret = iommu_copy_struct_from_full_user_array( - cmds, sizeof(*cmds), array, + cmds, sizeof(*cmds), &batch, IOMMU_VIOMMU_INVALIDATE_DATA_ARM_SMMUV3); if (ret) goto out; - last = cmds; - while (cur != end) { - ret = arm_vsmmu_convert_user_cmd(vsmmu, cur); + /* + * Convert the whole batch. Sending an illegal command is a VMM bug, so + * a single one fails the entire batch, issuing nothing. + */ + for (i = 0; i < batch.entry_num; i++) { + ret = arm_vsmmu_convert_user_cmd(vsmmu, &cmds[i]); if (ret) goto out; - - /* FIXME work in blocks of CMDQ_BATCH_ENTRIES and copy each block? */ - cur++; - if (cur != end && (cur - last) != CMDQ_BATCH_ENTRIES - 1) - continue; - - /* FIXME always uses the main cmdq rather than trying to group by type */ - ret = __arm_smmu_cmdq_issue_cmdlist(smmu, &smmu->cmdq, &last->cmd, - cur - last, true); - if (ret) { - cur--; - goto out; - } - last = cur; } + + /* FIXME always uses the main cmdq rather than trying to group by type */ + ret = __arm_smmu_cmdq_issue_cmdlist(smmu, &smmu->cmdq, &cmds->cmd, + batch.entry_num, true); + if (!ret) + processed = batch.entry_num; out: - array->entry_num = cur - cmds; - kfree(cmds); + array->entry_num = processed; return ret; } diff --git a/drivers/iommu/generic_pt/iommu_pt.h b/drivers/iommu/generic_pt/iommu_pt.h index 07ec2b3ab986..837d835e4fad 100644 --- a/drivers/iommu/generic_pt/iommu_pt.h +++ b/drivers/iommu/generic_pt/iommu_pt.h @@ -240,27 +240,14 @@ struct pt_iommu_dirty_args { }; static void record_dirty(struct pt_state *pts, - struct pt_iommu_dirty_args *dirty, - unsigned int num_contig_lg2) + struct pt_iommu_dirty_args *dirty) { - pt_vaddr_t dirty_len; - - if (num_contig_lg2 != ilog2(1)) { - unsigned int index = pts->index; - unsigned int end_index = log2_set_mod_max_t( - unsigned int, pts->index, num_contig_lg2); - - /* Adjust for being contained inside a contiguous page */ - end_index = min(end_index, pts->end_index); - dirty_len = (end_index - index) * - log2_to_int(pt_table_item_lg2sz(pts)); - } else { - dirty_len = log2_to_int(pt_table_item_lg2sz(pts)); - } + unsigned int lg2sz = pt_entry_oa_lg2sz(pts); + pt_vaddr_t start_va = log2_set_mod(pts->range->va, 0, lg2sz); + pt_vaddr_t dirty_len = log2_to_int(lg2sz); if (dirty->dirty->bitmap) - iova_bitmap_set(dirty->dirty->bitmap, pts->range->va, - dirty_len); + iova_bitmap_set(dirty->dirty->bitmap, start_va, dirty_len); if (!(dirty->flags & IOMMU_DIRTY_NO_CLEAR)) { /* @@ -268,8 +255,8 @@ static void record_dirty(struct pt_state *pts, * dirty tracking bits can't work together */ pt_entry_make_write_clean(pts); - iommu_iotlb_gather_add_range(dirty->dirty->gather, - pts->range->va, dirty_len); + iommu_iotlb_gather_add_range(dirty->dirty->gather, start_va, + dirty_len); } } @@ -289,8 +276,7 @@ static inline int __read_and_clear_dirty(struct pt_range *range, void *arg, continue; } if (pts.type == PT_ENTRY_OA && pt_entry_is_write_dirty(&pts)) - record_dirty(&pts, dirty, - pt_entry_num_contig_lg2(&pts)); + record_dirty(&pts, dirty); } return 0; } diff --git a/drivers/iommu/intel/nested.c b/drivers/iommu/intel/nested.c index 2b979bec56ce..36e0e8e85c06 100644 --- a/drivers/iommu/intel/nested.c +++ b/drivers/iommu/intel/nested.c @@ -93,7 +93,7 @@ static int intel_nested_cache_invalidate_user(struct iommu_domain *domain, { struct dmar_domain *dmar_domain = to_dmar_domain(domain); struct iommu_hwpt_vtd_s1_invalidate inv_entry; - u32 index, processed = 0; + u32 processed = 0; int ret = 0; if (array->type != IOMMU_HWPT_INVALIDATE_DATA_VTD_S1) { @@ -101,31 +101,37 @@ static int intel_nested_cache_invalidate_user(struct iommu_domain *domain, goto out; } - for (index = 0; index < array->entry_num; index++) { - ret = iommu_copy_struct_from_user_array(&inv_entry, array, - IOMMU_HWPT_INVALIDATE_DATA_VTD_S1, - index, __reserved); - if (ret) - break; - - if ((inv_entry.flags & ~IOMMU_VTD_INV_FLAGS_LEAF) || - inv_entry.__reserved) { - ret = -EOPNOTSUPP; - break; - } - - if (!IS_ALIGNED(inv_entry.addr, VTD_PAGE_SIZE) || - ((inv_entry.npages == U64_MAX) && inv_entry.addr)) { - ret = -EINVAL; - break; - } - - cache_tag_flush_range(dmar_domain, inv_entry.addr, - inv_entry.addr + nrpages_to_size(inv_entry.npages) - 1, - inv_entry.flags & IOMMU_VTD_INV_FLAGS_LEAF); - processed++; + /* + * The core re-invokes this op for the remaining requests, so handle one + * request per call. A zero-length array only probes the type, validated + * above. + */ + if (!array->entry_num) + return 0; + + ret = iommu_copy_struct_from_user_array( + &inv_entry, array, IOMMU_HWPT_INVALIDATE_DATA_VTD_S1, 0, + __reserved); + if (ret) + goto out; + + if ((inv_entry.flags & ~IOMMU_VTD_INV_FLAGS_LEAF) || + inv_entry.__reserved) { + ret = -EOPNOTSUPP; + goto out; + } + + if (!IS_ALIGNED(inv_entry.addr, VTD_PAGE_SIZE) || + (inv_entry.npages == U64_MAX && inv_entry.addr)) { + ret = -EINVAL; + goto out; } + cache_tag_flush_range(dmar_domain, inv_entry.addr, + inv_entry.addr + + nrpages_to_size(inv_entry.npages) - 1, + inv_entry.flags & IOMMU_VTD_INV_FLAGS_LEAF); + processed = 1; out: array->entry_num = processed; return ret; diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c index a664c70a6fe7..1b0bdf57ac8a 100644 --- a/drivers/iommu/iommufd/device.c +++ b/drivers/iommu/iommufd/device.c @@ -849,7 +849,8 @@ iommufd_group_do_replace_reserved_iova(struct iommufd_group *igroup, return 0; err_unresv: - iommufd_group_remove_reserved_iova(igroup, hwpt_paging); + if (!old_hwpt_paging || hwpt_paging->ioas != old_hwpt_paging->ioas) + iommufd_group_remove_reserved_iova(igroup, hwpt_paging); return rc; } @@ -889,6 +890,7 @@ iommufd_device_do_replace(struct iommufd_device *idev, ioasid_t pasid, return NULL; } + old_hwpt_paging = find_hwpt_paging(old_hwpt); if (attach_resv) { rc = iommufd_group_do_replace_reserved_iova(igroup, hwpt_paging); if (rc) @@ -899,7 +901,6 @@ iommufd_device_do_replace(struct iommufd_device *idev, ioasid_t pasid, if (rc) goto err_unresv; - old_hwpt_paging = find_hwpt_paging(old_hwpt); if (old_hwpt_paging && pasid == IOMMU_NO_PASID && (!hwpt_paging || hwpt_paging->ioas != old_hwpt_paging->ioas)) iommufd_group_remove_reserved_iova(igroup, old_hwpt_paging); @@ -920,7 +921,8 @@ iommufd_device_do_replace(struct iommufd_device *idev, ioasid_t pasid, /* Caller must destroy old_hwpt */ return old_hwpt; err_unresv: - if (attach_resv) + if (attach_resv && + (!old_hwpt_paging || hwpt_paging->ioas != old_hwpt_paging->ioas)) iommufd_group_remove_reserved_iova(igroup, hwpt_paging); err_unlock: mutex_unlock(&igroup->lock); diff --git a/drivers/iommu/iommufd/eventq.c b/drivers/iommu/iommufd/eventq.c index 747dd5155121..b8a54454dd17 100644 --- a/drivers/iommu/iommufd/eventq.c +++ b/drivers/iommu/iommufd/eventq.c @@ -396,9 +396,11 @@ static __poll_t iommufd_eventq_fops_poll(struct file *filep, static int iommufd_eventq_fops_release(struct inode *inode, struct file *filep) { struct iommufd_eventq *eventq = filep->private_data; + /* eventq may be freed once the users reference is dropped. */ + struct iommufd_ctx *ictx = eventq->ictx; refcount_dec(&eventq->obj.users); - iommufd_ctx_put(eventq->ictx); + iommufd_ctx_put(ictx); return 0; } diff --git a/drivers/iommu/iommufd/hw_pagetable.c b/drivers/iommu/iommufd/hw_pagetable.c index ef6e119c2a75..ae62db50885c 100644 --- a/drivers/iommu/iommufd/hw_pagetable.c +++ b/drivers/iommu/iommufd/hw_pagetable.c @@ -514,6 +514,8 @@ int iommufd_hwpt_invalidate(struct iommufd_ucmd *ucmd) .entry_len = cmd->entry_len, .entry_num = cmd->entry_num, }; + struct iommufd_hw_pagetable *hwpt = NULL; + struct iommufd_viommu *viommu = NULL; struct iommufd_object *pt_obj; u32 done_num = 0; int rc; @@ -540,31 +542,34 @@ int iommufd_hwpt_invalidate(struct iommufd_ucmd *ucmd) goto out; } if (pt_obj->type == IOMMUFD_OBJ_HWPT_NESTED) { - struct iommufd_hw_pagetable *hwpt = - container_of(pt_obj, struct iommufd_hw_pagetable, obj); - + hwpt = container_of(pt_obj, struct iommufd_hw_pagetable, obj); if (!hwpt->domain->ops || !hwpt->domain->ops->cache_invalidate_user) { rc = -EOPNOTSUPP; goto out_put_pt; } - rc = hwpt->domain->ops->cache_invalidate_user(hwpt->domain, - &data_array); } else if (pt_obj->type == IOMMUFD_OBJ_VIOMMU) { - struct iommufd_viommu *viommu = - container_of(pt_obj, struct iommufd_viommu, obj); - + viommu = container_of(pt_obj, struct iommufd_viommu, obj); if (!viommu->ops || !viommu->ops->cache_invalidate) { rc = -EOPNOTSUPP; goto out_put_pt; } - rc = viommu->ops->cache_invalidate(viommu, &data_array); } else { rc = -EINVAL; goto out_put_pt; } - done_num = data_array.entry_num; + do { + if (viommu) + rc = viommu->ops->cache_invalidate(viommu, &data_array); + else + rc = hwpt->domain->ops->cache_invalidate_user( + hwpt->domain, &data_array); + + done_num += data_array.entry_num; + data_array.uptr += data_array.entry_num * data_array.entry_len; + data_array.entry_num = cmd->entry_num - done_num; + } while (!rc && done_num != cmd->entry_num); out_put_pt: iommufd_put_object(ucmd->ictx, pt_obj); diff --git a/drivers/iommu/iommufd/io_pagetable.c b/drivers/iommu/iommufd/io_pagetable.c index 4e447ce74cf6..5ac736cf5bb5 100644 --- a/drivers/iommu/iommufd/io_pagetable.c +++ b/drivers/iommu/iommufd/io_pagetable.c @@ -589,7 +589,7 @@ iommu_read_and_clear_dirty(struct iommu_domain *domain, arg.iopt = iopt; arg.domain = domain; arg.dirty = &dirty; - iova_bitmap_for_each(iter, &arg, __iommu_read_and_clear_dirty); + ret = iova_bitmap_for_each(iter, &arg, __iommu_read_and_clear_dirty); if (!(flags & IOMMU_DIRTY_NO_CLEAR)) iommu_iotlb_sync(domain, &gather); @@ -605,6 +605,9 @@ int iommufd_check_iova_range(struct io_pagetable *iopt, size_t iommu_pgsize = iopt->iova_alignment; u64 last_iova; + if (!bitmap->length) + return -EINVAL; + if (check_add_overflow(bitmap->iova, bitmap->length - 1, &last_iova)) return -EOVERFLOW; diff --git a/drivers/iommu/iommufd/iova_bitmap.c b/drivers/iommu/iommufd/iova_bitmap.c index dac3e657d498..953f6a57d0ca 100644 --- a/drivers/iommu/iommufd/iova_bitmap.c +++ b/drivers/iommu/iommufd/iova_bitmap.c @@ -116,6 +116,9 @@ struct iova_bitmap { /* length of the IOVA range for the whole bitmap */ size_t length; + + /* first error from pinning a bitmap chunk; -EFAULT etc. */ + int err; }; /* @@ -384,7 +387,9 @@ static int iova_bitmap_advance_to(struct iova_bitmap *bitmap, int iova_bitmap_for_each(struct iova_bitmap *bitmap, void *opaque, iova_bitmap_fn_t fn) { - return fn(bitmap, bitmap->iova, bitmap->length, opaque); + int ret = fn(bitmap, bitmap->iova, bitmap->length, opaque); + + return ret ? ret : bitmap->err; } EXPORT_SYMBOL_NS_GPL(iova_bitmap_for_each, "IOMMUFD"); @@ -404,6 +409,18 @@ void iova_bitmap_set(struct iova_bitmap *bitmap, struct iova_bitmap_map *mapped = &bitmap->mapped; unsigned long cur_bit, last_bit, last_page_idx; + /* + * If the range cannot be recorded then the system call returns failure. + * This is to prevent loosing dirties if userspace is expecting to step + * the bitmap through the IOVA space. + */ + if (!length || length > bitmap->length || iova < bitmap->iova || + iova - bitmap->iova > bitmap->length - length) { + if (!bitmap->err) + bitmap->err = -EPROTO; + return; + } + update_indexes: if (unlikely(!iova_bitmap_mapped_range(mapped, iova, length))) { /* @@ -411,8 +428,13 @@ update_indexes: * may fail if it's out of bounds, or pinning the pages * returns an error. */ - if (iova_bitmap_advance_to(bitmap, iova)) + int ret = iova_bitmap_advance_to(bitmap, iova); + + if (ret) { + if (!bitmap->err) + bitmap->err = ret; return; + } } last_page_idx = mapped->npages - 1; diff --git a/drivers/iommu/iommufd/pages.c b/drivers/iommu/iommufd/pages.c index 404f31d8f729..2f6153108add 100644 --- a/drivers/iommu/iommufd/pages.c +++ b/drivers/iommu/iommufd/pages.c @@ -1178,6 +1178,15 @@ static int pfn_reader_fill_span(struct pfn_reader *pfns) WARN_ON(span->last_used < start_index)) return -EINVAL; + /* + * Always read a dmabuf from its phys, even where a domain already + * maps it: a domain can only report CPU memory PFNs, losing + * BATCH_MMIO. last_hole aliases last_used, so this covers any span. + */ + if (iopt_is_dmabuf(pfns->pages)) + return pfn_reader_fill_dmabuf(&pfns->dmabuf, &pfns->batch, + start_index, span->last_hole); + if (span->is_used == 1) { batch_from_xarray(&pfns->batch, &pfns->pages->pinned_pfns, start_index, span->last_used); @@ -1201,10 +1210,6 @@ static int pfn_reader_fill_span(struct pfn_reader *pfns) return 0; } - if (iopt_is_dmabuf(pfns->pages)) - return pfn_reader_fill_dmabuf(&pfns->dmabuf, &pfns->batch, - start_index, span->last_hole); - user = &pfns->user; if (start_index >= user->upages_end) { rc = pfn_reader_user_pin(user, pfns->pages, start_index, diff --git a/drivers/iommu/iommufd/selftest.c b/drivers/iommu/iommufd/selftest.c index ee706f18f7e9..25387969ab75 100644 --- a/drivers/iommu/iommufd/selftest.c +++ b/drivers/iommu/iommufd/selftest.c @@ -633,70 +633,63 @@ mock_viommu_alloc_domain_nested(struct iommufd_viommu *viommu, u32 flags, static int mock_viommu_cache_invalidate(struct iommufd_viommu *viommu, struct iommu_user_data_array *array) { - struct iommu_viommu_invalidate_selftest *cmds; - struct iommu_viommu_invalidate_selftest *cur; - struct iommu_viommu_invalidate_selftest *end; - int rc; + struct iommu_viommu_invalidate_selftest cmd; + struct mock_dev *mdev; + struct device *dev; + u32 processed = 0; + int rc = 0; + int i; - /* A zero-length array is allowed to validate the array type */ - if (array->entry_num == 0 && - array->type == IOMMU_VIOMMU_INVALIDATE_DATA_SELFTEST) { - array->entry_num = 0; - return 0; + if (array->type != IOMMU_VIOMMU_INVALIDATE_DATA_SELFTEST) { + rc = -EINVAL; + goto out; } - cmds = kzalloc_objs(*cmds, array->entry_num); - if (!cmds) - return -ENOMEM; - cur = cmds; - end = cmds + array->entry_num; + /* + * The core re-invokes this op for the remaining requests, so handle one + * request per call. A zero-length array only probes the type, validated + * above. + */ + if (!array->entry_num) + return 0; - static_assert(sizeof(*cmds) == 3 * sizeof(u32)); - rc = iommu_copy_struct_from_full_user_array( - cmds, sizeof(*cmds), array, - IOMMU_VIOMMU_INVALIDATE_DATA_SELFTEST); + rc = iommu_copy_struct_from_user_array( + &cmd, array, IOMMU_VIOMMU_INVALIDATE_DATA_SELFTEST, 0, + cache_id); if (rc) goto out; - while (cur != end) { - struct mock_dev *mdev; - struct device *dev; - int i; - - if (cur->flags & ~IOMMU_TEST_INVALIDATE_FLAG_ALL) { - rc = -EOPNOTSUPP; - goto out; - } - - if (cur->cache_id > MOCK_DEV_CACHE_ID_MAX) { - rc = -EINVAL; - goto out; - } + if (cmd.flags & ~IOMMU_TEST_INVALIDATE_FLAG_ALL) { + rc = -EOPNOTSUPP; + goto out; + } - xa_lock(&viommu->vdevs); - dev = iommufd_viommu_find_dev(viommu, - (unsigned long)cur->vdev_id); - if (!dev) { - xa_unlock(&viommu->vdevs); - rc = -EINVAL; - goto out; - } - mdev = container_of(dev, struct mock_dev, dev); + if (cmd.cache_id > MOCK_DEV_CACHE_ID_MAX) { + rc = -EINVAL; + goto out; + } - if (cur->flags & IOMMU_TEST_INVALIDATE_FLAG_ALL) { - /* Invalidate all cache entries and ignore cache_id */ - for (i = 0; i < MOCK_DEV_CACHE_NUM; i++) - mdev->cache[i] = 0; - } else { - mdev->cache[cur->cache_id] = 0; - } + xa_lock(&viommu->vdevs); + dev = iommufd_viommu_find_dev(viommu, (unsigned long)cmd.vdev_id); + if (!dev) { xa_unlock(&viommu->vdevs); - - cur++; + rc = -EINVAL; + goto out; } + mdev = container_of(dev, struct mock_dev, dev); + + if (cmd.flags & IOMMU_TEST_INVALIDATE_FLAG_ALL) { + /* Invalidate all cache entries and ignore cache_id */ + for (i = 0; i < MOCK_DEV_CACHE_NUM; i++) + mdev->cache[i] = 0; + } else { + mdev->cache[cmd.cache_id] = 0; + } + xa_unlock(&viommu->vdevs); + + processed = 1; out: - array->entry_num = cur - cmds; - kfree(cmds); + array->entry_num = processed; return rc; } @@ -877,42 +870,46 @@ mock_domain_cache_invalidate_user(struct iommu_domain *domain, struct mock_iommu_domain_nested *mock_nested = to_mock_nested(domain); struct iommu_hwpt_invalidate_selftest inv; u32 processed = 0; - int i = 0, j; int rc = 0; + int i; if (array->type != IOMMU_HWPT_INVALIDATE_DATA_SELFTEST) { rc = -EINVAL; goto out; } - for ( ; i < array->entry_num; i++) { - rc = iommu_copy_struct_from_user_array(&inv, array, - IOMMU_HWPT_INVALIDATE_DATA_SELFTEST, - i, iotlb_id); - if (rc) - break; + /* + * The core re-invokes this op for the remaining requests, so handle one + * request per call. A zero-length array only probes the type, validated + * above. + */ + if (!array->entry_num) + return 0; - if (inv.flags & ~IOMMU_TEST_INVALIDATE_FLAG_ALL) { - rc = -EOPNOTSUPP; - break; - } + rc = iommu_copy_struct_from_user_array( + &inv, array, IOMMU_HWPT_INVALIDATE_DATA_SELFTEST, 0, iotlb_id); + if (rc) + goto out; - if (inv.iotlb_id > MOCK_NESTED_DOMAIN_IOTLB_ID_MAX) { - rc = -EINVAL; - break; - } + if (inv.flags & ~IOMMU_TEST_INVALIDATE_FLAG_ALL) { + rc = -EOPNOTSUPP; + goto out; + } - if (inv.flags & IOMMU_TEST_INVALIDATE_FLAG_ALL) { - /* Invalidate all mock iotlb entries and ignore iotlb_id */ - for (j = 0; j < MOCK_NESTED_DOMAIN_IOTLB_NUM; j++) - mock_nested->iotlb[j] = 0; - } else { - mock_nested->iotlb[inv.iotlb_id] = 0; - } + if (inv.iotlb_id > MOCK_NESTED_DOMAIN_IOTLB_ID_MAX) { + rc = -EINVAL; + goto out; + } - processed++; + if (inv.flags & IOMMU_TEST_INVALIDATE_FLAG_ALL) { + /* Invalidate all mock iotlb entries and ignore iotlb_id */ + for (i = 0; i < MOCK_NESTED_DOMAIN_IOTLB_NUM; i++) + mock_nested->iotlb[i] = 0; + } else { + mock_nested->iotlb[inv.iotlb_id] = 0; } + processed = 1; out: array->entry_num = processed; return rc; @@ -2056,11 +2053,9 @@ static int iommufd_test_dmabuf_get(struct iommufd_ucmd *ucmd, } rc = dma_buf_fd(dmabuf, open_flags); - if (rc < 0) { + if (rc < 0) dma_buf_put(dmabuf); - return rc; - } - return 0; + return rc; err_free: kfree(priv->memory); diff --git a/include/linux/iommu.h b/include/linux/iommu.h index ac43b8b93f14..05f7cee1375e 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -773,8 +773,10 @@ struct iommu_ops { * passes in the cache invalidation requests, in form * of a driver data structure. The driver must update * array->entry_num to report the number of handled - * invalidation requests. The driver data structure - * must be defined in include/uapi/linux/iommufd.h + * invalidation requests. A driver may handle fewer than + * the requested, in which case the core re-invokes the + * op for the remainder. The driver data structure must + * be defined in include/uapi/linux/iommufd.h * @iova_to_phys: translate iova to physical address * @enforce_cache_coherency: Prevent any kind of DMA from bypassing IOMMU_CACHE, * including no-snoop TLPs on PCIe or other platform diff --git a/include/linux/iommufd.h b/include/linux/iommufd.h index 6e7efe83bc5d..3087f5b2def8 100644 --- a/include/linux/iommufd.h +++ b/include/linux/iommufd.h @@ -154,6 +154,8 @@ struct iommufd_hw_queue { * The @array passes in the cache invalidation requests, in * form of a driver data structure. A driver must update the * array->entry_num to report the number of handled requests. + * A driver may handle fewer than the requested entry_num, in + * which case the core re-invokes the op for the remainder. * The data structure of the array entry must be defined in * include/uapi/linux/iommufd.h * @vdevice_size: Size of the driver-defined vDEVICE structure per this vIOMMU diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h index 206fa667c782..010f47f7d3d2 100644 --- a/include/uapi/linux/iommufd.h +++ b/include/uapi/linux/iommufd.h @@ -776,7 +776,7 @@ enum iommufd_hw_info_flags { * @out_data_type: Output the iommu hardware info type as defined in the enum * iommu_hw_info_type. * @out_capabilities: Output the generic iommu capability info type as defined - * in the enum iommu_hw_capabilities. + * in the enum iommufd_hw_capabilities. * @out_max_pasid_log2: Output the width of PASIDs. 0 means no PASID support. * PCI devices turn to out_capabilities to check if the * specific capabilities is supported or not. @@ -871,6 +871,12 @@ enum iommufd_hwpt_get_dirty_bitmap_flags { * Walk the IOMMU pagetables for a given IOVA range to return a bitmap * with the dirty IOVAs. In doing so it will also by default clear any * dirty bit metadata set in the IOPTE. + * + * The IOVA range given must fully enclose any IOPTE. If the range starts or + * ends inside a dirty IOPTE then the ioctl will fail. This avoids partially + * reporting a dirty range. + * + * On failure the dirty state becomes unknown and the userspace must restart. */ struct iommu_hwpt_get_dirty_bitmap { __u32 size; @@ -948,7 +954,9 @@ struct iommu_hwpt_vtd_s1_invalidate { * CMDQ_OP_CFGI_CD * CMDQ_OP_CFGI_CD_ALL * - * -EIO will be returned if the command is not supported. + * User space must forward only valid commands: the kernel rejects, with + * -EIO, any command carrying an unsupported opcode, an unsupported field, + * or a field value that the underlying SMMU hardware does not implement. */ struct iommu_viommu_arm_smmuv3_invalidate { __aligned_le64 cmd[2]; diff --git a/tools/testing/selftests/iommu/iommufd.c b/tools/testing/selftests/iommu/iommufd.c index d44b34b05757..ca2b35e9d443 100644 --- a/tools/testing/selftests/iommu/iommufd.c +++ b/tools/testing/selftests/iommu/iommufd.c @@ -2175,9 +2175,9 @@ FIXTURE_SETUP(iommufd_dirty_tracking) test_ioctl_ioas_alloc(&self->ioas_id); /* - * For dirty testing it is important that the page size fed into - * the iommu page tables matches the size the dirty logic - * expects, or set_dirty can touch too much stuff. + * Disabling huge pages maps in PAGE_SIZE units, which can span + * multiple contiguous MOCK_PAGE_SIZE entries. The dirty tests must + * expect the entire mapping to be reported dirty. */ cmd.object_id = self->ioas_id; if (!variant->hugepages) @@ -2335,7 +2335,7 @@ TEST_F(iommufd_dirty_tracking, device_dirty_capability) TEST_F(iommufd_dirty_tracking, get_dirty_bitmap) { - uint32_t page_size = MOCK_PAGE_SIZE; + uint32_t page_size = PAGE_SIZE; uint32_t ioas_id = self->ioas_id; uint32_t hwpt_id; @@ -2377,7 +2377,7 @@ TEST_F(iommufd_dirty_tracking, get_dirty_bitmap) TEST_F(iommufd_dirty_tracking, get_dirty_bitmap_no_clear) { - uint32_t page_size = MOCK_PAGE_SIZE; + uint32_t page_size = PAGE_SIZE; uint32_t ioas_id = self->ioas_id; uint32_t hwpt_id; @@ -3613,4 +3613,16 @@ TEST_F(iommufd_device_pasid, pasid_attach) test_cmd_mock_domain_replace(self->stdev_id, self->ioas_id); } -TEST_HARNESS_MAIN +static bool iommufd_mock_available(void) +{ + return access("/sys/bus/iommufd_mock", F_OK) == 0; +} + +int main(int argc, char **argv) +{ + if (!iommufd_mock_available()) + ksft_exit_skip( + "no iommufd mock device, the tests need CONFIG_IOMMUFD_TEST=y and the iommufd module loaded\n"); + + return test_harness_run(argc, argv); +} diff --git a/tools/testing/selftests/iommu/iommufd_fail_nth.c b/tools/testing/selftests/iommu/iommufd_fail_nth.c index 25495d8dceb3..1a31fff3f56c 100644 --- a/tools/testing/selftests/iommu/iommufd_fail_nth.c +++ b/tools/testing/selftests/iommu/iommufd_fail_nth.c @@ -745,4 +745,16 @@ TEST_FAIL_NTH(basic_fail_nth, device) return 0; } -TEST_HARNESS_MAIN +static bool iommufd_mock_available(void) +{ + return access("/sys/bus/iommufd_mock", F_OK) == 0; +} + +int main(int argc, char **argv) +{ + if (!iommufd_mock_available()) + ksft_exit_skip( + "no iommufd mock device, the tests need CONFIG_IOMMUFD_TEST=y and the iommufd module loaded\n"); + + return test_harness_run(argc, argv); +} |