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

Xu Yilun
2025-12-23T10:55:39Z

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

>
>
>