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.

Reply via email to