Date   

[PATCH edk2-platforms v2 3/9] Silicon/NXP: Use edk2 recommended compilation flags

Pankaj Bansal
 

From: Pankaj Bansal <pankaj.bansal@nxp.com>

edk2 recommends to use MDEPKG_NDEBUG for release builds and to use
DISABLE_NEW_DEPRECATED_INTERFACES for all new platforms.

Therefore, enable these flags for NXP platforms as well

Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
---

Notes:
V2:
- No change

Silicon/NXP/NxpQoriqLs.dsc.inc | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Silicon/NXP/NxpQoriqLs.dsc.inc b/Silicon/NXP/NxpQoriqLs.dsc.inc
index 12e2b89fac58..ee639d552483 100644
--- a/Silicon/NXP/NxpQoriqLs.dsc.inc
+++ b/Silicon/NXP/NxpQoriqLs.dsc.inc
@@ -173,7 +173,12 @@
NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf

[BuildOptions]
- RVCT:*_*_ARM_PLATFORM_FLAGS == --cpu cortex-a9
+ GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG
+
+ #
+ # Disable deprecated APIs.
+ #
+ GCC:*_*_*_CC_FLAGS = -DDISABLE_NEW_DEPRECATED_INTERFACES

[BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
GCC:*_*_ARM_DLINK_FLAGS = -z common-page-size=0x1000
--
2.17.1


[PATCH edk2-platforms v2 2/9] Platform/NXP: Use Monotonic counter from MdeModulePkg

Pankaj Bansal
 

From: Pankaj Bansal <pankaj.bansal@nxp.com>

Monotonic counter module from EmbeddedPkg doesn't treat the
high 32 bit as non volatile, which is needed as per spec.

Therefore, use Monotonic counter module from MdeModulePkg

Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
---

Notes:
V2:
- No change

Silicon/NXP/NxpQoriqLs.dsc.inc | 2 +-
Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Silicon/NXP/NxpQoriqLs.dsc.inc b/Silicon/NXP/NxpQoriqLs.dsc.inc
index 54236e19531c..12e2b89fac58 100644
--- a/Silicon/NXP/NxpQoriqLs.dsc.inc
+++ b/Silicon/NXP/NxpQoriqLs.dsc.inc
@@ -337,7 +337,7 @@
MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf
- EmbeddedPkg/EmbeddedMonotonicCounter/EmbeddedMonotonicCounter.inf
+ MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf

# FDT installation
MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf
diff --git a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
index fede51ced10e..49d8885477c7 100644
--- a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
+++ b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
@@ -98,7 +98,7 @@ READ_LOCK_STATUS = TRUE
INF MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
INF MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
INF MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
- INF EmbeddedPkg/EmbeddedMonotonicCounter/EmbeddedMonotonicCounter.inf
+ INF MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
INF MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf

--
2.17.1


[PATCH edk2-platforms v2 1/9] Silicon/NXP: Use Metronome implementation from MdeModulePkg

Pankaj Bansal
 

From: Pankaj Bansal <pankaj.bansal@nxp.com>

There are two implementations of Metronome protocol.

EmbeddedPkg/MetronomeDxe/MetronomeDxe.inf
MdeModulePkg/Universal/Metronome/Metronome.inf

Although nowhere it has been specified, which one to use, but we are
going by the general practice of preferring MdeModulePkg/MdePkg over
EmbeddedPkg.

Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
---

Notes:
V2:
- No change

Silicon/NXP/NxpQoriqLs.dsc.inc | 4 +---
Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf | 2 +-
2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/Silicon/NXP/NxpQoriqLs.dsc.inc b/Silicon/NXP/NxpQoriqLs.dsc.inc
index 03759c7cee7c..54236e19531c 100644
--- a/Silicon/NXP/NxpQoriqLs.dsc.inc
+++ b/Silicon/NXP/NxpQoriqLs.dsc.inc
@@ -218,8 +218,6 @@
gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeout"|gEfiGlobalVariableGuid|0x0|10

[PcdsFixedAtBuild.common]
- gEmbeddedTokenSpaceGuid.PcdMetronomeTickPeriod|1000
- gEmbeddedTokenSpaceGuid.PcdTimerPeriod|10000 # expressed in 100ns units, 100,000 x 100 ns = 10,000,000 ns = 10 ms
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
@@ -348,7 +346,7 @@
MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
- EmbeddedPkg/MetronomeDxe/MetronomeDxe.inf
+ MdeModulePkg/Universal/Metronome/Metronome.inf
ArmPkg/Drivers/TimerDxe/TimerDxe.inf
ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
EmbeddedPkg/SimpleTextInOutSerial/SimpleTextInOutSerial.inf
diff --git a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
index 931d0bb14f9b..fede51ced10e 100644
--- a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
+++ b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
@@ -115,7 +115,7 @@ READ_LOCK_STATUS = TRUE
INF MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
INF MdeModulePkg/Universal/SerialDxe/SerialDxe.inf

- INF EmbeddedPkg/MetronomeDxe/MetronomeDxe.inf
+ INF MdeModulePkg/Universal/Metronome/Metronome.inf
INF EmbeddedPkg/SimpleTextInOutSerial/SimpleTextInOutSerial.inf

#
--
2.17.1


[PATCH edk2-platforms v2 0/9] Add LX2160ARDB Platform

Pankaj Bansal
 

From: Pankaj Bansal <pankaj.bansal@nxp.com>

LX2160A Reference Design Board (RDB) is a high-performance development
platform that supports the QorIQ LX2160A Layerscape Architecture SOCs.

This Platform is based on Layerscape Chassis3V2.

The code structure is same as Chassis2 and LS1043A SOC and LS1043ARDB
platform.

v1 of this series can be referred here:
https://edk2.groups.io/g/devel/message/59923

Pankaj Bansal (9):
Silicon/NXP: Use Metronome implementation from MdeModulePkg
Platform/NXP: Use Monotonic counter from MdeModulePkg
Silicon/NXP: Use edk2 recommended compilation flags
Platform/NXP/LX2160ARDB: Add ArmPlatformLib
Silicon/NXP: Implement PL011UartClockLib for NXP platforms
Silicon/NXP: Add Chassis3V2 Package
Silicon/NXP: Add LX2160A Soc package
Platform/NXP: Add LX2160ARDB Platform
Platform/NXP/LX2160aRdbPkg: Add VarStore

Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dec | 23 +++
Silicon/NXP/Chassis3V2/Chassis3V2.dec | 22 +++
Silicon/NXP/LX2160A/LX2160A.dec | 13 ++
Silicon/NXP/Chassis3V2/Chassis3V2.dsc.inc | 10 ++
Silicon/NXP/LX2160A/LX2160A.dsc.inc | 50 ++++++
Silicon/NXP/NxpQoriqLs.dsc.inc | 13 +-
Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc | 46 ++++++
Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf | 4 +-
.../LX2160aRdbPkg.fdf} | 33 ++--
.../Library/ArmPlatformLib/ArmPlatformLib.inf | 42 +++++
.../Library/ChassisLib/ChassisLib.inf | 33 ++++
Silicon/NXP/LX2160A/Library/SocLib/SocLib.inf | 27 ++++
.../PL011UartClockLib/PL011UartClockLib.inf | 24 +++
Silicon/NXP/Chassis3V2/Include/Chassis.h | 26 ++++
Silicon/NXP/LX2160A/Include/Soc.h | 38 +++++
.../Library/ArmPlatformLib/ArmPlatformLib.c | 147 ++++++++++++++++++
.../ArmPlatformLib/ArmPlatformLibMem.c | 80 ++++++++++
.../Library/ChassisLib/ChassisLib.c | 71 +++++++++
Silicon/NXP/LX2160A/Library/SocLib/SocLib.c | 80 ++++++++++
.../PL011UartClockLib/PL011UartClockLib.c | 22 +++
.../AArch64/ArmPlatformHelper.S | 45 ++++++
Platform/NXP/LX2160aRdbPkg/VarStore.fdf.inc | 91 +++++++++++
22 files changed, 913 insertions(+), 27 deletions(-)
create mode 100644 Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dec
create mode 100644 Silicon/NXP/Chassis3V2/Chassis3V2.dec
create mode 100644 Silicon/NXP/LX2160A/LX2160A.dec
create mode 100644 Silicon/NXP/Chassis3V2/Chassis3V2.dsc.inc
create mode 100644 Silicon/NXP/LX2160A/LX2160A.dsc.inc
create mode 100644 Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc
copy Platform/NXP/{LS1043aRdbPkg/LS1043aRdbPkg.fdf => LX2160aRdbPkg/LX2160aRdbPkg.fdf} (88%)
create mode 100644 Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
create mode 100644 Silicon/NXP/Chassis3V2/Library/ChassisLib/ChassisLib.inf
create mode 100644 Silicon/NXP/LX2160A/Library/SocLib/SocLib.inf
create mode 100644 Silicon/NXP/Library/PL011UartClockLib/PL011UartClockLib.inf
create mode 100644 Silicon/NXP/Chassis3V2/Include/Chassis.h
create mode 100644 Silicon/NXP/LX2160A/Include/Soc.h
create mode 100644 Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c
create mode 100644 Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c
create mode 100644 Silicon/NXP/Chassis3V2/Library/ChassisLib/ChassisLib.c
create mode 100644 Silicon/NXP/LX2160A/Library/SocLib/SocLib.c
create mode 100644 Silicon/NXP/Library/PL011UartClockLib/PL011UartClockLib.c
create mode 100644 Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/AArch64/ArmPlatformHelper.S
create mode 100644 Platform/NXP/LX2160aRdbPkg/VarStore.fdf.inc

--
2.17.1


Re: [PATCH edk2-platforms 1/1] Silicon/SynQuacer: use correct argument count for _EVT ACPI method

Ard Biesheuvel
 

On 5/26/20 10:49 PM, Leif Lindholm wrote:
On Mon, May 25, 2020 at 20:24:55 +0200, Ard Biesheuvel wrote:
The _EVT method on the ACPI0013 Generic Event device takes a single
argument. Even though we are not interested in its value (given that
there is only one interrupt source), we still have to declare the
prototype correctly, or the OS might complain and refuse to call it.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
Thanks

Pushed as 7121691cfcbc..85a50de1b459

---
Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl b/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl
index 0ea8ce6d5f44..50f1753c3565 100644
--- a/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl
+++ b/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl
@@ -226,7 +226,7 @@ DefinitionBlock ("DsdtTable.aml", "DSDT", 1, "SNI", "SYNQUACR",
MASK = 0xfffffeff
}
- Method (_EVT) {
+ Method (_EVT, 0x1) {
REQC = 0x100
Notify (\_SB.PWRB, 0x80)
}
--
2.17.1


Re: [PATCH 3/3] OvmfwPkg: Don't exclude XCODE Modules

Andrew Fish
 

On May 26, 2020, at 4:45 AM, Laszlo Ersek <lersek@redhat.com> wrote:

On 05/26/20 06:10, Andrew Fish wrote:


On May 25, 2020, at 12:31 PM, Laszlo Ersek <lersek@redhat.com> wrote:

Hi Andrew,

On 05/24/20 23:20, Andrew Fish via groups.io <http://groups.io/> wrote:
With this BZ getting fixed we no longer need to special case XCODE.

Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=557
Signed-off-by: Andrew Fish <afish@apple.com>

Signed-off-by: Andrew Fish <afish@apple.com>
---
OvmfPkg/OvmfPkgIa32.dsc | 3 +--
OvmfPkg/OvmfPkgIa32.fdf | 2 --
OvmfPkg/OvmfPkgIa32X64.dsc | 4 ++--
OvmfPkg/OvmfPkgIa32X64.fdf | 2 --
OvmfPkg/OvmfPkgX64.dsc | 3 +--
OvmfPkg/OvmfPkgX64.fdf | 2 --
OvmfPkg/OvmfXen.dsc | 3 +--
OvmfPkg/OvmfXen.fdf | 2 --
8 files changed, 5 insertions(+), 16 deletions(-)
Laszlo,

Thanks for the feedback.

Can I ask that you go to https://www.tianocore.org, click on How to Contribute and point me at the chain of links I did not follow, if I missed it tit is likely due to too many links and too much information being vended.
You didn't miss anything, starting from that page, I think. I don't
remember ever clicking "How to Contribute" on that page. :)

In fact if I grep the current edk2-wiki project source, at commit
de8fae02bbcc ("Add acknowledgements page", 2020-05-21), I find no
references to "SetupGit.py".

When people are starting out we should vend them the instructions that work and let them opt in to learning more.
"Working instructions" is a moving target. When I wrote the unkempt
guide, it was serious work, I had to set aside resources. When we
changed the workflow to replace "git-push" (by maintainers) with github
PRs (to trigger CI), documenting that was again serious work (for Mike
-- Mike updated both the official workflow article and the unkempt
guide, as I couldn't volunteer for the latter).

The more documentation we add, the larger the burden to update them
grows. It's an on-going commitment. Given the constant scarcity of
developer (and reviewer) cycles, we can only choose between:

- "code, plus more or less up-to-date docs", and
- "code, plus no docs".

Leif wrote SetupGit.py in the first place to save people the effort of
going through some of the unkempt guide steps.

If we decide that no SetupGit.py (or similar utilities) should be
written without documenting them in the wiki, that won't force
contributors to document their workflow-related contributions; it will
cause them to not writing the utilities in the first place.

Open source development communities teach contributors the workflow by
doing. For example, I have contributed to two open source projects that
are *exclusively* managed on github.com, using "github native" pull
requests and such. (Namely, openssl, and "p11-glue/p11-kit".) I'm the
kind of guy that very carefully reads the documentation first, and
starts pushing the buttons only second, and I *still* got wrong my
initial contributions to both projects.

With OpenSSL, I missed details of how review worked and details about
the CLA. Maintainers taught me those bits on the PR, while they were
reviewing my code.

With p11-kit, I had missed that I was expected to write a unit test at
once, for the new code. Maintainers pointed that out in my PR.

I posit that virtually no up-to-date technical documentation exists
unless an organization treats that documentation as a *product* (with
its own resource allocations, technical writers, subject matter expert
reviewers, project managers, and so on).


(1) Please run "BaseTools/Scripts/SetupGit.py" in your edk2 clone,
because right now, the patch is formatted/posted with too many CR
characters.
I filed https://bugzilla.tianocore.org/show_bug.cgi?id=2767 since I passed PatchCheck.py but did not run BaseTools/Scripts/SetupGit.py
Thanks.

For the record: I didn't reject your contribution (I'm very happy you
posted a patch series); instead, I asked for an update.
Lazlo,

I understand that is just a tooling issue on my side. Sorry for my delay in fixing up the patches but I have some higher priority work PRs I need to get resolved and that is delaying me fixing up the patch set.

At my work we have a tradition of trying to update our documentation when we onboard new people as that new set of eyes helps point out the small things that could have saved people a lot of time. So I'm not so much complaining, but just trying to help.

In particular, the number of empty lines inserted into the DSC files is
not consistent across the OVMF DSC files, and that fact has nothing to
do with git configuration -- it would need fixing identically even if
the series had been submitted via a github.com PR.
My brain is a little dyslexic so I tend to miss symmetry things like this when I review my own work, but that is why we review the code.

Thanks,

Andrew Fish


Thanks
Laszlo




Please re-sync your calendar to get meeting updates

Soumya Guptha
 

Dear community members,

The groups.io calendar (https://edk2.groups.io/g/devel/calendar?calstart=2020-06-04) has shown some issues in displaying the correct meeting time. When I sync to my outlook, it is off by an hour.

I figured that this is a reported bug in group.io. I worked around it and adjusted the calendar so that it accurately displays the time.

 

Please resync your calendar by subscribing to the calendar  (https://edk2.groups.io/g/devel/ics/7722204/1063275740/feed.ics), so that it will show the correct meeting time.

 

Please note that there is no change to our community meetings, they are scheduled on the first Thursday of every month from 9am-10am (PST) and 7.30pm-8.30pm (PST).

 

Thanks,

Soumya

 

Soumya Guptha
Open Source Firmware PM, SFP/IAGS


 


Re: [PATCH 0/5] ArmPkg/PlatformBootManagerLib: play nice without ConnectAll()

Ard Biesheuvel
 

On 5/27/20 12:01 AM, Leif Lindholm via groups.io wrote:
On Tue, May 26, 2020 at 18:13:54 +0200, Ard Biesheuvel wrote:
Currently, Armpkg's PlatformBootManagerLib connects all controller to
their drivers recursively, even if the active boot option does not
require it. There are some historical reasons for this, some of which are
being addressed in separate patches.

This series addresses the way ArmPkg's PlatformBootManagerLib implementation
deals with the UEFI Shell and the UiApp: currently, the shell is always
added as an ordinary boot option, which will be started if no other boot
options can be started, or if it is the first one in the boot order.

Once we remove the ConnectAll() call from PlatformBootManagerLib, those shells
will be launched without any block or other devices connected, which may
confuse novice users. So before doing so, let's make the handling a bit more
sane:
- add a 's' hotkey that enters the UEFI shell regardless of its priority
in the BootOrder - this shell will be entered with no devices connected
after patch #4
- enter the UiApp as a last resort if no boot options can be started
- make the UEFI Shell boot option hidden, so it is not started by default
(only by hotkey or BootNext)
- remove the ConnectAll() call from PlatformBootManagerLib
- finally, add a plugin library for UiApp to expose the UEFI Shell via an
ordinary main menu option (this works around the fact that patch #3 will
result in the UEFI Shell disappearing from the Boot Manager list).
Entering the shell this way will resemble the old situation, given that
UiApp connects all devices and refreshes all boot options etc at launch.
I get why this set was sent in isolation, but could we also discuss
somewhat what we would plan to do with the edk2-platforms that make
use of the ArmPkg PlatformBootManagerLib?
Not attempting a fallback boot onto network is probably an acceptable
change to pick up, but having an undocumented hotkey as the only way
into a shell that now doesn't map devices could be a bit aggravating.
It is not the only way, and it is not even the preferred way. Patch 5/5 adds an option to the UiApp root menu to enter the UEFI Shell, in a way that is independent from boot option handling. Since you enter UiApp first, all handles will be connected and boot options refreshed as usual.

In cases where you want to avoid this from happening, you can use the 's' key to drop into a shell directly.


Updated Event: TianoCore Community Meeting - APAC/NAMO #cal-invite

devel@edk2.groups.io Calendar <devel@...>
 

TianoCore Community Meeting - APAC/NAMO

When:
Thursday, 5 December 2019
7:30pm to 8:30pm
(UTC-08:00) America/Los Angeles
Repeats: Monthly on the first Thursday, through Wednesday, 3 June 2020

Where:
https://bluejeans.com/889357567?src=join_info

Organizer: Brian Richardson brian.richardson@...

Description:

Meeting URL

 

https://bluejeans.com/889357567?src=join_info

 

Meeting ID

 

889 357 567

 

Want to dial in from a phone?

 

Dial one of the following numbers:

 

+1.408.740.7256 (US (San Jose))

 

+1.408.317.9253 (US (Primary, San Jose))

 

(see all numbers - https://www.bluejeans.com/numbers)

 

Enter the meeting ID and passcode followed by #


Updated Event: TianoCore Community Meeting - EMEA/NAMO #cal-invite

devel@edk2.groups.io Calendar <devel@...>
 

TianoCore Community Meeting - EMEA/NAMO

When:
Thursday, 5 December 2019
9:00am to 10:00am
(UTC-08:00) America/Los Angeles
Repeats: Monthly on the first Thursday, through Wednesday, 3 June 2020

Where:
https://bluejeans.com/889357567?src=join_info

Organizer: Brian Richardson brian.richardson@...

Description:

Meeting URL

https://bluejeans.com/889357567?src=join_info

 

Meeting ID

889 357 567

 

Want to dial in from a phone?

 

Dial one of the following numbers:

+1.408.740.7256 (US (San Jose))

+1.408.317.9253 (US (Primary, San Jose))

(see all numbers - https://www.bluejeans.com/numbers)

 

Enter the meeting ID and passcode followed by #


Re: [PATCH 1/3] BaseTools/GenFv: Add PE/COFF resource sections injection to GenFw

Bob Feng
 

Hi Andrew,

This patch cause building GenFw failure.

cl.exe -c /nologo /Zi /c /O2 /MT /W4 /WX /D _CRT_SECURE_NO_DEPRECATE /D _CRT_NONSTDC_NO_DEPRECATE -I . -I D:\Edk2Maintain\edk2head\edk2\BaseTools\Source\C\Include -I D:\Edk2Maintain\edk2head\edk2\BaseTools\Source\C\Include\Ia32 -I D:\Edk2Maintain\edk2head\edk2\BaseTools\Source\C\Common GenFw.c -FoGenFw.obj
GenFw.c
GenFw.c(3047): error C2220: the following warning is treated as an error
GenFw.c(3047): warning C4244: '=': conversion from 'UINT32' to 'UINT16', possible loss of data
NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.25.28610\bin\HostX86\x86\cl.exe"' : return code '0x2'
Stop.

The possible fix could be change the parameter Type as UINT16 data type. Would you update the patch?

+
+STATIC
+EFI_STATUS
+PatchResourceData (
+ IN UINT32 Type,
+ IN UINT8 *ResourceData,
+ IN UINT32 ResourceDataSize,
+ IN OUT UINT8 **PeCoff,
+ IN OUT UINT32 *PeCoffSize
+ )
+/*++

And a minor comment is about the commit message.

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=557 should change to REF: https://bugzilla.tianocore.org/show_bug.cgi?id=557


Thanks,
Bob

-----Original Message-----
From: Andrew Fish <afish@apple.com>
Sent: Monday, May 25, 2020 5:20 AM
To: devel@edk2.groups.io
Cc: Andrew Fish <afish@apple.com>; Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: [PATCH 1/3] BaseTools/GenFv: Add PE/COFF resource sections injection to GenFw

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=557

The XCODE toolchain does not suport injecting resource sections via libraries so add --rc to GenFw to inject $(MODULE_NAME)hii.rc into the final PE/COFF image.

Since moving exiting code around would break source level debugging we must reuse an existing empty section, or add a new section header.
If there is not space to add a new section header append to the last section. The resource entry if found via a directory entry so the PE/COFF loading code does not depend on the section type.

Signed-off-by: Andrew Fish <afish@apple.com>
Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
---
BaseTools/Source/C/GenFw/GenFw.c | 370 +++++++++++++++++++++++++++++++
1 file changed, 370 insertions(+)

diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c
index 8cab70ba4d5f..748af5dff259 100644
--- a/BaseTools/Source/C/GenFw/GenFw.c
+++ b/BaseTools/Source/C/GenFw/GenFw.c
@@ -96,6 +96,16 @@ ZeroDebugData (
BOOLEAN ZeroDebug ); +STATIC+EFI_STATUS+PatchResourceData (+ IN UINT32 Type,+ IN UINT8 *ResourceData,+ IN UINT32 ResourceDataSize,+ IN OUT UINT8 **PeCoff,+ IN OUT UINT32 *PeCoffSize+ );+ STATIC EFI_STATUS SetStamp (@@ -267,6 +277,11 @@ Returns:
except for -o option. It is a action option.\n\ If it is combined with other action options, the later\n\ input action option will override the previous one.\n");+ fprintf (stdout, " --rc FlieName Append a Hii resource section to the\n\+ last PE/COFF section. The FileName is the resource section to append\n\+ If FileName does not exist this operation is skipped. This feature is\n\+ only intended for toolchains, like XCODE, that don't suport $(RC).\n\+ This option can only be combined with -e\n"); fprintf (stdout, " --rebase NewAddress Rebase image to new base address. New address \n\ is also set to the first none code section header.\n\ It can't be combined with other action options\n\@@ -1059,10 +1074,12 @@ Returns:
CHAR8 **InputFileName; char *OutImageName; char *ModuleType;+ char *RcFileName; CHAR8 *TimeStamp; FILE *fpIn; FILE *fpOut; FILE *fpInOut;+ FILE *fpRc; UINT32 Data; UINT32 *DataPointer; UINT32 *OldDataPointer;@@ -1080,6 +1097,8 @@ Returns:
UINT32 OutputFileLength; UINT8 *InputFileBuffer; UINT32 InputFileLength;+ UINT8 *RcFileBuffer;+ UINT32 RcFileLength; RUNTIME_FUNCTION *RuntimeFunction; UNWIND_INFO *UnwindInfo; STATUS Status;@@ -1116,6 +1135,7 @@ Returns:
time_t OutputFileTime; struct stat Stat_Buf; BOOLEAN ZeroDebugFlag;+ BOOLEAN InsertRcFile; SetUtilityName (UTILITY_NAME); @@ -1128,6 +1148,7 @@ Returns:
mInImageName = NULL; OutImageName = NULL; ModuleType = NULL;+ RcFileName = NULL; Type = 0; Status = STATUS_SUCCESS; FileBuffer = NULL;@@ -1164,6 +1185,7 @@ Returns:
InputFileTime = 0; OutputFileTime = 0; ZeroDebugFlag = FALSE;+ InsertRcFile = FALSE; if (argc == 1) { Error (NULL, 0, 1001, "Missing options", "No input options.");@@ -1436,6 +1458,20 @@ Returns:
continue; } + if (stricmp (argv[0], "--rc") == 0) {+ RcFileName = argv[1];+ argc -= 2;+ argv += 2;++ if (stat (RcFileName, &Stat_Buf) == 0) {+ //+ // File exists+ //+ InsertRcFile = TRUE;+ }+ continue;+ }+ if (argv[0][0] == '-') { Error (NULL, 0, 1000, "Unknown option", argv[0]); goto Finish;@@ -1568,6 +1604,10 @@ Returns:
break; } + if (InsertRcFile) {+ VerboseMsg ("RC input file %s", RcFileName);+ }+ if (ReplaceFlag) { VerboseMsg ("Overwrite the input file with the output content."); }@@ -2052,6 +2092,52 @@ Returns:
} } + //+ // Insert Resources into the image.+ //+ if (InsertRcFile) {+ fpRc = fopen (LongFilePath (RcFileName), "rb");+ if (fpRc == NULL) {+ Error (NULL, 0, 0001, "Error opening file", RcFileName);+ goto Finish;+ }++ RcFileLength = _filelength (fileno (fpRc));+ RcFileBuffer = malloc (RcFileLength);+ if (FileBuffer == NULL) {+ Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");+ fclose (fpRc);+ goto Finish;+ }++ fread (RcFileBuffer, 1, RcFileLength, fpRc);+ fclose (fpRc);++ Status = PatchResourceData (Type, RcFileBuffer, RcFileLength, &FileBuffer, &FileLength);+ if (EFI_ERROR (Status)) {+ Error (NULL, 0, 3000, "Invalid", "RC Patch Data Error status is 0x%x", (int) Status);+ goto Finish;+ }++ fpOut = fopen (LongFilePath (OutImageName), "wb");+ if (fpOut == NULL) {+ Error (NULL, 0, 0001, "Error opening output file", OutImageName);+ goto Finish;+ }++ fwrite (FileBuffer, 1, FileLength, fpOut);++ fclose (fpOut);+ fpOut = NULL;+ VerboseMsg ("the size of output file is %u bytes", (unsigned) FileLength);++ //+ // Write the updated Image+ //+ goto Finish;+ }++ // // Convert ELF image to PeImage //@@ -2761,6 +2847,290 @@ Finish:
return GetUtilityStatus (); } +#define ALIGN_VALUE(Value, Alignment) ((Value) + (((Alignment) - (Value)) & ((Alignment) - 1)))++STATIC+EFI_STATUS+PatchResourceData (+ IN UINT32 Type,+ IN UINT8 *ResourceData,+ IN UINT32 ResourceDataSize,+ IN OUT UINT8 **PeCoff,+ IN OUT UINT32 *PeCoffSize+ )+/*++++Routine Description:++ Embed Resource data into a PE/COFF image.++ If there is free space between the header and the image add a new+ .rsrc section to the PE/COFF image. If no space exists then append+ the resource data to the last section.++Arguments:++ Type - If not zero them update PE/COFF header module type.+ ResourceData - *hii.rc data to insert into the image.+ ResourceDataSize - Size of ResourceData in bytes.+ PeCoff - On input existing PE/COFF, on output input PE/COFF with+ ResourceData embedded.+ PeCoffSize - Size of PeCoff in bytes.++Returns:++ EFI_ABORTED - PeImage is invalid.+ EFI_SUCCESS - Zero debug data successfully.++--*/+{+ UINT8 *FileBuffer;+ UINT32 FileBufferSize;+ EFI_IMAGE_DOS_HEADER *DosHdr;+ EFI_IMAGE_FILE_HEADER *FileHdr;+ EFI_IMAGE_OPTIONAL_HEADER32 *Optional32Hdr;+ EFI_IMAGE_OPTIONAL_HEADER64 *Optional64Hdr;+ EFI_IMAGE_SECTION_HEADER *SectionHeader;+ UINT32 FileAlignment;+ UINT32 Max;+ INTN LastSection;+ INTN EmptySection;+ UINT32 ActualHeaderSize;+ UINT32 StartingPeCoffSize;+ UINT32 NewHeaderSize;+ CHAR8 *Base;+ EFI_IMAGE_RESOURCE_DIRECTORY *ResourceDirectory;+ EFI_IMAGE_RESOURCE_DIRECTORY_ENTRY *ResourceDirectoryEntry;+ EFI_IMAGE_RESOURCE_DIRECTORY_STRING *ResourceDirectoryString;+ EFI_IMAGE_RESOURCE_DATA_ENTRY *ResourceDataEntry;+ EFI_IMAGE_DATA_DIRECTORY *DirectoryEntry;+ CHAR16 *String;+ UINT32 Offset;+ UINT32 Index;++ //+ // Grow the file in units of FileAlignment+ //+ DosHdr = (EFI_IMAGE_DOS_HEADER *) *PeCoff;+ if (DosHdr->e_magic != EFI_IMAGE_DOS_SIGNATURE) {+ // NO DOS header, must start with PE/COFF header+ FileHdr = (EFI_IMAGE_FILE_HEADER *)(*PeCoff + sizeof (UINT32));+ } else {+ FileHdr = (EFI_IMAGE_FILE_HEADER *)(*PeCoff + DosHdr->e_lfanew + sizeof (UINT32));+ }++ Optional32Hdr = (EFI_IMAGE_OPTIONAL_HEADER32 *) ((UINT8*) FileHdr + sizeof (EFI_IMAGE_FILE_HEADER));+ Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr + sizeof (EFI_IMAGE_FILE_HEADER));+ if (Optional32Hdr->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {+ FileAlignment = Optional32Hdr->FileAlignment;+ SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) Optional32Hdr + FileHdr->SizeOfOptionalHeader);+ } else {+ FileAlignment = Optional64Hdr->FileAlignment;+ SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) Optional64Hdr + FileHdr->SizeOfOptionalHeader);+ }++ LastSection = -1;+ for (Index = 0, Max = 0; Index < FileHdr->NumberOfSections; Index++) {+ if (SectionHeader[Index].PointerToRawData > Max) {+ Max = SectionHeader[Index].PointerToRawData;+ LastSection = Index;+ }+ }++ EmptySection = -1;+ for (Index = 0; Index < FileHdr->NumberOfSections; Index++) {+ if ((SectionHeader[Index].Misc.VirtualSize == 0) && (SectionHeader[Index].SizeOfRawData == 0)) {+ //+ // No Data or Zero Fill so we can repurpose this entry.+ //+ EmptySection = Index;+ break;+ }+ }++ if (EmptySection == -1) {+ ActualHeaderSize = (UINTN)(&SectionHeader[FileHdr->NumberOfSections]) - (UINTN)*PeCoff;+ if ((ActualHeaderSize + sizeof (EFI_IMAGE_SECTION_HEADER)) <= Optional32Hdr->SizeOfHeaders) {+ //+ // There is space to inject a new section.+ //+ FileHdr->NumberOfSections += 1;+ EmptySection = Index;+ }+ }++ StartingPeCoffSize = SectionHeader[LastSection].PointerToRawData + SectionHeader[LastSection].SizeOfRawData;+ if (SectionHeader[LastSection].Misc.VirtualSize > SectionHeader[LastSection].SizeOfRawData) {+ StartingPeCoffSize += SectionHeader[LastSection].Misc.VirtualSize - SectionHeader[LastSection].SizeOfRawData;+ }++ FileBufferSize = ALIGN_VALUE(StartingPeCoffSize + ResourceDataSize, FileAlignment);+ FileBuffer = malloc (FileBufferSize);+ if (FileBuffer == NULL) {+ return RETURN_OUT_OF_RESOURCES;+ }+ memset (FileBuffer, 0, FileBufferSize);++ //+ // Append the Resource Data to the end of the PE/COFF image.+ //+ NewHeaderSize = Optional32Hdr->SizeOfHeaders;+ CopyMem (FileBuffer, *PeCoff, (StartingPeCoffSize > *PeCoffSize) ? *PeCoffSize: StartingPeCoffSize);+ CopyMem (FileBuffer + StartingPeCoffSize, ResourceData, ResourceDataSize);++ free (*PeCoff);+ *PeCoff = FileBuffer;+ *PeCoffSize = FileBufferSize;++ DosHdr = (EFI_IMAGE_DOS_HEADER *)FileBuffer;+ if (DosHdr->e_magic != EFI_IMAGE_DOS_SIGNATURE) {+ // NO DOS header, must start with PE/COFF header+ FileHdr = (EFI_IMAGE_FILE_HEADER *)(FileBuffer + sizeof (UINT32));+ } else {+ FileHdr = (EFI_IMAGE_FILE_HEADER *)(FileBuffer + DosHdr->e_lfanew + sizeof (UINT32));+ }++ //+ // Get Resource EntryTable offset, and Section header+ //+ Optional32Hdr = (EFI_IMAGE_OPTIONAL_HEADER32 *) ((UINT8*) FileHdr + sizeof (EFI_IMAGE_FILE_HEADER));+ Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr + sizeof (EFI_IMAGE_FILE_HEADER));+ DirectoryEntry = NULL;+ if (Optional32Hdr->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {+ SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) Optional32Hdr + FileHdr->SizeOfOptionalHeader);+ if (Optional32Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_RESOURCE) {+ DirectoryEntry = &Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_RESOURCE];+ }+ } else {+ SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) Optional64Hdr + FileHdr->SizeOfOptionalHeader);+ if (Optional64Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_RESOURCE) {+ DirectoryEntry = &Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_RESOURCE];+ }+ }++ if (DirectoryEntry == NULL) {+ return RETURN_OUT_OF_RESOURCES;+ }++ if (EmptySection != -1) {+ //+ // Create a new section+ //+ CopyMem (SectionHeader[EmptySection].Name, ".rsrc\0\0\0", EFI_IMAGE_SIZEOF_SHORT_NAME);+ SectionHeader[EmptySection].Misc.VirtualSize = ResourceDataSize;+ SectionHeader[EmptySection].VirtualAddress = StartingPeCoffSize;+ SectionHeader[EmptySection].SizeOfRawData = ALIGN_VALUE(ResourceDataSize, FileAlignment);+ SectionHeader[EmptySection].PointerToRawData = StartingPeCoffSize;++ SectionHeader[EmptySection].Characteristics = EFI_IMAGE_SCN_MEM_EXECUTE | EFI_IMAGE_SCN_MEM_READ;+ SectionHeader[EmptySection].Characteristics |= EFI_IMAGE_SCN_CNT_INITIALIZED_DATA | EFI_IMAGE_SCN_CNT_CODE;++ DirectoryEntry->VirtualAddress = SectionHeader[EmptySection].VirtualAddress;+ } else {+ //+ // Grow the last section to include the resources.+ // For Xcode this is always the .debug section.+ //+ SectionHeader[LastSection].SizeOfRawData = ALIGN_VALUE(SectionHeader[LastSection].SizeOfRawData + ResourceDataSize, FileAlignment);++ //+ // Make sure the Virtual Size uses the file aligned actual size, since we are growing the file.+ //+ SectionHeader[LastSection].Misc.VirtualSize = SectionHeader[LastSection].SizeOfRawData;++ DirectoryEntry->VirtualAddress = StartingPeCoffSize;+ }+ DirectoryEntry->Size = ResourceDataSize;++ Optional32Hdr->SizeOfImage = ALIGN_VALUE(*PeCoffSize, Optional32Hdr->SectionAlignment);+ if (Type != 0) {+ Optional32Hdr->Subsystem = Type;+ }++ //+ // It looks like the ResourceDataEntry->OffsetToData is relative to the rc file,+ // but PeCoffLoaderLoadImage() assumes it is a PE/COFF VirtualAddress so we need+ // to fix it up. Walk the ResourceDataEntry just like PeCoffLoaderLoadImage() and+ // patch it.+ //+ Base = (CHAR8 *)(FileBuffer + StartingPeCoffSize);+ ResourceDirectory = (EFI_IMAGE_RESOURCE_DIRECTORY *)Base;+ ResourceDirectoryEntry = (EFI_IMAGE_RESOURCE_DIRECTORY_ENTRY *)(ResourceDirectory + 1);++ for (Index = 0; Index < ResourceDirectory->NumberOfNamedEntries; Index++) {+ if (ResourceDirectoryEntry->u1.s.NameIsString) {+ //+ // Check the ResourceDirectoryEntry->u1.s.NameOffset before use it.+ //+ if (ResourceDirectoryEntry->u1.s.NameOffset >= DirectoryEntry->Size) {+ return RETURN_UNSUPPORTED;+ }+ ResourceDirectoryString = (EFI_IMAGE_RESOURCE_DIRECTORY_STRING *) (Base + ResourceDirectoryEntry->u1.s.NameOffset);+ String = &ResourceDirectoryString->String[0];++ if (ResourceDirectoryString->Length == 3 &&+ String[0] == L'H' &&+ String[1] == L'I' &&+ String[2] == L'I') {+ //+ // Resource Type "HII" found+ //+ if (ResourceDirectoryEntry->u2.s.DataIsDirectory) {+ //+ // Move to next level - resource Name+ //+ if (ResourceDirectoryEntry->u2.s.OffsetToDirectory >= DirectoryEntry->Size) {+ return RETURN_UNSUPPORTED;+ }+ ResourceDirectory = (EFI_IMAGE_RESOURCE_DIRECTORY *) (Base + ResourceDirectoryEntry->u2.s.OffsetToDirectory);+ Offset = ResourceDirectoryEntry->u2.s.OffsetToDirectory + sizeof (EFI_IMAGE_RESOURCE_DIRECTORY) ++ sizeof (EFI_IMAGE_RESOURCE_DIRECTORY_ENTRY) * (ResourceDirectory->NumberOfNamedEntries + ResourceDirectory->NumberOfIdEntries);+ if (Offset > DirectoryEntry->Size) {+ return RETURN_UNSUPPORTED;+ }+ ResourceDirectoryEntry = (EFI_IMAGE_RESOURCE_DIRECTORY_ENTRY *) (ResourceDirectory + 1);++ if (ResourceDirectoryEntry->u2.s.DataIsDirectory) {+ //+ // Move to next level - resource Language+ //+ if (ResourceDirectoryEntry->u2.s.OffsetToDirectory >= DirectoryEntry->Size) {+ return RETURN_UNSUPPORTED;+ }+ ResourceDirectory = (EFI_IMAGE_RESOURCE_DIRECTORY *) (Base + ResourceDirectoryEntry->u2.s.OffsetToDirectory);+ Offset = ResourceDirectoryEntry->u2.s.OffsetToDirectory + sizeof (EFI_IMAGE_RESOURCE_DIRECTORY) ++ sizeof (EFI_IMAGE_RESOURCE_DIRECTORY_ENTRY) * (ResourceDirectory->NumberOfNamedEntries + ResourceDirectory->NumberOfIdEntries);+ if (Offset > DirectoryEntry->Size) {+ return RETURN_UNSUPPORTED;+ }+ ResourceDirectoryEntry = (EFI_IMAGE_RESOURCE_DIRECTORY_ENTRY *) (ResourceDirectory + 1);+ }+ }++ //+ // Now it ought to be resource Data+ //+ if (!ResourceDirectoryEntry->u2.s.DataIsDirectory) {+ if (ResourceDirectoryEntry->u2.OffsetToData >= DirectoryEntry->Size) {+ return RETURN_UNSUPPORTED;+ }+ ResourceDataEntry = (EFI_IMAGE_RESOURCE_DATA_ENTRY *) (Base + ResourceDirectoryEntry->u2.OffsetToData);++ //+ // Adjust OffsetToData to be a PE/COFF Virtual address in the updated image.+ //+ ResourceDataEntry->OffsetToData += (UINTN)DirectoryEntry->VirtualAddress;+ break;+ }+ }+ }+ ResourceDirectoryEntry++;+ }++ return EFI_SUCCESS;+}++ STATIC EFI_STATUS ZeroDebugData (--
2.24.1 (Apple Git-126)


Re: [EXTERNAL] Re: Hard Feature Freeze starts now for edk2-stable202005

Bret Barkelew
 

Thanks!

 

- Bret

 


From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Liming Gao via groups.io <liming.gao@...>
Sent: Tuesday, May 26, 2020 7:21:00 PM
To: Bret Barkelew <Bret.Barkelew@...>; Laszlo Ersek <lersek@...>; devel@edk2.groups.io <devel@edk2.groups.io>; announce@edk2.groups.io <announce@edk2.groups.io>
Cc: Leif Lindholm <leif@...>; afish@... <afish@...>; Kinney, Michael D <michael.d.kinney@...>
Subject: Re: [edk2-devel] [EXTERNAL] Re: Hard Feature Freeze starts now for edk2-stable202005
 

I create PR https://github.com/tianocore/edk2/pull/644 for this patch.

 

Thanks

Liming

From: Bret Barkelew <Bret.Barkelew@...>
Sent: Wednesday, May 27, 2020 6:11 AM
To: Laszlo Ersek <lersek@...>; devel@edk2.groups.io; Gao, Liming <liming.gao@...>; announce@edk2.groups.io
Cc: Leif Lindholm <leif@...>; afish@...; Kinney, Michael D <michael.d.kinney@...>
Subject: Re: [EXTERNAL] Re: Hard Feature Freeze starts now for edk2-stable202005

 

I just looked into it, and I think that AsciiStrCpyS() was the wrong function to call in this loop anyway. AsciiStrCpyS() will fail without copying any characters.

AsciiStrnCpyS() will perform the string "slicing"/"chunking" that the loop seems to expect.

 

The bug stands and we should try to get that bug fix into the stable tag. Thanks!

 

- Bret

 


From: Laszlo Ersek <lersek@...>
Sent: Monday, May 25, 2020 10:46 AM
To: Bret Barkelew <Bret.Barkelew@...>; devel@edk2.groups.io <devel@edk2.groups.io>; liming.gao <liming.gao@...>; announce@edk2.groups.io <announce@edk2.groups.io>
Cc: Leif Lindholm <leif@...>; afish@... <afish@...>; Kinney, Michael D <michael.d.kinney@...>
Subject: [EXTERNAL] Re: Hard Feature Freeze starts now for edk2-stable202005

 

Hi Bret,

On 05/22/20 17:11, Bret Barkelew wrote:
> We’d like to ask that this patch be considered for the stable tag:
> [PATCH v1 1/1] UnitTestFrameworkPkg/UnitTestResultReportLib: Use AsciiStrnCpyS()
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2721&amp;data=02%7C01%7Cbret.barkelew%40microsoft.com%7C2698d0e553c04b47194c08d800d398b8%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637260256091133309&amp;sdata=MDKQ1CKq9%2B9AfPML0JxcND47UIcQUAibUSAVlfW5iZc%3D&amp;reserved=0
>
> The patch was reviewed prior to the hard freeze date, and is a small change that affects new(ish) code that is not heavily utilized yet.

does the original issue (reported in TianoCore#2721) persist with
TianoCore#2054 fixed?

My understanding (from TianoCore#2721) is that the original
AsciiStrCpyS() call is not buggy, it just triggers a (per spec) error
condition in AsciiStrCpyS(). Previously, that would indeed trip an
ASSERT(), but AIUI that issue has been resolved generally with
TianoCore#2054.

If the AsciiStrCpyS() call remains an issue with the ASSERT() removed,
then replacing the call with AsciiStrnCpyS() still seems like a bugfix
to me, not a "feature", so I think the patch is eligible for merging
during the HFF.

Mike, can you please merge the patch (if it's still needed)?

Thanks
Laszlo


>
> - Bret
>
> From: Liming Gao via groups.io<mailto:liming.gao@...>
> Sent: Friday, May 22, 2020 1:01 AM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; announce@edk2.groups.io<mailto:announce@edk2.groups.io>
> Cc: Laszlo Ersek<mailto:lersek@...>; Leif Lindholm<mailto:leif@...>; afish@...<mailto:afish@...>; Kinney, Michael D<mailto:michael.d.kinney@...>
> Subject: [EXTERNAL] [edk2-devel] Hard Feature Freeze starts now for edk2-stable202005
>
> Hi, all
>   Today, we enter into Hard Feature Freeze phase until edk2-stable202005 tag is created at 2020-05-29. In this phase, there is no feature to be pushed. The critical bug fix is still allowed.
>
>   If the patch is sent after Hard Feature Freeze, and plans to catch this stable tag, please add edk2-stable202005 key words in the patch title and BZ, and also cc to Tianocore Stewards, then Stewards can give the comments.
>
> Below is edk2-stable202005 tag planning.
> Date (00:00:00 UTC-8) Description
> 2020-03-04      Beginning of development
> 2020-05-08      Feature Planning Freeze
> 2020-05-15      Soft Feature Freeze
> 2020-05-22      Hard Feature Freeze
> 2020-05-29      Release
>
> Thanks
> Liming
> >
>


Re: [EXTERNAL] Re: Hard Feature Freeze starts now for edk2-stable202005

Liming Gao
 

I create PR https://github.com/tianocore/edk2/pull/644 for this patch.

 

Thanks

Liming

From: Bret Barkelew <Bret.Barkelew@...>
Sent: Wednesday, May 27, 2020 6:11 AM
To: Laszlo Ersek <lersek@...>; devel@edk2.groups.io; Gao, Liming <liming.gao@...>; announce@edk2.groups.io
Cc: Leif Lindholm <leif@...>; afish@...; Kinney, Michael D <michael.d.kinney@...>
Subject: Re: [EXTERNAL] Re: Hard Feature Freeze starts now for edk2-stable202005

 

I just looked into it, and I think that AsciiStrCpyS() was the wrong function to call in this loop anyway. AsciiStrCpyS() will fail without copying any characters.

AsciiStrnCpyS() will perform the string "slicing"/"chunking" that the loop seems to expect.

 

The bug stands and we should try to get that bug fix into the stable tag. Thanks!

 

- Bret

 


From: Laszlo Ersek <lersek@...>
Sent: Monday, May 25, 2020 10:46 AM
To: Bret Barkelew <Bret.Barkelew@...>; devel@edk2.groups.io <devel@edk2.groups.io>; liming.gao <liming.gao@...>; announce@edk2.groups.io <announce@edk2.groups.io>
Cc: Leif Lindholm <leif@...>; afish@... <afish@...>; Kinney, Michael D <michael.d.kinney@...>
Subject: [EXTERNAL] Re: Hard Feature Freeze starts now for edk2-stable202005

 

Hi Bret,

On 05/22/20 17:11, Bret Barkelew wrote:
> We’d like to ask that this patch be considered for the stable tag:
> [PATCH v1 1/1] UnitTestFrameworkPkg/UnitTestResultReportLib: Use AsciiStrnCpyS()
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2721&amp;data=02%7C01%7Cbret.barkelew%40microsoft.com%7C2698d0e553c04b47194c08d800d398b8%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637260256091133309&amp;sdata=MDKQ1CKq9%2B9AfPML0JxcND47UIcQUAibUSAVlfW5iZc%3D&amp;reserved=0
>
> The patch was reviewed prior to the hard freeze date, and is a small change that affects new(ish) code that is not heavily utilized yet.

does the original issue (reported in TianoCore#2721) persist with
TianoCore#2054 fixed?

My understanding (from TianoCore#2721) is that the original
AsciiStrCpyS() call is not buggy, it just triggers a (per spec) error
condition in AsciiStrCpyS(). Previously, that would indeed trip an
ASSERT(), but AIUI that issue has been resolved generally with
TianoCore#2054.

If the AsciiStrCpyS() call remains an issue with the ASSERT() removed,
then replacing the call with AsciiStrnCpyS() still seems like a bugfix
to me, not a "feature", so I think the patch is eligible for merging
during the HFF.

Mike, can you please merge the patch (if it's still needed)?

Thanks
Laszlo


>
> - Bret
>
> From: Liming Gao via groups.io<mailto:liming.gao@...>
> Sent: Friday, May 22, 2020 1:01 AM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; announce@edk2.groups.io<mailto:announce@edk2.groups.io>
> Cc: Laszlo Ersek<mailto:lersek@...>; Leif Lindholm<mailto:leif@...>; afish@...<mailto:afish@...>; Kinney, Michael D<mailto:michael.d.kinney@...>
> Subject: [EXTERNAL] [edk2-devel] Hard Feature Freeze starts now for edk2-stable202005
>
> Hi, all
>   Today, we enter into Hard Feature Freeze phase until edk2-stable202005 tag is created at 2020-05-29. In this phase, there is no feature to be pushed. The critical bug fix is still allowed.
>
>   If the patch is sent after Hard Feature Freeze, and plans to catch this stable tag, please add edk2-stable202005 key words in the patch title and BZ, and also cc to Tianocore Stewards, then Stewards can give the comments.
>
> Below is edk2-stable202005 tag planning.
> Date (00:00:00 UTC-8) Description
> 2020-03-04      Beginning of development
> 2020-05-08      Feature Planning Freeze
> 2020-05-15      Soft Feature Freeze
> 2020-05-22      Hard Feature Freeze
> 2020-05-29      Release
>
> Thanks
> Liming
> >
>


Re: [PATCH 1/1] BaseTools: Turn on Link Time Optimization (LTO) for XCOODE

Bob Feng
 

Andrew,

Would you please update the BZ status to IN_PROGRESS? And paste the patch review link to the comments?

Reviewed-by: Bob Feng <bob.c.feng@intel.com>

Thanks,
Bob

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Andrew Fish via groups.io
Sent: Monday, May 25, 2020 10:38 AM
To: devel@edk2.groups.io
Cc: Andrew Fish <afish@apple.com>; Gao, Liming <liming.gao@intel.com>; Liu, Zhiguang <zhiguang.liu@intel.com>
Subject: [edk2-devel] [PATCH 1/1] BaseTools: Turn on Link Time Optimization (LTO) for XCOODE

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1202

Turn on LTO for XCODE.

We need to pass -object_path_lto <file> to the linker to keep source level debugging working.

OVMF X64 before:
SECFV [14%Full] 212992 total, 30224 used, 182768 free PEIFV [29%Full] 917504 total, 273256 used, 644248 free DXEFV [40%Full] 12582912 total, 5096904 used, 7486008 free FVMAIN_COMPACT [37%Full] 3440640 total, 1290240 used, 2150400 free

After:
SECFV [10%Full] 212992 total, 23064 used, 189928 free PEIFV [20%Full] 917504 total, 192328 used, 725176 free DXEFV [33%Full] 12582912 total, 4193632 used, 8389280 free FVMAIN_COMPACT [33%Full] 3440640 total, 1165352 used, 2275288 free

Signed-off-by: Andrew Fish <afish@apple.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
---
BaseTools/Conf/tools_def.template | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template
index 923517b5c296..efe8e47af851 100755
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -2927,9 +2927,9 @@ RELEASE_XCODE5_*_MTOC_FLAGS = -align 0x20
#################### # IA-32 definitions ####################- DEBUG_XCODE5_IA32_DLINK_FLAGS = -arch i386 -u _$(IMAGE_ENTRY_POINT) -e _$(IMAGE_ENTRY_POINT) -preload -segalign 0x20 -pie -all_load -dead_strip -seg1addr 0x240 -read_only_relocs suppress -map $(DEST_DIR_DEBUG)/$(BASE_NAME).map+ DEBUG_XCODE5_IA32_DLINK_FLAGS = -arch i386 -u _$(IMAGE_ENTRY_POINT) -e _$(IMAGE_ENTRY_POINT) -preload -segalign 0x20 -pie -all_load -dead_strip -seg1addr 0x240 -read_only_relocs suppress -map $(DEST_DIR_DEBUG)/$(BASE_NAME).map -object_path_lto $(DEST_DIR_DEBUG)/$(BASE_NAME).lto NOOPT_XCODE5_IA32_DLINK_FLAGS = -arch i386 -u _$(IMAGE_ENTRY_POINT) -e _$(IMAGE_ENTRY_POINT) -preload -segalign 0x20 -pie -all_load -dead_strip -seg1addr 0x240 -read_only_relocs suppress -map $(DEST_DIR_DEBUG)/$(BASE_NAME).map-RELEASE_XCODE5_IA32_DLINK_FLAGS = -arch i386 -u _$(IMAGE_ENTRY_POINT) -e _$(IMAGE_ENTRY_POINT) -preload -segalign 0x20 -pie -all_load -dead_strip -seg1addr 0x240 -read_only_relocs suppress -map $(DEST_DIR_DEBUG)/$(BASE_NAME).map+RELEASE_XCODE5_IA32_DLINK_FLAGS = -arch i386 -u _$(IMAGE_ENTRY_POINT) -e _$(IMAGE_ENTRY_POINT) -preload -segalign 0x20 -pie -all_load -dead_strip -seg1addr 0x240 -read_only_relocs suppress -map $(DEST_DIR_DEBUG)/$(BASE_NAME).map -object_path_lto $(DEST_DIR_DEBUG)/$(BASE_NAME).lto *_XCODE5_IA32_SLINK_FLAGS = -static -o DEBUG_XCODE5_IA32_ASM_FLAGS = -arch i386 -g@@ -2938,16 +2938,16 @@ RELEASE_XCODE5_IA32_ASM_FLAGS = -arch i386
*_XCODE5_IA32_NASM_FLAGS = -f macho32 - DEBUG_XCODE5_IA32_CC_FLAGS = -arch i386 -c -g -Os -Wall -Werror -include AutoGen.h -funsigned-char -fno-stack-protector -fno-builtin -fshort-wchar -fasm-blocks -mdynamic-no-pic -mno-implicit-float -mms-bitfields -msoft-float -Wno-unused-parameter -Wno-missing-braces -Wno-missing-field-initializers -Wno-tautological-compare -Wno-sign-compare -Wno-varargs -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang $(PLATFORM_FLAGS)-RELEASE_XCODE5_IA32_CC_FLAGS = -arch i386 -c -Os -Wall -Werror -include AutoGen.h -funsigned-char -fno-stack-protector -fno-builtin -fshort-wchar -fasm-blocks -mdynamic-no-pic -mno-implicit-float -mms-bitfields -msoft-float -Wno-unused-parameter -Wno-missing-braces -Wno-missing-field-initializers -Wno-tautological-compare -Wno-sign-compare -Wno-varargs -Wno-unused-const-variable -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang $(PLATFORM_FLAGS)+ DEBUG_XCODE5_IA32_CC_FLAGS = -arch i386 -c -g -Os -flto -Wall -Werror -include AutoGen.h -funsigned-char -fno-stack-protector -fno-builtin -fshort-wchar -fasm-blocks -mdynamic-no-pic -mno-implicit-float -mms-bitfields -msoft-float -Wno-unused-parameter -Wno-missing-braces -Wno-missing-field-initializers -Wno-tautological-compare -Wno-sign-compare -Wno-varargs -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang $(PLATFORM_FLAGS)+RELEASE_XCODE5_IA32_CC_FLAGS = -arch i386 -c -Os -flto -Wall -Werror -include AutoGen.h -funsigned-char -fno-stack-protector -fno-builtin -fshort-wchar -fasm-blocks -mdynamic-no-pic -mno-implicit-float -mms-bitfields -msoft-float -Wno-unused-parameter -Wno-missing-braces -Wno-missing-field-initializers -Wno-tautological-compare -Wno-sign-compare -Wno-varargs -Wno-unused-const-variable -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang $(PLATFORM_FLAGS) NOOPT_XCODE5_IA32_CC_FLAGS = -arch i386 -c -g -O0 -Wall -Werror -include AutoGen.h -funsigned-char -fno-stack-protector -fno-builtin -fshort-wchar -fasm-blocks -mdynamic-no-pic -mno-implicit-float -mms-bitfields -msoft-float -Wno-unused-parameter -Wno-missing-braces -Wno-missing-field-initializers -Wno-tautological-compare -Wno-sign-compare -Wno-varargs -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang $(PLATFORM_FLAGS) ################## # X64 definitions ##################- DEBUG_XCODE5_X64_DLINK_FLAGS = -arch x86_64 -u _$(IMAGE_ENTRY_POINT) -e _$(IMAGE_ENTRY_POINT) -preload -segalign 0x20 -pie -all_load -dead_strip -seg1addr 0x240 -map $(DEST_DIR_DEBUG)/$(BASE_NAME).map+ DEBUG_XCODE5_X64_DLINK_FLAGS = -arch x86_64 -u _$(IMAGE_ENTRY_POINT) -e _$(IMAGE_ENTRY_POINT) -preload -segalign 0x20 -pie -all_load -dead_strip -seg1addr 0x240 -map $(DEST_DIR_DEBUG)/$(BASE_NAME).map -object_path_lto $(DEST_DIR_DEBUG)/$(BASE_NAME).lto NOOPT_XCODE5_X64_DLINK_FLAGS = -arch x86_64 -u _$(IMAGE_ENTRY_POINT) -e _$(IMAGE_ENTRY_POINT) -preload -segalign 0x20 -pie -all_load -dead_strip -seg1addr 0x240 -map $(DEST_DIR_DEBUG)/$(BASE_NAME).map-RELEASE_XCODE5_X64_DLINK_FLAGS = -arch x86_64 -u _$(IMAGE_ENTRY_POINT) -e _$(IMAGE_ENTRY_POINT) -preload -segalign 0x20 -pie -all_load -dead_strip -seg1addr 0x240 -map $(DEST_DIR_DEBUG)/$(BASE_NAME).map+RELEASE_XCODE5_X64_DLINK_FLAGS = -arch x86_64 -u _$(IMAGE_ENTRY_POINT) -e _$(IMAGE_ENTRY_POINT) -preload -segalign 0x20 -pie -all_load -dead_strip -seg1addr 0x240 -map $(DEST_DIR_DEBUG)/$(BASE_NAME).map -object_path_lto $(DEST_DIR_DEBUG)/$(BASE_NAME).lto *_XCODE5_X64_SLINK_FLAGS = -static -o DEBUG_XCODE5_X64_ASM_FLAGS = -arch x86_64 -g@@ -2957,9 +2957,9 @@ RELEASE_XCODE5_X64_ASM_FLAGS = -arch x86_64
*_XCODE5_*_PP_FLAGS = -E -x assembler-with-cpp -include AutoGen.h *_XCODE5_*_VFRPP_FLAGS = -x c -E -P -DVFRCOMPILE -include $(MODULE_NAME)StrDefs.h - DEBUG_XCODE5_X64_CC_FLAGS = -target x86_64-pc-win32-macho -c -g -gdwarf -Os -Wall -Werror -Wextra -include AutoGen.h -funsigned-char -fno-ms-extensions -fno-stack-protector -fno-builtin -fshort-wchar -mno-implicit-float -mms-bitfields -Wno-unused-parameter -Wno-missing-braces -Wno-missing-field-initializers -Wno-tautological-compare -Wno-sign-compare -Wno-varargs -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -D NO_MSABI_VA_FUNCS $(PLATFORM_FLAGS)+ DEBUG_XCODE5_X64_CC_FLAGS = -target x86_64-pc-win32-macho -c -g -gdwarf -Os -flto -Wall -Werror -Wextra -include AutoGen.h -funsigned-char -fno-ms-extensions -fno-stack-protector -fno-builtin -fshort-wchar -mno-implicit-float -mms-bitfields -Wno-unused-parameter -Wno-missing-braces -Wno-missing-field-initializers -Wno-tautological-compare -Wno-sign-compare -Wno-varargs -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -D NO_MSABI_VA_FUNCS $(PLATFORM_FLAGS) NOOPT_XCODE5_X64_CC_FLAGS = -target x86_64-pc-win32-macho -c -g -gdwarf -O0 -Wall -Werror -Wextra -include AutoGen.h -funsigned-char -fno-ms-extensions -fno-stack-protector -fno-builtin -fshort-wchar -mno-implicit-float -mms-bitfields -Wno-unused-parameter -Wno-missing-braces -Wno-missing-field-initializers -Wno-tautological-compare -Wno-sign-compare -Wno-varargs -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -D NO_MSABI_VA_FUNCS $(PLATFORM_FLAGS)-RELEASE_XCODE5_X64_CC_FLAGS = -target x86_64-pc-win32-macho -c -Os -Wall -Werror -Wextra -include AutoGen.h -funsigned-char -fno-ms-extensions -fno-stack-protector -fno-builtin -fshort-wchar -mno-implicit-float -mms-bitfields -Wno-unused-parameter -Wno-missing-braces -Wno-missing-field-initializers -Wno-tautological-compare -Wno-sign-compare -Wno-varargs -Wno-unused-const-variable -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -D NO_MSABI_VA_FUNCS $(PLATFORM_FLAGS)+RELEASE_XCODE5_X64_CC_FLAGS = -target x86_64-pc-win32-macho -c -Os -flto -Wall -Werror -Wextra -include AutoGen.h -funsigned-char -fno-ms-extensions -fno-stack-protector -fno-builtin -fshort-wchar -mno-implicit-float -mms-bitfields -Wno-unused-parameter -Wno-missing-braces -Wno-missing-field-initializers -Wno-tautological-compare -Wno-sign-compare -Wno-varargs -Wno-unused-const-variable -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -D NO_MSABI_VA_FUNCS $(PLATFORM_FLAGS) #################################################################################### #--
2.24.1 (Apple Git-126)


-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60202): https://edk2.groups.io/g/devel/message/60202
Mute This Topic: https://groups.io/mt/74449794/1768742
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [bob.c.feng@intel.com] -=-=-=-=-=-=


Re: [EXTERNAL] [edk2-devel] [edk2-rfc] GitHub Pull Request based Code Review Process

Bret Barkelew
 

So, today I followed the Wiki (that I had never seen) and now I’m staring down the barrel of this fellow…

 

[Not using SSL_VERIFY_PEER due to out-of-date IO::Socket::SSL.

To use SSL please install IO::Socket::SSL with version>=2.007 at /usr/share/perl5/core_perl/Net/SMTP.pm line 270.]

 

Anyone have thoughts? I’mma go get a scotch.

 

- Bret

 

From: Andrew Fish
Sent: Monday, May 25, 2020 11:28 AM
To: Laszlo Ersek
Cc: Bret Barkelew; devel@edk2.groups.io; spbrogan@...; rfc@edk2.groups.io; Desimone, Nathaniel L; Kinney, Michael D; Leif Lindholm (Nuvia address)
Subject: Re: [EXTERNAL] [edk2-devel] [edk2-rfc] GitHub Pull Request based Code Review Process

 



> On May 25, 2020, at 11:10 AM, Laszlo Ersek <lersek@...> wrote:
>
> Hi Andrew,
>
> On 05/25/20 06:09, Andrew Fish wrote:
>
>> I also found I had to Bing/Google to find the detailed instructions I
>> needed as a developer, as the Wiki seems to assume you just know the
>> Linux kernel patch process. That feels like an area we can improve.
>
> (apologies if I've lost context; please disregard my message below in
> that case).
>
> I wrote the following wiki article originally in 2016:
>
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki%2FLaszlo%27s-unkempt-git-guide-for-edk2-contributors-and-maintainers&amp;data=02%7C01%7CBret.Barkelew%40microsoft.com%7C2e084613c24f433ca0a508d800d978de%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637260281325061578&amp;sdata=nIMHQLnu8F%2F%2BTMMsLKVXbWnO6AWE9WuUu5k1TK4HgTQ%3D&amp;reserved=0
>
> I wrote it specifically for developers & maintainers with no (or almost
> no) prior git / mailing list experience. Multiple developers confirmed
> later that the article had helped them.
>

Laszlo,

Your wiki article was very very helpful. I just could not find it from the Tianocre wiki. It would be good if we could link to it from here [1], maybe as add to this: "Are you new to using git? If so, then the New to git page may be helpful."?

There are a lot folks who use git but don't use the email based review so they have never setup git with emali before. Your wiki, plus me figuring out the magic internal SMTP reflector (I reached out on an internal git malling list) is what got me unblocked.

[1] https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki%2FEDK-II-Development-Process&amp;data=02%7C01%7CBret.Barkelew%40microsoft.com%7C2e084613c24f433ca0a508d800d978de%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637260281325061578&amp;sdata=XPx6jrloPC9LW0iCecZuFmaz3JgjCSQYeF0PEyGW4I0%3D&amp;reserved=0

Thanks,

Andrew Fish

> Thanks
> Laszlo
>

 


Upcoming Event: TianoCore Bug Triage - APAC / NAMO - Tue, 05/26/2020 6:30pm-7:30pm #cal-reminder

devel@edk2.groups.io Calendar <devel@...>
 

Reminder: TianoCore Bug Triage - APAC / NAMO

When: Tuesday, 26 May 2020, 6:30pm to 7:30pm, (GMT-07:00) America/Los Angeles

Where:https://bluejeans.com/889357567?src=join_info

View Event

Organizer: Brian Richardson brian.richardson@...

Description:

https://www.tianocore.org/bug-triage

 

Meeting URL

https://bluejeans.com/889357567?src=join_info

 

Meeting ID

889 357 567

 

Want to dial in from a phone?

Dial one of the following numbers:

+1.408.740.7256 (US (San Jose))

+1.408.317.9253 (US (Primary, San Jose))

 

(see all numbers - https://www.bluejeans.com/numbers)

Enter the meeting ID and passcode followed by #


Re: [PATCH v2 01/11] PcAtChipsetPkg: Add MMIO Support to RTC driver

Guomin Jiang
 

I am interest with the patch,

I am busy and may review it after August.

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of André
Przywara
Sent: Friday, May 15, 2020 6:51 PM
To: Sami Mujawar <sami.mujawar@arm.com>; devel@edk2.groups.io
Cc: ard.biesheuvel@arm.com; leif@nuviainc.com; Ni, Ray
<ray.ni@intel.com>; Alexandru.Elisei@arm.com; Matteo.Carlini@arm.com;
Laura.Moretta@arm.com; nd@arm.com
Subject: Re: [edk2-devel] [PATCH v2 01/11] PcAtChipsetPkg: Add MMIO
Support to RTC driver

On 14/05/2020 09:40, Sami Mujawar wrote:

Hi Sami,

many thanks for your work on that!

Some virtual machine managers like kvmtool emulate the MC146818 RTC
controller in the MMIO space so that architectures that do not support
I/O Mapped I/O can use the RTC. This patch adds MMIO support to the
RTC controller driver.
Is there any chance you could read the MMIO base address from the DT? I
sent a kvmtool patch to add the missing DT node[1].
The compatible string would be "motorola,mc146818", with just the usual reg
property.
Maybe you could fall back to the current 0x70/0x71, if there is no DT node?

For a start, this low address (0x70/0x71) causes issues, so we probably need
to move this permanently.
But also we want to introduce a more flexible memory layout, so devices can
move depending on command line parameters.

It would be great if EDK-2 could cover that from the beginning, so that we
don't end up with compatibility issues.

Cheers,
Andre

[1] https://lists.cs.columbia.edu/pipermail/kvmarm/2020-May/040703.html

The PCD PcdRtcUseMmio has been added to select I/O or MMIO support.
If PcdRtcUseMmio is:
TRUE - Indicates the RTC port registers are in MMIO space.
FALSE - Indicates the RTC port registers are in I/O space.
Default is I/O space.

When MMIO support is selected (PcdRtcUseMmio == TRUE) the driver
maps
the MMIO region used by the RTC as runtime memory so that the RTC
registers are accessible post ExitBootServices.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---

Notes:
v2:
- Code review comments incorporated. [Sami]

v1:
- Add support to read/write from RTC registers using MMIO access
[Sami]
- Use wrapper functions for RtcRead/Write accessors [Leif]
Ref: https://edk2.groups.io/g/devel/topic/30915281#30695

PcAtChipsetPkg/PcAtChipsetPkg.dec | 8 ++
PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c | 117
++++++++++++++++--
PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h | 31
+++++
PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c |
130 +++++++++++++++++++-
PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntime
Dxe.inf | 8 ++
5 files changed, 280 insertions(+), 14 deletions(-)

diff --git a/PcAtChipsetPkg/PcAtChipsetPkg.dec
b/PcAtChipsetPkg/PcAtChipsetPkg.dec
index
88de5cceea593176c3a2425a5963b66b789f2b9e..76d0c7eda69bb505914ba904
e09c
89de170f69ae 100644
--- a/PcAtChipsetPkg/PcAtChipsetPkg.dec
+++ b/PcAtChipsetPkg/PcAtChipsetPkg.dec
@@ -6,6 +6,7 @@
#
# Copyright (c) 2009 - 2019, Intel Corporation. All rights
reserved.<BR> # Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+# Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -142,5 +143,12
@@ [PcdsFixedAtBuild, PcdsPatchableInModule]
# @Prompt RTC Update Timeout Value.

gPcAtChipsetPkgTokenSpaceGuid.PcdRealTimeClockUpdateTimeout|100000
|UIN
T32|0x00000020

+ ## Indicates the RTC port registers are in MMIO space, or in I/O space.
+ # Default is I/O space.<BR><BR>
+ # TRUE - RTC port registers are in MMIO space.<BR>
+ # FALSE - RTC port registers are in I/O space.<BR>
+ # @Prompt RTC port registers use MMIO.
+
+
gPcAtChipsetPkgTokenSpaceGuid.PcdRtcUseMmio|FALSE|BOOLEAN|0x0000
0021
+
[UserExtensions.TianoCore."ExtraFiles"]
PcAtChipsetPkgExtra.uni
diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
index
52af17941786ef81c3911512ee64551724e67209..df8dea83ab27bbba12351096d
1bf
d9ea31accb60 100644
--- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
+++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
@@ -3,6 +3,7 @@

Copyright (c) 2006 - 2018, Intel Corporation. All rights
reserved.<BR> Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.<BR>

SPDX-License-Identifier: BSD-2-Clause-Patent

@@ -10,6 +11,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent

#include "PcRtc.h"

+extern EFI_PHYSICAL_ADDRESS mRtcRegisterBase;
+
//
// Days of month.
//
@@ -21,6 +24,28 @@ UINTN mDayOfMonth[] = { 31, 29, 31, 30, 31, 30, 31,
31, 30, 31, 30, 31 };
CHAR16 mTimeZoneVariableName[] = L"RTC";

/**
+ A function pointer that evaluates to a function that reads the RTC
+ content through its registers either using IO or MMIO access.
+
+ @param Address Address offset of RTC. It is recommended to use
+ macros such as RTC_ADDRESS_SECONDS.
+
+ @return The data of UINT8 type read from RTC.
+**/
+RTC_READ RtcRead;
+
+/**
+ A function pointer that evaluates to a function that reads the RTC
+content
+ through its registers either using IO or MMIO access.
+
+ @param Address Address offset of RTC. It is recommended to use
+ macros such as RTC_ADDRESS_SECONDS.
+
+ @return The data of UINT8 type read from RTC.
+**/
+RTC_WRITE RtcWrite;
+
+/**
Compare the Hour, Minute and Second of the From time and the To time.

Only compare H/M/S in EFI_TIME and ignore other fields here.
@@ -54,41 +79,99 @@ IsWithinOneDay (
);

/**
- Read RTC content through its registers.
+ Read RTC content through its registers using IO access.

- @param Address Address offset of RTC. It is recommended to use
macros such as
- RTC_ADDRESS_SECONDS.
+ @param Address Address offset of RTC. It is recommended to use
+ macros such as RTC_ADDRESS_SECONDS.

@return The data of UINT8 type read from RTC.
**/
+STATIC
UINT8
-RtcRead (
+IoRtcRead (
IN UINT8 Address
)
{
- IoWrite8 (PcdGet8 (PcdRtcIndexRegister), (UINT8) (Address | (UINT8)
(IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80)));
+ IoWrite8 (
+ PcdGet8 (PcdRtcIndexRegister),
+ (UINT8)(Address | (UINT8)(IoRead8 (PcdGet8 (PcdRtcIndexRegister)) &
0x80))
+ );
return IoRead8 (PcdGet8 (PcdRtcTargetRegister)); }

/**
- Write RTC through its registers.
+ Write RTC through its registers using IO access.

- @param Address Address offset of RTC. It is recommended to use
macros such as
- RTC_ADDRESS_SECONDS.
- @param Data The content you want to write into RTC.
+ @param Address Address offset of RTC. It is recommended to use
+ macros such as RTC_ADDRESS_SECONDS.
+ @param Data The content you want to write into RTC.

**/
+STATIC
VOID
-RtcWrite (
+IoRtcWrite (
IN UINT8 Address,
IN UINT8 Data
)
{
- IoWrite8 (PcdGet8 (PcdRtcIndexRegister), (UINT8) (Address | (UINT8)
(IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80)));
+ IoWrite8 (
+ PcdGet8 (PcdRtcIndexRegister),
+ (UINT8)(Address | (UINT8)(IoRead8 (PcdGet8 (PcdRtcIndexRegister)) &
0x80))
+ );
IoWrite8 (PcdGet8 (PcdRtcTargetRegister), Data); }

/**
+ Read RTC content through its registers using MMIO access.
+
+ @param Address Address offset of RTC. It is recommended to use
+ macros such as RTC_ADDRESS_SECONDS.
+
+ @return The data of UINT8 type read from RTC.
+**/
+STATIC
+UINT8
+MmioRtcRead (
+ IN UINT8 Address
+ )
+{
+ MmioWrite8 (
+ mRtcRegisterBase,
+ (UINT8)(Address | (UINT8)(MmioRead8 (mRtcRegisterBase) & 0x80))
+ );
+ return MmioRead8 (
+ mRtcRegisterBase + (PcdGet8 (PcdRtcTargetRegister) -
+ PcdGet8 (PcdRtcIndexRegister))
+ );
+}
+
+/**
+ Write RTC through its registers using MMIO access.
+
+ @param Address Address offset of RTC. It is recommended to use
+ macros such as RTC_ADDRESS_SECONDS.
+ @param Data The content you want to write into RTC.
+
+**/
+STATIC
+VOID
+MmioRtcWrite (
+ IN UINT8 Address,
+ IN UINT8 Data
+ )
+{
+ MmioWrite8 (
+ mRtcRegisterBase,
+ (UINT8)(Address | (UINT8)(MmioRead8 (mRtcRegisterBase) & 0x80))
+ );
+ MmioWrite8 (
+ mRtcRegisterBase + (PcdGet8 (PcdRtcTargetRegister) -
+ PcdGet8 (PcdRtcIndexRegister)),
+ Data
+ );
+}
+
+/**
Initialize RTC.

@param Global For global use inside this module.
@@ -113,6 +196,18 @@ PcRtcInit (
BOOLEAN Pending;

//
+ // Initialize the RtcRead and RtcWrite functions // based on the
+ chosen IO/MMIO access.
+ //
+ if (FixedPcdGetBool (PcdRtcUseMmio)) {
+ RtcRead = MmioRtcRead;
+ RtcWrite = MmioRtcWrite;
+ } else {
+ RtcRead = IoRtcRead;
+ RtcWrite = IoRtcWrite;
+ }
+
+ //
// Acquire RTC Lock to make access to RTC atomic
//
if (!EfiAtRuntime ()) {
diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h
b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h
index
47293ce44c5a1f4792892892f7da40d7f0a5a001..e64dbbea48f7f0d2f317c65c2e
4b
93e7b1888efc 100644
--- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h
+++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h
@@ -3,6 +3,7 @@

Copyright (c) 2006 - 2018, Intel Corporation. All rights
reserved.<BR> Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.<BR>

SPDX-License-Identifier: BSD-2-Clause-Patent

@@ -371,4 +372,34 @@ PcRtcAcpiTableChangeCallback (
IN EFI_EVENT Event,
IN VOID *Context
);
+
+/**
+ Function pointer to Read RTC content through its registers.
+
+ @param Address Address offset of RTC. It is recommended to use
+ macros such as RTC_ADDRESS_SECONDS.
+
+ @return The data of UINT8 type read from RTC.
+**/
+typedef
+UINT8
+(EFIAPI *RTC_READ) (
+ IN UINT8 Address
+ );
+
+/**
+ Function pointer to Write RTC through its registers.
+
+ @param Address Address offset of RTC. It is recommended to use
+ macros such as RTC_ADDRESS_SECONDS.
+ @param Data The content you want to write into RTC.
+
+**/
+typedef
+VOID
+(EFIAPI *RTC_WRITE) (
+ IN UINT8 Address,
+ IN UINT8 Data
+ );
+
#endif
diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
index
ccda6331373bfe4069b0a59495b5e5cc731c8fc8..5d5dbeaf970ca8eb291c1e094f
d7
64d201f9071e 100644
--- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
+++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
@@ -2,16 +2,32 @@
Provides Set/Get time operations.

Copyright (c) 2006 - 2018, Intel Corporation. All rights
reserved.<BR>
+Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/

+#include <Library/DxeServicesTableLib.h>
#include "PcRtc.h"

PC_RTC_MODULE_GLOBALS mModuleGlobal;

EFI_HANDLE mHandle = NULL;

+STATIC EFI_EVENT mVirtualAddrChangeEvent;
+
+EFI_PHYSICAL_ADDRESS mRtcRegisterBase;
+
+//
+// Function pointer for the Rtc Read interface function //
+extern RTC_READ RtcRead;
+
+//
+// Function pointer for the Rtc Write interface function // extern
+RTC_WRITE RtcWrite;
+
/**
Returns the current time and date information, and the time-keeping
capabilities
of the hardware platform.
@@ -106,6 +122,33 @@ PcRtcEfiSetWakeupTime ( }

/**
+ Fixup internal data so that EFI can be called in virtual mode.
+ Call the passed in Child Notify event and convert any pointers in
+ lib to virtual mode.
+
+ @param[in] Event The Event that is being processed
+ @param[in] Context Event Context
+**/
+VOID
+EFIAPI
+LibRtcVirtualNotifyEvent (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ // Only needed if you are going to support the OS calling RTC
+functions in
+ // virtual mode. You will need to call EfiConvertPointer (). To
+convert any
+ // stored physical addresses to virtual address. After the OS
+transitions to
+ // calling in virtual mode, all future runtime calls will be made
+in virtual
+ // mode.
+ EfiConvertPointer (0x0, (VOID**)&mRtcRegisterBase);
+
+ // Convert the RtcRead and RtcWrite pointers for runtime use.
+ EfiConvertPointer (0x0, (VOID**)&RtcRead);
+ EfiConvertPointer (0x0, (VOID**)&RtcWrite); }
+
+/**
The user Entry Point for PcRTC module.

This is the entry point for PcRTC module. It installs the UEFI
runtime service @@ -125,12 +168,77 @@ InitializePcRtc (
IN EFI_SYSTEM_TABLE *SystemTable
)
{
- EFI_STATUS Status;
- EFI_EVENT Event;
+ EFI_STATUS Status;
+ EFI_EVENT Event;
+ EFI_PHYSICAL_ADDRESS RtcPageBase;

EfiInitializeLock (&mModuleGlobal.RtcLock, TPL_CALLBACK);
mModuleGlobal.CenturyRtcAddress = GetCenturyRtcAddress ();

+ if (FixedPcdGetBool (PcdRtcUseMmio)) {
+ mRtcRegisterBase = PcdGet8 (PcdRtcIndexRegister);
+ RtcPageBase = mRtcRegisterBase & ~(EFI_PAGE_SIZE - 1);
+
+ // Declare the controller as EFI_MEMORY_RUNTIME
+ Status = gDS->AddMemorySpace (
+ EfiGcdMemoryTypeMemoryMappedIo,
+ RtcPageBase,
+ EFI_PAGE_SIZE,
+ EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR, "Failed to add memory space. Status = %r\n",
+ Status
+ ));
+ return Status;
+ }
+
+ Status = gDS->AllocateMemorySpace (
+ EfiGcdAllocateAddress,
+ EfiGcdMemoryTypeMemoryMappedIo,
+ 0,
+ EFI_PAGE_SIZE,
+ &RtcPageBase,
+ ImageHandle,
+ NULL
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "Failed to allocate memory space. Status = %r\n",
+ Status
+ ));
+ gDS->RemoveMemorySpace (
+ RtcPageBase,
+ EFI_PAGE_SIZE
+ );
+ return Status;
+ }
+
+ Status = gDS->SetMemorySpaceAttributes (
+ RtcPageBase,
+ EFI_PAGE_SIZE,
+ EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "Failed to set memory attributes. Status = %r\n",
+ Status
+ ));
+ gDS->FreeMemorySpace (
+ RtcPageBase,
+ EFI_PAGE_SIZE
+ );
+ gDS->RemoveMemorySpace (
+ RtcPageBase,
+ EFI_PAGE_SIZE
+ );
+ return Status;
+ }
+ }
+
Status = PcRtcInit (&mModuleGlobal);
ASSERT_EFI_ERROR (Status);

@@ -165,7 +273,23 @@ InitializePcRtc (
NULL,
NULL
);
- ASSERT_EFI_ERROR (Status);
+ if (EFI_ERROR (Status)) {
+ ASSERT_EFI_ERROR (Status);
+ return Status;
+ }
+
+ if (FixedPcdGetBool (PcdRtcUseMmio)) {
+ // Register for the virtual address change event
+ Status = gBS->CreateEventEx (
+ EVT_NOTIFY_SIGNAL,
+ TPL_NOTIFY,
+ LibRtcVirtualNotifyEvent,
+ NULL,
+ &gEfiEventVirtualAddressChangeGuid,
+ &mVirtualAddrChangeEvent
+ );
+ ASSERT_EFI_ERROR (Status);
+ }

return Status;
}
diff --git
a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntim
eD
xe.inf
b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntim
eD
xe.inf index
c73ee98105e510f9e4e23c1a6c1e5c505325d2c9..3a373d11f8bfc7df0e4d00be8
b43
e90bfa06b192 100644
---
a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntim
eD
xe.inf
+++
b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRunt
+++ imeDxe.inf
@@ -6,6 +6,7 @@
#
# Copyright (c) 2006 - 2019, Intel Corporation. All rights
reserved.<BR> # Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+# Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -48,6 +49,7 @@
[LibraryClasses]
BaseLib
PcdLib
ReportStatusCodeLib
+ DxeServicesTableLib

[Protocols]
gEfiRealTimeClockArchProtocolGuid ## PRODUCES
@@ -61,10 +63,13 @@ [Guids]
## SOMETIMES_CONSUMES ## SystemTable
gEfiAcpiTableGuid

+ gEfiEventVirtualAddressChangeGuid
+
[FixedPcd]
gPcAtChipsetPkgTokenSpaceGuid.PcdInitialValueRtcRegisterA ##
CONSUMES
gPcAtChipsetPkgTokenSpaceGuid.PcdInitialValueRtcRegisterB ##
CONSUMES
gPcAtChipsetPkgTokenSpaceGuid.PcdInitialValueRtcRegisterD ##
CONSUMES
+ gPcAtChipsetPkgTokenSpaceGuid.PcdRtcUseMmio ##
CONSUMES

[Pcd]
gPcAtChipsetPkgTokenSpaceGuid.PcdRealTimeClockUpdateTimeout ##
CONSUMES
@@ -76,5 +81,8 @@ [Pcd]
[Depex]
gEfiVariableArchProtocolGuid AND gEfiVariableWriteArchProtocolGuid

+[Depex.common.DXE_RUNTIME_DRIVER]
+ gEfiCpuArchProtocolGuid
+
[UserExtensions.TianoCore."ExtraFiles"]
PcRtcExtra.uni


Re: Using debugger to debug UEFI application?

Lee Fisher
 

[...]I just found the Intel UDK debugger and hopping to use it [...]
I may be wrong, but I think Intel has put the Intel UDK debugger
solution as End-Of-Life. I'm not sure it's worth spending any time using
an end-of-life'd debugger solution. I think the main 2 alternatives to
the Intel UDK solution are: Intel System Studio (ISS), or Tianocore's
SourceLevelDebugPkg.

Over time, ISS has only been available if you registered, or not even as
freeware, so was only an option for some. But currently ISS is
freely-available without registration. ISS is not end-of-life, and I
presume they have a tech support team to help paying users with
questions, if that helps with your current situation.

There is at least one gdb-server project for UEFI outside the Intel UDK
solution and outside Tianocore:

https://github.com/night199uk/edk2-gdb-server

I think there is a second one, but I can't find it at the moment. I wish
Intel would open-source the Intel UDK debugger code, including it's
gdb-server, instead of just killing it off. Perhaps the Tiancore
community would make use of some of the code. Anything to improve UEFI
debugging is a worthy effort. 

Servers aside, I really I wish there there was an lldb.efi and a gdb.efi...


Re: [RFC edk2 v1 1/1] MdeModulePkg/Variable: Move FindVariable after AutoUpdateLangVariable

Guomin Jiang
 

Hi Huangming,

I will clarify it when I am free, please be patient.

If it is urgent for you, I suggest that you can use your patch temporarily.

Any other thing, I check your patch in the mail, it seem that not any change in code, can you double confirm it?

Best Regards

-----Original Message-----
From: Ming Huang <huangming23@huawei.com>
Sent: Tuesday, May 26, 2020 2:08 PM
To: Jiang, Guomin <guomin.jiang@intel.com>; devel@edk2.groups.io; Wang,
Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao,
Liming <liming.gao@intel.com>
Cc: lidongzhan@huawei.com; songdongkuang@huawei.com;
wanghuiqiang@huawei.com; qiuliangen@huawei.com;
shenlimei@huawei.com
Subject: Re: [edk2-devel] [RFC edk2 v1 1/1] MdeModulePkg/Variable: Move
FindVariable after AutoUpdateLangVariable



在 2020/5/26 8:39, Jiang, Guomin 写道:
Hi Huangming,

I am taking the bugzilla and I am sorry that I haven't provide you with
productive comment.

I am still busy until August.

I just want to know that:
1. Have you verified that the symptom will disappear after invoked
FindVariable() function?

Yes, the symptom will disappeare after add this patch.

2. Is it your suggestion that the FindVariable() need to be invoked but you
have no idea that how to fix it?

This patch can fix this issue, and I guess this issue was resulted by adding
AutoUpdateLangVariable feature.
I hope this patch can be merged to edk2 master.

Thanks
Ming


Best Regards
Guomin
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ming
Huang
Sent: Monday, May 25, 2020 7:34 PM
To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Wu,
Hao A <hao.a.wu@intel.com>; Gao, Liming <liming.gao@intel.com>
Cc: lidongzhan@huawei.com; huangming23@huawei.com;
songdongkuang@huawei.com; wanghuiqiang@huawei.com;
qiuliangen@huawei.com; shenlimei@huawei.com
Subject: [edk2-devel] [RFC edk2 v1 1/1] MdeModulePkg/Variable: Move
FindVariable after AutoUpdateLangVariable

When occur reclaim in AutoUpdateLangVariable(), the CurrPtr of
Variable is invalid. The State will be update with wrong position
after UpdateVariable in this situation and two valid PlatformLang or Lang
variables will exist.
BmForEachVariable() will enter endless loop while exist two valid
PlatformLang variables. So FindVariable() should be invoked atfer
AutoUpdateLangVariable().

https://bugzilla.tianocore.org/show_bug.cgi?id=2667

Signed-off-by: Ming Huang <huangming23@huawei.com>
---
MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 26
++++++++++----------
1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index 1e71fc6..0cec981 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -2741,6 +2741,19 @@ VariableServiceSetVariable (
mVariableModuleGlobal->NonVolatileLastVariableOffset = (UINTN)
NextVariable - (UINTN) Point;
}

+ if (!FeaturePcdGet (PcdUefiVariableDefaultLangDeprecate)) {
+ //
+ // Hook the operation of setting PlatformLangCodes/PlatformLang
+ and
LangCodes/Lang.
+ //
+ Status = AutoUpdateLangVariable (VariableName, Data, DataSize);
+ if (EFI_ERROR (Status)) {
+ //
+ // The auto update operation failed, directly return to avoid
inconsistency between PlatformLang and Lang.
+ //
+ goto Done;
+ }
+ }
+
//
// Check whether the input variable is already existed.
//
@@ -2763,19 +2776,6 @@ VariableServiceSetVariable (
}
}

- if (!FeaturePcdGet (PcdUefiVariableDefaultLangDeprecate)) {
- //
- // Hook the operation of setting PlatformLangCodes/PlatformLang and
LangCodes/Lang.
- //
- Status = AutoUpdateLangVariable (VariableName, Data, DataSize);
- if (EFI_ERROR (Status)) {
- //
- // The auto update operation failed, directly return to avoid
inconsistency
between PlatformLang and Lang.
- //
- goto Done;
- }
- }
-
if (mVariableModuleGlobal->VariableGlobal.AuthSupport) {
Status = AuthVariableLibProcessVariable (VariableName,
VendorGuid, Data, DataSize, Attributes);
} else {
--
2.8.1




Re: Updating to latest EDK2 and now get NMAKE: fatal error U1073: Don't know how to make.. on some items?

Guomin Jiang
 

Hi David,

 

I saw the below sentence from you.

I got the edk2-libc repo from GIT.  The edk2 repo I still use svn update to update it.  Using tortoise tools on windows.

 

As I know, we not maintain the edk2 in svn from long ago, you can see the svn last update data to confirm it.

I suggest that you use the edk2 repo from GIT and try again.

 

Best Regards

From: David F. <df7729@...>
Sent: Tuesday, May 26, 2020 3:18 PM
To: Jiang, Guomin <guomin.jiang@...>; devel@edk2.groups.io
Subject: Re: [edk2-devel] Updating to latest EDK2 and now get NMAKE: fatal error U1073: Don't know how to make.. on some items?

 

Let me start by what I did to get the build working with the old BaseTools.   I renamed BaseTools and put back the prior BaseTools from Nov 29 2018 (build.exe date).  It started building out of the gate, then hit nasm.inc not found, so digging around end up comparing old and new for things and brought over new NASM items from new GenMake.py to the old one.  Tried again, same thing, ended up fixing by bringing over the new build_rule.txt.  Now everything is building fine using VS2008 but the old BaseTools.

I can try using VS2017, I though I was staying with VS2008 is VS2017 added more behind the scenes stuff that I have to add replacements for (already have what I need for VS2008).  But I really don't remember. I'll do that when I get a chance once I catch up on things. 


Okay, now on to Guomin message,  Those messages are saying it's not in the .inf, which looking in the .inf, they are not.  Although the headers themselves exist.  I got the edk2-libc repo from GIT.  The edk2 repo I still use svn update to update it.  Using tortoise tools on windows.


Thanks.