Re: CVS commit: src/sys/arch/amd64

[Maxime Villard <max%m00nbsd.net@localhost>]

Le 05/01/2020 à 13:56, Maxime Villard a écrit :
> Le 05/01/2020 à 02:03, Emmanuel Dreyfus a écrit :
>> On Sat, Jan 04, 2020 at 08:43:16AM +0100, Maxime Villard wrote:
>>> +.section multiboot,"",@note
>>> Why @note? It will be in the .text anyway. Also why no dot in the section
>>> name? That's supposed to be the naming convention.
>>
>> The idea is that one day if ld gets more reasonable, it could go in
>> non-loading note ection at the beginning of the binary, but if you
>> prefer .text, let us go with that.
> 
> I think .text.multiboot is fine, and @note should be dropped
> 
>> Attached is my latest change set, including the locore cleanup you asked
>> for.
> 
> Notice how, after cleanup, the big copy crap comes down to literally just
> two instructions. Unfortunately that's not exactly it:
> 
>  - As I said more than three weeks ago [1], I think it's the whole
>    MULTIBOOT block that should be moved in a separate file, not just the
>    32bit copy function. Only the '.Lbegin' label (to be renamed) needs to
>    be in locore.S, the rest can (and should) be outside.
>  - multiboot2_pre_reloc_would_be_built_as_ia32 should be removed.
>  - The code is still not entirely KNF, search for "\t\n".
>  - Local labels should begin with ".L".
>  - Now I'm wondering why KEEP() in the ldscript? Why doesn't
>    "*(.text.multiboot)" suffice?
> 
> And also... Recovering from the heart attack I got after looking at
> multiboot2_copy_syms32, I'm a bit confused; did you just objdump the
> function and copy-paste it in the kernel? How did you obtain this
> code? [Is it normal that I am already worried about your next answer?]
> 
> Overall, I'm irritated, yes, because instead of reverting the change and
> taking just one peaceful hour to fix things correctly, you have decided to
> waste everybody's time with the breakage and absurd patch-work. I find
> myself having to _insist_ for you to clean up the junk, and now I'm even
> quoting emails from one month ago.
> 
> Maxime
> 
> [1] https://mail-index.netbsd.org/source-changes-d/2019/12/12/msg011882.html

Sorry, but not gonna waste more of my time.

I have now requested to core@ that multiboot in amd64 be reverted entirely.

The correct way to add multiboot can be discussed afterwards, when the patch
will have been shared and will have been agreed upon beforehand, in a way
that it at least isn't total junk and doesn't break other things.

Maxime