Received-SPF: pass (google.com: domain of linux-kernel+bounces-91478-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Tue, 5 Mar 2024 08:22:14 +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 15/29] KVM: selftests: TDX: Add TDX MSR read/write
 tests
Message-ID: <ZeZltsBoMbclr4yX@yzhao56-desk.sh.intel.com>
Reply-To: Yan Zhao <yan.y.zhao@intel.com>
References: <20231212204647.2170650-1-sagis@google.com>
 <20231212204647.2170650-16-sagis@google.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20231212204647.2170650-16-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?3KcUKitvZODug1855T9I0KePPw12buOX4bdFMYiV7hdIb2ONE+o7Vtco3Iku?=
 =?us-ascii?Q?IHcxxKRBZ1B/jAiWG4sVUVT5aFXELGvyCm+g3b59l9fo5Mk+q5TRJO4IlP0O?=
 =?us-ascii?Q?LsZ75gtYYpYbh8SJHzbK+BNsJ+MynZIrEM6plbdL1hqfyXsZONiM3GkwJWq8?=
 =?us-ascii?Q?lG6PbWBS377aNErLYcavY1UDaX3HdOW7o81EuTxmj2RFwI3nZS8/ok0AveiS?=
 =?us-ascii?Q?8tq/Ucbnztbi1FKAaJjgcyLoBHm9JuioYSBE6cC6U48QbK9nQBsFzrkLJaI8?=
 =?us-ascii?Q?iCblz+Ah4YRxpTuziLAVixloGLK0eBVXwM/YVyvLFIW7sgCUBcPdH8RTmMxa?=
 =?us-ascii?Q?CZDN462W+IueApG0Ec7ohI9aT9QtxPViK6VL5bQ0wNGOyI291FRm2y0Zze0i?=
 =?us-ascii?Q?BHR+y3AYoq1ThEhGW1Gj0vgLi//9tmx0jx3E59cxUOjbmb4D7NnqB5Q/Avt/?=
 =?us-ascii?Q?4ea0LSnuXeHxylb1d0RG9Q5BeUI6FX5XnilpFjMEzo7GaqPyU5V42Xb1gt2s?=
 =?us-ascii?Q?x/vx+bgIFOo29n3oAVi3FpZtm45t9bHYx4wc3NWZymTtJ82fsSDC/RhFgRTo?=
 =?us-ascii?Q?xsYagrkg+xSf4ou75zqVzKeidpbAyY5rt4BXJYFUlfgmmAmQhtVhrjPvUKw4?=
 =?us-ascii?Q?nGfrAqwskaG+6A1uWD7OMCjw8fT+KJ1Z5IMKgnTCP8vOEx7T4/SiPA2BSY5v?=
 =?us-ascii?Q?4ao5WWOLhQCPW0fa+dyG+fPkvclllk5dVVJmBMZ7IzRZ6xi1Zi1TYG2PRxIw?=
 =?us-ascii?Q?GyHIwq2bO9fDjDTPEZFwhuR8aKYY4AqCW93kQNiV1LYLNxUFQtDvb7XQ6JMq?=
 =?us-ascii?Q?6Hcj5kdZf2hEavDzs70F0LFoI972PZpQkIkU4lSG6OGuKAvlx36ttsClOr/u?=
 =?us-ascii?Q?TmIx55ImGmmiFeXunN16R3+euS1MueX75grSoIAFsGfRnFaovlftsFTt3aLL?=
 =?us-ascii?Q?6FIst5WSj4zYpANiprWYnuGCsrY20cPUbaL+rBix03qIiYmqcoW/LyxnGstl?=
 =?us-ascii?Q?YJgIA1SddtgznZpG5GWQpU4C2/d7bbV4yHpTF1k2EEEVs5eLmkl9FmaTziFz?=
 =?us-ascii?Q?2zTaS5Wm9EJEwqKKGhfKyqIULsXOzd3SB5Z3qDWz5zCwpOeO07vljFOwI0Bw?=
 =?us-ascii?Q?xtcxFgCQxq3fjhjvmmytZaN4KYrCpBhjElheWDh47/3a16de1C0EZbqSDeIy?=
 =?us-ascii?Q?/QD6H4+WxAgrRosbNV3wz6hHUcaEnK6RGk2OkSS8He8lCbTfl4rc3H5DL4M+?=
 =?us-ascii?Q?wFDUY2WlSwCB+0w1X5aPCfdhqsMm8CkmDNKb7UBKSOxPmYtl8aXJN1Ynx+Tn?=
 =?us-ascii?Q?ofIL7ANfXtgnBkVBfphllHIrNhaMk5QZ9u26rD9I/GiLw7kULI4wB0IqYJTo?=
 =?us-ascii?Q?d/wqmwLGs9ghfNe4dVTgPBPSJtyfv741YmDqTV63tRTH6Vlg6B6SAg/AwoBd?=
 =?us-ascii?Q?V4OvbQXRGnFcMEFkxTY4EHH+ckesx1oiw+6Oc/fI2YVUZQJzxPxuDsvX7hdk?=
 =?us-ascii?Q?JqVYG63pWZF3xDNixmH8po268BL12GXgrTV9s0KoxlmpGTYVuF9z+9boHhbF?=
 =?us-ascii?Q?uNQkmRUrFfwmcmX/vESPGYbfpMSbtM3HwVG5cPYE?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 91423ad8-8c1f-4be6-265a-08dc3cae7c6c
X-MS-Exchange-CrossTenant-AuthSource: DS7PR11MB5966.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Mar 2024 00:52:09.1127
 (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: yqJZ6PuuLOoaMrWIquVVJ6JN7h4KS4ZE/ZgmtNl+6o+OVGdyRI0Pf/WDqXSnpTktqMajYtZlWU21YrEsy8GAuw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN0PR11MB6111
X-OriginatorOrg: intel.com

> +void verify_guest_msr_writes(void)
> +{
> +	struct kvm_vcpu *vcpu;
> +	struct kvm_vm *vm;
> +
> +	uint64_t data;
> +	int ret;
> +
> +	vm = td_create();
> +	td_initialize(vm, VM_MEM_SRC_ANONYMOUS, 0);
> +
> +	/*
> +	 * Set explicit MSR filter map to control access to the MSR registers
> +	 * used in the test.
> +	 */
> +	printf("\t ... Setting test MSR filter\n");
> +	ret = kvm_check_cap(KVM_CAP_X86_USER_SPACE_MSR);
> +	TEST_ASSERT(ret, "KVM_CAP_X86_USER_SPACE_MSR is unavailable");
> +	vm_enable_cap(vm, KVM_CAP_X86_USER_SPACE_MSR, KVM_MSR_EXIT_REASON_FILTER);
> +
> +	ret = kvm_check_cap(KVM_CAP_X86_MSR_FILTER);
> +	TEST_ASSERT(ret, "KVM_CAP_X86_MSR_FILTER is unavailable");
> +
> +	ret = ioctl(vm->fd, KVM_X86_SET_MSR_FILTER, &tdx_msr_test_filter);
> +	TEST_ASSERT(ret == 0,
> +		    "KVM_X86_SET_MSR_FILTER failed, ret: %i errno: %i (%s)",
> +		    ret, errno, strerror(errno));
> +
> +	vcpu = td_vcpu_add(vm, 0, guest_msr_write);
> +	td_finalize(vm);
> +
> +	printf("Verifying guest msr writes:\n");
> +
> +	printf("\t ... Running guest\n");
> +	/* Only the write to MSR_IA32_MISC_ENABLE should trigger an exit */
> +	td_vcpu_run(vcpu);
> +	TDX_TEST_CHECK_GUEST_FAILURE(vcpu);
> +	data = tdx_test_read_64bit_report_from_guest(vcpu);
> +	TEST_ASSERT_EQ(data, TDG_VP_VMCALL_INVALID_OPERAND);
> +
> +	td_vcpu_run(vcpu);
> +	TDX_TEST_ASSERT_SUCCESS(vcpu);
> +
> +	printf("\t ... Verifying MSR values writen by guest\n");
> +
> +	TEST_ASSERT_EQ(vcpu_get_msr(vcpu, MSR_X2APIC_APIC_ICR), 4);
> +	TEST_ASSERT_EQ(vcpu_get_msr(vcpu, MSR_IA32_MISC_ENABLE), 0x1800);
It's not staightforward to assert MSR_IA32_MISC_ENABLE is 0x1800.
Rather than assume MSR_IA32_MISC_ENABLE is reset to
(MSR_IA32_MISC_ENABLE_PEBS_UNAVAIL | MSR_IA32_MISC_ENABLE_BTS_UNAVAIL)
which is 0x1800, why not call vcpu_get_msr() before guest write and compare
the saved value here?

> +	TEST_ASSERT_EQ(vcpu_get_msr(vcpu, MSR_IA32_POWER_CTL), 6);
> +
> +	kvm_vm_free(vm);
> +	printf("\t ... PASSED\n");
> +}
> +
> +
>  int main(int argc, char **argv)
>  {
>  	setbuf(stdout, NULL);
> @@ -531,6 +738,8 @@ int main(int argc, char **argv)
>  	run_in_new_process(&verify_get_td_vmcall_info);
>  	run_in_new_process(&verify_guest_writes);
>  	run_in_new_process(&verify_guest_reads);
> +	run_in_new_process(&verify_guest_msr_writes);
> +	run_in_new_process(&verify_guest_msr_reads);
>  
>  	return 0;
>  }
> -- 
> 2.43.0.472.g3155946c3a-goog
> 
>