From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-lj1-f176.google.com (mail-lj1-f176.google.com [209.85.208.176]) by mx.groups.io with SMTP id smtpd.web09.57.1627060654578476905 for ; Fri, 23 Jul 2021 10:17:35 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@google.com header.s=20161025 header.b=eoJJKPd4; spf=pass (domain: google.com, ip: 209.85.208.176, mailfrom: chengchieh@google.com) Received: by mail-lj1-f176.google.com with SMTP id m9so2529598ljp.7 for ; Fri, 23 Jul 2021 10:17:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=fHL55r0me1VOhPraKMY5Xh1j+vWIol6NzZndN1+s63I=; b=eoJJKPd4l/rh26qSKG/6uQA4ZMQjE+hVA+zEbgB60ho0dJeXmQihWEaVGpuEM4ei7t tJZT4PtFj1BXzok9UU4L6eUHrHL8XvC5cd3Mls5aQWgUO/VAOdGTBIT4KEXBIwN9LCq5 bjOqm/X1vMoqaIlHf3ixeW/wajf+PSFgXVMgbLoev8BJbNtW/WMhbT/zcLMP2j8yVY4G hITVaYC9nJWinv/GFeSYlIW9P8PXBvfjMBGslLXjTcz/p9bS6kX27Qkpo6DVZFe6tkTJ 8aQQDOhglMgQqZoVx1CYvnvZ48eIL0i7keVszPoFTd5k2TgFXz03Qd189TIO6MaCc3zT MbVA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=fHL55r0me1VOhPraKMY5Xh1j+vWIol6NzZndN1+s63I=; b=gx3mVWm2EWMzMq+FsGXZXFm5bNb+bx1jnhnGViIZV1aLHCL2nLbaZwz5VtjW2L8c0P 3muRq2iW/62kRPj4B+qK94/GrP5LEg0y1qR8HcRt9kFEUPVuMzCcfzo+iwfyPugDr6BX i9mymkfLp+drjQNCOLh6WI5p9P8bG3n3Dh9oAoS/svfl3cz3Mo/i/L5JKzp7mvDXG9DN SEIU2T42veA0pvVgel5o8I3lz0CmKEPdIQah71riutW0gXsiQpygUGHbzkk0bjleXREr ADfX1roO5VARUjm8c2GMvbUYzTUlc+8y2BVMJMPB+IC9JByjMlZijeTsILUeGB9hlO6g Kgbg== X-Gm-Message-State: AOAM532m+u1s53Ws+dk8VGNKpxB1b4WCmG8Sag9BYe/g0kHiLuXv8FDi JEyclQLLY9K6MHbaYooA5W5TEWyTBuGrqJHQlwyojQ== X-Google-Smtp-Source: ABdhPJxisQJZkFjgXz3Mk8KhfVLn9i3Jz8CzdNVOdjfY23iPcLnhbnJ32ZQqj29ipCyEffDinsg4AMnKQbSytoAZw6Y= X-Received: by 2002:a05:651c:130f:: with SMTP id u15mr3981685lja.485.1627060652528; Fri, 23 Jul 2021 10:17:32 -0700 (PDT) MIME-Version: 1.0 References: <20210721132328.1415485-1-chengchieh@google.com> <20210721132328.1415485-6-chengchieh@google.com> <000301d77e98$e1c0d9c0$a5428d40$@byosoft.com.cn> <013201d77fad$8b3d7750$a1b865f0$@byosoft.com.cn> In-Reply-To: From: Cheng-Chieh Huang Date: Sat, 24 Jul 2021 01:17:21 +0800 Message-ID: Subject: Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation To: "Kinney, Michael D" Cc: gaoliming , "devel@edk2.groups.io" Content-Type: multipart/alternative; boundary="000000000000397ecd05c7cd9428" --000000000000397ecd05c7cd9428 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi, This is not about UefiPayload. I did not have GCC5 in my environment, so I use a newer GCC to compile. However, in newer gcc (let's say GCC8), it generates floating point instructions in each function and system will crash as you mentioned that we did not handle it in UEFI. I uploaded this patch because many people from our group use newer GCC and it will be more convenient if we can disable it with a flag (otherwise, we need to maintain a local patch). I guess the real fix (bigger work) is for EDK2 to support the newer GCC version in its toolchain (create something like *_GCC8_*). Back to the question, I think it should be no harm to add these flags to tool_defs.txt if EDK2 did not expect the compiler to generate MMX/SSE instructions in the first place. -- Cheng-Chieh On Sat, Jul 24, 2021 at 12:46 AM Kinney, Michael D < michael.d.kinney@intel.com> wrote: > Can you provide details on why this flag was needed in the UefiPayloadPk= g > use case? > > > > If the newer versions of the GCC compilers are injecting use of MMX/SSE > instructions from optimization of C code, then we need to unconditionall= y > disable that usage in tools_def.txt. The only place MMX/SSE instruction= s > can be used is in NASM files with proper save/restore of the MMX/SSE sta= te. > > > > Here is an example with use of xmm0 that is saved/restored on the stack. > > > > > https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BaseMemoryL= ibSse2/X64/CopyMem.nasm > > > > Mike > > > > *From:* Cheng-Chieh Huang > *Sent:* Friday, July 23, 2021 4:17 AM > *To:* gaoliming > *Cc:* devel@edk2.groups.io; Kinney, Michael D > *Subject:* Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add > DISABLE_MMX_SSE to avoid generating floating points operation > > > > I got what you mean by "Tools_def.txt doesn=E2=80=99t support build flag= " now. I > thought the parser does support some sort of if/else condition, but it > seems to be not the case. Alternatively, we can do this during the copy > (from template to tools_def.txt), but everytime we need to recreate > tools_def.txt which is not ideal). > > > > In that case, I think we should just roll back to only doing this in > UefiPayloadPkg.dsc. If other packs want to add these flags, they will ne= ed > to do it on their own. > > > > On Fri, Jul 23, 2021 at 6:29 PM gaoliming > wrote: > > How do you add this support in tools_def? Can you give the proposal for = it? > > > > Thanks > > Liming > > *=E5=8F=91=E4=BB=B6=E4=BA=BA**:* Cheng-Chieh Huang > *=E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4:* 2021=E5=B9=B47=E6=9C=8822=E6=97= =A5 10:35 > *=E6=94=B6=E4=BB=B6=E4=BA=BA:* gaoliming > *=E6=8A=84=E9=80=81:* devel@edk2.groups.io; Kinney, Michael D > *=E4=B8=BB=E9=A2=98:* Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Ad= d DISABLE_MMX_SSE > to avoid generating floating points operation > > > > I mean, I will submit a patch to support DISABLE_GCC_MMX_SSE in tools_de= f. > What do you think? > > > > -- > > Cheng-Chieh > > > > On Thu, Jul 22, 2021 at 9:28 AM gaoliming > wrote: > > Tools_def.txt doesn=E2=80=99t support build flag DISABLE_GCC_MMX_SSE. If= this > flag is moved into BaseTools\Conf\tools_def.template, -mno-mmx -mno-sse > option will be the default GCC options. That means all platforms will ap= ply > them. > > > > Thanks > > Liming > > *=E5=8F=91=E4=BB=B6=E4=BA=BA**:* devel@edk2.groups.io *=E4=BB=A3=E8=A1=A8 *Cheng-Chieh > Huang via groups.io > *=E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4:* 2021=E5=B9=B47=E6=9C=8822=E6=97= =A5 1:43 > *=E6=94=B6=E4=BB=B6=E4=BA=BA:* Kinney, Michael D > *=E6=8A=84=E9=80=81:* devel@edk2.groups.io > *=E4=B8=BB=E9=A2=98:* Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Ad= d DISABLE_MMX_SSE > to avoid generating floating points operation > > > > Yes, we can. I will drop this patch for this uefipayload batch and send > another one for support DISABLE_GCC_MMX_SSE in tools_de.txt. > > > > -- > > Cheng-Chieh > > On Thu, Jul 22, 2021, 12:35 AM Kinney, Michael D < > michael.d.kinney@intel.com> wrote: > > Are those flags needed for all packages that build with GCC? > > Should this be moved into tools_def.txt? > > Mike > > > -----Original Message----- > > From: devel@edk2.groups.io On Behalf Of > Cheng-Chieh Huang via groups.io > > Sent: Wednesday, July 21, 2021 6:23 AM > > To: devel@edk2.groups.io > > Cc: Cheng-Chieh Huang > > Subject: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_S= SE > to avoid generating floating points operation > > > > This will allow we compile payload using gcc8 > > > > Signed-off-by: Cheng-Chieh Huang > > --- > > UefiPayloadPkg/UefiPayloadPkg.dsc | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc > b/UefiPayloadPkg/UefiPayloadPkg.dsc > > index 8aa5f18cd35c..fa41c5a24af5 100644 > > --- a/UefiPayloadPkg/UefiPayloadPkg.dsc > > +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc > > @@ -30,6 +30,8 @@ [Defines] > > DEFINE PS2_KEYBOARD_ENABLE =3D FALSE > > DEFINE UNIVERSAL_PAYLOAD =3D FALSE > > > > + DEFINE DISABLE_MMX_SSE =3D FALSE > > + > > # > > # SBL: UEFI payload for Slim Bootloader > > # COREBOOT: UEFI payload for coreboot > > @@ -96,6 +98,9 @@ [BuildOptions] > > *_*_*_CC_FLAGS =3D -D DISABLE_NEW_DEPRECATED_INTERF= ACES > > !if $(BOOTLOADER) =3D=3D "LINUXBOOT" > > *_*_*_CC_FLAGS =3D -D LINUXBOOT_PAYLOAD > > +!endif > > +!if $(DISABLE_MMX_SSE) > > + *_*_*_CC_FLAGS =3D -mno-mmx -mno-sse > > !endif > > GCC:*_UNIXGCC_*_CC_FLAGS =3D -DMDEPKG_NDEBUG > > GCC:RELEASE_*_*_CC_FLAGS =3D -DMDEPKG_NDEBUG > > -- > > 2.32.0.402.g57bb445576-goog > > > > > > > > > > > >=20 > > --000000000000397ecd05c7cd9428 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi,=C2=A0

This is not about = UefiPayload. I did not have GCC5 in my environment, so I use a newer GCC to= compile. However, in newer gcc (let's say GCC8), it generates floating= point instructions in each function=C2=A0and system will crash as you ment= ioned that we did not handle it in UEFI.=C2=A0

I u= ploaded this patch because many people=C2=A0from our group use newer GCC an= d it will be more convenient if we can disable it with a flag (otherwise, w= e need to maintain a local patch). I guess the real fix (bigger work) is fo= r EDK2 to support the newer GCC version in its toolchain (create something = like *_GCC8_*).=C2=A0

Back to the question, I thin= k it should be no harm to add these flags to tool_defs.txt if EDK2 did not = expect the compiler to generate MMX/SSE instructions in the first place.

--
Cheng-Chieh

<= div class=3D"gmail_quote">
On Sat, Jul= 24, 2021 at 12:46 AM Kinney, Michael D <michael.d.kinney@intel.com> wrote:

Can you provide details on why this flag was = needed in the UefiPayloadPkg us= e case?

=C2=A0

If the newer versions of the GCC compilers ar= e injecting use of MMX/SSE instructions from optimization of C code, then w= e need to unconditionally disable that usage in tools_def.txt.=C2=A0 The only place MMX/SSE instructions can be used is in NASM files wi= th proper save/restore of the MMX/SSE state.

=C2=A0

Here is an example with use of xmm0 that is s= aved/restored on the stack.

=C2=A0

https://github.com/tianocore/edk2/blob/ma= ster/MdePkg/Library/BaseMemoryLibSse2/X64/CopyMem.nasm

=C2=A0

Mike

=C2=A0

From: Cheng-Chieh Huang &= lt;chengchieh@go= ogle.com>
Sent: Friday, July 23, 2021 4:17 AM
To: gaoliming <gaoliming@byosoft.com.cn>
Cc: devel= @edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABL= E_MMX_SSE to avoid generating floating points operation

=C2=A0

I got what you mean by "Tools_def.txt doesn=E2= = =80=99t support build flag" now. I thought the parser does support so= me sort of if/else condition, but it seems to be not the case. Alternativel= y, we can do this during the copy (from template to tools_def.txt), but everytime we need to recreate tools_def.txt which is not ideal).

=C2=A0

In that case, I think we should just roll back to o= nly doing this in UefiPayloadPkg.dsc. If other packs want to add these flag= s, they will need to do it on their own.

=C2=A0

On Fri, Jul 23, 2021 at 6:29 PM gaoliming <gaoliming@byosoft.= com.cn> wrote:

How do you add this support in tools_def? Can you give the proposal for = it?

=C2=A0

Thanks

Liming

=E5=8F=91=E4=BB=B6= =E4=BA=BA: Cheng-Chieh Huang <chengchieh@google.com>
=E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2= 021=E5=B9=B47=E6=9C=8822=E6=97=A5 10:35
=E6=94=B6=E4=BB=B6=E4=BA=BA: gaoliming = <gaoliming= @byosoft.com.cn>
=E6=8A=84=E9=80=81: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com> =E4=B8=BB=E9=A2=98: Re: [edk2-devel] [P= ATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floati= ng points operation

=C2=A0

I mean, I will submit a patch to support DISA= BLE_GCC_MMX_SSE in tools_def. What do you think?

=C2=A0

--

Cheng-Chieh

=C2=A0

On Thu, Jul 22, 2021 at 9:28 AM gaoliming <= ;gaoliming@by= osoft.com.cn> wrote:

Tools_def.txt doesn=E2=80=99t support build = flag DISABLE_GCC_MMX_SSE. If this flag is moved into BaseTools\Con= f\tools_def.template, -mno-mmx -mno-sse option will be the default GCC opti= ons. That means all platforms will apply them.

=C2=A0

Thanks

Liming

=E5=8F=91=E4=BB=B6= =E4=BA=BA: devel@edk2.group= s.io <deve= l@edk2.groups.io> =E4=BB=A3=E8=A1=A8 Cheng-Chieh Huang vi= a groups.io
=E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2= 021=E5=B9=B47=E6=9C=8822=E6=97=A5 1:43
=E6=94=B6=E4=BB=B6=E4=BA=BA: Kinney, Mi= chael D <michael.d.kinney@intel.com>
=E6=8A=84=E9=80=81: devel@edk2.groups.io
=E4=B8=BB=E9=A2=98: Re: [edk2-devel] [P= ATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floati= ng points operation

=C2=A0

Yes, we can. I will drop this patch for this= =C2=A0 uefipayload batch and send another one for support DISABLE_GCC_MMX_= SSE in tools_de.txt.

=C2=A0

--

Cheng-Chieh=C2= =A0

On Thu, Jul 22, 2021, 12:35 AM Kinney, Michae= l D <mic= hael.d.kinney@intel.com> wrote:

Are those flags = needed for all packages that build with GCC?

Should this be moved into tools_def.txt?

Mike

> -----Original Message-----
> From: devel= @edk2.groups.io <devel@edk2.groups.io> On Behalf Of Cheng-Chieh Huang via groups.io
> Sent: Wednesday, July 21, 2021 6:23 AM
> To: devel@e= dk2.groups.io
> Cc: Cheng-Chieh Huang <chengchieh@google.com>
> Subject: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_= SSE to avoid generating floating points operation
>
> This will allow we compile payload using gcc8
>
> Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
> ---
>=C2=A0 UefiPayloadPkg/UefiPayloadPkg.dsc | 5 +++++
>=C2=A0 1 file changed, 5 insertions(+)
>
> diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiP= ayloadPkg.dsc
> index 8aa5f18cd35c..fa41c5a24af5 100644
> --- a/UefiPayloadPkg/UefiPayloadPkg.dsc
> +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
> @@ -30,6 +30,8 @@ [Defines]
>=C2=A0 =C2=A0 DEFINE PS2_KEYBOARD_ENABLE=C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =3D FALSE
>=C2=A0 =C2=A0 DEFINE UNIVERSAL_PAYLOAD=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =3D FALSE
>
> +=C2=A0 DEFINE DISABLE_MMX_SSE=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =3D FALSE
> +
>=C2=A0 =C2=A0 #
>=C2=A0 =C2=A0 # SBL:=C2=A0 =C2=A0 =C2=A0 UEFI payload for Slim Bootloa= der
>=C2=A0 =C2=A0 # COREBOOT: UEFI payload for coreboot
> @@ -96,6 +98,9 @@ [BuildOptions]
>=C2=A0 =C2=A0 *_*_*_CC_FLAGS=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = = =C2=A0 =C2=A0 =C2=A0=3D -D DISABLE_NEW_DEPRECATED_INTERFACES
>=C2=A0 !if $(BOOTLOADER) =3D=3D "LINUXBOOT"
>=C2=A0 =C2=A0 *_*_*_CC_FLAGS=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = = =C2=A0 =C2=A0 =C2=A0=3D -D LINUXBOOT_PAYLOAD
> +!endif
> +!if $(DISABLE_MMX_SSE)
> +=C2=A0 *_*_*_CC_FLAGS=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0=3D -mno-mmx -mno-sse
>=C2=A0 !endif
>=C2=A0 =C2=A0 GCC:*_UNIXGCC_*_CC_FLAGS=C2=A0 =C2=A0 =C2=A0 =C2=A0=3D -= DMDEPKG_NDEBUG
>=C2=A0 =C2=A0 GCC:RELEASE_*_*_CC_FLAGS=C2=A0 =C2=A0 =C2=A0 =C2=A0=3D -= DMDEPKG_NDEBUG
> --
> 2.32.0.402.g57bb445576-goog
>
>
>
>
>

--000000000000397ecd05c7cd9428--