ARM64 arch_mem_map() Overwrite Enable #76892
Replies: 6 comments 7 replies
-
|
On Fri, 9 Aug 2024, quic-chadk wrote:
Let me give an example. We initially configure a 2GB region as
cached. Address 0x80000000 to 0xBFFFFFFF. This gets mapped using a
single 2GB page table (but as the caller I am not aware of the page
table used). Later during execution I need to add a 4MB uncached
region inside of that 2GB region. Lets say address 0x90000000 to
0x903FFFFF. I have to call unmap with the address 0x90000000 and size
of 0x00400000. If I don't call unmap and just call map, I will get an
error because overwrite is hard coded disabled and the memory has
already been mapped. So I call unmap. This actually unmaps all 2GB
from 0x80000000 to 0xBFFFFFFF because that is the page table that
covered 0x90000000. To retain 0x80000000 to 0x8FFFFFFF and 0x90400000
to 0xBFFFFFFF I would have to know a 2GB page table was used to map
0x90000000 and make two more map calls to add the leading and trailing
memory back. I don't have that detailed information.
You should be able to simply unmap your 4MB region and remap it
uncached. If that doesn't work it needs fixing.
|
Beta Was this translation helpful? Give feedback.
-
|
That is not the behavior I am seeing. It seems once the page table that covers the region is found, not matter how much extra is being covered, that page table as a whole is unmapped. So to clarify, you are saying the behavior of unmap should be as follows:
If you agree this is the expected behavior I will debug unmap and post a PR with proposed corrections needed. Thank you. |
Beta Was this translation helpful? Give feedback.
-
|
No, your proposal is way too complicated.
If you unmap a range in the middle of a larger mapped area then it
should simply punch a hole in that area. The relevant code is in
`del_mapping()` where incidentally there is an assertion and a comment
that says: "We assume that block mappings will be unmapped as a whole
and not partially." In other words, it is not currently supported. The
same logic as found in `set_mapping()` should be applied i.e. invoke
`expand_to_table()` to break a block mapping into smaller parts until
the range to be deleted would fit.
|
Beta Was this translation helpful? Give feedback.
-
|
I see, thanks for finding the reason why it isn't currently working and explanation of what needs to be added to make it work. I'll add what you suggested, test, and post a PR. Thanks again. |
Beta Was this translation helpful? Give feedback.
-
|
Please consider PR #76983.
|
Beta Was this translation helpful? Give feedback.
-
|
That PR worked perfectly, thank you very much for all the help. I'll keep an eye on the PR for it to be mainlined. Thanks again! |
Beta Was this translation helpful? Give feedback.
-
|
Please could you provide your approval on the PR to help its merging?
|
Beta Was this translation helpful? Give feedback.
-
|
Sorry, I didn't know I had the ability to approve. Thought I had to be in a special group. I just approved. Thank you again. |
Beta Was this translation helpful? Give feedback.
-
|
@npitre, seems the review needs approval from two people with write access. None of the three people with write access are responding to asks to review. What is the next step forward to get the fix mainlined? |
Beta Was this translation helpful? Give feedback.
-
|
@nashif ^^ |
Beta Was this translation helpful? Give feedback.
-
|
need an approval from @carlocaione :) |
Beta Was this translation helpful? Give feedback.
-
|
done |
Beta Was this translation helpful? Give feedback.
-
|
Hello @nashif, now that Carlo has approved would it be possible for you to give the second approval so this can be mainlined? |
Beta Was this translation helpful? Give feedback.
-
Hello @npitre,
I had a question about commit bcef633
We are using Zephyr as a bootloader and have a scenario where we initially map a large region of DDR as cached and then need to mark smaller regions inside of this large region as uncached or with different attributes as execution continues. With the commit I just listed it is difficult to add these smaller regions as the overwrite option is hard coded disabled. The difficulty comes from having to call unmap on the region you want to modify and not having knowledge of what page table size was used to do the mapping. That smaller region could be part of a large region covered by a large single page table and more memory gets unmapped than you really want as that whole single page table is unmapped.
Let me give an example. We initially configure a 2GB region as cached. Address 0x80000000 to 0xBFFFFFFF. This gets mapped using a single 2GB page table (but as the caller I am not aware of the page table used). Later during execution I need to add a 4MB uncached region inside of that 2GB region. Lets say address 0x90000000 to 0x903FFFFF. I have to call unmap with the address 0x90000000 and size of 0x00400000. If I don't call unmap and just call map, I will get an error because overwrite is hard coded disabled and the memory has already been mapped. So I call unmap. This actually unmaps all 2GB from 0x80000000 to 0xBFFFFFFF because that is the page table that covered 0x90000000. To retain 0x80000000 to 0x8FFFFFFF and 0x90400000 to 0xBFFFFFFF I would have to know a 2GB page table was used to map 0x90000000 and make two more map calls to add the leading and trailing memory back. I don't have that detailed information.
As you can see the overwrite feature is useful. Would you object to overwrite not being hard coded or via the flags argument passed to arch_mem_map have a way to re-enable overwrite?
Beta Was this translation helpful? Give feedback.
All reactions