Re: [PATCH 03/10] mini-os: don't assume contiguous RAM when initializing in PVH mode

[Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>]

Juergen Gross, le lun. 13 déc. 2021 15:58:58 +0100, a ecrit:
> On 12.12.21 01:15, Samuel Thibault wrote:
> > Juergen Gross, le lun. 06 déc. 2021 08:23:30 +0100, a ecrit:
> > > -    unsigned long pfn, max = 0;
> > > +    unsigned long pfns, max = 0;
> > 
> > I'd say rather rename max to start.
> > 
> > >       e820_get_memmap();
> > > @@ -166,9 +166,12 @@ unsigned long e820_get_maxpfn(void)
> > >       {
> > >           if ( e820_map[i].type != E820_RAM )
> > >               continue;
> > > -        pfn = (e820_map[i].addr + e820_map[i].size) >> PAGE_SHIFT;
> > > -        if ( pfn > max )
> > > -            max = pfn;
> > > +        pfns = e820_map[i].size >> PAGE_SHIFT;
> > > +        max = e820_map[i].addr >> PAGE_SHIFT;
> > 
> > since it's it's always the start of the e820 entry.
> > 
> > > +        if ( pages <= pfns )
> > > +            return max + pages;
> > > +        pages -= pfns;
> > > +        max += pfns;
> > 
> > Here we don't need do change max, only pages.
> 
> It is needed in case the loop is finished.
> 
> And this was the reason for naming it max.

Ah, ok.

At first read the name was confusing me. Perhaps better use two
variables then: start and max, so that we have

start = e820_map[i].addr >> PAGE_SHIFT;
if ( pages <= pfns )
    return start + pages;
pages -= pfns;
max = start + pfns;

Samuel