On Thu, Jul 12, 2018 at 01:44:36PM +0200, Daniel Kiper wrote: > On Wed, Jul 11, 2018 at 12:53:01PM +0100, Leif Lindholm wrote: > > On Wed, Jul 11, 2018 at 01:03:12PM +0200, Daniel Kiper wrote: > > > On Mon, Jul 09, 2018 at 07:49:06PM +0100, Leif Lindholm wrote: > > > > Commit 0ba90a7f0178 ("efi: Move grub_reboot() into kernel") broke > > > > the build on i386-efi - genmoddep.awk bails out with message > > > > grub_reboot in reboot is duplicated in kernel > > > > This is because both lib/i386/reset.c and kern/efi/efi.c now provide > > > > this function. > > > > > > > > Rather than explicitly list each i386 platform variant in > > > > Makefile.core.def, include the contents of lib/i386/reset.c only when > > > > GRUB_MACHINE_EFI is not set. > > > > > > Could you try the patch below? It seems better to me. > > > > > > diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def > > > index fc4767f..820ddc3 100644 > > > --- a/grub-core/Makefile.core.def > > > +++ b/grub-core/Makefile.core.def > > > @@ -870,8 +870,8 @@ module = { > > > > > > module = { > > > name = reboot; > > > - i386 = lib/i386/reboot.c; > > > - i386 = lib/i386/reboot_trampoline.S; > > > + i386_pc = lib/i386/reboot.c; > > > + i386_pc = lib/i386/reboot_trampoline.S; > > > powerpc_ieee1275 = lib/ieee1275/reboot.c; > > > sparc64_ieee1275 = lib/ieee1275/reboot.c; > > > mips_arc = lib/mips/arc/reboot.c; > > > > I agree this looks a lot nicer, but I don't know enough to say whether > > that's valid for i386_coreboot, i386_multiboot and i386_qemu, which > > don't seem to have any other grub_reset() implementations. > > I also don't know how to test those beyond just compiling. > > > > (i386_)ieee1275 implements its own grub_reboot(), so that should be > > fine. (This does mean that i386_ieee1275 may currently be unable to > > load the reboot module on master.) > > Hmmm... So, it looks that your solution is safer. Then > > Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com> > > If there are no objections I will apply this in a week or so.
In that case, I think it may be worth extending the test to #if !defined (GRUB_MACHINE_EFI) && !defined (GRUB_MACHINE_IEEE1275) I had not noticed that bit when I sent the original patch. But this is theorising based on looking at source code without testing. / Leif _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel