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

[Baolu Lu]

On 10/1/2026 8:12 PM, michal.camacho.romero@xxxxxxxxxxxxxxx wrote:
> From: Michal Camacho Romero <michal.camacho.romero@xxxxxxxxxxxxxxx>
>
> When Intel TXT Protection Regions (TPRs) are present in the DTPR table,
> hardware-level DMA protection is already enforced by the SINIT ACM.
> In this case:
>
> - Skip forcing IOMMU enablement in tboot_force_iommu(), since TPRs
>    already provide DMA protection.
> - Tear down PMRs during intel_iommu_init() when TPRs are active,
>    while PMRs are redundant with TPR-based protection.
> - Call tboot_disable_tprs() from parse_dmar_table() to disable
>    TPR regions early, allowing the kernel to manage DMA protection
>    prior to the OS boot.
>
> Link: https://uefi.org/sites/default/files/resources/633933_Intel_TXT_DMA_Protection_Ranges_rev_0p73.pdf
> Link: https://cdrdv2-public.intel.com/315168/315168_TXT_MLE_DG_rev_017_7.pdf
> Reviewed-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>

Please drop this Reviewed-by tag. It was carried over from an earlier
version, and the changes are substantial enough that my previous review
no longer covers them.

> Signed-off-by: Michal Camacho Romero <michal.camacho.romero@xxxxxxxxxxxxxxx>
> ---
> Thanks for the careful review. Summary of how each point is handled:
>
> Answer for Question No.1:
>     The DTPR handling is now invoked only after dmar_walk_dmar_table()
>     returns success. If DMAR parsing fails, parse_dmar_table() returns the error
>     and IOMMU initialization aborts before any TPR teardown, so the TPRs stay
>     active and DMA protection is retained. They are disabled only, if DMAR parse
>     succeeds.
>
> Answer for Question No.2:
>     I've replaced tboot_parse_dtpr_table function with the tboot_disable_tprs(),
>     which is defined in the Kernel patch
>     "[PATCH v5 1/2] x86/tboot: Add support for parsing DTPR table and disabling TPRs".
>     It is void type function. It verifies now, both if the DTPR Table is
>     present and if TXT Heap exists. If not then function prints a proper warning
>     message and returns.
>
> The rest of your suggestions (including the stale txt_heap = NULL cleanup) have
> been applied in the current patch version.
>
> Regards,
> Michal Camacho Romero
>
>   drivers/iommu/intel/dmar.c  | 18 ++++++++++++++++--
>   drivers/iommu/intel/iommu.c |  9 ++++++++-
>   2 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
> index ba675b08cd20..e8ec2dfe3e33 100644
> --- a/drivers/iommu/intel/dmar.c
> +++ b/drivers/iommu/intel/dmar.c
> @@ -672,8 +672,22 @@ parse_dmar_table(void)
>   
>   	pr_info("Host address width %d\n", dmar->width + 1);
>   	ret = dmar_walk_dmar_table(dmar, &cb);
> -	if (ret == 0 && drhd_count == 0)
> -		pr_warn(FW_BUG "No DRHD structure found in DMAR table\n");
> +	if (ret == 0) {
> +		/* After DMAR Table successful parsing disable TPRs if necessary */
> +		void __iomem           *txt_heap;
> +		struct acpi_table_dtpr *dtpr = tboot_get_dtpr_table(&txt_heap);
> +
> +		if (dtpr) {
> +			/*
> +			 * TPRs are enabled. This will also tell not to establish IOMMU
> +			 * PMRs. It is necessary to parse DTPR Table and disable active TPRs.
> +			 */
> +			tboot_disable_tprs(dtpr, &txt_heap);

I am confused about the timing here. TPRs are now disabled in
parse_dmar_table(), which is called from dmar_table_init() very early in
boot. However, the IOMMU does not actually enable DMA translation until
much later, in intel_iommu_init(). It may even be disabled via a boot
option.

What enforces DMA protection between the point where TPRs are disabled
and the point where the Intel IOMMU driver takes over by enabling DMA
translation?

The commit message says, "disable TPR regions early, allowing the kernel
to manage DMA protection prior to the OS boot", I find this difficult to
understand. Do you mind explaining it?

> +		}
> +
> +		if (drhd_count == 0)
> +			pr_warn(FW_BUG "No DRHD structure found in DMAR table\n");
> +	}
>   
>   	return ret;
>   }
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 2e3b3ab216f8..ce40b1bf0296 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -2563,6 +2563,13 @@ static __init void tboot_force_iommu(void)
>   	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_enabled())
> +		return;

TPRs have already been explicitly disabled at this point, so how can
tboot_is_tpr_enabled() return true? Does it indicate that TPRs were
enabled before the OS booted, even though the OS has disabled them
during early boot?

> +
>   	if (!dmar_can_force_on(DMAR_FORCEON_TBOOT))
>   		panic("tboot: Failed to force IOMMU on\n");
>   
> @@ -2623,7 +2630,7 @@ 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_enabled()) {
>   			for_each_iommu(iommu, drhd)
>   				iommu_disable_protect_mem_regions(iommu);
>   		}

Thanks,
baolu