Re: [PATCH v18 018/121] KVM: TDX: Add helper functions to allocate/free TDX private host key id

[Binbin Wu]

On 1/23/2024 7:52 AM, isaku.yamahata@xxxxxxxxx wrote:
> From: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
>
> Add helper functions to allocate/free TDX private host key id (HKID).
>
> The memory controller encrypts TDX memory with the assigned TDX HKIDs.  The
> global TDX HKID is to encrypt the TDX module, its memory, and some dynamic
> data (TDR).  The private TDX HKID is assigned to guest TD to encrypt guest
> memory and the related data.  When VMM releases an encrypted page for
> reuse, the page needs a cache flush with the used HKID.  VMM needs the
> global TDX HKID and the private TDX HKIDs to flush encrypted pages.
>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
> ---
> v18:
> - Moved the functions to kvm tdx from arch/x86/virt/vmx/tdx/
> - Drop exporting symbols as the host tdx does.
> ---
>   arch/x86/kvm/vmx/tdx.c | 29 +++++++++++++++++++++++++++++
>   1 file changed, 29 insertions(+)
>
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index 9d3f593eacb8..ee9d6a687d93 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -11,6 +11,35 @@
>   #undef pr_fmt
>   #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>   
> +/*
> + * Key id globally used by TDX module: TDX module maps TDR with this TDX global
> + * key id.  TDR includes key id assigned to the TD.  Then TDX module maps other
> + * TD-related pages with the assigned key id.  TDR requires this TDX global key
> + * id for cache flush unlike other TD-related pages.
> + */
> +/* TDX KeyID pool */
> +static DEFINE_IDA(tdx_guest_keyid_pool);
> +
> +static int __used tdx_guest_keyid_alloc(void)
> +{
> +	if (WARN_ON_ONCE(!tdx_guest_keyid_start || !tdx_nr_guest_keyids))
> +		return -EINVAL;
> +
> +	/* The first keyID is reserved for the global key. */
Seems no need to add the comment here any more.

> +	return ida_alloc_range(&tdx_guest_keyid_pool, tdx_guest_keyid_start,
> +			       tdx_guest_keyid_start + tdx_nr_guest_keyids - 1,
> +			       GFP_KERNEL);
> +}
> +
> +static void __used tdx_guest_keyid_free(int keyid)
> +{
> +	/* keyid = 0 is reserved. */
> +	if (WARN_ON_ONCE(keyid <= 0))


Why not use tdx_guest_keyid_start and its range directly for the check?


> +		return;
> +
> +	ida_free(&tdx_guest_keyid_pool, keyid);
> +}
> +
>   static int __init tdx_module_setup(void)
>   {
>   	int ret;