On 9/20/25 12:14 AM, Jan Beulich wrote:
On 17.09.2025 23:55, Oleksii Kurochko wrote:+{ + return MASK_INSR(mfn_x(page_to_mfn(p2m->root)), HGATP_PPN) | + MASK_INSR(gstage_mode, HGATP_MODE_MASK) | + MASK_INSR(vmid, HGATP_VMID_MASK); +} + +static int p2m_alloc_root_table(struct p2m_domain *p2m) +{ + struct domain *d = p2m->domain; + struct page_info *page; + int rc; + + /* + * Return back P2M_ROOT_PAGES to assure the root table memory is also + * accounted against the P2M pool of the domain. + */ + if ( (rc = paging_ret_pages_to_domheap(d, P2M_ROOT_PAGES)) ) + return rc;I read the "ret" in the name as "return" here. However, ...+ /* + * As mentioned in the Priviliged Architecture Spec (version 20240411) + * in Section 18.5.1, for the paged virtual-memory schemes (Sv32x4, + * Sv39x4, Sv48x4, and Sv57x4), the root page table is 16 KiB and must + * be aligned to a 16-KiB boundary. + */ + page = alloc_domheap_pages(d, P2M_ROOT_ORDER, MEMF_no_owner); + if ( !page ) + { + /* + * If allocation of root table pages fails, the pages acquired above + * must be returned to the freelist to maintain proper freelist + * balance. + */ + paging_ret_pages_to_freelist(d, P2M_ROOT_PAGES);... "return" doesn't make sense here, so I wonder what the "ret" here means.
In both cases,|ret| was supposed to mean/"return"/, since in both cases we “return” memory. I agree that in the case of|paging_ret_pages_to_freelist()|, the flow is slightly different: a page is allocated from the domheap and then added back to the freelist. That looks more like/adding/ than/returning/. Still, I felt that “return” could also apply here, as the page is being given back. For more clarity, do you think it would make sense to rename |paging_ret_pages_to_freelist()| to|paging_add_page_to_freelist()|? ~ Oleksii
