linux-kernel - Re: [PATCH V2 1/1] x86/tdx: Route safe halt execution via tdx_safe

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <qzl5vkhykj4anuvjrhfco5qoeuib3oskffnxnqbcszotttnnqa@up5b4xl5l55g>
Date: Mon, 3 Feb 2025 22:09:13 +0200
From: "Kirill A. Shutemov" <kirill@...temov.name>
To: Dave Hansen <dave.hansen@...el.com>
Cc: Vishal Annapurve <vannapurve@...gle.com>, x86@...nel.org, 
	linux-kernel@...r.kernel.org, pbonzini@...hat.com, seanjc@...gle.com, erdemaktas@...gle.com, 
	ackerleytng@...gle.com, jxgao@...gle.com, sagis@...gle.com, oupton@...gle.com, 
	pgonda@...gle.com, dave.hansen@...ux.intel.com, linux-coco@...ts.linux.dev, 
	chao.p.peng@...ux.intel.com, isaku.yamahata@...il.com, stable@...r.kernel.org
Subject: Re: [PATCH V2 1/1] x86/tdx: Route safe halt execution via
 tdx_safe_halt()

On Mon, Feb 03, 2025 at 10:06:28AM -0800, Dave Hansen wrote:
> 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.

If we want to go this path, I would rather move safe_halt out of
PARAVIRT_XXL. PARAVIRT_XXL is kitchen sink, no new code should touch it.

But Sean's proposal with HLT check before enabling interrupts looks better
to me.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov