| Message ID | 20200905191524.1603618-1-sjg@chromium.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Bin Meng |
| Headers | show |
| Series | x86: Drop duplicate declaration of emulator state | expand |
Hi Simon, -----"U-Boot" <u-boot-bounces@lists.denx.de> schrieb: ----- > Betreff: [PATCH] x86: Drop duplicate declaration of emulator state > > With x86 we can execute an option ROM either natively or using the x86 > emulator (if enabled with CONFIG_BIOSEMU). Both of these share the > _X86EMU_env variable, with the native code using it to hold register state > during interrupt processing. > > At present, in 32-bit U-Boot, the variable is declared twice, once in > common code and once in code only compiled with CONFIG_BIOSEMU. > > With gcc-10 this causes a 'multiple definitions' error on boards with > CONFIG_BIOSEMU. > > Drop the emulator definition, except for 64-bit builds. > > Also drop inclusion of the emulator in 64-bit U-Boot since this does not > work at present, and generally isn't needed if 32-bit code has already set > up the option ROMs. > > Reported-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > drivers/bios_emulator/x86emu/sys.c | 4 ++++ > 1 file changed, 4 insertions(+) Tested-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com> Tested by building chromebook_coral_defconfig on an Arch Linux with GCC 10.1.0 regards, Wolfgang
Hi Simon, On Sun, Sep 6, 2020 at 3:15 AM Simon Glass <sjg@chromium.org> wrote: > > With x86 we can execute an option ROM either natively or using the x86 > emulator (if enabled with CONFIG_BIOSEMU). Both of these share the > _X86EMU_env variable, with the native code using it to hold register state > during interrupt processing. > > At present, in 32-bit U-Boot, the variable is declared twice, once in > common code and once in code only compiled with CONFIG_BIOSEMU. > > With gcc-10 this causes a 'multiple definitions' error on boards with > CONFIG_BIOSEMU. > > Drop the emulator definition, except for 64-bit builds. > > Also drop inclusion of the emulator in 64-bit U-Boot since this does not > work at present, and generally isn't needed if 32-bit code has already set > up the option ROMs. > > Reported-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > drivers/bios_emulator/x86emu/sys.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/bios_emulator/x86emu/sys.c b/drivers/bios_emulator/x86emu/sys.c > index c2db1213fe6..146586b3ceb 100644 > --- a/drivers/bios_emulator/x86emu/sys.c > +++ b/drivers/bios_emulator/x86emu/sys.c > @@ -44,7 +44,11 @@ > > /*------------------------- Global Variables ------------------------------*/ > > +/* Definite this here since the emulator is not present on 64-bit */ > +#ifdef CONFIG_X86_64 I believe the correct fix is to drop the one in arch/x86/lib/bios.c > X86EMU_sysEnv _X86EMU_env; /* Global emulator machine state */ > +#endif > + > X86EMU_intrFuncs _X86EMU_intrTab[256]; > > int debug_intr; > -- Regards, Bin
Am 21. September 2020 03:58:31 MESZ schrieb Bin Meng <bmeng.cn@gmail.com>: >Hi Simon, > >On Sun, Sep 6, 2020 at 3:15 AM Simon Glass <sjg@chromium.org> wrote: >> >> With x86 we can execute an option ROM either natively or using the >x86 >> emulator (if enabled with CONFIG_BIOSEMU). Both of these share the >> _X86EMU_env variable, with the native code using it to hold register >state >> during interrupt processing. >> >> At present, in 32-bit U-Boot, the variable is declared twice, once in >> common code and once in code only compiled with CONFIG_BIOSEMU. >> >> With gcc-10 this causes a 'multiple definitions' error on boards with >> CONFIG_BIOSEMU. >> >> Drop the emulator definition, except for 64-bit builds. >> >> Also drop inclusion of the emulator in 64-bit U-Boot since this does >not >> work at present, and generally isn't needed if 32-bit code has >already set >> up the option ROMs. >> >> Reported-by: Heinrich Schuchardt <xypron.glpk@gmx.de> >> Signed-off-by: Simon Glass <sjg@chromium.org> >> --- >> >> drivers/bios_emulator/x86emu/sys.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/bios_emulator/x86emu/sys.c >b/drivers/bios_emulator/x86emu/sys.c >> index c2db1213fe6..146586b3ceb 100644 >> --- a/drivers/bios_emulator/x86emu/sys.c >> +++ b/drivers/bios_emulator/x86emu/sys.c >> @@ -44,7 +44,11 @@ >> >> /*------------------------- Global Variables >------------------------------*/ >> >> +/* Definite this here since the emulator is not present on 64-bit */ %s/Definite/Define/ >> +#ifdef CONFIG_X86_64 > >I believe the correct fix is to drop the one in arch/x86/lib/bios.c > >> X86EMU_sysEnv _X86EMU_env; /* Global emulator machine state */ >> +#endif >> + >> X86EMU_intrFuncs _X86EMU_intrTab[256]; >> >> int debug_intr; >> -- > >Regards, >Bin
Hi Bin, On Sun, 20 Sep 2020 at 19:58, Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Simon, > > On Sun, Sep 6, 2020 at 3:15 AM Simon Glass <sjg@chromium.org> wrote: > > > > With x86 we can execute an option ROM either natively or using the x86 > > emulator (if enabled with CONFIG_BIOSEMU). Both of these share the > > _X86EMU_env variable, with the native code using it to hold register state > > during interrupt processing. > > > > At present, in 32-bit U-Boot, the variable is declared twice, once in > > common code and once in code only compiled with CONFIG_BIOSEMU. > > > > With gcc-10 this causes a 'multiple definitions' error on boards with > > CONFIG_BIOSEMU. > > > > Drop the emulator definition, except for 64-bit builds. > > > > Also drop inclusion of the emulator in 64-bit U-Boot since this does not > > work at present, and generally isn't needed if 32-bit code has already set > > up the option ROMs. > > > > Reported-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > > Signed-off-by: Simon Glass <sjg@chromium.org> > > --- > > > > drivers/bios_emulator/x86emu/sys.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/bios_emulator/x86emu/sys.c b/drivers/bios_emulator/x86emu/sys.c > > index c2db1213fe6..146586b3ceb 100644 > > --- a/drivers/bios_emulator/x86emu/sys.c > > +++ b/drivers/bios_emulator/x86emu/sys.c > > @@ -44,7 +44,11 @@ > > > > /*------------------------- Global Variables ------------------------------*/ > > > > +/* Definite this here since the emulator is not present on 64-bit */ > > +#ifdef CONFIG_X86_64 > > I believe the correct fix is to drop the one in arch/x86/lib/bios.c > > > X86EMU_sysEnv _X86EMU_env; /* Global emulator machine state */ > > +#endif > > + > > X86EMU_intrFuncs _X86EMU_intrTab[256]; > > > > int debug_intr; This currently causes a build error though, which is why I went with the other approach. Regards, Simon
diff --git a/drivers/bios_emulator/x86emu/sys.c b/drivers/bios_emulator/x86emu/sys.c index c2db1213fe6..146586b3ceb 100644 --- a/drivers/bios_emulator/x86emu/sys.c +++ b/drivers/bios_emulator/x86emu/sys.c @@ -44,7 +44,11 @@ /*------------------------- Global Variables ------------------------------*/ +/* Definite this here since the emulator is not present on 64-bit */ +#ifdef CONFIG_X86_64 X86EMU_sysEnv _X86EMU_env; /* Global emulator machine state */ +#endif + X86EMU_intrFuncs _X86EMU_intrTab[256]; int debug_intr;
With x86 we can execute an option ROM either natively or using the x86 emulator (if enabled with CONFIG_BIOSEMU). Both of these share the _X86EMU_env variable, with the native code using it to hold register state during interrupt processing. At present, in 32-bit U-Boot, the variable is declared twice, once in common code and once in code only compiled with CONFIG_BIOSEMU. With gcc-10 this causes a 'multiple definitions' error on boards with CONFIG_BIOSEMU. Drop the emulator definition, except for 64-bit builds. Also drop inclusion of the emulator in 64-bit U-Boot since this does not work at present, and generally isn't needed if 32-bit code has already set up the option ROMs. Reported-by: Heinrich Schuchardt <xypron.glpk@gmx.de> Signed-off-by: Simon Glass <sjg@chromium.org> --- drivers/bios_emulator/x86emu/sys.c | 4 ++++ 1 file changed, 4 insertions(+)