Hello Patrick Georgi,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/41046
to review the following change.
Change subject: tests: Add region-test for rdev API ......................................................................
tests: Add region-test for rdev API
This patch adds a basic test for the common region and region_device APIs, sanity checking the basic functions and things like overflow-handling. There is certainly more that could be added here, but it's a start.
Signed-off-by: Julius Werner jwerner@chromium.org Change-Id: I4932402f54768557e5b22b16e66220bd90ddebfd --- A tests/commonlib/Makefile.inc A tests/commonlib/region-test.c 2 files changed, 288 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/41046/1
diff --git a/tests/commonlib/Makefile.inc b/tests/commonlib/Makefile.inc new file mode 100644 index 0000000..128faf8 --- /dev/null +++ b/tests/commonlib/Makefile.inc @@ -0,0 +1,17 @@ +## +## This file is part of the coreboot project. +## +## This program is free software; you can redistribute it and/or modify +## it under the terms of the GNU General Public License as published by +## the Free Software Foundation; version 2 of the License. +## +## This program is distributed in the hope that it will be useful, +## but WITHOUT ANY WARRANTY; without even the implied warranty of +## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +## GNU General Public License for more details. +## + +tests-y += region-test + +region-test-srcs += tests/commonlib/region-test.c +region-test-srcs += src/commonlib/region.c diff --git a/tests/commonlib/region-test.c b/tests/commonlib/region-test.c new file mode 100644 index 0000000..e009446 --- /dev/null +++ b/tests/commonlib/region-test.c @@ -0,0 +1,271 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* This file is part of the coreboot project. */ + +#include <commonlib/region.h> + +static void test_region(void **state) +{ + struct region outer = { .offset = 0x200000000, .size = 0x400000000 }; + assert_int_equal(region_offset(&outer), 0x200000000); + assert_int_equal(region_sz(&outer), 0x400000000); + assert_int_equal(region_end(&outer), 0x600000000); + + struct region inner = { .offset = 0x250000000, .size = 0x390000000 }; + assert_true(region_is_subregion(&outer, &inner)); + + struct region touching_bottom = { .offset = 0x200000000, .size = 0x100000000 }; + assert_true(region_is_subregion(&outer, &touching_bottom)); + + struct region touching_top = { .offset = 0x390000000, .size = 0x10000000 }; + assert_true(region_is_subregion(&outer, &touching_top)); + + struct region overlap_bottom = { .offset = 0x1f0000000, .size = 0x20000000 }; + assert_false(region_is_subregion(&outer, &overlap_bottom)); + + struct region overlap_top = { .offset = 0x5f0000000, .size = 0x20000000 }; + assert_false(region_is_subregion(&outer, &overlap_top)); + + struct region below = { .offset = 0x0, .size = 0x100000000 }; + assert_false(region_is_subregion(&outer, &below)); + + struct region above = { .offset = 0xffffffff00000000, .size = 0x100000000 }; + assert_false(region_is_subregion(&outer, &above)); +} + +static void *mock_mmap(const struct region_device *rdev, size_t offset, size_t size) +{ + check_expected_ptr(rdev); + check_expected(offset); + check_expected(size); + + return mock_ptr_type(void *); +} + +static int mock_unmap(const struct region_device *rdev, void * mapping) +{ + check_expected_ptr(rdev); + check_expected_ptr(mapping); + + return mock(); +} + +static ssize_t mock_readat(const struct region_device *rdev, void *buffer, + size_t offset, size_t size) +{ + check_expected_ptr(rdev); + check_expected_ptr(buffer); + check_expected(offset); + check_expected(size); + + ssize_t ret = mock(); + if (!ret) + return size; +} + +static ssize_t mock_writeat(const struct region_device *rdev, const void *buffer, + size_t offset, size_t size) +{ + check_expected_ptr(rdev); + check_expected_ptr(buffer); + check_expected(offset); + check_expected(size); + + ssize_t ret = mock(); + if (!ret) + return size; +} + +static ssize_t mock_eraseat(const struct region_device *rdev, size_t offset, size_t size) +{ + check_expected_ptr(rdev); + check_expected(offset); + check_expected(size); + + ssize_t ret = mock(); + if (!ret) + return size; +} + +struct region_device_ops mock_rdev_ops = { + .mmap = mock_mmap, + .munmap = mock_unmap, + .readat = mock_readat, + .writeat = mock_writeat, + .eraseat = mock_eraseat, +}; + +struct region_device mock_rdev = REGION_DEV_INIT(&mock_rdev_ops, 0, 0xffffffffffffffff); +void *mmap_result = (void *)0x12345678; +const size_t mock_size = 256; +u8 mock_buffer[256]; + +static void test_rdev_basics(void **state) +{ + assert_int_equal(region_device_offset(&mock_rdev), 0); + assert_int_equal(region_device_sz(&mock_rdev), 0xffffffffffffffff); + assert_int_equal(region_device_end(&mock_rdev), 0xffffffffffffffff); +} + +/* + * This function sets up defaults for the mock_rdev_ops functions so we don't have to explicitly + * mock every parameter every time. cmocka is kinda dumb for this sort of use case and doesn't + * let you override these anymore once they're set (because these are stored as queues, not + * stacks, and once you store an "infinite" element the test can never proceed behind it), so + * tests will always have to enqueue any custom values they may need for the rest of the test + * function before calling this. + */ +static void rdev_mock_defaults(void) +{ + will_return_maybe(mock_mmap, mmap_result); + will_return_maybe(mock_unmap, 0); + will_return_maybe(mock_readat, 0); + will_return_maybe(mock_writeat, 0); + will_return_maybe(mock_eraseat, 0); + + expect_value_count(mock_mmap, rdev, &mock_rdev, -2); + expect_value_count(mock_unmap, rdev, &mock_rdev, -2); + expect_value_count(mock_readat, rdev, &mock_rdev, -2); + expect_value_count(mock_writeat, rdev, &mock_rdev, -2); + expect_value_count(mock_eraseat, rdev, &mock_rdev, -2); + + expect_value_count(mock_readat, buffer, &mock_buffer, -2); + expect_value_count(mock_writeat, buffer, &mock_buffer, -2); + + expect_value_count(mock_mmap, offset, 0, -2); + expect_value_count(mock_readat, offset, 0, -2); + expect_value_count(mock_writeat, offset, 0, -2); + expect_value_count(mock_eraseat, offset, 0, -2); + + expect_value_count(mock_mmap, size, mock_size, -2); + expect_value_count(mock_readat, size, mock_size, -2); + expect_value_count(mock_writeat, size, mock_size, -2); + expect_value_count(mock_eraseat, size, mock_size, -2); + + expect_value_count(mock_unmap, mapping, mmap_result, -2); +} + +static void test_rdev_success(void **state) +{ + struct region_device child; + + expect_value(mock_mmap, size, region_device_sz(&mock_rdev)); + + rdev_mock_defaults(); + + assert_ptr_equal(rdev_mmap_full(&mock_rdev), mmap_result); + + assert_ptr_equal(rdev_mmap(&mock_rdev, 0, mock_size), mmap_result); + assert_int_equal(rdev_munmap(&mock_rdev, mmap_result), 0); + assert_int_equal(rdev_readat(&mock_rdev, mock_buffer, 0, mock_size), mock_size); + assert_int_equal(rdev_writeat(&mock_rdev, mock_buffer, 0, mock_size), mock_size); + assert_int_equal(rdev_eraseat(&mock_rdev, 0, mock_size), mock_size); +} + +static void test_rdev_failure(void **state) +{ + will_return(mock_mmap, NULL); + will_return(mock_unmap, -1); + will_return(mock_readat, -1); + will_return(mock_writeat, -1); + will_return(mock_eraseat, -1); + + rdev_mock_defaults(); + + assert_ptr_equal(rdev_mmap(&mock_rdev, 0, mock_size), NULL); + assert_int_equal(rdev_munmap(&mock_rdev, mmap_result), -1); + assert_int_equal(rdev_readat(&mock_rdev, mock_buffer, 0, mock_size), -1); + assert_int_equal(rdev_writeat(&mock_rdev, mock_buffer, 0, mock_size), -1); + assert_int_equal(rdev_eraseat(&mock_rdev, 0, mock_size), -1); +} + +static void test_rdev_wrap(void **state) +{ + struct region_device child; + const size_t wrap_offs = 0xffffffff00000000; + const size_t wrap_size = 0x200000000; + const size_t fit_size = 0xffffffff; + + /* For the 'wrap' cases, the underlying rdev_ops aren't even called, so only add + expectations for the 'fit' cases. */ + + expect_value(mock_mmap, offset, wrap_offs); + expect_value(mock_readat, offset, wrap_offs); + expect_value(mock_writeat, offset, wrap_offs); + expect_value(mock_eraseat, offset, wrap_offs); + + expect_value(mock_mmap, size, fit_size); + expect_value(mock_readat, size, fit_size); + expect_value(mock_writeat, size, fit_size); + expect_value(mock_eraseat, size, fit_size); + + rdev_mock_defaults(); + + assert_ptr_equal(rdev_mmap(&mock_rdev, wrap_offs, wrap_size), NULL); + assert_int_equal(rdev_readat(&mock_rdev, mock_buffer, wrap_offs, wrap_size), -1); + assert_int_equal(rdev_writeat(&mock_rdev, mock_buffer, wrap_offs, wrap_size), -1); + assert_int_equal(rdev_eraseat(&mock_rdev, wrap_offs, wrap_size), -1); + assert_int_equal(rdev_chain(&child, &mock_rdev, wrap_offs, wrap_size), -1); + + assert_ptr_equal(rdev_mmap(&mock_rdev, wrap_offs, fit_size), mmap_result); + assert_int_equal(rdev_readat(&mock_rdev, mock_buffer, wrap_offs, fit_size), fit_size); + assert_int_equal(rdev_writeat(&mock_rdev, mock_buffer, wrap_offs, fit_size), fit_size); + assert_int_equal(rdev_eraseat(&mock_rdev, wrap_offs, fit_size), fit_size); + assert_int_equal(rdev_chain(&child, &mock_rdev, wrap_offs, fit_size), 0); +} + +static void test_rdev_chain(void **state) +{ + struct region_device child; + const size_t child_offs = 0x200000000; + const size_t child_size = 0x100000000; + const size_t offs = 0x10000; + const size_t ovrflw_size = child_size - offs + 0x8000; + + expect_value(mock_mmap, offset, child_offs + offs); + expect_value(mock_readat, offset, child_offs + offs); + expect_value(mock_writeat, offset, child_offs + offs); + expect_value(mock_eraseat, offset, child_offs + offs); + + rdev_mock_defaults(); + + assert_int_equal(rdev_chain_full(&child, &mock_rdev), 0); + assert_int_equal(region_device_sz(&child), region_device_sz(&mock_rdev)); + assert_int_equal(region_device_offset(&child), region_device_offset(&mock_rdev)); + assert_int_equal(rdev_relative_offset(&mock_rdev, &child), 0); + + assert_int_equal(rdev_chain(&child, &mock_rdev, child_offs, child_size), 0); + assert_int_equal(region_device_sz(&child), child_size); + assert_int_equal(region_device_offset(&child), child_offs); + assert_int_equal(region_device_end(&child), child_offs + child_size); + assert_int_equal(rdev_relative_offset(&mock_rdev, &child), child_offs); + + assert_ptr_equal(rdev_mmap(&child, offs, mock_size), mmap_result); + assert_int_equal(rdev_munmap(&child, mmap_result), 0); + assert_int_equal(rdev_readat(&child, mock_buffer, offs, mock_size), mock_size); + assert_int_equal(rdev_writeat(&child, mock_buffer, offs, mock_size), mock_size); + assert_int_equal(rdev_eraseat(&child, offs, mock_size), mock_size); + + assert_ptr_equal(rdev_mmap(&child, offs, ovrflw_size), NULL); + assert_int_equal(rdev_readat(&child, mock_buffer, offs, ovrflw_size), -1); + assert_int_equal(rdev_writeat(&child, mock_buffer, offs, ovrflw_size), -1); + assert_int_equal(rdev_eraseat(&child, offs, ovrflw_size), -1); + + assert_ptr_equal(rdev_mmap(&child, child_size, mock_size), NULL); + assert_int_equal(rdev_readat(&child, mock_buffer, child_size, mock_size), -1); + assert_int_equal(rdev_writeat(&child, mock_buffer, child_size, mock_size), -1); + assert_int_equal(rdev_eraseat(&child, child_size, mock_size), -1); +} + +int main(void) +{ + const struct CMUnitTest tests[] = { + cmocka_unit_test(test_region), + cmocka_unit_test(test_rdev_basics), + cmocka_unit_test(test_rdev_success), + cmocka_unit_test(test_rdev_failure), + cmocka_unit_test(test_rdev_wrap), + cmocka_unit_test(test_rdev_chain), + }; + + return cmocka_run_group_tests(tests, NULL, NULL); +}
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41046 )
Change subject: tests: Add region-test for rdev API ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41046/1/tests/commonlib/region-test... File tests/commonlib/region-test.c:
https://review.coreboot.org/c/coreboot/+/41046/1/tests/commonlib/region-test... PS1, Line 44: static int mock_unmap(const struct region_device *rdev, void * mapping) "foo * bar" should be "foo *bar"
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41046 )
Change subject: tests: Add region-test for rdev API ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41046/1/tests/commonlib/region-test... File tests/commonlib/region-test.c:
https://review.coreboot.org/c/coreboot/+/41046/1/tests/commonlib/region-test... PS1, Line 172: rdev_mock_defaults(); Tried my hand on writing a test to see how it works in practice. This part here is the one I don't like that much yet. I was hoping I could just put the statements from rdev_mock_defaults() at the top of a test function and then do things like
will_return(mock_mmap, NULL); assert_ptr_equal(rdev_mmap(&mock_rdev, 0, mock_size), NULL);
so that the one specific mocked return value I need for this one call is pushed to the top of the mocked return value stack, used by that call, and then it goes back to the default state (which is added with infinite count) below. But that doesn't really work because the mocked return values are queues, not stacks (even though the API documentation actually says stacks), so I had to write it like this where the expectation setup is a bit too far away from the call it belongs to for my tastes (and you have to make a new test function for everything because once you install the "infinite" expectations, you can never modify them again).
I think this would all work way better if they just used a stack instead of a queue for those things, I actually opened a ticket with the cmocka folks to ask about that (https://gitlab.com/cmocka/cmocka/-/issues/44), but they probably have their reasons why they wanted it to work this way. So for now, I think writing it like this is the best way you can use it.
Hello build bot (Jenkins), Martin Roth, Patrick Georgi, Jan Dabros,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41046
to look at the new patch set (#2).
Change subject: tests: Add region-test for rdev API ......................................................................
tests: Add region-test for rdev API
This patch adds a basic test for the common region and region_device APIs, sanity checking the basic functions and things like overflow-handling. There is certainly more that could be added here, but it's a start.
Signed-off-by: Julius Werner jwerner@chromium.org Change-Id: I4932402f54768557e5b22b16e66220bd90ddebfd --- A tests/commonlib/Makefile.inc A tests/commonlib/region-test.c 2 files changed, 288 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/41046/2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41046 )
Change subject: tests: Add region-test for rdev API ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41046/2/tests/commonlib/Makefile.in... File tests/commonlib/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/41046/2/tests/commonlib/Makefile.in... PS2, Line 11: ## GNU General Public License for more details. Please use SPDX headers.
Hello build bot (Jenkins), Martin Roth, Patrick Georgi, Jan Dabros,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41046
to look at the new patch set (#3).
Change subject: tests: Add region-test for rdev API ......................................................................
tests: Add region-test for rdev API
This patch adds a basic test for the common region and region_device APIs, sanity checking the basic functions and things like overflow-handling. There is certainly more that could be added here, but it's a start.
Signed-off-by: Julius Werner jwerner@chromium.org Change-Id: I4932402f54768557e5b22b16e66220bd90ddebfd --- A tests/commonlib/Makefile.inc A tests/commonlib/region-test.c 2 files changed, 278 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/41046/3
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41046 )
Change subject: tests: Add region-test for rdev API ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41046/2/tests/commonlib/Makefile.in... File tests/commonlib/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/41046/2/tests/commonlib/Makefile.in... PS2, Line 11: ## GNU General Public License for more details.
Please use SPDX headers.
Done
https://review.coreboot.org/c/coreboot/+/41046/1/tests/commonlib/region-test... File tests/commonlib/region-test.c:
https://review.coreboot.org/c/coreboot/+/41046/1/tests/commonlib/region-test... PS1, Line 172: rdev_mock_defaults();
Tried my hand on writing a test to see how it works in practice. […]
Ack
Jan Dabros has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41046 )
Change subject: tests: Add region-test for rdev API ......................................................................
Patch Set 3:
(5 comments)
https://review.coreboot.org/c/coreboot/+/41046/1/tests/commonlib/region-test... File tests/commonlib/region-test.c:
https://review.coreboot.org/c/coreboot/+/41046/1/tests/commonlib/region-test... PS1, Line 172: rdev_mock_defaults();
Ack
Good to hear that someone else is also verifying test infrastructure!
I see your problem and frankly - I'm also surprised, that implementation is not stack-based as described in the doc. Let's see what will be the response from community. However I'm afraid, that introducing such a significant change in test framework may break a lot of tests people have already written.
If there will be no need for putting specific return values at the beginning of a queue (but simply put value on the stack), then we would be able to use Cmocka's test fixtures with some init function like your rdev_mock_defaults().
I think that your proposal is the best what we can have as of now. Initially I thought about changing implementation of rdev_mock_defaults() to have some extra parameters, which allows us configure return values per-test, but this will be more complicated I think.
https://review.coreboot.org/c/coreboot/+/41046/3/tests/commonlib/region-test... File tests/commonlib/region-test.c:
https://review.coreboot.org/c/coreboot/+/41046/3/tests/commonlib/region-test... PS3, Line 8: struct region outer = { .offset = 0x200000000, .size = 0x400000000 }; What about using some macros like SIZE_8GB, SIZE_16GB, or 0x390000000 -> (SIZE_16GB - SIZE_1_75GB)? My point is that, it is very easy to get lost with all these zeros.
https://review.coreboot.org/c/coreboot/+/41046/3/tests/commonlib/region-test... PS3, Line 97: struct region_device mock_rdev = REGION_DEV_INIT(&mock_rdev_ops, 0, 0xffffffffffffffff); In this driver, you are calling your fake region_device as mock_rdev, while in i2c-test, we have i2c_ex_dev (from _ex_ample). We are inconsistent here - do we want to change this (either here or there)? My issue with _mock_ is that it has nothing to do with mocking of functions - which is actually the most fundamental feature of Cmocka.
https://review.coreboot.org/c/coreboot/+/41046/3/tests/commonlib/region-test... PS3, Line 119: will_return_maybe(mock_mmap, mmap_result); Just a note, this test won't build by default on Ubuntu 16.04 (I expect newer ones to also have the same problem), since it is distributed with libcmocka0 1.0.1-2. This version doesn't have support for will_return_maybe() and -2 argument for expect_value_count(). Do you think, that I should document requirements regarding version of Cmocka lib somewhere?
https://review.coreboot.org/c/coreboot/+/41046/3/tests/commonlib/region-test... PS3, Line 184: const size_t wrap_offs = 0xffffffff00000000; This variable is used also for non-wrap cases, I will simply use offs. This init value for offset isn't necessarily indicating overflow condition, only with conjunction with some values of size.
Hello build bot (Jenkins), Martin Roth, Patrick Georgi, Jan Dabros,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41046
to look at the new patch set (#4).
Change subject: tests: Add region-test for rdev API ......................................................................
tests: Add region-test for rdev API
This patch adds a basic test for the common region and region_device APIs, sanity checking the basic functions and things like overflow-handling. There is certainly more that could be added here, but it's a start.
Signed-off-by: Julius Werner jwerner@chromium.org Change-Id: I4932402f54768557e5b22b16e66220bd90ddebfd --- A tests/commonlib/Makefile.inc A tests/commonlib/region-test.c 2 files changed, 293 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/41046/4
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41046 )
Change subject: tests: Add region-test for rdev API ......................................................................
Patch Set 4:
(5 comments)
https://review.coreboot.org/c/coreboot/+/41046/1/tests/commonlib/region-test... File tests/commonlib/region-test.c:
https://review.coreboot.org/c/coreboot/+/41046/1/tests/commonlib/region-test... PS1, Line 172: rdev_mock_defaults();
Good to hear that someone else is also verifying test infrastructure! […]
Yeah, I could see many ways of how to make rdev_mock_defaults() more flexible to support more cases, but that would quickly become very complicated and like I wrote in that GitLab bug report I feel like I'd be basically reimplementing the stuff that the test framework was supposed to be there for then.
I agree chances for an upstream fix are probably unlikely. Let's go with this for now. If we have a couple of more tests we can decide how serious of a problem it really is and whether we need to build more custom infrastructure on top of (or maybe even fork) cmocka to address it.
https://review.coreboot.org/c/coreboot/+/41046/3/tests/commonlib/region-test... File tests/commonlib/region-test.c:
https://review.coreboot.org/c/coreboot/+/41046/3/tests/commonlib/region-test... PS3, Line 8: struct region outer = { .offset = 0x200000000, .size = 0x400000000 };
What about using some macros like SIZE_8GB, SIZE_16GB, or 0x390000000 -> (SIZE_16GB - SIZE_1_75GB)? […]
Yeah... actually there's also a problem with these large sizes because the tests would run on a 32-bit arch. I would like to test them this large to make sure there are no large number handling issues, but we probably can't just have the tests not work on some hosts either.
I found a new way to solve this issue which I think works better.
https://review.coreboot.org/c/coreboot/+/41046/3/tests/commonlib/region-test... PS3, Line 97: struct region_device mock_rdev = REGION_DEV_INIT(&mock_rdev_ops, 0, 0xffffffffffffffff);
In this driver, you are calling your fake region_device as mock_rdev, while in i2c-test, we have i2c […]
I'd say this is a mock in the purest sense of the word? struct region_device is a class (as far as C can have classes), struct region_device_ops are its methods, and this struct is a mock object implementing that class with the mock functions defined above as its methods.
But I also don't think we need to be super strict about naming local data in a test file. It doesn't really matter that much. In the vboot tests we call anything "mock" that's fake data for testing purposes in any sense of the word, that's just what I'm used to.
https://review.coreboot.org/c/coreboot/+/41046/3/tests/commonlib/region-test... PS3, Line 119: will_return_maybe(mock_mmap, mmap_result);
Just a note, this test won't build by default on Ubuntu 16. […]
Is there some good way we can put this in the Makefile, maybe? Some automatic check to make sure the cmocka version is larger than the minimum supported? (Maybe a bare -lcmocka is a bit brittle there anyway... don't people normally use pkg-config for this kind of stuff? I'm not very familiar with the practical aspects of building userspace code but I think that's what I see people do most of the time... and I think that has some kind of version awareness support as well?)
https://review.coreboot.org/c/coreboot/+/41046/3/tests/commonlib/region-test... PS3, Line 184: const size_t wrap_offs = 0xffffffff00000000;
This variable is used also for non-wrap cases, I will simply use offs. […]
Done
Jan Dabros has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41046 )
Change subject: tests: Add region-test for rdev API ......................................................................
Patch Set 4: Code-Review+1
(5 comments)
https://review.coreboot.org/c/coreboot/+/41046/3/tests/commonlib/region-test... File tests/commonlib/region-test.c:
https://review.coreboot.org/c/coreboot/+/41046/3/tests/commonlib/region-test... PS3, Line 8: struct region outer = { .offset = 0x200000000, .size = 0x400000000 };
Yeah... […]
I like your idea, more portable implementation.
https://review.coreboot.org/c/coreboot/+/41046/3/tests/commonlib/region-test... PS3, Line 97: struct region_device mock_rdev = REGION_DEV_INIT(&mock_rdev_ops, 0, 0xffffffffffffffff);
I'd say this is a mock in the purest sense of the word? struct region_device is a class (as far as C […]
I didn't want to say that your naming is improper. Object of this struct is a mock as you said. But for last couple of weeks I'm consistently working on Cmocka only, so when I hear "mock" I'm thinking - wrapping a function and mocking its implementation.. which is a rather shallow view when thinking about this more :) So feel free to ignore this and keep mock_rdev.
https://review.coreboot.org/c/coreboot/+/41046/3/tests/commonlib/region-test... PS3, Line 119: will_return_maybe(mock_mmap, mmap_result);
Is there some good way we can put this in the Makefile, maybe? Some automatic check to make sure the […]
Yes, pkg-config should do the trick with checking package version (and automatically provide flags for linker). However, I'm a little bit worried about how this will work on different distributions (since I think devs here are using different build systems). Will need to check this before proposing a solution.
https://review.coreboot.org/c/coreboot/+/41046/4/tests/commonlib/region-test... File tests/commonlib/region-test.c:
https://review.coreboot.org/c/coreboot/+/41046/4/tests/commonlib/region-test... PS4, Line 212: assert_ptr_equal(rdev_mmap(&mock_rdev, offs, wrap_size), NULL); Can you add a short comment why we are expecting failure of API calls here? Similar to what you have done for test_rdev_chain. This way, your test acts (almost) as a documentation, explaining what the rdev module is doing.
https://review.coreboot.org/c/coreboot/+/41046/4/tests/commonlib/region-test... PS4, Line 247: /* Remaining test use rdev chained to [child_offs:child_size) subregion. */ nit: *tests
Hello build bot (Jenkins), Martin Roth, Patrick Georgi, Jan Dabros,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41046
to look at the new patch set (#5).
Change subject: tests: Add region-test for rdev API ......................................................................
tests: Add region-test for rdev API
This patch adds a basic test for the common region and region_device APIs, sanity checking the basic functions and things like overflow-handling. There is certainly more that could be added here, but it's a start.
Signed-off-by: Julius Werner jwerner@chromium.org Change-Id: I4932402f54768557e5b22b16e66220bd90ddebfd --- M tests/Makefile.inc A tests/commonlib/Makefile.inc A tests/commonlib/region-test.c 3 files changed, 412 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/41046/5
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41046 )
Change subject: tests: Add region-test for rdev API ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/c/coreboot/+/41046/3/tests/commonlib/region-test... File tests/commonlib/region-test.c:
https://review.coreboot.org/c/coreboot/+/41046/3/tests/commonlib/region-test... PS3, Line 119: will_return_maybe(mock_mmap, mmap_result);
Yes, pkg-config should do the trick with checking package version (and automatically provide flags f […]
Could probably check the return value of the call and just fall back to -lcmocka without any extra tests if it didn't work (e.g. pkg-config not installed)?
https://review.coreboot.org/c/coreboot/+/41046/4/tests/commonlib/region-test... File tests/commonlib/region-test.c:
https://review.coreboot.org/c/coreboot/+/41046/4/tests/commonlib/region-test... PS4, Line 212: assert_ptr_equal(rdev_mmap(&mock_rdev, offs, wrap_size), NULL);
Can you add a short comment why we are expecting failure of API calls here? Similar to what you have […]
Done
https://review.coreboot.org/c/coreboot/+/41046/4/tests/commonlib/region-test... PS4, Line 247: /* Remaining test use rdev chained to [child_offs:child_size) subregion. */
nit: *tests
Done
Jan Dabros has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41046 )
Change subject: tests: Add region-test for rdev API ......................................................................
Patch Set 5: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/41046/3/tests/commonlib/region-test... File tests/commonlib/region-test.c:
https://review.coreboot.org/c/coreboot/+/41046/3/tests/commonlib/region-test... PS3, Line 119: will_return_maybe(mock_mmap, mmap_result);
Could probably check the return value of the call and just fall back to -lcmocka without any extra t […]
Hmm.. yes, this may be the best option. At least this will improve situation for setups with pkg-config installed.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41046 )
Change subject: tests: Add region-test for rdev API ......................................................................
Patch Set 6:
*ping*
Anyone wanna +2 this? Patrick?
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41046 )
Change subject: tests: Add region-test for rdev API ......................................................................
Patch Set 6: Code-Review+2
Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/41046 )
Change subject: tests: Add region-test for rdev API ......................................................................
tests: Add region-test for rdev API
This patch adds a basic test for the common region and region_device APIs, sanity checking the basic functions and things like overflow-handling. There is certainly more that could be added here, but it's a start.
Signed-off-by: Julius Werner jwerner@chromium.org Change-Id: I4932402f54768557e5b22b16e66220bd90ddebfd Reviewed-on: https://review.coreboot.org/c/coreboot/+/41046 Reviewed-by: Aaron Durbin adurbin@chromium.org Reviewed-by: Jan Dabros Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M tests/Makefile.inc A tests/commonlib/Makefile.inc A tests/commonlib/region-test.c 3 files changed, 412 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved Jan Dabros: Looks good to me, but someone else must approve
diff --git a/tests/Makefile.inc b/tests/Makefile.inc index 9ee27cd..be32434 100644 --- a/tests/Makefile.inc +++ b/tests/Makefile.inc @@ -24,8 +24,7 @@ # Path for Kconfig autoheader TEST_CFLAGS += -I$(dir $(TEST_KCONFIG_AUTOHEADER))
-TEST_CFLAGS += -std=gnu11 -Os -ffunction-sections -fdata-sections \ - -fno-builtin +TEST_CFLAGS += -std=gnu11 -Os -ffunction-sections -fdata-sections -fno-builtin
# Link against Cmocka TEST_LDFLAGS = -lcmocka -Wl,--gc-sections diff --git a/tests/commonlib/Makefile.inc b/tests/commonlib/Makefile.inc new file mode 100644 index 0000000..ce3499c --- /dev/null +++ b/tests/commonlib/Makefile.inc @@ -0,0 +1,7 @@ +# SPDX-License-Identifier: GPL-2.0-only +# This file is part of the coreboot project. + +tests-y += region-test + +region-test-srcs += tests/commonlib/region-test.c +region-test-srcs += src/commonlib/region.c diff --git a/tests/commonlib/region-test.c b/tests/commonlib/region-test.c new file mode 100644 index 0000000..2c960e0 --- /dev/null +++ b/tests/commonlib/region-test.c @@ -0,0 +1,404 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* This file is part of the coreboot project. */ + +#include <commonlib/region.h> +#include <string.h> +#include <tests/test.h> + +/* We'd like to test overflow conditions, but for tests size_t is dependent on the HOSTCC + architecture. We use this to normalize the available address space to [VAL(0x0):VAL(0xf)). */ +#define VAL(v) ((size_t)(v##ULL << (sizeof(size_t) * 8 - 4))) + +static void test_region(void **state) +{ + /* Self-test: make sure VAL() overflow works as intended. */ + assert_true(VAL(5) + VAL(10) > VAL(10)); + assert_true(VAL(7) + VAL(10) < VAL(10)); + + struct region outer = { .offset = VAL(2), .size = VAL(4) }; + assert_int_equal(region_offset(&outer), VAL(2)); + assert_int_equal(region_sz(&outer), VAL(4)); + assert_int_equal(region_end(&outer), VAL(6)); + + struct region inner = { .offset = VAL(3), .size = VAL(2) }; + assert_true(region_is_subregion(&outer, &inner)); + + struct region touching_bottom = { .offset = VAL(2), .size = VAL(1) }; + assert_true(region_is_subregion(&outer, &touching_bottom)); + + struct region touching_top = { .offset = VAL(5), .size = VAL(1) }; + assert_true(region_is_subregion(&outer, &touching_top)); + + struct region overlap_bottom = { .offset = VAL(1), .size = VAL(2) }; + assert_false(region_is_subregion(&outer, &overlap_bottom)); + + struct region overlap_top = { .offset = VAL(5), .size = VAL(2) }; + assert_false(region_is_subregion(&outer, &overlap_top)); + + struct region below = { .offset = 0, .size = VAL(1) }; + assert_false(region_is_subregion(&outer, &below)); + + struct region above = { .offset = VAL(0xf), .size = VAL(1) }; + assert_false(region_is_subregion(&outer, &above)); +} + +static void *mock_mmap(const struct region_device *rdev, size_t offset, size_t size) +{ + check_expected_ptr(rdev); + check_expected(offset); + check_expected(size); + + return mock_ptr_type(void *); +} + +static int mock_unmap(const struct region_device *rdev, void *mapping) +{ + check_expected_ptr(rdev); + check_expected_ptr(mapping); + + return mock(); +} + +static ssize_t mock_readat(const struct region_device *rdev, void *buffer, + size_t offset, size_t size) +{ + check_expected_ptr(rdev); + check_expected_ptr(buffer); + check_expected(offset); + check_expected(size); + + ssize_t ret = mock(); + if (!ret) + return size; +} + +static ssize_t mock_writeat(const struct region_device *rdev, const void *buffer, + size_t offset, size_t size) +{ + check_expected_ptr(rdev); + check_expected_ptr(buffer); + check_expected(offset); + check_expected(size); + + ssize_t ret = mock(); + if (!ret) + return size; +} + +static ssize_t mock_eraseat(const struct region_device *rdev, size_t offset, size_t size) +{ + check_expected_ptr(rdev); + check_expected(offset); + check_expected(size); + + ssize_t ret = mock(); + if (!ret) + return size; +} + +struct region_device_ops mock_rdev_ops = { + .mmap = mock_mmap, + .munmap = mock_unmap, + .readat = mock_readat, + .writeat = mock_writeat, + .eraseat = mock_eraseat, +}; + +struct region_device mock_rdev = REGION_DEV_INIT(&mock_rdev_ops, 0, ~(size_t)0); +void *mmap_result = (void *)0x12345678; +const size_t mock_size = 256; +u8 mock_buffer[256]; + +static void test_rdev_basics(void **state) +{ + assert_int_equal(region_device_offset(&mock_rdev), 0); + assert_int_equal(region_device_sz(&mock_rdev), ~(size_t)0); + assert_int_equal(region_device_end(&mock_rdev), ~(size_t)0); +} + +/* + * This function sets up defaults for the mock_rdev_ops functions so we don't have to explicitly + * mock every parameter every time. cmocka doesn't really work well for this sort of use case + * and won't let you override these anymore once they're set (because these are stored as + * queues, not stacks, and once you store an "infinite" element the test can never proceed + * behind it), so tests will always have to enqueue any custom values they may need for the rest + * of the test function before calling this. + */ +static void rdev_mock_defaults(void) +{ + will_return_maybe(mock_mmap, mmap_result); + will_return_maybe(mock_unmap, 0); + will_return_maybe(mock_readat, 0); + will_return_maybe(mock_writeat, 0); + will_return_maybe(mock_eraseat, 0); + + expect_value_count(mock_mmap, rdev, &mock_rdev, -2); + expect_value_count(mock_unmap, rdev, &mock_rdev, -2); + expect_value_count(mock_readat, rdev, &mock_rdev, -2); + expect_value_count(mock_writeat, rdev, &mock_rdev, -2); + expect_value_count(mock_eraseat, rdev, &mock_rdev, -2); + + expect_value_count(mock_readat, buffer, &mock_buffer, -2); + expect_value_count(mock_writeat, buffer, &mock_buffer, -2); + + expect_value_count(mock_mmap, offset, 0, -2); + expect_value_count(mock_readat, offset, 0, -2); + expect_value_count(mock_writeat, offset, 0, -2); + expect_value_count(mock_eraseat, offset, 0, -2); + + expect_value_count(mock_mmap, size, mock_size, -2); + expect_value_count(mock_readat, size, mock_size, -2); + expect_value_count(mock_writeat, size, mock_size, -2); + expect_value_count(mock_eraseat, size, mock_size, -2); + + expect_value_count(mock_unmap, mapping, mmap_result, -2); +} + +static void test_rdev_success(void **state) +{ + struct region_device child; + + expect_value(mock_mmap, size, region_device_sz(&mock_rdev)); + + rdev_mock_defaults(); + + assert_ptr_equal(rdev_mmap_full(&mock_rdev), mmap_result); + + assert_ptr_equal(rdev_mmap(&mock_rdev, 0, mock_size), mmap_result); + assert_int_equal(rdev_munmap(&mock_rdev, mmap_result), 0); + assert_int_equal(rdev_readat(&mock_rdev, mock_buffer, 0, mock_size), mock_size); + assert_int_equal(rdev_writeat(&mock_rdev, mock_buffer, 0, mock_size), mock_size); + assert_int_equal(rdev_eraseat(&mock_rdev, 0, mock_size), mock_size); +} + +static void test_rdev_failure(void **state) +{ + will_return(mock_mmap, NULL); + will_return(mock_unmap, -1); + will_return(mock_readat, -1); + will_return(mock_writeat, -1); + will_return(mock_eraseat, -1); + + rdev_mock_defaults(); + + assert_null(rdev_mmap(&mock_rdev, 0, mock_size)); + assert_int_equal(rdev_munmap(&mock_rdev, mmap_result), -1); + assert_int_equal(rdev_readat(&mock_rdev, mock_buffer, 0, mock_size), -1); + assert_int_equal(rdev_writeat(&mock_rdev, mock_buffer, 0, mock_size), -1); + assert_int_equal(rdev_eraseat(&mock_rdev, 0, mock_size), -1); +} + +static void test_rdev_wrap(void **state) +{ + struct region_device child; + const size_t offs = VAL(0xf); + const size_t wrap_size = VAL(2); + /* Known API limitation -- can't exactly touch address space limit from below. */ + const size_t fit_size = VAL(1) - 1; + + /* For the 'wrap' cases, the underlying rdev_ops aren't even called, so only add + expectations for the 'fit' cases. */ + expect_value(mock_mmap, offset, offs); + expect_value(mock_readat, offset, offs); + expect_value(mock_writeat, offset, offs); + expect_value(mock_eraseat, offset, offs); + + expect_value(mock_mmap, size, fit_size); + expect_value(mock_readat, size, fit_size); + expect_value(mock_writeat, size, fit_size); + expect_value(mock_eraseat, size, fit_size); + + rdev_mock_defaults(); + + /* Accesses to regions that wrap around the end of the address space should fail. */ + assert_null(rdev_mmap(&mock_rdev, offs, wrap_size)); + assert_int_equal(rdev_readat(&mock_rdev, mock_buffer, offs, wrap_size), -1); + assert_int_equal(rdev_writeat(&mock_rdev, mock_buffer, offs, wrap_size), -1); + assert_int_equal(rdev_eraseat(&mock_rdev, offs, wrap_size), -1); + assert_int_equal(rdev_chain(&child, &mock_rdev, offs, wrap_size), -1); + + /* Just barely touching the end of the address space (and the rdev) should be fine. */ + assert_ptr_equal(rdev_mmap(&mock_rdev, offs, fit_size), mmap_result); + assert_int_equal(rdev_readat(&mock_rdev, mock_buffer, offs, fit_size), fit_size); + assert_int_equal(rdev_writeat(&mock_rdev, mock_buffer, offs, fit_size), fit_size); + assert_int_equal(rdev_eraseat(&mock_rdev, offs, fit_size), fit_size); + assert_int_equal(rdev_chain(&child, &mock_rdev, offs, fit_size), 0); +} + +static void test_rdev_chain(void **state) +{ + struct region_device child; + const size_t child_offs = VAL(2); + const size_t child_size = VAL(4); + const size_t offs = VAL(1); + const size_t ovrflw_size = child_size - offs + 1; + + /* The mock_size test is the only one that will go through to underlying rdev_ops. */ + expect_value(mock_mmap, offset, child_offs + offs); + expect_value(mock_readat, offset, child_offs + offs); + expect_value(mock_writeat, offset, child_offs + offs); + expect_value(mock_eraseat, offset, child_offs + offs); + + rdev_mock_defaults(); + + /* First a quick test for rdev_chain_full(). */ + assert_int_equal(rdev_chain_full(&child, &mock_rdev), 0); + assert_int_equal(region_device_sz(&child), region_device_sz(&mock_rdev)); + assert_int_equal(region_device_offset(&child), region_device_offset(&mock_rdev)); + assert_int_equal(rdev_relative_offset(&mock_rdev, &child), 0); + + /* Remaining tests use rdev chained to [child_offs:child_size) subregion. */ + assert_int_equal(rdev_chain(&child, &mock_rdev, child_offs, child_size), 0); + assert_int_equal(region_device_sz(&child), child_size); + assert_int_equal(region_device_offset(&child), child_offs); + assert_int_equal(region_device_end(&child), child_offs + child_size); + assert_int_equal(rdev_relative_offset(&mock_rdev, &child), child_offs); + assert_int_equal(rdev_relative_offset(&child, &mock_rdev), -1); + + /* offs + mock_size < child_size, so will succeed. */ + assert_ptr_equal(rdev_mmap(&child, offs, mock_size), mmap_result); + assert_int_equal(rdev_munmap(&child, mmap_result), 0); + assert_int_equal(rdev_readat(&child, mock_buffer, offs, mock_size), mock_size); + assert_int_equal(rdev_writeat(&child, mock_buffer, offs, mock_size), mock_size); + assert_int_equal(rdev_eraseat(&child, offs, mock_size), mock_size); + + /* offs + ovrflw_size > child_size, so will fail. */ + assert_null(rdev_mmap(&child, offs, ovrflw_size)); + assert_int_equal(rdev_readat(&child, mock_buffer, offs, ovrflw_size), -1); + assert_int_equal(rdev_writeat(&child, mock_buffer, offs, ovrflw_size), -1); + assert_int_equal(rdev_eraseat(&child, offs, ovrflw_size), -1); + + /* Using child_size as offset, the start of the area will already be out of range. */ + assert_null(rdev_mmap(&child, child_size, mock_size)); + assert_int_equal(rdev_readat(&child, mock_buffer, child_size, mock_size), -1); + assert_int_equal(rdev_writeat(&child, mock_buffer, child_size, mock_size), -1); + assert_int_equal(rdev_eraseat(&child, child_size, mock_size), -1); +} + +static void test_rdev_double_chain(void **state) +{ + struct region_device first, second; + const size_t first_offs = VAL(2); + const size_t first_size = VAL(6); + const size_t second_offs = VAL(2); + const size_t second_size = VAL(2); + const size_t offs = VAL(1); + const size_t ovrflw_size = second_size - offs + 1; + + /* The mock_size test is the only one that will go through to underlying rdev_ops. */ + expect_value(mock_mmap, offset, first_offs + second_offs + offs); + expect_value(mock_readat, offset, first_offs + second_offs + offs); + expect_value(mock_writeat, offset, first_offs + second_offs + offs); + expect_value(mock_eraseat, offset, first_offs + second_offs + offs); + + rdev_mock_defaults(); + + /* First, chain an rdev to root over [first_offs:first_size). */ + assert_int_equal(rdev_chain(&first, &mock_rdev, first_offs, first_size), 0); + + /* Trying to chain a second to first beyond its end should fail. */ + assert_int_equal(rdev_chain(&second, &first, second_offs, first_size), -1); + + /* Chain second to first at [second_offs:second_size). */ + assert_int_equal(rdev_chain(&second, &first, second_offs, second_size), 0); + assert_int_equal(rdev_relative_offset(&first, &second), second_offs); + assert_int_equal(rdev_relative_offset(&mock_rdev, &second), first_offs + second_offs); + + /* offs + mock_size < second_size, so will succeed. */ + assert_ptr_equal(rdev_mmap(&second, offs, mock_size), mmap_result); + assert_int_equal(rdev_munmap(&second, mmap_result), 0); + assert_int_equal(rdev_readat(&second, mock_buffer, offs, mock_size), mock_size); + assert_int_equal(rdev_writeat(&second, mock_buffer, offs, mock_size), mock_size); + assert_int_equal(rdev_eraseat(&second, offs, mock_size), mock_size); + + /* offs + ovrflw_size > second_size, so will fail. */ + assert_null(rdev_mmap(&second, offs, ovrflw_size)); + assert_int_equal(rdev_readat(&second, mock_buffer, offs, ovrflw_size), -1); + assert_int_equal(rdev_writeat(&second, mock_buffer, offs, ovrflw_size), -1); + assert_int_equal(rdev_eraseat(&second, offs, ovrflw_size), -1); + + /* offs + second_size + offs way out of range. */ + assert_null(rdev_mmap(&second, second_size + offs, mock_size)); + assert_int_equal(rdev_readat(&second, mock_buffer, second_size + offs, mock_size), -1); + assert_int_equal(rdev_writeat(&second, mock_buffer, second_size + offs, mock_size), -1); + assert_int_equal(rdev_eraseat(&second, second_size + offs, mock_size), -1); +} + +static void test_mem_rdev(void **state) +{ + const size_t size = 256; + u8 backing[size]; + u8 scratch[size]; + int i; + struct mem_region_device mem = MEM_REGION_DEV_RW_INIT(backing, size); + + /* Test writing to and reading from full mapping. */ + memset(backing, 0xa5, size); + u8 *mapping = rdev_mmap_full(&mem.rdev); + assert_non_null(mapping); + for (i = 0; i < size; i++) + assert_int_equal(mapping[i], 0xa5); + memset(mapping, 0x5a, size); + for (i = 0; i < size; i++) + assert_int_equal(backing[i], 0x5a); + assert_int_equal(rdev_munmap(&mem.rdev, mapping), 0); + + /* Test read/write/erase of single bytes. */ + for (i = 0; i < size; i++) { + u8 val = i + 0xaa; + scratch[0] = val; + assert_int_equal(rdev_writeat(&mem.rdev, &scratch, i, 1), 1); + assert_int_equal(backing[i], val); + assert_int_equal(scratch[0], val); + val = i + 0x55; + backing[i] = val; + assert_int_equal(rdev_readat(&mem.rdev, &scratch, i, 1), 1); + assert_int_equal(scratch[0], val); + assert_int_equal(backing[i], val); + assert_int_equal(rdev_eraseat(&mem.rdev, i, 1), 1); + assert_int_equal(backing[i], 0); + } + + /* Test read/write/erase of larger chunk. */ + size_t offs = 0x47; + size_t chunk = 0x72; + memset(backing, 0, size); + memset(scratch, 0, size); + memset(scratch + offs, 0x39, chunk); + assert_int_equal(rdev_writeat(&mem.rdev, scratch + offs, offs, chunk), chunk); + assert_memory_equal(backing, scratch, size); + memset(backing, 0, size); + assert_int_equal(rdev_readat(&mem.rdev, scratch + offs, offs, chunk), chunk); + assert_memory_equal(backing, scratch, size); + memset(scratch + offs + 1, 0, chunk - 1); + assert_int_equal(rdev_eraseat(&mem.rdev, offs + 1, chunk - 1), chunk - 1); + assert_memory_equal(backing, scratch, size); + + /* Test mapping of larger chunk. */ + memset(backing, 0, size); + mapping = rdev_mmap(&mem.rdev, offs, chunk); + assert_non_null(mapping); + memset(scratch, 0x93, size); + memcpy(mapping, scratch, chunk); + memset(scratch, 0, size); + memset(scratch + offs, 0x93, chunk); + assert_memory_equal(backing, scratch, size); + assert_int_equal(rdev_munmap(&mem.rdev, mapping), 0); + assert_memory_equal(backing, scratch, size); +} + +int main(void) +{ + const struct CMUnitTest tests[] = { + cmocka_unit_test(test_region), + cmocka_unit_test(test_rdev_basics), + cmocka_unit_test(test_rdev_success), + cmocka_unit_test(test_rdev_failure), + cmocka_unit_test(test_rdev_wrap), + cmocka_unit_test(test_rdev_chain), + cmocka_unit_test(test_rdev_double_chain), + cmocka_unit_test(test_mem_rdev), + }; + + return cmocka_run_group_tests(tests, NULL, NULL); +}
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41046 )
Change subject: tests: Add region-test for rdev API ......................................................................
Patch Set 9:
Automatic boot test returned (PASS/FAIL/TOTAL): 4/0/4 Emulation targets: "QEMU x86 q35/ich9" using payload TianoCore : SUCCESS : https://lava.9esec.io/r/3544 "QEMU x86 q35/ich9" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3543 "QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3542 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/3541
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Aaron Durbin (Code Review)
-
build bot (Jenkins) (Code Review)
-
Jan Dabros (Code Review)
-
Julius Werner (Code Review)
-
Paul Menzel (Code Review)