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

[Samiullah Khawaja <skhawaja@google.com>]

On Tue, May 19, 2026 at 01:15:08PM +0000, Pranjal Shrivastava wrote:
>On Mon, Apr 27, 2026 at 05:56:23PM +0000, 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>
>> ---
>>  drivers/iommu/generic_pt/iommu_pt.h       | 131 ++++++++++++++++++++++
>>  drivers/iommu/generic_pt/kunit_iommu_pt.h |  28 +++++
>>  include/linux/generic_pt/iommu.h          |  19 +++-
>>  3 files changed, 177 insertions(+), 1 deletion(-)
>>
>
>[...]
>
>> +static const struct pt_iommu_ops NS(ops_immutable);
>> +
>> +/**
>> + * restore() - Restore page tables and other state of a domain.
>> + * @domain: Domain to preserve
>> + *
>> + * Returns: -ERRNO on failure, 0 on success.
>> + */
>> +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;
>> +
>> +	common->max_vasz_lg2 = ser->vasz;
>> +
>> +	/* Make this domain immutable.*/
>> +	iommu_table->ops = &NS(ops_immutable);
>> +
>
>Nit: Let's consider adding a comment since we just have .deinit in ops:
>
>/*
> * Restored page tables are strictly transient and only permitted to be
> * destroyed via .deinit. Because we only preserve user-assigned devices
> * utilizing pass-through frameworks (VFIO / IOMMUFD), any concurrent or
> * subsequent map/unmap operations on a restored domain are explicitly
> * blocked at the subsystem boundary (e.g., via IOAS immutability seals).
> */
>
>This makes it explicitly clear that any nullptr dererfs occuring due to
>immutable_ops->map_range etc. (in the future) are considered as bugs.

Sounds great, will add a comment here.
>
>> +	/*
>> +	 * It is safe to override this here since this domain is immutable and
>> +	 * can only be freed.
>> +	 */
>> +	common->features = 0;
>> +	if (ser->sign_extend)
>> +		common->features |= BIT(PT_FEAT_SIGN_EXTEND);
>> +
>> +	range = pt_all_range(common);
>> +	iommu_restore_page(ser->top_table_phys);
>> +
>> +	/* Free new table */
>> +	iommu_free_pages(range.top_table);
>> +
>> +	/* Set the restored top table */
>> +	pt_top_set(common, phys_to_virt(ser->top_table_phys), ser->top_level);
>> +
>> +	/* Restore all pages*/
>> +	range = pt_all_range(common);
>> +	return pt_walk_range(&range, __restore_tables, NULL);
>> +}
>> +EXPORT_SYMBOL_NS_GPL(DOMAIN_NS(restore), "GENERIC_PT_IOMMU");
>> +#endif
>> +
>>  struct pt_unmap_args {
>>  	struct iommu_pages_list free_list;
>>  	pt_vaddr_t unmapped;
>> @@ -1138,6 +1265,10 @@ static const struct pt_iommu_ops NS(ops) = {
>>  	.deinit = NS(deinit),
>>  };
>>
>> +static const struct pt_iommu_ops NS(ops_immutable) = {
>> +	.deinit = NS(deinit),
>> +};
>> +
>
>[...]
>
>Thanks,
>Praan

Sami