Re: [PATCH v2 4/4] coco/tdx-host: Initialize trusted DMA on all capable IOMMUs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] virt/coco/tdx-host: desynchronized host/hardware state on xa_insert failure
--

commit 83cda44603ec97cb10640905999d9a617eced100
Author: Lu Baolu <baolu.lu@linux.intel.com>

coco/tdx-host: Initialize trusted DMA on all capable IOMMUs

Assigning a device to a TD for private memory access requires that DMA
from that device be translated by structures the TDX module controls.
An Intel IOMMU that implements the trusted DMA extension provides a
parallel set of translation and invalidation resources for this purpose.

> diff --git a/drivers/virt/coco/tdx-host/iommu.c b/drivers/virt/coco/tdx-host/iommu.c
> new file mode 100644
> index 0000000000000..a80e57bf26a12
> --- /dev/null
> +++ b/drivers/virt/coco/tdx-host/iommu.c
[ ... ]
> +static int tdx_iommu_setup(struct dmar_drhd_unit *drhd)
> +{
[ ... ]
> +	state = no_free_ptr(iommu_state);
> +	state->drhd = drhd;
> +	state->tdx_iommu_id = tdx_iommu_id;
> +	state->mt_pages = no_free_ptr(iommu_mt);
> +	ret = xa_insert(&tdx_iommu_xa, (unsigned long)tdx_iommu_id, state, GFP_KERNEL);
> +	if (ret) {
> +		WARN_ON(tdx_iommu_clear(state));
> +		kfree(state);
> +		return ret;
> +	}

[Severity: High]
If xa_insert() fails and tdx_iommu_clear(state) also fails, this code
unconditionally frees state and returns an error. Does this leave the
hardware state desynchronized?

When this function returns an error, the caller dmar_tdxcs_iommu_init_all() in
drivers/iommu/intel/dmar.c restores iommu->max_domain_id to its full un-halved
range and leaves drhd->tdx_mode = false:

	ret = setup(drhd);
	...
	if (ret) {
		scoped_guard(mutex, &iommu->did_lock)
			iommu->max_domain_id = ndoms;
		...
		return ret;
	}

Because tdx_iommu_clear() failed, the physical hardware is permanently left in
Secure TDX Mode, but the software state now believes it is not. Will this
cause hardware faults or fatal domain ID conflicts when the host attempts to
use domain IDs that now belong exclusively to TDX?

Unlike tdx_iommu_disable_all() which safely detects a clear failure and leaves
tdx_mode true to confine the host to the lower half of the DID space, this
error path completely loses track of the hardware state, breaking isolation.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015010705.1973284-1-baolu.lu@linux.intel.com?part=4