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.

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

regions_mm: New memory mapping functions #9114

Merged
merged 2 commits into from
May 24, 2024

Conversation

softwarecki
Copy link
Collaborator

@softwarecki softwarecki commented May 10, 2024

The previous memory mapping function did not work properly if the buffer size exceeded the memory page size. In this case the size of the region to be checked passed to the sys_bitarray_is_region_cleared function was zero, causing the function to always return false. As a result, the allocator stated that a page was already mapped for a given address and did not map it. This led to a cpu exception when trying to access the allocated buffer.

The function responsible for unmapping memory had a similar problem. Additionally, the size of the freed area was incorrectly determined and an incorrect offset was passed to the sys_bitarray_is_region_cleared function.

New functions have been created to map and unmap memory pages for allocated buffers. It don't need allocation of temporary array and manipulation of memblocks bitarray.

Used static initialization of the vmh_list list head using macro.

A macro was used to statically initialize the vmh_list list head. This
allowed to resign from calling a function whose only task was to initialize
the list header. This function was removed as no needed anymore.

Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
@softwarecki softwarecki force-pushed the vmh-fix branch 3 times, most recently from addb4dc to a5e2397 Compare May 10, 2024 15:27
@softwarecki softwarecki changed the title regions_mm: New memory mapping functions + tests regions_mm: New memory mapping functions May 10, 2024
@softwarecki softwarecki marked this pull request as ready for review May 10, 2024 15:30
zephyr/lib/regions_mm.c Outdated Show resolved Hide resolved
zephyr/lib/regions_mm.c Outdated Show resolved Hide resolved
Copy link
Member

@dabekjakub dabekjakub left a comment

Choose a reason for hiding this comment

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

Changes look good I would like more commentary since this is complex code.

zephyr/lib/regions_mm.c Show resolved Hide resolved
Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

LGTM, @softwarecki do you think we will need to update the vmh ztests or are the existing ztests providing coverage for this update ?

@softwarecki
Copy link
Collaborator Author

@lgirdwood There is no need to extend vmh ztests. This PR does not add any new functionality, it only fixes bugs. Before merge let my apply @lyakh comments.

The previous memory mapping function did not work properly if the buffer
size exceeded the memory page size. In this case the size of the region to
be checked passed to the sys_bitarray_is_region_cleared function was zero,
causing the function to always return false. As a result, the allocator
stated that a page was already mapped for a given address and did not map
it. This led to a cpu exception when trying to access the allocated buffer.

The function responsible for unmapping memory had a similar problem.
Additionally, the size of the freed area was incorrectly determined and
an incorrect offset was passed to the sys_bitarray_is_region_cleared
function.

New functions have been created to map and unmap memory pages for allocated
buffers. It don't need allocation of temporary array and manipulation of
memblocks bitarray.

Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
zephyr/lib/regions_mm.c Show resolved Hide resolved
zephyr/lib/regions_mm.c Show resolved Hide resolved
@softwarecki
Copy link
Collaborator Author

SOFCI TEST

@kv2019i
Copy link
Collaborator

kv2019i commented May 23, 2024

@dabekjakub good to go? This seems to be ready for merge now.

@kv2019i kv2019i merged commit 5081044 into thesofproject:main May 24, 2024
41 of 46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants