Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Virtual Memory Heaps boot testing improvements #8775

Merged
merged 3 commits into from
Mar 5, 2024
Merged

Virtual Memory Heaps boot testing improvements #8775

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ce824b5
boot_test: change call of boot tests
dabekjakub Jan 23, 2024
13ba1b7
vmh_tests: remove current vmh test implementation
dabekjakub Jan 23, 2024
64bcd75
vmh_tests: Add set of parametrized tests and test suit
dabekjakub Jan 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 0 additions & 10 deletions src/ipc/ipc4/handler.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1539,14 +1539,4 @@ void ipc_cmd(struct ipc_cmd_hdr *_hdr)

ipc4_send_reply(&reply);
}

#if CONFIG_SOF_BOOT_TEST
/*
* When the first FW_GEN IPC has been processed we are in a stable
* running state, now if a test causes an exception, we have a good
* chance of capturing it.
*/
if (target == SOF_IPC4_MESSAGE_TARGET_FW_GEN_MSG)
TEST_RUN_ONCE(ztest_run_test_suite, sof_boot);
#endif
}
8 changes: 8 additions & 0 deletions zephyr/boot_test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,17 @@
*/

#include <zephyr/logging/log.h>
#include <sof/boot_test.h>

#include <zephyr/ztest.h>

LOG_MODULE_REGISTER(sof_boot_test, LOG_LEVEL_DBG);

ZTEST_SUITE(sof_boot, NULL, NULL, NULL, NULL, NULL);

void sys_run_boot_tests(void)
{
ztest_run_all(NULL, false, 1, 1);
}
SYS_INIT(sys_run_boot_tests, APPLICATION, 99);

289 changes: 236 additions & 53 deletions zephyr/test/vmh.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include <errno.h>
#include <stdbool.h>
#include <stdlib.h>

#include <adsp_memory_regions.h>
#include <sof/boot_test.h>
Expand All @@ -17,81 +18,263 @@

LOG_MODULE_DECLARE(sof_boot_test, CONFIG_SOF_LOG_LEVEL);

#define ALLOC_SIZE1 1616
#define ALLOC_SIZE2 26
/* Test creating and freeing a virtual memory heap */
static void test_vmh_init_and_free_heap(int memory_region_attribute,
struct vmh_heap_config *config,
int core_id,
bool allocating_continuously,
bool expect_success)
{
struct vmh_heap *heap = vmh_init_heap(config, memory_region_attribute,
core_id, allocating_continuously);
if (expect_success) {
zassert_not_null(heap,
"Heap initialization expected to succeed but failed");
}
else
zassert_is_null(heap, "Heap initialization expected to fail but succeeded");

if (heap) {
int ret = vmh_free_heap(heap);

zassert_equal(ret, 0, "Failed to free heap");
}
}
dabekjakub marked this conversation as resolved.
Show resolved Hide resolved

static int vmh_test_single(bool span)
/* Test for vmh_alloc and vmh_free */
static void test_vmh_alloc_free_no_check(struct vmh_heap *heap,
uint32_t alloc_size,
bool expect_success)
{
struct vmh_heap *h = vmh_init_heap(NULL, MEM_REG_ATTR_CORE_HEAP, 0, span);

if (!h)
return -EINVAL;

char *buf = vmh_alloc(h, ALLOC_SIZE1);
int ret1;

if (buf) {
buf[0] = 0;
buf[ALLOC_SIZE1 - 1] = 15;

ret1 = vmh_free(h, buf);
if (ret1 < 0)
goto out;
} else if (span) {
ret1 = -ENOMEM;
LOG_ERR("Failed to allocate %u in contiguous mode", ALLOC_SIZE1);
goto out;
} else {
LOG_WRN("Ignoring failure to allocate %u in non-contiguous mode",
ALLOC_SIZE1);
void *ptr = vmh_alloc(heap, alloc_size);

if (expect_success)
zassert_not_null(ptr, "Allocation expected to succeed but failed");
else
zassert_is_null(ptr, "Allocation expected to fail but succeeded");

if (ptr) {
int ret = vmh_free(heap, ptr);

zassert_equal(ret, 0, "Failed to free allocated memory");
}
}

static void verify_memory_content(void *ptr, uint32_t alloc_size)
{

/* Calculate check positions end and middle if applicable */
uint8_t *end_ptr = (uint8_t *)ptr + alloc_size - sizeof(uint32_t);
uint8_t *middle_ptr = (uint8_t *)ptr + (alloc_size / 2);
uint8_t test_value = 0xAA;
int test_write_size = 1;

buf = vmh_alloc(h, ALLOC_SIZE2);
/* Write test pattern to the allocated memory beginning middle and end */
memset(ptr, test_value, test_write_size);
memset(middle_ptr, test_value, test_write_size);
memset(end_ptr, test_value, test_write_size);

/* Verify the written test pattern at all points */
zassert_equal(*((uint8_t *)ptr), test_value,
"Memory content verification failed at the start");
zassert_equal(*end_ptr, test_value,
"Memory content verification failed at the end");
zassert_equal(*middle_ptr, test_value,
"Memory content verification failed in the middle");
}

/* Test function for vmh_alloc and vmh_free with memory read/write */
static void test_vmh_alloc_free_check(struct vmh_heap *heap,
uint32_t alloc_size,
bool expect_success)
{
void *ptr = vmh_alloc(heap, alloc_size);

if (!buf) {
ret1 = -ENOMEM;
LOG_ERR("Failed to allocate %u", ALLOC_SIZE2);
goto out;
if (expect_success)
zassert_not_null(ptr, "Allocation expected to succeed but failed");
else {
zassert_is_null(ptr, "Allocation expected to fail but succeeded");
return;
}

buf[0] = 0;
buf[ALLOC_SIZE2 - 1] = 15;
if (ptr)
verify_memory_content(ptr, alloc_size);

ret1 = vmh_free(h, buf);
if (ret1 < 0)
LOG_ERR("Free error %d", ret1);
int ret = vmh_free(heap, ptr);

out:
int ret2 = vmh_free_heap(h);
zassert_equal(ret, 0, "Failed to free allocated memory");
}

/* Test function for multiple allocations on the same heap with read/write */
static void test_vmh_multiple_allocs(struct vmh_heap *heap, int num_allocs,
uint32_t min_alloc_size,
uint32_t max_alloc_size)
{
void *ptrs[num_allocs];
uint32_t alloc_size;
bool success;
int ret;

/* Perform multiple allocations */
for (int i = 0; i < num_allocs; i++) {
/* Generate a random allocation size between min_alloc_size and max_alloc_size */
alloc_size = min_alloc_size +
k_cycle_get_32() % (max_alloc_size - min_alloc_size + 1);

if (ret2 < 0)
LOG_ERR("Free heap error %d", ret2);
ptrs[i] = vmh_alloc(heap, alloc_size);

if (!ret1)
ret1 = ret2;
if (!ptrs[i])
LOG_INF("Test allocation failed for size: %d", alloc_size);

return ret1;
zassert_true(ptrs[i] != NULL,
"Allocation of size %u expected to succeed but failed",
alloc_size);

if (ptrs[i])
verify_memory_content(ptrs[i], alloc_size);
}

for (int i = 0; i < num_allocs; i++) {
if (ptrs[i]) {
ret = vmh_free(heap, ptrs[i]);
zassert_equal(ret, 0, "Failed to free allocated memory");
}
}
}

static int vmh_test(void)
/* Test case for multiple allocations */
static void test_vmh_alloc_multiple_times(bool allocating_continuously)
{
int ret = vmh_test_single(false);
struct vmh_heap *heap =
vmh_init_heap(NULL, MEM_REG_ATTR_CORE_HEAP, 0, allocating_continuously);

if (ret < 0) {
LOG_ERR("Non-contiguous test error %d", ret);
return ret;
zassert_not_null(heap, "Heap initialization failed");

/* Test multiple allocations with small sizes */
test_vmh_multiple_allocs(heap, 16, 4, 8);
test_vmh_multiple_allocs(heap, 64, 4, 8);
test_vmh_multiple_allocs(heap, 16, 4, 1024);
test_vmh_multiple_allocs(heap, 64, 4, 1024);
if (allocating_continuously) {
test_vmh_multiple_allocs(heap, 16, 1024, 4096);
test_vmh_multiple_allocs(heap, 16, 4096, 8192);
}

ret = vmh_test_single(true);
if (ret < 0)
LOG_ERR("Contiguous test error %d", ret);
/* Clean up the heap after testing */
int ret = vmh_free_heap(heap);

zassert_equal(ret, 0, "Failed to free heap after multiple allocations");
}

/* Test case for vmh_alloc and vmh_free */
static void test_vmh_alloc_free(bool allocating_continuously)
{
struct vmh_heap *heap =
vmh_init_heap(NULL, MEM_REG_ATTR_CORE_HEAP, 0, allocating_continuously);

zassert_not_null(heap, "Heap initialization failed");

test_vmh_alloc_free_no_check(heap, 512, true);
test_vmh_alloc_free_no_check(heap, 1024, true);
test_vmh_alloc_free_no_check(heap, sizeof(int), true);
test_vmh_alloc_free_no_check(heap, 0, false);

test_vmh_alloc_free_check(heap, 512, true);
test_vmh_alloc_free_check(heap, 1024, true);
test_vmh_alloc_free_check(heap, sizeof(int), true);
test_vmh_alloc_free_check(heap, 0, false);

return ret;
int ret = vmh_free_heap(heap);

zassert_equal(ret, 0, "Failed to free heap");

/* Could add tests with configs for heaps*/
}

/* Test case for vmh_alloc and vmh_free with and without config */
static void test_heap_creation(void)
{
test_vmh_init_and_free_heap(MEM_REG_ATTR_CORE_HEAP, NULL, 0, false, true);

/* Try to setup with pre defined heap config */
struct vmh_heap_config config = {0};

config.block_bundles_table[0].block_size = 8;

config.block_bundles_table[0].number_of_blocks = 1024;

config.block_bundles_table[1].block_size = 16;

config.block_bundles_table[1].number_of_blocks = 512;

test_vmh_init_and_free_heap(MEM_REG_ATTR_CORE_HEAP, &config, 0, false, true);
}

/* Test case for alloc/free on configured heap */
static void test_alloc_on_configured_heap(bool allocating_continuously)
{

/* Try to setup with pre defined heap config */
struct vmh_heap_config config = {0};

config.block_bundles_table[0].block_size = 32;

config.block_bundles_table[0].number_of_blocks = 256;

/* Create continuous allocation heap for success test */
struct vmh_heap *heap =
vmh_init_heap(&config, MEM_REG_ATTR_CORE_HEAP, 0, allocating_continuously);

/* Will succeed on continuous and fail with single block alloc */
test_vmh_alloc_free_check(heap, 512, allocating_continuously);

int ret = vmh_free_heap(heap);

zassert_equal(ret, 0, "Failed to free heap");
}

/* Test cases for initializing heaps on all available regions */
static void test_vmh_init_all_heaps(void)
{
int num_regions = CONFIG_MP_MAX_NUM_CPUS + VIRTUAL_REGION_COUNT;
int i;
const struct sys_mm_drv_region *virtual_memory_region =
sys_mm_drv_query_memory_regions();

/* Test initializing all types of heaps */
for (i = 0; i < num_regions; i++) {

/* Zeroed size symbolizes end of regions table */
if (!virtual_memory_region[i].size)
break;

struct vmh_heap *heap = vmh_init_heap(NULL, virtual_memory_region[i].attr,
i, true);

zassert_not_null(heap, "Heap initialization expected to succeed but failed");

/* Test if it fails when heap already exists */
test_vmh_init_and_free_heap(virtual_memory_region[i].attr, NULL, i, true,
false);
dabekjakub marked this conversation as resolved.
Show resolved Hide resolved

if (heap) {
int ret = vmh_free_heap(heap);

zassert_equal(ret, 0, "Failed to free heap");
}
}
}

ZTEST(sof_boot, virtual_memory_heap)
{
int ret = vmh_test();
test_heap_creation();
test_vmh_init_all_heaps();
test_alloc_on_configured_heap(true);
test_alloc_on_configured_heap(false);
test_vmh_alloc_free(true);
test_vmh_alloc_free(false);
test_vmh_alloc_multiple_times(true);
test_vmh_alloc_multiple_times(false);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is quite a bit of testing, taking into account that many of these functions test multiple allocations too. Do we know how long this takes? I'm not sure we need that much testing - I'd basically test each (or most) configuration once - once in contiguous mode, once without, once with success, once with failure. Something rather lightweight for regular debug-overlay testing, possibly add a new Kconfig for extensive VMH testing. So do all these tests now pass? Can we re-enable boot-time testing by default?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall I assumed that this boot test feature is not intended to be enabled in the productized version. So we will only use the CONFIG_SOF_BOOT_TEST flag for testing only and are not that much impacted by test time.

However, for allocator testing, this is quite concise. Should not take that much time - will have better data on it as soon as I clean up all the issues those tests found.

Amount of tests: This test approach allows for clear identification of the issues. I tried to add tests for each case I encountered. For example, multiple allocations failed in my internal testing during development so now I want it tested extensively. Writing data to the allocated space also checks for correctly assigned physical pages.

Can we enable boot testing? No - I am still working out the issues in vmh API #8772 Once this is done It can be pushed.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good, that this helps you find issues with the VMH! Maybe we should first have all the issues fixed and then merge and enable this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am working on it so I see no problem In pushing both fixes and this at the same time.
Linked pr is for fixes I will do.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not matter how long the tests take to run as they should not be used in a production mode. i.e. we are not going to add latency to audio use case.
CI should be able to build this Kconfig and all tests should run for all subsystems.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to enable this in debug overlay, so this would get tested automatically with the current infra we have. That would require the tests to pass. Otherwise the code will very easily get broken (like it has when we temporarily disabled it in the debug overlay).


TEST_CHECK_RET(ret, "virtual_memory_heap");
TEST_CHECK_RET(true, "virtual_memory_heap");
}
Loading