On 24.09.2025 17:40, Oleksii Kurochko wrote:
>
> 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()|?
In place of "add" I'd perhaps use "refill" and then really refill_from_domheap
to properly indicate the opposite of ret_to_domheap. (For both of these: If
already you want to use such long-ish names.)
Jan
