Re: [PATCH v3 8/9] xen/riscv: page table handling

[oleksii.kurochko@xxxxxxxxx]

On Thu, 2024-08-01 at 12:43 +0200, Jan Beulich wrote:
> 
> 
> > > > +    mfn = pte_get_mfn(*entry);
> > > > +
> > > > +    xen_unmap_table(*table);
> > > > +    *table = xen_map_table(mfn);
> > > > +
> > > > +    return XEN_TABLE_NORMAL_PAGE;
> > > > +}
> > > 
> > > Normal page? Not normal table?
> > It just mean that this points not to super_page so potenially the
> > in
> > the next one table will have an entry that would be normal page.
> 
> Or a normal page table, if you haven't reached leaf level yet. IOW
> maybe
> better XEN_TABLE_NORMAL, covering both cases?
It would be better. Thanks.

> 
> > > > +int map_pages_to_xen(unsigned long virt,
> > > > +                     mfn_t mfn,
> > > > +                     unsigned long nr_mfns,
> > > > +                     unsigned int flags)
> > > > +{
> > > > +    return xen_pt_update(virt, mfn, nr_mfns, flags);
> > > > +}
> > > 
> > > Why this wrapping of two functions taking identical arguments?
> > map_pages_to_xen() sounds more clear regarding the way how it
> > should be
> > used.
> > 
> > xen_pt_update() will be also used inside other functions. Look at
> > the
> > example above.
> 
> They could as well use map_pages_to_xen() then? Or else the wrapper
> may
> want to check (assert) that it is _not_ called with one of the
> special
> case arguments that xen_pt_update() knows how to deal with?
Yes, map_pages_to_xen() will be used in other functions/wrappers.
At the momemnt, I don't see what should be checked additionally in
map_pages_to_xen(). It seems to me that xen_pt_update() covers all the
checks at the start it needs for now. ( i will double-check that ).

~ Oleksii