Hi Stefano,
> -----Original Message-----
> From: Stefano Stabellini <sstabell...@kernel.org>
> > + paddr_t reserved_heap_start = ~0, reserved_heap_end = 0,
>
> INVALID_PADDR or ~0ULL
Ack.
>
> > /*
> > * If the user has not requested otherwise via the command line
> > * then locate the xenheap using these constraints:
> > @@ -743,7 +766,8 @@ static void __init setup_mm(void)
> > * We try to allocate the largest xenheap possible within these
> > * constraints.
> > */
> > - heap_pages = ram_pages;
> > + heap_pages = !reserved_heap ? ram_pages : reserved_heap_pages;
> > +
> > if ( opt_xenheap_megabytes )
> > xenheap_pages = opt_xenheap_megabytes << (20-PAGE_SHIFT);
> > else
> > @@ -755,17 +779,21 @@ static void __init setup_mm(void)
> >
> > do
> > {
> > - e = consider_modules(ram_start, ram_end,
> > + e = !reserved_heap ?
> > + consider_modules(ram_start, ram_end,
> > pfn_to_paddr(xenheap_pages),
> > - 32<<20, 0);
> > + 32<<20, 0) :
> > + reserved_heap_end;
> > +
> > if ( e )
> > break;
> >
> > xenheap_pages >>= 1;
> > } while ( !opt_xenheap_megabytes && xenheap_pages > 32<<(20-
> PAGE_SHIFT) );
> >
> > - if ( ! e )
> > - panic("Not not enough space for xenheap\n");
> > + if ( ! e ||
> > + ( reserved_heap && reserved_heap_pages < 32<<(20-PAGE_SHIFT) ) )
> > + panic("Not enough space for xenheap\n");
>
>
> I would skip the do/while loop completely if reserved_heap. We don't
> need it anyway
I agree with this.
> and we can automatically calculate xenheap_pages in a single line.
Here I am a little bit confused. Sorry to ask but could you please explain
a little bit more about why we can calculate the xenheap_pages in a single
line? Below is the code snippet in my mind, is this correct?
if (reserved_heap)
e = reserved_heap_end;
else
{
do
{
e = consider_modules(ram_start, ram_end,
pfn_to_paddr(xenheap_pages),
32<<20, 0);
if ( e )
break;
xenheap_pages >>= 1;
} while ( !opt_xenheap_megabytes && xenheap_pages > 32<<(20-PAGE_SHIFT) );
}
>
> > domheap_pages = heap_pages - xenheap_pages;
> >
> > @@ -810,9 +838,9 @@ static void __init setup_mm(void)
> > static void __init setup_mm(void)
> > {
> > const struct meminfo *banks = &bootinfo.mem;
> > - paddr_t ram_start = ~0;
> > - paddr_t ram_end = 0;
> > - paddr_t ram_size = 0;
> > + paddr_t ram_start = ~0, bank_start = ~0;
> > + paddr_t ram_end = 0, bank_end = 0;
> > + paddr_t ram_size = 0, bank_size = 0;
> > unsigned int i;
>
> Please use INVALID_PADDR or ~0ULL
Ack.
Kind regards,
Henry
>
>
> >
> > init_pdx();
> > @@ -821,17 +849,36 @@ static void __init setup_mm(void)
> > * We need some memory to allocate the page-tables used for the
> xenheap
> > * mappings. But some regions may contain memory already allocated
> > * for other uses (e.g. modules, reserved-memory...).
> > - *
> > + * If reserved heap regions are properly defined, (only) add these
> regions
> > + * in the boot allocator.
> > + */
> > + if ( reserved_heap )
> > + {
> > + for ( i = 0 ; i < bootinfo.reserved_mem.nr_banks; i++ )
> > + {
> > + if ( bootinfo.reserved_mem.bank[i].xen_heap )
> > + {
> > + bank_start = bootinfo.reserved_mem.bank[i].start;
> > + bank_size = bootinfo.reserved_mem.bank[i].size;
> > + bank_end = bank_start + bank_size;
> > +
> > + init_boot_pages(bank_start, bank_end);
> > + }
> > + }
> > + }
> > + /*
> > + * No reserved heap regions:
> > * For simplicity, add all the free regions in the boot allocator.
> > */
> > - populate_boot_allocator();
> > + else
> > + populate_boot_allocator();
> >
> > total_pages = 0;
> >
> > for ( i = 0; i < banks->nr_banks; i++ )
> > {
> > const struct membank *bank = &banks->bank[i];
> > - paddr_t bank_end = bank->start + bank->size;
> > + bank_end = bank->start + bank->size;
> >
> > ram_size = ram_size + bank->size;
> > ram_start = min(ram_start, bank->start);
> > --
> > 2.17.1
> >
