Re: [PATCH v5 02/18] iommu: Implement IOMMU Live update FLB callbacks

[Nicolin Chen <nicolinc@nvidia.com>]

On Mon, Sep 21, 2026 at 12:48:18AM +0000, Samiullah Khawaja wrote:
> +struct iommu_flb_obj {
> +	struct mutex lock;
> +	struct iommu_flb_ser *ser;
> +
> +	struct iommu_hw_array_ser *curr_iommu_array;
> +	struct iommu_domain_array_ser *curr_domain_array;
> +	struct iommu_device_array_ser *curr_device_array;
> +};

IIUIC, there should be one pair of obj + ser in the entire system:
  - old kernel has one outgoing obj + ser
  - new kernel has one incoming obj + ser
right?

If so, things in iommu_flb_obj (except ser) are all transient, and
there is no need to preserve them across the two kernels.

It also feels redundant to have this iommu_flb_obj structure. Why
not link liveupdate_flb_op_args directly to the ser? Then, things
in iommu_flb_obj could be global?

> +static int iommu_liveupdate_flb_preserve(struct liveupdate_flb_op_args *argp)
> +{
> +	struct iommu_flb_obj *obj;
> +	struct iommu_flb_ser *ser;
> +	void *mem;
> +
> +	/* obj exists only in the current kernel to track preserved state */
> +	obj = kzalloc_obj(*obj, GFP_KERNEL);
> +	if (!obj)
> +		return -ENOMEM;
> +
> +	mutex_init(&obj->lock);
> +
> +	/* mem is allocated via KHO and will survive the kexec */
> +	mem = kho_alloc_preserve(sizeof(*ser));
> +	if (IS_ERR(mem))
> +		goto err_free_obj;
> +
> +	ser = mem;
> +	obj->ser = ser;
> +	ser->version = IOMMU_LUO_FLB_VERSION;

As version is per ser, ...

> +static int iommu_liveupdate_flb_retrieve(struct liveupdate_flb_op_args *argp)
> +{
> +	struct iommu_flb_obj *obj;
> +	struct iommu_flb_ser *ser;
> +
> +	obj = kzalloc_obj(*obj, GFP_KERNEL);
> +	if (!obj) {
> +		/*
> +		 * If retrieve fails, the finish path won't be called as
> +		 * can_finish() will fail, preventing the restore.
> +		 */
> +		return -ENOMEM;
> +	}
> +
> +	/* Data must be present and valid from the previous kernel */
> +	BUG_ON(!kho_restore_folio(argp->data));
> +
> +	mutex_init(&obj->lock);
> +	ser = phys_to_virt(argp->data);
> +	obj->ser = ser;
> +
> +	obj->curr_domain_array = iommu_liveupdate_restore_array(ser->iommu_domain_array_phys);
> +	obj->curr_device_array = iommu_liveupdate_restore_array(ser->device_array_phys);
> +	obj->curr_iommu_array = iommu_liveupdate_restore_array(ser->iommu_array_phys);

... should we validate ser->version before restoring arrays?

> +/**
> + * enum iommu_type_ser - Type of the IOMMU being preserved
> + * @IOMMU_INVALID: Invalid type of IOMMU
> + *
> + * IOMMU type is stored in the IOMMU HW state to differentiate between various
> + * IOMMU HWs.
> + */
> +enum iommu_type_ser {
> +	IOMMU_INVALID,
> +};

Nit: IOMMU_* sounds too generic. Given it's ser-specific, maybe
IOMMU_SER_TYPE_*?

> +/**
> + * struct iommu_domain_ser - Serialized state of an IOMMU domain
> + * @hdr: Common object header
> + * @top_table_phys: Physical address of the top-level page table
> + * @top_level: Level of the top-level page table
> + * @vasz: Virtual Address Size

Since it comes directly from iommupt, why not just reuse:
    @max_vasz_lg2: Maximum number of bits the VA can contain
?

> +/**
> + * struct iommu_dev_map_ser - Serialized mapping between device, domain,
> + *				    and IOMMU instance.
> + * @attachment_id: ID of the attachment between device and domain.
> + * @domain_phys: Physical address of the domain
> + * @iommu_phys: Physical address of the IOMMU
> + */
> +struct iommu_dev_map_ser {
> +	u64 attachment_id;
> +	u64 domain_phys;
> +	u64 iommu_phys;
> +} __packed;

Hmm, why iommu<->domain?

An attachment (software) is between device and domain.

A device is always behind an IOMMU IOMMU HW (fixed; hardware).

Should iommu_phys be moved under iommu_device_ser directly?

> +/**
> + * struct iommu_device_ser - Serialized state of a device
> + * @hdr: Common object header
> + * @devid: Device ID
> + * @pci_domain_nr: PCI domain number
> + * @dma_owner_token: Token to identify the DMA owner of this device
> + * @domain_iommu_ser: Domain and IOMMU mapping
> + */
> +struct iommu_device_ser {
> +	struct iommu_hdr_ser hdr;
> +	u32 devid;
> +	u32 pci_domain_nr;
> +	u64 dma_owner_token;
> +	struct iommu_dev_map_ser domain_iommu_ser;

I guess this single attachment_id needs to be fixed in phase 2 for
PASID?

> +} __packed;
> +
> +/**
> + * struct iommu_hw_ser - Serialized state of an IOMMU instance
> + * @hdr: Common object header
> + * @token: Unique token for the IOMMU

Could be clearer:
@token: Unique token to identify the IOMMU instance

Nicolin