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 X-Spam-Level: X-Spam-Status: No, score=-15.4 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FSL_HELO_FAKE, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 915C2C636CA for ; Fri, 16 Jul 2021 18:00:34 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 21F39613D3 for ; Fri, 16 Jul 2021 18:00:34 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 21F39613D3 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 147998D00F4; Fri, 16 Jul 2021 14:00:34 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 11E7F8D00EC; Fri, 16 Jul 2021 14:00:34 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EDA988D00F4; Fri, 16 Jul 2021 14:00:33 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0068.hostedemail.com [216.40.44.68]) by kanga.kvack.org (Postfix) with ESMTP id C34348D00EC for ; Fri, 16 Jul 2021 14:00:33 -0400 (EDT) Received: from smtpin23.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 7829D2B364 for ; Fri, 16 Jul 2021 18:00:32 +0000 (UTC) X-FDA: 78369215904.23.5BE7EF0 Received: from mail-pf1-f180.google.com (mail-pf1-f180.google.com [209.85.210.180]) by imf01.hostedemail.com (Postfix) with ESMTP id 317AB5019DAB for ; Fri, 16 Jul 2021 18:00:32 +0000 (UTC) Received: by mail-pf1-f180.google.com with SMTP id q10so9529923pfj.12 for ; Fri, 16 Jul 2021 11:00:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=JJT3j10iJIIOsPHEEnbNQuVLTw5JnzSQ/acvTlgZAJ8=; b=GwzdfStOFnu5bUYpHJmW5qaywa/s2A2QUEultL+m1JqZOp4yK+grRGfNrwTW9gYDni UqvwTqYFLACzmBr+3+oAJUIEDDk0SfGX3M+dkfZq23+XWfGrzkVviYu0lCuiXM9PkmfN VnO3DYxEBTHGP7EBr9YSyyvciKgdSDS/ekdvhUo9MHlz9j6dStUSGQcBD0oB8sHJKH3U gWA5LIEuf9cVeVGn0fTk3/vs6U082z3MZAKK3zm+d4QRce0q2sMrV2JKMM+i6LkRQuyQ 6PU1WFaWteTtKwq+nAD46Kkt1uKcDZ7ZIHdLOQiNxLErn0Ufz5tdxfui9LCKHwLL6ctE vBnA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=JJT3j10iJIIOsPHEEnbNQuVLTw5JnzSQ/acvTlgZAJ8=; b=ilA/pPQmRsMoV4N/kcFfeqWMABFi6AelMqfhUQURYDpoBeqKJr8WzMLRWC8iD1whIU 0F8MTxzJWI2YLS/Z2ujwlvOIkgXwm2zgAxmikW388HL3iaMUaDvvHPDwJmag7eu8ig1c 4EqZNNczEDYBLNXZXcTDs7MHPmH0+HnLQGEX5p4RzCofU0HFKzGcMykwtlqCfgMYgBiB fK9Uw1b6pXSwziUHjgoIWqefBaCKaidmXSlvCMy2zvzK4mFVhjxklAdmLA/ZqC2DAB5b uVN62seNBoj1sFTvhFPPAdlx4T/hw8f7W9MBPGTErQrB7LWXbpEMWJ0GYkkSZifKrl92 cSWw== X-Gm-Message-State: AOAM530xOwa/e/UAIZCuN7ARFJSnEC7jCpyJNCjYW2phKaaJkL6G+vw3 bkBzQn08Fr/wDekLm2rUKNbNVg== X-Google-Smtp-Source: ABdhPJyJxNSW4W+lQGEGZFuQKmM6xXoN4SsZLj1WhAcFQMeACiCuwpzphldu3fpnqiStbul1Zx/cQA== X-Received: by 2002:a65:6118:: with SMTP id z24mr11085274pgu.325.1626458430711; Fri, 16 Jul 2021 11:00:30 -0700 (PDT) Received: from google.com (157.214.185.35.bc.googleusercontent.com. [35.185.214.157]) by smtp.gmail.com with ESMTPSA id cx4sm12694608pjb.53.2021.07.16.11.00.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 16 Jul 2021 11:00:30 -0700 (PDT) Date: Fri, 16 Jul 2021 18:00:26 +0000 From: Sean Christopherson To: Brijesh Singh Cc: x86@kernel.org, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, linux-efi@vger.kernel.org, platform-driver-x86@vger.kernel.org, linux-coco@lists.linux.dev, linux-mm@kvack.org, linux-crypto@vger.kernel.org, Thomas Gleixner , Ingo Molnar , Joerg Roedel , Tom Lendacky , "H. Peter Anvin" , Ard Biesheuvel , Paolo Bonzini , Vitaly Kuznetsov , Wanpeng Li , Jim Mattson , Andy Lutomirski , Dave Hansen , Sergio Lopez , Peter Gonda , Peter Zijlstra , Srinivas Pandruvada , David Rientjes , Dov Murik , Tobin Feldman-Fitzthum , Borislav Petkov , Michael Roth , Vlastimil Babka , tony.luck@intel.com, npmccallum@redhat.com, brijesh.ksingh@gmail.com Subject: Re: [PATCH Part2 RFC v4 21/40] KVM: SVM: Add initial SEV-SNP support Message-ID: References: <20210707183616.5620-1-brijesh.singh@amd.com> <20210707183616.5620-22-brijesh.singh@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210707183616.5620-22-brijesh.singh@amd.com> X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 317AB5019DAB X-Stat-Signature: jaf4e9xwamk7u3hrwkmie96ygketfie5 Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=google.com header.s=20161025 header.b=GwzdfStO; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf01.hostedemail.com: domain of seanjc@google.com designates 209.85.210.180 as permitted sender) smtp.mailfrom=seanjc@google.com X-HE-Tag: 1626458432-844533 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Wed, Jul 07, 2021, Brijesh Singh wrote: > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c > index 411ed72f63af..abca2b9dee83 100644 > --- a/arch/x86/kvm/svm/sev.c > +++ b/arch/x86/kvm/svm/sev.c > @@ -52,9 +52,14 @@ module_param_named(sev, sev_enabled, bool, 0444); > /* enable/disable SEV-ES support */ > static bool sev_es_enabled = true; > module_param_named(sev_es, sev_es_enabled, bool, 0444); > + > +/* enable/disable SEV-SNP support */ > +static bool sev_snp_enabled = true; Is it safe to incrementally introduce SNP support? Or should the module param be hidden until all support is in place? E.g. what will happen when KVM allows userspace to create SNP guests but doesn't yet have the RMP management added? > +module_param_named(sev_snp, sev_snp_enabled, bool, 0444); > #else > #define sev_enabled false > #define sev_es_enabled false > +#define sev_snp_enabled false > #endif /* CONFIG_KVM_AMD_SEV */ > > #define AP_RESET_HOLD_NONE 0 > @@ -1825,6 +1830,7 @@ void __init sev_hardware_setup(void) > { > #ifdef CONFIG_KVM_AMD_SEV > unsigned int eax, ebx, ecx, edx, sev_asid_count, sev_es_asid_count; > + bool sev_snp_supported = false; > bool sev_es_supported = false; > bool sev_supported = false; > > @@ -1888,9 +1894,21 @@ void __init sev_hardware_setup(void) > pr_info("SEV-ES supported: %u ASIDs\n", sev_es_asid_count); > sev_es_supported = true; > > + /* SEV-SNP support requested? */ > + if (!sev_snp_enabled) > + goto out; > + > + /* Is SEV-SNP enabled? */ > + if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP)) Random question, why use cpu_feature_enabled? Did something change in cpufeatures that prevents using boot_cpu_has() here? > + goto out; > + > + pr_info("SEV-SNP supported: %u ASIDs\n", min_sev_asid - 1); Use sev_es_asid_count instead of manually recomputing the same; the latter obfuscates the fact that ES and SNP share the same ASID pool. Even better would be to report ES+SNP together, otherwise the user could easily interpret ES and SNP having separate ASID pools. And IMO the gotos for SNP are overkill, e.g. sev_es_supported = true; sev_snp_supported = sev_snp_enabled && cpu_feature_enabled(X86_FEATURE_SEV_SNP); pr_info("SEV-ES %ssupported: %u ASIDs\n", sev_snp_supported ? "and SEV-SNP " : "", sev_es_asid_count); > + sev_snp_supported = true; > + > out: > sev_enabled = sev_supported; > sev_es_enabled = sev_es_supported; > + sev_snp_enabled = sev_snp_supported; > #endif > } > > diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h > index 1175edb02d33..b9ea99f8579e 100644 > --- a/arch/x86/kvm/svm/svm.h > +++ b/arch/x86/kvm/svm/svm.h > @@ -58,6 +58,7 @@ enum { > struct kvm_sev_info { > bool active; /* SEV enabled guest */ > bool es_active; /* SEV-ES enabled guest */ > + bool snp_active; /* SEV-SNP enabled guest */ > unsigned int asid; /* ASID used for this guest */ > unsigned int handle; /* SEV firmware handle */ > int fd; /* SEV device fd */ > @@ -232,6 +233,17 @@ static inline bool sev_es_guest(struct kvm *kvm) > #endif > } > > +static inline bool sev_snp_guest(struct kvm *kvm) > +{ > +#ifdef CONFIG_KVM_AMD_SEV > + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info; > + > + return sev_es_guest(kvm) && sev->snp_active; Can't this be reduced to: return to_kvm_svm(kvm)->sev_info.snp_active; KVM should never set snp_active without also setting es_active. Side topic, I think it would also be worthwhile to add to_sev (or maybe to_kvm_sev) given the frequency of the "&to_kvm_svm(kvm)->sev_info" pattern. > +#else > + return false; > +#endif > +} > + > static inline void vmcb_mark_all_dirty(struct vmcb *vmcb) > { > vmcb->control.clean = 0; > -- > 2.17.1 >