Re: [PATCH 1/4] crypto: ccp - Shut down SEV firmware before copying to user

[Tom Lendacky <thomas.lendacky@amd.com>]

On 10/14/26 08:20, Michael Roth wrote:
> From: Ackerley Tng <ackerleytng@google.com>
> 
> When servicing PEK CSR and PDH certificate export ioctl commands while
> the SEV platform is in the uninitialized state, the driver temporarily
> transitions the firmware to the initialized state, executes the
> firmware command, and shuts the firmware back down.
> 
> Currently, the firmware shutdown is performed in the cleanup path
> after copying the command results back to userspace. To allow
> releasing the global SEV command mutex before copying results to
> userspace in a subsequent change, the temporary firmware state
> transition and shutdown must remain grouped with the firmware command
> execution rather than spanning across userspace memory copies.
> 
> Move the conditional firmware shutdown immediately after executing the
> firmware command, before copying the output buffers to userspace.
> 
> No functional change intended.
> 
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> Signed-off-by: Michael Roth <michael.roth@amd.com>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>  drivers/crypto/ccp/sev-dev.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index f833cb7e4da3..7de3b6bf71e1 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -2022,6 +2022,9 @@ static int sev_ioctl_do_pek_csr(struct sev_issue_cmd *argp, bool writable)
>  
>  	ret = __sev_do_cmd_locked(SEV_CMD_PEK_CSR, &data, &argp->error);
>  
> +	if (shutdown_required)
> +		__sev_firmware_shutdown(sev, false);
> +
>  	/*
>  	 * Firmware will returns the length of the CSR blob (either the minimum
>  	 * required length or the actual length written), return it to the user.
> @@ -2042,9 +2045,6 @@ static int sev_ioctl_do_pek_csr(struct sev_issue_cmd *argp, bool writable)
>  	}
>  
>  e_free_blob:
> -	if (shutdown_required)
> -		__sev_firmware_shutdown(sev, false);
> -
>  	kfree(blob);
>  	return ret;
>  }
> @@ -2505,6 +2505,9 @@ static int sev_ioctl_do_pdh_export(struct sev_issue_cmd *argp, bool writable)
>  
>  	ret = __sev_do_cmd_locked(SEV_CMD_PDH_CERT_EXPORT, &data, &argp->error);
>  
> +	if (shutdown_required)
> +		__sev_firmware_shutdown(sev, false);
> +
>  	/*
>  	 * Firmware will return the length of the blobs (either the minimum
>  	 * required length or the actual length written), return 'em to the user.
> @@ -2535,9 +2538,6 @@ static int sev_ioctl_do_pdh_export(struct sev_issue_cmd *argp, bool writable)
>  	}
>  
>  e_free_cert:
> -	if (shutdown_required)
> -		__sev_firmware_shutdown(sev, false);
> -
>  	kfree(cert_blob);
>  e_free_pdh:
>  	kfree(pdh_blob);