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.
user/init/src/main.rs
Outdated
| 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
2 times, most recently
from
4b78ab7 to
23f2136
Compare
|
Hi @joergroedel can you please take a look? |
user/init/src/main.rs
Outdated
| // 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?
user/lib/module.lds
Outdated
| /* 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?
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 userdm process. Co-developed-by: Peter Fang <[email protected]> Signed-off-by: Vijay Dhanraj <[email protected]>
The device model is a user mode program that manages VM's life cycles and handles various vmexit events. The initial version just calls exit() syscall without doing anything else. Co-developed-by: Chuanxiao Dong <[email protected]> Signed-off-by: Vijay Dhanraj <[email protected]>
Update the comment section for exec_user() to correctly document its return type. Signed-off-by: Peter Fang <[email protected]>
Print the task name in addition to its exit code to make logging a bit more informative. Signed-off-by: Peter Fang <[email protected]>
Pretty-printing SvsmError in Debug formatting doesn't seem to be
particularly helpful at this point. For example:
[SVSM] Failed to launch /init: FileSystem(
FileNotFound,
)
Change to using the default format to make logging more compact.
Signed-off-by: Peter Fang <[email protected]>
peterfang
force-pushed
the
init_dm_module
branch
from
23f2136 to
5768e12
Compare
|
Hi @joergroedel, I've updated this PR with some minor changes. |
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.