Received-SPF: pass (google.com: domain of linux-kernel+bounces-169405-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Mon, 6 May 2024 15:21:28 +0800
From: Chao Gao <chao.gao@intel.com>
To: Reinette Chatre <reinette.chatre@intel.com>
CC: Isaku Yamahata <isaku.yamahata@intel.com>, <kvm@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <isaku.yamahata@gmail.com>, Paolo Bonzini
	<pbonzini@redhat.com>, <erdemaktas@google.com>, Sean Christopherson
	<seanjc@google.com>, Sagi Shahar <sagis@google.com>, Kai Huang
	<kai.huang@intel.com>, <chen.bo@intel.com>, <hang.yuan@intel.com>,
	<tina.zhang@intel.com>, <isaku.yamahata@linux.intel.com>
Subject: Re: [PATCH v19 101/130] KVM: TDX: handle ept violation/misconfig exit
Message-ID: <ZjiE+O9fct5zI4Sf@chao-email>
References: <cover.1708933498.git.isaku.yamahata@intel.com>
 <f05b978021522d70a259472337e0b53658d47636.1708933498.git.isaku.yamahata@intel.com>
 <Zgoz0sizgEZhnQ98@chao-email>
 <20240403184216.GJ2444378@ls.amr.corp.intel.com>
 <43cbaf90-7af3-4742-97b7-2ea587b16174@intel.com>
 <20240501155620.GA13783@ls.amr.corp.intel.com>
 <399cec29-ddf4-4dd5-a34b-ffec72cbfa26@intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <399cec29-ddf4-4dd5-a34b-ffec72cbfa26@intel.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?qyxF7beS2NJjnX0pVUGLoLuMiO8N+v8XUPzkutb5MsfAryGMfiaZouHsuwAu?=
 =?us-ascii?Q?Oxdhm0XJbksU1OJ4N9pKk/ep4oFmmNGnn8Gpmr+Ie8U2u/hJ4VqCT96gtUom?=
 =?us-ascii?Q?OiROTlmjKZdRTigQmNjm6DyXpmAbp/NKpVkIZEMpfFZIi1WN4gI0YS+XsWfu?=
 =?us-ascii?Q?+DVb67JKBo0TCax6oJ1ZXEBk2SPBPLJs/Xiy3RKLpUCpIFRMl1ZSr9WMQ/Mn?=
 =?us-ascii?Q?2sKc7V9KfPAarszYswHwwsNTCNeqP9936D0JH1D/l08E3jnJVZMltHHhY3JI?=
 =?us-ascii?Q?VIKCrSqZE934AMPfOnvQ0RozF2yWscT8zT3IrKF02Fc8W0duLSX//+cSi4Hn?=
 =?us-ascii?Q?+6hXUTKAaH4gfUOStF3Xy0cif82cJ7pQ/7j8GFCoaEiuUx6VvAQiFRb8v7tk?=
 =?us-ascii?Q?RuauzK6xvzrZaeR7x+0nUezLJApzKqX+mOEuRRLpmcvndHDLAr7ewV71C5UP?=
 =?us-ascii?Q?AHp6Kx+mZFI0acJ2fdiox9z4cPLXQHsF+Taai04Y8j/+NL5CSGvn4djl4a/3?=
 =?us-ascii?Q?t0jsbsUo1OkNia1BE6ODzTIxfnppsDKakHtYkeu128kd9SvOL/pbc0bBQ8Ne?=
 =?us-ascii?Q?J7NRCwBMyAvE1YxGR/mfGLnNhblwqYLldUXEaIRO9oNBKmgT/flUmonHWMd+?=
 =?us-ascii?Q?bz9R4+zRRJ/qobCs3whaCO2jft/fkPkkSjVfrYtitPE7Q+xX5NZtVEnWIcp7?=
 =?us-ascii?Q?wYqOHaeedDRkNRyC92l+H4+kuTGa/Hf63voy02tDu0NtTbIwDSMrnQR+W5D0?=
 =?us-ascii?Q?0LmGO15GEgbrN8a4feG+YchYTUs9zbvqyI7MMrVAcf+9Kl/BPsYRjcnNxm5m?=
 =?us-ascii?Q?OuwLSP3L8gSOpwOzcq816Cf9s4Blx4SlqA9+kC1lQG+UbV8RGCWAQVptKN5b?=
 =?us-ascii?Q?D/8CWsfoeFz3UKfS/DfDNF5u4RFZY4zngnDv7o0DqN6ZnPxkWT2P/jM6thrc?=
 =?us-ascii?Q?RsYc7opJJpn3XpqN3ITYGD2KfR5PQDRUMngzGa0NSUAN3xGipMG+ph1ZsofV?=
 =?us-ascii?Q?U6vwsrlnfJqgMKPD/h06uto028f090GWp0NgEM3eGIpeGTlBoHY3So0BhAsr?=
 =?us-ascii?Q?Kc/XpWWGoT0uKfFtPwvJ/n2nHhVdCY+7LP+DkqsRyt581cYhn+zt1NwNooZm?=
 =?us-ascii?Q?uXuVlOqsWVnwt7SP83s24ol+bQ1/NqviOl8e8xAWUhwquIHByiFVVIK2ri1n?=
 =?us-ascii?Q?5AsRzcXzflv5ywp4YUdzRcE7yzWf+50cjB09tZV60B6QoTcsAzTzLra+ICn1?=
 =?us-ascii?Q?9vjfnnKD3t5iEEP+Tr9HqJ4sdhq3V97w6noP27kSdAd3XuktHrKn7xl7wTkE?=
 =?us-ascii?Q?fWehiYPkHDnrcyBDYuO+MU4whPPAVJq5pez0ZkxUm9v19yxpqFIAcdqcl+wq?=
 =?us-ascii?Q?n2WrvJkjhxzXRzeOInHvbHyIfHMcT39eJ6p7rGtSXJgr41P6HWLbJMCqTOQA?=
 =?us-ascii?Q?orCsq9tKiunm+gxc8ceLn+Jg825EQuED5u1dTyMgNJwH28t8CL0/JcPZC2wB?=
 =?us-ascii?Q?hilGVmdzcajobT3MjUDi2HsgH0lmgux9CKhM+QKUe9JXrYQ8MSNqK9gUaVkh?=
 =?us-ascii?Q?P55h7w/UskcB9vFTOJUuWsfS826cj4EhK9EsJHp/?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 926e6271-6615-49c0-5f54-08dc6d9d2aa0
X-MS-Exchange-CrossTenant-AuthSource: CH3PR11MB8660.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 May 2024 07:21:44.6623
 (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: xm+DpJ4v13wO1hzpoSI5X/Lupbige1hzUTemZ0hsHiLix3LtXi/FroD1DcESbQY1l3u6G6ecN7aTx5bCgowbLQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA3PR11MB7582
X-OriginatorOrg: intel.com

On Wed, May 01, 2024 at 09:54:07AM -0700, Reinette Chatre wrote:
>diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
>index 499c6cd9633f..ba81e6f68c97 100644
>--- a/arch/x86/kvm/vmx/tdx.c
>+++ b/arch/x86/kvm/vmx/tdx.c
>@@ -1305,11 +1305,20 @@ static int tdx_handle_ept_violation(struct kvm_vcpu *vcpu)
> 	} else {
> 		exit_qual = tdexit_exit_qual(vcpu);
> 		if (exit_qual & EPT_VIOLATION_ACC_INSTR) {
>+			union tdx_exit_reason exit_reason = to_tdx(vcpu)->exit_reason;
>+
>+			/*
>+			 * Instruction fetch in TD from shared memory
>+			 * causes a #PF.
>+			 */

It is better to hoist this comment above the if-statement.

		/*
		 * Instruction fetch in TD from shared memory never causes EPT
		 * violation. Warn if such an EPT violation occurs as the CPU
		 * probably is buggy.
		 */
 		if (exit_qual & EPT_VIOLATION_ACC_INSTR) {
		...
		}


but, I am wondering why KVM needs to perform this check. I prefer to drop this
check if KVM doesn't rely on it to handle EPT violation correctly.

> 			pr_warn("kvm: TDX instr fetch to shared GPA = 0x%lx @ RIP = 0x%lx\n",
> 				tdexit_gpa(vcpu), kvm_rip_read(vcpu));

how about using vcpu_unimpl()? it is a wrapper for printing such a message.

>-			vcpu->run->exit_reason = KVM_EXIT_EXCEPTION;
>-			vcpu->run->ex.exception = PF_VECTOR;
>-			vcpu->run->ex.error_code = exit_qual;
>+			vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
>+			vcpu->run->internal.suberror =
>+				KVM_INTERNAL_ERROR_UNEXPECTED_EXIT_REASON;
>+			vcpu->run->internal.ndata = 2;
>+			vcpu->run->internal.data[0] = exit_reason.full;
>+			vcpu->run->internal.data[1] = exit_qual;
> 			return 0;
> 		}
> 	}
>
>Reinette
>
>[1] https://github.com/kvm-x86/linux/blob/next/arch/x86/kvm/vmx/vmx.c#L6587
>[2] https://github.com/kvm-x86/linux/blob/next/arch/x86/kvm/svm/svm.c#L3436