Optimize to share upper half page table #509
Conversation
Sentry split memory address space into bottom half and upper half, kernel address space will be mapped into both ranges. And sentry will setup kernel address space map at upper half in every user task page table. Optimize is done to share the kernel upper half address space page table between sentry kernel and user task, this will reduce the user task page table memory consume and speed up user task initialize.
| pr.physical) | ||
| return true // Keep iterating. | ||
| }) | ||
| pageTables.MirrorKernelPageTable() |
There was a problem hiding this comment.
Per elsewhere, I think this would look something like:
pageTables.Mirror(m.kernel.PageTables, kernelStart, ....)
There was a problem hiding this comment.
Per elsewhere, I think this would look something like:
pageTables.Mirror(m.kernel.PageTables, kernelStart, ....)
Thanks for advise on the patch, add new method Mirror, and take range as parameter, also using the walk API, I will try to update the patch later.
| @@ -89,7 +89,7 @@ go_library( | |||
| "//pkg/sentry/platform/kvm:__subpackages__", | |||
| "//pkg/sentry/platform/ring0:__subpackages__", | |||
| ], | |||
| deps = ["//pkg/sentry/usermem"], | |||
| deps = ["//pkg/sentry/usermem","//pkg/sentry/platform/safecopy"], | |||
There was a problem hiding this comment.
Not formatted correctly (see other lines). Also, I don’t think you actually need safecopy at all. It’s not user memory.
|
|
||
| var kPageTable *PageTables | ||
|
|
||
| // MirrorInit set kernel page table as the mirror source. |
There was a problem hiding this comment.
This is kind of a strange API. It’s an object method but used to set a singleton, and will panic if already set :)
I don’t think the pageTables package needs to have a reference to this at all and you could just drop this. I’ll elaborate below.
| kPageTable = p | ||
| } | ||
|
|
||
| // Mirror the kernel upper address space page tables. |
There was a problem hiding this comment.
I think you really only need a single Mirror method, and can accept some pageTable argument. (I’ve shown this call above.) This let’s you get rid of the Singleton and weird API above, and the tweak to the walker API.
I don’t think you need to use safecopy to copy the PGDs, and would prefer if you accepted an explicit range to mirror (that would be the kernel range). It could just be called Mirror().
My other ask is that you document the Mirror() function really well. It would be really unsafe to use for any case where the lifetime of the other pageTables is not fixed and guaranteed, e.g. you can’t get creative and use it for fork. It’s a simple API but should only be used for kernel page tables.
| @@ -83,7 +83,9 @@ func (w *Walker) iterateRange(start, end uintptr) { | |||
| panic("alloc spans non-canonical range") | |||
| } | |||
| w.iterateRangeCanonical(start, lowerTop) | |||
| w.iterateRangeCanonical(upperBottom, end) | |||
There was a problem hiding this comment.
I don’t think you should do this, just document the API appropriately. Also, you already have w.pageTables, which is the same as the pageTables argument being passed here...
|
Just checking in. I'm happy to merge this once the API is tweaked. |
|
Checking in again. |
prattmic
added
area: platform
|
Note that per previous comments, if you have a change that addresses them then I don't think it has been pushed. I'd love to merge this with the fixes addressed, just let me know! |
|
I believe that this change will be obsoleted by #2256, which will require only mapping the entry/exit stubs in the upper half as we will switch to the kernel page tables there. Closing under that assumption. Please reopen if this is not valid! |
|
This pull request is stale because it has been open 90 days with no activity. Remove the stale label or comment or this will be closed in 30 days. |
Fixes: google#509 Signed-off-by: Lai Jiangshan <jiangshan.ljs@antfin.com> Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
Sentry split memory address space into bottom half
and upper half, kernel address space will be mapped
into both ranges.
And sentry will setup kernel address space map at
upper half in every user task page table.
Optimize is done to share the kernel upper half
address space page table between sentry kernel and
user task, this will reduce the user task page table
memory consume and speed up user task initialize.