Message ID | 20221129193717.513824-7-mlevitsk@redhat.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:f944:0:0:0:0:0 with SMTP id q4csp537317wrr; Tue, 29 Nov 2022 11:53:33 -0800 (PST) X-Google-Smtp-Source: AA0mqf7M3Gn7so284e1wXRyHRwJCJAqR/jSnU+2F9EcDG/80fB77ShUzLFXOPP2ZDmWkM1kRUlCB X-Received: by 2002:a17:906:2552:b0:7ad:917b:61ec with SMTP id j18-20020a170906255200b007ad917b61ecmr35680537ejb.513.1669751612877; Tue, 29 Nov 2022 11:53:32 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1669751612; cv=none; d=google.com; s=arc-20160816; b=icmPbXO4OSGIIsDnvdCVAJdBusUTDJJDKqtevRwaoV1sL0KZPBXTHRBdH3I5USMcO+ n1mrtNrvH7a1kw5PiX4MMhYF3sAs9h9L/Kxo3+QfB9KBMa6qR7emQz2v7NgHSbUcw8KS 344skuWiMoQvJA/YXjKuRlXEB9C86aFy2DhyLCJQIC3tm9jHRDejuQY29/dIL/uxDJqN mCxRXB5y3d/WbN1xahIlwYDyOhoCscg4EIwfciX9J22en8g3skmFPc7b9pSbT1u4JVXW /BY+jOP0Vg/EK33y1qEGRGtxboowR6opFf0NtSYNmZuLf6JeDzHrFBYG3kco/Iv8NZsn HjEA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=twqdjhN58Mc4yzZpBl1NNFSCL0kf8bUxY7h7dFC7RKY=; b=PbCurzeVaNQrNPAHWDZeKxJLGmSN6r2yJK2yiK2LayCvWBA9oRIkflPBnqsn+x3sS8 gyzmnpdNzvpy1R4o4WJvHTSOfsydySow1ml1hKHl6bQLa1KtkNXuqU92jCt4CjhLSoET M0RbzH6BjN549/0js4aXWrVNk6kE+fC6V7m3uMg7lqHN6T3YfCt3MnI7LwDPZdV59g6S 8yc3jhkCw09pbUvNWMzW9DQb/7JteyPbnOEL2WtLp1CD/g5qJnno2dwlpmjjuXm3ENJd KqbDAcyKmgv1WN1bVBe09bw42y2EgHJ4RDOek5R5GyxQ4oMqtjGrBVMLbFbpwNy+vpJ/ QQ3w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=cg9Rrxj3; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id dt17-20020a170907729100b007317ad1f9a4si11431784ejc.310.2022.11.29.11.53.09; Tue, 29 Nov 2022 11:53:32 -0800 (PST) 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; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=cg9Rrxj3; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236913AbiK2TpI (ORCPT <rfc822;rbbytesnap@gmail.com> + 99 others); Tue, 29 Nov 2022 14:45:08 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56370 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237000AbiK2Toh (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 29 Nov 2022 14:44:37 -0500 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 494F5317DA for <linux-kernel@vger.kernel.org>; Tue, 29 Nov 2022 11:40:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1669750825; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=twqdjhN58Mc4yzZpBl1NNFSCL0kf8bUxY7h7dFC7RKY=; b=cg9Rrxj3daVfhP1HtUMfDWPsXyN4QG6HTt2OeOWEBFLC5N4APCyhWnsrAkAYjro49IgJ/1 aw+hGqlW8hIQfgH2CqlrRysRT+wa5Q4A/FTqrEN6lz4/kJGIxjE5Jy7nXOgyTGFm3QiNTh Wt7vVyjOChoQagvrfqLCPpswgHSb0kA= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-8-e2k6pOH2OU-WXS70I9SEuw-1; Tue, 29 Nov 2022 14:37:48 -0500 X-MC-Unique: e2k6pOH2OU-WXS70I9SEuw-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 7E41F3817A6D; Tue, 29 Nov 2022 19:37:47 +0000 (UTC) Received: from localhost.localdomain (unknown [10.35.206.46]) by smtp.corp.redhat.com (Postfix) with ESMTP id BBA682028DC1; Tue, 29 Nov 2022 19:37:43 +0000 (UTC) From: Maxim Levitsky <mlevitsk@redhat.com> To: kvm@vger.kernel.org Cc: Sandipan Das <sandipan.das@amd.com>, Paolo Bonzini <pbonzini@redhat.com>, Jim Mattson <jmattson@google.com>, Peter Zijlstra <peterz@infradead.org>, Dave Hansen <dave.hansen@linux.intel.com>, Borislav Petkov <bp@alien8.de>, Pawan Gupta <pawan.kumar.gupta@linux.intel.com>, Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, Josh Poimboeuf <jpoimboe@kernel.org>, Daniel Sneddon <daniel.sneddon@linux.intel.com>, Jiaxi Chen <jiaxi.chen@linux.intel.com>, Babu Moger <babu.moger@amd.com>, linux-kernel@vger.kernel.org, Jing Liu <jing2.liu@intel.com>, Wyes Karny <wyes.karny@amd.com>, x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>, Sean Christopherson <seanjc@google.com>, Maxim Levitsky <mlevitsk@redhat.com> Subject: [PATCH v2 06/11] KVM: SVM: add wrappers to enable/disable IRET interception Date: Tue, 29 Nov 2022 21:37:12 +0200 Message-Id: <20221129193717.513824-7-mlevitsk@redhat.com> In-Reply-To: <20221129193717.513824-1-mlevitsk@redhat.com> References: <20221129193717.513824-1-mlevitsk@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 3.1 on 10.11.54.4 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,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=unavailable 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 X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1750861467238693693?= X-GMAIL-MSGID: =?utf-8?q?1750861467238693693?= |
Series |
SVM: vNMI (with my fixes)
|
|
Commit Message
Maxim Levitsky
Nov. 29, 2022, 7:37 p.m. UTC
SEV-ES guests don't use IRET interception for the detection of
an end of a NMI.
Therefore it makes sense to create a wrapper to avoid repeating
the check for the SEV-ES.
No functional change is intended.
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
arch/x86/kvm/svm/svm.c | 28 +++++++++++++++++++---------
1 file changed, 19 insertions(+), 9 deletions(-)
Comments
On 11/30/2022 1:07 AM, Maxim Levitsky wrote: > SEV-ES guests don't use IRET interception for the detection of > an end of a NMI. > > Therefore it makes sense to create a wrapper to avoid repeating > the check for the SEV-ES. > > No functional change is intended. > > Suggested-by: Sean Christopherson <seanjc@google.com> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > --- > arch/x86/kvm/svm/svm.c | 28 +++++++++++++++++++--------- > 1 file changed, 19 insertions(+), 9 deletions(-) > > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c > index 512b2aa21137e2..cfed6ab29c839a 100644 > --- a/arch/x86/kvm/svm/svm.c > +++ b/arch/x86/kvm/svm/svm.c > @@ -2468,16 +2468,29 @@ static int task_switch_interception(struct kvm_vcpu *vcpu) > has_error_code, error_code); > } > > +static void svm_disable_iret_interception(struct vcpu_svm *svm) > +{ > + if (!sev_es_guest(svm->vcpu.kvm)) > + svm_clr_intercept(svm, INTERCEPT_IRET); > +} > + > +static void svm_enable_iret_interception(struct vcpu_svm *svm) > +{ > + if (!sev_es_guest(svm->vcpu.kvm)) > + svm_set_intercept(svm, INTERCEPT_IRET); > +} > + nits: s/_iret_interception / _iret_intercept does that make sense? Thanks, Santosh > static int iret_interception(struct kvm_vcpu *vcpu) > { > struct vcpu_svm *svm = to_svm(vcpu); > > ++vcpu->stat.nmi_window_exits; > svm->awaiting_iret_completion = true; > - if (!sev_es_guest(vcpu->kvm)) { > - svm_clr_intercept(svm, INTERCEPT_IRET); > + > + svm_disable_iret_interception(svm); > + if (!sev_es_guest(vcpu->kvm)) > svm->nmi_iret_rip = kvm_rip_read(vcpu); > - } > + > kvm_make_request(KVM_REQ_EVENT, vcpu); > return 1; > } > @@ -3470,8 +3483,7 @@ static void svm_inject_nmi(struct kvm_vcpu *vcpu) > return; > > svm->nmi_masked = true; > - if (!sev_es_guest(vcpu->kvm)) > - svm_set_intercept(svm, INTERCEPT_IRET); > + svm_enable_iret_interception(svm); > ++vcpu->stat.nmi_injections; > } > > @@ -3614,12 +3626,10 @@ static void svm_set_nmi_mask(struct kvm_vcpu *vcpu, bool masked) > > if (masked) { > svm->nmi_masked = true; > - if (!sev_es_guest(vcpu->kvm)) > - svm_set_intercept(svm, INTERCEPT_IRET); > + svm_enable_iret_interception(svm); > } else { > svm->nmi_masked = false; > - if (!sev_es_guest(vcpu->kvm)) > - svm_clr_intercept(svm, INTERCEPT_IRET); > + svm_disable_iret_interception(svm); > } > } >
On Mon, 2022-12-05 at 21:11 +0530, Santosh Shukla wrote: > On 11/30/2022 1:07 AM, Maxim Levitsky wrote: > > SEV-ES guests don't use IRET interception for the detection of > > an end of a NMI. > > > > Therefore it makes sense to create a wrapper to avoid repeating > > the check for the SEV-ES. > > > > No functional change is intended. > > > > Suggested-by: Sean Christopherson <seanjc@google.com> > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > > --- > > arch/x86/kvm/svm/svm.c | 28 +++++++++++++++++++--------- > > 1 file changed, 19 insertions(+), 9 deletions(-) > > > > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c > > index 512b2aa21137e2..cfed6ab29c839a 100644 > > --- a/arch/x86/kvm/svm/svm.c > > +++ b/arch/x86/kvm/svm/svm.c > > @@ -2468,16 +2468,29 @@ static int task_switch_interception(struct kvm_vcpu *vcpu) > > has_error_code, error_code); > > } > > > > +static void svm_disable_iret_interception(struct vcpu_svm *svm) > > +{ > > + if (!sev_es_guest(svm->vcpu.kvm)) > > + svm_clr_intercept(svm, INTERCEPT_IRET); > > +} > > + > > +static void svm_enable_iret_interception(struct vcpu_svm *svm) > > +{ > > + if (!sev_es_guest(svm->vcpu.kvm)) > > + svm_set_intercept(svm, INTERCEPT_IRET); > > +} > > + > > nits: > s/_iret_interception / _iret_intercept > does that make sense? Makes sense. I can also move this to svm.h near the svm_set_intercept(), I think it better a better place for this function there if no objections. Best regards, Maxim Levitsky > > Thanks, > Santosh > > > static int iret_interception(struct kvm_vcpu *vcpu) > > { > > struct vcpu_svm *svm = to_svm(vcpu); > > > > ++vcpu->stat.nmi_window_exits; > > svm->awaiting_iret_completion = true; > > - if (!sev_es_guest(vcpu->kvm)) { > > - svm_clr_intercept(svm, INTERCEPT_IRET); > > + > > + svm_disable_iret_interception(svm); > > + if (!sev_es_guest(vcpu->kvm)) > > svm->nmi_iret_rip = kvm_rip_read(vcpu); > > - } > > + > > kvm_make_request(KVM_REQ_EVENT, vcpu); > > return 1; > > } > > @@ -3470,8 +3483,7 @@ static void svm_inject_nmi(struct kvm_vcpu *vcpu) > > return; > > > > svm->nmi_masked = true; > > - if (!sev_es_guest(vcpu->kvm)) > > - svm_set_intercept(svm, INTERCEPT_IRET); > > + svm_enable_iret_interception(svm); > > ++vcpu->stat.nmi_injections; > > } > > > > @@ -3614,12 +3626,10 @@ static void svm_set_nmi_mask(struct kvm_vcpu *vcpu, bool masked) > > > > if (masked) { > > svm->nmi_masked = true; > > - if (!sev_es_guest(vcpu->kvm)) > > - svm_set_intercept(svm, INTERCEPT_IRET); > > + svm_enable_iret_interception(svm); > > } else { > > svm->nmi_masked = false; > > - if (!sev_es_guest(vcpu->kvm)) > > - svm_clr_intercept(svm, INTERCEPT_IRET); > > + svm_disable_iret_interception(svm); > > } > > } > >
On 12/6/2022 5:44 PM, Maxim Levitsky wrote: > On Mon, 2022-12-05 at 21:11 +0530, Santosh Shukla wrote: >> On 11/30/2022 1:07 AM, Maxim Levitsky wrote: >>> SEV-ES guests don't use IRET interception for the detection of >>> an end of a NMI. >>> >>> Therefore it makes sense to create a wrapper to avoid repeating >>> the check for the SEV-ES. >>> >>> No functional change is intended. >>> >>> Suggested-by: Sean Christopherson <seanjc@google.com> >>> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> >>> --- >>> arch/x86/kvm/svm/svm.c | 28 +++++++++++++++++++--------- >>> 1 file changed, 19 insertions(+), 9 deletions(-) >>> >>> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c >>> index 512b2aa21137e2..cfed6ab29c839a 100644 >>> --- a/arch/x86/kvm/svm/svm.c >>> +++ b/arch/x86/kvm/svm/svm.c >>> @@ -2468,16 +2468,29 @@ static int task_switch_interception(struct kvm_vcpu *vcpu) >>> has_error_code, error_code); >>> } >>> >>> +static void svm_disable_iret_interception(struct vcpu_svm *svm) >>> +{ >>> + if (!sev_es_guest(svm->vcpu.kvm)) >>> + svm_clr_intercept(svm, INTERCEPT_IRET); >>> +} >>> + >>> +static void svm_enable_iret_interception(struct vcpu_svm *svm) >>> +{ >>> + if (!sev_es_guest(svm->vcpu.kvm)) >>> + svm_set_intercept(svm, INTERCEPT_IRET); >>> +} >>> + >> >> nits: >> s/_iret_interception / _iret_intercept >> does that make sense? > > Makes sense. I can also move this to svm.h near the svm_set_intercept(), I think > it better a better place for this function there if no objections. > I think current approach is fine since function used in svm.c only. but I have no strong opinion on moving to svm.h either ways. Thanks, Santosh > Best regards, > Maxim Levitsky >> >> Thanks, >> Santosh >> >>> static int iret_interception(struct kvm_vcpu *vcpu) >>> { >>> struct vcpu_svm *svm = to_svm(vcpu); >>> >>> ++vcpu->stat.nmi_window_exits; >>> svm->awaiting_iret_completion = true; >>> - if (!sev_es_guest(vcpu->kvm)) { >>> - svm_clr_intercept(svm, INTERCEPT_IRET); >>> + >>> + svm_disable_iret_interception(svm); >>> + if (!sev_es_guest(vcpu->kvm)) >>> svm->nmi_iret_rip = kvm_rip_read(vcpu); >>> - } >>> + >>> kvm_make_request(KVM_REQ_EVENT, vcpu); >>> return 1; >>> } >>> @@ -3470,8 +3483,7 @@ static void svm_inject_nmi(struct kvm_vcpu *vcpu) >>> return; >>> >>> svm->nmi_masked = true; >>> - if (!sev_es_guest(vcpu->kvm)) >>> - svm_set_intercept(svm, INTERCEPT_IRET); >>> + svm_enable_iret_interception(svm); >>> ++vcpu->stat.nmi_injections; >>> } >>> >>> @@ -3614,12 +3626,10 @@ static void svm_set_nmi_mask(struct kvm_vcpu *vcpu, bool masked) >>> >>> if (masked) { >>> svm->nmi_masked = true; >>> - if (!sev_es_guest(vcpu->kvm)) >>> - svm_set_intercept(svm, INTERCEPT_IRET); >>> + svm_enable_iret_interception(svm); >>> } else { >>> svm->nmi_masked = false; >>> - if (!sev_es_guest(vcpu->kvm)) >>> - svm_clr_intercept(svm, INTERCEPT_IRET); >>> + svm_disable_iret_interception(svm); >>> } >>> } >>> > >
On Thu, 2022-12-08 at 17:39 +0530, Santosh Shukla wrote: > > On 12/6/2022 5:44 PM, Maxim Levitsky wrote: > > On Mon, 2022-12-05 at 21:11 +0530, Santosh Shukla wrote: > > > On 11/30/2022 1:07 AM, Maxim Levitsky wrote: > > > > SEV-ES guests don't use IRET interception for the detection of > > > > an end of a NMI. > > > > > > > > Therefore it makes sense to create a wrapper to avoid repeating > > > > the check for the SEV-ES. > > > > > > > > No functional change is intended. > > > > > > > > Suggested-by: Sean Christopherson <seanjc@google.com> > > > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > > > > --- > > > > arch/x86/kvm/svm/svm.c | 28 +++++++++++++++++++--------- > > > > 1 file changed, 19 insertions(+), 9 deletions(-) > > > > > > > > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c > > > > index 512b2aa21137e2..cfed6ab29c839a 100644 > > > > --- a/arch/x86/kvm/svm/svm.c > > > > +++ b/arch/x86/kvm/svm/svm.c > > > > @@ -2468,16 +2468,29 @@ static int task_switch_interception(struct kvm_vcpu *vcpu) > > > > has_error_code, error_code); > > > > } > > > > > > > > +static void svm_disable_iret_interception(struct vcpu_svm *svm) > > > > +{ > > > > + if (!sev_es_guest(svm->vcpu.kvm)) > > > > + svm_clr_intercept(svm, INTERCEPT_IRET); > > > > +} > > > > + > > > > +static void svm_enable_iret_interception(struct vcpu_svm *svm) > > > > +{ > > > > + if (!sev_es_guest(svm->vcpu.kvm)) > > > > + svm_set_intercept(svm, INTERCEPT_IRET); > > > > +} > > > > + > > > > > > nits: > > > s/_iret_interception / _iret_intercept > > > does that make sense? > > > > Makes sense. I can also move this to svm.h near the svm_set_intercept(), I think > > it better a better place for this function there if no objections. > > > I think current approach is fine since function used in svm.c only. but I have > no strong opinion on moving to svm.h either ways. I also think so, just noticed something in case there are any objections. Best regards, Maxim Levitsky > > Thanks, > Santosh > > > Best regards, > > Maxim Levitsky > > > Thanks, > > > Santosh > > > > > > > static int iret_interception(struct kvm_vcpu *vcpu) > > > > { > > > > struct vcpu_svm *svm = to_svm(vcpu); > > > > > > > > ++vcpu->stat.nmi_window_exits; > > > > svm->awaiting_iret_completion = true; > > > > - if (!sev_es_guest(vcpu->kvm)) { > > > > - svm_clr_intercept(svm, INTERCEPT_IRET); > > > > + > > > > + svm_disable_iret_interception(svm); > > > > + if (!sev_es_guest(vcpu->kvm)) > > > > svm->nmi_iret_rip = kvm_rip_read(vcpu); > > > > - } > > > > + > > > > kvm_make_request(KVM_REQ_EVENT, vcpu); > > > > return 1; > > > > } > > > > @@ -3470,8 +3483,7 @@ static void svm_inject_nmi(struct kvm_vcpu *vcpu) > > > > return; > > > > > > > > svm->nmi_masked = true; > > > > - if (!sev_es_guest(vcpu->kvm)) > > > > - svm_set_intercept(svm, INTERCEPT_IRET); > > > > + svm_enable_iret_interception(svm); > > > > ++vcpu->stat.nmi_injections; > > > > } > > > > > > > > @@ -3614,12 +3626,10 @@ static void svm_set_nmi_mask(struct kvm_vcpu *vcpu, bool masked) > > > > > > > > if (masked) { > > > > svm->nmi_masked = true; > > > > - if (!sev_es_guest(vcpu->kvm)) > > > > - svm_set_intercept(svm, INTERCEPT_IRET); > > > > + svm_enable_iret_interception(svm); > > > > } else { > > > > svm->nmi_masked = false; > > > > - if (!sev_es_guest(vcpu->kvm)) > > > > - svm_clr_intercept(svm, INTERCEPT_IRET); > > > > + svm_disable_iret_interception(svm); > > > > } > > > > } > > > >
On Thu, Dec 08, 2022, Maxim Levitsky wrote: > On Thu, 2022-12-08 at 17:39 +0530, Santosh Shukla wrote: > > > > On 12/6/2022 5:44 PM, Maxim Levitsky wrote: > > > > > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c > > > > > index 512b2aa21137e2..cfed6ab29c839a 100644 > > > > > --- a/arch/x86/kvm/svm/svm.c > > > > > +++ b/arch/x86/kvm/svm/svm.c > > > > > @@ -2468,16 +2468,29 @@ static int task_switch_interception(struct kvm_vcpu *vcpu) > > > > > has_error_code, error_code); > > > > > } > > > > > > > > > > +static void svm_disable_iret_interception(struct vcpu_svm *svm) > > > > > +{ > > > > > + if (!sev_es_guest(svm->vcpu.kvm)) > > > > > + svm_clr_intercept(svm, INTERCEPT_IRET); > > > > > +} > > > > > + > > > > > +static void svm_enable_iret_interception(struct vcpu_svm *svm) > > > > > +{ > > > > > + if (!sev_es_guest(svm->vcpu.kvm)) > > > > > + svm_set_intercept(svm, INTERCEPT_IRET); > > > > > +} > > > > > + > > > > > > > > nits: > > > > s/_iret_interception / _iret_intercept > > > > does that make sense? > > > > > > Makes sense. I would rather go with svm_{clr,set}_iret_intercept(). I don't particularly like the SVM naming scheme, but I really dislike inconsistent naming. If we want to clean up naming, I would love unify VMX and SVM nomenclature for things like this. > > > I can also move this to svm.h near the svm_set_intercept(), I think > > > it better a better place for this function there if no objections. > > > > > I think current approach is fine since function used in svm.c only. but I have > > no strong opinion on moving to svm.h either ways. > > I also think so, just noticed something in case there are any objections. My vote is to keep it in svm.c unless we anticipate usage outside of svm.h. Keeping the implementation close to the usage makes it easer to understand what's going on, especially for something like this where there's a bit of "hidden" logic for SEV-ES.
On 2/1/2023 2:37 AM, Sean Christopherson wrote: > On Thu, Dec 08, 2022, Maxim Levitsky wrote: >> On Thu, 2022-12-08 at 17:39 +0530, Santosh Shukla wrote: >>> >>> On 12/6/2022 5:44 PM, Maxim Levitsky wrote: >>>>>> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c >>>>>> index 512b2aa21137e2..cfed6ab29c839a 100644 >>>>>> --- a/arch/x86/kvm/svm/svm.c >>>>>> +++ b/arch/x86/kvm/svm/svm.c >>>>>> @@ -2468,16 +2468,29 @@ static int task_switch_interception(struct kvm_vcpu *vcpu) >>>>>> has_error_code, error_code); >>>>>> } >>>>>> >>>>>> +static void svm_disable_iret_interception(struct vcpu_svm *svm) >>>>>> +{ >>>>>> + if (!sev_es_guest(svm->vcpu.kvm)) >>>>>> + svm_clr_intercept(svm, INTERCEPT_IRET); >>>>>> +} >>>>>> + >>>>>> +static void svm_enable_iret_interception(struct vcpu_svm *svm) >>>>>> +{ >>>>>> + if (!sev_es_guest(svm->vcpu.kvm)) >>>>>> + svm_set_intercept(svm, INTERCEPT_IRET); >>>>>> +} >>>>>> + >>>>> >>>>> nits: >>>>> s/_iret_interception / _iret_intercept >>>>> does that make sense? >>>> >>>> Makes sense. > > I would rather go with svm_{clr,set}_iret_intercept(). I don't particularly like ok. > the SVM naming scheme, but I really dislike inconsistent naming. If we want to > clean up naming, I would love unify VMX and SVM nomenclature for things like this. > >>>> I can also move this to svm.h near the svm_set_intercept(), I think >>>> it better a better place for this function there if no objections. >>>> >>> I think current approach is fine since function used in svm.c only. but I have >>> no strong opinion on moving to svm.h either ways. >> >> I also think so, just noticed something in case there are any objections. > > My vote is to keep it in svm.c unless we anticipate usage outside of svm.h. Keeping ok. Thanks, Santosh > the implementation close to the usage makes it easer to understand what's going on, > especially for something like this where there's a bit of "hidden" logic for SEV-ES.
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index 512b2aa21137e2..cfed6ab29c839a 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -2468,16 +2468,29 @@ static int task_switch_interception(struct kvm_vcpu *vcpu) has_error_code, error_code); } +static void svm_disable_iret_interception(struct vcpu_svm *svm) +{ + if (!sev_es_guest(svm->vcpu.kvm)) + svm_clr_intercept(svm, INTERCEPT_IRET); +} + +static void svm_enable_iret_interception(struct vcpu_svm *svm) +{ + if (!sev_es_guest(svm->vcpu.kvm)) + svm_set_intercept(svm, INTERCEPT_IRET); +} + static int iret_interception(struct kvm_vcpu *vcpu) { struct vcpu_svm *svm = to_svm(vcpu); ++vcpu->stat.nmi_window_exits; svm->awaiting_iret_completion = true; - if (!sev_es_guest(vcpu->kvm)) { - svm_clr_intercept(svm, INTERCEPT_IRET); + + svm_disable_iret_interception(svm); + if (!sev_es_guest(vcpu->kvm)) svm->nmi_iret_rip = kvm_rip_read(vcpu); - } + kvm_make_request(KVM_REQ_EVENT, vcpu); return 1; } @@ -3470,8 +3483,7 @@ static void svm_inject_nmi(struct kvm_vcpu *vcpu) return; svm->nmi_masked = true; - if (!sev_es_guest(vcpu->kvm)) - svm_set_intercept(svm, INTERCEPT_IRET); + svm_enable_iret_interception(svm); ++vcpu->stat.nmi_injections; } @@ -3614,12 +3626,10 @@ static void svm_set_nmi_mask(struct kvm_vcpu *vcpu, bool masked) if (masked) { svm->nmi_masked = true; - if (!sev_es_guest(vcpu->kvm)) - svm_set_intercept(svm, INTERCEPT_IRET); + svm_enable_iret_interception(svm); } else { svm->nmi_masked = false; - if (!sev_es_guest(vcpu->kvm)) - svm_clr_intercept(svm, INTERCEPT_IRET); + svm_disable_iret_interception(svm); } }