Copy edit uVisor documents #411
Merged
Conversation
This file contains hidden or 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
AnotherButler
commented
Feb 14, 2017
Contributor
Author
AnotherButler
left a comment
AnotherButler
left a comment
There was a problem hiding this comment.
I left several queries in the form of comments in addition to my copy edits. Please review my suggested changes to make sure I'm not accidentally changing content.
| | `ARMv7M_ALIGNMENR_BITS` | TODO | | ||
| | `ARMv7M_MPU_RESERVED_REGIONS` | TODO | | ||
| | `UVISOR_MAX_ACLS` | TODO | | ||
|
|
Contributor
Author
There was a problem hiding this comment.
Should all of these TODOs be here?
| </td> | ||
| <td> | ||
| It contains constant data in flash that describes the configuration of the secure boxes. It comprises the configuration tables (<code>.keep.uvisor.cfgtbl</code>) and the pointers to them (<code>.keep.uvisor.cfgtbl_ptr[_first]</code>), which are used by uVisor for box enumeration. | ||
| It contains constant data in flash that describes the configuration of the secure boxes. It comprises the configuration tables (<code>.keep.uvisor.cfgtbl</code>) and the pointers to them (<code>.keep.uvisor.cfgtbl_ptr[_first]</code>), which uVisor uses for box enumeration. |
Contributor
Author
There was a problem hiding this comment.
Does uvisor.secure make up the configuration tables and pointers to them, or do the configuration tables and pointers make up uvisor.secure?
| - Allocating a new stack also quickly exhausts the available pages when pages are large. You want large pages to avoid MPU fragmentation and slow downs due to excessive memory faults. | ||
| Likewise, using the process stack for thread stacks is also not feasible. This is because multiple threads cannot share the same stack if they are not guaranteed to stack and unstack in a consistent order (first in, first out). Consider the situation where a thread performs a secure gateway to another process, and then yields and gets swapped out by the RTOS, saving its registers on the process stack. Next, a different thread performs a secure gateway to this same process, and it also yields and gets swapped out. Now, the RTOS wants to swap back in the original thread. The RTOS uses the stack pointer for that thread and the registers are restored without any issue. The original thread then does a stack allocation that blaps over the saved state of the second thread, and the original thread then yields and gets swapped out. Finally, the RTOS wants to swap back in the second thread. It uses the stack pointer for the second thread, but this pointer is now pointed to some "garbage" data leftover from the first thread's stack allocation. | ||
| Likewise, using the process stack for thread stacks is also not feasible. This is because multiple threads cannot share the same stack if they are not guaranteed to stack and unstack in a consistent order (first in, first out). Consider the situation where a thread performs a secure gateway to another process, and then yields and gets swapped out by the RTOS, saving its registers on the process stack. Next, a different thread performs a secure gateway to this same process, and it also yields and gets swapped out. Now, the RTOS wants to swap back in the original thread. The RTOS uses the stack pointer for that thread, and the registers are restored without any issue. The original thread then does a stack allocation that blaps over the saved state of the second thread, and the original thread then yields and is swapped out. Finally, the RTOS wants to swap the second thread back in. It uses the stack pointer for the second thread, but this pointer is now pointed to some "garbage" data leftover from the first thread's stack allocation. |
Contributor
Author
There was a problem hiding this comment.
What is a better word for "blaps"?
| ``` | ||
|
|
||
| The function causes a unicorn to barf up some barfable thing (imaginarily, of course). | ||
|
|
Contributor
Author
There was a problem hiding this comment.
Please replace this example with a less graphic one.
|
|
||
| ### RTOS Integration Mechanics | ||
| As currently integrated with uVisor, the RTOS runs with uVisor privileges. This must be fixed. [This issue is tracked on GitHub](https://github.com/ARMmbed/uvisor/issues/235). For now, the RTOS is a privileged component of the overall system, sharing privileged residence with uVisor. uVisor and the RTOS must trust each other because of this. Generally, uVisor is the manager of interrupts and the MPU, and the RTOS is the manager of context switching. | ||
|
|
Contributor
Author
There was a problem hiding this comment.
This paragraph is confusing to me. Is it saying that RTOS has privileges but shouldn't? If so, why does the next paragraph list capabilities that support such integration? If not, what is it saying?
| An asynchronous secure gateway is also introduced, to make asynchronous RPC easier than setting up a queue and using `uvisor_ipc_send`. | ||
| --> | ||
|
|
Contributor
Author
There was a problem hiding this comment.
Is this whole section supposed to be hidden? If so, should we find a way to hide it properly, so users can't see it?
|
|
||
| <!-- | ||
| >>> Comment: This concept has been rejected for now, since this is prone to fragmentation. | ||
| >>> Comment: This concept has been rejected for now because this is prone to fragmentation. |
Contributor
Author
There was a problem hiding this comment.
To what does this comment refer? Just the note above it? The whole section? If it doesn't work, should we take it out of the docs?
Contributor
There was a problem hiding this comment.
Yes it refers to the previous note. I'd keep it for now!
AlessandroA
approved these changes
Mar 8, 2017
Contributor
AlessandroA
left a comment
AlessandroA
left a comment
There was a problem hiding this comment.
Thank you very much for the changes!
Update uVisor documents for active voice, consistency, grammar and spelling
AlessandroA
force-pushed
the
AnotherButler
branch
from
990107a to
4daae23
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Update uVisor documents for active voice, consistency, grammar and spelling