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

[Xu Yilun <yilun.xu@linux.intel.com>]

On Fri, Dec 19, 2025 at 11:51:15AM +0000, Jonathan Cameron wrote:
> 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.

I agree. My change below:

> 
> > +	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;
> > +}

-----------8<----------------------

diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
index 645b72270967..fd14de8775b6 100644
--- a/drivers/iommu/intel/dmar.c
+++ b/drivers/iommu/intel/dmar.c
@@ -1041,7 +1041,7 @@ static int keyp_config_unit_tvm_usable(union acpi_subtable_headers *header,
        int *tvm_usable = arg;

        if (acpi_cu->flags & ACPI_KEYP_F_TVM_USABLE)
-               *tvm_usable = true;
+               *tvm_usable = 1;

        return 0;
 }
@@ -1053,15 +1053,15 @@ static bool platform_is_tdxc_enhanced(void)

        /* only need to parse once */
        if (tvm_usable != -1)
-               return tvm_usable;
+               return !!tvm_usable;

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

-       return tvm_usable;
+       return !!tvm_usable;
 }


> 
> 
>