Re: [RFC PATCH v2 09/22] crypto/ccp: Implement SEV TIO firmware interface

[Francesco Lavra <francescolavra.fl@gmail.com>]

On 2025-02-18 at 11:09, Alexey Kardashevskiy wrote:
> diff --git a/drivers/crypto/ccp/sev-dev-tio.c
> b/drivers/crypto/ccp/sev-dev-tio.c
> new file mode 100644
> index 000000000000..bd55ad6c5fb3
> --- /dev/null
> +++ b/drivers/crypto/ccp/sev-dev-tio.c
> @@ -0,0 +1,1664 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +// Interface to PSP for CCP/SEV-TIO/SNP-VM
> +
> +#include <linux/pci.h>
> +#include <linux/tsm.h>
> +#include <linux/psp.h>
> +#include <linux/file.h>
> +#include <linux/vmalloc.h>
> +
> +#include <asm/sev-common.h>
> +#include <asm/sev.h>
> +#include <asm/page.h>
> +
> +#include "psp-dev.h"
> +#include "sev-dev.h"
> +#include "sev-dev-tio.h"
> +
> +static void *__prep_data_pg(struct tsm_dev_tio *dev_data, size_t
> len)
> +{
> +	void *r = dev_data->data_pg;
> +
> +	if (snp_reclaim_pages(virt_to_phys(r), 1, false))
> +		return NULL;
> +
> +	memset(r, 0, len);
> +
> +	if (rmp_make_private(page_to_pfn(virt_to_page(r)), 0,
> PG_LEVEL_4K, 0, true))

We have virt_to_pfn().

> +static struct sla_addr_t sla_alloc(size_t len, bool firmware_state)
> +{
> +	unsigned long i, npages = PAGE_ALIGN(len) >> PAGE_SHIFT;
> +	struct sla_addr_t *scatter = NULL;
> +	struct sla_addr_t ret = SLA_NULL;
> +	struct sla_buffer_hdr *buf;
> +	struct page *pg;
> +
> +	if (npages == 0)
> +		return ret;
> +
> +	if (WARN_ON_ONCE(npages > ((PAGE_SIZE / sizeof(struct
> sla_addr_t)) + 1)))

This should be (npages + 1 > (...)), because we need to fit `npages`
SLAs plus the final SLA_EOL.

> +/* Expands a buffer, only firmware owned buffers allowed for now */
> +static int sla_expand(struct sla_addr_t *sla, size_t *len)
> +{
> +	struct sla_buffer_hdr *oldbuf = sla_buffer_map(*sla),
> *newbuf;
> +	struct sla_addr_t oldsla = *sla, newsla;
> +	size_t oldlen = *len, newlen;
> +
> +	if (!oldbuf)
> +		return -EFAULT;
> +
> +	newlen = oldbuf->capacity_sz;
> +	if (oldbuf->capacity_sz == oldlen) {
> +		/* This buffer does not require expansion, must be
> another buffer */
> +		sla_buffer_unmap(oldsla, oldbuf);
> +		return 1;
> +	}
> +
> +	pr_notice("Expanding BUFFER from %ld to %ld bytes\n",
> oldlen, newlen);
> +
> +	newsla = sla_alloc(newlen, true);
> +	if (IS_SLA_NULL(newsla))
> +		return -ENOMEM;
> +
> +	newbuf = sla_buffer_map(newsla);
> +	if (!newbuf) {
> +		sla_free(newsla, newlen, true);
> +		return -EFAULT;
> +	}
> +
> +	memcpy(newbuf, oldbuf, oldlen);
> +
> +	sla_buffer_unmap(newsla, newbuf);
> +	sla_free(oldsla, oldlen, true);
> +	*sla = newsla;
> +	*len = newlen;
> +
> +	return 0;

Return values are inconsistent with how this function is used in
sev_tio_do_cmd(): a zero value should indicate that expansion is not
required.