Re: [PATCH 2/2] Allow SEV firmware to be chosen based on Family and Model

From: Gary R Hook <ghook@amd.com>
To: "Natarajan, Janakarajan" <Janakarajan.Natarajan@amd.com>,
	"linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Cc: "Lendacky, Thomas" <Thomas.Lendacky@amd.com>,
	"Hook, Gary" <Gary.Hook@amd.com>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	"David S . Miller" <davem@davemloft.net>,
	"Singh, Brijesh" <brijesh.singh@amd.com>
Subject: Re: [PATCH 2/2] Allow SEV firmware to be chosen based on Family and Model
Date: Wed, 19 Sep 2018 13:41:19 +0000	[thread overview]
Message-ID: <1ac0bcb0-92fd-ea8c-691f-7b80fff5e251@amd.com> (raw)
In-Reply-To: <5c58c32c0ce89c35a95e151734f5d43a3ed0dca0.1536959537.git.Janakarajan.Natarajan@amd.com>

On 09/14/2018 05:32 PM, Janakarajan Natarajan wrote:
> During PSP initialization, there is an attempt to update the SEV firmware
> by looking in /lib/firmware/amd/. Currently, sev.fw is the expected name
> of the firmware blob.
> 
> This patch will allow for firmware filenames based on the family and
> model of the processor.
> 
> Model specific firmware files are given highest priority. Followed by
> firmware for a subset of models. Lastly, failing the previous two options,
> fallback to looking for sev.fw.
> 
> Signed-off-by: Janakarajan Natarajan <Janakarajan.Natarajan@amd.com>

Acked-by: Gary R Hook <gary.hook@amd.com>

> ---
>   drivers/crypto/ccp/psp-dev.c | 44 ++++++++++++++++++++++++++++++++++++++++----
>   1 file changed, 40 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
> index f541e60..3b33863 100644
> --- a/drivers/crypto/ccp/psp-dev.c
> +++ b/drivers/crypto/ccp/psp-dev.c
> @@ -31,8 +31,9 @@
>   		((psp_master->api_major) >= _maj &&	\
>   		 (psp_master->api_minor) >= _min)
>   
> -#define DEVICE_NAME	"sev"
> -#define SEV_FW_FILE	"amd/sev.fw"
> +#define DEVICE_NAME		"sev"
> +#define SEV_FW_FILE		"amd/sev.fw"
> +#define SEV_FW_NAME_SIZE	64
>   
>   static DEFINE_MUTEX(sev_cmd_mutex);
>   static struct sev_misc_dev *misc_dev;
> @@ -440,6 +441,40 @@ static int sev_get_api_version(void)
>   	return 0;
>   }
>   
> +int sev_get_firmware(struct device *dev, const struct firmware **firmware)
> +{
> +	char fw_name_specific[SEV_FW_NAME_SIZE];
> +	char fw_name_subset[SEV_FW_NAME_SIZE];
> +
> +	snprintf(fw_name_specific, sizeof(fw_name_specific),
> +		 "amd/amd_sev_fam%.2xh_model%.2xh.sbin",
> +		 boot_cpu_data.x86, boot_cpu_data.x86_model);
> +
> +	snprintf(fw_name_subset, sizeof(fw_name_subset),
> +		 "amd/amd_sev_fam%.2xh_model%.1xxh.sbin",
> +		 boot_cpu_data.x86, (boot_cpu_data.x86_model & 0xf0) >> 4);
> +
> +	/* Check for SEV FW for a particular model.
> +	 * Ex. amd_sev_fam17h_model00h.sbin for Family 17h Model 00h
> +	 *
> +	 * or
> +	 *
> +	 * Check for SEV FW common to a subset of models.
> +	 * Ex. amd_sev_fam17h_model0xh.sbin for
> +	 *     Family 17h Model 00h -- Family 17h Model 0Fh
> +	 *
> +	 * or
> +	 *
> +	 * Fall-back to using generic name: sev.fw
> +	 */
> +	if ((firmware_request_nowarn(firmware, fw_name_specific, dev) >= 0) ||
> +	    (firmware_request_nowarn(firmware, fw_name_subset, dev) >= 0) ||
> +	    (firmware_request_nowarn(firmware, SEV_FW_FILE, dev) >= 0))
> +		return 0;
> +
> +	return -ENOENT;
> +}
> +
>   /* Don't fail if SEV FW couldn't be updated. Continue with existing SEV FW */
>   static int sev_update_firmware(struct device *dev)
>   {
> @@ -449,9 +484,10 @@ static int sev_update_firmware(struct device *dev)
>   	struct page *p;
>   	u64 data_size;
>   
> -	ret = request_firmware(&firmware, SEV_FW_FILE, dev);
> -	if (ret < 0)
> +	if (sev_get_firmware(dev, &firmware) == -ENOENT) {
> +		dev_dbg(dev, "No SEV firmware file present\n");
>   		return -1;
> +	}
>   
>   	/*
>   	 * SEV FW expects the physical address given to it to be 32
>