Re: [PATCH v2 1/4] x86/virt/tdx: Add SEAMCALL wrappers for IOMMU setup/clear

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] x86/virt/tdx: missing clflush on IOMMU parameter pages
--

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

x86/virt/tdx: Add SEAMCALL wrappers for IOMMU setup/clear

This commit adds SEAMCALL wrappers for TDH.IOMMU.SETUP and TDH.IOMMU.CLEAR.
These functions transition an IOMMU into and out of Secure TDX Mode to
provide the trusted foundation required before enabling TEE-IO security
protocols.

> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 1b9ff749dd8e0..5d31c1b0890ae 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c

[ ... ]

> @@ -2034,3 +2034,59 @@ void tdx_sys_disable(void)
>  	if (ret && (ret & TDX_SW_ERROR) != TDX_SW_ERROR)
>  		pr_err("TDH.SYS.DISABLE failed: 0x%016llx\n", ret);
>  }
> +
> +/*
> + * Wrapper for TDH.IOMMU.SETUP leaf, which transitions the IOMMU and related
> + * I/O stack into Secure TDX Mode.
> + *
> + * The @root page format is defined by the TDX Connect ABI spec (Table 3.35,
> + * Structure of IOMMU_MT Parameter): the first two entries describe the size
> + * and HPA (Host Physical Address) of the two contiguous buffers for the
> + * invalidation queue; the remaining entries provide HPAs of
> + * IOMMU_MT_PAGES_COUNT metadata pages. IOMMU_MT_PAGES_COUNT is obtained from
> + * trusted IOMMU global metadata.
> + */
> +u64 tdh_iommu_setup(u64 reg_base, void *root, u64 *tdx_iommu_id)
> +{
> +	struct tdx_module_args args = {
> +		.rcx = reg_base,
> +		.rdx = __pa(root),
> +	};

[Severity: High]
Does the root page need to be flushed from the CPU cache before being passed
to the TDX module?

The TDX module accesses memory using physical addresses. If CPU-cached,
unflushed memory pages are passed to the module during trusted DMA
initialization, it can lead to cache paradoxes and machine checks,
potentially causing host kernel panics.

Other TDX page-adding wrappers typically ensure pages are flushed. While the
caller is responsible for populating the parameter page, the caller introduced
in the follow-up patch (tdx_iommu_setup in iommu.c) allocates and populates
the metadata pages but does not appear to perform a cache flush either.

Should tdh_iommu_setup() perform the flush, or does the caller need to be
updated to handle it?

> +	u64 ret;
> +
> +	/*
> +	 * Don't loop forever:
> +	 *
> +	 *  - TDX_INTERRUPTED_RESUMABLE guarantees forward progress between
> +	 *    calls.
> +	 *  - On TDX_INTERRUPTED_RESUMABLE, all registers except RAX remain
> +	 *    unchanged.
> +	 */

[ ... ]

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