Re: [PATCH 05/14] iommupt: Implement preserve/unpreserve/restore callbacks

[Jason Gunthorpe]

On Fri, Mar 20, 2026 at 09:57:08PM +0000, Pranjal Shrivastava wrote:
> > +static int __restore_tables(struct pt_range *range, void *arg,
> > +			    unsigned int level, struct pt_table_p *table)
> > +{
> > +	struct pt_state pts = pt_init(range, level, table);
> > +	int ret;
> > +
> > +	for_each_pt_level_entry(&pts) {
> > +		if (pts.type == PT_ENTRY_TABLE) {
> > +			iommu_restore_page(virt_to_phys(pts.table_lower));
> > +			ret = pt_descend(&pts, arg, __restore_tables);
> > +			if (ret)
> > +				return ret;
> 
> If pt_descend() returns an error, we immediately return ret. However, we
> have already successfully called iommu_restore_page() on pts.table_lower
> and potentially many other tables earlier in the loop or higher up in
> the tree..

It doesn't return an error, it just propogates errors from the
callbacks which this one never errors. So this is just dead code.

> > +int DOMAIN_NS(restore)(struct iommu_domain *domain, struct iommu_domain_ser *ser)
> > +{
> > +	struct pt_iommu *iommu_table =
> > +		container_of(domain, struct pt_iommu, domain);
> > +	struct pt_common *common = common_from_iommu(iommu_table);
> > +	struct pt_range range = pt_all_range(common);
> > +
> > +	iommu_restore_page(ser->top_table);
> > +
> > +	/* Free new table */
> > +	iommu_free_pages(range.top_table);
> > +
> > +	/* Set the restored top table */
> > +	pt_top_set(common, phys_to_virt(ser->top_table), ser->top_level);
> > +
> > +	/* Restore all pages*/
> > +	range = pt_all_range(common);
> > +	return pt_walk_range(&range, __restore_tables, NULL);

This should probably be doing something with the FEAT flags and
ias/oas too or do you imagine the calling driver has to deal with
that?

Jason