On 8/20/25 17:19, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@xxxxxxx>
>
> When SEV-SNP is active, the TEE extended command header page and
> all output buffers for TEE extended commands (such as used by Seamless
> Firmware servicing support) must be in hypervisor-fixed state,
> assigned to the hypervisor and marked immutable in the RMP entrie(s).
>
> Add a new generic SEV API interface to allocate/free hypervisor fixed
> pages which abstracts hypervisor fixed page allocation/free for PSP
> sub devices. The API internally uses SNP_INIT_EX to transition pages
> to HV-Fixed page state.
>
> If SNP is not enabled then the allocator is simply a wrapper over
> alloc_pages() and __free_pages().
>
> When the sub device free the pages, they are put on a free list
> and future allocation requests will try to re-use the freed pages from
> this list. But this list is not preserved across PSP driver load/unload
> hence this free/reuse support is only supported while PSP driver is
> loaded. As HV_FIXED page state is only changed at reboot, these pages
> are leaked as they cannot be returned back to the page allocator and
> then potentially allocated to guests, which will cause SEV-SNP guests
> to fail to start or terminate when accessing the HV_FIXED page.
>
> Suggested-by: Thomas Lendacky <Thomas.Lendacky@xxxxxxx>
> Signed-off-by: Ashish Kalra <ashish.kalra@xxxxxxx>
> ---
> drivers/crypto/ccp/sev-dev.c | 182 +++++++++++++++++++++++++++++++++++
> drivers/crypto/ccp/sev-dev.h | 3 +
> 2 files changed, 185 insertions(+)
>
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index 4f000dc2e639..1560009c2f18 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -82,6 +82,21 @@ MODULE_FIRMWARE("amd/amd_sev_fam19h_model1xh.sbin"); /* 4th gen EPYC */
> static bool psp_dead;
> static int psp_timeout;
>
> +enum snp_hv_fixed_pages_state {
> + ALLOCATED,
> + HV_FIXED,
> +};
> +
> +struct snp_hv_fixed_pages_entry {
> + struct list_head list;
> + struct page *page;
> + unsigned int order;
> + bool free;
> + enum snp_hv_fixed_pages_state page_state;
> +};
> +
> +static LIST_HEAD(snp_hv_fixed_pages);
> +
> /* Trusted Memory Region (TMR):
> * The TMR is a 1MB area that must be 1MB aligned. Use the page allocator
> * to allocate the memory, which will return aligned memory for the specified
> @@ -1157,6 +1172,165 @@ static int snp_get_platform_data(struct sev_device *sev, int *error)
> return rc;
> }
>
> +/* Hypervisor Fixed pages API interface */
> +static void snp_hv_fixed_pages_state_update(struct sev_device *sev,
> + enum snp_hv_fixed_pages_state page_state)
> +{
> + struct snp_hv_fixed_pages_entry *entry;
> +
> + /* List is protected by sev_cmd_mutex */
> + lockdep_assert_held(&sev_cmd_mutex);
> +
> + if (list_empty(&snp_hv_fixed_pages))
> + return;
> +
> + list_for_each_entry(entry, &snp_hv_fixed_pages, list)
> + entry->page_state = page_state;
> +}
> +
> +/*
> + * Allocate HV_FIXED pages in 2MB aligned sizes to ensure the whole
> + * 2MB pages are marked as HV_FIXED.
> + */
> +struct page *snp_alloc_hv_fixed_pages(unsigned int num_2mb_pages)
> +{
> + struct psp_device *psp_master = psp_get_master_device();
> + struct snp_hv_fixed_pages_entry *entry;
> + struct sev_device *sev;
> + unsigned int order;
> + struct page *page;
> +
> + if (!psp_master || !psp_master->sev_data)
> + return NULL;
> +
> + sev = psp_master->sev_data;
> +
> + /*
> + * This API uses SNP_INIT_EX to transition allocated pages to HV_Fixed
> + * page state, fail if SNP is already initialized.
> + */
> + if (sev->snp_initialized)
> + return NULL;
> +
> + order = get_order(PMD_SIZE * num_2mb_pages);
> +
> + /*
> + * SNP_INIT_EX is protected by sev_cmd_mutex, therefore this list
> + * also needs to be protected using the same mutex.
> + */
> + guard(mutex)(&sev_cmd_mutex);
Does the guard() need to come before the snp_initialized check (or the
snp_initialized check after taking the mutex)? An SNP_INIT_EX (which
would grab the mutex and make this call wait) can occur between the
check and obtaining the mutex.
Thanks,
Tom
> +
