Received-SPF: pass (google.com: domain of linux-kernel+bounces-92003-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Tue, 5 Mar 2024 16:58:32 +0800
From: Yan Zhao <yan.y.zhao@intel.com>
To: Sagi Shahar <sagis@google.com>
CC: <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 18/29] KVM: selftests: TDX: Add TDX MMIO writes
 test
Message-ID: <ZebeuMoz8izs/SoG@yzhao56-desk.sh.intel.com>
Reply-To: Yan Zhao <yan.y.zhao@intel.com>
References: <20231212204647.2170650-1-sagis@google.com>
 <20231212204647.2170650-19-sagis@google.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20231212204647.2170650-19-sagis@google.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?55tDXJXnZkwUZ6s26DzYHKD1Ck7LQnTQohGxVBQAwelp2h9ExoeAl0dfnFHt?=
 =?us-ascii?Q?OcF7NLM5fg1tXKyFxQe03l9swNxUqdCNd/B+EhzuwNnRfAKEHGK/LsO/awSd?=
 =?us-ascii?Q?E/1ST74revbF/CXyJUtFl2HGO6NUboc+zOgkUYlW/S9Zb55MOZhFEnn19UqX?=
 =?us-ascii?Q?8zcAM7US2lck40d003yesSWjqZBftFGUDcGovaqo7C/PeK1ZpWhI63Qi1bee?=
 =?us-ascii?Q?xuIHFruG7NMSC95yLuTg+xfdVK46jiP4vQIuxnweODGfFmeEkvcSncNJKgVz?=
 =?us-ascii?Q?CtIQcVS20dYVAwiKAtj385RQSQ3mNtNg0wbn+lROXtCL5K8VzeB1I40FDe4s?=
 =?us-ascii?Q?A25xH8u4/ccsn6BjaPDsTIgFLXvhp2aKhEVAmzno020pw15sy5y3mDIMvrZr?=
 =?us-ascii?Q?HG7weA2wdkoJHLUpo3ts9VmrJ4HFeJupzg470nMoDK7jsnJHfDAf6Ymt3eII?=
 =?us-ascii?Q?98CVk+BhjWYghUAzH/iSMztzUS4XO2s96wI7d045HJ0wIyx4J82z6XaCuIXy?=
 =?us-ascii?Q?WzjPvBZkdWMBlUJqhPjg+EtzSacQPUj4X5J0c+xIpZrsxwxm1Mv1xAK4zsi8?=
 =?us-ascii?Q?piqzeJQ2qigy55G0GmK4vsrsSI1Qyj4zWRiARVv/k/5dp0SDAZVkkpGwzkWh?=
 =?us-ascii?Q?1jq/aSI9KfSNcTw+pvMfwaSyryd1TT+dzi3Zx/lfJ/zfvtPRn6dpxLFVoaqH?=
 =?us-ascii?Q?D3M7NMlK3gP8zKHw2QUZdDshbiuEFPBXo0t4lZyvjbr1wjkh+t/HsnZtxzgt?=
 =?us-ascii?Q?2uVB6G+jEwpAKQ6rXmP34dQiBvVSO3LkYGsCC5Ps8O9poPpv49uVWylrMvPI?=
 =?us-ascii?Q?ZUO8aarHwg+MhH5LbjDeg9cUtPNMyNP6jj3T+hh2kO0yzVvBMYM4PFwoRXMO?=
 =?us-ascii?Q?ZnhcT8ticVXG9dfgekgPLDKIHCyHKzQ7Bq5fL8SW0Szva3Jr4V6K5JDneCYw?=
 =?us-ascii?Q?yUjnDd9UHe8Em97W75bUukU473Lu9sGAOGaYRm93qiU6HSzadXpHPnST7psu?=
 =?us-ascii?Q?QZ89qFBEVhnzSebP2E/Rgm/HujNv9+po2P/Yv7+vLce56H9G9nP8A9+l608C?=
 =?us-ascii?Q?XFMKXO+4gpgGm14XZZ7cZ9tef3v1Tq8kM6KYoS55M1XSKWAJ8HFzFgajVps9?=
 =?us-ascii?Q?6H4TTCL95csuGZrsq/vZO1fDecqbDBejVrMlRt1RuIqCWq5G1rkSW9aowGDZ?=
 =?us-ascii?Q?tojpkUueAjNdXa8MWF3sQEGnY42ToWP/4DSOvdaL/uHI8kcSpsqsNyMVd9+q?=
 =?us-ascii?Q?720e25pzxV2RiKE7I82C855iRYTnQUroiPJhXkb1tBP/sbVisom3IWwIurL2?=
 =?us-ascii?Q?NNSUDtZd4mg4oMwTkd6m74qKAAIO0eZTKQldURmGjhDcJa5JnF1ziaaOOkQw?=
 =?us-ascii?Q?FfOc5KP3yPx2HbtitYdQGGfqGFsHRcsfVqE/7Ze4+vVX8eghVV73LhomfcdJ?=
 =?us-ascii?Q?01u7xHRPsiXtaNeJj4kguoBbbIAW7xmViNqNr1W/tFA7wbZBhUPPIIkR3h1z?=
 =?us-ascii?Q?dC9elubbenTTHsJVJZkufoZxjsuD+nFOjQKM17ximcmIv7Nc13ZEhzuduV9K?=
 =?us-ascii?Q?BPQq/2nn3TXsEIvULmLAfSvMeCcVG/+m3v0lCrby?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 01d80991-40e5-4bef-0472-08dc3cf69bd5
X-MS-Exchange-CrossTenant-AuthSource: DS7PR11MB5966.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Mar 2024 09:28:25.4803
 (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: MItJKylEV6cUXucYiUK25iSKA8fuQsjvubmK/6bon4ze9NfqISDQH2CkO6ntJqmZWiadoDBrX6nNz6L4FWQDEQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB5795
X-OriginatorOrg: intel.com

On Tue, Dec 12, 2023 at 12:46:33PM -0800, Sagi Shahar wrote:
> The test verifies MMIO writes of various sizes from the guest to the host.
> 
> Signed-off-by: Sagi Shahar <sagis@google.com>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> Signed-off-by: Ryan Afranji <afranji@google.com>
> ---
>  .../selftests/kvm/include/x86_64/tdx/tdx.h    |  2 +
>  .../selftests/kvm/lib/x86_64/tdx/tdx.c        | 14 +++
>  .../selftests/kvm/x86_64/tdx_vm_tests.c       | 85 +++++++++++++++++++
>  3 files changed, 101 insertions(+)
> 
> diff --git a/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h b/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
> index 13ce60df5684..502b670ea699 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
> @@ -25,5 +25,7 @@ uint64_t tdg_vp_vmcall_instruction_wrmsr(uint64_t index, uint64_t value);
>  uint64_t tdg_vp_vmcall_instruction_hlt(uint64_t interrupt_blocked_flag);
>  uint64_t tdg_vp_vmcall_ve_request_mmio_read(uint64_t address, uint64_t size,
>  					uint64_t *data_out);
> +uint64_t tdg_vp_vmcall_ve_request_mmio_write(uint64_t address, uint64_t size,
> +					uint64_t data_in);
>  
>  #endif // SELFTEST_TDX_TDX_H
> diff --git a/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c b/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c
> index b19f07ebc0e7..f4afa09f7e3d 100644
> --- a/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c
> +++ b/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c
> @@ -143,3 +143,17 @@ uint64_t tdg_vp_vmcall_ve_request_mmio_read(uint64_t address, uint64_t size,
>  
>  	return ret;
>  }
> +
> +uint64_t tdg_vp_vmcall_ve_request_mmio_write(uint64_t address, uint64_t size,
> +					uint64_t data_in)
> +{
> +	struct tdx_hypercall_args args = {
> +		.r11 = TDG_VP_VMCALL_VE_REQUEST_MMIO,
> +		.r12 = size,
> +		.r13 = TDG_VP_VMCALL_VE_REQUEST_MMIO_WRITE,
> +		.r14 = address,
> +		.r15 = data_in,
> +	};
> +
> +	return __tdx_hypercall(&args, 0);
> +}
> diff --git a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
> index 48902b69d13e..5e28ba828a92 100644
> --- a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
> +++ b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
> @@ -885,6 +885,90 @@ void verify_mmio_reads(void)
>  	printf("\t ... PASSED\n");
>  }
>  
> +void guest_mmio_writes(void)
> +{
> +	uint64_t ret;
> +
> +	ret = tdg_vp_vmcall_ve_request_mmio_write(TDX_MMIO_TEST_ADDR, 1, 0x12);
> +	if (ret)
> +		tdx_test_fatal(ret);
> +
> +	ret = tdg_vp_vmcall_ve_request_mmio_write(TDX_MMIO_TEST_ADDR, 2, 0x1234);
> +	if (ret)
> +		tdx_test_fatal(ret);
> +
> +	ret = tdg_vp_vmcall_ve_request_mmio_write(TDX_MMIO_TEST_ADDR, 4, 0x12345678);
> +	if (ret)
> +		tdx_test_fatal(ret);
> +
> +	ret = tdg_vp_vmcall_ve_request_mmio_write(TDX_MMIO_TEST_ADDR, 8, 0x1234567890ABCDEF);
> +	if (ret)
> +		tdx_test_fatal(ret);
> +
> +	// Write across page boundary.
> +	ret = tdg_vp_vmcall_ve_request_mmio_write(PAGE_SIZE - 1, 8, 0);
> +	if (ret)
> +		tdx_test_fatal(ret);
> +
> +	tdx_test_success();
> +}
> +
> +/*
> + * Varifies guest MMIO writes.
> + */
> +void verify_mmio_writes(void)
> +{
> +	struct kvm_vm *vm;
> +	struct kvm_vcpu *vcpu;
> +
> +	uint8_t byte_1;
> +	uint16_t byte_2;
> +	uint32_t byte_4;
> +	uint64_t byte_8;
> +
> +	vm = td_create();
> +	td_initialize(vm, VM_MEM_SRC_ANONYMOUS, 0);
> +	vcpu = td_vcpu_add(vm, 0, guest_mmio_writes);
> +	td_finalize(vm);
> +
> +	printf("Verifying TD MMIO writes:\n");
> +
> +	td_vcpu_run(vcpu);
> +	TDX_TEST_CHECK_GUEST_FAILURE(vcpu);
> +	TDX_TEST_ASSERT_MMIO(vcpu, TDX_MMIO_TEST_ADDR, 1, TDG_VP_VMCALL_VE_REQUEST_MMIO_WRITE);
> +	byte_1 = *(uint8_t *)(vcpu->run->mmio.data);
> +
> +	td_vcpu_run(vcpu);
> +	TDX_TEST_CHECK_GUEST_FAILURE(vcpu);
> +	TDX_TEST_ASSERT_MMIO(vcpu, TDX_MMIO_TEST_ADDR, 2, TDG_VP_VMCALL_VE_REQUEST_MMIO_WRITE);
> +	byte_2 = *(uint16_t *)(vcpu->run->mmio.data);
> +
> +	td_vcpu_run(vcpu);
> +	TDX_TEST_CHECK_GUEST_FAILURE(vcpu);
> +	TDX_TEST_ASSERT_MMIO(vcpu, TDX_MMIO_TEST_ADDR, 4, TDG_VP_VMCALL_VE_REQUEST_MMIO_WRITE);
> +	byte_4 = *(uint32_t *)(vcpu->run->mmio.data);
> +
> +	td_vcpu_run(vcpu);
> +	TDX_TEST_CHECK_GUEST_FAILURE(vcpu);
> +	TDX_TEST_ASSERT_MMIO(vcpu, TDX_MMIO_TEST_ADDR, 8, TDG_VP_VMCALL_VE_REQUEST_MMIO_WRITE);
> +	byte_8 = *(uint64_t *)(vcpu->run->mmio.data);
> +
> +	TEST_ASSERT_EQ(byte_1, 0x12);
> +	TEST_ASSERT_EQ(byte_2, 0x1234);
> +	TEST_ASSERT_EQ(byte_4, 0x12345678);
> +	TEST_ASSERT_EQ(byte_8, 0x1234567890ABCDEF);
> +
> +	td_vcpu_run(vcpu);
> +	TEST_ASSERT_EQ(vcpu->run->exit_reason, KVM_EXIT_SYSTEM_EVENT);
> +	TEST_ASSERT_EQ(vcpu->run->system_event.data[1], TDG_VP_VMCALL_INVALID_OPERAND);
Is it possible that this event is caused by an failure of the last 8 byte write?
i.e. though MMIO exit to host with correct value 0x1234567890ABCDEF, but guest
sees ret as TDG_VP_VMCALL_INVALID_OPERAND.

And if, coincidently, guest gets a ret=0 in the next across page boundary write,
the selftest will show "PASSED", which is not right.


> +	td_vcpu_run(vcpu);
> +	TDX_TEST_ASSERT_SUCCESS(vcpu);
> +
> +	kvm_vm_free(vm);
> +	printf("\t ... PASSED\n");
> +}
> +
>  int main(int argc, char **argv)
>  {
>  	setbuf(stdout, NULL);
> @@ -905,6 +989,7 @@ int main(int argc, char **argv)
>  	run_in_new_process(&verify_guest_msr_reads);
>  	run_in_new_process(&verify_guest_hlt);
>  	run_in_new_process(&verify_mmio_reads);
> +	run_in_new_process(&verify_mmio_writes);
>  
>  	return 0;
>  }
> -- 
> 2.43.0.472.g3155946c3a-goog
> 
>