diff mbox series

[Part2,v6,12/49] crypto: ccp: Add support to initialize the AMD-SP for SEV-SNP

Message ID 87a0481526e66ddd5f6192cbb43a50708aee2883.1655761627.git.ashish.kalra@amd.com (mailing list archive)
State New, archived
Headers show
Series Add AMD Secure Nested Paging (SEV-SNP) | expand

Commit Message

Kalra, Ashish June 20, 2022, 11:04 p.m. UTC
From: Brijesh Singh <brijesh.singh@amd.com>

Before SNP VMs can be launched, the platform must be appropriately
configured and initialized. Platform initialization is accomplished via
the SNP_INIT command. Make sure to do a WBINVD and issue DF_FLUSH command
to prepare for the first SNP guest launch after INIT.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Signed-off by: Ashish Kalra <ashish.kalra@amd.com>
---
 drivers/crypto/ccp/sev-dev.c | 121 +++++++++++++++++++++++++++++++++++
 drivers/crypto/ccp/sev-dev.h |   2 +
 include/linux/psp-sev.h      |  16 +++++
 3 files changed, 139 insertions(+)

Comments

Borislav Petkov Oct. 1, 2022, 5:33 p.m. UTC | #1
On Mon, Jun 20, 2022 at 11:04:29PM +0000, Ashish Kalra wrote:
> +static int __sev_snp_init_locked(int *error)
> +{
> +	struct psp_device *psp = psp_master;
> +	struct sev_device *sev;
> +	int rc = 0;
> +
> +	if (!psp || !psp->sev_data)
> +		return -ENODEV;
> +
> +	sev = psp->sev_data;
> +
> +	if (sev->snp_inited)

snp_inited? That's silly.

	snp_initialized

pls.

> +		return 0;
> +
> +	/*
> +	 * The SNP_INIT requires the MSR_VM_HSAVE_PA must be set to 0h

	/* Clear MSR_VM_HSAVE_PA on all cores before SNP_INIT */

> +	 * across all cores.
> +	 */
> +	on_each_cpu(snp_set_hsave_pa, NULL, 1);
> +
> +	/* Issue the SNP_INIT firmware command. */

Useless comment.

> +	rc = __sev_do_cmd_locked(SEV_CMD_SNP_INIT, NULL, error);
> +	if (rc)
> +		return rc;
> +
> +	/* Prepare for first SNP guest launch after INIT */
> +	wbinvd_on_all_cpus();

Can you put a wbinvd() in snp_set_hsave_pa() instead and save yourself
the second IPI?

Or is that order of the commands:

	1. clear MSR IPI
	2. SNP_INIT
	3. WBINVD IPI
	4. ...

mandatory?

...

> +static int __sev_snp_shutdown_locked(int *error)
> +{
> +	struct sev_device *sev = psp_master->sev_data;
> +	int ret;
> +
> +	if (!sev->snp_inited)
> +		return 0;
> +
> +	/* SHUTDOWN requires the DF_FLUSH */
> +	wbinvd_on_all_cpus();
> +	__sev_do_cmd_locked(SEV_CMD_SNP_DF_FLUSH, NULL, NULL);

Why isn't this retval checked?

> +
> +	ret = __sev_do_cmd_locked(SEV_CMD_SNP_SHUTDOWN, NULL, error);
> +	if (ret) {
> +		dev_err(sev->dev, "SEV-SNP firmware shutdown failed\n");
> +		return ret;
> +	}
> +
> +	sev->snp_inited = false;
> +	dev_dbg(sev->dev, "SEV-SNP firmware shutdown\n");
> +
> +	return ret;
> +}

...

>  void sev_dev_destroy(struct psp_device *psp)
> @@ -1287,6 +1385,26 @@ void sev_pci_init(void)
>  		}
>  	}
>  
> +	/*
> +	 * If boot CPU supports the SNP, then first attempt to initialize

s/the SNP/SNP/g

> +	 * the SNP firmware.
> +	 */
> +	if (cpu_feature_enabled(X86_FEATURE_SEV_SNP)) {
> +		if (!sev_version_greater_or_equal(SNP_MIN_API_MAJOR, SNP_MIN_API_MINOR)) {
> +			dev_err(sev->dev, "SEV-SNP support requires firmware version >= %d:%d\n",
> +				SNP_MIN_API_MAJOR, SNP_MIN_API_MINOR);
> +		} else {
> +			rc = sev_snp_init(&error);
> +			if (rc) {
> +				/*
> +				 * If we failed to INIT SNP then don't abort the probe.

Who's "we"?

> +				 * Continue to initialize the legacy SEV firmware.
> +				 */
> +				dev_err(sev->dev, "SEV-SNP: failed to INIT error %#x\n", error);
> +			}
> +		}
> +	}
> +
>  	/* Obtain the TMR memory area for SEV-ES use */
>  	sev_es_tmr = sev_fw_alloc(SEV_ES_TMR_SIZE);
>  	if (!sev_es_tmr)

...

> diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
> index 01ba9dc46ca3..ef4d42e8c96e 100644
> --- a/include/linux/psp-sev.h
> +++ b/include/linux/psp-sev.h
> @@ -769,6 +769,20 @@ struct sev_data_snp_init_ex {
>   */
>  int sev_platform_init(int *error);
>  
> +/**
> + * sev_snp_init - perform SEV SNP_INIT command
> + *
> + * @error: SEV command return code
> + *
> + * Returns:
> + * 0 if the SEV successfully processed the command
> + * -%ENODEV    if the SEV device is not available
> + * -%ENOTSUPP  if the SEV does not support SEV
> + * -%ETIMEDOUT if the SEV command timed out
> + * -%EIO       if the SEV returned a non-zero return code

Something's weird with those args. I think it should be

	%-ENODEV

and so on...
Kalra, Ashish Oct. 14, 2022, 9:09 p.m. UTC | #2
Hello Boris,

On 10/1/2022 12:33 PM, Borislav Petkov wrote:
> On Mon, Jun 20, 2022 at 11:04:29PM +0000, Ashish Kalra wrote:
>> +static int __sev_snp_init_locked(int *error)
>> +{
>> +	struct psp_device *psp = psp_master;
>> +	struct sev_device *sev;
>> +	int rc = 0;
>> +
>> +	if (!psp || !psp->sev_data)
>> +		return -ENODEV;
>> +
>> +	sev = psp->sev_data;
>> +
>> +	if (sev->snp_inited)
> 
> snp_inited? That's silly.
> 
> 	snp_initialized
> 
> pls.

Yes.

> 
>> +		return 0;
>> +
>> +	/*
>> +	 * The SNP_INIT requires the MSR_VM_HSAVE_PA must be set to 0h
> 
> 	/* Clear MSR_VM_HSAVE_PA on all cores before SNP_INIT */
> 
>> +	 * across all cores.
>> +	 */
>> +	on_each_cpu(snp_set_hsave_pa, NULL, 1);
>> +
>> +	/* Issue the SNP_INIT firmware command. */
> 
> Useless comment.
> 
>> +	rc = __sev_do_cmd_locked(SEV_CMD_SNP_INIT, NULL, error);
>> +	if (rc)
>> +		return rc;
>> +
>> +	/* Prepare for first SNP guest launch after INIT */
>> +	wbinvd_on_all_cpus();
> 
> Can you put a wbinvd() in snp_set_hsave_pa() instead and save yourself
> the second IPI?
> 
> Or is that order of the commands:
> 
> 	1. clear MSR IPI
> 	2. SNP_INIT
> 	3. WBINVD IPI
> 	4. ...
> 
> mandatory?
> 

Yes, we need to do:

wbinvd_on_all_cpus();
SNP_DF_FLUSH

Need to ensure all the caches are clear before launching the first guest 
and this has to be a combination of WBINVD and SNP_DF_FLUSH command.

> ...
> 
>> +static int __sev_snp_shutdown_locked(int *error)
>> +{
>> +	struct sev_device *sev = psp_master->sev_data;
>> +	int ret;
>> +
>> +	if (!sev->snp_inited)
>> +		return 0;
>> +
>> +	/* SHUTDOWN requires the DF_FLUSH */
>> +	wbinvd_on_all_cpus();
>> +	__sev_do_cmd_locked(SEV_CMD_SNP_DF_FLUSH, NULL, NULL);
> 
> Why isn't this retval checked?

 From the SNP FW ABI specs, for the SNP_SHUTDOWN command:

Firmware checks for every encryption capable ASID that the ASID is not 
in use by a guest and a DF_FLUSH is not required. If a DF_FLUSH is 
required, the firmware returns DFFLUSH_REQUIRED.

Considering that SNP_SHUTDOWN command will check if DF_FLUSH was
required and if so, and not invoked before that command, returns
an error indicating that DFFLUSH is required.

This way, we can cleverly avoid taking the error code path for
DF_FLUSH command here and instead let the SNP_SHUTDOWN command
failure below indicate if DF_FLUSH command failed.

This also ensures that we always invoke SNP_SHUTDOWN command,
irrespective of SNP_DF_FLUSH command failure as SNP_DF_FLUSH may
actually not be required by the SHUTDOWN command.

> 
>> +
>> +	ret = __sev_do_cmd_locked(SEV_CMD_SNP_SHUTDOWN, NULL, error);
>> +	if (ret) {
>> +		dev_err(sev->dev, "SEV-SNP firmware shutdown failed\n");
>> +		return ret;
>> +	}
>> +
>> +	sev->snp_inited = false;
>> +	dev_dbg(sev->dev, "SEV-SNP firmware shutdown\n");
>> +
>> +	return ret;
>> +}
> 
> ...
> 
>>   void sev_dev_destroy(struct psp_device *psp)
>> @@ -1287,6 +1385,26 @@ void sev_pci_init(void)
>>   		}
>>   	}
>>   
>> +	/*
>> +	 * If boot CPU supports the SNP, then first attempt to initialize
> 
> s/the SNP/SNP/g
> 
>> +	 * the SNP firmware.
>> +	 */
>> +	if (cpu_feature_enabled(X86_FEATURE_SEV_SNP)) {
>> +		if (!sev_version_greater_or_equal(SNP_MIN_API_MAJOR, SNP_MIN_API_MINOR)) {
>> +			dev_err(sev->dev, "SEV-SNP support requires firmware version >= %d:%d\n",
>> +				SNP_MIN_API_MAJOR, SNP_MIN_API_MINOR);
>> +		} else {
>> +			rc = sev_snp_init(&error);
>> +			if (rc) {
>> +				/*
>> +				 * If we failed to INIT SNP then don't abort the probe.
> 
> Who's "we"?
> 
>> +				 * Continue to initialize the legacy SEV firmware.
>> +				 */
>> +				dev_err(sev->dev, "SEV-SNP: failed to INIT error %#x\n", error);
>> +			}
>> +		}
>> +	}
>> +
>>   	/* Obtain the TMR memory area for SEV-ES use */
>>   	sev_es_tmr = sev_fw_alloc(SEV_ES_TMR_SIZE);
>>   	if (!sev_es_tmr)
> 
> ...
> 
>> diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
>> index 01ba9dc46ca3..ef4d42e8c96e 100644
>> --- a/include/linux/psp-sev.h
>> +++ b/include/linux/psp-sev.h
>> @@ -769,6 +769,20 @@ struct sev_data_snp_init_ex {
>>    */
>>   int sev_platform_init(int *error);
>>   
>> +/**
>> + * sev_snp_init - perform SEV SNP_INIT command
>> + *
>> + * @error: SEV command return code
>> + *
>> + * Returns:
>> + * 0 if the SEV successfully processed the command
>> + * -%ENODEV    if the SEV device is not available
>> + * -%ENOTSUPP  if the SEV does not support SEV
>> + * -%ETIMEDOUT if the SEV command timed out
>> + * -%EIO       if the SEV returned a non-zero return code
> 
> Something's weird with those args. I think it should be
> 
> 	%-ENODEV
> 
> and so on...
> 

Yes, off course %-<errno>

Thanks,
Ashish
Kalra, Ashish Oct. 14, 2022, 9:31 p.m. UTC | #3
Some more follow up regarding avoiding the second IPI:

>>
>>> +    rc = __sev_do_cmd_locked(SEV_CMD_SNP_INIT, NULL, error);
>>> +    if (rc)
>>> +        return rc;
>>> +
>>> +    /* Prepare for first SNP guest launch after INIT */
>>> +    wbinvd_on_all_cpus();
>>
>> Can you put a wbinvd() in snp_set_hsave_pa() instead and save yourself
>> the second IPI?
>>
>> Or is that order of the commands:
>>
>>     1. clear MSR IPI
>>     2. SNP_INIT
>>     3. WBINVD IPI
>>     4. ...
>>
>> mandatory?
>>
> 
> Yes, we need to do:
> 
> wbinvd_on_all_cpus();
> SNP_DF_FLUSH
> 
> Need to ensure all the caches are clear before launching the first guest 
> and this has to be a combination of WBINVD and SNP_DF_FLUSH command.
> 

I had related discussions with the HW architect:

SNP firmware will fail ACTIVATE if DFFLUSH isn't called, and DFFLUSH 
requires the WBINVD on all cores. By requiring WBIDVD on all cores, 
we're a) requiring the caches to be flushed, and b) forcing the 
hypervisor to exit all guests at least once since SEV/SNP has been 
enabled, since the WBINVDs must be done in host mode.

The order is:
VM_HSAVE_PA IPI
SNP_INIT
WBIVND (IPI)
DF_FLUSH

so that means we can't combine the IPIs.

Also, this is not a performance critical path, so should we really be so 
concerned about it?

Thanks,
Ashish
Kalra, Ashish Oct. 19, 2022, 6:48 p.m. UTC | #4
Another follow up:

>>>   int sev_platform_init(int *error);
>>> +/**
>>> + * sev_snp_init - perform SEV SNP_INIT command
>>> + *
>>> + * @error: SEV command return code
>>> + *
>>> + * Returns:
>>> + * 0 if the SEV successfully processed the command
>>> + * -%ENODEV    if the SEV device is not available
>>> + * -%ENOTSUPP  if the SEV does not support SEV
>>> + * -%ETIMEDOUT if the SEV command timed out
>>> + * -%EIO       if the SEV returned a non-zero return code
>>
>> Something's weird with those args. I think it should be
>>
>>     %-ENODEV
>>
>> and so on...
>>
> 
> Yes, off course %-<errno>
> 

I see that other drivers are also using the same convention:

include/linux/regset.h:
..
/**
  * user_regset_set_fn - type of @set function in &struct user_regset
  * @target:     thread being examined
  * @regset:     regset being examined
  * @pos:        offset into the regset data to access, in bytes
  * @count:      amount of data to copy, in bytes
  * @kbuf:       if not %NULL, a kernel-space pointer to copy from
  * @ubuf:       if @kbuf is %NULL, a user-space pointer to copy from
  *
  * Store register values.  Return %0 on success; -%EIO or -%ENODEV
  * are usual failure returns.  The @pos and @count values are in
...

include/linux/psp-tee.h:
..
/**
  * psp_tee_process_cmd() - Process command in Trusted Execution Environment
  * @cmd_id:     TEE command ID (&enum tee_cmd_id)
  * @buf:        Command buffer for TEE processing. On success, is updated
  *              with the response
  * @len:        Length of command buffer in bytes
  * @status:     On success, holds the TEE command execution status
  *
  * This function submits a command to the Trusted OS for processing in the
  * TEE environment and waits for a response or until the command times out.
  *
  * Returns:
  * 0 if TEE successfully processed the command
  * -%ENODEV    if PSP device not available
  * -%EINVAL    if invalid input
  * -%ETIMEDOUT if TEE command timed out
  * -%EBUSY     if PSP device is not responsive
  */
...

Thanks,
Ashish
Jarkko Sakkinen Oct. 23, 2022, 9:17 p.m. UTC | #5
On Wed, Oct 19, 2022 at 01:48:48PM -0500, Kalra, Ashish wrote:
> Another follow up:
> 
> > > >   int sev_platform_init(int *error);
> > > > +/**
> > > > + * sev_snp_init - perform SEV SNP_INIT command
> > > > + *
> > > > + * @error: SEV command return code
> > > > + *
> > > > + * Returns:
> > > > + * 0 if the SEV successfully processed the command
> > > > + * -%ENODEV    if the SEV device is not available
> > > > + * -%ENOTSUPP  if the SEV does not support SEV
> > > > + * -%ETIMEDOUT if the SEV command timed out
> > > > + * -%EIO       if the SEV returned a non-zero return code
> > > 
> > > Something's weird with those args. I think it should be
> > > 
> > >     %-ENODEV
> > > 
> > > and so on...
> > > 
> > 
> > Yes, off course %-<errno>
> > 
> 
> I see that other drivers are also using the same convention:
> 
> include/linux/regset.h:
> ..
> /**
>  * user_regset_set_fn - type of @set function in &struct user_regset
>  * @target:     thread being examined
>  * @regset:     regset being examined
>  * @pos:        offset into the regset data to access, in bytes
>  * @count:      amount of data to copy, in bytes
>  * @kbuf:       if not %NULL, a kernel-space pointer to copy from
>  * @ubuf:       if @kbuf is %NULL, a user-space pointer to copy from
>  *
>  * Store register values.  Return %0 on success; -%EIO or -%ENODEV
>  * are usual failure returns.  The @pos and @count values are in
> ...
> 
> include/linux/psp-tee.h:
> ..
> /**
>  * psp_tee_process_cmd() - Process command in Trusted Execution Environment
>  * @cmd_id:     TEE command ID (&enum tee_cmd_id)
>  * @buf:        Command buffer for TEE processing. On success, is updated
>  *              with the response
>  * @len:        Length of command buffer in bytes
>  * @status:     On success, holds the TEE command execution status
>  *
>  * This function submits a command to the Trusted OS for processing in the
>  * TEE environment and waits for a response or until the command times out.
>  *
>  * Returns:
>  * 0 if TEE successfully processed the command
>  * -%ENODEV    if PSP device not available
>  * -%EINVAL    if invalid input
>  * -%ETIMEDOUT if TEE command timed out
>  * -%EBUSY     if PSP device is not responsive
>  */
> ...
> 
> Thanks,
> Ashis

In SGX driver the convention is:

* Return:
* - -ENODEV  ...
* - -EINVAL  ...

"Return:" because this is what kernel-nanodoc-HOWTO.txt suggests.

The dashes are recognized by Sphinx as list so each return code
gets a newline when processed through "make htmldocs". Otherwise,
you get a mess because newlines will be removed.

I don't actually have idea of '%' and neither see it in the kernel
documenation.

BR, Jarkko
Borislav Petkov Oct. 25, 2022, 8:30 a.m. UTC | #6
On Fri, Oct 14, 2022 at 04:09:11PM -0500, Kalra, Ashish wrote:
> Yes, we need to do:
> 
> wbinvd_on_all_cpus();
> SNP_DF_FLUSH
> 
> Need to ensure all the caches are clear before launching the first guest and
> this has to be a combination of WBINVD and SNP_DF_FLUSH command.

Ok.

> > Why isn't this retval checked?
> 
> From the SNP FW ABI specs, for the SNP_SHUTDOWN command:
> 
> Firmware checks for every encryption capable ASID that the ASID is not in
> use by a guest and a DF_FLUSH is not required. If a DF_FLUSH is required,
> the firmware returns DFFLUSH_REQUIRED.
>
> Considering that SNP_SHUTDOWN command will check if DF_FLUSH was
> required and if so, and not invoked before that command, returns
> an error indicating that DFFLUSH is required.
> 
> This way, we can cleverly avoid taking the error code path for
> DF_FLUSH command here and instead let the SNP_SHUTDOWN command
> failure below indicate if DF_FLUSH command failed.
> 
> This also ensures that we always invoke SNP_SHUTDOWN command,
> irrespective of SNP_DF_FLUSH command failure as SNP_DF_FLUSH may
> actually not be required by the SHUTDOWN command.

This all sounds just silly. The proper way to do this is:

retry:
        ret = __sev_do_cmd_locked(SEV_CMD_SNP_SHUTDOWN, NULL, error);
        if (ret == DFFLUSH_REQUIRED) {
		ret = __sev_do_cmd_locked(SEV_CMD_SNP_DF_FLUSH, NULL, NULL);
		if (ret)
			"... DF_FLUSH failed...";

		goto retry;
	}

I'm assuming here the firmware is smart enough to not keep returning
DFFLUSH_REQUIRED constantly and cause an endless loop.

Thx.
Borislav Petkov Oct. 25, 2022, 8:56 a.m. UTC | #7
On Fri, Oct 14, 2022 at 04:31:42PM -0500, Kalra, Ashish wrote:
> so that means we can't combine the IPIs.

Ok, thanks for checking.

> Also, this is not a performance critical path, so should we really be so
> concerned about it?

Not that concerned but we're always trying to be as optimal as possible.

Thx.
Borislav Petkov Oct. 25, 2022, 9:07 a.m. UTC | #8
On Wed, Oct 19, 2022 at 01:48:48PM -0500, Kalra, Ashish wrote:
> I see that other drivers are also using the same convention:

It is only convention. Look at the .rst output:

0 if the SEV successfully processed the command
-``ENODEV``    if the SEV device is not available
-``ENOTSUPP``  if the SEV does not support SEV
-``ETIMEDOUT`` if the SEV command timed out
-``EIO``       if the SEV returned a non-zero return code

vs

0 if the SEV successfully processed the command
``-ENODEV``    if the SEV device is not available
``-ENOTSUPP``  if the SEV does not support SEV
``-ETIMEDOUT`` if the SEV command timed out
``-EIO``       if the SEV returned a non-zero return code

so in the html output of this, the minus sign will be displayed either
with text font or with monospaced font as part of the error type.

I wanna say the second is better as the '-' is part of the error code
but won't waste too much time debating this. :)

Btw

$ ./scripts/kernel-doc include/linux/psp-sev.h

complains a lot. Might wanna fix those up when bored or someone else
who's reading this and feels bored too. :-)

Thx.
diff mbox series

Patch

diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index 9cb3265f3bef..f1173221d0b9 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -33,6 +33,10 @@ 
 #define SEV_FW_FILE		"amd/sev.fw"
 #define SEV_FW_NAME_SIZE	64
 
+/* Minimum firmware version required for the SEV-SNP support */
+#define SNP_MIN_API_MAJOR	1
+#define SNP_MIN_API_MINOR	51
+
 static DEFINE_MUTEX(sev_cmd_mutex);
 static struct sev_misc_dev *misc_dev;
 
@@ -775,6 +779,98 @@  static int sev_update_firmware(struct device *dev)
 	return ret;
 }
 
+static void snp_set_hsave_pa(void *arg)
+{
+	wrmsrl(MSR_VM_HSAVE_PA, 0);
+}
+
+static int __sev_snp_init_locked(int *error)
+{
+	struct psp_device *psp = psp_master;
+	struct sev_device *sev;
+	int rc = 0;
+
+	if (!psp || !psp->sev_data)
+		return -ENODEV;
+
+	sev = psp->sev_data;
+
+	if (sev->snp_inited)
+		return 0;
+
+	/*
+	 * The SNP_INIT requires the MSR_VM_HSAVE_PA must be set to 0h
+	 * across all cores.
+	 */
+	on_each_cpu(snp_set_hsave_pa, NULL, 1);
+
+	/* Issue the SNP_INIT firmware command. */
+	rc = __sev_do_cmd_locked(SEV_CMD_SNP_INIT, NULL, error);
+	if (rc)
+		return rc;
+
+	/* Prepare for first SNP guest launch after INIT */
+	wbinvd_on_all_cpus();
+	rc = __sev_do_cmd_locked(SEV_CMD_SNP_DF_FLUSH, NULL, error);
+	if (rc)
+		return rc;
+
+	sev->snp_inited = true;
+	dev_dbg(sev->dev, "SEV-SNP firmware initialized\n");
+
+	return rc;
+}
+
+int sev_snp_init(int *error)
+{
+	int rc;
+
+	if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+		return -ENODEV;
+
+	mutex_lock(&sev_cmd_mutex);
+	rc = __sev_snp_init_locked(error);
+	mutex_unlock(&sev_cmd_mutex);
+
+	return rc;
+}
+EXPORT_SYMBOL_GPL(sev_snp_init);
+
+static int __sev_snp_shutdown_locked(int *error)
+{
+	struct sev_device *sev = psp_master->sev_data;
+	int ret;
+
+	if (!sev->snp_inited)
+		return 0;
+
+	/* SHUTDOWN requires the DF_FLUSH */
+	wbinvd_on_all_cpus();
+	__sev_do_cmd_locked(SEV_CMD_SNP_DF_FLUSH, NULL, NULL);
+
+	ret = __sev_do_cmd_locked(SEV_CMD_SNP_SHUTDOWN, NULL, error);
+	if (ret) {
+		dev_err(sev->dev, "SEV-SNP firmware shutdown failed\n");
+		return ret;
+	}
+
+	sev->snp_inited = false;
+	dev_dbg(sev->dev, "SEV-SNP firmware shutdown\n");
+
+	return ret;
+}
+
+static int sev_snp_shutdown(int *error)
+{
+	int rc;
+
+	mutex_lock(&sev_cmd_mutex);
+	rc = __sev_snp_shutdown_locked(NULL);
+	mutex_unlock(&sev_cmd_mutex);
+
+	return rc;
+}
+
 static int sev_ioctl_do_pek_import(struct sev_issue_cmd *argp, bool writable)
 {
 	struct sev_device *sev = psp_master->sev_data;
@@ -1231,6 +1327,8 @@  static void sev_firmware_shutdown(struct sev_device *sev)
 			   get_order(NV_LENGTH));
 		sev_init_ex_buffer = NULL;
 	}
+
+	sev_snp_shutdown(NULL);
 }
 
 void sev_dev_destroy(struct psp_device *psp)
@@ -1287,6 +1385,26 @@  void sev_pci_init(void)
 		}
 	}
 
+	/*
+	 * If boot CPU supports the SNP, then first attempt to initialize
+	 * the SNP firmware.
+	 */
+	if (cpu_feature_enabled(X86_FEATURE_SEV_SNP)) {
+		if (!sev_version_greater_or_equal(SNP_MIN_API_MAJOR, SNP_MIN_API_MINOR)) {
+			dev_err(sev->dev, "SEV-SNP support requires firmware version >= %d:%d\n",
+				SNP_MIN_API_MAJOR, SNP_MIN_API_MINOR);
+		} else {
+			rc = sev_snp_init(&error);
+			if (rc) {
+				/*
+				 * If we failed to INIT SNP then don't abort the probe.
+				 * Continue to initialize the legacy SEV firmware.
+				 */
+				dev_err(sev->dev, "SEV-SNP: failed to INIT error %#x\n", error);
+			}
+		}
+	}
+
 	/* Obtain the TMR memory area for SEV-ES use */
 	sev_es_tmr = sev_fw_alloc(SEV_ES_TMR_SIZE);
 	if (!sev_es_tmr)
@@ -1302,6 +1420,9 @@  void sev_pci_init(void)
 		dev_err(sev->dev, "SEV: failed to INIT error %#x, rc %d\n",
 			error, rc);
 
+	dev_info(sev->dev, "SEV%s API:%d.%d build:%d\n", sev->snp_inited ?
+		"-SNP" : "", sev->api_major, sev->api_minor, sev->build);
+
 	return;
 
 err:
diff --git a/drivers/crypto/ccp/sev-dev.h b/drivers/crypto/ccp/sev-dev.h
index 666c21eb81ab..186ad20cbd24 100644
--- a/drivers/crypto/ccp/sev-dev.h
+++ b/drivers/crypto/ccp/sev-dev.h
@@ -52,6 +52,8 @@  struct sev_device {
 	u8 build;
 
 	void *cmd_buf;
+
+	bool snp_inited;
 };
 
 int sev_dev_init(struct psp_device *psp);
diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
index 01ba9dc46ca3..ef4d42e8c96e 100644
--- a/include/linux/psp-sev.h
+++ b/include/linux/psp-sev.h
@@ -769,6 +769,20 @@  struct sev_data_snp_init_ex {
  */
 int sev_platform_init(int *error);
 
+/**
+ * sev_snp_init - perform SEV SNP_INIT command
+ *
+ * @error: SEV command return code
+ *
+ * Returns:
+ * 0 if the SEV successfully processed the command
+ * -%ENODEV    if the SEV device is not available
+ * -%ENOTSUPP  if the SEV does not support SEV
+ * -%ETIMEDOUT if the SEV command timed out
+ * -%EIO       if the SEV returned a non-zero return code
+ */
+int sev_snp_init(int *error);
+
 /**
  * sev_platform_status - perform SEV PLATFORM_STATUS command
  *
@@ -876,6 +890,8 @@  sev_platform_status(struct sev_user_data_status *status, int *error) { return -E
 
 static inline int sev_platform_init(int *error) { return -ENODEV; }
 
+static inline int sev_snp_init(int *error) { return -ENODEV; }
+
 static inline int
 sev_guest_deactivate(struct sev_data_deactivate *data, int *error) { return -ENODEV; }