Re: [PATCH 4/9] iommu/vt-d: Consolidate dmar state management and force_on logic

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

On 6/4/2026 1:15 PM, Kevin Tian wrote:
> Currently the dmar state is carried by multiple variables (no_iommu,
> dmar_disabled, no_platform_optin, etc.) with error-prone force_on logic
> scattered in multiple places.
> 
> Unify the state management and centralize the policy/priority for
> various force_on scenarios.
> 
> No functional impact except one case - "intel_iommu=off" sets
> no_platform_optin which is checked in platform_optin_force_iommu()
> but not in detect_intel_iommu(), leading to ACS unnecessarily requested
> when iommu could not be forced on later. Now with the unified logic
> this becomes more consistent.
> 
> Signed-off-by: Kevin Tian<kevin.tian@intel.com>
> ---
>   drivers/iommu/intel/dmar.c  | 57 ++++++++++++++++++++++++++++++++++---
>   drivers/iommu/intel/iommu.c |  7 +++++
>   drivers/iommu/intel/iommu.h | 45 +++++++++++++++++++++++++++++
>   3 files changed, 105 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
> index e8f01e56cf46..791b91a7be29 100644
> --- a/drivers/iommu/intel/dmar.c
> +++ b/drivers/iommu/intel/dmar.c
> @@ -915,14 +915,60 @@ dmar_validate_one_drhd(struct acpi_dmar_header *entry, void *arg)
>   	return 0;
>   }
>   
> +/*
> + * Centralized helper for deciding the force_on policy
> + *
> + * dmar disabled states (for DMA Remapping) are defined from stronger
> + * disables (more negative values) to weaker disables (less negative
> + * values).
> + *
> + * When a force_on type is passed in, it is associated to a reference
> + * level for comparison. force_on is permitted when dmar is in a
> + * disabled state less negative than the reference level (if dmar is
> + * enabled then the check is always true).
> + *
> + * For supported force_on types:
> + *
> + * - DMAR_FORCEON_TBOOT: tboot strictly requires DMA remapping for secure
> + *   boot hence supersedes any user opts ("iommu=off" or "intel_iommu=off")
> + *   and weaker disables.
> + *
> + * - DMAR_FORCEON_PLATFORM: external-facing devices requires DMA
> + *   remapping to prevent malicious downstream external devices from
> + *   composing DMA attacks. force_on is permitted only if dmar is disabled
> + *   by build configurations (CONFIG_INTEL_IOMMU_DEFAULT_ON=off).
> + */

It reads like tboot successfully overrides the user's choices, whereas
the platform opt-in cannot. Could you shed more light on this? My
understanding is that "trusted boot environment" is considered stronger
than "user choices", which in turn is stronger than a "platform opt-in
hint".

If this is indeed the core design philosophy, would you mind spelling it
out explicitly in this comment? Making the exact precedence clear
will help future development follow the same philosophy.

> +bool dmar_can_force_on(enum dmar_force_on force_on)
> +{
> +	int level;
> +
> +	switch (force_on) {
> +	case DMAR_FORCEON_TBOOT:
> +		level = DMAR_DISABLED_USER;
> +		break;
> +	case DMAR_FORCEON_PLATFORM:
> +		level = DMAR_DISABLED_AUTO;
> +		break;
> +	default:
> +		pr_warn("Unsupported force_on type (%d)\n", force_on);
> +		/* '0' means returning true only when dmar is enabled */
> +		level = 0;
> +		break;
> +	}
> +
> +	return dmar_state >= level;
> +}

If this helper returns false (meaning a requested force_on type cannot
be enforced), would it be better to notify the user via a clear pr_info/
pr_warn in the kernel log?

Normally, a failure to enforce a requested "force_on" condition might
mean security or trust implications that the administrator should be
aware of.

The rest of the patch looks good to me. Thanks!

Thanks,
baolu