from:"Jake Garver via groups.io"

[edk2-devel] [PATCH v2] BaseTools/GenFw: Correct offset when relocating an ADR

2023-12-20 Thread Jake Garver via groups.io

In the R_AARCH64_ADR_GOT_PAGE case on AARCH64, we may encounter an ADR
instead of an ADRP when the toolchain is working around Cortex-A53
erratum #843419.  If that's the case, be sure to calculate the offset
appropriately.

This resolves an issue experienced when building a StandaloneMm image
with stack protection enabled on GCC compiled with
"--enable-fix-cortex-a53-843419".  This scenario sometimes generates an
ADR with a R_AARCH64_ADR_GOT_PAGE relocation.

In this scenario, the following code is being generated by the
toolchain:

# Load to set the stack canary
2ffc:   10028020adr x0, 8000 
3008:   f940d400ldr x0, [x0, #424]

# Load to check the stack canary
30cc:   b020adrpx0, 8000 
30d0:   f940d400ldr x0, [x0, #424]

GenFw rewrote that to:

# Load to set the stack canary
2ffc:   1480adr x0, 0x308c
3008:   912ec000add x0, x0, #0xbb0

# Load to check the stack canary
30cc:   f460adrpx0, 0x92000
30d0:   912ec000add x0, x0, #0xbb0

Note that we're now setting the stack canary from the wrong address,
resulting in an erroneous stack fault.

After this fix, the offset will be calculated correctly for an ADR and
the stack canary is set correctly.

Signed-off-by: Jake Garver 
---

Notes:
  v2: Implement approach proposed by Ard Biesheuvel.
- title changed to: Correct offset when relocating an ADR
  v1: Original title: Change opcode when converting ADR to ADRP

 BaseTools/Source/C/GenFw/Elf64Convert.c | 22 +-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c 
b/BaseTools/Source/C/GenFw/Elf64Convert.c
index 9911db65af..9d04fc612e 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -1562,7 +1562,27 @@ WriteSections64 (
 // subsequent LDR instruction (covered by a 
R_AARCH64_LD64_GOT_LO12_NC
 // relocation) into an ADD instruction - this is handled above.
 //
-Offset = (Sym->st_value - (Rel->r_offset & ~0xfff)) >> 12;
+// In order to handle Cortex-A53 erratum #843419, the GCC toolchain
+// may convert an ADRP instruction at the end of a page (0xffc
+// offset) into an ADR instruction. If so, be sure to calculate the
+// offset for an ADR instead of ADRP.
+//
+if ((*(UINT32 *)Targ & BIT31) == 0) {
+  //
+  // Calculate the offset for an ADR.
+  //
+  Offset = (Sym->st_value & ~0xfff) - Rel->r_offset;
+  if (Offset < -0x10 || Offset > 0xf) {
+Error (NULL, 0, 3000, "Invalid", "WriteSections64(): %s  due 
to its size (> 1 MB), unable to relocate ADR.",
+  mInImageName);
+break;
+  }
+} else {
+  //
+  // Calculate the offset for an ADRP.
+  //
+  Offset = (Sym->st_value - (Rel->r_offset & ~0xfff)) >> 12;
+}
 
 *(UINT32 *)Targ &= 0x901f;
 *(UINT32 *)Targ |= ((Offset & 0x1c) << (5 - 2)) | ((Offset & 
0x3) << 29);
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112769): https://edk2.groups.io/g/devel/message/112769
Mute This Topic: https://groups.io/mt/103287393/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP

2023-12-19 Thread Jake Garver via groups.io

Ard, Pedro,

It's all connecting for me:

  *   I was able to recreate this problem with a crosstool-ng built toolchain.  
That confirms it's not specific to Ubuntu and is indeed related to 
"--enable-fix-cortex-a53-843419".
  *   I also verified that this issue is specific to our StandaloneMm-based 
images because of the use of -fpie.

Next up, I'll put together a patch to match Ard's below and test it.

Thanks,
Jake

From: Jake Garver 
Sent: Wednesday, December 13, 2023 2:47 PM
To: Pedro Falcato ; Ard Biesheuvel 
Cc: devel@edk2.groups.io ; rebe...@bsdio.com 
; gaolim...@byosoft.com.cn ; 
bob.c.f...@intel.com ; yuwei.c...@intel.com 

Subject: Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when 
converting ADR to ADRP

Fantastic!

When we hit this in GCC 12.3 toolchain, the ADR was indeed at a 0xffc page 
offset.  So, that connects.

This also explains why the issue seemed to be specific to stack protection: 
Because it's comparing values right away.  If we hit this with other loads, we 
might not notice until later or at all.

I have one more dot to connect:  When I built crosstool-ng, I was using 
"--enable-fix-cortex-a53-843419".  However, I'm guessing I wasn't lucky enough 
to end up with an ADRP at the end of a page.  I'll try to manufacture that 
situation as further confirmation.

Thanks,
Jake

From: Pedro Falcato 
Sent: Wednesday, December 13, 2023 1:01 PM
To: Ard Biesheuvel 
Cc: Jake Garver ; devel@edk2.groups.io ; 
rebe...@bsdio.com ; gaolim...@byosoft.com.cn 
; bob.c.f...@intel.com ; 
yuwei.c...@intel.com 
Subject: Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when 
converting ADR to ADRP

External email: Use caution opening links or attachments

On Wed, Dec 13, 2023 at 5:31 PM Ard Biesheuvel  wrote:
>
> On Wed, 13 Dec 2023 at 15:58, Jake Garver  wrote:
> >
> > Totally understand and agree, Ard.
> >
> > In the meantime, I've now experienced the issue with Ubuntu22's GCC 12.3.  
> > Originally, we didn't see the issue on this toolchain, but a developer ran 
> > into when preparing a change.  Even more concerning, when I instrumented 
> > that change, it went away.  So, it seems to be very sensitive to the input, 
> > which will make it hard to reproduce.
> >
> > Specifically, like the Ubuntu20 10.5 toolchain, the Ubuntu 12.3 toolchain 
> > generated an R_AARCH64_ADR_GOT_PAGE relocation against an ADR instruction.  
> > Further, it was when loading the value of __stack_chk_guard.
> >
> > I was again unable to reproduce this using a crosstool-ng build of GCC 
> > 12.3, even when matching the ./configure arguments.
> >
> > Since it's now reproducible in a toolchain we're actively using, I'll 
> > continue looking at it.  I'll let you know what I find.
>
> OK, mystery solved.
>
> # Load to set the stack canary
> 2ffc:   1480adr x0, 0x308c
> 3008:   912ec000add x0, x0, #0xbb0
>
> The location of the ADRP instruction is at the end of a 4k page
> (0xffc), which could trigger erratum #843419 on Cortex-A53, and is
> therefore converted into ADR.

Ha! Great deduction! And because GCC builds don't turn on the a53 ADRP
errata by default, the toolchains Jake built weren't catching this
issue.

--
Pedro

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112735): https://edk2.groups.io/g/devel/message/112735
Mute This Topic: https://groups.io/mt/102202314/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP

2023-12-13 Thread Jake Garver via groups.io

Fantastic!

When we hit this in GCC 12.3 toolchain, the ADR was indeed at a 0xffc page 
offset.  So, that connects.

This also explains why the issue seemed to be specific to stack protection: 
Because it's comparing values right away.  If we hit this with other loads, we 
might not notice until later or at all.

I have one more dot to connect:  When I built crosstool-ng, I was using 
"--enable-fix-cortex-a53-843419".  However, I'm guessing I wasn't lucky enough 
to end up with an ADRP at the end of a page.  I'll try to manufacture that 
situation as further confirmation.

Thanks,
Jake

From: Pedro Falcato 
Sent: Wednesday, December 13, 2023 1:01 PM
To: Ard Biesheuvel 
Cc: Jake Garver ; devel@edk2.groups.io ; 
rebe...@bsdio.com ; gaolim...@byosoft.com.cn 
; bob.c.f...@intel.com ; 
yuwei.c...@intel.com 
Subject: Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when 
converting ADR to ADRP

External email: Use caution opening links or attachments


On Wed, Dec 13, 2023 at 5:31 PM Ard Biesheuvel  wrote:
>
> On Wed, 13 Dec 2023 at 15:58, Jake Garver  wrote:
> >
> > Totally understand and agree, Ard.
> >
> > In the meantime, I've now experienced the issue with Ubuntu22's GCC 12.3.  
> > Originally, we didn't see the issue on this toolchain, but a developer ran 
> > into when preparing a change.  Even more concerning, when I instrumented 
> > that change, it went away.  So, it seems to be very sensitive to the input, 
> > which will make it hard to reproduce.
> >
> > Specifically, like the Ubuntu20 10.5 toolchain, the Ubuntu 12.3 toolchain 
> > generated an R_AARCH64_ADR_GOT_PAGE relocation against an ADR instruction.  
> > Further, it was when loading the value of __stack_chk_guard.
> >
> > I was again unable to reproduce this using a crosstool-ng build of GCC 
> > 12.3, even when matching the ./configure arguments.
> >
> > Since it's now reproducible in a toolchain we're actively using, I'll 
> > continue looking at it.  I'll let you know what I find.
>
> OK, mystery solved.
>
> # Load to set the stack canary
> 2ffc:   1480adr x0, 0x308c
> 3008:   912ec000add x0, x0, #0xbb0
>
> The location of the ADRP instruction is at the end of a 4k page
> (0xffc), which could trigger erratum #843419 on Cortex-A53, and is
> therefore converted into ADR.

Ha! Great deduction! And because GCC builds don't turn on the a53 ADRP
errata by default, the toolchains Jake built weren't catching this
issue.

--
Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112490): https://edk2.groups.io/g/devel/message/112490
Mute This Topic: https://groups.io/mt/102202314/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP

2023-12-13 Thread Jake Garver via groups.io

Totally understand and agree, Ard.

In the meantime, I've now experienced the issue with Ubuntu22's GCC 12.3.  
Originally, we didn't see the issue on this toolchain, but a developer ran into 
when preparing a change.  Even more concerning, when I instrumented that 
change, it went away.  So, it seems to be very sensitive to the input, which 
will make it hard to reproduce.

Specifically, like the Ubuntu20 10.5 toolchain, the Ubuntu 12.3 toolchain 
generated an R_AARCH64_ADR_GOT_PAGE relocation against an ADR instruction.  
Further, it was when loading the value of __stack_chk_guard.

I was again unable to reproduce this using a crosstool-ng build of GCC 12.3, 
even when matching the ./configure arguments.

Since it's now reproducible in a toolchain we're actively using, I'll continue 
looking at it.  I'll let you know what I find.

Thanks,
Jake

From: Ard Biesheuvel 
Sent: Tuesday, December 12, 2023 4:22 AM
To: Jake Garver 
Cc: Pedro Falcato ; devel@edk2.groups.io 
; rebe...@bsdio.com ; 
gaolim...@byosoft.com.cn ; bob.c.f...@intel.com 
; yuwei.c...@intel.com 
Subject: Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when 
converting ADR to ADRP

External email: Use caution opening links or attachments

On Wed, 6 Dec 2023 at 17:51, Jake Garver  wrote:
>
> Thanks, Pedro and Ard,
>
> An update on this issue:
>
> It seems to be very specific to Ubuntu20's 10.5 build of GCC.
> I could not reproduce it using a crosstool-ng build of 10.5, even after 
> trying to configure it identically to Ubuntu20's.  It might be something in 
> Ubuntu's patchset, but nothing stuck out to me there.
> We have migrated to Ubuntu22 as a preferred platform.  Their build of GCC 
> 12.1 and 12.3 do not have this issue.  As a result, this issue is no longer a 
> high runner for us.
>
>
> Next step: Try reproducing it by rebuilding Ubuntu's GCC debs.  I still want 
> to get to the bottom of this, just to make sure it doesn't pop-up in later 
> builds on GCC on Ubuntu.
>
> Pedro: I haven't attempted to build without LTO yet.  That looked painful to 
> setup, so I didn't make it a priority.
>

Thanks for the update. If this appears to be specific to Ubuntu's GCC
build, and the issue is no longer reproducible on more recent builds,
I'd be inclined not to bother. But if you do insist and find the
culprit, I'd be happy to take another look.

--
Ard.

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112477): https://edk2.groups.io/g/devel/message/112477
Mute This Topic: https://groups.io/mt/102202314/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP

2023-12-06 Thread Jake Garver via groups.io

Thanks, Pedro and Ard,

An update on this issue:

  *   It seems to be very specific to Ubuntu20's 10.5 build of GCC.
  *   I could not reproduce it using a crosstool-ng build of 10.5, even after 
trying to configure it identically to Ubuntu20's.  It might be something in 
Ubuntu's patchset, but nothing stuck out to me there.
  *   We have migrated to Ubuntu22 as a preferred platform.  Their build of GCC 
12.1 and 12.3 do not have this issue.  As a result, this issue is no longer a 
high runner for us.

Next step: Try reproducing it by rebuilding Ubuntu's GCC debs.  I still want to 
get to the bottom of this, just to make sure it doesn't pop-up in later builds 
on GCC on Ubuntu.

Pedro: I haven't attempted to build without LTO yet.  That looked painful to 
setup, so I didn't make it a priority.

Thanks again,
Jake

From: Pedro Falcato 
Sent: Thursday, November 2, 2023 8:47 AM
To: Jake Garver 
Cc: Ard Biesheuvel ; devel@edk2.groups.io 
; rebe...@bsdio.com ; 
gaolim...@byosoft.com.cn ; bob.c.f...@intel.com 
; yuwei.c...@intel.com 
Subject: Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when 
converting ADR to ADRP

External email: Use caution opening links or attachments

On Thu, Nov 2, 2023 at 11:47 AM Jake Garver  wrote:
>
> Ard, Pedro,
>
> How would you like to proceed here?
>
> Do nothing.  Treat it as a toolchain bug and not attempt a work-around.  
> Practically speaking, this means the Ubuntu20 container at 
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Fcontainers&data=05%7C01%7Cjake%40nvidia.com%7C0977cd2f66b749f07d7508dbdba1e777%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638345260662824037%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=Ef71H%2FaGAFezT%2Fs6YBanbDnUQge%2B%2F6YZ3OLRI6MuAb8%3D&reserved=0
>  cannot be updated to the latest GCC 10.x (10.5).  We can pin it to 10.3 or 
> EOL it, migrating to Ubuntu22.
> Rework the patch as a work-around.
> ??? Something I missed.
>
> Note in option 1, I'm assuming that other versions of GCC, e.g. 12.x used in 
> Ubuntu22, don't eventually inherit this regression. But we can always revisit 
> the work-around if we determine that to be the case.  I'm starting Ubuntu22 
> testing now, so I may have an answer soon.

I think the correct way to proceed here is to find out what exactly is
causing this. Once we find out, we can edit the patch's commit message
and push (or drop the patch, depending on what the problem is).
The patch itself looks fine to me (And I already Rb'd it I think), and
it should be harmless to apply it, even for toolchains that do not
share this mysterious problem.

Also, FWIW: GCC 10.5 is a stable release, so I find it hard to believe
that something actually broke between 10.4 and 10.5.

One more thing: What happens if you build without LTO?

--
Pedro

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112133): https://edk2.groups.io/g/devel/message/112133
Mute This Topic: https://groups.io/mt/102202314/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2] BaseStackCheckLib: Fix STACK FAULT message

2023-12-06 Thread Jake Garver via groups.io

Any further comments on this change?

I'd like to get it merged.

Thanks,
Jake

From: Jake Garver 
Sent: Wednesday, October 18, 2023 10:45 AM
To: gaoliming ; devel@edk2.groups.io 

Cc: michael.d.kin...@intel.com ; 
zhiguang@intel.com 
Subject: Re: [PATCH v2] BaseStackCheckLib: Fix STACK FAULT message

Thanks for the review, Liming Gao.

Any further comments on this change?

Thanks,
Jake

From: gaoliming 
Sent: Saturday, October 7, 2023 1:05 AM
To: Jake Garver ; devel@edk2.groups.io 
Cc: michael.d.kin...@intel.com ; 
zhiguang@intel.com 
Subject: 回复: [PATCH v2] BaseStackCheckLib: Fix STACK FAULT message

External email: Use caution opening links or attachments


Reviewed-by: Liming Gao 

> -邮件原件-
> 发件人: Jake Garver 
> 发送时间: 2023年10月6日 0:19
> 收件人: devel@edk2.groups.io
> 抄送: michael.d.kin...@intel.com; gaolim...@byosoft.com.cn;
> zhiguang@intel.com; Jake Garver 
> 主题: [PATCH v2] BaseStackCheckLib: Fix STACK FAULT message
>
> __builtin_return_address returns a pointer, not a string.  Fix the STACK
> FAULT message in BaseStackCheckLib appropriately.
>
> Signed-off-by: Jake Garver 
> ---
>  MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c
> b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c
> index 0d2918668e..ea168841b6 100644
> --- a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c
> +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c
> @@ -6,6 +6,7 @@
>   to exiting the function. If the "canary" is overwritten
__stack_chk_fail()
>   is called. This is GCC specific code.
>
> + Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights
reserved.
>   Copyright (c) 2012, Apple Inc. All rights reserved.
>   SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -34,7 +35,7 @@ __stack_chk_fail (
>  {
>UINT8  DebugPropertyMask;
>
> -  DEBUG ((DEBUG_ERROR, "STACK FAULT: Buffer Overflow in
> function %a.\n", __builtin_return_address (0)));
> +  DEBUG ((DEBUG_ERROR, "STACK FAULT: Buffer Overflow at 0x%p.\n",
> RETURN_ADDRESS (0)));
>
>//
>// Generate a Breakpoint, DeadLoop, or NOP based on PCD settings even
> if
> --
> 2.34.1





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112131): https://edk2.groups.io/g/devel/message/112131
Mute This Topic: https://groups.io/mt/102040342/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP

2023-11-02 Thread Jake Garver via groups.io

Ard, Pedro,

How would you like to proceed here?

  1.
Do nothing.  Treat it as a toolchain bug and not attempt a work-around.  
Practically speaking, this means the Ubuntu20 container at 
https://github.com/tianocore/containers cannot be updated to the latest GCC 
10.x (10.5).  We can pin it to 10.3 or EOL it, migrating to Ubuntu22.
  2.  Rework the patch as a work-around.
  3.  ??? Something I missed.

Note in option 1, I'm assuming that other versions of GCC, e.g. 12.x used in 
Ubuntu22, don't eventually inherit this regression. But we can always revisit 
the work-around if we determine that to be the case.  I'm starting Ubuntu22 
testing now, so I may have an answer soon.

Thanks,
Jake

From: Jake Garver 
Sent: Friday, October 27, 2023 11:52 AM
To: Ard Biesheuvel ; Pedro Falcato 
Cc: devel@edk2.groups.io ; rebe...@bsdio.com 
; gaolim...@byosoft.com.cn ; 
bob.c.f...@intel.com ; yuwei.c...@intel.com 

Subject: Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when 
converting ADR to ADRP

Ard, Pedro,

> Could you try adding -Wl,--no-relax to the DLINK_FLAGS for your build and see 
> if it makes a difference?

With "-Wl,--no-relax", I still see an ADR instruction.  Clean build.  Verified 
the "-Wl,--no-relax" is part of the gcc all to build this dll.

2fec :
2fec:   a9b97bfd    stp   x29, x30, [sp, #-112]!
2ff0:   910003fd    mov   x29, sp
2ff4:   a90363f7    stp   x23, x24, [sp, #48]
2ff8:   aa0003f8    mov   x24, x0
2ffc:   10020020    adr   x0, 7000 <_cont+0xe98>
  2ffc: R_AARCH64_ADR_GOT_PAGE  __stack_chk_guard
3000:   a90153f3    stp   x19, x20, [sp, #16]
3004:   aa0103f3    mov   x19, x1
3008:   f946cc00    ldr   x0, [x0, #3480]
  3008: R_AARCH64_LD64_GOT_LO12_NC__stack_chk_guard

How do you think I should proceed?

Thanks for all your help!
Jake

From: Ard Biesheuvel 
Sent: Friday, October 27, 2023 10:43 AM
To: Pedro Falcato 
Cc: devel@edk2.groups.io ; Jake Garver ; 
rebe...@bsdio.com ; gaolim...@byosoft.com.cn 
; bob.c.f...@intel.com ; 
yuwei.c...@intel.com 
Subject: Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when 
converting ADR to ADRP

External email: Use caution opening links or attachments

On Fri, 27 Oct 2023 at 16:26, Pedro Falcato  wrote:
>
> On Fri, Oct 27, 2023 at 3:13 PM Ard Biesheuvel  wrote:
> >
> > On Fri, 27 Oct 2023 at 16:09, Jake Garver via groups.io
> >  wrote:
> > >
> > > Hi Ard:
> > >
> > > > Can you double check the object file? I suspect this is a linker 
> > > > relaxation not a compiler issue.
> > >
> > > With LTO in play, is there a way to check the object file?  It's not in 
> > > aarch64 assembly.
> > >
> >
> > Perhaps not.
> >
> > Could you try adding -Wl,--no-relax to the DLINK_FLAGS for your build
> > and see if it makes a difference?
> >
> > We should be adding that in any case, but it would be good to know if
> > it helps here too.
>
> I've never checked on aarch64, but don't you get solid space savings
> with linker relaxation? Or is it mostly meaningless?
>

The most important use case for linker relaxation is turning GOT
lookups into relative references, given that PIC object files are
typically constructed without prior knowledge of whether a given
external reference will be satisfied by the same dynamic object or a
different one. So each relaxation removes a load from the program and
may reduce the size of the GOT (if no other non-relaxable references
to it exist) and this might matter on a hot path.

None of this makes sense for UEFI, though, given that all executables
are fully linked and performance doesn't usually matter to the same
extent.

On AArch64, there is a specific advantage to being able to relax ADRP
to ADR (and this is why GenFw implements this relaxation itself too):
ADRP instructions need to appear at the same offset modulo 4k as they
were linked at, which means 4k alignment for the entire code section.
EDK2 builds are made up of lots of tiny SEC and PEI drivers and
rounding up each of those to 4k would result in a substantial increase
in footprint of the firmware image.

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110524): https://edk2.groups.io/g/devel/message/110524
Mute This Topic: https://groups.io/mt/102202314/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP

2023-10-27 Thread Jake Garver via groups.io

Ard, Pedro,

> Could you try adding -Wl,--no-relax to the DLINK_FLAGS for your build and see 
> if it makes a difference?

With "-Wl,--no-relax", I still see an ADR instruction.  Clean build.  Verified 
the "-Wl,--no-relax" is part of the gcc all to build this dll.

2fec :
2fec:   a9b97bfd    stp   x29, x30, [sp, #-112]!
2ff0:   910003fd    mov   x29, sp
2ff4:   a90363f7    stp   x23, x24, [sp, #48]
2ff8:   aa0003f8    mov   x24, x0
2ffc:   10020020    adr   x0, 7000 <_cont+0xe98>
  2ffc: R_AARCH64_ADR_GOT_PAGE  __stack_chk_guard
3000:   a90153f3    stp   x19, x20, [sp, #16]
3004:   aa0103f3    mov   x19, x1
3008:   f946cc00    ldr   x0, [x0, #3480]
  3008: R_AARCH64_LD64_GOT_LO12_NC__stack_chk_guard

How do you think I should proceed?

Thanks for all your help!
Jake

From: Ard Biesheuvel 
Sent: Friday, October 27, 2023 10:43 AM
To: Pedro Falcato 
Cc: devel@edk2.groups.io ; Jake Garver ; 
rebe...@bsdio.com ; gaolim...@byosoft.com.cn 
; bob.c.f...@intel.com ; 
yuwei.c...@intel.com 
Subject: Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when 
converting ADR to ADRP

External email: Use caution opening links or attachments

On Fri, 27 Oct 2023 at 16:26, Pedro Falcato  wrote:
>
> On Fri, Oct 27, 2023 at 3:13 PM Ard Biesheuvel  wrote:
> >
> > On Fri, 27 Oct 2023 at 16:09, Jake Garver via groups.io
> >  wrote:
> > >
> > > Hi Ard:
> > >
> > > > Can you double check the object file? I suspect this is a linker 
> > > > relaxation not a compiler issue.
> > >
> > > With LTO in play, is there a way to check the object file?  It's not in 
> > > aarch64 assembly.
> > >
> >
> > Perhaps not.
> >
> > Could you try adding -Wl,--no-relax to the DLINK_FLAGS for your build
> > and see if it makes a difference?
> >
> > We should be adding that in any case, but it would be good to know if
> > it helps here too.
>
> I've never checked on aarch64, but don't you get solid space savings
> with linker relaxation? Or is it mostly meaningless?
>

The most important use case for linker relaxation is turning GOT
lookups into relative references, given that PIC object files are
typically constructed without prior knowledge of whether a given
external reference will be satisfied by the same dynamic object or a
different one. So each relaxation removes a load from the program and
may reduce the size of the GOT (if no other non-relaxable references
to it exist) and this might matter on a hot path.

None of this makes sense for UEFI, though, given that all executables
are fully linked and performance doesn't usually matter to the same
extent.

On AArch64, there is a specific advantage to being able to relax ADRP
to ADR (and this is why GenFw implements this relaxation itself too):
ADRP instructions need to appear at the same offset modulo 4k as they
were linked at, which means 4k alignment for the entire code section.
EDK2 builds are made up of lots of tiny SEC and PEI drivers and
rounding up each of those to 4k would result in a substantial increase
in footprint of the firmware image.

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110219): https://edk2.groups.io/g/devel/message/110219
Mute This Topic: https://groups.io/mt/102202314/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP

2023-10-27 Thread Jake Garver via groups.io

Hi Ard:

> Can you double check the object file? I suspect this is a linker relaxation 
> not a compiler issue.

With LTO in play, is there a way to check the object file?  It's not in aarch64 
assembly.

Thanks,
Jake

From: Ard Biesheuvel 
Sent: Friday, October 27, 2023 9:46 AM
To: devel@edk2.groups.io ; Jake Garver 
Cc: Pedro Falcato ; rebe...@bsdio.com 
; gaolim...@byosoft.com.cn ; 
bob.c.f...@intel.com ; yuwei.c...@intel.com 
; ardb+tianoc...@kernel.org 
Subject: Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when 
converting ADR to ADRP

External email: Use caution opening links or attachments


Hello all,

Apologies for the late response.

On Fri, 27 Oct 2023 at 14:44, Jake Garver via groups.io
 wrote:
>
> Thanks for your response, Pedro.
>
> I chased this as a toolchain bug originally, but concluded that the ADR 
> indeed works before GenFw rewrites it.  But I see your point regarding the 
> relocation statement.
>
> As requested, below is the disassembled function along with relocations.  
> This was generated from the dll using "aarch64-linux-gnu-objdump -r -D".  The 
> ADR in question is at 2ffc.
>

Can you double check the object file? I suspect this is a linker
relaxation not a compiler issue.

> This code was generated by the current version of the GCC 10.x toolchain on 
> Ubuntu20.  So, if we're concluding this is a toolchain issue, then it's with 
> a fairly "stock" toolchain.
>
> 2fec :
> 2fec:   a9b97bfd    stp   x29, x30, [sp, #-112]!
> 2ff0:   910003fd    mov   x29, sp
> 2ff4:   a90363f7    stp   x23, x24, [sp, #48]
> 2ff8:   aa0003f8    mov   x24, x0
> 2ffc:   10020020    adr   x0, 7000 <_cont+0xe98>
>   2ffc: R_AARCH64_ADR_GOT_PAGE  __stack_chk_guard

The nasty thing with relying on --emit-relocs is that they get out of
sync. R_AARCH64_ADR_GOT_PAGE is documented as applying to ADRP only,
so this code is non-compliant one way or the other.

There are other relaxations that also confuse GenFw when the static
relocs don't get updated accordingly.


> 3000:   a90153f3    stp   x19, x20, [sp, #16]
> 3004:   aa0103f3    mov   x19, x1
> 3008:   f946cc00    ldr   x0, [x0, #3480]
>   3008: R_AARCH64_LD64_GOT_LO12_NC__stack_chk_guard
> 300c:   a9025bf5    stp   x21, x22, [sp, #32]
> 3010:   a9046bf9    stp   x25, x26, [sp, #64]
> 3014:   f9002bfb    str   x27, [sp, #80]
> 3018:   f941    ldr   x1, [x0]
> 301c:   f90037e1    str   x1, [sp, #104]
> 3020:   d281    mov   x1, #0x0      // #0
> 3024:   aa1303e0    mov   x0, x19
> 3028:   97fffd04    bl2438 
>   3028: R_AARCH64_CALL26  .text+0x1438
> 302c:   aa0003f5    mov   x21, x0
> 3030:   aa1803e0    mov   x0, x24
> 3034:   97fff807    bl1050 
>   3034: R_AARCH64_CALL26  .text+0x50
> 3038:   2a0003f4    mov   w20, w0
> 303c:   35000260    cbnz  w0, 3088 
> 3040:   39400260    ldrb  w0, [x19]
> 3044:   93407ea1    sxtw  x1, w21
> 3048:   8b35c275    add   x21, x19, w21, sxtw
> 304c:   7100bc1f    cmp   w0, #0x2f
> 3050:   54000420    b.eq  30d4   // b.none
> 3054:   528005e2    mov   w2, #0x2f     // #47
> 3058:   aa1303e0    mov   x0, x19
> 305c:   972f    bl2d18 
>   305c: R_AARCH64_CALL26  .text+0x1d18
> 3060:   f11f    cmp   x0, #0x0
> 3064:   9a951016    csel  x22, x0, x21, ne  // ne = any
> 3068:   f001    adrp  x1, 6000 <__stack_chk_fail+0x7c>
>   3068: R_AARCH64_ADR_PREL_PG_HI21.text+0x58b6
> 306c:   9122d821    add   x1, x1, #0x8b6
>   306c: R_AARCH64_ADD_ABS_LO12_NC .text+0x58b6
> 3070:   aa1803e0    mov   x0, x24
> 3074:   cb1302d4    sub   x20, x22, x19
> 3078:   97dd    bl2fec 
> 307c:   2a0003e1    mov   w1, w0
> 3080:   36f80140    tbz   w0, #31, 30a8 
> 3084:   12800094    mov   w20, #0xfffb  // #-5
> 3088:   9020    adrp  x0, 7000 <_cont+0xe98>
>   3088: R_AARCH64_ADR_GOT_PAGE  __stack_chk_guard
> 308c:   f946cc00    ldr   x0, [x0, #3480]
>   308c: R_AARCH64_LD64_GOT_LO12_NC__stack_chk_guard
> 3090:   f94037e1    ldr   x1, [sp, #104]
> 3094:   f942    ldr   x2, [x0]
> 3098:   eb020021    subs  x1, x1, x2
> 309c:   d282    mov   x2, #0x0      // #0
> 30a0:   54000a00    b.eq  31e0   // b.none
> 30a4:   94000bb8    bl5f84 <__stack_chk_fail>
>   30a4: R_AARCH64_CALL26  __stack_chk_fail
> 30a8:   2a1403e3    mov

Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP

2023-10-27 Thread Jake Garver via groups.io

2c:   7100029f    cmp   w20, #0x0
3130:   7a40a801    ccmp  w0, #0x0, #0x1, ge  // ge = tcont
3134:   548a    b.ge  3144   // b.tcont
3138:   36f803c0    tbz   w0, #31, 31b0 
313c:   12800014    mov   w20, #0x  // #-1
3140:   17d2    b 3088 
3144:   7100041f    cmp   w0, #0x1
3148:   54e0    b.eq  3164   // b.none
314c:   2a1403e1    mov   w1, w20
3150:   aa1a03e2    mov   x2, x26
3154:   aa1803e0    mov   x0, x24
3158:   97fff87c    bl1348 
  3158: R_AARCH64_CALL26  .text+0x348
315c:   2a0003f4    mov   w20, w0
3160:   17f2    b 3128 
3164:   2a1b03e2    mov   w2, w27
3168:   11001281    add   w1, w20, #0x4
316c:   aa1803e0    mov   x0, x24
3170:   97fff7da    bl10d8 
  3170: R_AARCH64_CALL26  .text+0xd8
3174:   aa0003f9    mov   x25, x0
3178:   b4fffea0    cbz   x0, 314c 
317c:   f10002ff    cmp   x23, #0x0
3180:   fa4012c4    ccmp  x22, x0, #0x4, ne  // ne = any
3184:   54000201    b.ne  31c4   // b.any
3188:   38776b20    ldrb  w0, [x25, x23]
318c:   34000120    cbz   w0, 31b0 
3190:   aa1703e1    mov   x1, x23
3194:   aa1603e0    mov   x0, x22
3198:   52800802    mov   w2, #0x40     // #64
319c:   97fffedf    bl2d18 
  319c: R_AARCH64_CALL26  .text+0x1d18
31a0:   b5fffd60    cbnz  x0, 314c 
31a4:   38776b20    ldrb  w0, [x25, x23]
31a8:   7101001f    cmp   w0, #0x40
31ac:   54fffd01    b.ne  314c   // b.any
31b0:   37fff6d4    tbnz  w20, #31, 3088 
31b4:   eb1302bf    cmp   x21, x19
31b8:   54fff689    b.ls  3088   // b.plast
31bc:   aa1303f6    mov   x22, x19
31c0:   17ca    b 30e8 
31c4:   aa1703e2    mov   x2, x23
31c8:   aa1603e1    mov   x1, x22
31cc:   97fffef7    bl2da8 
  31cc: R_AARCH64_CALL26  .text+0x1da8
31d0:   35fffbe0    cbnz  w0, 314c 
31d4:   17ed    b 3188 
31d8:   2a0003f4    mov   w20, w0
31dc:   17f5    b 31b0 
31e0:   2a1403e0    mov   w0, w20
31e4:   a94153f3    ldp   x19, x20, [sp, #16]
31e8:   a9425bf5    ldp   x21, x22, [sp, #32]
31ec:   a94363f7    ldp   x23, x24, [sp, #48]
31f0:   a9446bf9    ldp   x25, x26, [sp, #64]
31f4:   f9402bfb    ldr   x27, [sp, #80]
31f8:   a8c77bfd    ldp   x29, x30, [sp], #112
31fc:   d65f03c0    ret
3200:   14000400    b 4200 
3204:   d503201f    nop
3208:   f946cc00    ldr   x0, [x0, #3480]
320c:   1780    b 300c 

Jake

From: Pedro Falcato 
Sent: Thursday, October 26, 2023 2:46 PM
To: devel@edk2.groups.io ; Jake Garver 
Cc: rebe...@bsdio.com ; gaolim...@byosoft.com.cn 
; bob.c.f...@intel.com ; 
yuwei.c...@intel.com ; ardb+tianoc...@kernel.org 

Subject: Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when 
converting ADR to ADRP

External email: Use caution opening links or attachments


On Thu, Oct 26, 2023 at 4:32 PM Jake Garver via groups.io
 wrote:
>
> In the R_AARCH64_ADR_GOT_PAGE case on AARCH64, be sure to change the
> opcode to ADRP.  Prior to this change, we updated the address, but not
> the opcode.
>
> This resolves an issue experienced when building a StandaloneMm image
> with stack protection enabled on GCC 10.5.  This scenario generates an
> ADR where an ADRP is more common in other versions of GCC tested.  That
> explains the obscurity of the issue.  However, an ADR is valid and
> should be handled by GenFw.

Is this not a toolchain bug?
The aarch64 ELF ABI
(https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FARM-software%2Fabi-aa%2Fblob%2Fmain%2Faaelf64%2Faaelf64.rst&data=05%7C01%7Cjake%40nvidia.com%7C607b433f7ddc40266e3c08dbd653f1c1%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638339428273243439%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=6DcbbXjY4UfjTwAUpd525B%2BvqPEWkZ1RStdc62%2F2er4%3D&reserved=0<https://github.com/ARM-software/abi-aa/blob/main/aaelf64/aaelf64.rst>)
says:
"Set the immediate value of an ADRP to bits [32:12] of X; check that
–232 <= X < 232"

So it mentions this relocation pointing *to an ADRP* specifically. And
AFAIK there's no way
Page(G(GDAT(S+A)))-Page(P)

would ever make sense if we're looking at an adr and not an adrp.

Can you post the full disassembly for the function, plus relevant relocations?

--
Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110205): https://edk2.groups.io/g/devel/message/110205
Mute This Topic: https://groups.io/mt/102202314/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP

2023-10-26 Thread Jake Garver via groups.io

In the R_AARCH64_ADR_GOT_PAGE case on AARCH64, be sure to change the
opcode to ADRP.  Prior to this change, we updated the address, but not
the opcode.

This resolves an issue experienced when building a StandaloneMm image
with stack protection enabled on GCC 10.5.  This scenario generates an
ADR where an ADRP is more common in other versions of GCC tested.  That
explains the obscurity of the issue.  However, an ADR is valid and
should be handled by GenFw.

Using the stack protection scenario as an example, the following code is
being generated by the toolchain:

# Load to set the stack canary
2ffc:   10028020adr x0, 8000 
3008:   f940d400ldr x0, [x0, #424]

# Load to check the stack canary
30cc:   b020adrpx0, 8000 
30d0:   f940d400ldr x0, [x0, #424]

GenFw rewrote that to:

# Load to set the stack canary
2ffc:   1480adr x0, 0x308c
3008:   912ec000add x0, x0, #0xbb0

# Load to check the stack canary
30cc:   f460adrpx0, 0x92000
30d0:   912ec000add x0, x0, #0xbb0

Note that we're now setting the stack canary from the wrong address,
resulting in an erroneous stack fault.

After this fix, the opcode is also updated, so GenFw rewrites it to:

2ffc:   9480adrpx0, 0x92000
3008:   912ec000add x0, x0, #0xbb0

And the stack canary is set correctly.

Signed-off-by: Jake Garver 
---
 BaseTools/Source/C/GenFw/Elf64Convert.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c 
b/BaseTools/Source/C/GenFw/Elf64Convert.c
index 9911db65af..4669ac3a2d 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -1565,6 +1565,7 @@ WriteSections64 (
 Offset = (Sym->st_value - (Rel->r_offset & ~0xfff)) >> 12;
 
 *(UINT32 *)Targ &= 0x901f;
+*(UINT32 *)Targ |= 0x9000;
 *(UINT32 *)Targ |= ((Offset & 0x1c) << (5 - 2)) | ((Offset & 
0x3) << 29);
 
 /* fall through */
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110120): https://edk2.groups.io/g/devel/message/110120
Mute This Topic: https://groups.io/mt/102202314/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2] BaseStackCheckLib: Fix STACK FAULT message

2023-10-18 Thread Jake Garver via groups.io

Thanks for the review, Liming Gao.

Any further comments on this change?

Thanks,
Jake

From: gaoliming 
Sent: Saturday, October 7, 2023 1:05 AM
To: Jake Garver ; devel@edk2.groups.io 
Cc: michael.d.kin...@intel.com ; 
zhiguang@intel.com 
Subject: 回复: [PATCH v2] BaseStackCheckLib: Fix STACK FAULT message

External email: Use caution opening links or attachments


Reviewed-by: Liming Gao 

> -邮件原件-
> 发件人: Jake Garver 
> 发送时间: 2023年10月6日 0:19
> 收件人: devel@edk2.groups.io
> 抄送: michael.d.kin...@intel.com; gaolim...@byosoft.com.cn;
> zhiguang@intel.com; Jake Garver 
> 主题: [PATCH v2] BaseStackCheckLib: Fix STACK FAULT message
>
> __builtin_return_address returns a pointer, not a string.  Fix the STACK
> FAULT message in BaseStackCheckLib appropriately.
>
> Signed-off-by: Jake Garver 
> ---
>  MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c
> b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c
> index 0d2918668e..ea168841b6 100644
> --- a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c
> +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c
> @@ -6,6 +6,7 @@
>   to exiting the function. If the "canary" is overwritten
__stack_chk_fail()
>   is called. This is GCC specific code.
>
> + Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights
reserved.
>   Copyright (c) 2012, Apple Inc. All rights reserved.
>   SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -34,7 +35,7 @@ __stack_chk_fail (
>  {
>UINT8  DebugPropertyMask;
>
> -  DEBUG ((DEBUG_ERROR, "STACK FAULT: Buffer Overflow in
> function %a.\n", __builtin_return_address (0)));
> +  DEBUG ((DEBUG_ERROR, "STACK FAULT: Buffer Overflow at 0x%p.\n",
> RETURN_ADDRESS (0)));
>
>//
>// Generate a Breakpoint, DeadLoop, or NOP based on PCD settings even
> if
> --
> 2.34.1





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109756): https://edk2.groups.io/g/devel/message/109756
Mute This Topic: https://groups.io/mt/102040342/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2] BaseStackCheckLib: Fix STACK FAULT message

2023-10-05 Thread Jake Garver via groups.io

__builtin_return_address returns a pointer, not a string.  Fix the STACK
FAULT message in BaseStackCheckLib appropriately.

Signed-off-by: Jake Garver 
---
 MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c 
b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c
index 0d2918668e..ea168841b6 100644
--- a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c
+++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c
@@ -6,6 +6,7 @@
  to exiting the function. If the "canary" is overwritten __stack_chk_fail()
  is called. This is GCC specific code.
 
+ Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
  Copyright (c) 2012, Apple Inc. All rights reserved.
  SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -34,7 +35,7 @@ __stack_chk_fail (
 {
   UINT8  DebugPropertyMask;
 
-  DEBUG ((DEBUG_ERROR, "STACK FAULT: Buffer Overflow in function %a.\n", 
__builtin_return_address (0)));
+  DEBUG ((DEBUG_ERROR, "STACK FAULT: Buffer Overflow at 0x%p.\n", 
RETURN_ADDRESS (0)));
 
   //
   // Generate a Breakpoint, DeadLoop, or NOP based on PCD settings even if
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109359): https://edk2.groups.io/g/devel/message/109359
Mute This Topic: https://groups.io/mt/101779895/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseStackCheckLib: Fix STACK FAULT message

2023-10-04 Thread Jake Garver via groups.io

Thanks.  I'll submit a v2 with that change.

From: Kinney, Michael D 
Sent: Tuesday, October 3, 2023 12:00 PM
To: Name j...@nvidia.com ; devel@edk2.groups.io 

Cc: Gao, Liming ; Liu, Zhiguang 
; Jake Garver ; Kinney, Michael D 

Subject: RE: [PATCH] BaseStackCheckLib: Fix STACK FAULT message

External email: Use caution opening links or attachments


I think the macro RETURN_ADDRESS from Base.h should be used instead of
direct use of the builtin.

Mike

> -Original Message-
> From: Name j...@nvidia.com 
> Sent: Tuesday, October 3, 2023 6:20 AM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D ; Gao, Liming
> ; Liu, Zhiguang ; Jake
> Garver 
> Subject: [PATCH] BaseStackCheckLib: Fix STACK FAULT message
>
> From: Jake Garver 
>
> __builtin_return_address returns a pointer, not a string.  Fix the STACK
> FAULT message in BaseStackCheckLib appropriately.
>
> Signed-off-by: Jake Garver 
> ---
>  MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c
> b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c
> index 0d2918668e..3b970391b7 100644
> --- a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c
> +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c
> @@ -34,7 +34,7 @@ __stack_chk_fail (
>  {
>UINT8  DebugPropertyMask;
>
> -  DEBUG ((DEBUG_ERROR, "STACK FAULT: Buffer Overflow in function %a.\n",
> __builtin_return_address (0)));
> +  DEBUG ((DEBUG_ERROR, "STACK FAULT: Buffer Overflow at 0x%p.\n",
> __builtin_return_address (0)));
>
>//
>// Generate a Breakpoint, DeadLoop, or NOP based on PCD settings even if
> --
> 2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109313): https://edk2.groups.io/g/devel/message/109313
Mute This Topic: https://groups.io/mt/101736789/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH] MdeModulePkg/RegularExpressinoDxe: Fix clang error

2023-10-03 Thread Jake Garver via groups.io

Ignore old style declaration warnings in oniguruma/src/st.c.  This was
already ignored for MSFT, but newer versions of clang complain as well.

Signed-off-by: Jake Garver 
---
 .../Universal/RegularExpressionDxe/RegularExpressionDxe.inf  | 1 +
 1 file changed, 1 insertion(+)

diff --git 
a/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf 
b/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf
index 84489c2942..0092531a67 100644
--- a/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf
+++ b/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf
@@ -102,6 +102,7 @@
 
   # Oniguruma: old style declaration in st.c
   MSFT:*_*_*_CC_FLAGS = /wd4131
+  GCC:*_*_*_CC_FLAGS = -Wno-deprecated-non-prototype
 
   # Oniguruma: 'type cast' : truncation from 'OnigUChar *' to 'unsigned int'
   MSFT:*_*_*_CC_FLAGS = /wd4305 /wd4306
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109284): https://edk2.groups.io/g/devel/message/109284
Mute This Topic: https://groups.io/mt/101735690/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH] CryptoPkg/Library: Replace ARM/AARCH64 sections in SmmCryptLib.inf

2023-01-25 Thread Jake Garver via groups.io

These sections were removed mistakenly.  SmmCryptLib.inf supports these
architectures.

Signed-off-by: Jake Garver 
---
 CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf 
b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
index 55df99f843..ab19930871 100644
--- a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
@@ -75,6 +75,12 @@
 [Sources.X64]
   Rand/CryptRandTsc.c
 
+[Sources.ARM]
+  Rand/CryptRand.c
+
+[Sources.AARCH64]
+  Rand/CryptRand.c
+
 [Packages]
   MdePkg/MdePkg.dec
   CryptoPkg/CryptoPkg.dec
-- 
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99041): https://edk2.groups.io/g/devel/message/99041
Mute This Topic: https://groups.io/mt/96528220/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH] BaseTools: Use BUILD_CC when checking gcc version in DevicePath

2022-12-20 Thread Jake Garver via groups.io

When checking the version in DevicePath's Makefile, use BUILD_CC instead
of assuming "gcc".  BUILD_CC is set in header.makefile and is the
compiler that will actually be used to build DevicePath.  It defaults to
"gcc", but may be overridden.

Signed-off-by: Jake Garver 
---
 BaseTools/Source/C/DevicePath/GNUmakefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Source/C/DevicePath/GNUmakefile 
b/BaseTools/Source/C/DevicePath/GNUmakefile
index 17f213879e..13b54ead65 100644
--- a/BaseTools/Source/C/DevicePath/GNUmakefile
+++ b/BaseTools/Source/C/DevicePath/GNUmakefile
@@ -13,7 +13,7 @@ OBJECTS = DevicePath.o UefiDevicePathLib.o 
DevicePathFromText.o  DevicePathUtili
 
 include $(MAKEROOT)/Makefiles/app.makefile
 
-GCCVERSION = $(shell gcc -dumpversion | awk -F'.' '{print $$1}')
+GCCVERSION = $(shell $(BUILD_CC) -dumpversion | awk -F'.' '{print $$1}')
 ifneq ("$(GCCVERSION)", "5")
 ifneq ($(CXX), llvm)
 ifneq ($(DARWIN),Darwin)
-- 
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97626): https://edk2.groups.io/g/devel/message/97626
Mute This Topic: https://groups.io/mt/95785320/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools: Generate deps for Arm targets

2022-12-19 Thread Jake Garver via groups.io

Resend with BaseTools maintainers CC'd.

Hi, folks.  Is there interest in accepting this change?  Any comments?

Thanks,
Jake

From: Jake Garver 
Sent: Monday, December 19, 2022 11:36 AM
To: devel@edk2.groups.io ; Jeff Brasen 
; Ashish Singhal 
Subject: Re: [PATCH] BaseTools: Generate deps for Arm targets

Hi, folks.  Is there interest in accepting this change?  Any comments?

Thanks,
Jake

From: Jake Garver 
Sent: Thursday, December 8, 2022 11:22 AM
To: devel@edk2.groups.io ; Jeff Brasen 
; Ashish Singhal 
Cc: Jake Garver 
Subject: [PATCH] BaseTools: Generate deps for Arm targets

Prior to this change, deps were not generated for Arm and AARCH64
libraries when MODULE_TYPE was BASE, SEC, PEI_CORE, or PIEM. That
resulted in bad incremental builds.

Signed-off-by: Jake Garver 
Reviewed-by: Jeff Brasen 
---
 BaseTools/Conf/build_rule.template | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Conf/build_rule.template 
b/BaseTools/Conf/build_rule.template
index af4819de92..ec83638144 100755
--- a/BaseTools/Conf/build_rule.template
+++ b/BaseTools/Conf/build_rule.template
@@ -145,7 +145,7 @@
 $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj

 
-"$(CC)" $(CC_FLAGS) $(CC_XIPFLAGS) -c -o ${dst} $(INC) ${src}
+"$(CC)" $(DEPS_FLAGS) $(CC_FLAGS) $(CC_XIPFLAGS) -c -o ${dst} $(INC) 
${src}

 [C-Header-File]
 
--
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97570): https://edk2.groups.io/g/devel/message/97570
Mute This Topic: https://groups.io/mt/95541348/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools: Generate deps for Arm targets

2022-12-19 Thread Jake Garver via groups.io

Hi, folks.  Is there interest in accepting this change?  Any comments?

Thanks,
Jake

From: Jake Garver 
Sent: Thursday, December 8, 2022 11:22 AM
To: devel@edk2.groups.io ; Jeff Brasen 
; Ashish Singhal 
Cc: Jake Garver 
Subject: [PATCH] BaseTools: Generate deps for Arm targets

Prior to this change, deps were not generated for Arm and AARCH64
libraries when MODULE_TYPE was BASE, SEC, PEI_CORE, or PIEM. That
resulted in bad incremental builds.

Signed-off-by: Jake Garver 
Reviewed-by: Jeff Brasen 
---
 BaseTools/Conf/build_rule.template | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Conf/build_rule.template 
b/BaseTools/Conf/build_rule.template
index af4819de92..ec83638144 100755
--- a/BaseTools/Conf/build_rule.template
+++ b/BaseTools/Conf/build_rule.template
@@ -145,7 +145,7 @@
 $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj

 
-"$(CC)" $(CC_FLAGS) $(CC_XIPFLAGS) -c -o ${dst} $(INC) ${src}
+"$(CC)" $(DEPS_FLAGS) $(CC_FLAGS) $(CC_XIPFLAGS) -c -o ${dst} $(INC) 
${src}

 [C-Header-File]
 
--
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97568): https://edk2.groups.io/g/devel/message/97568
Mute This Topic: https://groups.io/mt/95541348/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] Fix cyclic dependency error on OptionROM build

2022-12-13 Thread Jake Garver via groups.io

Thanks, Konstantin,

I approve this version.

Thanks,
Jake

From: Konstantin Aladyshev 
Sent: Tuesday, December 13, 2022 11:22 AM
To: devel@edk2.groups.io 
Cc: bob.c.f...@intel.com ; gaolim...@byosoft.com.cn 
; yuwei.c...@intel.com ; Jake 
Garver ; Konstantin Aladyshev 
Subject: [PATCH] Fix cyclic dependency error on OptionROM build

External email: Use caution opening links or attachments


EDKII build system supports OptionROM generation if particular PCI_*
defines are present in the module INF file:
```
[Defines]
  ...
  PCI_VENDOR_ID  = <...>
  PCI_DEVICE_ID  = <...>
  PCI_CLASS_CODE = <...>
  PCI_REVISION   = <...>
```
Although after the commit d372ab585a2cdc5348af5f701c56c631235fe698
("BaseTools/Conf: Fix Dynamic-Library-File template") it is no longer
possible.
The build system fails with the error:
```
Cyclic dependency detected while generating rule for
"<...>/DEBUG/<...>.efi" file
```
Remove "$(DEBUG_DIR)(+)$(MODULE_NAME).efi" from the 'dll' output files
to fix the cyclic dependency.
---
 BaseTools/Conf/build_rule.template | 1 -
 1 file changed, 1 deletion(-)

diff --git a/BaseTools/Conf/build_rule.template 
b/BaseTools/Conf/build_rule.template
index af4819de92..21ccd864fa 100755
--- a/BaseTools/Conf/build_rule.template
+++ b/BaseTools/Conf/build_rule.template
@@ -342,7 +342,6 @@


 

 $(OUTPUT_DIR)(+)$(MODULE_NAME).efi

-$(DEBUG_DIR)(+)$(MODULE_NAME).efi

 $(OUTPUT_DIR)(+)$(MODULE_NAME).map



 

--
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97319): https://edk2.groups.io/g/devel/message/97319
Mute This Topic: https://groups.io/mt/95647902/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] Revert "BaseTools/Conf: Fix Dynamic-Library-File template"

2022-12-13 Thread Jake Garver via groups.io

Hi, Konstantin,

Thanks for working with me offline to reproduce this.

Short answer:  We can live without the "$(DEBUG_DIR)(+)$(MODULE_NAME).efi" 
output and I think removing that will resolve your issue.  Rather than revert 
the original change, can you submit a new patch that removes just that line, 
after verifying?  Thanks.

Long answer:  The problem is in how AutoGen/ModuleAutoGen.py builds up 
RuleChain in _ApplyBuildRule().  It cannot handle two outputs of the same type. 
 In this case, we have "$(OUTPUT_DIR)(+)$(MODULE_NAME).efi" and 
"$(DEBUG_DIR)(+)$(MODULE_NAME).efi".  In the _ApplyBuildRule(), each gets a 
FileType="EFI-IMAGE".  That looks like a cyclic dep, but really, it's just a 
fork in the dep tree.  I think it would be better if _ApplyBuildRule() would 
consider the output names instead of the output types, but that would be a 
significant change.  And one that I think is unnecessary at this stage.

In this case, I think we can live without identifying 
"$(DEBUG_DIR)(+)$(MODULE_NAME).efi" as an output of the build rule.  Then we'll 
dodge the _ApplyBuildRule() issue.  My original issue was with an unidentified 
dependency on the "map" output, but I also added the debug efi for good measure 
because it is also an output of that rule template.  However, it appears to be 
an output that nothing depends on, so it's unnecessary.

Thanks,
Jake

From: Konstantin Aladyshev 
Sent: Monday, December 12, 2022 11:07 AM
To: Jake Garver 
Cc: devel@edk2.groups.io ; bob.c.f...@intel.com 
; gaolim...@byosoft.com.cn ; 
yuwei.c...@intel.com 
Subject: Re: [PATCH] Revert "BaseTools/Conf: Fix Dynamic-Library-File template"

External email: Use caution opening links or attachments

Hi, Jake!

No, unfortunately I don't have any way to fix this, besides the patch revert.

To reproduce the issue you can add the mentioned PCI_* defines to some
simple DXE_DRIVER.

For example:

SimpleDriver/SimpleDriver.inf
```
[Defines]
  INF_VERSION= 1.25
  BASE_NAME  = SimpleDriver
  FILE_GUID  = 384aeb18-105d-4af1-bf17-5e349e8f4d4c
  MODULE_TYPE= UEFI_DRIVER
  VERSION_STRING = 1.0
  ENTRY_POINT= SimpleDriverEntryPoint
  UNLOAD_IMAGE   = SimpleDriverUnload
  PCI_VENDOR_ID  = 0x1234
  PCI_DEVICE_ID  = 0x5678
  PCI_CLASS_CODE = 0x0001
  PCI_REVISION   = 0x0002

[Sources]
  SimpleDriver.c

[Packages]
  MdePkg/MdePkg.dec

[LibraryClasses]
  UefiDriverEntryPoint
  UefiLib
```

SimpleDriver/SimpleDriver.c
```
#include 
#include 

EFI_STATUS
EFIAPI
SimpleDriverUnload (
  EFI_HANDLE ImageHandle
  )
{
  return EFI_SUCCESS;
}

EFI_STATUS
EFIAPI
SimpleDriverEntryPoint (
  IN EFI_HANDLEImageHandle,
  IN EFI_SYSTEM_TABLE  *SystemTable
  )
{
  return EFI_SUCCESS;
}
```
It is enough to reproduce the issue.
Normally the build system should compile the driver code fine and then
call the EfiRom utility to produce the OptionROM image. But with the
d372ab585a2cdc5348af5f701c56c631235fe698 in place the driver build
fails.

Best regards,
Konstantin Aladyshev

On Mon, Dec 12, 2022 at 6:35 PM Jake Garver  wrote:
>
> Hi, Konstantin,
>
> Do you have a fix for the cyclic redundancy issue when building OptionROMs?  
> If not, can you help me reproduce it?
>
> I'd hate to revert d372ab as it fixed dependency issues we frequently ran 
> into during parallel builds.
>
> Thanks,
> Jake
> 
> From: Konstantin Aladyshev 
> Sent: Monday, December 12, 2022 8:09 AM
> To: devel@edk2.groups.io 
> Cc: bob.c.f...@intel.com ; gaolim...@byosoft.com.cn 
> ; yuwei.c...@intel.com ; Jake 
> Garver ; Konstantin Aladyshev 
> Subject: [PATCH] Revert "BaseTools/Conf: Fix Dynamic-Library-File template"
>
> External email: Use caution opening links or attachments
>
>
> Revert commit d372ab585a2cdc5348af5f701c56c631235fe698.
>
> EdkII build system supports OptionROM generation if particular PCI_*
> defines are present in the module INF file:
> ```
> [Defines]
>   ...
>   PCI_VENDOR_ID  = <...>
>   PCI_DEVICE_ID  = <...>
>   PCI_CLASS_CODE = <...>
>   PCI_REVISION   = <...>
> ```
> Although after the commit d372ab585a2cdc5348af5f701c56c631235fe698
> it is no longer possible.
> The build system fails with the error:
> ```
> Cyclic dependency detected while generating rule for
> "<...>/DEBUG/<...>.efi" file
> ```
> Revert d372ab585a2cdc5348af5f701c56c631235fe698 until the issue
> is resolved.
> ---
>  BaseTools/Conf/build_rule.template | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/BaseTools/Conf/build_rule.template 
> b/BaseTools/Conf/build_rule.template
> index af4819de92..32053f6353 100755
> --- a/BaseTools/Conf/build_rule.template
> +++ b/BaseTools/Conf/build_rule.template
> @@ -342,8 +342,6 @@
>
>
>  
>
>  $(OUTPUT_DIR)(+)$(MODULE

Re: [edk2-devel] [PATCH] Revert "BaseTools/Conf: Fix Dynamic-Library-File template"

2022-12-12 Thread Jake Garver via groups.io

Hi, Konstantin,

Do you have a fix for the cyclic redundancy issue when building OptionROMs?  If 
not, can you help me reproduce it?

I'd hate to revert d372ab as it fixed dependency issues we frequently ran into 
during parallel builds.

Thanks,
Jake

From: Konstantin Aladyshev 
Sent: Monday, December 12, 2022 8:09 AM
To: devel@edk2.groups.io 
Cc: bob.c.f...@intel.com ; gaolim...@byosoft.com.cn 
; yuwei.c...@intel.com ; Jake 
Garver ; Konstantin Aladyshev 
Subject: [PATCH] Revert "BaseTools/Conf: Fix Dynamic-Library-File template"

External email: Use caution opening links or attachments


Revert commit d372ab585a2cdc5348af5f701c56c631235fe698.

EdkII build system supports OptionROM generation if particular PCI_*
defines are present in the module INF file:
```
[Defines]
  ...
  PCI_VENDOR_ID  = <...>
  PCI_DEVICE_ID  = <...>
  PCI_CLASS_CODE = <...>
  PCI_REVISION   = <...>
```
Although after the commit d372ab585a2cdc5348af5f701c56c631235fe698
it is no longer possible.
The build system fails with the error:
```
Cyclic dependency detected while generating rule for
"<...>/DEBUG/<...>.efi" file
```
Revert d372ab585a2cdc5348af5f701c56c631235fe698 until the issue
is resolved.
---
 BaseTools/Conf/build_rule.template | 2 --
 1 file changed, 2 deletions(-)

diff --git a/BaseTools/Conf/build_rule.template 
b/BaseTools/Conf/build_rule.template
index af4819de92..32053f6353 100755
--- a/BaseTools/Conf/build_rule.template
+++ b/BaseTools/Conf/build_rule.template
@@ -342,8 +342,6 @@


 

 $(OUTPUT_DIR)(+)$(MODULE_NAME).efi

-$(DEBUG_DIR)(+)$(MODULE_NAME).efi

-$(OUTPUT_DIR)(+)$(MODULE_NAME).map



 

 "$(GENFW)" -e $(MODULE_TYPE) -o ${dst} ${src} $(GENFW_FLAGS)

--
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97274): https://edk2.groups.io/g/devel/message/97274
Mute This Topic: https://groups.io/mt/95620341/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH] BaseTools: Generate deps for Arm targets

2022-12-08 Thread Jake Garver via groups.io

Prior to this change, deps were not generated for Arm and AARCH64
libraries when MODULE_TYPE was BASE, SEC, PEI_CORE, or PIEM. That
resulted in bad incremental builds.

Signed-off-by: Jake Garver 
Reviewed-by: Jeff Brasen 
---
 BaseTools/Conf/build_rule.template | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Conf/build_rule.template 
b/BaseTools/Conf/build_rule.template
index af4819de92..ec83638144 100755
--- a/BaseTools/Conf/build_rule.template
+++ b/BaseTools/Conf/build_rule.template
@@ -145,7 +145,7 @@
 $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj
 
 
-"$(CC)" $(CC_FLAGS) $(CC_XIPFLAGS) -c -o ${dst} $(INC) ${src}
+"$(CC)" $(DEPS_FLAGS) $(CC_FLAGS) $(CC_XIPFLAGS) -c -o ${dst} $(INC) 
${src}
 
 [C-Header-File]
 
-- 
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97140): https://edk2.groups.io/g/devel/message/97140
Mute This Topic: https://groups.io/mt/95541348/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools: Fix dependency issue in PcdValueInit

2022-04-25 Thread Jake Garver via groups.io

Hello maintainers,

Any interest in accepting this and the "BaseTools/Conf: Fix 
Dynamic-Library-File template" patch?

Thanks,
Jake


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#89268): https://edk2.groups.io/g/devel/message/89268
Mute This Topic: https://groups.io/mt/90317701/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH] BaseTools/Conf: Fix Dynamic-Library-File template

2022-04-07 Thread Jake Garver via groups.io

In the Dynamic-Library-File template, add missing output file
declarations.  These files are generated by the template and other rules
explicitly depend on them.

This change resolves missing dependency issues we encountered while
running a recursive make with job control.

Signed-off-by: Jake Garver 
---
 BaseTools/Conf/build_rule.template | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/BaseTools/Conf/build_rule.template 
b/BaseTools/Conf/build_rule.template
index f401182344..4356623512 100755
--- a/BaseTools/Conf/build_rule.template
+++ b/BaseTools/Conf/build_rule.template
@@ -366,6 +366,8 @@
 
 
 $(OUTPUT_DIR)(+)$(MODULE_NAME).efi
+$(DEBUG_DIR)(+)$(MODULE_NAME).efi
+$(OUTPUT_DIR)(+)$(MODULE_NAME).map
 
 
 "$(GENFW)" -e $(MODULE_TYPE) -o ${dst} ${src} $(GENFW_FLAGS)
-- 
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#88549): https://edk2.groups.io/g/devel/message/88549
Mute This Topic: https://groups.io/mt/90317800/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH] BaseTools: Fix dependency issue in PcdValueInit

2022-04-07 Thread Jake Garver via groups.io

The generated Makefile was missing a dependency.  This resulted in a
build-time race condition if the recursive make is multi-threaded and
shares job control.

Signed-off-by: Jake Garver 
---
 BaseTools/Source/Python/Workspace/DscBuildData.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py 
b/BaseTools/Source/Python/Workspace/DscBuildData.py
index fc1e773417..d55ea1bbe2 100644
--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
@@ -97,7 +97,8 @@ PcdMakefileEnd = '''
 
 AppTarget = '''
 all: $(APPFILE)
-$(APPFILE): $(OBJECTS)
+$(APPLICATION): $(OBJECTS)
+$(APPFILE): $(APPLICATION)
 %s
 '''
 
-- 
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#88548): https://edk2.groups.io/g/devel/message/88548
Mute This Topic: https://groups.io/mt/90317701/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools: Fix error leg in DscBuildData.py

2022-02-14 Thread Jake Garver via groups.io

Hi folks,

Any interest in accepting this simple fix?

Thanks,
Jake


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#86654): https://edk2.groups.io/g/devel/message/86654
Mute This Topic: https://groups.io/mt/88573504/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH] Features/Ext4Pkg: Modularize Ext4 DSC/FDF files

2022-01-25 Thread Jake Garver via groups.io

This change modularizes the Ext4 DSC/FDF files following the model used
in edk2/NetworkPkg.

A platform DSC can include Ext4 using "!include Features/Ext4.dsc.inc".
Ext.dsc.inc includes all the required information to enable Ext4
features.  Similarly, "!include Features/Ext4.fdf.inc" would be used in
the platform FDF.

The Ext4 feature is enabled by default, but could be disabled at build
time using:

BLD_*_EXT4_ENABLE=FALSE

Signed-off-by: Jake Garver 
---
 Features/Ext4Pkg/Ext4.dsc.inc   | 16 
 Features/Ext4Pkg/Ext4.fdf.inc   | 11 +++
 Features/Ext4Pkg/Ext4Components.dsc.inc | 14 ++
 Features/Ext4Pkg/Ext4Defines.dsc.inc| 14 ++
 Features/Ext4Pkg/Ext4Libs.dsc.inc   | 11 +++
 5 files changed, 66 insertions(+)
 create mode 100644 Features/Ext4Pkg/Ext4.dsc.inc
 create mode 100644 Features/Ext4Pkg/Ext4.fdf.inc
 create mode 100644 Features/Ext4Pkg/Ext4Components.dsc.inc
 create mode 100644 Features/Ext4Pkg/Ext4Defines.dsc.inc
 create mode 100644 Features/Ext4Pkg/Ext4Libs.dsc.inc

diff --git a/Features/Ext4Pkg/Ext4.dsc.inc b/Features/Ext4Pkg/Ext4.dsc.inc
new file mode 100644
index 00..b560b9acbb
--- /dev/null
+++ b/Features/Ext4Pkg/Ext4.dsc.inc
@@ -0,0 +1,16 @@
+## @file
+# Ext4 DSC include file for Platform DSC
+#
+# SPDX-FileCopyrightText: Copyright (c) 2022 NVIDIA CORPORATION & AFFILIATES. 
All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+!include Features/Ext4Pkg/Ext4Defines.dsc.inc
+
+[LibraryClasses]
+!include Features/Ext4Pkg/Ext4Libs.dsc.inc
+
+[Components.common]
+!include Features/Ext4Pkg/Ext4Components.dsc.inc
diff --git a/Features/Ext4Pkg/Ext4.fdf.inc b/Features/Ext4Pkg/Ext4.fdf.inc
new file mode 100644
index 00..fb36070860
--- /dev/null
+++ b/Features/Ext4Pkg/Ext4.fdf.inc
@@ -0,0 +1,11 @@
+## @file
+# Ext4 FDF include file for All Architectures.
+#
+# SPDX-FileCopyrightText: Copyright (c) 2022 NVIDIA CORPORATION & AFFILIATES. 
All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+!if $(EXT4_ENABLE) == TRUE
+  INF Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.inf
+!endif
diff --git a/Features/Ext4Pkg/Ext4Components.dsc.inc 
b/Features/Ext4Pkg/Ext4Components.dsc.inc
new file mode 100644
index 00..34401272a0
--- /dev/null
+++ b/Features/Ext4Pkg/Ext4Components.dsc.inc
@@ -0,0 +1,14 @@
+## @file
+# Ext4 DSC include file for [Components] section of all Architectures.
+#
+# SPDX-FileCopyrightText: Copyright (c) 2022 NVIDIA CORPORATION & AFFILIATES. 
All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+!if $(EXT4_ENABLE) == TRUE
+  Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.inf {
+
+  gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8007
+  }
+!endif
diff --git a/Features/Ext4Pkg/Ext4Defines.dsc.inc 
b/Features/Ext4Pkg/Ext4Defines.dsc.inc
new file mode 100644
index 00..b02eac59cf
--- /dev/null
+++ b/Features/Ext4Pkg/Ext4Defines.dsc.inc
@@ -0,0 +1,14 @@
+## @file
+# Ext4 DSC include file for [Defines] section of all Architectures.
+#
+# SPDX-FileCopyrightText: Copyright (c) 2022 NVIDIA CORPORATION & AFFILIATES. 
All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+!ifndef EXT4_ENABLE
+  #
+  # This flag is to enable or disable the ext4 feature.
+  #
+  DEFINE EXT4_ENABLE = TRUE
+!endif
diff --git a/Features/Ext4Pkg/Ext4Libs.dsc.inc 
b/Features/Ext4Pkg/Ext4Libs.dsc.inc
new file mode 100644
index 00..078183e0cc
--- /dev/null
+++ b/Features/Ext4Pkg/Ext4Libs.dsc.inc
@@ -0,0 +1,11 @@
+## @file
+# Ext4 DSC include file for [LibraryClasses] section of all Architectures.
+#
+# SPDX-FileCopyrightText: Copyright (c) 2022 NVIDIA CORPORATION & AFFILIATES. 
All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+!if $(EXT4_ENABLE) == TRUE
+  BaseUcs2Utf8Lib|RedfishPkg/Library/BaseUcs2Utf8Lib/BaseUcs2Utf8Lib.inf
+!endif
-- 
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#86050): https://edk2.groups.io/g/devel/message/86050
Mute This Topic: https://groups.io/mt/88671995/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH] BaseTools: Fix error leg in DscBuildData.py

2022-01-20 Thread Jake Garver via groups.io

Fix a Edk2Logger.warn() message format to match the arguments.

We ran into this after a failure in PcdValueInit.  The failure was
masked by a new exception, "TypeError: not all arguments converted
during string formatting".

Signed-off-by: Jake Garver 
---
 BaseTools/Source/Python/Workspace/DscBuildData.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py 
b/BaseTools/Source/Python/Workspace/DscBuildData.py
index 35ec5b37ff..d29b9bf13d 100644
--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
@@ -3033,7 +3033,7 @@ class DscBuildData(PlatformBuildClassObject):
 returncode, StdOut, StdErr = DscBuildData.ExecuteCommand (Command)
 EdkLogger.verbose ('%s\n%s\n%s' % (Command, StdOut, StdErr))
 if returncode != 0:
-EdkLogger.warn('Build', COMMAND_FAILURE, 'Can not collect 
output from command: %s\n%s\n' % (Command, StdOut, StdErr))
+EdkLogger.warn('Build', COMMAND_FAILURE, 'Can not collect 
output from command: %s\n%s\n%s\n' % (Command, StdOut, StdErr))
 
 #start update structure pcd final value
 File = open (OutputValueFile, 'r')
-- 
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#85882): https://edk2.groups.io/g/devel/message/85882
Mute This Topic: https://groups.io/mt/88573504/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2] BaseTools/GenFw: Correct offset when relocating an ADR

Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP

Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP

Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP

Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP

Re: [edk2-devel] [PATCH v2] BaseStackCheckLib: Fix STACK FAULT message

Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP

Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP

Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP

Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP

[edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP

Re: [edk2-devel] [PATCH v2] BaseStackCheckLib: Fix STACK FAULT message

[edk2-devel] [PATCH v2] BaseStackCheckLib: Fix STACK FAULT message

Re: [edk2-devel] [PATCH] BaseStackCheckLib: Fix STACK FAULT message

[edk2-devel] [PATCH] MdeModulePkg/RegularExpressinoDxe: Fix clang error

[edk2-devel] [PATCH] CryptoPkg/Library: Replace ARM/AARCH64 sections in SmmCryptLib.inf

[edk2-devel] [PATCH] BaseTools: Use BUILD_CC when checking gcc version in DevicePath

Re: [edk2-devel] [PATCH] BaseTools: Generate deps for Arm targets

Re: [edk2-devel] [PATCH] BaseTools: Generate deps for Arm targets

Re: [edk2-devel] [PATCH] Fix cyclic dependency error on OptionROM build

Re: [edk2-devel] [PATCH] Revert "BaseTools/Conf: Fix Dynamic-Library-File template"

Re: [edk2-devel] [PATCH] Revert "BaseTools/Conf: Fix Dynamic-Library-File template"

[edk2-devel] [PATCH] BaseTools: Generate deps for Arm targets

Re: [edk2-devel] [PATCH] BaseTools: Fix dependency issue in PcdValueInit

[edk2-devel] [PATCH] BaseTools/Conf: Fix Dynamic-Library-File template

[edk2-devel] [PATCH] BaseTools: Fix dependency issue in PcdValueInit

Re: [edk2-devel] [PATCH] BaseTools: Fix error leg in DscBuildData.py

[edk2-devel] [PATCH] Features/Ext4Pkg: Modularize Ext4 DSC/FDF files

[edk2-devel] [PATCH] BaseTools: Fix error leg in DscBuildData.py

29 matches

Site Navigation

Mail list logo

Footer information