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

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

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);
...
(Note: I missed to return a value returned by p2m_alloc_root_table() in p2m_set_allocation()
so it isn't really propagated, at the moment, but I will fix that in the next patch
version) ...

> > Probably, you meant that it is needed to update p2m->pages?
> >       
> That (I think) I commented on elsewhere, yes.
>     
    ... 
if it is needed really to update p2m->pages when a page is allocated, I think
it will be better to in p2m_allocate_root() immediately after alloc_domheap_pages()
is called in p2m_allocate_root().

~ Oleksii