Re: [for 4.22 v5 10/18] xen/riscv: implement p2m_set_range()

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 11/17/25 9:56 AM, Jan Beulich wrote:

> > > > +#define P2M_MAX_ROOT_LEVEL 4
> > > > +
> > > > +#define P2M_DECLARE_OFFSETS(var, addr) \
> > > > +    unsigned int var[P2M_MAX_ROOT_LEVEL] = {-1};\
> > > > +    for ( unsigned int i = 0; i <= gstage_root_level; i++ ) \
> > > > +        var[i] = calc_offset(i, addr);
> > > This surely is more than just "declare", and it's dealing with all levels no
> > > matter whether you actually will use all offsets.
> > I will rename|P2M_DECLARE_OFFSETS| to|P2M_BUILD_LEVEL_OFFSETS()|.
> >
> > But how can I know which offset I will actually need to use?
> > If we take the following loop as an example:
> >    |for( level = P2M_ROOT_LEVEL; level > target; level-- ) { ||/* ||* Don't try to allocate intermediate page tables if the mapping ||* is about to be removed. ||*/ ||rc = p2m_next_level(p2m, !removing_mapping, ||level, &table, offsets[level]); ||... ||} |It walks from|P2M_ROOT_LEVEL| down to|target|, where|target| is determined at runtime.
> >
> > If you mean that, for example, when the G-stage mode is Sv39, there is no need to allocate
> > an array with 4 entries (or 5 entries if we consider Sv57, so P2M_MAX_ROOT_LEVEL should be
> > updated), because Sv39 only uses 3 page table levels — then yes, in theory it could be
> > smaller. But I don't think it is a real issue if the|offsets[]| array on the stack has a
> > few extra unused entries.
> >
> > If preferred, Icould allocate the array dynamically based on|gstage_root_level|.
> > Would that be better?
> Having a few unused entries isn't a big deal imo. What I'm not happy with here is
> that you may _initialize_ more entries than actually needed. I have no good
> suggestion within the conceptual framework you use for page walking (the same
> issue iirc exists in host page table walks, just that the calculations there are
> cheaper).
>     
The loop inside P2M_DECLARE_OFFSETS() uses gstage_root_level, so only the entries that
are actually going to be used are initialized.

You probably mean that it’s possible we don’t need to walk all the tables because a
leaf page-table entry appears earlier than the L0 page table for some gfns? IMO, it’s not
really a big deal, because at worst we just spend some time calculating something that
isn’t actually needed, but considering that it will be just extra 2 calls in the worst case
(when mapping is 1g, for no reason we calculated offsets for L1 and L0) of calc_offset()
it won't affect performance too much.

~ Oleksii