222 executables parse #224
Conversation
There was a problem hiding this comment.
I wouldnt commit an executable to the git repo :)
There was a problem hiding this comment.
yeah, i'm working on adding a new makefile goal to compile it from sources, i will add the source, removing this file as soon as i finish it.
| @@ -47,7 +47,7 @@ void *map_phys_to_virt_addr_hh(void* physical_address, void* address, size_t fla | |||
| clean_new_table(new_table_hhdm); | |||
| pdpr_root = new_table_hhdm; | |||
| } else { | |||
| pretty_log(Verbose, "No need to allocate pml4"); | |||
| //pretty_log(Verbose, "No need to allocate pml4"); | |||
There was a problem hiding this comment.
instead of commenting these out, why not remove them? can always re-add them later if needed
| idle_thread = idle_task->threads; | ||
| task_t* userspace_task = create_task("userspace_idle", NULL, &a, false); | ||
| //task_t* userspace_task = create_task_from_func("userspace_idle", NULL, &a, false); | ||
| task_t* elf_task = create_task_from_elf("elf_idle", NULL, (Elf64_Ehdr *) (uintptr_t) hhdm_get_variable(elf_module_start_phys)); |
There was a problem hiding this comment.
whats the purpose of the double cast: (Elf64_Ehdr*)(uintptr_t)?
| new_task->threads = thread; | ||
| } | ||
| scheduler_add_task(new_task); | ||
| //re-enable interrupts | ||
| asm("sti"); |
There was a problem hiding this comment.
you enable interrupts unconditionally here, what if they were previously disabled before calling this function? Now the caller thinks they are disabled but in reality they arent. This is a pattern that will be used a lot, so it can be helpful to have something like:
const bool restore_intrs = disable_interrupts(); //returns previous state and ensure interrupts are disabled
[ ... ]
if (restore_intrs)
enable_interrupts();| pretty_logf(Verbose, "Preparing to launch an ELF. entry_pint = 0x%x", (uint64_t) _entry_point); | ||
| new_thread->execution_frame->rip = (uint64_t)_entry_point; | ||
| } else { | ||
| new_thread->execution_frame->rip = prepare_userspace_function(&(parent_task->vmm_data)); |
There was a problem hiding this comment.
whats the purpose of doing it this way? I feel like you could have prepare_userspace_function called when creating the task for the elf, and then create_thread doesnt need to know if the thread is for an elf or not. You can just tell it what address to begin executing and itll use that. It would also simplify things a bit.
There was a problem hiding this comment.
the prepare_userspace_function is/was a temporary function that was basically copying a program (same for the one that now i'm running from ELF) from a char array into a userspace memory space and launch it.
I think this will be removed soon (but i think i'll keep it around until i fix the building of the elf that match the kernel PAGE_SIZE)
There was a problem hiding this comment.
But yeah, that function maybe can be repurposed to prepare the memory space for a new thread being launched from an executable file.
Closes #222