Re: [PATCH v5 2/2] iommu/vt-d: Disable PMRs and skip force-IOMMU when TXT TPRs are active

[Baolu Lu <baolu.lu@linux.intel.com>]

On 10/12/2026 6:22 PM, Michal Camacho Romero wrote:
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 2e3b3ab216f8..ea32730701cf 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -2558,11 +2558,18 @@ static int __init probe_acpi_namespace_devices(void)
>   	return 0;
>   }
>   
> -static __init void tboot_force_iommu(void)
> +static __init void tboot_force_iommu(s64 num_of_enabled_tprs)
>   {
>   	if (!tboot_enabled() || intel_iommu_tboot_noforce)
>   		return;
>   
> +	/*
> +	 * If TPR is enabled we don't need to force IOMMU, TPR set by SINIT
> +	 * ACM will take care of DMA protection.
> +	 */
> +	if (tboot_is_tpr_protection_enabled(num_of_enabled_tprs))
> +		return;
> +

I don't quite follow this. The TPRs are disabled once DMA remapping is
enabled in intel_iommu_init(). After that, the IOMMU is the only runtime
DMA protection, which is why tboot forces it on in the first place.

With this change, booting with intel_iommu=off on a TPR-capable platform
would no longer force the IOMMU on. What happens next?

- If the TPRs are left enabled, devices can't DMA into the protected
   ranges, so the system is likely broken.

- If the TPRs are later disabled, the measured launch has no DMA
   protection at all.

Neither seems acceptable. I may have overlooked or misunderstood
something, so please explain.

>   	if (!dmar_can_force_on(DMAR_FORCEON_TBOOT))
>   		panic("tboot: Failed to force IOMMU on\n");
>   
> @@ -2584,7 +2591,15 @@ int __init intel_iommu_init(void)
>   	 * Intel IOMMU is required for a TXT/tboot launch or platform
>   	 * opt in, so enforce that.
>   	 */
> -	tboot_force_iommu();
> +	void __iomem           *txt_heap = NULL;
> +	struct acpi_table_dtpr *dtpr     = tboot_get_dtpr_table(&txt_heap);
> +	s64 num_of_enabled_tprs = 0;

Too much tboot internal state leaks into the IOMMU driver. Could tboot
keep this state internally and expose only simple helpers, for example
tboot_tpr_active() and tboot_tpr_disable()?

> +	if (dtpr) {
> +		/* For the existing DTPR Table, verify TPRs activity status */
> +		num_of_enabled_tprs = tboot_check_tprs(dtpr, &txt_heap);
> +	}
> +	
> +	tboot_force_iommu(num_of_enabled_tprs);
>   	if (!dmar_policy_force_on())
>   		platform_optin_force_iommu();
>   
> @@ -2623,7 +2638,8 @@ int __init intel_iommu_init(void)
>   		 * calling SENTER, but the kernel is expected to reset/tear
>   		 * down the PMRs.
>   		 */
> -		if (intel_iommu_tboot_noforce) {
> +		if (intel_iommu_tboot_noforce ||
> +			tboot_is_tpr_protection_enabled(num_of_enabled_tprs)) {

tboot_noforce should apply to TPRs as well. See the comment on its
definition:

/*
  * Skip forcing iommu on and avoid tboot-related kernel panics during
  * initialization when set to 1 (via intel_iommu=tboot_noforce).
  */
int intel_iommu_tboot_noforce;

It's a debugging feature to avoid kernel panics during early boot.

>   			for_each_iommu(iommu, drhd)
>   				iommu_disable_protect_mem_regions(iommu);
>   		}
> @@ -2697,7 +2713,24 @@ int __init intel_iommu_init(void)
>   			iommu_enable_translation(iommu);
>   
>   		iommu_disable_protect_mem_regions(iommu);
> +

Unnecessary change.

>   	}
> +
> +	if (tboot_is_tpr_protection_enabled(num_of_enabled_tprs) && dtpr) {
> +		/*
> +		 * TPRs are enabled. It is necessary to disable active TPRs
> +		 * to enable DMA usage by the system.
> +		 */
> +		num_of_enabled_tprs = tboot_disable_tprs(dtpr, &txt_heap,
> +		                                         num_of_enabled_tprs);
> +		if (num_of_enabled_tprs != 0)
> +			panic("tboot: Failed to disable TPRs. DMA is still blocked.\n");
> +	}
> +
> +	/* Release the TXT heap memory if it was allocated */
> +	if (txt_heap)
> +		tboot_free_txt_heap_mem(&txt_heap);
> +
>   	up_read(&dmar_global_lock);
>   
>   	pr_info("Intel(R) Virtualization Technology for Directed I/O\n");
> @@ -2707,6 +2740,9 @@ int __init intel_iommu_init(void)
>   	return 0;
>   
>   out_free_dmar:
> +	if (txt_heap)
> +		tboot_free_txt_heap_mem(&txt_heap);
> +
>   	intel_iommu_free_dmars();
>   	up_write(&dmar_global_lock);
>   	return ret;
> -- 2.55.0

Thanks,
baolu