Re: [RFC PATCH] i386: don't include lib/i386/reset.c in EFI builds

[Daniel Kiper]

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 <address@hidden>

If there are no objections I will apply this in a week or so.

Daniel