Received-SPF: pass (google.com: domain of linux-kernel+bounces-93227-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: Wed, 6 Mar 2024 09:36:27 +0800
From: Yan Zhao <yan.y.zhao@intel.com>
To: Sagi Shahar <sagis@google.com>, <linux-kselftest@vger.kernel.org>,
	Ackerley Tng <ackerleytng@google.com>, Ryan Afranji <afranji@google.com>,
	Erdem Aktas <erdemaktas@google.com>, Isaku Yamahata
	<isaku.yamahata@intel.com>, Sean Christopherson <seanjc@google.com>, "Paolo
 Bonzini" <pbonzini@redhat.com>, Shuah Khan <shuah@kernel.org>, Peter Gonda
	<pgonda@google.com>, Haibo Xu <haibo1.xu@intel.com>, Chao Peng
	<chao.p.peng@linux.intel.com>, Vishal Annapurve <vannapurve@google.com>,
	Roger Wang <runanwang@google.com>, Vipin Sharma <vipinsh@google.com>,
	<jmattson@google.com>, <dmatlack@google.com>, <linux-kernel@vger.kernel.org>,
	<kvm@vger.kernel.org>, <linux-mm@kvack.org>
Subject: Re: [RFC PATCH v5 23/29] KVM: selftests: TDX: Add shared memory test
Message-ID: <ZefIm+plPAzUww51@yzhao56-desk.sh.intel.com>
Reply-To: Yan Zhao <yan.y.zhao@intel.com>
References: <20231212204647.2170650-1-sagis@google.com>
 <20231212204647.2170650-24-sagis@google.com>
 <ZeHD44xpj75ixcUr@yzhao56-desk.sh.intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <ZeHD44xpj75ixcUr@yzhao56-desk.sh.intel.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?PmuXreJk1tSaan9fPPEiqJ+apeii9qigyNbJNTbcS85UexqN5530wRfrk6Oh?=
 =?us-ascii?Q?DXpB0Zdxt1L/mMUNQzyOcSWipzDoV+be1iQFRDcU80DxDjC6mKIcH8W6zsFq?=
 =?us-ascii?Q?5b6H2XvlgCPaPsTxQClsasJ4XhZinVgoNJqFN0hbq+vpU1c50WCe9s84lcpR?=
 =?us-ascii?Q?f7oE987ggKyTn49eNT+vfMlgqI6cORtSfA0Pb/nb3Zzq+c7FgMFZy1JjpjEz?=
 =?us-ascii?Q?ja4yDs0NyDCamDYXuE+4zVIw8gOaUzI3TnA+vLL+ku4IVfZ52u4XP9trNwha?=
 =?us-ascii?Q?EIpbNcg/QdsAF9tiRhp2rD04Y/zfKtzNh8VMT7d0pG6LamFalIF1YJ5+ttCe?=
 =?us-ascii?Q?SSJjqiIMo1Ou7ZTmgeB30ps2GZhlJmWh35iRlppJRxyj5DOlI9oubXdrhVKx?=
 =?us-ascii?Q?U7mIMqfvYRmzwo6PmVfg/BJf6v/KNrIWWlgEn9VPgGMhoZfQS0/DA6Na9fL9?=
 =?us-ascii?Q?r6IEA//yXU7aKAs+H2f5APbJT+d6uT5kSHCUtW+wrNXH8pt3qnuHYuPOg2dK?=
 =?us-ascii?Q?zbzg/lHY29Uk7zb+iTFGdx9NZZijH+a81vqWam7AA54/UsVNubZlpXyrUUSN?=
 =?us-ascii?Q?VkqroTLMiVe5wmqwz0bbP9YZ16tpU3yOm3mI5oWJsbe8yvRddrDhPdA9F33g?=
 =?us-ascii?Q?2h17k1SOIP73ZccmjQZ43ZmMfjANU2F61a4Jtm2C9LlzKDP/+pdga+vljMoY?=
 =?us-ascii?Q?sb9fqVOKqnimYfjh941dycF68bO0uyc4L+TzU40fTqo7O9VOS45wlVUd7W4A?=
 =?us-ascii?Q?hW2BoowZdFCjApv5tcvFzvruk/b4U6CSNL1PArAZwXbUlIlBHowT5pVjE00E?=
 =?us-ascii?Q?mn3kVPw27V0eXATGwjbKqfJW4ZKSmDTBCyLXFX2NSzdlf1Wz/hZdTbAKfoC6?=
 =?us-ascii?Q?5FKoqgwDrr3PwmUIVizKhB2xU++KrjLY8KhXDol8kJ/+ml7kOJ/iW9NMsohQ?=
 =?us-ascii?Q?e0X9SDdKZbep7Oti43VniSVLMa2u2KZ5fYa7T+A6Ux6wFnnQynEnMWXjkpLt?=
 =?us-ascii?Q?AAZPeXMFbdMaI15A+XEuyze1c0ZUEpdRfV9/skNeAR+gsnWJvhy3AAvUGflj?=
 =?us-ascii?Q?99cfhDETVFGmxNigMvdqL1U0wGkTW90RXNOLMMmfcu6h21786EhlArL6N7+u?=
 =?us-ascii?Q?dkUDzEevXgKIgmLPCmaw95uXz8MuqfU3C1ZZ3eRhNXu4TsvXSSYRHCqzfyZj?=
 =?us-ascii?Q?s7YXwvPlsbHpRunkKrlgORV0jZUaYuNNLmNeWIs4HnxmtSG0Eima/E0IwAdQ?=
 =?us-ascii?Q?ZQ0/sqLxh7ZvWNlMOnIX4lGg24EWbSjaFFG5Or10RRZgNwFxY5m0C7J83mgL?=
 =?us-ascii?Q?W83t5N0VueGhMMROl/LA4geNAp+raFCDyUURYyoOS4xYSe3Zr5GJTGl7HKvM?=
 =?us-ascii?Q?UhE5obiiMjqDiGPrx9RAiNSPLiumx7vF3Ka4yYmU18BAqUpEqDuh+ZWgrnr6?=
 =?us-ascii?Q?I3rz0kAbPaU8cgBoE+pKao81YRTn/tHcGnYWiqQL+QR0UTORwCMKYm3k3ewU?=
 =?us-ascii?Q?zGg60kSnj+DXP4Awp8sdsN/Y87eOmPVyX+lUGqk1Gk9G5SiP+QVJjFHbecfr?=
 =?us-ascii?Q?FaZScVMTlu91aFkxZJAcMIqPw4+ElH6ljhLJ8YDn?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 7eaac369-e526-4ec7-b32d-08dc3d820372
X-MS-Exchange-CrossTenant-AuthSource: DS7PR11MB5966.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Mar 2024 02:06:19.4086
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: JJFp4SOUeXEBdQIiuoK6yT3kC/zCpAm9YN2TVy3SKrIAI7vrX9r7/g7rPVjpUUPsoaulYhiIZK2XHStS01Phzw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR11MB8325
X-OriginatorOrg: intel.com

On Fri, Mar 01, 2024 at 08:02:43PM +0800, Yan Zhao wrote:
> > +void guest_shared_mem(void)
> > +{
> > +	uint32_t *test_mem_shared_gva =
> > +		(uint32_t *)TDX_SHARED_MEM_TEST_SHARED_GVA;
> > +
> > +	uint64_t placeholder;
> > +	uint64_t ret;
> > +
> > +	/* Map gpa as shared */
> > +	ret = tdg_vp_vmcall_map_gpa(test_mem_shared_gpa, PAGE_SIZE,
> > +				    &placeholder);
> > +	if (ret)
> > +		tdx_test_fatal_with_data(ret, __LINE__);
> > +
> > +	*test_mem_shared_gva = TDX_SHARED_MEM_TEST_GUEST_WRITE_VALUE;
> > +
> > +	/* Exit so host can read shared value */
> > +	ret = tdg_vp_vmcall_instruction_io(TDX_SHARED_MEM_TEST_INFO_PORT, 4,
> > +					   TDG_VP_VMCALL_INSTRUCTION_IO_WRITE,
> > +					   &placeholder);
> > +	if (ret)
> > +		tdx_test_fatal_with_data(ret, __LINE__);
> > +
> > +	/* Read value written by host and send it back out for verification */
> > +	ret = tdg_vp_vmcall_instruction_io(TDX_SHARED_MEM_TEST_INFO_PORT, 4,
> > +					   TDG_VP_VMCALL_INSTRUCTION_IO_WRITE,
> > +					   (uint64_t *)test_mem_shared_gva);
> > +	if (ret)
> > +		tdx_test_fatal_with_data(ret, __LINE__);
> > +}
> > +
> > +int verify_shared_mem(void)
> > +{
> > +	struct kvm_vm *vm;
> > +	struct kvm_vcpu *vcpu;
> > +
> > +	vm_vaddr_t test_mem_private_gva;
> > +	uint32_t *test_mem_hva;
> > +
> > +	vm = td_create();
> > +	td_initialize(vm, VM_MEM_SRC_ANONYMOUS, 0);
> > +	vcpu = td_vcpu_add(vm, 0, guest_shared_mem);
> > +
> > +	/*
> > +	 * Set up shared memory page for testing by first allocating as private
> > +	 * and then mapping the same GPA again as shared. This way, the TD does
> > +	 * not have to remap its page tables at runtime.
> > +	 */
> > +	test_mem_private_gva = vm_vaddr_alloc(vm, vm->page_size,
> > +					      TDX_SHARED_MEM_TEST_PRIVATE_GVA);
> > +	TEST_ASSERT_EQ(test_mem_private_gva, TDX_SHARED_MEM_TEST_PRIVATE_GVA);
> > +
> > +	test_mem_hva = addr_gva2hva(vm, test_mem_private_gva);
> > +	TEST_ASSERT(test_mem_hva != NULL,
> > +		    "Guest address not found in guest memory regions\n");
> > +
> > +	test_mem_private_gpa = addr_gva2gpa(vm, test_mem_private_gva);
> > +	virt_pg_map_shared(vm, TDX_SHARED_MEM_TEST_SHARED_GVA,
> > +			   test_mem_private_gpa);
> > +
> > +	test_mem_shared_gpa = test_mem_private_gpa | BIT_ULL(vm->pa_bits - 1);
> > +	sync_global_to_guest(vm, test_mem_private_gpa);
> > +	sync_global_to_guest(vm, test_mem_shared_gpa);
> > +
> > +	td_finalize(vm);
> > +
> > +	printf("Verifying shared memory accesses for TDX\n");
> > +
> > +	/* Begin guest execution; guest writes to shared memory. */
> > +	printf("\t ... Starting guest execution\n");
> > +
> > +	/* Handle map gpa as shared */
> > +	td_vcpu_run(vcpu);
> > +	TDX_TEST_CHECK_GUEST_FAILURE(vcpu);
> The first VMExit should be caused by tdvmcall map gpa, so it's
> impossible to be guest failure.
>
Ah, if KVM has bugs and returns to guest's map gpa tdvmcall as error without
exiting to user space, then it's possible to meet guest failure here.

> Move this line TDX_TEST_CHECK_GUEST_FAILURE(vcpu) to after the next td_vcpu_run()
> is better.
So, looks it's required to be checked after every vcpu run.
Without checking it (as in below), the selftest will not be able to print out
the guest reported fatal error.

> > +
> > +	td_vcpu_run(vcpu);
> > +	TDX_TEST_ASSERT_IO(vcpu, TDX_SHARED_MEM_TEST_INFO_PORT, 4,
> > +			   TDG_VP_VMCALL_INSTRUCTION_IO_WRITE);
> > +	TEST_ASSERT_EQ(*test_mem_hva, TDX_SHARED_MEM_TEST_GUEST_WRITE_VALUE);
> > +
> > +	*test_mem_hva = TDX_SHARED_MEM_TEST_HOST_WRITE_VALUE;
> > +	td_vcpu_run(vcpu);
> > +	TDX_TEST_ASSERT_IO(vcpu, TDX_SHARED_MEM_TEST_INFO_PORT, 4,
> > +			   TDG_VP_VMCALL_INSTRUCTION_IO_WRITE);
> > +	TEST_ASSERT_EQ(
> > +		*(uint32_t *)((void *)vcpu->run + vcpu->run->io.data_offset),
> > +		TDX_SHARED_MEM_TEST_HOST_WRITE_VALUE);
> > +
> > +	printf("\t ... PASSED\n");
> > +
> > +	kvm_vm_free(vm);
> > +
> > +	return 0;
> > +}
>  
>