Re: [PATCH v2 09/16] iommu/vt-d: Restore IOMMU state and reclaimed domain ids

[Samiullah Khawaja <skhawaja@google.com>]

On Tue, May 19, 2026 at 09:46:52PM +0000, Pranjal Shrivastava wrote:
>On Mon, Apr 27, 2026 at 05:56:26PM +0000, Samiullah Khawaja wrote:
>> During boot fetch the preserved state of IOMMU unit and if found then
>> restore the state.
>>
>> - Reuse the root_table that was preserved in the previous kernel.
>> - Reclaim the domain ids of the preserved domains for each preserved
>>   devices so these are not acquired by another domain.
>>
>> Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
>> ---
>>  drivers/iommu/intel/iommu.c      | 55 ++++++++++++++++++++++--------
>>  drivers/iommu/intel/iommu.h      |  7 ++++
>>  drivers/iommu/intel/liveupdate.c | 57 ++++++++++++++++++++++++++++++++
>>  3 files changed, 105 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
>> index 68fecd4e57fa..4118a0861f38 100644
>> --- a/drivers/iommu/intel/iommu.c
>> +++ b/drivers/iommu/intel/iommu.c
>> @@ -670,10 +670,17 @@ void dmar_fault_dump_ptes(struct intel_iommu *iommu, u16 source_id,
>>  #endif
>>
>>  /* iommu handling */
>> -static int iommu_alloc_root_entry(struct intel_iommu *iommu)
>> +static int iommu_alloc_root_entry(struct intel_iommu *iommu,
>> +				  struct iommu_hw_ser *iommu_ser)
>>  {
>>  	struct root_entry *root;
>>
>> +	if (iommu_ser) {
>> +		intel_iommu_liveupdate_restore_root_table(iommu, iommu_ser);
>> +		__iommu_flush_cache(iommu, iommu->root_entry, ROOT_SIZE);
>> +		return 0;
>> +	}
>> +
>
>Minor nit: I still believe this condition block can be moved into the
>caller? Since the called fetches iommu_ser, it can call this as a stand
>alone helper and bypass calling iommu_alloc_root_entry if (iommu_ser).

Agreed. Also as Baolu suggested the cache flush is not needed, so we can
move it out as it is self-contained. Will update this.
>
>>  	root = iommu_alloc_pages_node_sz(iommu->node, GFP_ATOMIC, SZ_4K);
>>  	if (!root) {
>>  		pr_err("Allocating root entry for %s failed\n",
>> @@ -992,15 +999,16 @@ static void disable_dmar_iommu(struct intel_iommu *iommu)
>>  		iommu_disable_translation(iommu);
>>  }
>>

[snip]
>
>> +
>> +static int _restore_used_domain_ids(struct iommu_device_ser *ser, void *arg)
>> +{
>> +	int id = ser->domain_iommu_ser.attachment_id;
>> +	struct iommu_hw_ser *iommu_hw_ser;
>> +	struct intel_iommu *iommu = arg;
>> +
>> +	iommu_hw_ser = phys_to_virt(ser->domain_iommu_ser.iommu_phys);
>
>We should check for iommu_phys being NULL here.. I know corruptions can
>be funnier but a WARN_ON(!iommu_phys) could help catch NULL corruptions

Agreed. I will add a WARN_ON here.
>
>> +	if (iommu_hw_ser->type != IOMMU_INTEL)
>> +		return 0;
>> +
>> +	/* Only allocate domain ID from associated IOMMU HW unit */
>> +	if (iommu_hw_ser->intel.phys_addr != iommu->reg_phys)
>> +		return 0;
>> +
>> +	/*
>> +	 * This can fail as multiple preserved devices can share the same domain
>> +	 * ID. Since this is done during DMAR init so these failures can be
>> +	 * ignored.
>> +	 */
>> +	ida_alloc_range(&iommu->domain_ida, id, id, GFP_ATOMIC);
>> +	return 0;
>> +}
>> +
>
>Thanks,
>Praan

Sami