Hi Julien,
> -----Original Message-----
> From: Julien Grall <[email protected]>
> > - /* find first memory range not bound to a Xen domain */
> > - for ( i = 0; i < mem->nr_banks && mem->bank[i].xen_domain; i++ )
> > + /* find first memory range not bound to a Xen domain nor heap */
>
> This comment could become stale if we are adding a new type. So how about:
>
> /* find the first memory range that is reserved for device (or firmware) */
Ok, will use this one.
>
> > +enum membank_type {
> > + /*
> > + * The MEMBANK_DEFAULT type refers to either reserved memory for
> the
> > + * device (or firmware) or any memory that will be used by the
> > allocator.
>
> I realize the part of the 'or' is what I suggested. However, I wasn't
> correct here (sorry).
No problem, actually, I've learned a lot about how Xen does the memory
management from your comments. So I should say thank you.
>
> In the context of 'mem' this is referring to any RAM. The setup code
> will then find the list of the regions that doesn't overlap with the
> 'reserved_mem' and then give the pages to the boot allocator (and
> subsequently the buddy allocator). Also...
>
> > + * The meaning depends on where the memory bank is actually used.
>
> ... this doesn't tell the reader which means applies where. So I would
> suggest the following:
>
> The MEMBANK_DEFAULT type refers to either reserved memory for the
> device/firmware (when the bank is in 'reserved_mem') or any RAM (when
> the bank is in 'mem'
Ok will follow that.
>
> The rest of the code looks good to me. So once we settled on the two
> comments above:
>
> Reviewed-by: Julien Grall <[email protected]>
Thanks!
Kind regards,
Henry
>
> Cheers,
>
> --
> Julien Grall
