Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39484 )
Change subject: memrange: Add support for stealing required memory from given ranges ......................................................................
Patch Set 2:
(5 comments)
https://review.coreboot.org/c/coreboot/+/39484/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39484/1//COMMIT_MSG@9 PS1, Line 9: This change adds support for
nit: just "Add"?
Done
https://review.coreboot.org/c/coreboot/+/39484/1/src/include/memrange.h File src/include/memrange.h:
https://review.coreboot.org/c/coreboot/+/39484/1/src/include/memrange.h@172 PS1, Line 172: Limit beyond which the stolen memory cannot grow
so an upper bound for the memory range to reserve?
Done
https://review.coreboot.org/c/coreboot/+/39484/1/src/include/memrange.h@177 PS1, Line 177: If it is not possible to fulfill the request as per given constraints, this function returns : * false. Else, it sets the base address of stolen memory in stolen_base, creates a hole of : * required size at the stolen_base and returns true to indicate success
I think it's better to state the successful case first. […]
Done
https://review.coreboot.org/c/coreboot/+/39484/1/src/lib/memrange.c File src/lib/memrange.c:
https://review.coreboot.org/c/coreboot/+/39484/1/src/lib/memrange.c@397 PS1, Line 397:
if (size == 0) return NULL; otherwise memranges_steal() with a computed size==0 leads to weird artif […]
Done
https://review.coreboot.org/c/coreboot/+/39484/1/src/lib/memrange.c@403 PS1, Line 403: base = ALIGN_UP(r->begin, align);
do you intend to use the new memrange align attribute for comparison, too? if (!IS_ALIGNED(align, r- […]
Done. Add it before the loop since each range entry does not have a separate alignment. It is applicable to entire list of ranges.