Attention is currently required from: Jakub Czapiga, Jan Dabros. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43305 )
Change subject: tests: Add lib/memrange-test test case ......................................................................
Patch Set 6: Code-Review+2
(3 comments)
File tests/lib/memrange-test.c:
https://review.coreboot.org/c/coreboot/+/43305/comment/0fc55deb_a0dca914 PS5, Line 35: { .enabled = 1, .resource_list = &res_mock_1[CACHEABLE_TAG],
Your example looks cleaner and simpler.
Ack
https://review.coreboot.org/c/coreboot/+/43305/comment/1d675bdc_6a23e5a8 PS5, Line 140: static void test_memrange_basic(void **state)
Good point. I added assert for that. Please check if it is, what you meant.
Ack
https://review.coreboot.org/c/coreboot/+/43305/comment/3df5e1ff_b37c8941 PS5, Line 438:
I covered that case but I have one concern. […]
Right, I guess under normal circumstances it always steals from the front of the range so the total number of ranges doesn't change. It would change if it were to steal from somewhere out of the middle of the range, splitting the remaining memory into two ranges. I think this could happen for example if memranges_steal() requires an alignment larger than what the base address of the range that is stolen from is aligned to. Maybe worth adding an extra test case for that one?
-
Julius Werner (Code Review)