Le 12/12/2019 à 10:20, Maxime Villard a écrit :
Le 10/12/2019 à 03:06, Emmanuel Dreyfus a écrit :
Module Name: src
Committed By: manu
Date: Tue Dec 10 02:06:07 UTC 2019
Modified Files:
src/sys/arch/amd64/amd64: locore.S machdep.c
src/sys/arch/amd64/conf: GENERIC files.amd64 kern.ldscript
src/sys/arch/x86/x86: efi.c multiboot2.c
Log Message:
Add multiboot 2 support to amd64 kernel
I don't see how this can work
mov $(__kernel_end - kernel_text), %rcx /* size *.
Malformed comment means the next instructions are commented, too.
In addition, it seems that you inlined the whole bcopy.S file into locore.S,
with all the unnecessary defines, duplicates and irrelevant comments. Why?
It seems you could have just used a byte-by-byte copy, which would have
avoided all the unnecessary garbage.
Also, I would have put the whole #ifdefined(MULTIBOOT) block into a separate
function, instead of inlining everything in start(), which makes the code
hard to read. Or even better: in a separate file.
IMO this commit needs to be revisited (and I see that the kern.ldscript
change got reverted already).
Maxime
In addition:
745 movl $1,%eax
746 movl %eax,RELOC(multiboot2_enabled)
multiboot2_enabled is a boolean, so it should be movb. Here we are overwriting
the other booleans in memory. Then, later:
1634 call _C_LABEL(multiboot2_post_reloc)
Here, there should be a check on multiboot2_enabled before calling the function,
not inside of it. This breaks shadow-based sanitizers, like kASan.
I will disable this code for now.