[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3 3/4] x86/boot: Rewrite EFI/MBI2 code partly in C
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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |