RE: [PATCH v5 06/15] iommupt: Add unmap_pages op

Tian, Kevin
2025-09-24T09:28:22Z

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Thursday, September 4, 2025 1:47 AM
>
> +static __maybe_unused int __unmap_range(struct pt_range *range, void
> *arg,
> + unsigned int level,
> + struct pt_table_p *table)
> +{
> + struct pt_state pts = pt_init(range, level, table);
> + struct pt_unmap_args *unmap = arg;
> + unsigned int num_oas = 0;
> + unsigned int start_index;
> + int ret = 0;
> +
> + _pt_iter_first(&pts);
> + start_index = pts.index;
> + pts.type = pt_load_entry_raw(&pts);
> + /*
> + * A starting index is in the middle of a contiguous entry
> + *
> + * The IOMMU API does not require drivers to support unmapping
> parts of
> + * large pages. Long ago VFIO would try to split maps but the current
> + * version never does.
> + *
> + * Instead when unmap reaches a partial unmap of the start of a
> large
> + * IOPTE it should remove the entire IOPTE and return that size to the
> + * caller.
> + */
> + if (pts.type == PT_ENTRY_OA) {
> + if (log2_mod(range->va, pt_entry_oa_lg2sz(&pts)))
> + return -EINVAL;
> + goto start_oa;
> + }

it's not typical goto a location inside a loop.

Actually even w/o that goto, the flow will continue to...

> +
> + do {
> + if (pts.type != PT_ENTRY_OA) {
> + bool fully_covered;
> +
> + if (pts.type != PT_ENTRY_TABLE) {
> + ret = -EINVAL;
> + break;
> + }
> +
> + if (pts.index != start_index)
> + pt_index_to_va(&pts);
> + pts.table_lower = pt_table_ptr(&pts);
> +
> + fully_covered = pt_item_fully_covered(
> + &pts, pt_table_item_lg2sz(&pts));
> +
> + ret = pt_descend(&pts, arg, __unmap_range);
> + if (ret)
> + break;
> +
> + /*
> + * If the unmapping range fully covers the table then
> we
> + * can free it as well. The clear is delayed until we
> + * succeed in clearing the lower table levels.
> + */
> + if (fully_covered) {
> + iommu_pages_list_add(&unmap->free_list,
> + pts.table_lower);
> + pt_clear_entry(&pts, ilog2(1));
> + }
> + pts.index++;
> + } else {
> + unsigned int num_contig_lg2;

...here naturally.

> +start_oa:
> + /*
> + * If the caller requested an last that falls within a
> + * single entry then the entire entry is unmapped and
> + * the length returned will be larger than requested.
> + */
> + num_contig_lg2 = pt_entry_num_contig_lg2(&pts);
> + pt_clear_entry(&pts, num_contig_lg2);
> + num_oas += log2_to_int(num_contig_lg2);
> + pts.index += log2_to_int(num_contig_lg2);
> + }
> + if (pts.index >= pts.end_index)
> + break;
> + pts.type = pt_load_entry_raw(&pts);
> + } while (true);
> +
> + unmap->unmapped += log2_mul(num_oas,
> pt_table_item_lg2sz(&pts));
> + return ret;
> +}
> +

the rest looks good:

Reviewed-by: Kevin Tian <kevin.tian@intel.com>