Userspace support: Refactor init to launch a simple device model #547
Conversation
vijaydhanraj
force-pushed
the
init_dm_module
branch
from
b3c9856 to
d5c5be3
Compare
vijaydhanraj
force-pushed
the
init_dm_module
branch
2 times, most recently
from
5a5c846 to
0000873
Compare
There was a problem hiding this comment.
As a general note the init process should have a configuration file which lists the modules to launch. We should also start thinking about a feature detection mechanism for the SVSM so that init can make decisions based on the environment it is running in. But that is outside the scope of this PR.
| let bin = CString::new("/").unwrap(); | ||
| let Ok(obj) = opendir(&bin) else { |
There was a problem hiding this comment.
Can this be just opendir(b"/\0")?
There was a problem hiding this comment.
opendir expects a &CStr type but using a raw byte slice may cause a compilation error. Also, current implementation is extensible if in future we want to change path from /.
There was a problem hiding this comment.
Yes, I understand, but creating a CStr usually requires allocator support. I am wondering whether there is a simpler way.
There was a problem hiding this comment.
How about something like this?
- let bin = CString::new("/").unwrap();
- let Ok(obj) = opendir(&bin) else {
+ let bin = CStr::from_bytes_with_nul(b"/\0").unwrap();
+ let Ok(obj) = opendir(bin) else {
return 0;
};
Is the preference not to use any heap allocator for now? I added buddy_system_allocator only as a temporary solution until we have our own heap allocator.
BTW, I do have an alternate implementation using the stack and avoiding heap allocation entirely. With this implementation we can skip adding buddy_system_allocator crate.
joergroedel
added
in-review
Yes, this makes sense. I can look into this. |
vijaydhanraj
force-pushed
the
init_dm_module
branch
from
0000873 to
4b78ab7
Compare
Add initial version of init to launch a dummy device model. Use `opendir` and `readdir` syscalls to discover the first file under "/" and `exec` the dm process. Signed-off-by: Vijay Dhanraj <[email protected]> Signed-off-by: Peter Fang <[email protected]>
Binaries without uninitialized global variable may generate a `.bss` LOAD segment with no valid content (`p_filesz` = 0, `p_memsz` = 0). The problem with such a segment is that ELF parser may fail with unaligned segment address error. To avoid this, add a dummy `.bss` to force non-zero memory allocation, ensuring a meaningful PT_LOAD. Signed-off-by: Vijay Dhanraj <[email protected]>
The device model is a user mode program which manages VM's life cycles and handles various vmexits events. The initial version just calls exit() syscall without doing anything else. Signed-off-by: Vijay Dhanraj <[email protected]> Signed-off-by: Chuanxiao Dong <[email protected]>
vijaydhanraj
force-pushed
the
init_dm_module
branch
from
4b78ab7 to
23f2136
Compare
|
Hi @joergroedel can you please take a look? |
| // SAFETY: The `begin` and `size` values refer to a valid, properly aligned, and | ||
| // accessible memory region. | ||
| unsafe { | ||
| HEAP_ALLOCATOR | ||
| .lock() | ||
| .init(core::ptr::addr_of!(HEAP) as usize, HEAP_SIZE); | ||
| } |
There was a problem hiding this comment.
Heap allocator initialization needs to happen in the userlib crate. When main starts the heap should be initialized already.
There was a problem hiding this comment.
Understood. However, if we aim to avoid using the heap allocator, this comment may not be applicable. Could you please clarify your preference?
| /* Fallback dummy section if `.bss` is empty */ | ||
| . = ALIGN(4096); | ||
| .bss_dummy (NOLOAD) : { | ||
| LONG(0); | ||
| } :bss | ||
|
|
There was a problem hiding this comment.
Can you elaborate on why this is needed?
There was a problem hiding this comment.
I added this workaround to resolve this issue, #539 (comment). Is there any better way to address this?
This PR adds an initial version of
initto launch a dummy device model. The initialdmversion just callsexitsyscall without doing anything else.Note: This PR is built on top of #520 and #539 PRs.