Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Jon Kohler <jon@nutanix.com>
To:     Maxim Levitsky <mlevitsk@redhat.com>
CC:     Sean Christopherson <seanjc@google.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        "x86@kernel.org" <x86@kernel.org>,
        "H. Peter Anvin" <hpa@zytor.com>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] KVM: X86: set EXITING_GUEST_MODE as soon as vCPU exits
Thread-Topic: [PATCH] KVM: X86: set EXITING_GUEST_MODE as soon as vCPU exits
Thread-Index: AQHZBB+UbwHM6Ykv6ESmts8I/MP46K5WToGAgAACgYA=
Date:   Tue, 29 Nov 2022 19:56:12 +0000
Message-ID: <E3693FA8-7818-495B-9F40-42AEEE756142@nutanix.com>
References: <20221129182226.82087-1-jon@nutanix.com>
 <5162c4cbaaae8de01c77093ac005c2f5abc1d040.camel@redhat.com>
In-Reply-To: <5162c4cbaaae8de01c77093ac005c2f5abc1d040.camel@redhat.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?7GePqog7URwWTwNHu9e7Eo+mDZ4uSc/PyhE+ObCQ93oZrEp/rbEnJEm10+ls?=
 =?us-ascii?Q?NWWP6f3bBMEbH2sl+1hgXTxTtDUqosf4HR03Ej37MDPFWJvS3REJpp3MCrUU?=
 =?us-ascii?Q?yUsZiaKa5df0qBHX8+06d40zz0MaykUE4LquWxTAGiks1e+RCK2EVKXUzBUV?=
 =?us-ascii?Q?d0/xVuqFZQUErbrufLjDH73pcUeu1CjN1wpP6myfilXds/ZZ5jHl/h0cCbo9?=
 =?us-ascii?Q?yYxHLyJpmBMzGmiA3LbMCSL3ItW+CaAxDe2u3TAJ12spvQ1ycw9fOoKQd82L?=
 =?us-ascii?Q?EVziJv8wotb6JnswwBT/d2dVWDt2Oo6XVtcpzbuROcB4do7SbmQus7O7IDrj?=
 =?us-ascii?Q?Lon0r3/Ikpp+CW+lOb0P+9V3KzUDeE2FB4tKRah8/0dfELDMIWcNlvt+Adgw?=
 =?us-ascii?Q?eyTKQoywmV1CyHkE/0MplXqosdq4S71cceUicza3vL6hU/6PbL7bCAbtXq8/?=
 =?us-ascii?Q?IyBa+KipJLqXxCAZbyddDG9UfT+NUF4lzuCQP8rOzhJRmU9DJ2FJCCpvhD67?=
 =?us-ascii?Q?k4mJyiwJwdgwyn1qHq/Dv1/h9NZr/vChxHYW7pzZVsV+YWjUHxgVCl/DehF8?=
 =?us-ascii?Q?01eEcxlagiKVqcol3h/nkws+xL3yK0amctvbaQiTgZUM89XSYeJbW8FHeoPF?=
 =?us-ascii?Q?JcbA1MfEyfk55nMCJFCvYn8M/CfSfUl+MfDpa1zTuEb/XIDgk5MBPTiyGb+e?=
 =?us-ascii?Q?Xdl1hqTPHTHWyjN75VucTG+GCv7yxK+t3e5onaPQ76W5wilTA+7KluKMrFuA?=
 =?us-ascii?Q?q6IC5XLWltLCGak8ASd/6NFJU6VrfNAvs7ZZjxswY7aDgvF6rbmUQAOJIwst?=
 =?us-ascii?Q?dIaISRiBnsdzeqd9AlZWfMRK16rJi+ChQNfC0ZVMyF+8SQxKGJYFakUPSGPQ?=
 =?us-ascii?Q?UfHe0KWgav/8TmqJzqnI9dfUO2QKNwJGf7S7FPQfdVfT0YrJ6d6XduJPlm9s?=
 =?us-ascii?Q?AAoI+CRyGmXJ5lhRAV2lDWnSy7hJe6+o82Q9ea8jBtqyBH6GVbJ8FqtBrUyt?=
 =?us-ascii?Q?WBF8b9XW9LHYgPlZqentz9yiGDeaEdeEm9ZVtX6D/17bHFrJoTgT0RzdJFnb?=
 =?us-ascii?Q?vAbXwhmooXWUa+7AjnzccWSc1XywZIOwvsFqGv57Mny4yAYqjmGcUEnpyki8?=
 =?us-ascii?Q?I0rVN9xcYOXqe9qtyt4sY0xO23Nta12TkBOHIvXp7sN2/qiIalMg4SJtGTpZ?=
 =?us-ascii?Q?C/u9n6qrpwEmpnbsrid0t/z/vLLJZQpmUjEC6hD1sUzoFd9xqtb8K491PxZ8?=
 =?us-ascii?Q?bYE41Do6yDFxmWTQ8M07Tasg+aiAdTNmUUoZql1ntI94RYEaq7uFSXDRbHkG?=
 =?us-ascii?Q?ghS4GBzn2bfAh//HKlhgBfmhmWbibgMpwBk3isCyJFeSAnWppTpT+OyJjwDj?=
 =?us-ascii?Q?Aw4DsvhMI3O5roOMxhUgctQUYA4rDSxoW4BODdgpZ9k8UCHdj21vP/cuMx3V?=
 =?us-ascii?Q?e7vR/qyFEgpE66WP1VQs4NMWcM56Votkj+tJ2yW6hc4gRCVk2UtyJKEyeAdf?=
 =?us-ascii?Q?7v7Ep13VSfjK0jTrCVucL6LFTiTVILGk3GCoxYxIedrs0QC60WyzRTkWnvMY?=
 =?us-ascii?Q?fSZNv3/x+toxYIc7ZeyaIwYXk9EX6PkopfCdN45btXxbyZK7fRI+braKbxFJ?=
 =?us-ascii?Q?E0LKZw4Ikzo/RJtM/jbm9IezEGhX2y1VUS3nJnU6AFzunU80BO3znorWoTL9?=
 =?us-ascii?Q?8Gx+mQ=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3EB713BFA99B5B45981829130A77A0D9@namprd02.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: nutanix.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BL0PR02MB4579.namprd02.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1dd9d0b3-9799-4023-9dc4-08dad243c434
X-MS-Exchange-CrossTenant-originalarrivaltime: 29 Nov 2022 19:56:12.2458
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: bb047546-786f-4de1-bd75-24e5b6f79043
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: zsUpse0WTDbgdrJoGnZ94EwfVXbAQHtXD7mEhpR+Nih8R5KY4h13CRqZfEgX0rUC2gGWXABcOwjee8Y803vRZGoKerPTlVRCaEBhOxsVTIQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR02MB7610
X-Proofpoint-GUID: kNbxlxyrPKeDEmHsJCv7_b9IL9PLP81H
X-Proofpoint-ORIG-GUID: kNbxlxyrPKeDEmHsJCv7_b9IL9PLP81H
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.219,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1
 definitions=2022-11-29_11,2022-11-29_01,2022-06-22_01
X-Proofpoint-Spam-Reason: safe
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,
        SPF_NONE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> On Nov 29, 2022, at 2:47 PM, Maxim Levitsky <mlevitsk@redhat.com> wrote:
>=20
> On Tue, 2022-11-29 at 13:22 -0500, Jon Kohler wrote:
>> Set vcpu->mode to EXITING_GUEST_MODE as soon vCPU exits to reflect
>> that we are indeed exiting guest mode, but not quite out of guest
>> mode yet. Note: This is done lazily without an explicit memory
>> barrier so that we do not regress the cost in the critical path
>> of going from the exit to the exit handler.
>>=20
>> Flip back to IN_GUEST_MODE for exits that use
>> EXIT_FASTPATH_REENTER_GUEST, such that we are IN_GUEST_MODE upon
>> reentry.
>>=20
>> Changing vcpu->mode away from IN_GUEST_MODE as early as possible
>> gives IPI senders as much runway as possible to avoid ringing
>> doorbell or sending posted interrupt IPI in AMD and Intel,
>> respectively. Since this is done without an explicit memory
>> barrier, the worst case is that the IPI sender sees IN_GUEST_MODE
>> still and sends a spurious event, which is the behavior prior
>> to this patch.
>=20
> Beware that we had a king sized bug in regard to AVIC inhibition races
> vs guest entries, this this should be carefully checked for this.

Thanks, Maxim - any pointers on what we should be looking for here?

>=20
> Also, do you have any perf numbers to see if that actually improves perfo=
rmance?
> (I am just curious, I do think that this can improve performance).
>=20

Yes indeed! Sorry I should have put that right in the commit msg as a note,
but using the kvm-unit-tests vmexit_ipi with -smp 20 on an Intel 8168 its
roughly ~3% better (~3325-ish vs ~3400-ish), though the test is a bit noisy
even with taskset to a single socket.

To help validate that we were even getting *any* benefit, in a local build
I added a log statement (rough code below) to IPI delivery path, and did se=
e=20
many, many IPIs getting suppressed that would have otherwise fired.

kvm_vcpu_trigger_posted_interrupt() {
...
    if (vcpu->mode =3D=3D EXITING_GUEST_MODE) {
        pr_warn_ratelimited("exiting suppression worked");
    }
...
}

> Best regards,
> 	Maxim Levitsky
>=20
>=20
>>=20
>> Signed-off-by: Jon Kohler <jon@nutanix.com>
>> ---
>> arch/x86/kvm/svm/svm.c |  7 +++++++
>> arch/x86/kvm/vmx/vmx.c | 23 +++++++++++++++++++++++
>> arch/x86/kvm/x86.c     |  8 ++++++++
>> 3 files changed, 38 insertions(+)
>>=20
>> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
>> index ce362e88a567..5f0c118a3ffd 100644
>> --- a/arch/x86/kvm/svm/svm.c
>> +++ b/arch/x86/kvm/svm/svm.c
>> @@ -3907,6 +3907,13 @@ static noinstr void svm_vcpu_enter_exit(struct kv=
m_vcpu *vcpu, bool spec_ctrl_in
>> 	else
>> 		__svm_vcpu_run(svm, spec_ctrl_intercepted);
>>=20
>> +	/* Optimize IPI reduction by setting mode immediately after vmexit
>> +	 * without a memmory barrier as this as not paired anywhere. vcpu->mod=
e
>> +	 * is will be set to OUTSIDE_GUEST_MODE in x86 common code with a memo=
ry
>> +	 * barrier, after the host is done fully restoring various host states=
.
>> +	 */
>> +	vcpu->mode =3D EXITING_GUEST_MODE;
>> +
>> 	guest_state_exit_irqoff();
>> }
>>=20
>> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>> index 63247c57c72c..243dcb87c727 100644
>> --- a/arch/x86/kvm/vmx/vmx.c
>> +++ b/arch/x86/kvm/vmx/vmx.c
>> @@ -5878,6 +5878,17 @@ static fastpath_t handle_fastpath_preemption_time=
r(struct kvm_vcpu *vcpu)
>>=20
>> 	if (!vmx->req_immediate_exit &&
>> 	    !unlikely(vmx->loaded_vmcs->hv_timer_soft_disabled)) {
>> +		/* Reset IN_GUEST_MODE since we're going to reenter
>> +		 * guest as part of this fast path. This is done as
>> +		 * an optimization without a memory barrier since
>> +		 * EXITING_GUEST_MODE is also set without a memory
>> +		 * barrier. This also needs to be reset prior to
>> +		 * calling apic_timer_expired() so that
>> +		 * kvm_use_posted_timer_interrupt() returns the proper
>> +		 * value.
>> +		 */
>> +		if (vcpu->mode =3D=3D EXITING_GUEST_MODE)
>> +			vcpu->mode =3D IN_GUEST_MODE;
>> 		kvm_lapic_expired_hv_timer(vcpu);
>> 		return EXIT_FASTPATH_REENTER_GUEST;
>> 	}
>> @@ -7031,6 +7042,18 @@ void noinstr vmx_update_host_rsp(struct vcpu_vmx =
*vmx, unsigned long host_rsp)
>> void noinstr vmx_spec_ctrl_restore_host(struct vcpu_vmx *vmx,
>> 					unsigned int flags)
>> {
>> +	struct kvm_vcpu *vcpu =3D &vmx->vcpu;
>> +
>> +	/* Optimize IPI reduction by setting mode immediately after vmexit
>> +	 * without a memmory barrier as this as not paired anywhere. vcpu->mod=
e
>> +	 * is will be set to OUTSIDE_GUEST_MODE in x86 common code with a memo=
ry
>> +	 * barrier, after the host is done fully restoring various host states=
.
>> +	 * Since the rdmsr and wrmsr below are expensive, this must be done
>> +	 * first, so that the IPI suppression window covers the time dealing
>> +	 * with fixing up SPEC_CTRL.
>> +	 */
>> +	vcpu->mode =3D EXITING_GUEST_MODE;
>> +
>> 	u64 hostval =3D this_cpu_read(x86_spec_ctrl_current);
>>=20
>> 	if (!cpu_feature_enabled(X86_FEATURE_MSR_SPEC_CTRL))
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 2835bd796639..0e0d228f3fa5 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -2160,6 +2160,14 @@ fastpath_t handle_fastpath_set_msr_irqoff(struct =
kvm_vcpu *vcpu)
>> 		data =3D kvm_read_edx_eax(vcpu);
>> 		if (!handle_fastpath_set_tscdeadline(vcpu, data)) {
>> 			kvm_skip_emulated_instruction(vcpu);
>> +			/* Reset IN_GUEST_MODE since we're going to reenter
>> +			 * guest as part of this fast path. This is done as
>> +			 * an optimization without a memory barrier since
>> +			 * EXITING_GUEST_MODE is also set without a memory
>> +			 * barrier.
>> +			 */
>> +			if (vcpu->mode =3D=3D EXITING_GUEST_MODE)
>> +				vcpu->mode =3D IN_GUEST_MODE;
>> 			ret =3D EXIT_FASTPATH_REENTER_GUEST;
>> 		}
>> 		break;