date:20211124

Hi Liming,

I agree it is both low risk and prevents an ASSERT() condition that should 
never be triggered.

I approve for edk2-stable202111

Mike

From: devel@edk2.groups.io  On Behalf Of gaoliming
Sent: Wednesday, November 24, 2021 5:53 PM
To: devel@edk2.groups.io; Wu, Jiaxin ; Anbazhagan, 
Baraneedharan 
Cc: maciej.rab...@linux.intel.com; Fu, Siyuan ; 
l...@nuviainc.com; 'Andrew Fish' ; Kinney, Michael D 

Subject: 回复: [edk2-devel] [Patch edk2-stable202111] NetworkPkg: Fix invalid 
pointer for DNS response token on error

I suggest to merge this fix for the stable tag 202111. This fix is low risk.

Thanks
Liming
发件人: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>> 代表 Wu, Jiaxin
发送时间: 2021年11月25日 9:37
收件人: Anbazhagan, Baraneedharan mailto:anbazha...@hp.com>>; 
devel@edk2.groups.io
抄送: maciej.rab...@linux.intel.com; Fu, 
Siyuan mailto:siyuan...@intel.com>>
主题: Re: [edk2-devel] NetworkPkg: Fix invalid pointer for DNS response token on 
error

Reviewed-by: Wu Jiaxin mailto:jiaxin...@intel.com>>



From: Anbazhagan, Baraneedharan mailto:anbazha...@hp.com>>
Sent: Thursday, November 25, 2021 9:28 AM
To: devel@edk2.groups.io
Cc: maciej.rab...@linux.intel.com; Wu, 
Jiaxin mailto:jiaxin...@intel.com>>; Fu, Siyuan 
mailto:siyuan...@intel.com>>
Subject: RE: NetworkPkg: Fix invalid pointer for DNS response token on error

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

Token->RspData.H2AData is de-allocated on error but it is not
set to NULL. HTTP module attempts to free again and cause assert.

Signed-off-by: Baraneedharan Anbazhagan 
anbazha...@hp.com
---
NetworkPkg/DnsDxe/DnsImpl.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/NetworkPkg/DnsDxe/DnsImpl.c b/NetworkPkg/DnsDxe/DnsImpl.c
index 2edcb280ac..78a56f2b56 100644
--- a/NetworkPkg/DnsDxe/DnsImpl.c
+++ b/NetworkPkg/DnsDxe/DnsImpl.c
@@ -1700,6 +1700,7 @@ ON_EXIT:
   }

   FreePool (Dns4TokenEntry->Token->RspData.H2AData);
+  Dns4TokenEntry->Token->RspData.H2AData = NULL;
 }
   }
 }
@@ -1731,6 +1732,7 @@ ON_EXIT:
   }

   FreePool (Dns6TokenEntry->Token->RspData.H2AData);
+  Dns6TokenEntry->Token->RspData.H2AData = NULL;
 }
   }
 }
--
2.33.0.windows.2




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

[edk2-devel] Uncrustify Conversion Detailed Plan and Extended Hard Freeze Update #1

2021-11-24 Thread Anbazhagan, Baraneedharan via groups.io

Hello,

Michael Kubacki and I have prepared the patches required to apply the
uncrustify changes and enable EDK II CI to check all submitted
patches have been run through uncrustify.

We have verified through the CompareBuild GitHub Action that the
format changes performed by uncrustify have no functional changes.
All of the OBJ, LIB, DLL, EFI, FFS, FV, and FD files match 100%
across 70 VS2019/GCC5 builds of all package/platform DSC files in
the edk2 repo.

The hard freeze will be extended after the edk2-stable202111 tag until
all uncrustify related changes are committed. We do not expect this
to take more than a few days. Do not push any PRs until the hard
freeze is lifted.

The changes are broken up into 7 patch series/PRs. The PRs are ordered
so they can be submitted using the normal submission process and EDK II
CI will pass for each one. Details are listed below.

Uncrustify 73.0.3 for EDK II
=
* Sources: https://dev.azure.com/projectmu/_git/Uncrustify
* Documentation:
https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme
* Download:
https://dev.azure.com/projectmu/Uncrustify/_packaging?_a=package&feed=mu_uncrustify&package=mu-uncrustify-release&protocolType=NuGet&version=73.0.3

Installing Uncrustify
==
The Uncrustify tool is installed automatically when the Pytools
environment is used and the stuart* commands are run to complete the
environment setup. Please see:

https://github.com/tianocore/edk2/tree/master/.pytool#running-ci-locally

Uncrustify can also be installed from the download page listed above
or built from sources from the source link above.

The Documentation link provides instruction on how to run uncrustify from
the command line or install as a Visual Studio Code plugin. The main
uncrustify documentation also describes how to integrate with a few other
editors.

We have also discussed a client side githook. That effort has not started.
Let us know if that is a feature you would find useful.

Developer impact for new code reviews
==
Once the uncrustify checker is active in EDK II CI, developers must
make sure their patches are run through the uncrustify tool before
sending the patches for review.

Developers must install and run uncrustify against changes files before
sending patch review emails or submitting PR for EDK II CI. If EDK II CI
detects and differences in soure formatting, then EDK II CI will fail
and the developer must run uncrustify and resubmit the patches.

Developer impact to patch series/PRs reviewed during edk2-stable201121
soft/hard freeze
===
Developers must rebase their changes after the uncrustify source changes are
committed. The branch with a preview of the uncrustify changes can be used
to start this rebase work.

https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyChanges_V5

Impacts to tracing history across the uncrusity changes
===
<>
<>

Order of PRs to apply during extended hard freeze
==
1) Update EmulatorPkg Win Host [BuildOptions] MSFT CC_FLAGS to not force debug
information
* https://bugzilla.tianocore.org/show_bug.cgi?id=3747
*
https://github.com/mdkinney/edk2/tree/Bug_3747_EmulatorPkg_WinHost_ReproducibleBuild
* https://github.com/tianocore/edk2/pull/2215
* Required for EmulatorPkg to pass CompareBuild for VS2019 IA32/X64 builds.
* Status: Review complete

2) EccCheck should not revert staged and local changes
* https://bugzilla.tianocore.org/show_bug.cgi?id=2986
* https://github.com/mdkinney/edk2/tree/Bug_2986_EccCheckRemoveGitRevert_V2
* https://github.com/tianocore/edk2/pull/2216
* Required for EDK II CI to complete in a reasonable period of time when
processing the 4000+ source file style changes made by uncrustify.
* Also fixes critical bugs that can potentially corrupt git state when
EccCheck is run locally.
* Status:
Reviewed-by: Liming Gao mailto:gaolim...@byosoft.com.cn
Waiting for Review/Ack from .pytool maintainer.

3) Update pytool LicenseCheck plugin to use temp directory for diff output file
* https://bugzilla.tianocore.org/show_bug.cgi?id=3746
*
https://github.com/mdkinney/edk2/tree/Bug_3746_LicenseCheckUseDiffOutputFile_V2
* https://github.com/tianocore/edk2/pull/2217
* Required to reduce EDK II CI build times.
* Status:
Acked-by: Michael Kubacki mailto:michael.kuba...@microsoft.com
Reviewed-by: Liming Gao mailto:gaolim...@byosoft.com.cn
Waiting for Review/Ack from .pytool maintainer.

4) Update Package YAML to ignore specific ECC files/errors
* https://bugzilla.tianocore.org/show_bug.cgi?id=3749
* https://github.com/mdkinney/edk2/tree/Bug_3749_EccCheckIgnoreFilesErrors
* h

回复: [edk2-devel] [Patch edk2-stable202111] NetworkPkg: Fix invalid pointer for DNS response token on error

2021-11-24 Thread gaoliming

I suggest to merge this fix for the stable tag 202111. This fix is low risk.


 

Thanks

Liming

发件人: devel@edk2.groups.io  代表 Wu, Jiaxin
发送时间: 2021年11月25日 9:37
收件人: Anbazhagan, Baraneedharan ; devel@edk2.groups.io
抄送: maciej.rab...@linux.intel.com; Fu, Siyuan 
主题: Re: [edk2-devel] NetworkPkg: Fix invalid pointer for DNS response
token on error

 

Reviewed-by: Wu Jiaxin mailto:jiaxin...@intel.com> >

 

 

 

From: Anbazhagan, Baraneedharan mailto:anbazha...@hp.com> > 
Sent: Thursday, November 25, 2021 9:28 AM
To: devel@edk2.groups.io  
Cc: maciej.rab...@linux.intel.com  ;
Wu, Jiaxin mailto:jiaxin...@intel.com> >; Fu, Siyuan
mailto:siyuan...@intel.com> >
Subject: RE: NetworkPkg: Fix invalid pointer for DNS response token on error

 

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

 

Token->RspData.H2AData is de-allocated on error but it is not

set to NULL. HTTP module attempts to free again and cause assert.

 

Signed-off-by: Baraneedharan Anbazhagan anbazha...@hp.com
 

---

NetworkPkg/DnsDxe/DnsImpl.c | 2 ++

1 file changed, 2 insertions(+)

 

diff --git a/NetworkPkg/DnsDxe/DnsImpl.c b/NetworkPkg/DnsDxe/DnsImpl.c

index 2edcb280ac..78a56f2b56 100644

--- a/NetworkPkg/DnsDxe/DnsImpl.c

+++ b/NetworkPkg/DnsDxe/DnsImpl.c

@@ -1700,6 +1700,7 @@ ON_EXIT:

   }

 

   FreePool (Dns4TokenEntry->Token->RspData.H2AData);

+  Dns4TokenEntry->Token->RspData.H2AData = NULL;

 }

   }

 }

@@ -1731,6 +1732,7 @@ ON_EXIT:

   }

 

   FreePool (Dns6TokenEntry->Token->RspData.H2AData);

+  Dns6TokenEntry->Token->RspData.H2AData = NULL;

 }

   }

 }

-- 

2.33.0.windows.2

 





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

Re: [edk2-devel] NetworkPkg: Fix invalid pointer for DNS response token on error

2021-11-24 Thread Wu, Jiaxin

Reviewed-by: Wu Jiaxin 



From: Anbazhagan, Baraneedharan 
Sent: Thursday, November 25, 2021 9:28 AM
To: devel@edk2.groups.io
Cc: maciej.rab...@linux.intel.com; Wu, Jiaxin ; Fu, Siyuan 

Subject: RE: NetworkPkg: Fix invalid pointer for DNS response token on error

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

Token->RspData.H2AData is de-allocated on error but it is not
set to NULL. HTTP module attempts to free again and cause assert.

Signed-off-by: Baraneedharan Anbazhagan 
anbazha...@hp.com
---
NetworkPkg/DnsDxe/DnsImpl.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/NetworkPkg/DnsDxe/DnsImpl.c b/NetworkPkg/DnsDxe/DnsImpl.c
index 2edcb280ac..78a56f2b56 100644
--- a/NetworkPkg/DnsDxe/DnsImpl.c
+++ b/NetworkPkg/DnsDxe/DnsImpl.c
@@ -1700,6 +1700,7 @@ ON_EXIT:
   }

   FreePool (Dns4TokenEntry->Token->RspData.H2AData);
+  Dns4TokenEntry->Token->RspData.H2AData = NULL;
 }
   }
 }
@@ -1731,6 +1732,7 @@ ON_EXIT:
   }

   FreePool (Dns6TokenEntry->Token->RspData.H2AData);
+  Dns6TokenEntry->Token->RspData.H2AData = NULL;
 }
   }
 }
--
2.33.0.windows.2



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

Re: [edk2-devel] NetworkPkg: Fix invalid pointer for DNS response token on error

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

Token->RspData.H2AData is de-allocated on error but it is not
set to NULL. HTTP module attempts to free again and cause assert.

Signed-off-by: Baraneedharan Anbazhagan 
anbazha...@hp.com
---
NetworkPkg/DnsDxe/DnsImpl.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/NetworkPkg/DnsDxe/DnsImpl.c b/NetworkPkg/DnsDxe/DnsImpl.c
index 2edcb280ac..78a56f2b56 100644
--- a/NetworkPkg/DnsDxe/DnsImpl.c
+++ b/NetworkPkg/DnsDxe/DnsImpl.c
@@ -1700,6 +1700,7 @@ ON_EXIT:
   }

   FreePool (Dns4TokenEntry->Token->RspData.H2AData);
+  Dns4TokenEntry->Token->RspData.H2AData = NULL;
 }
   }
 }
@@ -1731,6 +1732,7 @@ ON_EXIT:
   }

   FreePool (Dns6TokenEntry->Token->RspData.H2AData);
+  Dns6TokenEntry->Token->RspData.H2AData = NULL;
 }
   }
 }
--
2.33.0.windows.2



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

[edk2-devel] [PATCH v5 1/1] .pytool/Plugin/UncrustifyCheck: Add Uncrustify CI plugin

From: Michael Kubacki 

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

Adds a new CI plugin for Uncrustify. This is used to check
coding standard compliance of source code to the EDK II C Coding
Standards Specification.

An external dependency is added in the plugin directory to retrieve
the Uncrustify executable. Currently, the executable is from an edk2
fork of the application host in this repository:

https://dev.azure.com/projectmu/Uncrustify/

Note that the Uncrustify application outputs the commit ID that the
application was built from when given the --version parameter.

This ID can be mapped to the version specified in the Uncrustify CI
plugin external dependency file (uncrustify_ext_dep.yaml) such as
73.0.3 by visiting the Uncrustify edk2 fork release pipeline page
which associates the NuGet package version with the commit ID it was
built from:

https://dev.azure.com/projectmu/Uncrustify/_build

The default Uncrustify configuration files are added in the plugin
directory. Additional details are in the Readme.md file added in
the Uncrustify plugin directory.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Sean Brogan 
Cc: Bret Barkelew 
Signed-off-by: Michael Kubacki 
---

Notes:
V5 changes:

1. Disable (comment out the setting in the Uncrustify config file)
   for file and function header templates due to the large number
   of failures that will take time to resolve.

   These settings can be enabled in the future after packages are
   updated to not fail the check.

V4 changes:

1. Updated commit subject to use the new plugin name.
2. Enabled file and function header templates to be placed in
   files missing a file or function header (comment block).
3. Added support to detect whether a file header or function
   header is missing in a file by searching files modified by
   Uncrustify for the template file text.
4. Updated the plugin name displayed in some log output messages.

V3 changes:

1. Renamed the CI plugin to "UncrustifyCheck" from "Uncrustify"
   to follow similar naming conventions used in other plugins
   that check files
2. Added a clarifying paragraph to the beginning of the Uncrustify
   CI plugin Readme.md file that the plugin is enabled by default
   and test results can be ignored by enabling "AuditOnly" mode.
3. Added instructions in the commit message on how to map the
   version reported by the Uncrustify application (--version) to
   the version specified in the CI plugin external dependency
   YAML file.

V2 changes:

1. Changed plugin temp directory to Build/.pytool/Plugin/Uncrustify
2. Added TCBZ REF in patch commit message

 .pytool/Plugin/UncrustifyCheck/Readme.md   | 120 
 .pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py  | 617 

 .pytool/Plugin/UncrustifyCheck/default_file_header.txt |   9 +
 .pytool/Plugin/UncrustifyCheck/default_function_header.txt |  15 +
 .pytool/Plugin/UncrustifyCheck/uncrustify.cfg  | 462 
+++
 .pytool/Plugin/UncrustifyCheck/uncrustify_ext_dep.yaml |  16 +
 .pytool/Plugin/UncrustifyCheck/uncrustify_plug_in.yaml |  11 +
 .pytool/Readme.md  |   4 +
 8 files changed, 1254 insertions(+)

diff --git a/.pytool/Plugin/UncrustifyCheck/Readme.md 
b/.pytool/Plugin/UncrustifyCheck/Readme.md
new file mode 100644
index ..bb263bcc87d7
--- /dev/null
+++ b/.pytool/Plugin/UncrustifyCheck/Readme.md
@@ -0,0 +1,120 @@
+# UncrustifyCheck Plugin
+
+This CiBuildPlugin scans all the files in a given package and checks for 
coding standard compliance issues.
+
+This plugin is enabled by default. If a package would like to prevent the 
plugin from reporting errors, it can do
+so by enabling [`AuditOnly`](#auditonly) mode.
+
+This plugin requires the directory containing the Uncrustify executable that 
should be used for this plugin to
+be specified in an environment variable named `UNCRUSTIFY_CI_PATH`. This 
unique variable name is used to avoid confusion
+with other paths to Uncrustify which might not be the expected build for use 
by this plugin.
+
+By default, an Uncrustify configuration file named "uncrustify.cfg" located in 
the same directory as the plugin is
+used. The value can be overridden to a package-specific path with the 
`ConfigFilePath` configuration file option.
+
+* Uncrustify source code and documentation: 
https://github.com/uncrustify/uncrustify
+* Project Mu Uncrustify fork source code and documentation: 
https://dev.azure.com/projectmu/Uncrustify
+
+## Files Checked in a Package
+
+By default, this plugin will discover all files in the package with the 
following default paths:
+
+```python
+[
+# C source
+"*.c",
+"*.h"
+]
+```
+
+From this list of files, any files ignored by Git or residing in a Git 
submodule will be removed. If Git is not
+found, submodules are n

Re: [edk2-devel] [PATCH v4 1/1] .pytool/Plugin/UncrustifyCheck: Add Uncrustify CI plugin


Hi Mike,

How about I quickly send a v5 with the lines commented out and follow up 
with a patch to uncomment them that we can review but merge when ready?


Thanks,
Michael

On 11/24/2021 7:16 PM, Michael D Kinney wrote:

The 2nd option I listed won't work.  That will fail UncrustifyCheck as well 
because EDK II CI
will insert missing file/function headers and the files will not match.

We will need to comment out these 2 lines and submit an extra commit to 
re-enable.

Mike


-Original Message-
From: Kinney, Michael D 
Sent: Wednesday, November 24, 2021 4:08 PM
To: devel@edk2.groups.io; mikub...@linux.microsoft.com; Kinney, Michael D 

Cc: Liming Gao ; Sean Brogan 
; Bret Barkelew

Subject: RE: [edk2-devel] [PATCH v4 1/1] .pytool/Plugin/UncrustifyCheck: Add 
Uncrustify CI plugin

Hi Michael,

Unfortunately, enabling the templates in the uncrustify.cfg files modifies 340 
files with missing
file/function headers that will now fail UncrustifyCheck.

 cmt_insert_file_header  = default_file_header.txt
 cmt_insert_func_header  = default_function_header.txt

Perhaps we should leave this feature off to get the uncrustify format changes 
committed and
enable this feature immediately after so patch reviews after that will slowly 
fix these missing
file/function headers.

Or I can comment these two lines out when I generate the final version of the 
patch series so
we don’t have to do an extra commit to re-enable.

Mike


-Original Message-
From: Kinney, Michael D 
Sent: Wednesday, November 24, 2021 3:06 PM
To: devel@edk2.groups.io; mikub...@linux.microsoft.com; Kinney, Michael D 

Cc: Liming Gao ; Sean Brogan 
; Bret Barkelew

Subject: RE: [edk2-devel] [PATCH v4 1/1] .pytool/Plugin/UncrustifyCheck: Add 
Uncrustify CI plugin

Thanks.  That makes sense.

Mike


-Original Message-
From: devel@edk2.groups.io  On Behalf Of Michael Kubacki
Sent: Wednesday, November 24, 2021 2:55 PM
To: devel@edk2.groups.io; Kinney, Michael D 
Cc: Liming Gao ; Sean Brogan 
; Bret Barkelew

Subject: Re: [edk2-devel] [PATCH v4 1/1] .pytool/Plugin/UncrustifyCheck: Add 
Uncrustify CI plugin

Yes. Those were redundant and detected by the Python configparser module
when I used it to parse the config file.

Thanks,
Michael

On 11/24/2021 5:45 PM, Michael D Kinney wrote:

Hi Michael,

I see a couple settings deleted from the uncrustify.cfg.  Was that in purpose?  
Does it change the format?


-nl_func_call_start_multi_line   = true  # Whether to add a newline after 
'(' in a function call if '(' and ')'

are

-# in different lines.


-nl_func_call_args_multi_line = true
-nl_func_call_args_multi_line_ignore_closures = false


Thanks,

Mike


-Original Message-
From: mikub...@linux.microsoft.com 
Sent: Wednesday, November 24, 2021 1:42 PM
To: devel@edk2.groups.io
Cc: Kinney, Michael D ; Liming Gao 
; Sean Brogan
; Bret Barkelew 
Subject: [PATCH v4 1/1] .pytool/Plugin/UncrustifyCheck: Add Uncrustify CI plugin

From: Michael Kubacki 

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

Adds a new CI plugin for Uncrustify. This is used to check
coding standard compliance of source code to the EDK II C Coding
Standards Specification.

An external dependency is added in the plugin directory to retrieve
the Uncrustify executable. Currently, the executable is from an edk2
fork of the application host in this repository:

https://dev.azure.com/projectmu/Uncrustify/

Note that the Uncrustify application outputs the commit ID that the
application was built from when given the --version parameter.

This ID can be mapped to the version specified in the Uncrustify CI
plugin external dependency file (uncrustify_ext_dep.yaml) such as
73.0.3 by visiting the Uncrustify edk2 fork release pipeline page
which associates the NuGet package version with the commit ID it was
built from:

https://dev.azure.com/projectmu/Uncrustify/_build

The default Uncrustify configuration files are added in the plugin
directory. Additional details are in the Readme.md file added in
the Uncrustify plugin directory.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Sean Brogan 
Cc: Bret Barkelew 
Signed-off-by: Michael Kubacki 
---

Notes:
  V4 changes:

  1. Updated commit subject to use the new plugin name.
  2. Enabled file and function header templates to be placed in
 files missing a file or function header (comment block).
  3. Added support to detect whether a file header or function
 header is missing in a file by searching files modified by
 Uncrustify for the template file text.
  4. Updated the plugin name displayed in some log output messages.

  V3 changes:

  1. Renamed the CI plugin to "UncrustifyCheck" from "Uncrustify"
 to follow similar naming conventions used in other plugins
 that check files
  2. Added a clarifying paragraph to the beginning of the Uncrustify
 CI plugin

Re: [edk2-devel] [PATCH v4 1/1] .pytool/Plugin/UncrustifyCheck: Add Uncrustify CI plugin

The 2nd option I listed won't work.  That will fail UncrustifyCheck as well 
because EDK II CI
will insert missing file/function headers and the files will not match.

We will need to comment out these 2 lines and submit an extra commit to 
re-enable.

Mike

> -Original Message-
> From: Kinney, Michael D 
> Sent: Wednesday, November 24, 2021 4:08 PM
> To: devel@edk2.groups.io; mikub...@linux.microsoft.com; Kinney, Michael D 
> 
> Cc: Liming Gao ; Sean Brogan 
> ; Bret Barkelew
> 
> Subject: RE: [edk2-devel] [PATCH v4 1/1] .pytool/Plugin/UncrustifyCheck: Add 
> Uncrustify CI plugin
> 
> Hi Michael,
> 
> Unfortunately, enabling the templates in the uncrustify.cfg files modifies 
> 340 files with missing
> file/function headers that will now fail UncrustifyCheck.
> 
> cmt_insert_file_header  = default_file_header.txt
> cmt_insert_func_header  = default_function_header.txt
> 
> Perhaps we should leave this feature off to get the uncrustify format changes 
> committed and
> enable this feature immediately after so patch reviews after that will slowly 
> fix these missing
> file/function headers.
> 
> Or I can comment these two lines out when I generate the final version of the 
> patch series so
> we don’t have to do an extra commit to re-enable.
> 
> Mike
> 
> > -Original Message-
> > From: Kinney, Michael D 
> > Sent: Wednesday, November 24, 2021 3:06 PM
> > To: devel@edk2.groups.io; mikub...@linux.microsoft.com; Kinney, Michael D 
> > 
> > Cc: Liming Gao ; Sean Brogan 
> > ; Bret Barkelew
> > 
> > Subject: RE: [edk2-devel] [PATCH v4 1/1] .pytool/Plugin/UncrustifyCheck: 
> > Add Uncrustify CI plugin
> >
> > Thanks.  That makes sense.
> >
> > Mike
> >
> > > -Original Message-
> > > From: devel@edk2.groups.io  On Behalf Of Michael 
> > > Kubacki
> > > Sent: Wednesday, November 24, 2021 2:55 PM
> > > To: devel@edk2.groups.io; Kinney, Michael D 
> > > Cc: Liming Gao ; Sean Brogan 
> > > ; Bret Barkelew
> > > 
> > > Subject: Re: [edk2-devel] [PATCH v4 1/1] .pytool/Plugin/UncrustifyCheck: 
> > > Add Uncrustify CI plugin
> > >
> > > Yes. Those were redundant and detected by the Python configparser module
> > > when I used it to parse the config file.
> > >
> > > Thanks,
> > > Michael
> > >
> > > On 11/24/2021 5:45 PM, Michael D Kinney wrote:
> > > > Hi Michael,
> > > >
> > > > I see a couple settings deleted from the uncrustify.cfg.  Was that in 
> > > > purpose?  Does it change the format?
> > > >
> > > >
> > > > -nl_func_call_start_multi_line   = true  # Whether to add a newline 
> > > > after '(' in a function call if '(' and ')'
> > are
> > > > -# in different lines.
> > > >
> > > >
> > > > -nl_func_call_args_multi_line = true
> > > > -nl_func_call_args_multi_line_ignore_closures = false
> > > >
> > > >
> > > > Thanks,
> > > >
> > > > Mike
> > > >
> > > >> -Original Message-
> > > >> From: mikub...@linux.microsoft.com 
> > > >> Sent: Wednesday, November 24, 2021 1:42 PM
> > > >> To: devel@edk2.groups.io
> > > >> Cc: Kinney, Michael D ; Liming Gao 
> > > >> ; Sean Brogan
> > > >> ; Bret Barkelew 
> > > >> 
> > > >> Subject: [PATCH v4 1/1] .pytool/Plugin/UncrustifyCheck: Add Uncrustify 
> > > >> CI plugin
> > > >>
> > > >> From: Michael Kubacki 
> > > >>
> > > >> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3748
> > > >>
> > > >> Adds a new CI plugin for Uncrustify. This is used to check
> > > >> coding standard compliance of source code to the EDK II C Coding
> > > >> Standards Specification.
> > > >>
> > > >> An external dependency is added in the plugin directory to retrieve
> > > >> the Uncrustify executable. Currently, the executable is from an edk2
> > > >> fork of the application host in this repository:
> > > >>
> > > >> https://dev.azure.com/projectmu/Uncrustify/
> > > >>
> > > >> Note that the Uncrustify application outputs the commit ID that the
> > > >> application was built from when given the --version parameter.
> > > >>
> > > >> This ID can be mapped to the version specified in the Uncrustify CI
> > > >> plugin external dependency file (uncrustify_ext_dep.yaml) such as
> > > >> 73.0.3 by visiting the Uncrustify edk2 fork release pipeline page
> > > >> which associates the NuGet package version with the commit ID it was
> > > >> built from:
> > > >>
> > > >> https://dev.azure.com/projectmu/Uncrustify/_build
> > > >>
> > > >> The default Uncrustify configuration files are added in the plugin
> > > >> directory. Additional details are in the Readme.md file added in
> > > >> the Uncrustify plugin directory.
> > > >>
> > > >> Cc: Michael D Kinney 
> > > >> Cc: Liming Gao 
> > > >> Cc: Sean Brogan 
> > > >> Cc: Bret Barkelew 
> > > >> Signed-off-by: Michael Kubacki 
> > > >> ---
> > > >>
> > > >> Notes:
> > > >>  V4 changes:
> > > >>
> > > >>  1. Updated commit subject to use the new plugin name.
> > > >>  2. Enabled file and function header templates to be placed in
>

Re: [edk2-devel] [Patch 1/1] EmulatorPkg/Win/Host: Update CC_FLAGS

2021-11-24 Thread Ni, Ray

Reviewed-by: Ray Ni 

> -Original Message-
> From: Kinney, Michael D 
> Sent: Wednesday, November 24, 2021 1:26 AM
> To: devel@edk2.groups.io
> Cc: Andrew Fish ; Ni, Ray ; Michael 
> Kubacki 
> Subject: [Patch 1/1] EmulatorPkg/Win/Host: Update CC_FLAGS
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3747
> 
> * Remove deprecated option /Gm that generates a warning.
> * Remove /Zi and use '=' instead of '== to let DEBUG/RELEASE/NOOPT
>   profile from tools_def.txt enable debug information
> * Remove /Gs8192 option that is overriding the larger setting of
>   /GS32768 from tools_def.txt that generates a warning.
> 
> Cc: Andrew Fish 
> Cc: Ray Ni 
> Cc: Michael Kubacki 
> Signed-off-by: Michael D Kinney 
> ---
>  EmulatorPkg/Win/Host/WinHost.inf | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/EmulatorPkg/Win/Host/WinHost.inf 
> b/EmulatorPkg/Win/Host/WinHost.inf
> index 4d2ce19620a1..2030ac084702 100644
> --- a/EmulatorPkg/Win/Host/WinHost.inf
> +++ b/EmulatorPkg/Win/Host/WinHost.inf
> @@ -86,7 +86,7 @@ [Pcd]
> 
>  [BuildOptions]
>MSFT:*_*_*_DLINK_FLAGS== /out:"$(BIN_DIR)\$(BASE_NAME).exe" 
> /base:0x1000
> /pdb:"$(BIN_DIR)\$(BASE_NAME).pdb"
> -  MSFT:*_*_*_CC_FLAGS   == /nologo /W4 /WX /Gy /c /D UNICODE /Od 
> /Oy- /FIAutoGen.h /EHs-c- /GF /Gs8192 /Zi /Gm
> /D _CRT_SECURE_NO_WARNINGS /D _CRT_SECURE_NO_DEPRECATE
> +  MSFT:*_*_*_CC_FLAGS= /nologo /W4 /WX /Gy /c /D UNICODE /Od 
> /Oy- /FIAutoGen.h /EHs-c- /GF /D
> _CRT_SECURE_NO_WARNINGS /D _CRT_SECURE_NO_DEPRECATE
>MSFT:*_*_*_PP_FLAGS   == /nologo /E /TC /FIAutoGen.h
> 
>MSFT:*_VS2015_IA32_DLINK_FLAGS = /LIBPATH:"%VS2015_PREFIX%Lib" 
> /LIBPATH:"%VS2015_PREFIX%VC\Lib"
> /LIBPATH:"%UniversalCRTSdkDir%lib\%UCRTVersion%\ucrt\x86"
> /LIBPATH:"%WindowsSdkDir%lib\%WindowsSDKLibVersion%\um\x86" /NOLOGO 
> /SUBSYSTEM:CONSOLE /NODEFAULTLIB
> /IGNORE:4086 /MAP /OPT:REF /DEBUG /MACHINE:I386 /LTCG Kernel32.lib 
> MSVCRTD.lib Gdi32.lib User32.lib Winmm.lib
> Advapi32.lib vcruntimed.lib ucrtd.lib
> --
> 2.32.0.windows.1



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

Re: [edk2-devel] [PATCH v4 1/1] .pytool/Plugin/UncrustifyCheck: Add Uncrustify CI plugin

Hi Michael,

Unfortunately, enabling the templates in the uncrustify.cfg files modifies 340 
files with missing 
file/function headers that will now fail UncrustifyCheck.

cmt_insert_file_header  = default_file_header.txt
cmt_insert_func_header  = default_function_header.txt

Perhaps we should leave this feature off to get the uncrustify format changes 
committed and
enable this feature immediately after so patch reviews after that will slowly 
fix these missing
file/function headers.

Or I can comment these two lines out when I generate the final version of the 
patch series so
we don’t have to do an extra commit to re-enable.

Mike 

> -Original Message-
> From: Kinney, Michael D 
> Sent: Wednesday, November 24, 2021 3:06 PM
> To: devel@edk2.groups.io; mikub...@linux.microsoft.com; Kinney, Michael D 
> 
> Cc: Liming Gao ; Sean Brogan 
> ; Bret Barkelew
> 
> Subject: RE: [edk2-devel] [PATCH v4 1/1] .pytool/Plugin/UncrustifyCheck: Add 
> Uncrustify CI plugin
> 
> Thanks.  That makes sense.
> 
> Mike
> 
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of Michael 
> > Kubacki
> > Sent: Wednesday, November 24, 2021 2:55 PM
> > To: devel@edk2.groups.io; Kinney, Michael D 
> > Cc: Liming Gao ; Sean Brogan 
> > ; Bret Barkelew
> > 
> > Subject: Re: [edk2-devel] [PATCH v4 1/1] .pytool/Plugin/UncrustifyCheck: 
> > Add Uncrustify CI plugin
> >
> > Yes. Those were redundant and detected by the Python configparser module
> > when I used it to parse the config file.
> >
> > Thanks,
> > Michael
> >
> > On 11/24/2021 5:45 PM, Michael D Kinney wrote:
> > > Hi Michael,
> > >
> > > I see a couple settings deleted from the uncrustify.cfg.  Was that in 
> > > purpose?  Does it change the format?
> > >
> > >
> > > -nl_func_call_start_multi_line   = true  # Whether to add a newline 
> > > after '(' in a function call if '(' and ')'
> are
> > > -# in different lines.
> > >
> > >
> > > -nl_func_call_args_multi_line = true
> > > -nl_func_call_args_multi_line_ignore_closures = false
> > >
> > >
> > > Thanks,
> > >
> > > Mike
> > >
> > >> -Original Message-
> > >> From: mikub...@linux.microsoft.com 
> > >> Sent: Wednesday, November 24, 2021 1:42 PM
> > >> To: devel@edk2.groups.io
> > >> Cc: Kinney, Michael D ; Liming Gao 
> > >> ; Sean Brogan
> > >> ; Bret Barkelew 
> > >> Subject: [PATCH v4 1/1] .pytool/Plugin/UncrustifyCheck: Add Uncrustify 
> > >> CI plugin
> > >>
> > >> From: Michael Kubacki 
> > >>
> > >> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3748
> > >>
> > >> Adds a new CI plugin for Uncrustify. This is used to check
> > >> coding standard compliance of source code to the EDK II C Coding
> > >> Standards Specification.
> > >>
> > >> An external dependency is added in the plugin directory to retrieve
> > >> the Uncrustify executable. Currently, the executable is from an edk2
> > >> fork of the application host in this repository:
> > >>
> > >> https://dev.azure.com/projectmu/Uncrustify/
> > >>
> > >> Note that the Uncrustify application outputs the commit ID that the
> > >> application was built from when given the --version parameter.
> > >>
> > >> This ID can be mapped to the version specified in the Uncrustify CI
> > >> plugin external dependency file (uncrustify_ext_dep.yaml) such as
> > >> 73.0.3 by visiting the Uncrustify edk2 fork release pipeline page
> > >> which associates the NuGet package version with the commit ID it was
> > >> built from:
> > >>
> > >> https://dev.azure.com/projectmu/Uncrustify/_build
> > >>
> > >> The default Uncrustify configuration files are added in the plugin
> > >> directory. Additional details are in the Readme.md file added in
> > >> the Uncrustify plugin directory.
> > >>
> > >> Cc: Michael D Kinney 
> > >> Cc: Liming Gao 
> > >> Cc: Sean Brogan 
> > >> Cc: Bret Barkelew 
> > >> Signed-off-by: Michael Kubacki 
> > >> ---
> > >>
> > >> Notes:
> > >>  V4 changes:
> > >>
> > >>  1. Updated commit subject to use the new plugin name.
> > >>  2. Enabled file and function header templates to be placed in
> > >> files missing a file or function header (comment block).
> > >>  3. Added support to detect whether a file header or function
> > >> header is missing in a file by searching files modified by
> > >> Uncrustify for the template file text.
> > >>  4. Updated the plugin name displayed in some log output messages.
> > >>
> > >>  V3 changes:
> > >>
> > >>  1. Renamed the CI plugin to "UncrustifyCheck" from "Uncrustify"
> > >> to follow similar naming conventions used in other plugins
> > >> that check files
> > >>  2. Added a clarifying paragraph to the beginning of the Uncrustify
> > >> CI plugin Readme.md file that the plugin is enabled by default
> > >> and test results can be ignored by enabling "AuditOnly" mode.
> > >>  3. Added instructions in the commi

Re: [edk2-devel] [Patch 10/12] StandaloneMmPkg: Update YAML to ignore specific ECC files/errors



> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Sami Mujawar
> Sent: Wednesday, November 24, 2021 2:55 AM
> To: Kinney, Michael D ; devel@edk2.groups.io
> Cc: Ard Biesheuvel ; Yao, Jiewen 
> ; Supreeth Venkatesh
> ; Sean Brogan ; Bret 
> Barkelew ; Liming
> Gao ; Michael Kubacki 
> ; nd 
> Subject: Re: [edk2-devel] [Patch 10/12] StandaloneMmPkg: Update YAML to 
> ignore specific ECC files/errors
> 
> Hi Mike,
> 
> 
> On 23/11/2021 09:21 PM, Michael D Kinney wrote:
> > +"EccCheck": {
> > +## Exception sample looks like below:
> > +## "ExceptionList": [
> > +## "", ""
> > +## ]
> > +"ExceptionList": [
> > +"4002", "HobConstructor"
> > +],
> 
> I noticed there are other ECC errors with ID 4002 in StandaloneMmPkg.
> 
> e.g.
> 
> edk2\StandaloneMmPkg\Library\StandaloneMmCoreHobLib\Arm\StandaloneMmCoreHobLib.c(24):
> [4002]Function header doesn't exist Function [CreateHob] has NO comment
> immediately preceding it.
> edk2\StandaloneMmPkg\Library\StandaloneMmCoreHobLib\Arm\StandaloneMmCoreHobLibInternal.c(24):
> [4002]Function header doesn't exist Function [HobConstructor] has NO
> comment immediately preceding it.
> edk2\StandaloneMmPkg\Library\StandaloneMmHobLib\StandaloneMmHobLib.c(247):
> [4002]Function header doesn't exist Function [CreateHob] has NO comment
> immediately preceding it.
> edk2\StandaloneMmPkg\Library\StandaloneMmPeCoffExtraActionLib\AArch64\StandaloneMmPeCoffExtraActionLib.c(26):
> [4002]Function header doesn't exist Function [UpdatePeCoffPermissions]
> has NO comment immediately preceding it.
> 
> I am trying to undestand how the ExceptionList works. Would Keyword =
> HobConstructor ignore the other errors too?

Good question.  EccCheck in EDK II CI only checks lines of files that have been
added or changed.  So if these lines were not modified by uncrustify, then they
would not block the commit, even if they have ECC issues.

> 
> We will look into getting these issues fixed. However, to unblock this
> patch can proceed.

Thank you.  Yes.  That is the intention here.  Temporarily use YAML file
to get EccCheck to pass, then the work with package owners to update code 
so these exceptions can be removed.

> 
> Reviewed-by: Sami Mujawar 
> 
> Regards,
> 
> Sami Mujawar
> 
> 
> 
> 
> 



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

Re: [edk2-devel] [PATCH v4 1/1] .pytool/Plugin/UncrustifyCheck: Add Uncrustify CI plugin

Thanks.  That makes sense.

Mike

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Michael Kubacki
> Sent: Wednesday, November 24, 2021 2:55 PM
> To: devel@edk2.groups.io; Kinney, Michael D 
> Cc: Liming Gao ; Sean Brogan 
> ; Bret Barkelew
> 
> Subject: Re: [edk2-devel] [PATCH v4 1/1] .pytool/Plugin/UncrustifyCheck: Add 
> Uncrustify CI plugin
> 
> Yes. Those were redundant and detected by the Python configparser module
> when I used it to parse the config file.
> 
> Thanks,
> Michael
> 
> On 11/24/2021 5:45 PM, Michael D Kinney wrote:
> > Hi Michael,
> >
> > I see a couple settings deleted from the uncrustify.cfg.  Was that in 
> > purpose?  Does it change the format?
> >
> >
> > -nl_func_call_start_multi_line   = true  # Whether to add a newline 
> > after '(' in a function call if '(' and ')' are
> > -# in different lines.
> >
> >
> > -nl_func_call_args_multi_line = true
> > -nl_func_call_args_multi_line_ignore_closures = false
> >
> >
> > Thanks,
> >
> > Mike
> >
> >> -Original Message-
> >> From: mikub...@linux.microsoft.com 
> >> Sent: Wednesday, November 24, 2021 1:42 PM
> >> To: devel@edk2.groups.io
> >> Cc: Kinney, Michael D ; Liming Gao 
> >> ; Sean Brogan
> >> ; Bret Barkelew 
> >> Subject: [PATCH v4 1/1] .pytool/Plugin/UncrustifyCheck: Add Uncrustify CI 
> >> plugin
> >>
> >> From: Michael Kubacki 
> >>
> >> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3748
> >>
> >> Adds a new CI plugin for Uncrustify. This is used to check
> >> coding standard compliance of source code to the EDK II C Coding
> >> Standards Specification.
> >>
> >> An external dependency is added in the plugin directory to retrieve
> >> the Uncrustify executable. Currently, the executable is from an edk2
> >> fork of the application host in this repository:
> >>
> >> https://dev.azure.com/projectmu/Uncrustify/
> >>
> >> Note that the Uncrustify application outputs the commit ID that the
> >> application was built from when given the --version parameter.
> >>
> >> This ID can be mapped to the version specified in the Uncrustify CI
> >> plugin external dependency file (uncrustify_ext_dep.yaml) such as
> >> 73.0.3 by visiting the Uncrustify edk2 fork release pipeline page
> >> which associates the NuGet package version with the commit ID it was
> >> built from:
> >>
> >> https://dev.azure.com/projectmu/Uncrustify/_build
> >>
> >> The default Uncrustify configuration files are added in the plugin
> >> directory. Additional details are in the Readme.md file added in
> >> the Uncrustify plugin directory.
> >>
> >> Cc: Michael D Kinney 
> >> Cc: Liming Gao 
> >> Cc: Sean Brogan 
> >> Cc: Bret Barkelew 
> >> Signed-off-by: Michael Kubacki 
> >> ---
> >>
> >> Notes:
> >>  V4 changes:
> >>
> >>  1. Updated commit subject to use the new plugin name.
> >>  2. Enabled file and function header templates to be placed in
> >> files missing a file or function header (comment block).
> >>  3. Added support to detect whether a file header or function
> >> header is missing in a file by searching files modified by
> >> Uncrustify for the template file text.
> >>  4. Updated the plugin name displayed in some log output messages.
> >>
> >>  V3 changes:
> >>
> >>  1. Renamed the CI plugin to "UncrustifyCheck" from "Uncrustify"
> >> to follow similar naming conventions used in other plugins
> >> that check files
> >>  2. Added a clarifying paragraph to the beginning of the Uncrustify
> >> CI plugin Readme.md file that the plugin is enabled by default
> >> and test results can be ignored by enabling "AuditOnly" mode.
> >>  3. Added instructions in the commit message on how to map the
> >> version reported by the Uncrustify application (--version) to
> >> the version specified in the CI plugin external dependency
> >> YAML file.
> >>
> >>  V2 changes:
> >>
> >>  1. Changed plugin temp directory to Build/.pytool/Plugin/Uncrustify
> >>  2. Added TCBZ REF in patch commit message
> >>
> >>   .pytool/Plugin/UncrustifyCheck/Readme.md   | 120 
> >>   .pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py  | 617 
> >> 
> >>   .pytool/Plugin/UncrustifyCheck/default_file_header.txt |   9 +
> >>   .pytool/Plugin/UncrustifyCheck/default_function_header.txt |  15 +
> >>   .pytool/Plugin/UncrustifyCheck/uncrustify.cfg  | 462 
> >> +++
> >>   .pytool/Plugin/UncrustifyCheck/uncrustify_ext_dep.yaml |  16 +
> >>   .pytool/Plugin/UncrustifyCheck/uncrustify_plug_in.yaml |  11 +
> >>   .pytool/Readme.md  |   4 +
> >>   8 files changed, 1254 insertions(+)
> >>
> >> diff --git a/.pytool/Plugin/UncrustifyCheck/Readme.md 
> >> b/.pytool/Plugin/UncrustifyCheck/Readme.md
> >> new file mode 100644
> >> index ..bb263bcc87d7
>

Re: [edk2-devel] [PATCH v4 1/1] .pytool/Plugin/UncrustifyCheck: Add Uncrustify CI plugin

Yes. Those were redundant and detected by the Python configparser module 
when I used it to parse the config file.


Thanks,
Michael

On 11/24/2021 5:45 PM, Michael D Kinney wrote:

Hi Michael,

I see a couple settings deleted from the uncrustify.cfg.  Was that in purpose?  
Does it change the format?


-nl_func_call_start_multi_line   = true  # Whether to add a newline after 
'(' in a function call if '(' and ')' are
-# in different lines.


-nl_func_call_args_multi_line = true
-nl_func_call_args_multi_line_ignore_closures = false


Thanks,

Mike


-Original Message-
From: mikub...@linux.microsoft.com 
Sent: Wednesday, November 24, 2021 1:42 PM
To: devel@edk2.groups.io
Cc: Kinney, Michael D ; Liming Gao 
; Sean Brogan
; Bret Barkelew 
Subject: [PATCH v4 1/1] .pytool/Plugin/UncrustifyCheck: Add Uncrustify CI plugin

From: Michael Kubacki 

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

Adds a new CI plugin for Uncrustify. This is used to check
coding standard compliance of source code to the EDK II C Coding
Standards Specification.

An external dependency is added in the plugin directory to retrieve
the Uncrustify executable. Currently, the executable is from an edk2
fork of the application host in this repository:

https://dev.azure.com/projectmu/Uncrustify/

Note that the Uncrustify application outputs the commit ID that the
application was built from when given the --version parameter.

This ID can be mapped to the version specified in the Uncrustify CI
plugin external dependency file (uncrustify_ext_dep.yaml) such as
73.0.3 by visiting the Uncrustify edk2 fork release pipeline page
which associates the NuGet package version with the commit ID it was
built from:

https://dev.azure.com/projectmu/Uncrustify/_build

The default Uncrustify configuration files are added in the plugin
directory. Additional details are in the Readme.md file added in
the Uncrustify plugin directory.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Sean Brogan 
Cc: Bret Barkelew 
Signed-off-by: Michael Kubacki 
---

Notes:
 V4 changes:

 1. Updated commit subject to use the new plugin name.
 2. Enabled file and function header templates to be placed in
files missing a file or function header (comment block).
 3. Added support to detect whether a file header or function
header is missing in a file by searching files modified by
Uncrustify for the template file text.
 4. Updated the plugin name displayed in some log output messages.

 V3 changes:

 1. Renamed the CI plugin to "UncrustifyCheck" from "Uncrustify"
to follow similar naming conventions used in other plugins
that check files
 2. Added a clarifying paragraph to the beginning of the Uncrustify
CI plugin Readme.md file that the plugin is enabled by default
and test results can be ignored by enabling "AuditOnly" mode.
 3. Added instructions in the commit message on how to map the
version reported by the Uncrustify application (--version) to
the version specified in the CI plugin external dependency
YAML file.

 V2 changes:

 1. Changed plugin temp directory to Build/.pytool/Plugin/Uncrustify
 2. Added TCBZ REF in patch commit message

  .pytool/Plugin/UncrustifyCheck/Readme.md   | 120 
  .pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py  | 617 

  .pytool/Plugin/UncrustifyCheck/default_file_header.txt |   9 +
  .pytool/Plugin/UncrustifyCheck/default_function_header.txt |  15 +
  .pytool/Plugin/UncrustifyCheck/uncrustify.cfg  | 462 
+++
  .pytool/Plugin/UncrustifyCheck/uncrustify_ext_dep.yaml |  16 +
  .pytool/Plugin/UncrustifyCheck/uncrustify_plug_in.yaml |  11 +
  .pytool/Readme.md  |   4 +
  8 files changed, 1254 insertions(+)

diff --git a/.pytool/Plugin/UncrustifyCheck/Readme.md 
b/.pytool/Plugin/UncrustifyCheck/Readme.md
new file mode 100644
index ..bb263bcc87d7
--- /dev/null
+++ b/.pytool/Plugin/UncrustifyCheck/Readme.md
@@ -0,0 +1,120 @@
+# UncrustifyCheck Plugin
+
+This CiBuildPlugin scans all the files in a given package and checks for 
coding standard compliance issues.
+
+This plugin is enabled by default. If a package would like to prevent the 
plugin from reporting errors, it can do
+so by enabling [`AuditOnly`](#auditonly) mode.
+
+This plugin requires the directory containing the Uncrustify executable that 
should be used for this plugin to
+be specified in an environment variable named `UNCRUSTIFY_CI_PATH`. This 
unique variable name is used to avoid confusion
+with other paths to Uncrustify which might not be the expected build for use 
by this plugin.
+
+By default, an Uncrustify configuration file named "uncrustify.cfg" located in 
the same directory as the plugin is
+used. The value can be overridden to a package-specifi

Re: [edk2-devel] CdePkgBlog 2021-11-14

2021-11-24 Thread Pedro Falcato

Hi Kilian,

I agree with Maciej. There should be a big emphasis on getting an
*open-source* libc available in EDK2, and not relying on a proprietary,
opaque binary blob.

In my opinion EDK2 should pivot to using a high-quality, license-compatible
libc such as Newlib, musl, etc. If you believe your Toro C library is the
best tool for the job, you should open-source it (with a compatible
license) as soon as possible to let the community decide :)

Best regards,
Pedro

On Wed, Nov 24, 2021 at 8:21 PM Kilian Kegel 
wrote:

> Hi Maciej,
>
>
>
> *CdePkg* is already integrated into EDK2 and satisfies all your needs for
> pre-/post-memory
>
> PEI, DXE, (SMM in latest releases only), BDS and UEFI Shell too.
>
>
>
> I have introduced *CdePkg* for POST usage 2 years ago without any
> interest of the “Tianocore Community”:
>
>
> https://edk2.groups.io/g/devel/message/51562?p=,,,20,0,0,0::Created,,CdePkg,20,2,0,65191785
>
>
>
>1. Klick on the email link above
>2. Klick on the first link in that email, that goes to the
>edk2-staging\cdepkg folder
>
>
>
>
>1. read the HOWTO:
>https://github.com/tianocore/edk2-staging/blob/CdePkg/README.md#howto
>
>*and go for emulation – that works perfectly.*
>
>1. I simply can’t do more for you, than to urgently encourage to try
>it out yourself, test it, see what it can do for you
>
>
>
> This time *toro C Library* (that is absolutely the same as the library
> part of the latest
>
> CdePkg
> https://github.com/KilianKegel/CdePkg/blob/master/CdeLib/CdeLib.mak#L15)
>
> for UEFI Shell will be discussed first, since “Tianocore Community” is now
> aware of serious problems of edk2libc
>
> and probably ready to examine my solution, that lacks all of those issues.
>
>
>
> You, Maciej, can support *CdePkg/ toro C Library* by
>
>
>
>1. testing *CdePkg*
>2. giving feedback to the community
>3. updating *CdePkg* to latest AAEON WhiskeyLake platform (that is on
>my Desk for months but I simply don’t have time to get it running…)
>
> (MinnowBoard is deprecated now)
>
>
>
> The license issue *can not* be discussed *before* there is any serious
> interest in *CdePkg* and *toro C Library*.
>
>
>
> But if you are seeking  for a free solution, that
>
>- requires unknown additional working hours
>- requires a couple of high motivated maintainers, specialized in
>Standard C
>- countless  patches will spam your inbox folder for years
>- additionally with unpredictable result
>
> please use  edk2libc, PDCLIB or adjust NewLib for your needs.
>
>
>
> >Unless those problems are solved, I simply cannot use it.
>
> Right, but you are invited to take a test drive!
>
> In the EDK2 Emulator. Debugging with the best Debugtool ever seen…
>
> Try to get SysPrep and Redfish running in the Emulator.
>
> Really, just do it!
>
>
>
> > How can I verify what was actually implemented inside?
>
> I will publish source code of *toro C Library* beginning 2022 and discuss
> the design details along those files.
>
> But, did you check glibc’s fopen-source before using it?
>
> Have you ever seen
> Microsoft/Borland/Symantec/ARM/IBM/Dinkumware/Greenhill/IAR … C Libraries
> source code?
>
>
>
> Thanks,
>
> Kilian
>
>
>
>
>
> Sent from Mail  for
> Windows
>
>
>
> *From: *Rabeda, Maciej 
> *Sent: *Monday, November 22, 2021 05:31 PM
> *To: *devel@edk2.groups.io; kilian_ke...@outlook.com
> *Cc: *Rebecca Cran ; tim.le...@insyde.com; Mike
> Kinney ; af...@apple.com; Leif Lindholm
> ; Richardson, Brian 
> *Subject: *Re: [edk2-devel] CdePkgBlog 2021-11-14
>
>
>
> Hi Kilian,
>
> From my point of view, the main problem with adoption of CdePkg to EDK2 is
> that it relies on Torito C library.
>
>1. Torito C library License (
>https://github.com/KilianKegel/toro-C-Library/blob/master/LICENSE.md)
>only allows for creating UEFI Shell applications.
>   - What about applications that do not rely on ShellPkg (example:
>   SysPrep application that might want to use Redfish, which depends on C
>   standard library)?
>   - What about drivers/libraries that rely on C standard library?
>   - How is that compatible with EDK2 BSD-2-Clause-Patent?
>2. Torito C is pre-compiled.
>   - How can I verify what was actually implemented inside? Industry
>   would have to trust your tests, perform own set of tests or/and 
> disassemble
>   it (doable, but unacceptable effort-wise).
>
> Unless those problems are solved, I simply cannot use it.
>
> Thanks,
> Maciej
>
> On 14-Nov-21 20:51, Kilian Kegel wrote:
>
> Hi All,
>
>
>
> as announced last summer, I’d like to start a comprehensive tutorial on
>
> how to use Standard C / ANSI C in the UEFI environment and how to design
> and implement
>
> such a library for Shell and POST usage.
>
>
>
> Since most parts of that comprehensive work are already done,
>
> I will report, demonstrate and discuss implementation principles and
> details on edk2.gro

Re: [edk2-devel] [PATCH v4 1/1] .pytool/Plugin/UncrustifyCheck: Add Uncrustify CI plugin

Hi Michael,

I see a couple settings deleted from the uncrustify.cfg.  Was that in purpose?  
Does it change the format?


-nl_func_call_start_multi_line   = true  # Whether to add a newline after 
'(' in a function call if '(' and ')' are
-# in different lines.


-nl_func_call_args_multi_line = true
-nl_func_call_args_multi_line_ignore_closures = false


Thanks,

Mike

> -Original Message-
> From: mikub...@linux.microsoft.com 
> Sent: Wednesday, November 24, 2021 1:42 PM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D ; Liming Gao 
> ; Sean Brogan
> ; Bret Barkelew 
> Subject: [PATCH v4 1/1] .pytool/Plugin/UncrustifyCheck: Add Uncrustify CI 
> plugin
> 
> From: Michael Kubacki 
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3748
> 
> Adds a new CI plugin for Uncrustify. This is used to check
> coding standard compliance of source code to the EDK II C Coding
> Standards Specification.
> 
> An external dependency is added in the plugin directory to retrieve
> the Uncrustify executable. Currently, the executable is from an edk2
> fork of the application host in this repository:
> 
> https://dev.azure.com/projectmu/Uncrustify/
> 
> Note that the Uncrustify application outputs the commit ID that the
> application was built from when given the --version parameter.
> 
> This ID can be mapped to the version specified in the Uncrustify CI
> plugin external dependency file (uncrustify_ext_dep.yaml) such as
> 73.0.3 by visiting the Uncrustify edk2 fork release pipeline page
> which associates the NuGet package version with the commit ID it was
> built from:
> 
> https://dev.azure.com/projectmu/Uncrustify/_build
> 
> The default Uncrustify configuration files are added in the plugin
> directory. Additional details are in the Readme.md file added in
> the Uncrustify plugin directory.
> 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Sean Brogan 
> Cc: Bret Barkelew 
> Signed-off-by: Michael Kubacki 
> ---
> 
> Notes:
> V4 changes:
> 
> 1. Updated commit subject to use the new plugin name.
> 2. Enabled file and function header templates to be placed in
>files missing a file or function header (comment block).
> 3. Added support to detect whether a file header or function
>header is missing in a file by searching files modified by
>Uncrustify for the template file text.
> 4. Updated the plugin name displayed in some log output messages.
> 
> V3 changes:
> 
> 1. Renamed the CI plugin to "UncrustifyCheck" from "Uncrustify"
>to follow similar naming conventions used in other plugins
>that check files
> 2. Added a clarifying paragraph to the beginning of the Uncrustify
>CI plugin Readme.md file that the plugin is enabled by default
>and test results can be ignored by enabling "AuditOnly" mode.
> 3. Added instructions in the commit message on how to map the
>version reported by the Uncrustify application (--version) to
>the version specified in the CI plugin external dependency
>YAML file.
> 
> V2 changes:
> 
> 1. Changed plugin temp directory to Build/.pytool/Plugin/Uncrustify
> 2. Added TCBZ REF in patch commit message
> 
>  .pytool/Plugin/UncrustifyCheck/Readme.md   | 120 
>  .pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py  | 617 
> 
>  .pytool/Plugin/UncrustifyCheck/default_file_header.txt |   9 +
>  .pytool/Plugin/UncrustifyCheck/default_function_header.txt |  15 +
>  .pytool/Plugin/UncrustifyCheck/uncrustify.cfg  | 462 
> +++
>  .pytool/Plugin/UncrustifyCheck/uncrustify_ext_dep.yaml |  16 +
>  .pytool/Plugin/UncrustifyCheck/uncrustify_plug_in.yaml |  11 +
>  .pytool/Readme.md  |   4 +
>  8 files changed, 1254 insertions(+)
> 
> diff --git a/.pytool/Plugin/UncrustifyCheck/Readme.md 
> b/.pytool/Plugin/UncrustifyCheck/Readme.md
> new file mode 100644
> index ..bb263bcc87d7
> --- /dev/null
> +++ b/.pytool/Plugin/UncrustifyCheck/Readme.md
> @@ -0,0 +1,120 @@
> +# UncrustifyCheck Plugin
> +
> +This CiBuildPlugin scans all the files in a given package and checks for 
> coding standard compliance issues.
> +
> +This plugin is enabled by default. If a package would like to prevent the 
> plugin from reporting errors, it can do
> +so by enabling [`AuditOnly`](#auditonly) mode.
> +
> +This plugin requires the directory containing the Uncrustify executable that 
> should be used for this plugin to
> +be specified in an environment variable named `UNCRUSTIFY_CI_PATH`. This 
> unique variable name is used to avoid confusion
> +with other paths to Uncrustify which might not be the expected build for use 
> by this plugin.
> +
> +By default, an Uncrustify configuration file named "uncrustify.cfg" located 
> in the same directory as the plugin is
> +used. The value can be overridden to a pa

[edk2-devel] [PATCH v4 1/1] .pytool/Plugin/UncrustifyCheck: Add Uncrustify CI plugin

From: Michael Kubacki 

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

Adds a new CI plugin for Uncrustify. This is used to check
coding standard compliance of source code to the EDK II C Coding
Standards Specification.

An external dependency is added in the plugin directory to retrieve
the Uncrustify executable. Currently, the executable is from an edk2
fork of the application host in this repository:

https://dev.azure.com/projectmu/Uncrustify/

Note that the Uncrustify application outputs the commit ID that the
application was built from when given the --version parameter.

This ID can be mapped to the version specified in the Uncrustify CI
plugin external dependency file (uncrustify_ext_dep.yaml) such as
73.0.3 by visiting the Uncrustify edk2 fork release pipeline page
which associates the NuGet package version with the commit ID it was
built from:

https://dev.azure.com/projectmu/Uncrustify/_build

The default Uncrustify configuration files are added in the plugin
directory. Additional details are in the Readme.md file added in
the Uncrustify plugin directory.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Sean Brogan 
Cc: Bret Barkelew 
Signed-off-by: Michael Kubacki 
---

Notes:
V4 changes:

1. Updated commit subject to use the new plugin name.
2. Enabled file and function header templates to be placed in
   files missing a file or function header (comment block).
3. Added support to detect whether a file header or function
   header is missing in a file by searching files modified by
   Uncrustify for the template file text.
4. Updated the plugin name displayed in some log output messages.

V3 changes:

1. Renamed the CI plugin to "UncrustifyCheck" from "Uncrustify"
   to follow similar naming conventions used in other plugins
   that check files
2. Added a clarifying paragraph to the beginning of the Uncrustify
   CI plugin Readme.md file that the plugin is enabled by default
   and test results can be ignored by enabling "AuditOnly" mode.
3. Added instructions in the commit message on how to map the
   version reported by the Uncrustify application (--version) to
   the version specified in the CI plugin external dependency
   YAML file.

V2 changes:

1. Changed plugin temp directory to Build/.pytool/Plugin/Uncrustify
2. Added TCBZ REF in patch commit message

 .pytool/Plugin/UncrustifyCheck/Readme.md   | 120 
 .pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py  | 617 

 .pytool/Plugin/UncrustifyCheck/default_file_header.txt |   9 +
 .pytool/Plugin/UncrustifyCheck/default_function_header.txt |  15 +
 .pytool/Plugin/UncrustifyCheck/uncrustify.cfg  | 462 
+++
 .pytool/Plugin/UncrustifyCheck/uncrustify_ext_dep.yaml |  16 +
 .pytool/Plugin/UncrustifyCheck/uncrustify_plug_in.yaml |  11 +
 .pytool/Readme.md  |   4 +
 8 files changed, 1254 insertions(+)

diff --git a/.pytool/Plugin/UncrustifyCheck/Readme.md 
b/.pytool/Plugin/UncrustifyCheck/Readme.md
new file mode 100644
index ..bb263bcc87d7
--- /dev/null
+++ b/.pytool/Plugin/UncrustifyCheck/Readme.md
@@ -0,0 +1,120 @@
+# UncrustifyCheck Plugin
+
+This CiBuildPlugin scans all the files in a given package and checks for 
coding standard compliance issues.
+
+This plugin is enabled by default. If a package would like to prevent the 
plugin from reporting errors, it can do
+so by enabling [`AuditOnly`](#auditonly) mode.
+
+This plugin requires the directory containing the Uncrustify executable that 
should be used for this plugin to
+be specified in an environment variable named `UNCRUSTIFY_CI_PATH`. This 
unique variable name is used to avoid confusion
+with other paths to Uncrustify which might not be the expected build for use 
by this plugin.
+
+By default, an Uncrustify configuration file named "uncrustify.cfg" located in 
the same directory as the plugin is
+used. The value can be overridden to a package-specific path with the 
`ConfigFilePath` configuration file option.
+
+* Uncrustify source code and documentation: 
https://github.com/uncrustify/uncrustify
+* Project Mu Uncrustify fork source code and documentation: 
https://dev.azure.com/projectmu/Uncrustify
+
+## Files Checked in a Package
+
+By default, this plugin will discover all files in the package with the 
following default paths:
+
+```python
+[
+# C source
+"*.c",
+"*.h"
+]
+```
+
+From this list of files, any files ignored by Git or residing in a Git 
submodule will be removed. If Git is not
+found, submodules are not found, or ignored files are not found no changes are 
made to the list of discovered files.
+
+To control the paths checked in a given package, review the configuration 
options described in this file.
+
+## Configuration
+
+The plugin can be configured with a few optional configuration options.
+
+``` yaml
+  "UncrustifyCheck

Re: [edk2-devel] CdePkgBlog 2021-11-14

2021-11-24 Thread Kilian Kegel

Hi Maciej,

CdePkg is already integrated into EDK2 and satisfies all your needs for 
pre-/post-memory
PEI, DXE, (SMM in latest releases only), BDS and UEFI Shell too.

I have introduced CdePkg for POST usage 2 years ago without any interest of the 
“Tianocore Community”:
https://edk2.groups.io/g/devel/message/51562?p=,,,20,0,0,0::Created,,CdePkg,20,2,0,65191785

  1.  Klick on the email link above
  2.  Klick on the first link in that email, that goes to the 
edk2-staging\cdepkg folder

[cid:image001.png@01D7E178.E5854300]

  1.  read the HOWTO: 
https://github.com/tianocore/edk2-staging/blob/CdePkg/README.md#howto
   and go for emulation – that works perfectly.

  1.  I simply can’t do more for you, than to urgently encourage to try it out 
yourself, test it, see what it can do for you

This time toro C Library (that is absolutely the same as the library part of 
the latest
CdePkg https://github.com/KilianKegel/CdePkg/blob/master/CdeLib/CdeLib.mak#L15)
for UEFI Shell will be discussed first, since “Tianocore Community” is now 
aware of serious problems of edk2libc
and probably ready to examine my solution, that lacks all of those issues.

You, Maciej, can support CdePkg/ toro C Library by

  1.  testing CdePkg
  2.  giving feedback to the community
  3.  updating CdePkg to latest AAEON WhiskeyLake platform (that is on my Desk 
for months but I simply don’t have time to get it running…)

(MinnowBoard is deprecated now)

The license issue can not be discussed before there is any serious interest in 
CdePkg and toro C Library.

But if you are seeking  for a free solution, that

  *   requires unknown additional working hours
  *   requires a couple of high motivated maintainers, specialized in Standard C
  *   countless  patches will spam your inbox folder for years
  *   additionally with unpredictable result
please use  edk2libc, PDCLIB or adjust NewLib for your needs.

>Unless those problems are solved, I simply cannot use it.
Right, but you are invited to take a test drive!
In the EDK2 Emulator. Debugging with the best Debugtool ever seen…
Try to get SysPrep and Redfish running in the Emulator.
Really, just do it!

> How can I verify what was actually implemented inside?
I will publish source code of toro C Library beginning 2022 and discuss the 
design details along those files.
But, did you check glibc’s fopen-source before using it?
Have you ever seen Microsoft/Borland/Symantec/ARM/IBM/Dinkumware/Greenhill/IAR 
… C Libraries source code?

Thanks,
Kilian

Sent from Mail for Windows

From: Rabeda, Maciej
Sent: Monday, November 22, 2021 05:31 PM
To: devel@edk2.groups.io; 
kilian_ke...@outlook.com
Cc: Rebecca Cran; 
tim.le...@insyde.com; Mike 
Kinney; 
af...@apple.com; Leif 
Lindholm; Richardson, 
Brian
Subject: Re: [edk2-devel] CdePkgBlog 2021-11-14

Hi Kilian,

>From my point of view, the main problem with adoption of CdePkg to EDK2 is 
>that it relies on Torito C library.

  1.  Torito C library License 
(https://github.com/KilianKegel/toro-C-Library/blob/master/LICENSE.md) only 
allows for creating UEFI Shell applications.
 *   What about applications that do not rely on ShellPkg (example: SysPrep 
application that might want to use Redfish, which depends on C standard 
library)?
 *   What about drivers/libraries that rely on C standard library?
 *   How is that compatible with EDK2 BSD-2-Clause-Patent?
  2.  Torito C is pre-compiled.
 *   How can I verify what was actually implemented inside? Industry would 
have to trust your tests, perform own set of tests or/and disassemble it 
(doable, but unacceptable effort-wise).

Unless those problems are solved, I simply cannot use it.
Thanks,
Maciej
On 14-Nov-21 20:51, Kilian Kegel wrote:
Hi All,

as announced last summer, I’d like to start a comprehensive tutorial on
how to use Standard C / ANSI C in the UEFI environment and how to design and 
implement
such a library for Shell and POST usage.

Since most parts of that comprehensive work are already done,
I will report, demonstrate and discuss implementation principles and details on 
edk2.groups.io
as a kind of “blog” on a biweekly basis.

Please checkout my first CdePkgBlog 
https://github.com/tianocore/edk2-staging/tree/CdePkg/blogs/2021-11-14#cdepkgblog-2021-11-14
and enjoy the breathtaking build speed if compiler and linker are used 
exclusively to create MY LEGACY TOOLBOX,
a handy set of one-trick-ponies that I have been using for about 25 years.

Have fun,
Kilian

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84057): https://edk2.groups.io/g/devel/message/84057
Mute This Topic: https

Re: [edk2-devel] [PATCH edk2-platforms v1 00/10] Fix Arm platforms CI errors

2021-11-24 Thread Chris Jones

This patch series looks good to me.

Reviewed-by: Chris Jones 

Thanks,
Chris


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

Re: [edk2-devel] Uncrustify Conversion Detailed Plan and Extended Hard Freeze

Hello,

Quick update on the CompareBuild failures with EmulatorPkg VS2019 IA32/X64.
It was pilot error running comparison against the incorrect git reference.

PR #1 (Update EmulatorPkg Win Host [BuildOptions] MSFT CC_FLAGS to not force 
debug information)
is required for both runs to do the comparison.  I re-ran the BuildCompare 
GitHub Action
using the following settings:

Branch: https://github.com/mdkinney/edk2/tree/TestOnly_Uncrustify_PR_Series
--ref1:198028bf5e46d17f9c502b4ae35ecc406e211c37
--ref2:TestOnly_Uncrustify_PR_Series
Extra Options: -n 4 --quiet
Results: https://github.com/mdkinney/edk2/actions/runs/1498056902 
30 VS2019 build comparisons PASS
40 GCC5 build comparisons PASS
100% PASS

The following git log shows the set of patches from --ref1 to --ref 2across
which there are no differences in any of the OBJ/LIB/DLL/EFI/FFS/FV/FD files.

--ref2 641895102bcc (HEAD -> TestOnly_Uncrustify_PR_Series) 
UnitTestFrameworkPkg: Apply uncrusitify changes
   abdb2f5634db UefiPayloadPkg: Apply uncrusitify changes
   cb9d471105c2 UefiCpuPkg: Apply uncrusitify changes
   e52477cd96a1 StandaloneMmPkg: Apply uncrusitify changes
   c94b69de1b0c SourceLevelDebugPkg: Apply uncrusitify changes
   f4732a917d54 SignedCapsulePkg: Apply uncrusitify changes
   815dcf7b2cd6 ShellPkg: Apply uncrusitify changes
   8a699af3521c SecurityPkg: Apply uncrusitify changes
   d5907a145b37 RedfishPkg: Apply uncrusitify changes
   5ce406d3823b PcAtChipsetPkg: Apply uncrusitify changes
   082006536617 OvmfPkg: Apply uncrusitify changes
   9c670c478ab4 NetworkPkg: Apply uncrusitify changes
   51fa52ef0066 MdePkg: Apply uncrusitify changes
   1b4476301abb MdeModulePkg: Apply uncrusitify changes
   3701ed8a5588 IntelFsp2WrapperPkg: Apply uncrusitify changes
   87db8c3e8f31 IntelFsp2Pkg: Apply uncrusitify changes
   add105d79021 FmpDevicePkg: Apply uncrusitify changes
   f8543fc3e5c5 FatPkg: Apply uncrusitify changes
   65b5fc52fbb4 EmulatorPkg: Apply uncrusitify changes
   3f436d2570b0 EmbeddedPkg: Apply uncrusitify changes
   cf01f39dd80d DynamicTablesPkg: Apply uncrusitify changes
   550476264900 CryptoPkg: Apply uncrusitify changes
   c09e92b19e13 ArmVirtPkg: Apply uncrusitify changes
   3d86c449ef90 ArmPlatformPkg: Apply uncrusitify changes
   bfa1b9c09f07 ArmPkg: Apply uncrusitify changes
   541b0f55afa8 UefiCpuPkg: Change use of EFI_D_* to DEBUG_*
   b783d2499182 SourceLevelDebugPkg: Change use of EFI_D_* to DEBUG_*
   df1821d6ac0e ShellPkg: Change use of EFI_D_* to DEBUG_*
   0c8159204288 SecurityPkg: Change use of EFI_D_* to DEBUG_*
   cb6643c08293 PcAtChipsetPkg: Change use of EFI_D_* to DEBUG_*
   db3f085cb414 OvmfPkg: Change use of EFI_D_* to DEBUG_*
   04199e7b8779 NetworkPkg: Change use of EFI_D_* to DEBUG_*
   a429397c68ef MdePkg: Change use of EFI_D_* to DEBUG_*
   408daf989bb1 MdeModulePkg: Change use of EFI_D_* to DEBUG_*
   7846ceaf387e FatPkg: Change use of EFI_D_* to DEBUG_*
   9934d3b85a31 EmulatorPkg: Change use of EFI_D_* to DEBUG_*
   e5b2066f2cc7 EmbeddedPkg: Change use of EFI_D_* to DEBUG_*
   c4b99a53238f ArmVirtPkg: Change use of EFI_D_* to DEBUG_*
   db8702afc3cf ArmPlatformPkg: Change use of EFI_D_* to DEBUG_*
   f62ed485f30f ArmPkg: Change use of EFI_D_* to DEBUG_*
   04e68228fbbb .pytool/Plugin/Uncrustify: Add Uncrustify plugin
   3d136bbc9935 .azurepipelines/templates: Update max pipeline job time to 
2 hours
   34bd4be55e66 UnitTestFrameworkPkg: Update YAML to ignore specific ECC 
files/errors
   793bedcdfb00 UefiPayloadPkg: Update YAML to ignore specific ECC 
files/errors
   7e2e089a77f2 StandaloneMmPkg: Update YAML to ignore specific ECC 
files/errors
   504e1839503c ShellPkg: Update YAML to ignore specific ECC files/errors
   fda332b58dfb SecurityPkg: Update YAML to ignore specific ECC files/errors
   8d24c7c5e971 MdePkg: Update YAML to ignore specific ECC files/errors
   e1182fcdbcea MdeModulePkg: Update YAML to ignore specific ECC 
files/errors
   8a2b93ad4848 EmulatorPkg: Update YAML to ignore specific ECC files/errors
   eabfb14f432a CryptoPkg: Update YAML to ignore specific ECC files/errors
   cc4fc209113e ArmVirtPkg: Update YAML to ignore specific ECC files/errors
   9ec1a1f0e996 ArmPlatformPkg: Update YAML to ignore specific ECC 
files/errors
   3e6a038a080a ArmPkg: Update YAML to ignore specific ECC files/errors
   b93693132ec6 .pytools/Plugin/LicenseCheck: Use temp directory for git 
diff output
   b9fee7fe9d75 .pytool/Plugin/EccCheck: Add performance optimizations
   26164e6e120c .pytool/Plugin/EccCheck: Remove temp directory on exception
   8715acca4b68 .pytool/Plugin/EccCheck: Remove RevertCode()
--ref1 198028bf5e46 EmulatorPkg/Win/Host: Update CC_FLAGS
   e1e7306b5414 (origin/master) OvmfPkg/Librar

[edk2-devel] [PATCH edk2-platforms v1 10/10] Platform/ARM: Fix Ecc error 5005

From: Pierre Gondois 

This patch fixes the following Ecc reported error:
The body of a function should be contained by open
and close braces that must be in the first column.

Signed-off-by: Pierre Gondois 
---
 .../ARM/JunoPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c| 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Platform/ARM/JunoPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c 
b/Platform/ARM/JunoPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c
index 0ec53782835b..c54291220348 100644
--- a/Platform/ARM/JunoPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c
+++ b/Platform/ARM/JunoPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c
@@ -23,7 +23,8 @@ GetSataDevice (
   IN  SATA_SI3132_INSTANCE* SataInstance,
   IN  UINT16 Port,
   IN  UINT16 PortMultiplierPort
-) {
+  )
+{
   LIST_ENTRY  *List;
   SATA_SI3132_PORT*SataPort;
   SATA_SI3132_DEVICE  *SataDevice;
--
2.25.1


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

[edk2-devel] [PATCH edk2-platforms v1 09/10] Platform/ARM: Fix Ecc error 9005

From: Pierre Gondois 

This patch fixes the following Ecc reported error:
Only Doxygen commands '@bug', '@todo', '@example',
'@file', '@attention', '@param', '@post', '@pre',
'@retval', '@return', '@sa', '@since', '@test',
'@note', '@par', '@endcode', '@code', '@{', '@}'
are allowed to mark the code

Signed-off-by: Pierre Gondois 
---
 .../ArmVExpressPciHostBridgeLib/ArmVExpressPciHostBridgeLib.c   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/Platform/ARM/VExpressPkg/Library/ArmVExpressPciHostBridgeLib/ArmVExpressPciHostBridgeLib.c
 
b/Platform/ARM/VExpressPkg/Library/ArmVExpressPciHostBridgeLib/ArmVExpressPciHostBridgeLib.c
index 54add33b8912..4f6e0c5c44b8 100644
--- 
a/Platform/ARM/VExpressPkg/Library/ArmVExpressPciHostBridgeLib/ArmVExpressPciHostBridgeLib.c
+++ 
b/Platform/ARM/VExpressPkg/Library/ArmVExpressPciHostBridgeLib/ArmVExpressPciHostBridgeLib.c
@@ -106,7 +106,7 @@ STATIC CONST CHAR16 mAcpiAddrSpaceTypeStr[][4] = {

   @param [out] Count  Return the count of root bridge instances.

-  @returns All the root bridge instances in an array.
+  @return  All the root bridge instances in an array.
The array should be passed into PciHostBridgeFreeRootBridges()
when it's not used.
 **/
--
2.25.1


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

[edk2-devel] [PATCH edk2-platforms v1 04/10] Platform/ARM: Modify duplicated GUID in ArmVExpressPkg.dec

From: Pierre Gondois 

The two following packages have the same GUID:
- Platform/ARM/VExpressPkg/ArmVExpressPkg.dec
- edk2/ArmPlatformPkg/ArmPlatformPkg.dec

The gArmVExpressTokenSpaceGuid in
Platform/ARM/VExpressPkg/ArmVExpressPkg.dec is also
identical.

All the GUIDs have been added at the same time, initially
in the edk2 repository. Modify the GUIDs in the edk2-platforms
repository.

Signed-off-by: Pierre Gondois 
---
 Platform/ARM/VExpressPkg/ArmVExpressPkg.dec | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Platform/ARM/VExpressPkg/ArmVExpressPkg.dec 
b/Platform/ARM/VExpressPkg/ArmVExpressPkg.dec
index 848510bff17e..76ba4f7f6485 100644
--- a/Platform/ARM/VExpressPkg/ArmVExpressPkg.dec
+++ b/Platform/ARM/VExpressPkg/ArmVExpressPkg.dec
@@ -10,7 +10,7 @@
 [Defines]
   DEC_SPECIFICATION  = 0x00010005
   PACKAGE_NAME   = ArmVExpressPkg
-  PACKAGE_GUID   = 9c0aaed4-74c5-4043-b417-a3223814ce76
+  PACKAGE_GUID   = 9F8DA3A6-3B84-44E4-9E36-E51972F6F688
   PACKAGE_VERSION= 0.1

 

@@ -29,7 +29,7 @@ [LibraryClasses]
   ArmPlatformSysConfigLib|Include/Library/ArmPlatformSysConfigLib.h

 [Guids.common]
-  gArmVExpressTokenSpaceGuid=  { 0x9c0aaed4, 0x74c5, 0x4043, { 0xb4, 0x17, 
0xa3, 0x22, 0x38, 0x14, 0xce, 0x76 } }
+  gArmVExpressTokenSpaceGuid=  { 0xD8D68B8E, 0x0ACE, 0x4942, { 0xA4, 0xB9, 
0x36, 0xEE, 0x9E, 0x80, 0x80, 0x9D } }

 [PcdsFeatureFlag.common]

--
2.25.1


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

[edk2-devel] [PATCH edk2-platforms v1 08/10] Platform/ARM: Fix Ecc error 3003

From: Pierre Gondois 

This patch fixes the following Ecc reported error:
A comparison of any pointer to zero must be done
via the NULL type

Signed-off-by: Pierre Gondois 
---
 Platform/ARM/JunoPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c | 4 ++--
 .../ARM/JunoPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c   | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Platform/ARM/JunoPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c 
b/Platform/ARM/JunoPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c
index ad7cc1cd75a1..6f2ce401b3cc 100644
--- a/Platform/ARM/JunoPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c
+++ b/Platform/ARM/JunoPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c
@@ -91,7 +91,7 @@ SataSiI3132Constructor (
   SATA_SI3132_INSTANCE*Instance;
   EFI_ATA_PASS_THRU_MODE  *AtaPassThruMode;

-  if (!SataSiI3132Instance) {
+  if (SataSiI3132Instance == NULL) {
 return EFI_INVALID_PARAMETER;
   }

@@ -223,7 +223,7 @@ SataSiI3132Initialization (
   UINTN Index;
   EFI_PCI_IO_PROTOCOL*  PciIo;

-  if (!SataSiI3132Instance) {
+  if (SataSiI3132Instance == NULL) {
 return EFI_INVALID_PARAMETER;
   }

diff --git a/Platform/ARM/JunoPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c 
b/Platform/ARM/JunoPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c
index f15b59788310..0ec53782835b 100644
--- a/Platform/ARM/JunoPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c
+++ b/Platform/ARM/JunoPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c
@@ -260,7 +260,7 @@ SiI3132AtaPassThruCommand (
 // Clear Command Complete
 SATA_PORT_WRITE32 (SataPort->RegBase + SII3132_PORT_INTSTATUS_REG, 
SII3132_PORT_INT_CMDCOMPL << 16);

-if (PciAllocMapping) {
+if (PciAllocMapping != NULL) {
   Status = PciIo->Unmap (PciIo, PciAllocMapping);
   ASSERT (!EFI_ERROR (Status));
 }
--
2.25.1


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

[edk2-devel] [PATCH edk2-platforms v1 07/10] Platform/ARM: Fix Ecc error 1012

From: Pierre Gondois 

This patch fixes the following Ecc reported error:
There should be no trailing white space in one line.

Signed-off-by: Pierre Gondois 
---
 Platform/ARM/JunoPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Platform/ARM/JunoPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c 
b/Platform/ARM/JunoPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
index a5a675b2a613..76084279a558 100644
--- a/Platform/ARM/JunoPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
+++ b/Platform/ARM/JunoPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
@@ -797,7 +797,7 @@ InstallAllStructures (
   // Fixup some table values
   mArmDefaultType0.Base.SystemBiosMajorRelease = (PcdGet32 ( 
PcdFirmwareRevision ) >> 16) & 0xFF;
   mArmDefaultType0.Base.SystemBiosMinorRelease = PcdGet32 ( 
PcdFirmwareRevision ) & 0xFF;
-  if ( JunoRevision == JUNO_REVISION_R1 )
+  if ( JunoRevision == JUNO_REVISION_R1 )
   {
 mArmDefaultType2.Base.Version = 6;
   }
--
2.25.1


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

[edk2-devel] [PATCH edk2-platforms v1 06/10] Platform/ARM: Fix Ecc error 1008

From: Pierre Gondois 

This patch fixes the following Ecc reported error:
File has invalid Non-ACSII char.

Signed-off-by: Pierre Gondois 
---
 .../Drivers/SmbiosPlatformDxe/Type17MemoryDevice.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type17MemoryDevice.c 
b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type17MemoryDevice.c
index b51e2b3fa1a6..90907bc19a8f 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type17MemoryDevice.c
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type17MemoryDevice.c
@@ -259,7 +259,7 @@ UpdateMemorySize (
   }

   /* Ref: SMBIOS Specifiation, Version 3.4.0, Document Identifier: DSP0134,
- Table 75 – Memory Device (Type 17) structure, description for Size field.
+ Table 75 - Memory Device (Type 17) structure, description for Size field.
  If the value is 0, no memory device is installed in the socket; if
  the size is unknown, the field value is h.
   */
@@ -276,13 +276,13 @@ UpdateMemorySize (
   }

   /* Ref: SMBIOS Specifiation, Version 3.4.0, Document Identifier: DSP0134,
- Table 75 – Memory Device (Type 17) structure, description for Size field.
+ Table 75 - Memory Device (Type 17) structure, description for Size field.
  If the size is 32 GB-1 MB or greater, the field value is 7FFFh and the
  actual size is stored in the Extended Size field.
   */
   if (MemorySize < (SIZE_32GB - SIZE_1MB)) {
 /* Ref: SMBIOS Specifiation, Version 3.4.0, Document Identifier: DSP0134,
-   section 7.18.5 Memory Device — Extended Size
+   section 7.18.5 Memory Device - Extended Size
For compatibility with older SMBIOS parsers, memory devices
smaller than (32 GB - 1 MB) should be represented using their
size in the Size field, leaving the Extended Size field set to 0.
@@ -290,7 +290,7 @@ UpdateMemorySize (
 Type17Table->ExtendedSize = 0;

 /* Ref: SMBIOS Specifiation, Version 3.4.0, Document Identifier: DSP0134,
-   Table 75 – Memory Device (Type 17) structure, description for Size 
field.
+   Table 75 - Memory Device (Type 17) structure, description for Size 
field.
The granularity in which the value is specified depends on the setting
of the most-significant bit (bit 15). If the bit is 0, the value is
specified in megabyte units; if the bit is 1, the value is specified
@@ -308,7 +308,7 @@ UpdateMemorySize (
   }

   /* Ref: SMBIOS Specifiation, Version 3.4.0, Document Identifier: DSP0134,
-  section 7.18.5 Memory Device — Extended Size
+  section 7.18.5 Memory Device - Extended Size
   The Extended Size field is intended to represent memory devices
   larger than 32,767 MB (32 GB - 1 MB), which cannot be described
   using the Size field. This field is only meaningful if the value
@@ -317,12 +317,12 @@ UpdateMemorySize (
   Type17Table->Size = 0x7FFF;

   /* Ref: SMBIOS Specifiation, Version 3.4.0, Document Identifier: DSP0134,
- section 7.18.5 Memory Device — Extended Size
+ section 7.18.5 Memory Device - Extended Size
  Bit 31 is reserved for future use and must be set to 0.
  Bits 30:0 represent the size of the memory device in megabytes.
  EXAMPLE: _8000h indicates a 32 GB memory device (32,768 MB),
   0002_h represents a 128 GB memory device (131,072 MB), and
-  _7FFFh represents a 32,767 MB (32 GB – 1 MB) device.
+  _7FFFh represents a 32,767 MB (32 GB - 1 MB) device.
   */
   Type17Table->ExtendedSize = (MemorySize >> 20) & (~BIT31);
   return EFI_SUCCESS;
--
2.25.1


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

[edk2-devel] [PATCH edk2-platforms v1 05/10] Platform/ARM: Fix Ecc error 1001

From: Pierre Gondois 

This patch fixes the following Ecc reported error:
'TAB' character is not allowed in source code,
please replace each 'TAB' with two spaces.

The patch also format and document the InstallStructures
function to comply to edk2 coding style.

Signed-off-by: Pierre Gondois 
---
 .../SmbiosPlatformDxe/SmbiosPlatformDxe.c | 38 ++-
 .../Drivers/Lan9118Dxe/Lan9118DxeUtil.h   |  4 +-
 2 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/Platform/ARM/JunoPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c 
b/Platform/ARM/JunoPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
index ac18b16ca676..a5a675b2a613 100644
--- a/Platform/ARM/JunoPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
+++ b/Platform/ARM/JunoPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
@@ -743,36 +743,40 @@ InstallMemoryStructure (
 }

 /**
-   Install a whole table worth of structructures
+  Install a whole table worth of structures.

-   @parm
+  @param [in] Smbios Smbios protocol.
+  @param [in] DefaultTables  Default Smbios tables.
+
+  @retval EFI_SUCCESS   Record was added.
+  @retval EFI_OUT_OF_RESOURCES  Record was not added.
+  @retval EFI_ALREADY_STARTED   The SmbiosHandle passed in was already in use.
 **/
 EFI_STATUS
 InstallStructures (
-   IN EFI_SMBIOS_PROTOCOL   *Smbios,
-   IN CONST VOID *DefaultTables[]
+   IN EFI_SMBIOS_PROTOCOL   *Smbios,
+   IN CONST VOID*DefaultTables[]
)
 {
 EFI_STATUSStatus = EFI_SUCCESS;
 EFI_SMBIOS_HANDLE SmbiosHandle;
+UINTN TableEntry;

-int TableEntry;
-for ( TableEntry=0; DefaultTables[TableEntry] != NULL; TableEntry++)
-{
-   SmbiosHandle = 
((EFI_SMBIOS_TABLE_HEADER*)DefaultTables[TableEntry])->Handle;
-   Status = Smbios->Add (
-   Smbios,
-   NULL,
-   &SmbiosHandle,
-   (EFI_SMBIOS_TABLE_HEADER*) DefaultTables[TableEntry]
-   );
-   if (EFI_ERROR(Status))
-   break;
+for (TableEntry = 0; DefaultTables[TableEntry] != NULL; TableEntry++) {
+  SmbiosHandle = 
((EFI_SMBIOS_TABLE_HEADER*)DefaultTables[TableEntry])->Handle;
+  Status = Smbios->Add (
+ Smbios,
+ NULL,
+ &SmbiosHandle,
+ (EFI_SMBIOS_TABLE_HEADER*)DefaultTables[TableEntry]
+ );
+  if (EFI_ERROR(Status)) {
+break;
+  }
 }
 return Status;
 }

-
 /**
Install all structures from the DefaultTables structure

diff --git a/Platform/ARM/VExpressPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.h 
b/Platform/ARM/VExpressPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.h
index 601714c94387..e50c458f8c3a 100644
--- a/Platform/ARM/VExpressPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.h
+++ b/Platform/ARM/VExpressPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.h
@@ -45,7 +45,7 @@ Lan9118RawMmioRead32(
   UINTN Delay
   );
 #define Lan9118MmioRead32(a) \
-   Lan9118RawMmioRead32(a, a ## _RD_DELAY)
+  Lan9118RawMmioRead32(a, a ## _RD_DELAY)

 UINT32
 Lan9118RawMmioWrite32(
@@ -54,7 +54,7 @@ Lan9118RawMmioWrite32(
   UINTN Delay
   );
 #define Lan9118MmioWrite32(a, v) \
-   Lan9118RawMmioWrite32(a, v, a ## _WR_DELAY)
+  Lan9118RawMmioWrite32(a, v, a ## _WR_DELAY)

 /* -- MAC CSR Access --- */

--
2.25.1


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

[edk2-devel] [PATCH edk2-platforms v1 03/10] Platform/ARM: Modify duplicated GUID in ArmVExpressLibSec

From: Pierre Gondois 

The two following modules have the same GUID:
- Platform/ARM/VExpressPkg/Library/
  ArmVExpressLibRTSM/ArmVExpressLibSec.inf
- Silicon/Hisilicon/Library/
  ArmPlatformLibHisilicon/ArmPlatformLibSec.inf

The inf file in the Platform/ARM/ folder is based
on the one in Silicon/Hisilicon/. Modify the one
in the Silicon/Hisilicon/.

Signed-off-by: Pierre Gondois 
---
 .../Library/ArmPlatformLibHisilicon/ArmPlatformLibSec.inf   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/Silicon/Hisilicon/Library/ArmPlatformLibHisilicon/ArmPlatformLibSec.inf 
b/Silicon/Hisilicon/Library/ArmPlatformLibHisilicon/ArmPlatformLibSec.inf
index ac587deedfd8..7fd7b5183e5b 100644
--- a/Silicon/Hisilicon/Library/ArmPlatformLibHisilicon/ArmPlatformLibSec.inf
+++ b/Silicon/Hisilicon/Library/ArmPlatformLibHisilicon/ArmPlatformLibSec.inf
@@ -12,7 +12,7 @@
 [Defines]
   INF_VERSION= 0x00010005
   BASE_NAME  = ArmPlatformLibSec
-  FILE_GUID  = a79eed97-4b98-4974-9690-37b32d6a5b56
+  FILE_GUID  = ABF3B82B-892F-438F-901F-F148C2DF89E6
   MODULE_TYPE= BASE
   VERSION_STRING = 1.0
   LIBRARY_CLASS  = ArmPlatformLib
--
2.25.1


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

[edk2-devel] [PATCH edk2-platforms v1 02/10] Platform/ARM: Modify duplicated GUID in ArmVExpressLib

From: Pierre Gondois 

The two following modules have the same GUID:
- Platform/ARM/VExpressPkg/Library/
  ArmVExpressLibRTSM/ArmVExpressLib.inf
- Platform/ARM/VExpressPkg/Library/
  ArmVExpressLibCTA15-A7/ArmVExpressLib.inf

Both files were imported at the same time. Modify the
GUID of the ArmVExpressLibRTSM/ArmVExpressLib.inf.

Signed-off-by: Pierre Gondois 
---
 .../VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf 
b/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf
index 929dadc28a84..4b582fdfc945 100644
--- a/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf
+++ b/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf
@@ -8,7 +8,7 @@
 [Defines]
   INF_VERSION= 0x00010005
   BASE_NAME  = RTSMArmVExpressLib
-  FILE_GUID  = b98a6cb7-d472-4128-ad62-a7347f85ce13
+  FILE_GUID  = BCD1958C-B7D1-48E9-B7F9-5ED25F5EBE0B
   MODULE_TYPE= BASE
   VERSION_STRING = 1.0
   LIBRARY_CLASS  = ArmPlatformLib
--
2.25.1


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

[edk2-devel] [PATCH edk2-platforms v1 00/10] Fix Arm platforms CI errors

From: Pierre Gondois 

Running the edk2 CI locally raised some warnings. Fix them.
The patches also aim to enable a CI for the edk2-platforms
repository.

The changes can be seen 
at:https://github.com/PierreARM/edk2-platforms/tree/1957_Fix_Arm_platforms_CI_errors_v1

Pierre Gondois (10):
  Platform/ARM: Modify duplicated GUID in PciHostBridgeLib
  Platform/ARM: Modify duplicated GUID in ArmVExpressLib
  Platform/ARM: Modify duplicated GUID in ArmVExpressLibSec
  Platform/ARM: Modify duplicated GUID in ArmVExpressPkg.dec
  Platform/ARM: Fix Ecc error 1001
  Platform/ARM: Fix Ecc error 1008
  Platform/ARM: Fix Ecc error 1012
  Platform/ARM: Fix Ecc error 3003
  Platform/ARM: Fix Ecc error 9005
  Platform/ARM: Fix Ecc error 5005

 .../Drivers/SataSiI3132Dxe/SataSiI3132.c  |  4 +-
 .../SataSiI3132Dxe/SiI3132AtaPassThru.c   |  5 ++-
 .../SmbiosPlatformDxe/SmbiosPlatformDxe.c | 40 ++-
 .../PciHostBridgeLib/PciHostBridgeLibFvp.inf  |  2 +-
 .../SmbiosPlatformDxe/Type17MemoryDevice.c| 14 +++
 Platform/ARM/VExpressPkg/ArmVExpressPkg.dec   |  4 +-
 .../Drivers/Lan9118Dxe/Lan9118DxeUtil.h   |  4 +-
 .../ArmVExpressLibRTSM/ArmVExpressLib.inf |  2 +-
 .../ArmVExpressPciHostBridgeLib.c |  2 +-
 .../ArmPlatformLibSec.inf |  2 +-
 10 files changed, 42 insertions(+), 37 deletions(-)

--
2.25.1


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

[edk2-devel] [PATCH edk2-platforms v1 01/10] Platform/ARM: Modify duplicated GUID in PciHostBridgeLib

From: Pierre Gondois 

The two following modules have the same GUID:
- Platform/ARM/Morello/Library/PciHostBridgeLib/
  PciHostBridgeLibFvp.inf
- Platform/ARM/SgiPkg/Library/PciHostBridgeLib/
  PciHostBridgeLib.inf

The module in the SgiPkg was created first. Update the GUID
of the module in the Morello folder.

Signed-off-by: Pierre Gondois 
---
 .../Morello/Library/PciHostBridgeLib/PciHostBridgeLibFvp.inf| 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/Platform/ARM/Morello/Library/PciHostBridgeLib/PciHostBridgeLibFvp.inf 
b/Platform/ARM/Morello/Library/PciHostBridgeLib/PciHostBridgeLibFvp.inf
index 1d6c5b01d13d..8524b8572af8 100644
--- a/Platform/ARM/Morello/Library/PciHostBridgeLib/PciHostBridgeLibFvp.inf
+++ b/Platform/ARM/Morello/Library/PciHostBridgeLib/PciHostBridgeLibFvp.inf
@@ -10,7 +10,7 @@
 [Defines]
   INF_VERSION= 0x0001001B
   BASE_NAME  = PciHostBridgeLib
-  FILE_GUID  = 6879CEAD-DC94-42EB-895C-096D36B8083C
+  FILE_GUID  = 972E5394-AF1D-45FE-8DC4-F9E5C896E04D
   MODULE_TYPE= DXE_DRIVER
   VERSION_STRING = 1.0
   LIBRARY_CLASS  = PciHostBridgeLib|DXE_DRIVER
--
2.25.1


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

Re: [edk2-devel] [PATCH V3 15/29] OvmfPkg: Update SecEntry.nasm to support Tdx

2021-11-24 Thread Yao, Jiewen

One more clarification: My comment below is only applicable for the TDVF 
platform, but not applicable to a general platform including OVMF.

In TDVF, Feature X* is a very small set, but in OVMF or general platform, 
Feature X* is a large set.

For example, if a platform need support S3 resume, Recovery, Capsule Update, 
then I won't recommend to remove PEI.
The reason is 2) the delta of risk becomes high then. Current PEI already 
provides a mature (and complex) infrastructure for them.
Moving those feature to somewhere else means to carry the burden to reinvent 
the infrastructure for S3, recovery, capsule update.

Here, I only recommend to remove for TDVF config-B, because the Feature X* is 
so simple that it could be moved to SEC without extra risk.

Removing PEI for general OVMF is a different topic. I don’t want to discuss in 
this thread.

Thank you
Yao Jiewen

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Yao, Jiewen
> Sent: Wednesday, November 24, 2021 11:00 PM
> To: devel@edk2.groups.io; j...@linux.ibm.com; Gerd Hoffmann
> 
> Cc: Xu, Min M ; Ard Biesheuvel
> ; Justen, Jordan L ;
> Brijesh Singh ; Erdem Aktas
> ; Tom Lendacky 
> Subject: Re: [edk2-devel] [PATCH V3 15/29] OvmfPkg: Update SecEntry.nasm to
> support Tdx
> 
> OK. Got it.
> Let me explain it in more detail.
> 
> Let's assume PEI phase include 3 major classes {PEI Core, PEI Arch PEM*,
> Feature X*}. * means 0~multiple.
> To all of us what really matter is Feature X, the existence of PEI Core + PEI 
> Arch
> PEIM* is to support Feature X*.
> 
> From architecture perspective, if a platform is complex (e.g. there are lots 
> of
> Feature X*) and feature X* have lots of inter-dependency, then PEI is a good
> place to coordinate the Feature X*. (Example, Feature X* are memory init and
> silicon init)
> But if a platform simple (e.g. there is only few Feature X*) and feature X* 
> have
> no much dependency, the including PEI does not bring too much value. That is
> why you see multiple platforms in EDKII does not include PEI.
> 
> From security perspective, Feature X* shall always perform check, no matter
> where the feature X sits in SEC, PEI or DXE. The risk of Feature X always 
> exists,
> no matter where the feature X sits in SEC, PEI or DXE. I completely agree.
> At same time, the PEI Core + PEI Arch PEIM* also bring unknown security risk.
> That was TRUE in history. It did happen. So my motivation to remove PEI phase
> is to reduce the risk introduced by PEI Core + PEI Arch PEIM*. Again, I do not
> mean to reduce the risk introduced by Feature X.
> 
> Now it seems we are really debating two things: (please correct me if I am
> wrong)
> 1) What is risk introduced by PEI Core + PEI Arch PEIM* ?
> 2) What is the delta of risk by moving Feature X from PEI to other place (SEC 
> or
> DXE) ?
> 
> For 1), my answer is that the risk is definitely bigger than zero, based upon
> history data. (This is an objective answer.) That is the main of my 
> motivation to
> make it become zero by removing PEI.
> For 2), my answer is that the delta is almost 0, based upon my experience. (I
> admit this is a subjective answer, because I cannot prove.). We are trying our
> best to reduce the risk of the Feature A* as well. Assuming delta of risk <= 
> risk,
> then it will become very smaller.
> 
> So, my judgement is by removing PEI, we can reduce the risk introduce by PEI
> Core + PEI Arch PEIM*. Reducing code == Reducing Security Risk.
> Also, this gives us a chance to focus on reviewing Feature X itself, instead 
> of the
> complex interaction with PEI Core + PEI Arch PEIM*. Reducing complexity ==
> Reducing Security Risk.
> (In history, we got lots of complain on the complexity of the 
> non-deterministic
> flow by CALLBACK and NOTIFY function in Core. A feature developer might not
> have idea on when the code will be called, and what the system status is at 
> that
> moment.)
> 
> 
> Thank you
> Yao Jiewen
> 
> 
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of James
> > Bottomley
> > Sent: Wednesday, November 24, 2021 10:07 PM
> > To: Yao, Jiewen ; devel@edk2.groups.io; Gerd
> > Hoffmann 
> > Cc: Xu, Min M ; Ard Biesheuvel
> > ; Justen, Jordan L ;
> > Brijesh Singh ; Erdem Aktas
> > ; Tom Lendacky 
> > Subject: Re: [edk2-devel] [PATCH V3 15/29] OvmfPkg: Update SecEntry.nasm
> to
> > support Tdx
> >
> > On Wed, 2021-11-24 at 14:03 +, Yao, Jiewen wrote:
> > > James
> > > I am sorry that it is hard for me to understand your point.
> > >
> > > To be honest, I am not sure what is objective on the discussion.
> > > Are you question the general threat model analysis on UEFI PI
> > > architecture?
> >
> > The object is for me to understand why you think eliminating PEI
> > improves security because I think it moves it in the opposite
> > direction.
> >
> > > Or are you trying to persuade me we should include PEI in TDVF,
> > > because you think it is safer to add code in PEI ?
> > > Or something else?
> >

回复: [edk2-devel][PATCH] BaseTools: Increase the DevicePath length for support more PCD value.

2021-11-24 Thread gaoliming

Xiaolu:
  BaseTools C source can directly use std libc API. You can use strlen() to
get the length of the input ascii string. 

  With this change, Reviewed-by: Liming Gao 

Thanks
Liming
> -邮件原件-
> 发件人: devel@edk2.groups.io  代表 Xiaolu.Jiang
> 发送时间: 2021年11月16日 9:42
> 收件人: devel@edk2.groups.io
> 抄送: Xiaolu.Jiang ; Bob Feng
> ; Liming Gao ; Yuwei
> Chen 
> 主题: [edk2-devel][PATCH] BaseTools: Increase the DevicePath length for
> support more PCD value.
> 
> Currently the PCD Value only support 13 Guid,When use more 13 pcd will
> cause the build tool fail,
> Need calculate the required memory,then allocate it.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3718
> 
> Cc: Bob Feng 
> Cc: Liming Gao 
> Cc: Yuwei Chen 
> 
> Signed-off-by: Xiaolu Jiang 
> ---
>  BaseTools/Source/C/DevicePath/DevicePath.c | 36
> +-
>  1 file changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/BaseTools/Source/C/DevicePath/DevicePath.c
> b/BaseTools/Source/C/DevicePath/DevicePath.c
> index c4d224ed61..8a0f5f4e2a 100644
> --- a/BaseTools/Source/C/DevicePath/DevicePath.c
> +++ b/BaseTools/Source/C/DevicePath/DevicePath.c
> @@ -144,6 +144,40 @@ Returns:
>*UniString = '\0';
> 
>  }
> 
> 
> 
> +
> 
> +/**
> 
> +  Returns the length of a Null-terminated ASCII string.
> 
> +
> 
> +  This function returns the number of ASCII characters in the
> Null-terminated
> 
> +  ASCII string specified by String.
> 
> +
> 
> +  If Length > 0 and Destination is NULL, then ASSERT().
> 
> +  If Length > 0 and Source is NULL, then ASSERT().
> 
> +  If PcdMaximumAsciiStringLength is not zero and String contains more
than
> 
> +  PcdMaximumAsciiStringLength ASCII characters, not including the
> Null-terminator,
> 
> +  then ASSERT().
> 
> +
> 
> +  @param  String  A pointer to a Null-terminated ASCII string.
> 
> +
> 
> +  @return The length of String.
> 
> +
> 
> +**/
> 
> +UINTN
> 
> +EFIAPI
> 
> +AsciiStrLen (
> 
> +  IN  CONST CHAR8   *String
> 
> +  )
> 
> +{
> 
> +  UINTN Length;
> 
> +
> 
> +  ASSERT (String != NULL);
> 
> +
> 
> +  for (Length = 0; *String != '\0'; String++, Length++) {
> 
> +
> 
> +  }
> 
> +  return Length;
> 
> +}
> 
> +
> 
>  int main(int argc, CHAR8 *argv[])
> 
>  {
> 
>CHAR8 * Str;
> 
> @@ -170,7 +204,7 @@ int main(int argc, CHAR8 *argv[])
>  fprintf(stderr, "Invalid option value, Device Path can't be NULL");
> 
>  return STATUS_ERROR;
> 
>}
> 
> -  Str16 = (CHAR16 *)malloc(1024);
> 
> +  Str16 = (CHAR16 *)malloc((AsciiStrLen (Str) + 1) * sizeof (CHAR16));
> 
>if (Str16 == NULL) {
> 
>  fprintf(stderr, "Resource, memory cannot be allocated");
> 
>  return STATUS_ERROR;
> 
> --
> 2.30.2.windows.1
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#83769): https://edk2.groups.io/g/devel/message/83769
> Mute This Topic: https://groups.io/mt/87089392/4905953
> Group Owner: devel+ow...@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [gaolim...@byosoft.com.cn]
> -=-=-=-=-=-=
> 





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

Re: [edk2-devel] [PATCH V3 15/29] OvmfPkg: Update SecEntry.nasm to support Tdx

2021-11-24 Thread Yao, Jiewen

OK. Got it.
Let me explain it in more detail.

Let's assume PEI phase include 3 major classes {PEI Core, PEI Arch PEM*, 
Feature X*}. * means 0~multiple.
To all of us what really matter is Feature X, the existence of PEI Core + PEI 
Arch PEIM* is to support Feature X*.

From architecture perspective, if a platform is complex (e.g. there are lots of 
Feature X*) and feature X* have lots of inter-dependency, then PEI is a good 
place to coordinate the Feature X*. (Example, Feature X* are memory init and 
silicon init)
But if a platform simple (e.g. there is only few Feature X*) and feature X* 
have no much dependency, the including PEI does not bring too much value. That 
is why you see multiple platforms in EDKII does not include PEI.

From security perspective, Feature X* shall always perform check, no matter 
where the feature X sits in SEC, PEI or DXE. The risk of Feature X always 
exists, no matter where the feature X sits in SEC, PEI or DXE. I completely 
agree.
At same time, the PEI Core + PEI Arch PEIM* also bring unknown security risk. 
That was TRUE in history. It did happen. So my motivation to remove PEI phase 
is to reduce the risk introduced by PEI Core + PEI Arch PEIM*. Again, I do not 
mean to reduce the risk introduced by Feature X.

Now it seems we are really debating two things: (please correct me if I am 
wrong)
1) What is risk introduced by PEI Core + PEI Arch PEIM* ?
2) What is the delta of risk by moving Feature X from PEI to other place (SEC 
or DXE) ?

For 1), my answer is that the risk is definitely bigger than zero, based upon 
history data. (This is an objective answer.) That is the main of my motivation 
to make it become zero by removing PEI.
For 2), my answer is that the delta is almost 0, based upon my experience. (I 
admit this is a subjective answer, because I cannot prove.). We are trying our 
best to reduce the risk of the Feature A* as well. Assuming delta of risk <= 
risk, then it will become very smaller.

So, my judgement is by removing PEI, we can reduce the risk introduce by PEI 
Core + PEI Arch PEIM*. Reducing code == Reducing Security Risk.
Also, this gives us a chance to focus on reviewing Feature X itself, instead of 
the complex interaction with PEI Core + PEI Arch PEIM*. Reducing complexity == 
Reducing Security Risk.
(In history, we got lots of complain on the complexity of the non-deterministic 
flow by CALLBACK and NOTIFY function in Core. A feature developer might not 
have idea on when the code will be called, and what the system status is at 
that moment.)

Thank you
Yao Jiewen

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of James
> Bottomley
> Sent: Wednesday, November 24, 2021 10:07 PM
> To: Yao, Jiewen ; devel@edk2.groups.io; Gerd
> Hoffmann 
> Cc: Xu, Min M ; Ard Biesheuvel
> ; Justen, Jordan L ;
> Brijesh Singh ; Erdem Aktas
> ; Tom Lendacky 
> Subject: Re: [edk2-devel] [PATCH V3 15/29] OvmfPkg: Update SecEntry.nasm to
> support Tdx
> 
> On Wed, 2021-11-24 at 14:03 +, Yao, Jiewen wrote:
> > James
> > I am sorry that it is hard for me to understand your point.
> >
> > To be honest, I am not sure what is objective on the discussion.
> > Are you question the general threat model analysis on UEFI PI
> > architecture?
> 
> The object is for me to understand why you think eliminating PEI
> improves security because I think it moves it in the opposite
> direction.
> 
> > Or are you trying to persuade me we should include PEI in TDVF,
> > because you think it is safer to add code in PEI ?
> > Or something else?
> >
> > Please enlighten me that.
> 
> Somewhere a decision was taken to remove PEI from the OVMF that is used
> to bring up TDX on the grounds of "improving security".  I'm struggling
> to understand the rationale for this.
> 
> James
> 
> 
> 
> 
> 
> 

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

Re: [edk2-devel] [PATCH V3 15/29] OvmfPkg: Update SecEntry.nasm to support Tdx

2021-11-24 Thread James Bottomley

On Wed, 2021-11-24 at 14:03 +, Yao, Jiewen wrote:
> James
> I am sorry that it is hard for me to understand your point.
> 
> To be honest, I am not sure what is objective on the discussion.
> Are you question the general threat model analysis on UEFI PI
> architecture?

The object is for me to understand why you think eliminating PEI
improves security because I think it moves it in the opposite
direction.

> Or are you trying to persuade me we should include PEI in TDVF,
> because you think it is safer to add code in PEI ?
> Or something else?
> 
> Please enlighten me that.

Somewhere a decision was taken to remove PEI from the OVMF that is used
to bring up TDX on the grounds of "improving security".  I'm struggling
to understand the rationale for this.

James




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

Re: [edk2-devel] [PATCH V3 15/29] OvmfPkg: Update SecEntry.nasm to support Tdx

2021-11-24 Thread James Bottomley

On Wed, 2021-11-24 at 11:08 +, Yao, Jiewen wrote:
> > -Original Message-
> > From: Gerd Hoffmann 
[...]
> > There isn't much external input to process in PEI phase.  Virtual
> > machines are a bit different than physical machines.  They need to
> > process some input from the host here which describes the virtual
> > hardware so they can initialize it properly.  For example parse the
> > etc/e820 fw_cfg file to figure how much memory is installed (or
> > parse the td hob in case tdx is used).
> > 
> > That platform-specific code for virtual machine initialization must
> > do careful sanity checking when you don't want trust the VMM of
> > course. Whenever that code lives in SEC or PEI doesn't change the
> > picture much though.

I don't disagree on this because we don't have a rom root of trust in
OVMF ... however it matters for most of the rest of edk2

> [Jiewen] I am not sure what information you are trying to convey.
> I never said that TDVF don’t need check the input after remove PEI.
> The check is always needed, no matter in PEI or SEC. I totally agree.
> 
> However, what I want to say is PEI has much more unnecessary code
> than SEC.
> You never know the quality of the those unnecessary code. Maybe it
> has a security bug, but it is just not triggered.
> For example, can you guarantee PEI code has not bug? Or DXE IPL has
> not bug?

Code removal to thin down the image is orthogonal to the location of
that code ... I don't think anyone objected to securing the path
through PEI by reducing unnecessary code; the doubt is that the
elimination of PEI somehow improves security.

> 
> What I did is a process of risk management - if PEI is removed, I
> don’t need care the risk brought by PEI.

Well, as I said above, you can remove the unnecessary code in PEI (and
SEC and DXE).  However, once you've done that, you don't eliminate risk
by removing PEI because all you do is move the necessary code that was
in PEI to somewhere else.  If you move it to SEC, I agree with Gerd
that it doesn't alter the security position much because SEC is a low
exposure domain like PEI.  If you move it to DXE it actually decreases
your security measurably because DXE is a high exposure security
domain.

> Unless you can prove that the risk of PEI is zero, then the risk is
> there.

But you don't make it zero by eliminating PEI, you simply move the risk
elsewhere because the necessary code still has to execute.

> > > 2. "bugs in PEI code can't be used to exploit the system when it
> > > has transitioned to the DXE domain."
> > > [Jiewen] I disagree. A bug in PEI code may already modify the
> > > HOB, while the HOB is an architecture data input for DXE.
> > > If DXE relies on some malicious data from PEI, DXE might be
> > > exploited later.
> > 
> > Attacking PEI is harder though because the external input it
> > processes is limited when compared to DXE.
> > 
> > Once you are transitioned to DXE you can't call into PEI Modules
> > any more.
> > 
> > So, how would an attacker trick PEI code into modifying HOBs (or
> > carrying out other actions under attackers control)?
> 
> [Jiewen] I would disagree " Attacking PEI is harder though because
> the external input it processes is limited when compared to DXE " 
> First, the number of extern input != the difficulty of the attack.
> Second, the number of extern input != the severity of the
> consequence.

Attacking PEI is harder than attacking DXE because of the limited
exposure to external influences. It doesn't mean it can't be done but
it does mean exploiting the same code can be harder or impossible in
PEI compared to DXE.

The key point is there are some classes of bug that can't be exploited
in PEI because of the limited exposure.  These bugs can be exploited in
DXE because of the wider exposure.  This is why moving code from DXE to
PEI increases the risk.

> On how to trick PEI, if you search the public UEFI BIOS attack
> history in the web, you can find example.
> The attack simply used an integer overflow, to cause buffer overflow.
> And the buffer overflow can be used to inject shellcode, then gain
> the execution privilege.
> Finally, it can control the whole system.
> 
> Modifying HOB is not a difficult task, as long as there is proper
> vulnerability, being used to gain a memory write.
> 
> In security world, we always say: Even if you cannot do it, you
> cannot assume other people cannot do it.
> Unless you can prove it cannot be done. Otherwise, anything might be
> possible.
> 
> That is why people are in favor of crypto notation and formal
> verification to prove something is correct.

There are many ways of improving security.  Formal verification has its
place, but grows exponentially harder with the complexity of the
system.  Separation into multiple security domains is also a common
technique (which also facilitates formal verification because it
reduces the state space).  What I worry about is that elimination of
one of the UEFI security domains causes

Re: [edk2-devel] [PATCH v2 2/8] ArmPkg: PCD to select conduit for monitor calls

On Wed, Nov 24, 2021 at 14:07:22 +0100, Ard Biesheuvel wrote:
> On Wed, 24 Nov 2021 at 14:05, Leif Lindholm  wrote:
> >
> > On Wed, Nov 24, 2021 at 14:03:51 +0100, Ard Biesheuvel wrote:
> > > On Wed, 24 Nov 2021 at 13:07, Leif Lindholm  wrote:
> > > >
> > > > Ard - how does this interact with e.g. ArmVirtPsciResetSystemLib,
> > > > which reads its conduit out of the DT passed to it by QEMU?
> > > >
> > >
> > > As long as ArmCallSmc() and ArmCallHvc() continue to exist and behave
> > > as before, I don't think those libraries are affected.
> >
> > Well, I was thinking more that you can now end up with a firmware
> > build where the conduit is sometimes dynamically determined, and
> > sometimes compile-time determined.
> 
> There is nothing wrong with that by itself: if you have a DT that
> describes this, it makes sense to carry support for both. If the bare
> metal you are targetting only ever uses SMC, no need for the discovery
> and a PCD is fine.
> 
> Question remains whether this PCD could/should be configurable as
> dynamic, so we collapse the static/dynamic options into a single PCD
> based choice.

That would feel cleaner. Although not required for this set.

/
Leif


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84037): https://edk2.groups.io/g/devel/message/84037
Mute This Topic: https://groups.io/mt/87092744/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 2/8] ArmPkg: PCD to select conduit for monitor calls

2021-11-24 Thread Ard Biesheuvel

On Wed, 24 Nov 2021 at 14:05, Leif Lindholm  wrote:
>
> On Wed, Nov 24, 2021 at 14:03:51 +0100, Ard Biesheuvel wrote:
> > On Wed, 24 Nov 2021 at 13:07, Leif Lindholm  wrote:
> > >
> > > Ard - how does this interact with e.g. ArmVirtPsciResetSystemLib,
> > > which reads its conduit out of the DT passed to it by QEMU?
> > >
> >
> > As long as ArmCallSmc() and ArmCallHvc() continue to exist and behave
> > as before, I don't think those libraries are affected.
>
> Well, I was thinking more that you can now end up with a firmware
> build where the conduit is sometimes dynamically determined, and
> sometimes compile-time determined.
>

There is nothing wrong with that by itself: if you have a DT that
describes this, it makes sense to carry support for both. If the bare
metal you are targetting only ever uses SMC, no need for the discovery
and a PCD is fine.

Question remains whether this PCD could/should be configurable as
dynamic, so we collapse the static/dynamic options into a single PCD
based choice.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84036): https://edk2.groups.io/g/devel/message/84036
Mute This Topic: https://groups.io/mt/87092744/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 2/8] ArmPkg: PCD to select conduit for monitor calls

On Wed, Nov 24, 2021 at 14:03:51 +0100, Ard Biesheuvel wrote:
> On Wed, 24 Nov 2021 at 13:07, Leif Lindholm  wrote:
> >
> > Ard - how does this interact with e.g. ArmVirtPsciResetSystemLib,
> > which reads its conduit out of the DT passed to it by QEMU?
> >
> 
> As long as ArmCallSmc() and ArmCallHvc() continue to exist and behave
> as before, I don't think those libraries are affected.

Well, I was thinking more that you can now end up with a firmware
build where the conduit is sometimes dynamically determined, and
sometimes compile-time determined.

/
Leif

> > On Tue, Nov 16, 2021 at 11:32:54 +, Sami Mujawar wrote:
> > > Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)
> > >
> > > Define a PCD 'PcdMonitorConduitHvc' to select the conduit to use for
> > > monitor calls. PcdMonitorConduitHvc is defined as FALSE by default,
> > > meaning the SMC conduit is enabled as default.
> > >
> > > Adding PcdMonitorConduitHvc allows selection of HVC conduit to be used
> > > by virtual firmware implementations.
> > >
> > > Signed-off-by: Sami Mujawar 
> > > ---
> > >
> > > Notes:
> > > v2:
> > >  - No code change since v1. Re-sending with V2 series.   [SAMI]
> > >
> > >  ArmPkg/ArmPkg.dec | 5 +
> > >  1 file changed, 5 insertions(+)
> > >
> > > diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
> > > index 
> > > 9da1bbc9f2166dc8ae93f96a34d3165fffed34dc..9a53888ae52f00eec50e631cf1bfcacecf8bba87
> > >  100644
> > > --- a/ArmPkg/ArmPkg.dec
> > > +++ b/ArmPkg/ArmPkg.dec
> > > @@ -132,6 +132,11 @@ [PcdsFeatureFlag.common]
> > ># Define if the GICv3 controller should use the GICv2 legacy
> > >gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy|FALSE|BOOLEAN|0x0042
> > >
> > > +  ## Define the conduit to use for monitor calls.
> > > +  # Default PcdMonitorConduitHvc = FALSE, conduit = SMC
> > > +  # If PcdMonitorConduitHvc = TRUE, conduit = HVC
> > > +  gArmTokenSpaceGuid.PcdMonitorConduitHvc|FALSE|BOOLEAN|0x0047
> > > +
> > >  [PcdsFeatureFlag.ARM]
> > ># Whether to map normal memory as non-shareable. FALSE is the safe 
> > > choice, but
> > ># TRUE may be appropriate to fix performance problems if you don't 
> > > care about
> > > --
> > > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> > >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84035): https://edk2.groups.io/g/devel/message/84035
Mute This Topic: https://groups.io/mt/87092744/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 2/8] ArmPkg: PCD to select conduit for monitor calls

2021-11-24 Thread Ard Biesheuvel

On Wed, 24 Nov 2021 at 13:07, Leif Lindholm  wrote:
>
> Ard - how does this interact with e.g. ArmVirtPsciResetSystemLib,
> which reads its conduit out of the DT passed to it by QEMU?
>

As long as ArmCallSmc() and ArmCallHvc() continue to exist and behave
as before, I don't think those libraries are affected.

>
> On Tue, Nov 16, 2021 at 11:32:54 +, Sami Mujawar wrote:
> > Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)
> >
> > Define a PCD 'PcdMonitorConduitHvc' to select the conduit to use for
> > monitor calls. PcdMonitorConduitHvc is defined as FALSE by default,
> > meaning the SMC conduit is enabled as default.
> >
> > Adding PcdMonitorConduitHvc allows selection of HVC conduit to be used
> > by virtual firmware implementations.
> >
> > Signed-off-by: Sami Mujawar 
> > ---
> >
> > Notes:
> > v2:
> >  - No code change since v1. Re-sending with V2 series.   [SAMI]
> >
> >  ArmPkg/ArmPkg.dec | 5 +
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
> > index 
> > 9da1bbc9f2166dc8ae93f96a34d3165fffed34dc..9a53888ae52f00eec50e631cf1bfcacecf8bba87
> >  100644
> > --- a/ArmPkg/ArmPkg.dec
> > +++ b/ArmPkg/ArmPkg.dec
> > @@ -132,6 +132,11 @@ [PcdsFeatureFlag.common]
> ># Define if the GICv3 controller should use the GICv2 legacy
> >gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy|FALSE|BOOLEAN|0x0042
> >
> > +  ## Define the conduit to use for monitor calls.
> > +  # Default PcdMonitorConduitHvc = FALSE, conduit = SMC
> > +  # If PcdMonitorConduitHvc = TRUE, conduit = HVC
> > +  gArmTokenSpaceGuid.PcdMonitorConduitHvc|FALSE|BOOLEAN|0x0047
> > +
> >  [PcdsFeatureFlag.ARM]
> ># Whether to map normal memory as non-shareable. FALSE is the safe 
> > choice, but
> ># TRUE may be appropriate to fix performance problems if you don't care 
> > about
> > --
> > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84034): https://edk2.groups.io/g/devel/message/84034
Mute This Topic: https://groups.io/mt/87092744/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 3/8] ArmPkg: Add Arm Firmware TRNG library

Hi Sami,

On Tue, Nov 16, 2021 at 11:32:55 +, Sami Mujawar wrote:
> Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)
> 
> The Arm True Random Number Generator Firmware, Interface 1.0,
> Platform Design Document
> (https://developer.arm.com/documentation/den0098/latest/)
> defines an interface between an Operating System (OS) executing
> at EL1 and Firmware (FW) exposing a conditioned entropy source
> that is provided by a TRNG back end.
> 
> The conditioned entropy, that is provided by the TRNG FW interface,
> is commonly used to seed deterministic random number generators.
> 
> This patch adds a TrngLib library that implements the Arm TRNG
> firmware interface.
> 
> Signed-off-by: Sami Mujawar 
> ---
> 
> Notes:
> v2:
>  - MdePkg\Include\Library\TrngLib.h is base type [LIMING]
>library. It can use RETURN_STATUS instead of
>EFI_STATUS.
>  - Replaced EFI_STATUS with RETURN_STATUS.   [SAMI]
>  - MdePkg\Include\Library\TrngLib.h API parameter[LIMING]
>doesn't require CONST. CONST means the value
>specified by the input pointer will not be
>changed in API implementation.
>  - Removed the use of constant pointers in the   [SAMI]
>TRNG API.
> 
>  ArmPkg/ArmPkg.dsc|   1 +
>  ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h  |  64 +++
>  ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c   | 483 
>  ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf |  34 ++
>  4 files changed, 582 insertions(+)
> 
> diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
> index 
> 59fd8f295d4f614cc68ee1021e691f94e279ab81..23df68c5eb53df11de5d96bde4949f3c833c9b2c
>  100644
> --- a/ArmPkg/ArmPkg.dsc
> +++ b/ArmPkg/ArmPkg.dsc
> @@ -156,6 +156,7 @@ [Components.common]
>ArmPkg/Drivers/ArmCrashDumpDxe/ArmCrashDumpDxe.inf
>ArmPkg/Drivers/ArmScmiDxe/ArmScmiDxe.inf
>  
> +  ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf
>ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClassDxe.inf
>ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
>ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLibNull.inf
> diff --git a/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h 
> b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h
> new file mode 100644
> index 
> ..42236e743d972df0df205b1565496afeff5785f3
> --- /dev/null
> +++ b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngDefs.h
> @@ -0,0 +1,64 @@
> +/** @file
> +  Arm Firmware TRNG definitions.
> +
> +  Copyright (c) 2021, Arm Limited. All rights reserved.
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +  @par Reference(s):
> +  - [1] Arm True Random Number Generator Firmware, Interface 1.0,
> +Platform Design Document.
> +(https://developer.arm.com/documentation/den0098/latest/)
> +
> +  @par Glossary:
> +- TRNG - True Random Number Generator
> +- FID  - Function ID
> +**/
> +
> +#ifndef ARM_FW_TRNG_DEFS_H_
> +#define ARM_FW_TRNG_DEFS_H_
> +
> +// Firmware TRNG interface Function IDs
> +#define FID_TRNG_VERSION  0x8450
> +#define FID_TRNG_FEATURES 0x8451
> +#define FID_TRNG_GET_UUID 0x8452
> +#define FID_TRNG_RND_AARCH32  0x8453
> +#define FID_TRNG_RND_AARCH64  0xC453

Do these belong in ArmStdSmc.h?

> +
> +// Firmware TRNG revision mask and shift
> +#define TRNG_REV_MAJOR_MASK   0x7FFF
> +#define TRNG_REV_MINOR_MASK   0x
> +#define TRNG_REV_MAJOR_SHIFT  16
> +#define TRNG_REV_MINOR_SHIFT  0
> +
> +// Firmware TRNG status codes
> +#define TRNG_STATUS_SUCCESS (INT32)(0)
> +#define TRNG_NOT_SUPPORTED  (INT32)(-1)
> +#define TRNG_INVALID_PARAMETER  (INT32)(-2)
> +#define TRNG_NO_ENTROPY (INT32)(-3)

And the rest of the stuff to here, really?

> +#if defined (MDE_CPU_ARM)
> +/** FID to use on AArch32 platform to request entropy.
> +*/
> +#define FID_TRNG_RNDFID_TRNG_RND_AARCH32
> +
> +/** Maximum bits of entropy supported on AArch32.
> +*/
> +#define MAX_ENTROPY_BITS96
> +#elif defined (MDE_CPU_AARCH64)
> +/** FID to use on AArch64 platform to request entropy.
> +*/
> +#define FID_TRNG_RNDFID_TRNG_RND_AARCH64
> +
> +/** Maximum bits of entropy supported on AArch64.
> +*/
> +#define MAX_ENTROPY_BITS192
> +#else
> +#error "Firmware TRNG not supported. Unknown chipset."
> +#endif
> +
> +/** Typedef for SMC or HVC arguments.
> +*/
> +typedef ARM_SMC_ARGS  ARM_MONITOR_ARGS;
> +
> +#endif // ARM_FW_TRNG_DEFS_H_
> diff --git a/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c 
> b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c
> new file mode 100644
> index 
> ..314e7ffbc232ae90bbb77306f9c7113ce63012c8
> --- /dev/null
> +++ b/ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.c
> @@ -0,0 +1,483 @@
> +/** @file
> +  Arm Firmware TRNG interface library.
> +
> +  Copyright (c) 2021, Arm Limited. All rights reserved.
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +  @par Reference(s):
> +  - [1

Re: [edk2-devel] [PATCH v2 2/8] ArmPkg: PCD to select conduit for monitor calls

Ard - how does this interact with e.g. ArmVirtPsciResetSystemLib,
which reads its conduit out of the DT passed to it by QEMU?

/
Leif

On Tue, Nov 16, 2021 at 11:32:54 +, Sami Mujawar wrote:
> Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)
> 
> Define a PCD 'PcdMonitorConduitHvc' to select the conduit to use for
> monitor calls. PcdMonitorConduitHvc is defined as FALSE by default,
> meaning the SMC conduit is enabled as default.
> 
> Adding PcdMonitorConduitHvc allows selection of HVC conduit to be used
> by virtual firmware implementations.
> 
> Signed-off-by: Sami Mujawar 
> ---
> 
> Notes:
> v2:
>  - No code change since v1. Re-sending with V2 series.   [SAMI]
> 
>  ArmPkg/ArmPkg.dec | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
> index 
> 9da1bbc9f2166dc8ae93f96a34d3165fffed34dc..9a53888ae52f00eec50e631cf1bfcacecf8bba87
>  100644
> --- a/ArmPkg/ArmPkg.dec
> +++ b/ArmPkg/ArmPkg.dec
> @@ -132,6 +132,11 @@ [PcdsFeatureFlag.common]
># Define if the GICv3 controller should use the GICv2 legacy
>gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy|FALSE|BOOLEAN|0x0042
>  
> +  ## Define the conduit to use for monitor calls.
> +  # Default PcdMonitorConduitHvc = FALSE, conduit = SMC
> +  # If PcdMonitorConduitHvc = TRUE, conduit = HVC
> +  gArmTokenSpaceGuid.PcdMonitorConduitHvc|FALSE|BOOLEAN|0x0047
> +
>  [PcdsFeatureFlag.ARM]
># Whether to map normal memory as non-shareable. FALSE is the safe choice, 
> but
># TRUE may be appropriate to fix performance problems if you don't care 
> about
> -- 
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> 


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

Re: [edk2-devel] [Patch 03/12] ArmVirtPkg: Update YAML to ignore specific ECC files/errors

On Tue, Nov 23, 2021 at 13:21:13 -0800, Michael D Kinney wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3749
> 
> Update package YAML files to ignore ECC errors that are
> already present.  These issues must be fixed in the future,
> but should not block source code changes for these known
> issues.
> 
> Cc: Leif Lindholm 
> Cc: Ard Biesheuvel 
> Cc: Sami Mujawar 
> Cc: Gerd Hoffmann 
> Cc: Sean Brogan 
> Cc: Bret Barkelew 
> Cc: Liming Gao 
> Cc: Michael Kubacki 
> Signed-off-by: Michael D Kinney 

Reviewed-by: Leif Lindholm 

> ---
>  ArmVirtPkg/ArmVirtPkg.ci.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/ArmVirtPkg/ArmVirtPkg.ci.yaml b/ArmVirtPkg/ArmVirtPkg.ci.yaml
> index f80500ab53b0..67b0e9594f3e 100644
> --- a/ArmVirtPkg/ArmVirtPkg.ci.yaml
> +++ b/ArmVirtPkg/ArmVirtPkg.ci.yaml
> @@ -24,6 +24,7 @@
>  ],
>  ## Both file path and directory path are accepted.
>  "IgnoreFiles": [
> +"Library/PlatformBootManagerLib/PlatformBm.c"
>  ]
>  },
>  ## options defined .pytool/Plugin/CompilerPlugin
> -- 
> 2.32.0.windows.1
> 


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

Re: [edk2-devel] [Patch 02/12] ArmPlatformPkg: Update YAML to ignore specific ECC files/errors

On Tue, Nov 23, 2021 at 13:21:12 -0800, Michael D Kinney wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3749
> 
> Update package YAML files to ignore ECC errors that are
> already present.  These issues must be fixed in the future,
> but should not block source code changes for these known
> issues.
> 
> Cc: Leif Lindholm 
> Cc: Ard Biesheuvel 
> Cc: Sean Brogan 
> Cc: Bret Barkelew 
> Cc: Liming Gao 
> Cc: Michael Kubacki 
> Signed-off-by: Michael D Kinney 

Reviewed-by: Leif Lindholm 


> ---
>  ArmPlatformPkg/ArmPlatformPkg.ci.yaml | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/ArmPlatformPkg/ArmPlatformPkg.ci.yaml 
> b/ArmPlatformPkg/ArmPlatformPkg.ci.yaml
> index 3dbbcc673bf0..03632e74df73 100644
> --- a/ArmPlatformPkg/ArmPlatformPkg.ci.yaml
> +++ b/ArmPlatformPkg/ArmPlatformPkg.ci.yaml
> @@ -19,7 +19,8 @@
>  ],
>  ## Both file path and directory path are accepted.
>  "IgnoreFiles": [
> -"Scripts/Ds5/"
> +"Scripts/Ds5/",
> +"Drivers/PL061GpioDxe/PL061Gpio.c"
>  ]
>  },
>  
> -- 
> 2.32.0.windows.1
> 
> 
> 
> 
> 
> 


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

Re: [edk2-devel] [Patch 01/12] ArmPkg: Update YAML to ignore specific ECC files/errors

On Tue, Nov 23, 2021 at 13:21:11 -0800, Michael D Kinney wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3749
> 
> Update package YAML files to ignore ECC errors that are
> already present.  These issues must be fixed in the future,
> but should not block source code changes for these known
> issues.
> 
> Cc: Leif Lindholm 
> Cc: Ard Biesheuvel 
> Cc: Sean Brogan 
> Cc: Bret Barkelew 
> Cc: Liming Gao 
> Cc: Michael Kubacki 
> Signed-off-by: Michael D Kinney 

Reviewed-by: Leif Lindholm 


> ---
>  ArmPkg/ArmPkg.ci.yaml | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/ArmPkg/ArmPkg.ci.yaml b/ArmPkg/ArmPkg.ci.yaml
> index a0d6a75fe881..b7e07aaef675 100644
> --- a/ArmPkg/ArmPkg.ci.yaml
> +++ b/ArmPkg/ArmPkg.ci.yaml
> @@ -19,7 +19,10 @@
>  ],
>  ## Both file path and directory path are accepted.
>  "IgnoreFiles": [
> -"Library/ArmSoftFloatLib/berkeley-softfloat-3"
> +"Library/ArmSoftFloatLib/berkeley-softfloat-3",
> +"Library/ArmSoftFloatLib/ArmSoftFloatLib.c",
> +"Library/CompilerIntrinsicsLib",
> +"Universal/Smbios/SmbiosMiscDxe"
>  ]
>  },
>  
> -- 
> 2.32.0.windows.1
> 


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

Re: [edk2-devel] [PATCH V3 15/29] OvmfPkg: Update SecEntry.nasm to support Tdx

2021-11-24 Thread Yao, Jiewen



> -Original Message-
> From: Gerd Hoffmann 
> Sent: Wednesday, November 24, 2021 4:12 PM
> To: Yao, Jiewen 
> Cc: j...@linux.ibm.com; devel@edk2.groups.io; Xu, Min M
> ; Ard Biesheuvel ; Justen,
> Jordan L ; Brijesh Singh ;
> Erdem Aktas ; Tom Lendacky
> 
> Subject: Re: [edk2-devel] [PATCH V3 15/29] OvmfPkg: Update SecEntry.nasm to
> support Tdx
> 
>   Hi,
> 
> > 1. " the PEI domain has very limited exposure, it's the DXE domain that has 
> > full
> exposure "
> > [Jiewen] I don’t understand how that is concluded, on " limited exposure ", 
> > "
> full exposure ".
> 
> exposure == "the need to process external input, which an attacker might
> use to exploit bugs in edk2 by crafting input data accordingly."

[Jiewen] Thanks.

> 
> There isn't much external input to process in PEI phase.  Virtual
> machines are a bit different than physical machines.  They need to
> process some input from the host here which describes the virtual
> hardware so they can initialize it properly.  For example parse the
> etc/e820 fw_cfg file to figure how much memory is installed (or parse
> the td hob in case tdx is used).
> 
> That platform-specific code for virtual machine initialization must do
> careful sanity checking when you don't want trust the VMM of course.
> Whenever that code lives in SEC or PEI doesn't change the picture much
> though.

[Jiewen] I am not sure what information you are trying to convey.
I never said that TDVF don’t need check the input after remove PEI.
The check is always needed, no matter in PEI or SEC. I totally agree.

However, what I want to say is PEI has much more unnecessary code than SEC.
You never know the quality of the those unnecessary code. Maybe it has a 
security bug, but it is just not triggered.
For example, can you guarantee PEI code has not bug? Or DXE IPL has not bug?

What I did is a process of risk management - if PEI is removed, I don’t need 
care the risk brought by PEI.
Unless you can prove that the risk of PEI is zero, then the risk is there.



> 
> > 2. "bugs in PEI code can't be used to exploit the system when it has
> transitioned to the DXE domain."
> > [Jiewen] I disagree. A bug in PEI code may already modify the HOB, while the
> HOB is an architecture data input for DXE.
> > If DXE relies on some malicious data from PEI, DXE might be exploited later.
> 
> Attacking PEI is harder though because the external input it
> processes is limited when compared to DXE.
> 
> Once you are transitioned to DXE you can't call into PEI Modules
> any more.
> 
> So, how would an attacker trick PEI code into modifying HOBs (or
> carrying out other actions under attackers control)?

[Jiewen] I would disagree " Attacking PEI is harder though because the external 
input it processes is limited when compared to DXE " 
First, the number of extern input != the difficulty of the attack.
Second, the number of extern input != the severity of the consequence.

On how to trick PEI, if you search the public UEFI BIOS attack history in the 
web, you can find example.
The attack simply used an integer overflow, to cause buffer overflow.
And the buffer overflow can be used to inject shellcode, then gain the 
execution privilege.
Finally, it can control the whole system.

Modifying HOB is not a difficult task, as long as there is proper 
vulnerability, being used to gain a memory write.

In security world, we always say: Even if you cannot do it, you cannot assume 
other people cannot do it.
Unless you can prove it cannot be done. Otherwise, anything might be possible.

That is why people are in favor of crypto notation and formal verification to 
prove something is correct.




> 
> take care,
>   Gerd



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

Re: [edk2-devel] [Patch 10/12] StandaloneMmPkg: Update YAML to ignore specific ECC files/errors


Hi Mike,


On 23/11/2021 09:21 PM, Michael D Kinney wrote:

+"EccCheck": {
+## Exception sample looks like below:
+## "ExceptionList": [
+## "", ""
+## ]
+"ExceptionList": [
+"4002", "HobConstructor"
+],


I noticed there are other ECC errors with ID 4002 in StandaloneMmPkg.

e.g.

edk2\StandaloneMmPkg\Library\StandaloneMmCoreHobLib\Arm\StandaloneMmCoreHobLib.c(24): 
[4002]Function header doesn't exist Function [CreateHob] has NO comment 
immediately preceding it.
edk2\StandaloneMmPkg\Library\StandaloneMmCoreHobLib\Arm\StandaloneMmCoreHobLibInternal.c(24): 
[4002]Function header doesn't exist Function [HobConstructor] has NO 
comment immediately preceding it.
edk2\StandaloneMmPkg\Library\StandaloneMmHobLib\StandaloneMmHobLib.c(247): 
[4002]Function header doesn't exist Function [CreateHob] has NO comment 
immediately preceding it.
edk2\StandaloneMmPkg\Library\StandaloneMmPeCoffExtraActionLib\AArch64\StandaloneMmPeCoffExtraActionLib.c(26): 
[4002]Function header doesn't exist Function [UpdatePeCoffPermissions] 
has NO comment immediately preceding it.


I am trying to undestand how the ExceptionList works. Would Keyword = 
HobConstructor ignore the other errors too?


We will look into getting these issues fixed. However, to unblock this 
patch can proceed.


Reviewed-by: Sami Mujawar 

Regards,

Sami Mujawar



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

Re: [edk2-devel] [Patch 03/12] ArmVirtPkg: Update YAML to ignore specific ECC files/errors


Hi Mike,

Thank you for this patch.

Reviewed-by: Sami Mujawar 

Regards,

Sami Mujawar


On 23/11/2021 09:21 PM, Michael D Kinney wrote:

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

Update package YAML files to ignore ECC errors that are
already present.  These issues must be fixed in the future,
but should not block source code changes for these known
issues.

Cc: Leif Lindholm 
Cc: Ard Biesheuvel 
Cc: Sami Mujawar 
Cc: Gerd Hoffmann 
Cc: Sean Brogan 
Cc: Bret Barkelew 
Cc: Liming Gao 
Cc: Michael Kubacki 
Signed-off-by: Michael D Kinney 
---
  ArmVirtPkg/ArmVirtPkg.ci.yaml | 1 +
  1 file changed, 1 insertion(+)

diff --git a/ArmVirtPkg/ArmVirtPkg.ci.yaml b/ArmVirtPkg/ArmVirtPkg.ci.yaml
index f80500ab53b0..67b0e9594f3e 100644
--- a/ArmVirtPkg/ArmVirtPkg.ci.yaml
+++ b/ArmVirtPkg/ArmVirtPkg.ci.yaml
@@ -24,6 +24,7 @@
  ],
  ## Both file path and directory path are accepted.
  "IgnoreFiles": [
+"Library/PlatformBootManagerLib/PlatformBm.c"
  ]
  },
  ## options defined .pytool/Plugin/CompilerPlugin




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

Re: [edk2-devel] [Patch 02/12] ArmPlatformPkg: Update YAML to ignore specific ECC files/errors


Hi Mike,

Thank you for this patch.

Reviewed-by: Sami Mujawar 

Regards,

Sami Mujawar


On 23/11/2021 09:21 PM, Michael D Kinney via groups.io wrote:

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

Update package YAML files to ignore ECC errors that are
already present.  These issues must be fixed in the future,
but should not block source code changes for these known
issues.

Cc: Leif Lindholm 
Cc: Ard Biesheuvel 
Cc: Sean Brogan 
Cc: Bret Barkelew 
Cc: Liming Gao 
Cc: Michael Kubacki 
Signed-off-by: Michael D Kinney 
---
  ArmPlatformPkg/ArmPlatformPkg.ci.yaml | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ArmPlatformPkg/ArmPlatformPkg.ci.yaml 
b/ArmPlatformPkg/ArmPlatformPkg.ci.yaml
index 3dbbcc673bf0..03632e74df73 100644
--- a/ArmPlatformPkg/ArmPlatformPkg.ci.yaml
+++ b/ArmPlatformPkg/ArmPlatformPkg.ci.yaml
@@ -19,7 +19,8 @@
  ],
  ## Both file path and directory path are accepted.
  "IgnoreFiles": [
-"Scripts/Ds5/"
+"Scripts/Ds5/",
+"Drivers/PL061GpioDxe/PL061Gpio.c"
  ]
  },
  




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

Re: [edk2-devel] [Patch 01/12] ArmPkg: Update YAML to ignore specific ECC files/errors