uk/plat/memory: Introduce pg_off and pg_count memregion fields
#1212
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This proposed improvement has been discussed during the Arm CCA GSoC track to make debugging memory region setup on newer, not yet supported, platforms easier. |
razvand
requested review from
razvanvirtan
and removed request for
michpappas
mogasergiu
force-pushed
the
smoga/mrd_pgoff_numpg_rebased
branch
from
f358334
to
d66f042
Compare
github-actions
bot
added
area/include
mogasergiu
force-pushed
the
smoga/mrd_pgoff_numpg_rebased
branch
2 times, most recently
from
5646902
to
61a9eb5
Compare
michpappas
pushed a commit
to michpappas/unikraft
that referenced
this pull request
Apr 1, 2024
PR unikraft#1212 Signed-off-by: Sergiu Moga <sergiu@unikraft.io>
4 tasks
mogasergiu
force-pushed
the
smoga/mrd_pgoff_numpg_rebased
branch
from
61a9eb5
to
f57d8b2
Compare
michpappas
pushed a commit
to michpappas/unikraft
that referenced
this pull request
Apr 28, 2024
Signed-off-by: Sergiu Moga <sergiu@unikraft.io>
mogasergiu
force-pushed
the
smoga/mrd_pgoff_numpg_rebased
branch
from
f57d8b2
to
68cc84c
Compare
michpappas
pushed a commit
to michpappas/unikraft
that referenced
this pull request
Apr 29, 2024
Introduce `pg_off` and `pg_count` memregion fields Signed-off-by: Sergiu Moga <sergiu@unikraft.io>
michpappas
requested changes
May 7, 2024
There was a problem hiding this comment.
@mogasergiu this is a nice changeset. Besides the incline comments, a few grammar issues I spotted in the commit message of the 2nd commit.
As a basis, MRD's must
MRDs (or mrds to be consistent with the rest)
validation macro's were added.
macros
addition of these macro's,
macros
criteria's
critieria
we use meets the criterias that all free memory regions must have
criteria
of
ukplat_memregion_list_coalesceagainst the general criterias
criteria
criterias that must be met by all memory region descriptors.
criteria
Properly validate mrd's
mrds
mogasergiu
commented
May 13, 2024
To make memory region management easier w.r.t. alignment handling, define two additional fields for `struct ukplat_memregion_desc`: - `pg_off` to represent the in-page offset from where the actual resource this memory region is dedicated to starts - `pg_count` to represent the length of the entire, end-to-end page-aligned, memory region in number of pages Thus, the definition of some other fields shall then change: - `pbase` will be the physical page-aligned base address of the region. This means that in order to get the actual address of a resource, one may have to make the following basic addition: `pbase` + `pg_off` - `vbase` same as `pbase` but for virtual base address - `len` will now represent the length of the resource inside the region, not the length of the region. E.g. For a resource with address `0x1050` and length `0x430` the corresponding memory region descriptor will have the following values: - `pbase` and `vbase` equal to `0x1000` (`PAGE_ALIGN_DOWN(0x1050)`) - `pg_off` equal to `0x50` (`0x1050 & ~PAGE_MASK`) - `pg_count` equal to `5` (`PAGE_COUNT(0x1050 + 0x430)`) - `len` equal to `0x430` The other fields (`type`, `flags`, `name`) will keep their meaning. Now with the new structure, make sure that every call site to `ukplat_memregion_list_insert` also initializes `pg_off` and `pg_count` accordingly. Most importantly, deprecate the manual alignment and restoration of memory regions during coalescing, as it is not longer needed. The newly introduced fields guarantee that `pbase` and `pg_count` combined will always yield end-to-end aligned memory regions. In the case of printing, make memory map printing functionality show two sets of address ranges for each memory region descriptor: one for the page-aligned start and end of the memory region and one for the real, potentially misaligned, start and end addresses of the memory region descriptor (the actual start/end addresses of the resource the region is meant to map). Signed-off-by: Sergiu Moga <sergiu@unikraft.io> s into prev s into prev
Implement a memory region type specific spin-off of `UK_ASSERT` that will throw assertions specific to the type of the memory region being asserted As a basis, MRDs must meet the following, generic, criteria: - must be of only one valid type as per `UK_ASSERT_VALID_MRD_TYPE` - must only have valid flags as per `UK_ASSERT_VALID_MRD_FLAGS` - memory region is not empty or of length 0 - virtual/physical base addresses are page-aligned - resource in-page offset must be in the range [0, PAGE_SIZE) For now, two additional specialized MRD validation macros were added. In the case of `UK_ASSERT_VALID_FREE_MRD`, they must meet the following criteria: - must meet the criteria of a general valid memory region descriptor - virtual/physical base addresses are equal - region is aligned end-to-end, therefore length is multiple of PAGE_SIZE times region's page count and the resource's in-page offset must be 0 In the case of `UK_ASSERT_VALID_KERNEL_MRD`, they must meet the - must meet the criteria of a general valid memory region descriptor - region is aligned end-to-end, therefore length is multiple of PAGE_SIZE times region's page count and the resource's in-page offset must be 0 That being said, with the addition of these macros, sprinkle around some mrd validations. After successfully fetching the `initrd` memory region, use `UK_ASSERT_VALID_MRD to validate it against a predefined set of criteria that generally apply to all memory regions, regardless of type. `ukplat_memregion_alloc` makes use of free memory regions in order to break a chunk out of them and assign it a given type depending on the requested allocation. Make sure that the free memory region we use meets the criteria that all free memory regions must have by using `UK_ASSERT_VALID_FREE_MRD` before modifying it. Validate all memory regions that we process during the execution of `ukplat_memregion_list_coalesce` against the general criteria that must be met by all memory region descriptors. When not using `HAVE_PAGING`, we go through each memory region in the list and drop those that are outside the static boot page tables since we can't extend it. Thus, make sure we also make sure that these memory region descriptors are valid w.r.t. the general criteria that must be met by all memory region descriptors. Properly validate mrds during `ukplat_paging_init` such that: - for the first loop that adds free memory region descriptors to the frame allocator, we validate with `UK_ASSERT_VALID_FREE_MRD` - for the second loop that unmaps according to `UKPLAT_MEMRF_UNMAP` memory region descriptors, we have a one use-case, very particular subset of conditions that must be met by this kind of memory regions only: - must be non-empty and aligned end-to-end, i.e. length must be !0 and the page count times PAGE_SIZE must be equal to length - virtual base address must be page-aligned and a valid value - physical base address must be 0 - page offset must be 0 - for the last loop, the one that processes `UKPLAT_MEMRF_MAP` memory region descriptors, we enforce the general rules, i.e. use `UK_ASSERT_VALID_MRD` and, furthermore, we also ensure that we not somehow map free memory regions. Lastly, when enforcing `W^X` we go through memory regions and get permissions from there. Ensure that these processed memory region descriptors are valid w.r.t. the predefined criteria that must be met by all memory region descriptors. NOTE: This is an initial attempt at validating as many directly referenced memory region descriptors as possible. It is not exhaustive and does not guarantee validating every single possible reference. Signed-off-by: Sergiu Moga <sergiu@unikraft.io>
mogasergiu
force-pushed
the
smoga/mrd_pgoff_numpg_rebased
branch
from
68cc84c
to
c5c34fd
Compare
michpappas
approved these changes
May 14, 2024
There was a problem hiding this comment.
Thanks!
Reviewed-by: Michalis Pappas michalis@unikraft.io
Approved-by: Michalis Pappas mpappas@fastmail.fm
razvanvirtan
approved these changes
May 15, 2024
There was a problem hiding this comment.
Thank you, @mogasergiu!
Reviewed-by: Razvan Virtan virtanrazvan@gmail.com
razvand
pushed a commit
that referenced
this pull request
May 15, 2024
To make memory region management easier w.r.t. alignment handling, define two additional fields for `struct ukplat_memregion_desc`: - `pg_off` to represent the in-page offset from where the actual resource this memory region is dedicated to starts - `pg_count` to represent the length of the entire, end-to-end page-aligned, memory region in number of pages Thus, the definition of some other fields shall then change: - `pbase` will be the physical page-aligned base address of the region. This means that in order to get the actual address of a resource, one may have to make the following basic addition: `pbase` + `pg_off` - `vbase` same as `pbase` but for virtual base address - `len` will now represent the length of the resource inside the region, not the length of the region. E.g. For a resource with address `0x1050` and length `0x430` the corresponding memory region descriptor will have the following values: - `pbase` and `vbase` equal to `0x1000` (`PAGE_ALIGN_DOWN(0x1050)`) - `pg_off` equal to `0x50` (`0x1050 & ~PAGE_MASK`) - `pg_count` equal to `5` (`PAGE_COUNT(0x1050 + 0x430)`) - `len` equal to `0x430` The other fields (`type`, `flags`, `name`) will keep their meaning. Now with the new structure, make sure that every call site to `ukplat_memregion_list_insert` also initializes `pg_off` and `pg_count` accordingly. Most importantly, deprecate the manual alignment and restoration of memory regions during coalescing, as it is not longer needed. The newly introduced fields guarantee that `pbase` and `pg_count` combined will always yield end-to-end aligned memory regions. In the case of printing, make memory map printing functionality show two sets of address ranges for each memory region descriptor: one for the page-aligned start and end of the memory region and one for the real, potentially misaligned, start and end addresses of the memory region descriptor (the actual start/end addresses of the resource the region is meant to map). s into prev s into prev Signed-off-by: Sergiu Moga <sergiu@unikraft.io> Approved-by: Michalis Pappas <mpappas@fastmail.fm> Reviewed-by: Michalis Pappas <michalis@unikraft.io> Reviewed-by: Razvan Virtan <virtanrazvan@gmail.com> GitHub-Closes: #1212
razvand
pushed a commit
that referenced
this pull request
May 15, 2024
Implement a memory region type specific spin-off of `UK_ASSERT` that will throw assertions specific to the type of the memory region being asserted As a basis, MRDs must meet the following, generic, criteria: - must be of only one valid type as per `UK_ASSERT_VALID_MRD_TYPE` - must only have valid flags as per `UK_ASSERT_VALID_MRD_FLAGS` - memory region is not empty or of length 0 - virtual/physical base addresses are page-aligned - resource in-page offset must be in the range [0, PAGE_SIZE) For now, two additional specialized MRD validation macros were added. In the case of `UK_ASSERT_VALID_FREE_MRD`, they must meet the following criteria: - must meet the criteria of a general valid memory region descriptor - virtual/physical base addresses are equal - region is aligned end-to-end, therefore length is multiple of PAGE_SIZE times region's page count and the resource's in-page offset must be 0 In the case of `UK_ASSERT_VALID_KERNEL_MRD`, they must meet the - must meet the criteria of a general valid memory region descriptor - region is aligned end-to-end, therefore length is multiple of PAGE_SIZE times region's page count and the resource's in-page offset must be 0 That being said, with the addition of these macros, sprinkle around some mrd validations. After successfully fetching the `initrd` memory region, use `UK_ASSERT_VALID_MRD to validate it against a predefined set of criteria that generally apply to all memory regions, regardless of type. `ukplat_memregion_alloc` makes use of free memory regions in order to break a chunk out of them and assign it a given type depending on the requested allocation. Make sure that the free memory region we use meets the criteria that all free memory regions must have by using `UK_ASSERT_VALID_FREE_MRD` before modifying it. Validate all memory regions that we process during the execution of `ukplat_memregion_list_coalesce` against the general criteria that must be met by all memory region descriptors. When not using `HAVE_PAGING`, we go through each memory region in the list and drop those that are outside the static boot page tables since we can't extend it. Thus, make sure we also make sure that these memory region descriptors are valid w.r.t. the general criteria that must be met by all memory region descriptors. Properly validate mrds during `ukplat_paging_init` such that: - for the first loop that adds free memory region descriptors to the frame allocator, we validate with `UK_ASSERT_VALID_FREE_MRD` - for the second loop that unmaps according to `UKPLAT_MEMRF_UNMAP` memory region descriptors, we have a one use-case, very particular subset of conditions that must be met by this kind of memory regions only: - must be non-empty and aligned end-to-end, i.e. length must be !0 and the page count times PAGE_SIZE must be equal to length - virtual base address must be page-aligned and a valid value - physical base address must be 0 - page offset must be 0 - for the last loop, the one that processes `UKPLAT_MEMRF_MAP` memory region descriptors, we enforce the general rules, i.e. use `UK_ASSERT_VALID_MRD` and, furthermore, we also ensure that we not somehow map free memory regions. Lastly, when enforcing `W^X` we go through memory regions and get permissions from there. Ensure that these processed memory region descriptors are valid w.r.t. the predefined criteria that must be met by all memory region descriptors. NOTE: This is an initial attempt at validating as many directly referenced memory region descriptors as possible. It is not exhaustive and does not guarantee validating every single possible reference. Signed-off-by: Sergiu Moga <sergiu@unikraft.io> Approved-by: Michalis Pappas <mpappas@fastmail.fm> Reviewed-by: Michalis Pappas <michalis@unikraft.io> Reviewed-by: Razvan Virtan <virtanrazvan@gmail.com> GitHub-Closes: #1212
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prerequisite checklist
checkpatch.ukon your commit series before opening this PR;Base target
Additional configuration
Description of changes
To make memory region management easier w.r.t. alignment handling,
define two additional fields for
struct ukplat_memregion_desc:pg_offto represent the in-page offset from where the actualresource this memory region is dedicated to starts
pg_countto represent the length of the entire, end-to-endpage-aligned, memory region in number of pages
Thus, the definition of some other fields shall then change:
pbasewill be the physical page-aligned base address of theregion. This means that in order to get the actual address of a
resource, one may have to make the following basic addition:
pbase+pg_offvbasesame aspbasebut for virtual base addresslenwill now represent the length of the resource inside theregion, not the length of the region.
E.g.
For a resource with address
0x1050and length0x430thecorresponding memory region descriptor will have the following
values:
pbaseandvbaseequal to0x1000(PAGE_ALIGN_DOWN(0x1050))pg_offequal to0x50(0x1050 & ~PAGE_MASK)pg_countequal to5(PAGE_COUNT(0x1050 + 0x430))lenequal to0x430The other fields (
type,flags,name) will keep their meaning.