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

Jason Gunthorpe
2026-04-10T14:16:55Z

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