Re: [for 4.22 v5 17/18] xen/riscv: add support of page lookup by GFN

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 11/17/25 5:00 PM, Jan Beulich wrote:

> On 17.11.2025 16:52, Oleksii Kurochko wrote:
> > On 11/10/25 5:46 PM, Jan Beulich wrote:
> > > On 20.10.2025 17:58, Oleksii Kurochko wrote:
> > > > +static bool check_outside_boundary(gfn_t gfn, gfn_t boundary, bool is_lower,
> > > > +                                   unsigned int *level_out)
> > > > +{
> > > > +    unsigned int level;
> > > > +
> > > > +    if ( is_lower ? gfn_x(gfn) < gfn_x(boundary)
> > > > +                  : gfn_x(gfn) > gfn_x(boundary) )
> > > > +    {
> > > > +        unsigned long mask = 0;
> > > > +
> > > > +        for ( level = P2M_ROOT_LEVEL; level; level-- )
> > > > +        {
> > > > +            unsigned long masked_gfn;
> > > > +
> > > > +            mask |= PFN_DOWN(P2M_LEVEL_MASK(level));
> > > > +            masked_gfn = gfn_x(gfn) & mask;
> > > > +
> > > > +            if ( is_lower ? masked_gfn < gfn_x(boundary)
> > > > +                          : masked_gfn > gfn_x(boundary) )
> > > > +            {
> > > > +                *level_out = level;
> > > >           
> > > For this to be correct in the is_lower case, don't you need to fill the
> > > bottom bits of masked_gfn with all 1s, rather than with all 0s? Otherwise
> > > the tail of the range may be above boundary.
> > >         
> > I think that I didn't get what you mean by "the range" here and so I can't understand
> > what is "the tail of the range".
> > Could you please clarify?
> >       
> By applying "mask" you effectively produce a range (with "gfn" somewhere in
> the middle). For the level (which you return to the caller) to be correct,
> the entire range must be matching "gfn" in being below or above of the
> boundary. My impression is that this isn't the case when is_lower is true.
>     
Oh, got it. Then I agree that when is_lower is true we really need to fill the bottoms
bits of masked_gfn with all 1s.

Thanks for clarifying.

~ Oleksii