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

[Samiullah Khawaja <skhawaja@google.com>]

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@google.com>
>>---
>>@@ -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
>