linux-kernel - Re: [PATCH 10/13] x86/virt/tdx: Add SEAMCALL wrappers to remove a TD private page

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: <3a32ce4a-b108-4f06-a22d-14e9c2e135f7@intel.com>
Date: Wed, 8 Jan 2025 08:31:14 -0800
From: Dave Hansen <dave.hansen@...el.com>
To: Yan Zhao <yan.y.zhao@...el.com>
Cc: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>,
 "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
 "pbonzini@...hat.com" <pbonzini@...hat.com>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 "sean.j.christopherson@...el.com" <sean.j.christopherson@...el.com>,
 "Huang, Kai" <kai.huang@...el.com>,
 "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
 "Yamahata, Isaku" <isaku.yamahata@...el.com>
Subject: Re: [PATCH 10/13] x86/virt/tdx: Add SEAMCALL wrappers to remove a TD
 private page

On 1/7/25 16:41, Yan Zhao wrote:
> There is a proposed fix to change the type of KeyID to u16 as shown below (not
> yet split and sent out). Do you think this change to u16 makes sense?

I just think that the concept of a KeyID and the current implementation
on today's hardware are different things. Don't confuse an
implementation with the _concept_.

It can also make a lot of sense to pass around a 16-bit value in an
'int' in some cases. Think about NUMA nodes. You can't have negative
NUMA nodes in hardware, but we use 'int' in the kernel everywhere
because NUMA_NO_NODE gets passed around a lot.

Anyway, my point is that the underlying hardware types stop having
meaning at _some_ level of abstraction in the interfaces.

I'd personally probably just keep 'hkid' as an int everywhere until the
point where it gets shoved into the TDX module ABI.

Oh, and casts like this:

>  static inline void tdx_disassociate_vp(struct kvm_vcpu *vcpu)
> @@ -2354,7 +2354,8 @@ static int __tdx_td_init(struct kvm *kvm, struct td_params *td_params,
>  	ret = tdx_guest_keyid_alloc();
>  	if (ret < 0)
>  		return ret;
> -	kvm_tdx->hkid = ret;
> +	kvm_tdx->hkid = (u16)ret;
> +	kvm_tdx->hkid_assigned = true;

are a bit silly, don't you think?