linux-kernel - Re: [PATCH v2 7/9] crypto: ccp: Add new SEV/SNP platform initialization API

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2cbe9026-ce18-42c6-b8fd-750c55dde5a3@amd.com>
Date: Fri, 27 Dec 2024 21:25:12 +1100
From: Alexey Kardashevskiy <aik@....com>
To: Ashish Kalra <Ashish.Kalra@....com>, seanjc@...gle.com,
 pbonzini@...hat.com, tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
 dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com,
 thomas.lendacky@....com, john.allen@....com, herbert@...dor.apana.org.au,
 davem@...emloft.net
Cc: michael.roth@....com, dionnaglaze@...gle.com, kvm@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-crypto@...r.kernel.org,
 linux-coco@...ts.linux.dev
Subject: Re: [PATCH v2 7/9] crypto: ccp: Add new SEV/SNP platform
 initialization API

On 17/12/24 10:59, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@....com>
> 
> Add new SNP platform initialization API to allow separate SEV and SNP
> initialization.
> 
> Signed-off-by: Ashish Kalra <ashish.kalra@....com>
> ---
>   drivers/crypto/ccp/sev-dev.c | 15 +++++++++++++++
>   include/linux/psp-sev.h      | 17 +++++++++++++++++
>   2 files changed, 32 insertions(+)
> 
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index 001e7a401a6d..53c438b2b712 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -1375,6 +1375,21 @@ int sev_platform_init(struct sev_platform_init_args *args)
>   }
>   EXPORT_SYMBOL_GPL(sev_platform_init);
>   
> +int sev_snp_platform_init(struct sev_platform_init_args *args)
> +{
> +	int rc;
> +
> +	if (!psp_master || !psp_master->sev_data)
> +		return -ENODEV;
> +
> +	mutex_lock(&sev_cmd_mutex);


I'm told that in 2024 we should use guard(mutex)(&sev_cmd_mutex) and 
drop explicit mutex_unlock(). I'm not a huge fan but there is a point :)


> +	rc = __sev_snp_init_locked(&args->error);
> +	mutex_unlock(&sev_cmd_mutex);
> +
> +	return rc;
> +}
> +EXPORT_SYMBOL_GPL(sev_snp_platform_init);
> +
>   static int __sev_platform_shutdown_locked(int *error)
>   {
>   	struct psp_device *psp = psp_master;
> diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
> index 335b29b31457..e50643aef8a9 100644
> --- a/include/linux/psp-sev.h
> +++ b/include/linux/psp-sev.h
> @@ -828,6 +828,21 @@ struct sev_data_snp_commit {
>    */
>   int sev_platform_init(struct sev_platform_init_args *args);
>   
> +/**
> + * sev_snp_platform_init - perform SNP INIT command
> + *
> + * @args: struct sev_platform_init_args to pass in arguments
> + *
> + * Returns:
> + * 0 if the SEV successfully processed the command
> + * -%ENODEV    if the SNP support is not enabled
> + * -%ENOMEM    if the SNP range list allocation failed
> + * -%E2BIG     if the HV_Fixed list is too big
> + * -%ETIMEDOUT if the SEV command timed out
> + * -%EIO       if the SEV returned a non-zero return code

The only caller ignores these, may be drop the returning value and print 
the errors inside sev_snp_platform_init() (if whatever 
__sev_snp_init_locked() already prints is not enough)?

Also, looks like 5/9 6/9 7/9 can be squashed into one patch, they touch 
the same files, equally do nothing until later patches, pretty straight 
forward. Thanks,


> + */
> +int sev_snp_platform_init(struct sev_platform_init_args *args);
> +
>   /**
>    * sev_platform_status - perform SEV PLATFORM_STATUS command
>    *
> @@ -955,6 +970,8 @@ sev_platform_status(struct sev_user_data_status *status, int *error) { return -E
>   
>   static inline int sev_platform_init(struct sev_platform_init_args *args) { return -ENODEV; }
>   
> +static inline int sev_snp_platform_init(struct sev_platform_init_args *args) { return -ENODEV; }
> +
>   static inline int
>   sev_guest_deactivate(struct sev_data_deactivate *data, int *error) { return -ENODEV; }
>   

-- 
Alexey