Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
Date:   Wed, 1 Nov 2023 12:21:32 +0800
From:   Chao Gao <chao.gao@intel.com>
To:     Yang Weijiang <weijiang.yang@intel.com>
CC:     <seanjc@google.com>, <pbonzini@redhat.com>, <kvm@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <dave.hansen@intel.com>,
        <peterz@infradead.org>, <rick.p.edgecombe@intel.com>,
        <john.allen@amd.com>
Subject: Re: [PATCH v6 24/25] KVM: nVMX: Introduce new VMX_BASIC bit for
 event error_code delivery to L1
Message-ID: <ZUHSTEGpdWGjL93M@chao-email>
References: <20230914063325.85503-1-weijiang.yang@intel.com>
 <20230914063325.85503-25-weijiang.yang@intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230914063325.85503-25-weijiang.yang@intel.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?RsKzhYdTjfL+PgU4ExVanZmE7EaXVAxbYuR+yJ0YRgZdYYdsWLtBKW4TF+gZ?=
 =?us-ascii?Q?qLL/58UF9J0W+be/RtMsMRsW5jk7GCvzuM+6XUwi95YKWyb2Ph7qJILZ368O?=
 =?us-ascii?Q?lC3wZggkFTEkyvGG4TEDH89wFhbwfi+mCf0nhvALs7f3RAYVb2NDdGjo5OAo?=
 =?us-ascii?Q?WBxFiHVKHVF0uWXQCE5Z99pAJnVq1mUtrAohoXHY/reQ0UckfrUmNir06fi1?=
 =?us-ascii?Q?4qY/ZddftnNvfcjE4KUC9LeWxd+2AXIDvvuZ0Guf5hTaVKYAhRykExMy7Qvg?=
 =?us-ascii?Q?zFQmibNkx3a8c2Ln//LFaOJVvTKvJPbDO5JCr2Fepy/JI+Ukznbyk8mBPkE8?=
 =?us-ascii?Q?9hmIDMO7J714X5jI3NG+nJPYf7NdviAgdTa1ce4xqnk5rbc6PdpHin3xLEkB?=
 =?us-ascii?Q?fXyI2Bpa8vBt75NEPnMy7i4pnDPyEsG4rQ+OFikvX2UudJO61REq3TiJQDz7?=
 =?us-ascii?Q?RtB/fhoTkbVS5rcSslAbKm+/8wMp4tJ6yfgPpSmYWYCW+P8Nx6Tydax5LR5I?=
 =?us-ascii?Q?W4TCXt+BePjK2U43Lb9TVbvI3C7bN3B4Tv5FqJnEUB541TNacn7zY9lH8m81?=
 =?us-ascii?Q?uNU7scer1EX69jptLFikwZUhHii1YNUE99titz84GjNRFoGOtB3hJzHEew/S?=
 =?us-ascii?Q?+1LJghHyrhz56uc61O60sK3zODlYd1ChDmJ5BSuC8eg/D88/NmpNZjOdnNc5?=
 =?us-ascii?Q?DME//PX3lRf9vP6g4QPU02RWT1TCrqeCl7ORS42kWQQnvuB7nWEiFdYCEMwJ?=
 =?us-ascii?Q?kyckhD3cdd+VzsnTjB19DwgGaiaJnfId+dHkeHfH99kH3fNs2VMmGs5W77da?=
 =?us-ascii?Q?Eye1BvDETX0tFJiGD+ug/QN9OlHeJ6+ZglR1n5+D6kbiKdl6UTURA+DIdW9Y?=
 =?us-ascii?Q?P0h4AETIu2cQJ87aPDZaimRThWI8R6CeCq3bHmvQ1f0bjZ5CmnBOsCNXy8QM?=
 =?us-ascii?Q?w4dUBfd2SigHvOoFKon1CXLkm7G0BUSnPU7oXvZeD+gTJ+nurFwJndYJcp9A?=
 =?us-ascii?Q?o0MI5g6RsydQ+oKwksLWRwKcbK/ckHM5NVMGLCybFZx9b/QsJdWM0n/NWwB7?=
 =?us-ascii?Q?cio2CdqKs00AcGoRsKYD4daAKgoqX2GUz45a/tKfoa8oDkug7DGYbTsMygM2?=
 =?us-ascii?Q?Jb2NvcwGtYwcvlZDrwa/yTt+VGzo4si5djfm5EEs9d7tQjILv6uRkL4ATeWi?=
 =?us-ascii?Q?9actmHAw713xpL+V87xCwPK+1SJ0J/MchK2rC4MYQrGiYmZSdAIghHFhwgh3?=
 =?us-ascii?Q?4L6CHqiddCkFfTAO+67KwggXT5WpyIU9nAfbo1NC7Ppvdjyg3VCo5e2pisJ5?=
 =?us-ascii?Q?1P2gIgbXCm5c+aTUFjLl53p9Xed2doNs4Ul0gMLlyi47PlwBdH9dLtN3igsn?=
 =?us-ascii?Q?PXifZ5Z9aU5xH7eKfB6WpVKiyM7TASR2nkEEnLNTYZcgUOUmSdcXIV7Fjij8?=
 =?us-ascii?Q?YCSCotWiVrpDY4+chJTxAkbSCY+gZ0TjADSACNW/6z5Yeci2VfRE+cZTid9h?=
 =?us-ascii?Q?M0d7S3Em3e4+0r9wbcZyV7C/LOfAZ279T9RuI4r3RwHYtFdiShm9vb82RKgO?=
 =?us-ascii?Q?2tF6QN28btLZTgpwoG+uZhJCbfd6KD6j6CVgDagC?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 2a3f8eeb-3dbb-4211-b259-08dbda920c7c
X-MS-Exchange-CrossTenant-AuthSource: CH3PR11MB8660.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Nov 2023 04:21:41.3945
 (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: 4HXzk5TwGusN1IDJALCesZ9B8NbSTyeH1eWI0RZPFWFC4jfqko+rqxlIRiD/+2LDpfUxPLq1vowK4gifrJXRIg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CYYPR11MB8306
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-1.3 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,T_SCC_BODY_TEXT_LINE,T_SPF_HELO_TEMPERROR,
	T_SPF_TEMPERROR,URIBL_BLOCKED autolearn=unavailable autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on pete.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (pete.vger.email [0.0.0.0]); Tue, 31 Oct 2023 21:22:16 -0700 (PDT)

On Thu, Sep 14, 2023 at 02:33:24AM -0400, Yang Weijiang wrote:
>Per SDM description(Vol.3D, Appendix A.1):
>"If bit 56 is read as 1, software can use VM entry to deliver a hardware
>exception with or without an error code, regardless of vector"
>
>Modify has_error_code check before inject events to nested guest. Only
>enforce the check when guest is in real mode, the exception is not hard
>exception and the platform doesn't enumerate bit56 in VMX_BASIC, in all
>other case ignore the check to make the logic consistent with SDM.
>
>Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
>---
> arch/x86/kvm/vmx/nested.c | 22 ++++++++++++++--------
> arch/x86/kvm/vmx/nested.h |  5 +++++
> 2 files changed, 19 insertions(+), 8 deletions(-)
>
>diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
>index c5ec0ef51ff7..78a3be394d00 100644
>--- a/arch/x86/kvm/vmx/nested.c
>+++ b/arch/x86/kvm/vmx/nested.c
>@@ -1205,9 +1205,9 @@ static int vmx_restore_vmx_basic(struct vcpu_vmx *vmx, u64 data)
> {
> 	const u64 feature_and_reserved =
> 		/* feature (except bit 48; see below) */
>-		BIT_ULL(49) | BIT_ULL(54) | BIT_ULL(55) |
>+		BIT_ULL(49) | BIT_ULL(54) | BIT_ULL(55) | BIT_ULL(56) |
> 		/* reserved */
>-		BIT_ULL(31) | GENMASK_ULL(47, 45) | GENMASK_ULL(63, 56);
>+		BIT_ULL(31) | GENMASK_ULL(47, 45) | GENMASK_ULL(63, 57);
> 	u64 vmx_basic = vmcs_config.nested.basic;
> 
> 	if (!is_bitwise_subset(vmx_basic, data, feature_and_reserved))
>@@ -2846,12 +2846,16 @@ static int nested_check_vm_entry_controls(struct kvm_vcpu *vcpu,
> 		    CC(intr_type == INTR_TYPE_OTHER_EVENT && vector != 0))
> 			return -EINVAL;
> 
>-		/* VM-entry interruption-info field: deliver error code */
>-		should_have_error_code =
>-			intr_type == INTR_TYPE_HARD_EXCEPTION && prot_mode &&
>-			x86_exception_has_error_code(vector);
>-		if (CC(has_error_code != should_have_error_code))
>-			return -EINVAL;
>+		if (!prot_mode || intr_type != INTR_TYPE_HARD_EXCEPTION ||
>+		    !nested_cpu_has_no_hw_errcode_cc(vcpu)) {
>+			/* VM-entry interruption-info field: deliver error code */
>+			should_have_error_code =
>+				intr_type == INTR_TYPE_HARD_EXCEPTION &&
>+				prot_mode &&
>+				x86_exception_has_error_code(vector);
>+			if (CC(has_error_code != should_have_error_code))
>+				return -EINVAL;
>+		}

prot_mode and intr_type are used twice, making the code a little hard to read.

how about:
		/*
		 * Cannot deliver error code in real mode or if the
		 * interruption type is not hardware exception. For other
		 * cases, do the consistency check only if the vCPU doesn't
		 * enumerate VMX_BASIC_NO_HW_ERROR_CODE_CC.
		 */
		if (!prot_mode || intr_type != INTR_TYPE_HARD_EXCEPTION) {
			if (CC(has_error_code))
				return -EINVAL;
		} else if (!nested_cpu_has_no_hw_errcode_cc(vcpu)) {
			if (CC(has_error_code != x86_exception_has_error_code(vector)))
				return -EINVAL;
		}

and drop should_have_error_code.