Re: [PATCH v1 12/26] iommu/vt-d: Reserve the MSB domain ID bit for the TDX module

[Jonathan Cameron <jonathan.cameron@huawei.com>]

On Mon, 17 Nov 2025 10:22:56 +0800
Xu Yilun <yilun.xu@linux.intel.com> wrote:

> From: Lu Baolu <baolu.lu@linux.intel.com>
> 
> The Intel TDX Connect Architecture Specification defines some enhancements
> for the VT-d architecture to introduce IOMMU support for TEE-IO requests.
> Section 2.2, 'Trusted DMA' states that:
> 
> "I/O TLB and DID Isolation – When IOMMU is enabled to support TDX
> Connect, the IOMMU restricts the VMM’s DID setting, reserving the MSB bit
> for the TDX module. The TDX module always sets this reserved bit on the
> trusted DMA table. IOMMU tags IOTLB, PASID cache, and context entries to
> indicate whether they were created from TEE-IO transactions, ensuring
> isolation between TEE and non-TEE requests in translation caches."
> 
> Reserve the MSB in the domain ID for the TDX module's use if the
> enhancement is required, which is detected if the ECAP.TDXCS bit in the
> VT-d extended capability register is set and the TVM Usable field of the
> ACPI KEYP table is set.
> 
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Hi,
One comment inline.

Thanks,

Jonathan

> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
> index a54934c0536f..e9d65b26ad64 100644
> --- a/drivers/iommu/intel/dmar.c
> +++ b/drivers/iommu/intel/dmar.c
> @@ -1033,6 +1033,56 @@ static int map_iommu(struct intel_iommu *iommu, struct dmar_drhd_unit *drhd)
>  	return err;
>  }
>  
> +static int keyp_config_unit_tvm_usable(union acpi_subtable_headers *header,
> +				       void *arg, const unsigned long end)
> +{
> +	struct acpi_keyp_config_unit *acpi_cu =
> +		(struct acpi_keyp_config_unit *)&header->keyp;
> +	int *tvm_usable = arg;
> +
> +	if (acpi_cu->flags & ACPI_KEYP_F_TVM_USABLE)
> +		*tvm_usable = true;
As below. Be consistent on int vs bool as otherwise the subtle use of -1 is very confusing.
> +
> +	return 0;
> +}
> +
> +static bool platform_is_tdxc_enhanced(void)
> +{
> +	static int tvm_usable = -1;
> +	int ret;
> +
> +	/* only need to parse once */
> +	if (tvm_usable != -1)
> +		return tvm_usable;
> +
> +	tvm_usable = false;

This is flipping between an int and a bool which seems odd.
I'd stick to an integer then make it a bool only at return.

> +	ret = acpi_table_parse_keyp(ACPI_KEYP_TYPE_CONFIG_UNIT,
> +				    keyp_config_unit_tvm_usable, &tvm_usable);
> +	if (ret < 0)
> +		tvm_usable = false;
> +
> +	return tvm_usable;
> +}