Attention is currently required from: Julius Werner, Jan Dabros. Jakub Czapiga 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:
(5 comments)
File tests/lib/memrange-test.c:
https://review.coreboot.org/c/coreboot/+/43305/comment/b6581ec4_38414a3b PS5, Line 23: /* Order of entries matters, since it must reflect mem_types enum */
nit: not quite sure what you're trying to say here but I don't think it's true. […]
You are right. That is what it was supposed to mean.
https://review.coreboot.org/c/coreboot/+/43305/comment/1ae2529e_d2810e62 PS5, Line 35: { .enabled = 1, .resource_list = &res_mock_1[CACHEABLE_TAG],
Why always the complication of having two devices? I mean, you're not really trying to test device_u […]
Your example looks cleaner and simpler.
https://review.coreboot.org/c/coreboot/+/43305/comment/2aa631d6_0cbe887b PS5, Line 40: /* Boundary 1 byte below 4GiB and 1 byte above 4GiB. */
Looks to me like it's 16K + 1 byte below 4GiB and exactly at 4GiB? The comment doesn't quite seem to […]
Yes, data is incorrect. I had issues with alignment and forgot to change it back.
https://review.coreboot.org/c/coreboot/+/43305/comment/d2184e6b_5eb8d7a2 PS5, Line 140: static void test_memrange_basic(void **state)
Can we have some kind of test to check that memranges_each_entry() always returns ranges in increasi […]
Good point. I added assert for that. Please check if it is, what you meant.
https://review.coreboot.org/c/coreboot/+/43305/comment/43d8cfdf_875ba23f PS5, Line 438:
I think some kind of test to make sure that a newly inserted range that's directly adjacent to an ex […]
I covered that case but I have one concern. I do not see why number of ranges would change (grow or shring). memranges_steal only shrinks range we are stealing from and does not create new range with e.g. HOLE_TAG.
-
Jakub Czapiga (Code Review)