Re: [PATCH v2 07/15] iommupt: Add map_pages op

[Jason Gunthorpe <jgg@nvidia.com>]

On Tue, May 13, 2025 at 05:15:50AM +0000, Ankit Soni wrote:
> > +static int increase_top(struct pt_iommu *iommu_table, struct pt_range *range,
> > +			struct pt_iommu_map_args *map)
> > +{
> > +	struct iommu_pages_list free_list = IOMMU_PAGES_LIST_INIT(free_list);
> > +	struct pt_common *common = common_from_iommu(iommu_table);
> > +	uintptr_t top_of_table = READ_ONCE(common->top_of_table);
> > +	uintptr_t new_top_of_table = top_of_table;
> > +	struct pt_table_p *table_mem;
> > +	unsigned int new_level;
> > +	spinlock_t *domain_lock;
> > +	unsigned long flags;
> > +	int ret;
> > +
> > +	while (true) {
> > +		struct pt_range top_range =
> > +			_pt_top_range(common, new_top_of_table);
> > +		struct pt_state pts = pt_init_top(&top_range);
> > +
> > +		top_range.va = range->va;
> > +		top_range.last_va = range->last_va;
> > +
> > +		if (!pt_check_range(&top_range) && map->leaf_level <= pts.level)
> > +			break;
> > +
> > +		pts.level++;
> > +		if (pts.level > PT_MAX_TOP_LEVEL ||
> > +		    pt_table_item_lg2sz(&pts) >= common->max_vasz_lg2) {
> > +			ret = -ERANGE;
> > +			goto err_free;
> > +		}
> > +
> > +		new_level = pts.level;
> > +		table_mem = table_alloc_top(
> > +			common, _pt_top_set(NULL, pts.level), map->attrs.gfp);
> > +		if (IS_ERR(table_mem))
> > +			return PTR_ERR(table_mem);
> 
> For subsequent calls for while, it should necessitate invoking goto, correct?

I don't understand this remark?

There are two loops here, no goto. The loop above is generating all
the required page table levels to reach the required level. Think of
them as being built in a linked list, but not installed into the main
table.

> > +	domain_lock = iommu_table->hw_flush_ops->get_top_lock(iommu_table);
> > +	spin_lock_irqsave(domain_lock, flags);
> > +	if (common->top_of_table != top_of_table) {
> > +		spin_unlock_irqrestore(domain_lock, flags);
> > +		ret = -EAGAIN;
> > +		goto err_free;
> > +	}

The second loop is here, where -EAGAIN makes the caller run the whole
function again.

Jason