Hi Penny,
On 31/08/2022 03:40, Penny Zheng wrote:
+/*
+ * 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:
+ unprepare_staticmem_pages(page, 1, false);
Looking at assign_domstatic_pages(). It will already call
unprepare_staticmem_pages() in one of the error path. It doesn't look
like the latter can be called twice on a page.
To be honest, I find a bit odd that assign_domstatic_pages() is calling
unprepare_staticmem_pages() because the former doesn't call the
"prepare" function.
AFAICT, this is an issue introduced in this patch. So I would remove the
call from assign_domstatic_pages() and then let the caller calls
unprepare_staticmem_pages() (this would need to be added in
acquire_domstatic_pages()).
Also, I think it would be good to explain why we don't need to scrub.
Something like:
"The page was never accessible by the domain. So scrubbing can be skipped".
Cheers,
--
Julien Grall
