Re: [PATCH v3 3/4] x86/boot: Rewrite EFI/MBI2 code partly in C

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.09.2024 16:28, Frediano Ziglio wrote:
> On Tue, Sep 24, 2024 at 3:17 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>
>> On 24.09.2024 12:28, Frediano Ziglio wrote:
>>> No need to have it coded in assembly.
>>>
>>> Signed-off-by: Frediano Ziglio <frediano.ziglio@xxxxxxxxx>
>>> ---
>>> Changes since v1:
>>> - update some comments;
>>> - explain why %ebx is saved before calling efi_parse_mbi2;
>>> - move lea before test instruction;
>>> - removed asmlinkage from efi_multiboot2 and add to efi_parse_mbi2;
>>> - fix line length;
>>> - update an error message specifying "Multiboot2" instead of "Multiboot";
>>> - use obj-bin-X instead of obj-X in Makefile;
>>> - avoid restoring %eax (MBI magic).
>>
>> Despite this long list of changes earlier comments were left unaddressed.
>> The new function is still named as if it did only parsing, the stub change
>> is still in here and (if already not separated out) not mentioned at all
>> in the description, and (as Andrew has now also pointed out) the
>> declaration of efi_multiboot2() didn't move to a header. Maybe I forgot
>> some more. Please make sure you address earlier comments before sending a
>> new version.
> 
> What about renaming efi_parse_mbi2 to "efi_multiboot2_entry_common"
> and renaming "efi_multiboot2" as "efi_multiboot2_entry".

I don't see a need to rename efi_multiboot2() as well. In
"efi_multiboot2_entry_common" I'm having trouble seeing what "common"
stands for. Imo a small improvement would already be efi_process_mbi2(),
as "process" covers parsing and the handing on of the result of the
parsing. However, if already the new code can't be folded into
efi_multiboot2(), how about naming the new function
efi_multiboot2_prelude()?

> I remember I replied to the stub change and nobody get back, so I
> thought it was fine as it was.
> I also replied to the header asking for a location to put it, and I
> don't remember any reply.

I'm sure I gave you a reply, but that was only yesterday, after I was
back from travelling / PTO.

Jan