Re: [PATCH v8 06/11] KVM: TDX: Allocate PAMT memory for TD and vCPU control structures

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Acquiring `cpus_read_lock()` after holding `kvm->lock` and `kvm->slots_lock` creates an ABBA deadlock risk.
--

--- Patch [6]: [PATCH v8 06/11] KVM: TDX: Allocate PAMT memory for TD and vCPU control structures ---
commit 777b107da8496af75d46b0c5bfc966aabe90a027
Author: Kiryl Shutsemau <kas@kernel.org>

KVM: TDX: Allocate PAMT memory for TD and vCPU control structures

This commit replaces raw page allocation and free functions with TDX control
page helpers for TD and vCPU control structures. This allows the operations
to properly handle Dynamic PAMT backing.

> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index d1af0a752e97e..528ebb3cb0389 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c

[ ... ]

> @@ -2472,7 +2472,7 @@ static int __tdx_td_init(struct kvm *kvm, struct td_params *td_params,
>  		goto free_tdr;
>  
>  	for (i = 0; i < kvm_tdx->td.tdcs_nr_pages; i++) {
> -		tdcs_pages[i] = alloc_page(GFP_KERNEL_ACCOUNT);
> +		tdcs_pages[i] = tdx_alloc_control_page();
>  		if (!tdcs_pages[i])
>  			goto free_tdcs;
>  	}

[Severity: High]
This is a pre-existing issue, but does taking cpus_read_lock() inside this
function create an ABBA deadlock risk?

Looking at __tdx_td_init(), just a few lines below this allocation loop, the
code calls cpus_read_lock():

	for (i = 0; i < kvm_tdx->td.tdcs_nr_pages; i++) {
		tdcs_pages[i] = tdx_alloc_control_page();
		if (!tdcs_pages[i])
			goto free_tdcs;
	}

	if (!zalloc_cpumask_var(&packages, GFP_KERNEL))
		goto free_tdcs;

	cpus_read_lock();

This function is reached via a KVM_TDX_INIT_VM ioctl, which triggers
tdx_vm_ioctl(). That ioctl handler acquires the VM state locks using
CLASS(tdx_vm_state_guard, guard)(kvm) before calling __tdx_td_init().

Is it safe to acquire cpus_read_lock() while kvm->lock and kvm->slots_lock
are already held, or does this violate the KVM locking hierarchy where
cpus_read_lock() should be the outermost lock to prevent deadlocks against
CPU hotplug operations?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260725002302.3337017-1-rick.p.edgecombe@intel.com?part=6