Re: [PATCH v9 8/8] xen: retrieve reserved pages on populate_physmap

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.07.2022 07:46, Penny Zheng wrote:
> When a static domain populates memory through populate_physmap at runtime,
> it shall retrieve reserved pages from resv_page_list to make sure that
> guest RAM is still restricted in statically configured memory regions.
> This commit also introduces a new helper acquire_reserved_page to make it 
> work.
> 
> Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>
> ---
> v9 changes:
> - Use ASSERT_ALLOC_CONTEXT() in acquire_reserved_page
> - Add free_staticmem_pages to undo prepare_staticmem_pages when
> assign_domstatic_pages fails

May I suggest to re-consider naming of the various functions? Undoing
what "prepare" did by "free" is, well, counterintuitive.

> +/*
> + * Acquire a page from reserved page list(resv_page_list), when populating
> + * memory for static domain on runtime.
> + */
> +mfn_t acquire_reserved_page(struct domain *d, unsigned int memflags)
> +{
> +    struct page_info *page;
> +
> +    ASSERT_ALLOC_CONTEXT();
> +
> +    /* Acquire a page from reserved page list(resv_page_list). */
> +    spin_lock(&d->page_alloc_lock);
> +    page = page_list_remove_head(&d->resv_page_list);
> +    spin_unlock(&d->page_alloc_lock);
> +    if ( unlikely(!page) )
> +        return INVALID_MFN;
> +
> +    if ( !prepare_staticmem_pages(page, 1, memflags) )
> +        goto fail;
> +
> +    if ( assign_domstatic_pages(d, page, 1, memflags) )
> +        goto fail_assign;
> +
> +    return page_to_mfn(page);
> +
> + fail_assign:
> +    free_staticmem_pages(page, 1, memflags & MEMF_no_scrub);
> + fail:
> +    page_list_add_tail(page, &d->resv_page_list);
> +    return INVALID_MFN;
> +}

What about locking on this error path?

Jan