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

[Samiullah Khawaja]

On Fri, Apr 10, 2026 at 11:16:52AM -0300, Jason Gunthorpe wrote:
> 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?

During boot the iommu_domain is recreated in driver and it sets up the
FEAT flags and ias/oas properly. Then this generic callback is used to
restore the page tables.

Currently the FEAT flags of a domain are not explicitly preserved, I
will preserve them and error out here if there is a mismatch.
> Jason