Re: [PATCH RFC v8 23/56] crypto: ccp: Introduce snp leaked pages list

[Vlastimil Babka]

On 2/20/23 19:38, Michael Roth wrote:
> From: Ashish Kalra <ashish.kalra@xxxxxxx>
> 
> Pages are unsafe to be released back to the page-allocator, if they
> have been transitioned to firmware/guest state and can't be reclaimed
> or transitioned back to hypervisor/shared state. In this case add
> them to an internal leaked pages list to ensure that they are not freed
> or touched/accessed to cause fatal page faults.
> 
> Signed-off-by: Ashish Kalra <ashish.kalra@xxxxxxx>
> Signed-off-by: Michael Roth <michael.roth@xxxxxxx>
> ---
>  drivers/crypto/ccp/sev-dev.c | 28 ++++++++++++++++++++++++++++
>  include/linux/psp-sev.h      |  8 ++++++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index 35f605936f1b..eca4e59b0f44 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -42,6 +42,12 @@
>  static DEFINE_MUTEX(sev_cmd_mutex);
>  static struct sev_misc_dev *misc_dev;
>  
> +/* list of pages which are leaked and cannot be reclaimed */
> +static LIST_HEAD(snp_leaked_pages_list);
> +static DEFINE_SPINLOCK(snp_leaked_pages_list_lock);
> +
> +static atomic_long_t snp_nr_leaked_pages = ATOMIC_LONG_INIT(0);
> +
>  static int psp_cmd_timeout = 100;
>  module_param(psp_cmd_timeout, int, 0644);
>  MODULE_PARM_DESC(psp_cmd_timeout, " default timeout value, in seconds, for PSP commands");
> @@ -188,6 +194,28 @@ static int sev_cmd_buffer_len(int cmd)
>  	return 0;
>  }
>  
> +void snp_mark_pages_offline(unsigned long pfn, unsigned int npages)

Why call it offline which has usually a memory hotplug-related meaning? What
about e.g. snp_leak_bad_pages() ?

> +{
> +	struct page *page = pfn_to_page(pfn);
> +
> +	WARN(1, "psc failed, pfn 0x%lx pages %d (marked offline)\n", pfn, npages);
> +
> +	spin_lock(&snp_leaked_pages_list_lock);
> +	while (npages--) {
> +		/*
> +		 * Reuse the page's buddy list for chaining into the leaked
> +		 * pages list. This page should not be on a free list currently
> +		 * and is also unsafe to be added to a free list.
> +		 */
> +		list_add_tail(&page->buddy_list, &snp_leaked_pages_list);
> +		sev_dump_rmpentry(pfn);
> +		pfn++;
> +	}
> +	spin_unlock(&snp_leaked_pages_list_lock);
> +	atomic_long_inc(&snp_nr_leaked_pages);
> +}
> +EXPORT_SYMBOL_GPL(snp_mark_pages_offline);
> +
>  static void *sev_fw_alloc(unsigned long len)
>  {
>  	struct page *page;
> diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
> index 46f61e3ae33b..8edf5c548fbf 100644
> --- a/include/linux/psp-sev.h
> +++ b/include/linux/psp-sev.h
> @@ -923,6 +923,12 @@ int sev_do_cmd(int cmd, void *data, int *psp_ret);
>  
>  void *psp_copy_user_blob(u64 uaddr, u32 len);
>  
> +/**
> + * sev_mark_pages_offline - insert non-reclaimed firmware/guest pages
> + * into a leaked pages list.
> + */
> +void snp_mark_pages_offline(unsigned long pfn, unsigned int npages);
> +
>  #else	/* !CONFIG_CRYPTO_DEV_SP_PSP */
>  
>  static inline int
> @@ -951,6 +957,8 @@ sev_issue_cmd_external_user(struct file *filep, unsigned int id, void *data, int
>  
>  static inline void *psp_copy_user_blob(u64 __user uaddr, u32 len) { return ERR_PTR(-EINVAL); }
>  
> +void snp_mark_pages_offline(unsigned long pfn, unsigned int npages) {}
> +
>  #endif	/* CONFIG_CRYPTO_DEV_SP_PSP */
>  
>  #endif	/* __PSP_SEV_H__ */