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

[Juergen Gross <jgross@xxxxxxxx>]

On 13.12.21 22:22, Samuel Thibault wrote:
> 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;
Hmm, or I can rename max to start, drop the "max += pfns;" and do a
"return start + pfns;" at the end of the function.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature