Re: [PATCH v2 06/16] iommupt: Implement preserve/unpreserve/restore callbacks

[Samiullah Khawaja]

On Thu, May 07, 2026 at 10:55:42AM +0800, Baolu Lu wrote:
> On 4/28/26 01:56, Samiullah Khawaja wrote:
> > Implement the iommu domain ops for presevation, unpresevation and
> > restoration of iommu domains for liveupdate. Use the existing page
> > walker to preserve the ioptdesc of the top_table and the lower tables.
> >
> > Preserve top_level, VASZ and FEAT Sign Extended to restore the domain in
> > the next kernel. On restore the domain has only the preserved features
> > enabled and all the other features are zeroed. This is ok since the
> > restored domain is made immutable and can only be freed. A kunit test is
> > added to verify that the IOMMU domain free can be done with trimmed
> > features.
> >
> > Signed-off-by: Samiullah Khawaja<skhawaja@xxxxxxxxxx>
> > ---
> > @@ -251,6 +252,12 @@ struct pt_iommu_cfg {
> >  #define IOMMU_PROTOTYPES(fmt)                                                  \
> >  	phys_addr_t pt_iommu_##fmt##_iova_to_phys(struct iommu_domain *domain, \
> >  						  dma_addr_t iova);            \
> > +	int pt_iommu_##fmt##_preserve(struct iommu_domain *domain,             \
> > +				      struct iommu_domain_ser *ser);           \
> > +	void pt_iommu_##fmt##_unpreserve(struct iommu_domain *domain,          \
> > +					 struct iommu_domain_ser *ser);        \
> > +	int pt_iommu_##fmt##_restore(struct iommu_domain *domain,              \
> > +				     struct iommu_domain_ser *ser);            \
> >  	int pt_iommu_##fmt##_read_and_clear_dirty(                             \
> >  		struct iommu_domain *domain, unsigned long iova, size_t size,  \
> >  		unsigned long flags, struct iommu_dirty_bitmap *dirty);        \
> > @@ -266,12 +273,22 @@ struct pt_iommu_cfg {
> >  	};                              \
> >  	IOMMU_PROTOTYPES(fmt)
> > +#ifdef CONFIG_IOMMU_LIVEUPDATE
> > +#define IOMMU_PT_LIVEUPDATE_OPS(fmt)			\
> > +	, .preserve = &pt_iommu_##fmt##_preserve,	\
> > +	.unpreserve = &pt_iommu_##fmt##_unpreserve,	\
> > +	.restore = &pt_iommu_##fmt##_restore
>
> Nit: would it look better if we put it like this?
>
> #define IOMMU_PT_LIVEUPDATE_OPS(fmt)			\
> 	, .preserve = &pt_iommu_##fmt##_preserve	\
> 	, .unpreserve = &pt_iommu_##fmt##_unpreserve	\
> 	, .restore = &pt_iommu_##fmt##_restore

Agreed. I will change this.
> > +#else
> > +#define IOMMU_PT_LIVEUPDATE_OPS(fmt)
> > +#endif
>
> Thanks,
> baolu