Re: [PATCH 11/14] iommufd-lu: Persist iommu hardware pagetables for live update

Samiullah Khawaja
2026-03-03T18:51:21Z

On Tue, Mar 03, 2026 at 05:56:15AM +0000, Ankit Soni wrote:
>Hi,
>
>On Tue, Feb 03, 2026 at 10:09:45PM +0000, Samiullah Khawaja wrote:
>> From: YiFei Zhu <zhuyifei@google.com>
>>
>> The caller is expected to mark each HWPT to be preserved with an ioctl
>> call, with a token that will be used in restore. At preserve time, each
>> HWPT's domain is then called with iommu_domain_preserve to preserve the
>> iommu domain.
>>
>> The HWPTs containing dma mappings backed by unpreserved memory should
>> not be preserved. During preservation check if the mappings contained in
>> the HWPT being preserved are only file based and all the files are
>> preserved.
>>
>> The memfd file preservation check is not enough when preserving iommufd.
>> The memfd might have shrunk between the mapping and memfd preservation.
>> This means that if it shrunk some pages that are right now pinned due to
>> iommu mappings are not preserved with the memfd. Only allow iommufd
>> preservation when all the iopt_pages are file backed and the memory file
>> was seal sealed during mapping. This guarantees that all the pages that
>> were backing memfd when it was mapped are preserved.
>>
>> Once HWPT is preserved the iopt associated with the HWPT is made
>> immutable. Since the map and unmap ioctls operates directly on iopt,
>> which contains an array of domains, while each hwpt contains only one
>> domain. The logic then becomes that mapping and unmapping is prohibited
>> if any of the domains in an iopt belongs to a preserved hwpt. However,
>> tracing to the hwpt through the domain is a lot more tedious than
>> tracing through the ioas, so if an hwpt is preserved, hwpt->ioas->iopt
>> is made immutable.
>>
>> When undoing this (making the iopts mutable again), there's never
>> a need to make some iopts mutable and some kept immutable, since
>> the undo only happen on unpreserve and error path of preserve.
>> Simply iterate all the ioas and clear the immutability flag on all
>> their iopts.
>>
>> Signed-off-by: YiFei Zhu <zhuyifei@google.com>
>> Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
>> ---
>> drivers/iommu/iommufd/io_pagetable.c | 17 ++
>> drivers/iommu/iommufd/io_pagetable.h | 1 +
>> drivers/iommu/iommufd/iommufd_private.h | 25 ++
>> drivers/iommu/iommufd/liveupdate.c | 300 ++++++++++++++++++++++++
>> drivers/iommu/iommufd/main.c | 14 +-
>> drivers/iommu/iommufd/pages.c | 8 +
>> include/linux/kho/abi/iommufd.h | 39 +++
>> 7 files changed, 403 insertions(+), 1 deletion(-)
>> create mode 100644 include/linux/kho/abi/iommufd.h
>>
>> +
>> +static int iommufd_save_hwpts(struct iommufd_ctx *ictx,
>> + struct iommufd_lu *iommufd_lu,
>> + struct liveupdate_session *session)
>> +{
>> + struct iommufd_hwpt_paging *hwpt, **hwpts = NULL;
>> + struct iommu_domain_ser *domain_ser;
>> + struct iommufd_hwpt_lu *hwpt_lu;
>> + struct iommufd_object *obj;
>> + unsigned int nr_hwpts = 0;
>> + unsigned long index;
>> + unsigned int i;
>> + int rc = 0;
>> +
>> + if (iommufd_lu) {
>> + hwpts = kcalloc(iommufd_lu->nr_hwpts, sizeof(*hwpts),
>> + GFP_KERNEL);
>> + if (!hwpts)
>> + return -ENOMEM;
>> + }
>> +
>> + xa_lock(&ictx->objects);
>> + xa_for_each(&ictx->objects, index, obj) {
>> + if (obj->type != IOMMUFD_OBJ_HWPT_PAGING)
>> + continue;
>> +
>> + hwpt = container_of(obj, struct iommufd_hwpt_paging, common.obj);
>> + if (!hwpt->lu_preserve)
>> + continue;
>> +
>> + if (hwpt->ioas) {
>> + /*
>> + * Obtain exclusive access to the IOAS and IOPT while we
>> + * set immutability
>> + */
>> + mutex_lock(&hwpt->ioas->mutex);
>> + down_write(&hwpt->ioas->iopt.domains_rwsem);
>> + down_write(&hwpt->ioas->iopt.iova_rwsem);
>
>Taking mutex/rwsem under spin-lock is not a good idea.

Agreed. I will move this out by taking a reference on the object and
then setting it separately without xa_lock.
>
>> +
>> + hwpt->ioas->iopt.lu_map_immutable = true;
>> +
>> + up_write(&hwpt->ioas->iopt.iova_rwsem);
>> + up_write(&hwpt->ioas->iopt.domains_rwsem);
>> + mutex_unlock(&hwpt->ioas->mutex);
>> + }
>> +
>> + if (!hwpt->common.domain) {
>> + rc = -EINVAL;
>> + xa_unlock(&ictx->objects);
>> + goto out;
>> + }
>> +
>> + if (!iommufd_lu) {
>> + rc = check_iopt_pages_preserved(session, hwpt);
>> + if (rc) {
>> + xa_unlock(&ictx->objects);
>> + goto out;
>> + }
>> + } else if (iommufd_lu) {
>
>Redundant else_if().

Will remove
>
>-Ankit
>
>> + hwpts[nr_hwpts] = hwpt;
>> + hwpt_lu = &iommufd_lu->hwpts[nr_hwpts];
>> +
>> + hwpt_lu->token = hwpt->lu_token;
>> + hwpt_lu->reclaimed = false;
>> + }
>> +
>> + nr_hwpts++;
>> + }
>> + xa_unlock(&ictx->objects);
>> +

Thanks for looking into this.

Sami