Received-SPF: pass (google.com: domain of linux-kernel+bounces-141985-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Fri, 12 Apr 2024 05:34:54 +0000
In-Reply-To: <3d69b44a-8542-4a11-b233-16487e980d54@intel.com> (dongsheng.x.zhang@intel.com)
Precedence: bulk
Mime-Version: 1.0
Message-ID: <diqzttk7jg8x.fsf@ctop-sg.c.googlers.com>
Subject: Re: [RFC PATCH v5 05/29] KVM: selftests: Add helper functions to
 create TDX VMs
From: Ackerley Tng <ackerleytng@google.com>
To: dongsheng.x.zhang@intel.com
Cc: sagis@google.com, linux-kselftest@vger.kernel.org, afranji@google.com, 
	erdemaktas@google.com, isaku.yamahata@intel.com, seanjc@google.com, 
	pbonzini@redhat.com, shuah@kernel.org, pgonda@google.com, haibo1.xu@intel.com, 
	chao.p.peng@linux.intel.com, vannapurve@google.com, runanwang@google.com, 
	vipinsh@google.com, jmattson@google.com, dmatlack@google.com, 
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org, linux-mm@kvack.org
Content-Type: text/plain; charset="UTF-8"


Thank you for your other comments!

>> <snip>

>> +static void load_td_per_vcpu_parameters(struct td_boot_parameters *params,
>> +					struct kvm_sregs *sregs,
>> +					struct kvm_vcpu *vcpu,
>> +					void *guest_code)
>> +{
>> +	/* Store vcpu_index to match what the TDX module would store internally */
>> +	static uint32_t vcpu_index;
>> +
>> +	struct td_per_vcpu_parameters *vcpu_params = &params->per_vcpu[vcpu_index];
>
> I think we can use vcpu->id in place of vcpu_index in this function, thus removing vcpu_index
>

td_per_vcpu_parameters is used in the selftest setup code (see
tools/testing/selftests/kvm/lib/x86_64/tdx/td_boot.S), (read via ESI) to
access the set of parameters belonging to the vcpu running the selftest
code, based on vcpu_index.

ESI is used because according to the TDX base spec, RSI contains the
vcpu index, which starts "from 0 and allocated sequentially on each
successful TDH.VP.INIT".

Hence, vcpu_index is set up to be static and is incremented once every
time load_td_per_vcpu_parameters() is called, which is once every time
td_vcpu_add() is called, which is aligned with the TDX base spec.

vcpu->id can be specified by the user when vm_vcpu_add() is called, but
that may not be the same as vcpu_index.

>> +
>> +	TEST_ASSERT(vcpu->initial_stack_addr != 0,
>> +		"initial stack address should not be 0");
>> +	TEST_ASSERT(vcpu->initial_stack_addr <= 0xffffffff,
>> +		"initial stack address must fit in 32 bits");
>> +	TEST_ASSERT((uint64_t)guest_code <= 0xffffffff,
>> +		"guest_code must fit in 32 bits");
>> +	TEST_ASSERT(sregs->cs.selector != 0, "cs.selector should not be 0");
>> +
>> +	vcpu_params->esp_gva = (uint32_t)(uint64_t)vcpu->initial_stack_addr;
>> +	vcpu_params->ljmp_target.eip_gva = (uint32_t)(uint64_t)guest_code;
>> +	vcpu_params->ljmp_target.code64_sel = sregs->cs.selector;
>> +
>> +	vcpu_index++;
>> +}

>> <snip>