linux-kernel - RE: [RFC PATCH] clocksource: hyper-v: Enable the tsc

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]

Date: Fri, 24 May 2024 08:45:42 +0000
From: Dexuan Cui <decui@...rosoft.com>
To: Dave Hansen <dave.hansen@...el.com>, "x86@...nel.org" <x86@...nel.org>,
	"linux-coco@...ts.linux.dev" <linux-coco@...ts.linux.dev>, "bp@...en8.de"
	<bp@...en8.de>, "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
	Haiyang Zhang <haiyangz@...rosoft.com>, "hpa@...or.com" <hpa@...or.com>,
	"kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>, KY
 Srinivasan <kys@...rosoft.com>, "luto@...nel.org" <luto@...nel.org>,
	"mingo@...hat.com" <mingo@...hat.com>, "peterz@...radead.org"
	<peterz@...radead.org>, "sathyanarayanan.kuppuswamy@...ux.intel.com"
	<sathyanarayanan.kuppuswamy@...ux.intel.com>, "tglx@...utronix.de"
	<tglx@...utronix.de>, "wei.liu@...nel.org" <wei.liu@...nel.org>, jason
	<jason@...c4.com>, "mhklinux@...look.com" <mhklinux@...look.com>,
	"thomas.lendacky@....com" <thomas.lendacky@....com>, "tytso@....edu"
	<tytso@....edu>, "ardb@...nel.org" <ardb@...nel.org>
CC: "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Tianyu Lan
	<Tianyu.Lan@...rosoft.com>
Subject: RE: [RFC PATCH] clocksource: hyper-v: Enable the tsc_page for a TDX
 VM in TD mode

> From: Dave Hansen <dave.hansen@...el.com>
> Sent: Thursday, May 23, 2024 7:26 AM
> [...]
> On 5/22/24 19:24, Dexuan Cui wrote:
> ...
> > +static bool noinstr intel_cc_platform_td_l2(enum cc_attr attr)
> > +{
> > +	switch (attr) {
> > +	case CC_ATTR_GUEST_MEM_ENCRYPT:
> > +	case CC_ATTR_MEM_ENCRYPT:
> > +		return true;
> > +	default:
> > +		return false;
> > +	}
> > +}
> > +
> >  static bool noinstr intel_cc_platform_has(enum cc_attr attr)
> >  {
> > +	if (tdx_partitioned_td_l2)
> > +		return intel_cc_platform_td_l2(attr);
> > +
> >  	switch (attr) {
> >  	case CC_ATTR_GUEST_UNROLL_STRING_IO:
> >  	case CC_ATTR_HOTPLUG_DISABLED:
> 
> On its face, this _looks_ rather troubling.  It just hijacks all of the
> attributes.  It totally bifurcates the code.  Anything that gets added
> to intel_cc_platform_has() now needs to be considered for addition to
> intel_cc_platform_td_l2().

Maybe the bifurcation is necessary? TD mode is different from
Partitioned TD mode (L2), after all. Another reason for the bifurcation
is:  currently online/offline'ing is disallowed for a TD VM, but actually
Hyper-V is able to support CPU online/offline'ing for a TD VM in
Partitioned TD mode (L2) -- how can we allow online/offline'ing for such
a VM?

BTW, the bifurcation code is copied from amd_cc_platform_has(), where
an AMD SNP VM may run in the vTOM mode.

> > --- a/arch/x86/mm/mem_encrypt_amd.c
> > +++ b/arch/x86/mm/mem_encrypt_amd.c
> ...
> > @@ -529,7 +530,7 @@ void __init
> mem_encrypt_free_decrypted_mem(void)
> >  	 * CC_ATTR_MEM_ENCRYPT, aren't necessarily equivalent in a
> Hyper-V VM
> >  	 * using vTOM, where sme_me_mask is always zero.
> >  	 */
> > -	if (sme_me_mask) {
> > +	if (sme_me_mask || (cc_vendor == CC_VENDOR_INTEL
> && !tdx_partitioned_td_l2)) {
> >  		r = set_memory_encrypted(vaddr, npages);
> >  		if (r) {
> >  			pr_warn("failed to free unused decrypted
> pages\n");
> 
> If _ever_ there were a place for a new CC_ attribute, this would be it.
Not sure how to add a new CC attribute for the __bss_decrypted support.

For the cpu online/offline'ing support, I'm not sure how to add a new
CC attribute and not introduce the bifurcation.

> It's also a bit concerning that now we've got a (cc_vendor ==
> CC_VENDOR_INTEL) check in an amd.c file.
I agree my change here is ugly...
Currently the __bss_decrypted support is only used for SNP.
Not sure if we should get it to work for TDX as well.

> So all of that on top of Kirill's "why do we need this in the first
> place" questions leave me really scratching my head on this one.
Probably I'll just use local APIC timer in such a VM or delay enabling
Hyper-V TSC page to a later place where set_memory_decrypted()
works for me. However, I still would like to find out how to allow
CPU online/offline'ing for a TDX VM in Partitioned TD mode (L2).

Thanks,
Dexuan