Re: [PATCH v2 05/16] iommu/pages: Add APIs to preserve/unpreserve/restore iommu pages

[Samiullah Khawaja]

On Mon, May 18, 2026 at 02:23:17PM +0000, Pranjal Shrivastava wrote:
> On Mon, Apr 27, 2026 at 05:56:22PM +0000, Samiullah Khawaja wrote:
> > IOMMU pages are allocated/freed using APIs using struct ioptdesc. For
> > the proper preservation and restoration of ioptdesc add helper
> > functions.
> >
> > Signed-off-by: Samiullah Khawaja <skhawaja@xxxxxxxxxx>
> > ---
> >  drivers/iommu/iommu-pages.c | 108 ++++++++++++++++++++++++++++++++++--
> >  drivers/iommu/iommu-pages.h |  30 ++++++++++
> >  2 files changed, 134 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/iommu/iommu-pages.c b/drivers/iommu/iommu-pages.c
> > index 3bab175d8557..b1ffeb930e6d 100644
> > --- a/drivers/iommu/iommu-pages.c
> > +++ b/drivers/iommu/iommu-pages.c
> > @@ -6,6 +6,7 @@
> >  #include "iommu-pages.h"
> >  #include <linux/dma-mapping.h>
> >  #include <linux/gfp.h>
> > +#include <linux/kexec_handover.h>
> >  #include <linux/mm.h>
>
> [snip]
>
> > +
> > +/**
> > + * iommu_unpreserve_pages - Unpreserve pages that were preserved in KHO
> > + * @list: List of pages to unpreserve
> > + */
> > +void iommu_unpreserve_pages(struct iommu_pages_list *list)
> > +{
> > +	struct ioptdesc *iopt;
> > +
> > +	list_for_each_entry(iopt, &list->pages, iopt_freelist_elm)
> > +		kho_unpreserve_folio(ioptdesc_folio(iopt));
> > +}
> > +EXPORT_SYMBOL_GPL(iommu_unpreserve_pages);
> > +
> > +/**
> > + * iommu_restore_page - Restore a page that was preserved in KHO
> > + * @phys: Physical address of a page
> > + */
> > +void iommu_restore_page(u64 phys)
> > +{
> > +	struct ioptdesc *iopt;
> > +	struct folio *folio;
> > +	unsigned long pgcnt;
> > +	unsigned int order;
> > +
> > +	folio = kho_restore_folio(phys);
> > +	BUG_ON(!folio);
> > +
> > +	iopt = folio_ioptdesc(folio);
> > +
> > +	/*
> > +	 * For the restored pages incoherent is set to false as these are not
> > +	 * mapped using the DMA_API. The remapping of these pages using DMA_API
> > +	 * is not needed as these are not going to be written to by the new
> > +	 * kernel.
> > +	 */
>
> Looks good.
>
> > +	iopt->incoherent = false;
> > +
> > +	order = folio_order(folio);
> > +	pgcnt = 1UL << order;
> > +	iommu_folio_update_stats(folio, pgcnt);
> > +}
> > +EXPORT_SYMBOL_GPL(iommu_restore_page);
> > +
> > +/**
> > + * iommu_preserve_pages - Preserve pages during kexec handover
> > + * @list: List of pages to preserve
> > + *
> > + * Returns 0 on success, negative error on failure
> > + */
> > +int iommu_preserve_pages(struct iommu_pages_list *list)
> > +{
> > +	struct ioptdesc *iopt;
> > +	int ret;
> > +
> > +	list_for_each_entry(iopt, &list->pages, iopt_freelist_elm) {
> > +		ret = kho_preserve_folio(ioptdesc_folio(iopt));
> > +		if (ret)
> > +			goto err;
> > +	}
> > +
> > +	return 0;
> > +
> > +err:
> > +	list_for_each_entry_continue_reverse(iopt, &list->pages, iopt_freelist_elm)
> > +		kho_unpreserve_folio(ioptdesc_folio(iopt));
> > +
>
> Looks good.
>
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(iommu_preserve_pages);
> > +#endif
> > +
>
> [snip]
>
> > +static inline int iommu_preserve_pages(struct iommu_pages_list *list)
> > +{
> > +	return -EOPNOTSUPP;
> > +}
> > +
> > +static inline void iommu_unpreserve_pages(struct iommu_pages_list *list, int count)
>
> Nit: This is inconsistent from the signature in it's counter part in
> IS_ENABLED(CONFIG_IOMMU_LIVEUPDATE)

Oh. I will update this in next revision
> Let's remove `int count` from the arguments to avoid broken builds.
>
> > +{
> > +}
> > +
> > +static inline void iommu_restore_page(u64 phys)
> > +{
> > +}
> > +#endif
> > +
>
> With that stub argument fixed,
> Reviewed-by: Pranjal Shrivastava praan@xxxxxxxxxx

Thanks Pranjal.
> Thanks,
> Praan