Re: [PATCH v2 06/17] x86/virt/tdx: Re-initialize the extensions on runtime TDX module update

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Critical] Infinite loop in `tdx_ext_init()` when an external interrupt becomes pending during `stop_machine_cpuslocked()`.

Pre-existing issues:
- [High] Infinite loop in `tdx_sys_disable()` when an external interrupt becomes pending during a kernel panic or crash.
--

commit 03507e2a23bf57aa355b6cf99d6b8107f41ea488
Author: Xu Yilun <yilun.xu@linux.intel.com>

x86/virt/tdx: Re-initialize the extensions on runtime TDX module update

Runtime TDX module update introduces a mechanism to update the module
firmware while preserving and restoring TDX operations. As part of the
restoration process, the post-update initialization detects if extensions
are supported, rejects updates requiring additional memory, and
initializes the extensions via TDH.EXT.INIT.

> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 900928de373ae..4d2940f4538a2 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c

[ ... ]

> @@ -1308,6 +1308,31 @@ static __init int init_tdx_module_extensions(void)
>  	return tdx_ext_init();
>  }
>  
> +/*
> + * Mostly the same flow as init_tdx_module_extensions(), but rejects adding
> + * more memory.
> + */
> +static int update_tdx_module_extensions(void)
> +{
> +	struct tdx_sys_info_ext sysinfo_ext;
> +	int ret;
> +
> +	if (!(tdx_sysinfo.features.tdx_features0 & TDX_FEATURES0_EXT))
> +		return 0;
> +
> +	ret = get_tdx_sys_info_ext(&sysinfo_ext);
> +	if (ret)
> +		return ret;
> +
> +	if (!sysinfo_ext.ext_required)
> +		return 0;
> +
> +	if (sysinfo_ext.memory_pool_required_pages)
> +		return -EFAULT;
> +
> +	return tdx_ext_init();

[Severity: Critical]
Could this result in an infinite loop during a runtime update?

The tdx_module_run_update() function runs under stop_machine(), meaning
local interrupts are explicitly disabled. When this calls tdx_ext_init(), 
it executes the following polling loop:

    do {
        r = seamcall(TDH_EXT_INIT, &args);
    } while (r == TDX_INTERRUPTED_RESUMABLE);

If an external hardware interrupt becomes pending during the update, the TDX
module exits to the host and returns TDX_INTERRUPTED_RESUMABLE. Because the
host CPU has interrupts disabled, it cannot service the interrupt and simply
continues execution.

Does this mean the loop condition evaluates to true, immediately re-executing
the seamcall, and permanently deadlocking the system?

> +}
> +
>  static __init int init_tdx_module(void)
>  {

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but while looking at how
TDX_INTERRUPTED_RESUMABLE is handled, I noticed a similar pattern in
tdx_sys_disable():

    do {
        ret = seamcall(TDH_SYS_DISABLE, &args);
    } while (ret == TDX_INTERRUPTED_RESUMABLE || ret == TDX_SYS_BUSY);

During a kernel panic or crash, machine_crash_shutdown() executes with local
interrupts disabled. If an external interrupt becomes pending, the TDX module
exits with TDX_INTERRUPTED_RESUMABLE.

With host interrupts permanently disabled in the panic context, will this
loop endlessly and prevent the crash dump or kexec process from completing?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618081355.3253581-1-yilun.xu@linux.intel.com?part=6