Re: [RFC PATCH] clocksource: hyper-v: Enable the tsc_page for a TDX VM in TD mode

[Dave Hansen]

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.