Re: [PATCH 1/2] crypto: ccp - simplify sev_update_firmware()

[Tom Lendacky]

On 3/2/26 09:02, Tycho Andersen wrote:
> From: "Tycho Andersen (AMD)" <tycho@xxxxxxxxxx>
> 
> sev_do_cmd() has its own command buffer (sev->cmd_buf) with the correct
> alignment, perms, etc. that it copies the command into, so prepending it to
> the firmware data is unnecessary.
> 
> Switch sev_update_firmware() to using a stack allocated command in light of
> this copy, and drop all of the resulting pointer math.
> 
> Signed-off-by: Tycho Andersen (AMD) <tycho@xxxxxxxxxx>

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

> ---
>  drivers/crypto/ccp/sev-dev.c | 27 +++++++++------------------
>  1 file changed, 9 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index 096f993974d1..c45c74190c75 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -1967,11 +1967,11 @@ static int sev_get_firmware(struct device *dev,
>  /* Don't fail if SEV FW couldn't be updated. Continue with existing SEV FW */
>  static int sev_update_firmware(struct device *dev)
>  {
> -	struct sev_data_download_firmware *data;
> +	struct sev_data_download_firmware data;
>  	const struct firmware *firmware;
>  	int ret, error, order;
>  	struct page *p;
> -	u64 data_size;
> +	void *fw_blob;
>  
>  	if (!sev_version_greater_or_equal(0, 15)) {
>  		dev_dbg(dev, "DOWNLOAD_FIRMWARE not supported\n");
> @@ -1983,16 +1983,7 @@ static int sev_update_firmware(struct device *dev)
>  		return -1;
>  	}
>  
> -	/*
> -	 * SEV FW expects the physical address given to it to be 32
> -	 * byte aligned. Memory allocated has structure placed at the
> -	 * beginning followed by the firmware being passed to the SEV
> -	 * FW. Allocate enough memory for data structure + alignment
> -	 * padding + SEV FW.
> -	 */
> -	data_size = ALIGN(sizeof(struct sev_data_download_firmware), 32);
> -
> -	order = get_order(firmware->size + data_size);
> +	order = get_order(firmware->size);
>  	p = alloc_pages(GFP_KERNEL, order);
>  	if (!p) {
>  		ret = -1;
> @@ -2003,20 +1994,20 @@ static int sev_update_firmware(struct device *dev)
>  	 * Copy firmware data to a kernel allocated contiguous
>  	 * memory region.
>  	 */
> -	data = page_address(p);
> -	memcpy(page_address(p) + data_size, firmware->data, firmware->size);
> +	fw_blob = page_address(p);
> +	memcpy(fw_blob, firmware->data, firmware->size);
>  
> -	data->address = __psp_pa(page_address(p) + data_size);
> -	data->len = firmware->size;
> +	data.address = __psp_pa(fw_blob);
> +	data.len = firmware->size;
>  
> -	ret = sev_do_cmd(SEV_CMD_DOWNLOAD_FIRMWARE, data, &error);
> +	ret = sev_do_cmd(SEV_CMD_DOWNLOAD_FIRMWARE, &data, &error);
>  
>  	/*
>  	 * A quirk for fixing the committed TCB version, when upgrading from
>  	 * earlier firmware version than 1.50.
>  	 */
>  	if (!ret && !sev_version_greater_or_equal(1, 50))
> -		ret = sev_do_cmd(SEV_CMD_DOWNLOAD_FIRMWARE, data, &error);
> +		ret = sev_do_cmd(SEV_CMD_DOWNLOAD_FIRMWARE, &data, &error);
>  
>  	if (ret)
>  		dev_dbg(dev, "Failed to update SEV firmware: %#x\n", error);
> 
> base-commit: 11439c4635edd669ae435eec308f4ab8a0804808