From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5AB75C4332F for ; Mon, 21 Nov 2022 17:20:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231219AbiKURUg (ORCPT ); Mon, 21 Nov 2022 12:20:36 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43314 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231624AbiKURTz (ORCPT ); Mon, 21 Nov 2022 12:19:55 -0500 Received: from mail-pf1-x42a.google.com (mail-pf1-x42a.google.com [IPv6:2607:f8b0:4864:20::42a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 82394D39F5 for ; Mon, 21 Nov 2022 09:19:00 -0800 (PST) Received: by mail-pf1-x42a.google.com with SMTP id z26so11938571pff.1 for ; Mon, 21 Nov 2022 09:19:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=6eRZBGDIdbbq9qEGvVuNy8kXEqzsVLRSF+qgp5vgUO8=; b=WgKA2gNPMTazNZjE8OQZOHcA6LZD75wsUT5jr8Nis1NcAia7+INt91bOnVbHOQZZmQ SOHJ8kd8g3nzVjswgCKzRgrxNGttkw2Ao3r0ntVn25DJ9TPD/2VG8psKvY3UusWBgEBg MbPgThhotVjseSYey/ir+dHgGKFdC0Hyv5PSxI7oBD6wZfvkcVbmYB5J5R4l1DMFcSrl L+tiKq7d0f/cKy4hnsmN84hC6NuqgX81qytsDFkG/ayFiru+uhXTKpd+cuZ63UjKasYL X99C7Py5r1ejvF19zLeJQJwh68StE4QpQoOn8BLeG5i2qlgHp3A7jKsJ2wS4HXtcpyz4 5EFg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=6eRZBGDIdbbq9qEGvVuNy8kXEqzsVLRSF+qgp5vgUO8=; b=gr9y/ORmxJnuRmNCnlOfuoAEWFk0dtRQ1bVdCjpCrAxMbhceRrzUBjfouhbVly0osg yEMOcr5T0DviFiWSs6klzUuc65OLBOYA+mzNkbjnDv20oaHYuhjBDVkpP3NtXu+BTYaf CpTAvvpAV3AgFgDhztQWCFyqdDrG1lNdEMVenQspAwGiwIFDYxd9FtPI/NKuHSDmjxZg Vlwtg65+jsiJw1mUMsUg45DFyHzjcQyr+blZaZuOYoZEa3ICK8MTpNiAFQKS1lZzsjBk gFRG5MDbFwGMw7g5jKhiSZy7bKuiSUVB3fEUnWNeWJ8HkVEE5hBMrGA7lDWa6UwZYH0A 0JsA== X-Gm-Message-State: ANoB5pl7TAMmLPXoKcPrinj4Vze3p5CjIlDDmNHRXV6ydONru3tEslgr yx5WAoBphU/F8oLikgqIszaTmQ== X-Google-Smtp-Source: AA0mqf6UbtOJWyPSSbxlcWPAqvLAbrsV8KdeveRYIEhwW1KycZzX12rmKKmP7aXQLAqvVYAdiPDyCw== X-Received: by 2002:aa7:8d88:0:b0:562:69d4:f9f3 with SMTP id i8-20020aa78d88000000b0056269d4f9f3mr985874pfr.5.1669051139867; Mon, 21 Nov 2022 09:18:59 -0800 (PST) Received: from google.com (7.104.168.34.bc.googleusercontent.com. [34.168.104.7]) by smtp.gmail.com with ESMTPSA id cu9-20020a17090afa8900b00209a12b3879sm8033740pjb.37.2022.11.21.09.18.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Nov 2022 09:18:59 -0800 (PST) Date: Mon, 21 Nov 2022 17:18:56 +0000 From: Sean Christopherson To: Maxim Levitsky Cc: kvm@vger.kernel.org, Paolo Bonzini , Ingo Molnar , "H. Peter Anvin" , Dave Hansen , linux-kernel@vger.kernel.org, Peter Zijlstra , Thomas Gleixner , Sandipan Das , Daniel Sneddon , Jing Liu , Josh Poimboeuf , Wyes Karny , Borislav Petkov , Babu Moger , Pawan Gupta , Jim Mattson , x86@kernel.org, Santosh Shukla Subject: Re: [PATCH 07/13] KVM: SVM: Add VNMI support in get/set_nmi_mask Message-ID: References: <20221117143242.102721-1-mlevitsk@redhat.com> <20221117143242.102721-8-mlevitsk@redhat.com> <4aadf4616e4f1c6219e7c83ee491494feefa78e1.camel@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <4aadf4616e4f1c6219e7c83ee491494feefa78e1.camel@redhat.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Nov 21, 2022, Maxim Levitsky wrote: > On Thu, 2022-11-17 at 18:54 +0000, Sean Christopherson wrote: > > E.g. with HF_NMI_MASK => svm->nmi_masked, the end result can be something like: > > > > static bool __is_vnmi_enabled(struct *vmcb) > > { > >         return !!(vmcb->control.int_ctl & V_NMI_ENABLE); > > } > > > > static bool is_vnmi_enabled(struct vcpu_svm *svm) > > { > >         struct vmcb *vmcb = get_vnmi_vmcb(svm); > > > >         return vmcb && __is_vnmi_enabled(vmcb); > > } > > > > static bool svm_get_nmi_mask(struct kvm_vcpu *vcpu) > > { > >         struct vcpu_svm *svm = to_svm(vcpu); > >         struct vmcb *vmcb = get_vnmi_vmcb(svm); > > > >         if (vmcb && __is_vnmi_enabled(vmcb)) > >                 return !!(vmcb->control.int_ctl & V_NMI_MASK); > >         else > >                 return !!(vcpu->arch.hflags & HF_NMI_MASK); > > } > > > > static void svm_set_or_clear_vnmi_mask(struct vmcb *vmcb, bool set) > > { > >         if (set) > >                 vmcb->control.int_ctl |= V_NMI_MASK; > >         else > >                 vmcb->control.int_ctl &= ~V_NMI_MASK; > > } > > > > static void svm_set_nmi_mask(struct kvm_vcpu *vcpu, bool masked) > > { > >         struct vcpu_svm *svm = to_svm(vcpu); > >         struct vmcb *vmcb = get_vnmi_vmcb(svm); > > > >         if (vmcb && __is_vnmi_enabled(vmcb)) { > >                 if (masked) > >                         vmcb->control.int_ctl |= V_NMI_MASK; > >                 else > >                         vmcb->control.int_ctl &= ~V_NMI_MASK; > >         } else { > >                 svm->nmi_masked = masked; > >         } > > > >         if (!masked) > >                 svm_disable_iret_interception(svm); > > } > > OK, this is one of the ways to do it, makes sense overall. > I actualy wanted to do something like that but opted to not touch > the original code too much, but only what I needed. I can do this > in a next version. After looking at more of this code, I think having get_vnmi_vmcb() is a mistake. It just ends up being a funky wrapper to the current svm->vmcb. And the manual check on the "vnmi" global is pointless. If KVM sets V_NMI_ENABLE in any VMCB when vnmi=false, then that's a KVM bug. Dropping the wrapper eliminates the possibility of a NULL VMCB pointer, and IMO yields far more readable code. static bool is_vnmi_enabled(struct vcpu_svm *svm) { return !!(svm->vmcb->control.int_ctl & V_NMI_ENABLE); } static bool svm_get_nmi_mask(struct kvm_vcpu *vcpu) { struct vcpu_svm *svm = to_svm(vcpu); if (is_vnmi_enabled(svm)) return !!(svm->vmcb->control.int_ctl & V_NMI_MASK); else return !!(vcpu->arch.hflags & HF_NMI_MASK); } static void svm_set_nmi_mask(struct kvm_vcpu *vcpu, bool masked) { struct vcpu_svm *svm = to_svm(vcpu); if (is_vnmi_enabled(svm)) { if (masked) svm->vmcb->control.int_ctl |= V_NMI_MASK; else svm->vmcb->control.int_ctl &= ~V_NMI_MASK; } else { svm->nmi_masked = masked; } if (!masked) svm_disable_iret_interception(svm); }