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]

Message-ID: <299a83e8-cb13-4599-9737-adb3bb922169@intel.com>
Date: Thu, 23 May 2024 07:25:47 -0700
From: Dave Hansen <dave.hansen@...el.com>
To: Dexuan Cui <decui@...rosoft.com>, x86@...nel.org,
 linux-coco@...ts.linux.dev, bp@...en8.de, dave.hansen@...ux.intel.com,
 haiyangz@...rosoft.com, hpa@...or.com, kirill.shutemov@...ux.intel.com,
 kys@...rosoft.com, luto@...nel.org, mingo@...hat.com, peterz@...radead.org,
 sathyanarayanan.kuppuswamy@...ux.intel.com, tglx@...utronix.de,
 wei.liu@...nel.org, Jason@...c4.com, mhklinux@...look.com,
 thomas.lendacky@....com, tytso@....edu, ardb@...nel.org
Cc: linux-hyperv@...r.kernel.org, linux-kernel@...r.kernel.org,
 Tianyu.Lan@...rosoft.com
Subject: Re: [RFC PATCH] clocksource: hyper-v: Enable the tsc_page for a TDX
 VM in TD mode

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().

> --- 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.
It's also a bit concerning that now we've got a (cc_vendor ==
CC_VENDOR_INTEL) check in an amd.c file.

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.