Re: [PATCH v3 06/20] xen/riscv: add root page table allocation

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.08.2025 17:59, Oleksii Kurochko wrote:
> On 8/7/25 5:30 PM, Jan Beulich wrote:
>> On 07.08.2025 14:00, Oleksii Kurochko wrote:
>>> On 8/5/25 12:37 PM, Jan Beulich wrote:
>>>> On 31.07.2025 17:58, Oleksii Kurochko wrote:
>>>>> +    /*
>>>>> +     * Return back nr_root_pages to assure the root table memory is also
>>>>> +     * accounted against the P2M pool of the domain.
>>>>> +     */
>>>>> +    if ( !paging_ret_pages_to_domheap(d, nr_root_pages) )
>>>>> +        return -ENOMEM;
>>>>> +
>>>>> +    page = p2m_allocate_root(d);
>>>>> +    if ( !page )
>>>>> +        return -ENOMEM;
>>>> Hmm, and the pool is then left shrunk by 4 pages?
>>> Yes until they are used for root table it shouldn't be in p2m pool 
>>> (freelist),
>>> when root table will be freed then it makes sense to return them back.
>>> Am I missing something?
>> I'm commenting specifically on the error path here.
> 
> Ohh, got it.
> 
> In this case, should we really care about this 4 pages as a domain can't be 
> ran
> without allocated page root table and a panic() will be occured anyway 
> according
> to the create_domUs() common code (construct_domU() -> 
> domain_p2m_set_allocation()
> -> p2m_set_allocation() -> p2m_alloc_root_table()):
> ...
>          rc = construct_domU(&ki, node);
>          if ( rc )
>              panic("Could not set up domain %s (rc = %d)\n",
>                    dt_node_name(node), rc);

Well, that's for dom0less. Even for tool-stack created VMs there would be
no problem. But root tables required on demand (altp2m, nested) would be
different. So what you do here may be good enough for now, but likely will
want improving later on. (Such temporary restrictions may want putting
down somewhere.)

Jan