[PATCH] x86: Drop duplicate declaration of emulator state

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] x86: Drop duplicate declaration of emulator state

Simon Glass-3
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 <[hidden email]>
Signed-off-by: Simon Glass <[hidden email]>
---

 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
 X86EMU_sysEnv _X86EMU_env; /* Global emulator machine state */
+#endif
+
 X86EMU_intrFuncs _X86EMU_intrTab[256];
 
 int debug_intr;
--
2.28.0.526.ge36021eeef-goog

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86: Drop duplicate declaration of emulator state

Wolfgang Wallner
Hi Simon,

-----"U-Boot" <[hidden email]> 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 <[hidden email]>
> Signed-off-by: Simon Glass <[hidden email]>
> ---
>
>  drivers/bios_emulator/x86emu/sys.c | 4 ++++
>  1 file changed, 4 insertions(+)

Tested-by: Wolfgang Wallner <[hidden email]>
Tested by building chromebook_coral_defconfig on an Arch Linux with GCC 10.1.0

regards, Wolfgang

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86: Drop duplicate declaration of emulator state

Bin Meng-2
In reply to this post by Simon Glass-3
Hi Simon,

On Sun, Sep 6, 2020 at 3:15 AM Simon Glass <[hidden email]> 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 <[hidden email]>
> Signed-off-by: Simon Glass <[hidden email]>
> ---
>
>  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
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86: Drop duplicate declaration of emulator state

Heinrich Schuchardt
Am 21. September 2020 03:58:31 MESZ schrieb Bin Meng <[hidden email]>:

>Hi Simon,
>
>On Sun, Sep 6, 2020 at 3:15 AM Simon Glass <[hidden email]> 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 <[hidden email]>
>> Signed-off-by: Simon Glass <[hidden email]>
>> ---
>>
>>  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

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86: Drop duplicate declaration of emulator state

Simon Glass-3
In reply to this post by Bin Meng-2
Hi Bin,

On Sun, 20 Sep 2020 at 19:58, Bin Meng <[hidden email]> wrote:

>
> Hi Simon,
>
> On Sun, Sep 6, 2020 at 3:15 AM Simon Glass <[hidden email]> 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 <[hidden email]>
> > Signed-off-by: Simon Glass <[hidden email]>
> > ---
> >
> >  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