Re: [PATCH v4 09/18] iommu: Add APIs to get iommu and device preserved state

[Ankit Soni]

On Sat, Aug 08, 2026 at 02:27:14AM +0000, Samiullah Khawaja wrote:
> The preserved state of the device and IOMMU needs to be fetched during
> shutdown and boot in the next kernel. Add APIs that can be used to fetch
> the preserved state of a device and IOMMU. The APIs will only be used
> during shutdown and after liveupdate so no locking needed.
> 
> Reviewed-by: Pranjal Shrivastava <praan@xxxxxxxxxx>
> Signed-off-by: Samiullah Khawaja <skhawaja@xxxxxxxxxx>
> ---
>  drivers/iommu/liveupdate.c       | 122 +++++++++++++++++++++++++++++++
>  include/linux/iommu-liveupdate.h |  45 ++++++++++++
>  2 files changed, 167 insertions(+)
> 
> diff --git a/drivers/iommu/liveupdate.c b/drivers/iommu/liveupdate.c
> index 7f349ae5a124..20acf123b47a 100644
> --- a/drivers/iommu/liveupdate.c
> +++ b/drivers/iommu/liveupdate.c

../..

> @@ -262,6 +273,117 @@ void iommu_liveupdate_unregister_flb(struct liveupdate_file_handler *handler)
>  }
>  EXPORT_SYMBOL(iommu_liveupdate_unregister_flb);
>  
> +/*
> + * iommu_liveupdate_flb_get_incoming() - Helper function to get FLB state
> + * @flb_objp: Pointer to get the restored FLB object
> + *
> + * Return: 0 if FLB state found and restored, error if no data found
> + */
> +static int iommu_liveupdate_flb_get_incoming(struct iommu_flb_obj **flb_objp)
> +{
> +	struct iommu_flb_obj *flb_obj;
> +	int ret;
> +
> +	ret = liveupdate_flb_get_incoming(&iommu_flb, (void **)flb_objp);
> +	if (ret)
> +		return ret;
> +
> +	flb_obj = *flb_objp;
> +	mutex_lock(&flb_obj->lock);
> +
> +	/*
> +	 * FLB version mismatch is considered fatal for security reasons for
> +	 * now.
> +	 */
> +	if (flb_obj->ser->version != IOMMU_LUO_FLB_VERSION)
> +		goto err_fatal;
> +
> +	/*
> +	 * Array Phys of each type should be valid if the FLB was created for
> +	 * preservation. This is true even if no devices, iommus or domains were
> +	 * preserved.
> +	 */
> +	if (!flb_obj->ser->iommu_array_phys ||
> +	    !flb_obj->ser->device_array_phys ||
> +	    !flb_obj->ser->iommu_domain_array_phys)
> +		goto err_fatal;
> +
> +	mutex_unlock(&flb_obj->lock);
> +	return 0;
> +
> +err_fatal:
> +	panic("Failed to restore IOMMU Live Update FLB\n");
> +}

../..

> +/**
> + * iommu_get_preserved_data() - Get preserved data for an IOMMU HW
> + * @token: Token used to preserve this IOMMU HW
> + * @type: IOMMU type in preserved state
> + *
> + * Gets the preserved state of an IOMMU HW using token and the IOMMU type.
> + *
> + * Return: struct iommu_hw_ser on success, NULL if no preserved state found.
> + */
> +struct iommu_hw_ser *iommu_get_preserved_data(u64 token, enum iommu_type_ser type)
> +{
> +	struct iommu_hw_ser *iommu_ser = NULL;
> +	struct iommu_hw_array_ser *array;
> +	struct iommu_flb_obj *flb_obj;
> +	int ret, idx;
> +
> +	ret = iommu_liveupdate_flb_get_incoming(&flb_obj);
> +	if (ret)
> +		return NULL;

Hi,

Returning NULL for every failure loses a distinction the caller needs.
"Nothing was handed over" and "something was handed over and the lookup
failed" are different states, and this helper is the only place that can
tell them apart.

Of the errors reachable here, three mean nothing was handed over:

        -EOPNOTSUPP     live update disabled or not built
        -ENODATA        no incoming handover data
        -ENOENT         the handover carried no IOMMU FLB

An -ENOMEM out of a .retrieve() callback means the opposite, and returns
the same NULL.

The distinction matters because callers read NULL as a cold boot and act
on it. Both VT-d call sites in 10/18 do. init_dmars():

        if (!iommu_ser)
                init_translation_status(iommu);

        if (translation_pre_enabled(iommu) && !is_kdump_kernel()) {
                iommu_disable_translation(iommu);
                clear_translation_pre_enabled(iommu);
                pr_warn("Translation was enabled for %s but we are not in kdump mode\n",
                        iommu->name);
        }

and intel_iommu_add():

        if (!iommu_ser && iommu->gcmd & DMA_GCMD_TE)
                iommu_disable_translation(iommu);

On an -ENOMEM both disable an IOMMU that the previous kernel left
translating, underneath devices still doing DMA through it. The warning
attributes it to a stale enable outside kdump, so the log points away from
the actual cause.

The helper already treats this class of failure as fatal a few lines
lower. A version mismatch and a missing array pointer both reach err_fatal
and panic(), because data was handed over and cannot be trusted. A failed
fetch is the same class -- something was handed over and could not be read
back -- but it returns an error that becomes NULL, and callers read that
as a cold boot.

A single failure also spreads. luo_flb_retrieve_one() caches a failed
.retrieve() in incoming.retrieve_status and returns it directly on every
later call, so one -ENOMEM makes the lookup return NULL for every IOMMU in
the system, not just the one that hit it.

Is returning NULL for all of them intentional? If callers are meant to
treat every failure as "nothing was handed over", that is worth saying in
a comment here.

If not, the smallest fix is to return NULL for the absent cases only and
an ERR_PTR for everything else:

        ret = iommu_liveupdate_flb_get_incoming(&flb_obj);
        if (ret)
                return iommu_incoming_absent(ret) ? NULL : ERR_PTR(ret);

where iommu_incoming_absent() tests the three codes above.
intel_iommu_add() can then return PTR_ERR() directly, and init_dmars() can
goto free_iommu. Nothing else about the helper needs to change.

The whole point is that a caller can tell a cold boot from a failed lookup.

Thanks,
Ankit

> +
> +	array = phys_to_virt(flb_obj->ser->iommu_array_phys);
> +	iommu_liveupdate_for_each_arr(array) {
> +		iommu_liveupdate_for_each_obj(array, iommu_ser, idx) {
> +			if (iommu_ser->token == token && iommu_ser->type == type)
> +				goto out;
> +		}
> +	}
> +
> +	iommu_ser = NULL;
> +out:
> +	liveupdate_flb_put_incoming(&iommu_flb);
> +	return iommu_ser;
> +}
> +EXPORT_SYMBOL(iommu_get_preserved_data);
> -- 
> 2.55.0.679.g6767b8d81c-goog
>