Re: [PATCH V4 10/10] xen/arm: introduce allocate_static_memory

[Julien Grall <julien@xxxxxxx>]

On 16/08/2021 08:51, Penny Zheng wrote:
> > > +               d, bank, pbase, pbase + psize);
> > > +
> > > +        /*
> > > +         * It shall be mapped to the fixed guest RAM address rambase[i],
> > > +         * And until it exhausts the ramsize[i], it will seek to the next
> > > +         * rambase[i+1].
> > > +         */
> > > +        while ( 1 )
> > > +        {
> > > +            /*
> > > +             * The current physical bank is fully mapped.
> > > +             * Handle the next physical bank.
> > > +             */
> > > +            if ( gsize >= psize )
> > > +            {
> > > +                if ( !append_static_memory_to_bank(d, &kinfo->mem.bank[gbank],
> > > +                                                   smfn, psize) )
> > > +                    goto fail;
> > > +
> > > +                gsize = gsize - psize;
> > > +                bank++;
> > > +                break;
> > > +            }
> > > +            /*
> > > +             * Current guest bank memory is not enough to map.
> > > +             * Check if we have another guest bank available.
> > > +             * gbank refers guest memory bank index.
> > > +             */
> > > +            else if ( (gbank + 2) > GUEST_RAM_BANKS ) {
> > I don't understand the +2. Can you clarify it?
> gbank refers to the index of the guest bank, and here since current guest 
> bank(gbank)
>   memory is not enough to map, users seeks to the next one(gbank + 1),
>
> gbank + 2 is the number of requested guest memory banks right now, and shall 
> not be
> larger than GUEST_RAM_BANKS.
Thanks for the clarification. When I read "2" I tend to think we are 
checking the bank after the next. How about writing:
(gbank + 1) >= GUEST_RAM_BANKS

or

gbank >= (GUEST_RAM_BANKS - 1)

This as the same end results, but we check the index rather than the 
number of banks.
Anyway, I can settle with 2 if you really prefer it.

Cheers,

--
Julien Grall