Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1;
Date:   Tue, 10 Oct 2023 13:35:08 +0200
From:   Greg KH <gregkh@linuxfoundation.org>
To:     =?iso-8859-1?Q?Jos=E9?= Pekkarinen <jose.pekkarinen@foxhound.fi>
Cc:     seanjc@google.com, pbonzini@redhat.com, skhan@linuxfoundation.org,
        dave.hansen@linux.intel.com, kvm@vger.kernel.org, x86@kernel.org,
        linux-kernel@vger.kernel.org, mingo@redhat.com, bp@alien8.de,
        hpa@zytor.com, tglx@linutronix.de,
        linux-kernel-mentees@lists.linuxfoundation.org
Subject: Re: [PATCH] kvm/sev: make SEV/SEV-ES asids configurable
Message-ID: <2023101050-scuff-overstay-9b43@gregkh>
References: <20231010100441.30950-1-jose.pekkarinen@foxhound.fi>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20231010100441.30950-1-jose.pekkarinen@foxhound.fi>
Precedence: bulk

On Tue, Oct 10, 2023 at 01:04:39PM +0300, Jos? Pekkarinen wrote:
> There are bioses that doesn't allow to configure the
> number of asids allocated for SEV/SEV-ES, for those
> cases, the default behaviour allocates all the asids
> for SEV, leaving no room for SEV-ES to have some fun.

"fun"?

Also, please use the full 72 columns for your changelog.

> If the user request SEV-ES to be enabled, it will
> find the kernel just run out of resources and ignored
> user request. This following patch will address this
> issue by making the number of asids for SEV/SEV-ES
> configurable over kernel module parameters.
> 
> Signed-off-by: Jos? Pekkarinen <jose.pekkarinen@foxhound.fi>
> ---
>  arch/x86/kvm/svm/sev.c | 28 +++++++++++++++++++++++-----
>  1 file changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 07756b7348ae..68a63b42d16a 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -51,9 +51,18 @@
>  static bool sev_enabled = true;
>  module_param_named(sev, sev_enabled, bool, 0444);
>  
> +/* nr of asids requested for SEV */
> +static unsigned int requested_sev_asids;
> +module_param_named(sev_asids, requested_sev_asids, uint, 0444);
> +
>  /* enable/disable SEV-ES support */
>  static bool sev_es_enabled = true;
>  module_param_named(sev_es, sev_es_enabled, bool, 0444);
> +
> +/* nr of asids requested for SEV-ES */
> +static unsigned int requested_sev_es_asids;
> +module_param_named(sev_es_asids, requested_sev_asids, uint, 0444);

Why more module parameters?  Why can't this "just work" properly without
forcing a user to make manual changes?  This isn't the 1990's anymore.


> +
>  #else
>  #define sev_enabled false
>  #define sev_es_enabled false
> @@ -2194,6 +2203,11 @@ void __init sev_hardware_setup(void)
>  	if (!max_sev_asid)
>  		goto out;
>  
> +	if (requested_sev_asids + requested_sev_es_asids > max_sev_asid) {
> +		pr_info("SEV asids requested more than available: %u ASIDs\n", max_sev_asid);

Why isn't this an error?

thanks,

greg k-h