On 1/31/25 18:32, Vishal Annapurve wrote: ...
Are you hinting towards a model where TDX guest prohibits such call sites from being configured? I am not sure if it's a sustainable model if we just rely on the host not advertising these features as the guest kernel can still add new paths that are not controlled by the host that lead to *_safe_halt().
Let's say we required PARAVIRT_XXL for TDX guests and had TDX setup do:
static const typeof(pv_ops) tdx_irq_ops __initconst = { .irq = { .safe_halt = tdx_safe_halt, }, };
We could get rid of a _bit_ of what TDX is doing now, like:
} else if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST)) { pr_info("using TDX aware idle routine\n"); static_call_update(x86_idle, tdx_safe_halt);
and it would also fix this issue. Right?
This commit:
bfe6ed0c6727 ("x86/tdx: Add HLT support for TDX guests")
Makes it seem totally possible:
Alternative choices like PV ops have been considered for adding safe_halt() support. But it was rejected because HLT paravirt calls only exist under PARAVIRT_XXL, and enabling it in TDX guest just for safe_halt() use case is not worth the cost.
and honestly it's seeming more "worth the cost" now since that partial approach has a bug and might have more bugs in the future.