Re: [edk2-devel] Tianocore community page on who we are - please review

[Yao, Jiewen]

Some other thought is about maintainer's role definition:

The role of a maintainer is to:

  1.  Maintainer assignments to packages and source file name patterns are 
provided in the 
"Maintainers.txt"
 file.
  2.  Subscribe to the "edk2-bugs" mailing list https://edk2.groups.io/g/bugs, 
which propagates TianoCore Bugzilla https://bugzilla.tianocore.org/ actions via 
email. Keep a close eye on new issues reported for their assigned packages. 
Participate in triaging and analyzing bugs filed for their assigned packages.
  3.  Responsible for reviewing patches and answering questions from 
contributors, on the edk2-devel mailing list https://edk2.groups.io/g/devel/.
  4.  Responsible for coordinating patch review with co-maintainers and 
reviewers of the same package.
  5.  Has push / merge access to the merge branch.
  6.  Responsible for merging approved patches into the master branch.
  7.  Follow the EDK II development 
process.

IMHO, the 1~4 need technical expertise, while 5~7 need process expertise.
Logically, the can be two separated roles and be done by two different persons.
A people who has strong technical expertise might NOT be the best person to do 
the integration, and vice versa. I hope we can let right person do right thing 
in right way.
For example, to avoid mistake during check in, 5~7 can be done by a role named 
"integrator".

My dream is that check-in process is just one click button. But it seems we are 
still far from it...

My two cents.

Thank you
Yao Jiewen

From: devel@edk2.groups.io  On Behalf Of Yao, Jiewen
Sent: Saturday, September 26, 2020 1:09 PM
To: devel@edk2.groups.io; Guptha, Soumya K ; 
annou...@edk2.groups.io
Subject: Re: [edk2-devel] Tianocore community page on who we are - please review

Thanks Soumya. I think this is a good start.

Recently we are discussing the maintainer's work in EDKII mailing list, with 
title "more development process failure".

I feel the process mentioned in 
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process
 is not clear enough to follow, especially for the maintainer who is not full 
time working on EDKII.

I wish we can have this opportunity to revisit the "Follow the EDK II 
development 
process"
 and make "the process" simpler and clearer.

Then all maintainers can sign to follow one rule. The rule we define and the 
rule we agree with.

Thank you
Yao Jiewen


From: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>> On Behalf Of Soumya Guptha
Sent: Saturday, September 26, 2020 6:35 AM
To: annou...@edk2.groups.io; 
devel@edk2.groups.io
Subject: [edk2-devel] Tianocore community page on who we are - please review


Dear Community members,



I have drafted a document "who we are", explaining Tianocore community 
structure, members of the community, their role and the current development 
process. I have drafted this document with the help of the Tianocore Stewards.

We view this as a living document, as our development processes evolve, I will 
keep this document updated.



Please review the draft version of the document (link below) and provide your 
feedback. Please send it to me, no need to reply all.

I appreciate your input by Friday, Oct 2. After this, I plan on make it live on 
our TianoCore wiki site.



Link: https://github.com/tianocore/tianocore.github.io/wiki/Who-we-are

Thanks,
Soumya

Soumya Guptha
TianoCore Community Manager





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

Re: [edk2-devel] Tianocore community page on who we are - please review

[Yao, Jiewen]

Thanks Soumya. I think this is a good start.

Recently we are discussing the maintainer's work in EDKII mailing list, with 
title "more development process failure".

I feel the process mentioned in 
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process
 is not clear enough to follow, especially for the maintainer who is not full 
time working on EDKII.

I wish we can have this opportunity to revisit the "Follow the EDK II 
development 
process"
 and make "the process" simpler and clearer.

Then all maintainers can sign to follow one rule. The rule we define and the 
rule we agree with.

Thank you
Yao Jiewen


From: devel@edk2.groups.io  On Behalf Of Soumya Guptha
Sent: Saturday, September 26, 2020 6:35 AM
To: annou...@edk2.groups.io; devel@edk2.groups.io
Subject: [edk2-devel] Tianocore community page on who we are - please review


Dear Community members,



I have drafted a document "who we are", explaining Tianocore community 
structure, members of the community, their role and the current development 
process. I have drafted this document with the help of the Tianocore Stewards.

We view this as a living document, as our development processes evolve, I will 
keep this document updated.



Please review the draft version of the document (link below) and provide your 
feedback. Please send it to me, no need to reply all.

I appreciate your input by Friday, Oct 2. After this, I plan on make it live on 
our TianoCore wiki site.



Link: https://github.com/tianocore/tianocore.github.io/wiki/Who-we-are

Thanks,
Soumya

Soumya Guptha
TianoCore Community Manager






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

Re: [EXTERNAL] [edk2-devel] ECC: Won't somebody PLEASE think of the... test structures.

[Andrew Fish via groups.io]

Bret,

Details aside this ECC issue got me thinking it might be useful to some how tag 
code that follows different, subsetted, or alternate rules, or uses different 
build environments. I was kind of thinking of doing something like how we tag 
the licenses in the header comments [1]. I would say nothing means edk2 rules. 
I can see vendors maybe having different internal rules, or us wanting to 
distinguish test code from production code. The general idea if we start this 
tools can be smarter and that seems like a good thing. 

 This is just a wild idea, so I’d like to see what other people think?

[1] SPDX-License-Identifier: BSD-2-Clause-Patent

Thanks,

Andrew Fish

> On Sep 24, 2020, at 7:57 PM, Bret Barkelew  
> wrote:
> 
> Andrew,
>
> That’s actually what got me here. EccCheck runs as part of our CI now (but it 
> didn’t when I wrote these tests a year ago). I need to either figure out how 
> to get this code to pass EccCheck in a reasonable way, or just not contribute 
> the tests and say “go to Mu for the tests, if you want them”.
>
> Skipping the contribution isn’t a desirable outcome at all for a number of 
> reasons, not the least of which is that we (MS and others) are trying 
> encourage all contributions to come with tests, so the process of writing 
> them needs to be simple and painless.
>
> - Bret 
>
> From: Andrew Fish 
> Sent: Thursday, September 24, 2020 7:48 PM
> To: edk2-devel-groups-io ; Bret Barkelew 
> 
> Cc: Ken Taylor 
> Subject: [EXTERNAL] Re: [edk2-devel] ECC: Won't somebody PLEASE think of 
> the... test structures.
>
> Bret,
>
> I’ve had this issue with EFI code too. It will compile with for DEBUG and 
> RELEASE as the optimizer removes the memcpy/memset. So you only see a build 
> failure when you compiler NOOPT (and there are no intrinsic libs). I mostly 
> see this in platform code when I try to compile a single driver/lib NOOPT and 
> it fails to link due to the missing intrinsic. 
>
> The easy to enforce this is to compile with optimizations enabled and don’t 
> enable intrinsic libs. Not sure if that is really practical from the test 
> point of view. 
>
> Seems the tool caught the coding style violation so I guess we could try to 
> “make running that tool easier”. Maybe hooking into patchcheck.py, making 
> some kind of githook, or adding a git command?
>
> Thanks,
>
> Andrew Fish
>
> 
> 
> On Sep 24, 2020, at 7:25 PM, Bret Barkelew via groups.io 
> 
>   > wrote:
>
> So for context, this is a new host-based test that should only run within a 
> platform OS, so intrinsics aren’t the big deal that they would be in FW code.
>
> But we do need to figure out how to simultaneously adhere to the coding 
> convention while enabling test authoring.
> Or we chose to not enforce quite as many things for tests.
>
> I’d prefer the first. 
>
> - Bret 
>
> From: Ken Taylor 
> Sent: Thursday, September 24, 2020 6:57 PM
> To: devel@edk2.groups.io ; Bret Barkelew 
> 
> Subject: [EXTERNAL] RE: ECC: Won't somebody PLEASE think of the... test 
> structures.
>
> If the structure is a non-static local variable, most compilers will silently 
> inject an intrinsic call to memcpy in function initialization.  This leads to 
> an intermittent linker error.
>
> If the compiler you use automatically supports an intrinsic memcpy in the 
> given architecture or optimizes out the memcpy, it will build for you and you 
> won’t know you need to link to an intrinsic support library in order to build 
> cross platform.  This leads to code that builds for you, but not for me.
>
> Regards,
> -Ken.
>
> From: devel@edk2.groups.io  
> [mailto:devel@edk2.groups.io ] On Behalf Of Bret 
> Barkelew via groups.io 
> 
> Sent: Thursday, September 24, 2020 6:23 PM
> To: devel@edk2.groups.io 
> Subject: [edk2-devel] ECC: Won't somebody PLEASE think of the... test 
> structures.
>
> ERROR - EFI coding style error
> ERROR - *Error code: 5007
> ERROR - *There should be no initialization of a variable as part of its 
> declaration
> ERROR - *file: 
> //home/corthon/_uefi/edk2_qemu_ci/edk2/MdeModulePkg/Libr

[edk2-devel] [PATCH v4 6/6] FmpDevicePkg/FmpDeviceLib: Add Last Attempt Status to Check/Set API

[Michael Kubacki]

From: Michael Kubacki 

Provides the ability for a given FMP device library instance to
return a Last Attempt Status code during check image and set image
operations with FmpDeviceCheckImageEx() and FmpDeviceSetImageEx().

Cc: Liming Gao 
Cc: Michael D Kinney 
Cc: Guomin Jiang 
Cc: Wei6 Xu 
Signed-off-by: Michael Kubacki 
---
 FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c | 132 +++-
 FmpDevicePkg/Include/Library/FmpDeviceLib.h  | 121 +-
 2 files changed, 251 insertions(+), 2 deletions(-)

diff --git a/FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c 
b/FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c
index 316de12e910c..4e1ce8bf2294 100644
--- a/FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c
+++ b/FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c
@@ -2,7 +2,7 @@
   Provides firmware device specific services to support updates of a firmware
   image stored in a firmware device.
 
-  Copyright (c) 2016, Microsoft Corporation. All rights reserved.
+  Copyright (c) Microsoft Corporation.
   Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -10,6 +10,7 @@
 **/
 
 #include 
+#include 
 #include 
 
 /**
@@ -410,6 +411,55 @@ FmpDeviceCheckImage (
   return EFI_SUCCESS;
 }
 
+/**
+  Checks if a new firmware image is valid for the firmware device.  This
+  function allows firmware update operation to validate the firmware image
+  before FmpDeviceSetImage() is called.
+
+  @param[in]  Image   Points to a new firmware image.
+  @param[in]  ImageSize   Size, in bytes, of a new firmware image.
+  @param[out] ImageUpdatable  Indicates if a new firmware image is valid 
for
+  a firmware update to the firmware device.  
The
+  following values from the Firmware Management
+  Protocol are supported:
+IMAGE_UPDATABLE_VALID
+IMAGE_UPDATABLE_INVALID
+IMAGE_UPDATABLE_INVALID_TYPE
+IMAGE_UPDATABLE_INVALID_OLD
+IMAGE_UPDATABLE_VALID_WITH_VENDOR_CODE
+  @param[out] LastAttemptStatus   A pointer to a UINT32 that holds the last 
attempt
+  status to report back to the ESRT table in 
case
+  of error. This value will only be checked 
when this
+  function returns an error.
+
+  The return status code must fall in the 
range of
+  
LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE to
+  
LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE.
+
+  If the value falls outside this range, it 
will be converted
+  to LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL.
+
+  @retval EFI_SUCCESSThe image was successfully checked.  
Additional
+ status information is returned in
+ ImageUpdatable.
+  @retval EFI_INVALID_PARAMETER  Image is NULL.
+  @retval EFI_INVALID_PARAMETER  ImageUpdatable is NULL.
+
+**/
+EFI_STATUS
+EFIAPI
+FmpDeviceCheckImageWithStatus (
+  IN  CONST VOID  *Image,
+  IN  UINTN   ImageSize,
+  OUT UINT32  *ImageUpdatable,
+  OUT UINT32  *LastAttemptStatus
+  )
+{
+  *LastAttemptStatus = LAST_ATTEMPT_STATUS_SUCCESS;
+
+  return EFI_SUCCESS;
+}
+
 /**
   Updates a firmware device with a new firmware image.  This function returns
   EFI_UNSUPPORTED if the firmware image is not updatable.  If the firmware 
image
@@ -476,6 +526,86 @@ FmpDeviceSetImage (
   return EFI_UNSUPPORTED;
 }
 
+/**
+  Updates a firmware device with a new firmware image.  This function returns
+  EFI_UNSUPPORTED if the firmware image is not updatable.  If the firmware 
image
+  is updatable, the function should perform the following minimal validations
+  before proceeding to do the firmware image update.
+- Validate that the image is a supported image for this firmware device.
+  Return EFI_ABORTED if the image is not supported.  Additional details
+  on why the image is not a supported image may be returned in AbortReason.
+- Validate the data from VendorCode if is not NULL.  Firmware image
+  validation must be performed before VendorCode data validation.
+  VendorCode data is ignored or considered invalid if image validation
+  fails.  Return EFI_ABORTED if the VendorCode data is invalid.
+
+  VendorCode enables vendor to implement vendor-specific firmware image update
+  policy.  Null if the caller did not specify the policy or use the default
+  policy.  As an example, vendor can implement a policy to allow an option to
+  force a firmware image update when th

[edk2-devel] [PATCH v4 3/6] FmpDevicePkg/FmpDxe: Add check image path Last Attempt Status capability

[Michael Kubacki]

From: Michael Kubacki 

CheckTheImage() is currently used to provide the CheckImage()
implementation for the EFI_FIRMWARE_MANAGEMENT_PROTOCOL instance
produced by FmpDxe in addition to being called internally in the
SetImage() path.

Since CheckTheImage() plays a major role in determining the
validity of a given firmware image, an internal version of the
function is introduced - CheckTheImageInternal() that is capable
of returning a Last Attempt Status code to internal callers such
as SetTheImage().

The CheckImage() API as defined in the UEFI Specification for
EFI_FIRMWARE_MANAGEMENT_PROTOCOL is not impacted by this change.

CheckTheImageInternal() contains unique Last Attempt Status codes
during error paths in the function so it is easier to identify
the issue with a particular image through the Last Attempt Status
code value.

Cc: Liming Gao 
Cc: Michael D Kinney 
Cc: Guomin Jiang 
Cc: Wei6 Xu 
Signed-off-by: Michael Kubacki 
---
 FmpDevicePkg/FmpDxe/FmpDxe.c | 98 +---
 FmpDevicePkg/FmpDxe/FmpDxe.h |  4 +-
 2 files changed, 90 insertions(+), 12 deletions(-)

diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c b/FmpDevicePkg/FmpDxe/FmpDxe.c
index 427b215ddc5f..858cffd8e5bd 100644
--- a/FmpDevicePkg/FmpDxe/FmpDxe.c
+++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
@@ -721,6 +721,14 @@ GetAllHeaderSize (
   @param[in]  ImageSize  Size of the new image in bytes.
   @param[out] ImageUpdatable Indicates if the new image is valid for 
update. It also provides,
  if available, additional information if the 
image is invalid.
+  @param[out] LastAttemptStatus  A pointer to a UINT32 that holds the last 
attempt status to report
+ back to the ESRT table in case of error.  If 
an error does not occur,
+ this function will set the value to 
LAST_ATTEMPT_STATUS_SUCCESS.
+
+ This function will return error codes that 
occur within this function
+ implementation within a driver range of last 
attempt error codes from
+ 
LAST_ATTEMPT_STATUS_DRIVER_MIN_ERROR_CODE_VALUE
+ to 
LAST_ATTEMPT_STATUS_DRIVER_MAX_ERROR_CODE_VALUE.
 
   @retval EFI_SUCCESSThe image was successfully checked.
   @retval EFI_ABORTEDThe operation is aborted.
@@ -731,15 +739,17 @@ GetAllHeaderSize (
 **/
 EFI_STATUS
 EFIAPI
-CheckTheImage (
+CheckTheImageInternal (
   IN  EFI_FIRMWARE_MANAGEMENT_PROTOCOL  *This,
   IN  UINT8 ImageIndex,
   IN  CONST VOID*Image,
   IN  UINTN ImageSize,
-  OUT UINT32*ImageUpdatable
+  OUT UINT32*ImageUpdatable,
+  OUT UINT32*LastAttemptStatus
   )
 {
   EFI_STATUSStatus;
+  UINT32LocalLastAttemptStatus;
   FIRMWARE_MANAGEMENT_PRIVATE_DATA  *Private;
   UINTN RawSize;
   VOID  *FmpPayloadHeader;
@@ -755,23 +765,37 @@ CheckTheImage (
   EFI_FIRMWARE_IMAGE_DEP*Dependencies;
   UINT32DependenciesSize;
 
-  Status   = EFI_SUCCESS;
-  RawSize  = 0;
-  FmpPayloadHeader = NULL;
-  FmpPayloadSize   = 0;
-  Version  = 0;
-  FmpHeaderSize= 0;
-  AllHeaderSize= 0;
-  Dependencies = NULL;
-  DependenciesSize = 0;
+  Status  = EFI_SUCCESS;
+  LocalLastAttemptStatus  = LAST_ATTEMPT_STATUS_SUCCESS;
+  RawSize = 0;
+  FmpPayloadHeader= NULL;
+  FmpPayloadSize  = 0;
+  Version = 0;
+  FmpHeaderSize   = 0;
+  AllHeaderSize   = 0;
+  Dependencies= NULL;
+  DependenciesSize= 0;
 
   if (!FeaturePcdGet (PcdFmpDeviceStorageAccessEnable)) {
 return EFI_UNSUPPORTED;
   }
 
+  if (LastAttemptStatus == NULL) {
+DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckTheImageInternal() - 
LastAttemptStatus is NULL.\n", mImageIdName));
+Status = EFI_INVALID_PARAMETER;
+goto cleanup;
+  }
+
+  //
+  // A last attempt status error code will always override the success
+  // value before returning from the function
+  //
+  *LastAttemptStatus = LAST_ATTEMPT_STATUS_SUCCESS;
+
   if (This == NULL) {
 DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckImage() - This is NULL.\n", 
mImageIdName));
 Status = EFI_INVALID_PARAMETER;
+*LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_PROTOCOL_ARG_MISSING;
 goto cleanup;
   }
 
@@ -789,6 +813,7 @@ CheckTheImage (
   if (ImageUpdatable == NULL) {
 DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckImage() - ImageUpdatable Pointer 
Parameter is NULL.\n", mImageIdName));
 Status = EFI_INVALID_PARAMETER;
+*LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_IMAGE_NOT_UPDATABLE;
 goto cleanup;
   }
 
@@ -808,6 +833,7 @@

[edk2-devel] [PATCH v4 2/6] FmpDevicePkg: Add Last Attempt Status header files

[Michael Kubacki]

From: Michael Kubacki 

Introduces a public and a private header file to define more
granular usage of the UEFI Specification defined unsuccessful
vendor range for Last Attempt Status codes. The unsuccessful
vendor range is described in UEFI Specification 2.8A section 23.4.

The public header file Include/LastAttemptStatus.h defines ranges
within the unsuccessful vendor range. At a high-level, the two
main ranges are defined are the FMP Reserved range and the Device
Library Reserved range.

The FMP Reserved range is reserved for usage of components within
FmpDevicePkg. PrivateInclude/FmpLastAttemptStatus.h contains
usage details and specific Last Attempt Status code definitions.

The Device Library Reserved range is reserved for usage by
FmpDeviceLib instances. Each library may define custom Last
Attempt Status codes within the bounds defined in
Include/LastAttemptStatus.h:
[LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE,
 LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE]

Cc: Liming Gao 
Cc: Michael D Kinney 
Cc: Guomin Jiang 
Cc: Wei6 Xu 
Signed-off-by: Michael Kubacki 
---
 FmpDevicePkg/Include/LastAttemptStatus.h   | 81 
 FmpDevicePkg/PrivateInclude/FmpLastAttemptStatus.h | 81 
 2 files changed, 162 insertions(+)

diff --git a/FmpDevicePkg/Include/LastAttemptStatus.h 
b/FmpDevicePkg/Include/LastAttemptStatus.h
new file mode 100644
index ..0dcd1ef71621
--- /dev/null
+++ b/FmpDevicePkg/Include/LastAttemptStatus.h
@@ -0,0 +1,81 @@
+/** @file
+  Defines last attempt status code ranges within the UEFI Specification
+  defined unsuccessful vendor range.
+
+  Copyright (c) Microsoft Corporation.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef __LAST_ATTEMPT_STATUS_H__
+#define __LAST_ATTEMPT_STATUS_H__
+
+///
+/// Last Attempt Status Unsuccessful Vendor Range Map
+///
+/// Update this map any time new ranges are added. Pre-existing range 
definitions cannot be modified
+/// to keep status code definitions consistent over time.
+///
+/// START | END   | Usage
+/// --|
+/// 0x1000| 0x17FF| FmpDevicePkg  |
+///0x1000 |0x107F | FmpDxe driver |
+///0x1080 |0x109F | FmpDependencyLib  |
+///0x10A0 |0x10BF | FmpDependencyCheckLib |
+///0x10C0 |0x17FF | Unused. Available for future expansion.   |
+/// 0x1800| 0x1FFF| FmpDeviceLib instances implementation |
+/// 0x2000| 0x3FFF| Unused. Available for future expansion.   |
+///
+
+///
+/// The minimum value of the FMP reserved range.
+///
+#define LAST_ATTEMPT_STATUS_FMP_RESERVED_MIN_ERROR_CODE_VALUE   
0x1000
+
+///
+/// The maximum value of the FMP reserved range.
+///
+#define LAST_ATTEMPT_STATUS_FMP_RESERVED_MAX_ERROR_CODE_VALUE   
0x1FFF
+
+///
+/// The minimum value allowed for FmpDxe driver-specific errors.
+///
+#define LAST_ATTEMPT_STATUS_DRIVER_MIN_ERROR_CODE_VALUE 
0x1000
+
+///
+/// The maximum value allowed for FmpDxe driver-specific errors.
+///
+#define LAST_ATTEMPT_STATUS_DRIVER_MAX_ERROR_CODE_VALUE 
0x107F
+
+///
+/// The minimum value allowed for FmpDependencyLib related errors.
+///
+#define LAST_ATTEMPT_STATUS_FMP_DEPENDENCY_LIB_MIN_ERROR_CODE_VALUE 
0x1080
+
+///
+/// The maximum value allowed for FmpDependencyLib related errors.
+///
+#define LAST_ATTEMPT_STATUS_FMP_DEPENDENCY_LIB_MAX_ERROR_CODE_VALUE 
0x109F
+
+///
+/// The minimum value allowed for FmpDependencyCheckLib related errors.
+///
+#define LAST_ATTEMPT_STATUS_FMP_DEPENDENCY_CHECK_LIB_MIN_ERROR_CODE_VALUE   
0x10A0
+
+///
+/// The maximum value allowed for FmpDependencyCheckLib related errors.
+///
+#define LAST_ATTEMPT_STATUS_FMP_DEPENDENCY_CHECK_LIB_MAX_ERROR_CODE_VALUE   
0x10BF
+
+///
+/// The minimum value allowed for FMP device library errors.
+///
+#define LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE 
0x1800
+
+///
+/// The maximum value allowed for FMP device library errors.
+///
+#define LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE 
0x1FFF
+
+#endif
diff --git a/FmpDevicePkg/PrivateInclude/FmpLastAttemptStatus.h 
b/FmpDevicePkg/PrivateInclude/FmpLastAttemptStatus.h
new file mode 100644
index ..a0cc94f69955
--- /dev/null
+++ b/FmpDevicePkg/PrivateInclude/FmpLastAttemptStatus.h
@@ -0,0 +1,81 @@
+/** @file
+  Defines private last attempt status codes used in FmpDevicePkg.
+
+  Copyright (c) Microsoft Corporation.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef __FMP_LAST_ATTEMPT_STATUS_H__
+#define __FMP_LAST_ATTEMPT_STATUS_H__
+
+///
+/// Last attempt status codes defined for additional granularity in 
FmpDevicePkg components.
+///
+/// These codes are defined within the last attempt status FMP reserved r

[edk2-devel] [PATCH v4 5/6] FmpDevicePkg: Add Last Attempt Status support to dependency libs

[Michael Kubacki]

From: Michael Kubacki 

The FMP dependency libraries are leveraged during firmware update
to check for dependencies required to update the image.

This change adds granular Last Attempt Status code support to these
services so failures can be more easily observed during the firmware
update process via Last Attempt Status codes.

Cc: Liming Gao 
Cc: Michael D Kinney 
Cc: Guomin Jiang 
Cc: Wei6 Xu 
Signed-off-by: Michael Kubacki 
---
 FmpDevicePkg/FmpDxe/FmpDxe.c   
  | 31 ++-
 FmpDevicePkg/Library/FmpDependencyCheckLib/FmpDependencyCheckLib.c 
  | 39 +---
 FmpDevicePkg/Library/FmpDependencyCheckLibNull/FmpDependencyCheckLibNull.c 
  | 14 ++-
 FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c   
  | 93 +---
 
FmpDevicePkg/Test/UnitTest/Library/FmpDependencyLib/EvaluateDependencyUnitTest.c
 |  7 +-
 FmpDevicePkg/Include/Library/FmpDependencyCheckLib.h   
  |  8 +-
 FmpDevicePkg/Include/Library/FmpDependencyLib.h
  | 44 +
 7 files changed, 189 insertions(+), 47 deletions(-)

diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c b/FmpDevicePkg/FmpDxe/FmpDxe.c
index 68aacc518a40..843ecbd78b58 100644
--- a/FmpDevicePkg/FmpDxe/FmpDxe.c
+++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
@@ -730,6 +730,15 @@ GetAllHeaderSize (
  
LAST_ATTEMPT_STATUS_DRIVER_MIN_ERROR_CODE_VALUE
  to 
LAST_ATTEMPT_STATUS_DRIVER_MAX_ERROR_CODE_VALUE.
 
+ This function might also return error codes 
that occur within libraries
+ linked against this module that return last 
attempt error codes such as:
+
+ 
LAST_ATTEMPT_STATUS_FMP_DEPENDENCY_LIB_MIN_ERROR_CODE_VALUE to
+ 
LAST_ATTEMPT_STATUS_FMP_DEPENDENCY_LIB_MAX_ERROR_CODE_VALUE
+
+ 
LAST_ATTEMPT_STATUS_FMP_DEPENDENCY_CHECK_LIB_MIN_ERROR_CODE_VALUE to
+ 
LAST_ATTEMPT_STATUS_FMP_DEPENDENCY_CHECK_LIB_MAX_ERROR_CODE_VALUE
+
   @retval EFI_SUCCESSThe image was successfully checked.
   @retval EFI_ABORTEDThe operation is aborted.
   @retval EFI_INVALID_PARAMETER  The Image was NULL.
@@ -925,7 +934,16 @@ CheckTheImageInternal (
   //
   // Get the dependency from Image.
   //
-  Dependencies = GetImageDependency ((EFI_FIRMWARE_IMAGE_AUTHENTICATION 
*)Image, ImageSize, &DependenciesSize);
+  Dependencies =  GetImageDependency (
+(EFI_FIRMWARE_IMAGE_AUTHENTICATION *) Image,
+ImageSize,
+&DependenciesSize,
+LastAttemptStatus
+);
+  if (*LastAttemptStatus != LAST_ATTEMPT_STATUS_SUCCESS) {
+Status = EFI_ABORTED;
+goto cleanup;
+  }
 
   //
   // Check the FmpPayloadHeader
@@ -964,11 +982,18 @@ CheckTheImageInternal (
   //
   // Evaluate dependency expression
   //
-  Private->DependenciesSatisfied = CheckFmpDependency 
(Private->Descriptor.ImageTypeId, Version, Dependencies, DependenciesSize);
+  Private->DependenciesSatisfied =  CheckFmpDependency (
+  Private->Descriptor.ImageTypeId,
+  Version,
+  Dependencies,
+  DependenciesSize,
+  &LocalLastAttemptStatus
+  );
   if (!Private->DependenciesSatisfied) {
 DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckTheImage() - Dependency check 
failed.\n", mImageIdName));
 *ImageUpdatable = IMAGE_UPDATABLE_INVALID;
 Status = EFI_SUCCESS;
+*LastAttemptStatus = LocalLastAttemptStatus;
 goto cleanup;
   }
 
@@ -1181,7 +1206,7 @@ SetTheImage (
   //
   // Get the dependency from Image.
   //
-  Dependencies = GetImageDependency ((EFI_FIRMWARE_IMAGE_AUTHENTICATION 
*)Image, ImageSize, &DependenciesSize);
+  Dependencies = GetImageDependency ((EFI_FIRMWARE_IMAGE_AUTHENTICATION 
*)Image, ImageSize, &DependenciesSize, &LastAttemptStatus);
 
   //
   // No functional error in CheckTheImage.  Attempt to get the Version to
diff --git a/FmpDevicePkg/Library/FmpDependencyCheckLib/FmpDependencyCheckLib.c 
b/FmpDevicePkg/Library/FmpDependencyCheckLib/FmpDependencyCheckLib.c
index 02ed600e0e95..cca83dbe4a14 100644
--- a/FmpDevicePkg/Library/FmpDependencyCheckLib/FmpDependencyCheckLib.c
+++ b/FmpDevicePkg/Library/FmpDependencyCheckLib/FmpDependencyCheckLib.c
@@ -17,6 +17,9 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 
 /**
   Check dependency for firmware update.
@@ -25,6 +28,10 @@
   @param[in]  VersionNew version.
   @param[in]  Dependencies   Fmp dependency.
   @param[in]  DependenciesSize   Size, in bytes, of the Fmp dependency.
+  @param[out] LastAttemptStatus  An optional 

[edk2-devel] [PATCH v4 4/6] FmpDevicePkg/FmpDxe: Improve set image path Last Attempt Status granularity

[Michael Kubacki]

From: Michael Kubacki 

Increases the level of granularity for Last Attempt Status codes
returned from SetTheImage() in FmpDxe. This allows better
identification of the error that occurred in the set image
operation using Last Attempt Status codes.

Cc: Liming Gao 
Cc: Michael D Kinney 
Cc: Guomin Jiang 
Cc: Wei6 Xu 
Signed-off-by: Michael Kubacki 
---
 FmpDevicePkg/FmpDxe/FmpDxe.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c b/FmpDevicePkg/FmpDxe/FmpDxe.c
index 858cffd8e5bd..68aacc518a40 100644
--- a/FmpDevicePkg/FmpDxe/FmpDxe.c
+++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
@@ -1138,6 +1138,7 @@ SetTheImage (
   if (This == NULL) {
 DEBUG ((DEBUG_ERROR, "FmpDxe(%s): SetTheImage() - This is NULL.\n", 
mImageIdName));
 Status = EFI_INVALID_PARAMETER;
+LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_PROTOCOL_ARG_MISSING;
 goto cleanup;
   }
 
@@ -1163,6 +1164,7 @@ SetTheImage (
   //
   if (Private->FmpDeviceLocked) {
 DEBUG ((DEBUG_ERROR, "FmpDxe(%s): SetTheImage() - Device is already 
locked.  Can't update.\n", mImageIdName));
+LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_DEVICE_LOCKED;
 Status = EFI_UNSUPPORTED;
 goto cleanup;
   }
@@ -1170,12 +1172,9 @@ SetTheImage (
   //
   // Call check image to verify the image
   //
-  Status = CheckTheImage (This, ImageIndex, Image, ImageSize, &Updateable);
+  Status = CheckTheImageInternal (This, ImageIndex, Image, ImageSize, 
&Updateable, &LastAttemptStatus);
   if (EFI_ERROR (Status)) {
 DEBUG ((DEBUG_ERROR, "FmpDxe(%s): SetTheImage() - Check The Image failed 
with %r.\n", mImageIdName, Status));
-if (Status == EFI_SECURITY_VIOLATION) {
-  LastAttemptStatus = LAST_ATTEMPT_STATUS_ERROR_AUTH_ERROR;
-}
 goto cleanup;
   }
 
@@ -1191,6 +1190,7 @@ SetTheImage (
   FmpHeader = GetFmpHeader ( (EFI_FIRMWARE_IMAGE_AUTHENTICATION *)Image, 
ImageSize, DependenciesSize, &FmpPayloadSize );
   if (FmpHeader == NULL) {
 DEBUG ((DEBUG_ERROR, "FmpDxe(%s): SetTheImage() - GetFmpHeader failed.\n", 
mImageIdName));
+LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_GET_FMP_HEADER;
 Status = EFI_ABORTED;
 goto cleanup;
   }
@@ -1218,6 +1218,7 @@ SetTheImage (
 
   if (Progress == NULL) {
 DEBUG ((DEBUG_ERROR, "FmpDxe(%s): SetTheImage() - Invalid progress 
callback\n", mImageIdName));
+LastAttemptStatus = 
LAST_ATTEMPT_STATUS_DRIVER_ERROR_PROGRESS_CALLBACK_ERROR;
 Status = EFI_INVALID_PARAMETER;
 goto cleanup;
   }
@@ -1238,6 +1239,7 @@ SetTheImage (
   Status = CheckSystemPower (&BooleanValue);
   if (EFI_ERROR (Status)) {
 DEBUG ((DEBUG_ERROR, "FmpDxe(%s): SetTheImage() - CheckSystemPower - API 
call failed %r.\n", mImageIdName, Status));
+LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_CHECK_POWER_API;
 goto cleanup;
   }
   if (!BooleanValue) {
@@ -1258,10 +1260,12 @@ SetTheImage (
   Status = CheckSystemThermal (&BooleanValue);
   if (EFI_ERROR (Status)) {
 DEBUG ((DEBUG_ERROR, "FmpDxe(%s): SetTheImage() - CheckSystemThermal - API 
call failed %r.\n", mImageIdName, Status));
+LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_CHECK_SYS_THERMAL_API;
 goto cleanup;
   }
   if (!BooleanValue) {
 Status = EFI_ABORTED;
+LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_THERMAL;
 DEBUG (
   (DEBUG_ERROR,
   "FmpDxe(%s): SetTheImage() - CheckSystemThermal - returned False.  
Update not allowed due to System Thermal.\n", mImageIdName)
@@ -1277,10 +1281,12 @@ SetTheImage (
   Status = CheckSystemEnvironment (&BooleanValue);
   if (EFI_ERROR (Status)) {
 DEBUG ((DEBUG_ERROR, "FmpDxe(%s): SetTheImage() - CheckSystemEnvironment - 
API call failed %r.\n", mImageIdName, Status));
+LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_CHECK_SYS_ENV_API;
 goto cleanup;
   }
   if (!BooleanValue) {
 Status = EFI_ABORTED;
+LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_SYSTEM_ENV;
 DEBUG (
   (DEBUG_ERROR,
   "FmpDxe(%s): SetTheImage() - CheckSystemEnvironment - returned False.  
Update not allowed due to System Environment.\n", mImageIdName)
@@ -1302,12 +1308,14 @@ SetTheImage (
   Status = GetFmpPayloadHeaderSize (FmpHeader, FmpPayloadSize, &FmpHeaderSize);
   if (EFI_ERROR (Status)) {
 DEBUG ((DEBUG_ERROR, "FmpDxe(%s): SetTheImage() - GetFmpPayloadHeaderSize 
failed %r.\n", mImageIdName, Status));
+LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_GET_FMP_HEADER_SIZE;
 goto cleanup;
   }
 
   AllHeaderSize = GetAllHeaderSize ((EFI_FIRMWARE_IMAGE_AUTHENTICATION 
*)Image, FmpHeaderSize + DependenciesSize);
   if (AllHeaderSize == 0) {
 DEBUG ((DEBUG_ERROR, "FmpDxe(%s): SetTheImage() - GetAllHeaderSize 
failed.\n", mImageIdName));
+LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_GET_ALL_HEADER_SIZE;
 Status = EFI_ABORTED;
 goto cleanup;
   }
@@ -1372,6 +1380,7 @@ SetTheImage (
   mProgressFunc = NULL;
 
   if (Priv

[edk2-devel] [PATCH v4 1/6] MdePkg/SystemResourceTable.h: Add vendor range values

[Michael Kubacki]

From: Michael Kubacki 

Adds the following macros to define the unsuccessful vendor range
min and max (defined in UEFI Specification 2.8):
  1. LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL_VENDOR_RANGE_MIN
  2. LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL_VENDOR_RANGE_MAX

Cc: Liming Gao 
Cc: Michael D Kinney 
Cc: Guomin Jiang 
Cc: Wei6 Xu 
Cc: Zhiguang Liu 
Signed-off-by: Michael Kubacki 
---
 MdePkg/Include/Guid/SystemResourceTable.h | 13 +
 1 file changed, 13 insertions(+)

diff --git a/MdePkg/Include/Guid/SystemResourceTable.h 
b/MdePkg/Include/Guid/SystemResourceTable.h
index 418b8c8d055a..17a26fe7e688 100644
--- a/MdePkg/Include/Guid/SystemResourceTable.h
+++ b/MdePkg/Include/Guid/SystemResourceTable.h
@@ -1,6 +1,7 @@
 /** @file
   Guid & data structure used for EFI System Resource Table (ESRT)
 
+  Copyright (c) Microsoft Corporation.
   Copyright (c) 2015 - 2020, Intel Corporation. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -44,6 +45,18 @@
 #define LAST_ATTEMPT_STATUS_ERROR_PWR_EVT_BATT  0x0007
 #define LAST_ATTEMPT_STATUS_ERROR_UNSATISFIED_DEPENDENCIES  0x0008
 
+///
+/// LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL_VENDOR_RANGE_MAX is defined as
+/// 0x4000 as of UEFI Specification 2.8B. This will be modified in the
+/// future to the correct value 0x3FFF. To ensure correct implementation,
+/// this change is preemptively made in the value defined below.
+///
+/// When the UEFI Specification is updated, this comment block can be
+/// removed.
+///
+#define LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL_VENDOR_RANGE_MIN 0x1000
+#define LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL_VENDOR_RANGE_MAX 0x3FFF
+
 typedef struct {
   ///
   /// The firmware class field contains a GUID that identifies a firmware 
component
-- 
2.28.0.windows.1



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

[edk2-devel] [PATCH v4 0/6] Extend Last Attempt Status Usage

[Michael Kubacki]

From: Michael Kubacki 

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

This patch series adds more granularity to Last Attempt Status
codes reported during FMP check image and set image operations
that greatly improve precision of the status codes.

The unsuccessful vendor range (0x1000 - 0x4000) was introduced
in UEFI Specification 2.8. At a high-level, two subranges are
defined within that range in this patch series:
  1. The FMP Reserved range - reserved for components implemented
 in FmpDevicePkg.
  2. The FMP Device Library Reserved range - reserved for
 FmpDeviceLib instance-specific usage.

The ranges are described in a public header file LastAttemptStatus.h
while the specific codes used within FmpDevicePkg implementation
are defined in a private header file FmpLastAttemptStatus.h.

FmpDeviceLib instances should use the range definition from the
public header file to define Last Attempt Status codes local to
their library instance.

Of note, there's multiple approaches to assigning private status
codes in the FMP Reserved range. For example, individual components
could define their last attempt status codes locally with the
range allocated to the component defined in a package-wide private
header file. However, one goal of the granularity being introduced
is to provide straightforward traceability to an error source.

For that reason, it was chosen to define a constant set of codes at
the package level in FmpLastAttemptStatus.h. For example, if a new
FmpDependencyLib instance is added, it would not be able to reassign
status code values in the pre-existing FMP Dependency range; it
would reuse codes for the same error source and be able to add new
codes onto the range for its usage.

V4 changes:
  1. Simplified range value definitions in LastAttemptStatus.h.
 Directly assign the values in the macro definition instead
 of using calculations.
  2. Adjusted range sizes to leave more room for future expansion.

 OLD:
 START | END   | Usage
 |
 0x1000| 0x1FFF| FmpDevicePkg|
0x1000 |0x107F | FmpDxe driver   |
0x1080 |0x109F | FMP dependency Libs |
 0x2000| 0x3FFF| FmpDeviceLib instances  |

 NEW:
 START | END   | Usage
 |
 0x1000| 0x17FF| FmpDevicePkg|
0x1000 |0x107F | FmpDxe driver   |
0x1080 |0x109F | FmpDependencyLib|
0x10A0 |0x10BF | FmpDependencyCheckLib   |
0x10C0 |0x17FF | Unused. Available for future expansion. |
 0x1800| 0x1FFF| FmpDeviceLib instances implementation   |
 0x2000| 0x3FFF| Unused. Available for future expansion. |

  3. Broke the single range in v3 for FMP Dependency libraries into
 separate ranges.
  4. Clarified LastAttemptStatus return values in each function
 description.
  5. Returned an expected LastAttemptStatus value for some functions
 that previously did not return a value.
  6. Reverted changes in FmpDxe to call the new FmpDeviceLib APIs
 for FmpDeviceCheckImage () and FmpDeviceSetImage (). These will
 be added in a future series after impacted platforms in
 edk2-platforms are updated to use the new APIs.
  7. Instead of directly changing the pre-existing APIs in
 FmpDeviceLib to add a LastAttemptStatus parameter, the new
 functions were added to the library interface:
   * FmpDeviceCheckImageWithStatus ()
   * FmpDeviceSetImageWithStatus ()

V3 changes:
  1. Enhanced range definitions in LastAttemptStatus.h with more
 completeness providing length, min, and max values.
  2. Moved the actual Last Attempt Status code assignments to a
 private header file PrivateInclude/FmpLastAttemptStatus.h.
  3. Changed the value of
 LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL_VENDOR_RANGE_MAX
 to 0x3FFF instead of 0x4000 even though 0x4000 is defined in
 the UEFI specification. The length is 0x4000 but the max
 allowed value should be 0x3FFF. This change was made now to
 prevent implementation compatibility issues in the future.
  4. Included "DEVICE" in the following macro name to clearly
 associate it with the FmpDeviceLib library class:
 LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_ERROR_xxx
  5. Included a map to help the reader better visualize the range
 definitions in LastAttemptStatus.h.
  6. Included additional documentation describing the enum in
 FmpLastAttemptStatus.h. An explicit statement stating that new
 codes should be added onto the end of ranges to preserve the
 values was added.
  7. Simplified error handling logic in FmpDxe for FmpDeviceLib
 calls that return Last Attempt Status.
  8. V2 had a single memory allocation failure code used for
 different memory allocati

Re: [edk2-devel] [edk2-staging/EdkRepo] [PATCH V3] EdkRepo: Return exit codes in edkrepo_entry_point.py

[Nate DeSimone]

Reviewed-by: Nate DeSimone 

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Ashley E Desimone
Sent: Wednesday, September 9, 2020 2:42 PM
To: devel@edk2.groups.io
Cc: Desimone, Nathaniel L ; Pandya, Puja 
; Bret Barkelew ; Agyeman, 
Prince ; Bjorge, Erik C 
Subject: [edk2-devel] [edk2-staging/EdkRepo] [PATCH V3] EdkRepo: Return exit 
codes in edkrepo_entry_point.py

Return the error codes generated by edkrepo_cli that is called in main()

Cc: Nate DeSimone 
Cc: Puja Pandya 
Cc: Bret Barkelew 
Cc: Prince Agyeman 
Cc: Erik Bjorge 
Signed-off-by: Ashley E Desimone 
---
 edkrepo/edkrepo_entry_point.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/edkrepo/edkrepo_entry_point.py b/edkrepo/edkrepo_entry_point.py 
index c2ff56a..dd2b36e 100644
--- a/edkrepo/edkrepo_entry_point.py
+++ b/edkrepo/edkrepo_entry_point.py
@@ -91,12 +91,12 @@ def main():
 try: mod = importlib.import_module(pref_entry) func = 
getattr(mod, pref_entry_func)-func()+return(func()) except 
Exception as e: print('Unable to launch preferred entry point. 
Launching default entry point edkrepo.edkrepo_cli.py') 
traceback.print_exc() import edkrepo.edkrepo_cli-
edkrepo.edkrepo_cli.main()+return edkrepo.edkrepo_cli.main()  if 
__name__ == "__main__": try:-- 
2.26.2.windows.1


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

View/Reply Online (#65164): https://edk2.groups.io/g/devel/message/65164
Mute This Topic: https://groups.io/mt/76743015/1767664
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  
[nathaniel.l.desim...@intel.com] -=-=-=-=-=-=



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

[edk2-devel] [edk2-staging/EdkRepo] [PATCH V3 0/2] EdkRepo: Add support for SUBST drives

[Nate DeSimone]

Changes in V3:
- Changed loop for finding subst drive to single if statement

Changes in V2:
- Changed get_subst_drive_list() to get_subst_drive_dict()

EdkRepo currently does not handle virtual drives created using the SUBST 
command.
Specifically, when cloning or syncing a project to a subst drive the includeIf 
statements
that redirect submodule fetches to mirror servers will be generated with the
subst drive information.� This causes git to not activate the includeif since
it specifies the subst path and not the actual path.

To resolve this, EdkRepo will now enumerate the virtual drives created by SUBST 
and if
the current workspace is on a SUBST virtual drive EdkRepo will convert the 
workspace
path to the path on the real volume.

Cc: Ashley E Desimone 
Cc: Puja Pandya 
Cc: Bret Barkelew 
Cc: Prince Agyeman 
Cc: Erik Bjorge 
Signed-off-by: Nate DeSimone 

Nate DeSimone (2):
  EdkRepo: Add function to enumerate subst drives
  EdkRepo: Add support for subst drives

 edkrepo/commands/clone_command.py |  8 +
 edkrepo/common/pathfix.py | 50 ++-
 edkrepo/config/config_factory.py  | 10 ++-
 3 files changed, 66 insertions(+), 2 deletions(-)

-- 
2.27.0.windows.1



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

[edk2-devel] [edk2-staging/EdkRepo] [PATCH V3 2/2] EdkRepo: Add support for subst drives

[Nate DeSimone]

get_workspace_path() now converts a virtual drive path
to a real path before any git repo operations are done.

Cc: Ashley E Desimone 
Cc: Puja Pandya 
Cc: Bret Barkelew 
Cc: Prince Agyeman 
Cc: Erik Bjorge 
Signed-off-by: Nate DeSimone 
---
 edkrepo/commands/clone_command.py |  8 
 edkrepo/config/config_factory.py  | 10 +-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/edkrepo/commands/clone_command.py 
b/edkrepo/commands/clone_command.py
index f638090..8769102 100644
--- a/edkrepo/commands/clone_command.py
+++ b/edkrepo/commands/clone_command.py
@@ -9,6 +9,7 @@
 
 import os
 import shutil
+import sys
 
 from edkrepo.commands.edkrepo_command import EdkrepoCommand
 from edkrepo.commands.edkrepo_command import SubmoduleSkipArgument, 
SourceManifestRepoArgument
@@ -20,6 +21,7 @@ from edkrepo.common.edkrepo_exception import 
EdkrepoInvalidParametersException,
 from edkrepo.common.edkrepo_exception import EdkrepoManifestNotFoundException
 from edkrepo.common.humble import CLONE_INVALID_WORKSPACE, 
CLONE_INVALID_PROJECT_ARG, CLONE_INVALID_COMBO_ARG
 from edkrepo.common.humble import SPARSE_CHECKOUT, CLONE_INVALID_LOCAL_ROOTS
+from edkrepo.common.pathfix import get_subst_drive_dict
 from edkrepo.common.workspace_maintenance.workspace_maintenance import 
case_insensitive_single_match
 from edkrepo.common.workspace_maintenance.manifest_repos_maintenance import 
pull_all_manifest_repos, find_project_in_all_indices
 from edkrepo.common.workspace_maintenance.manifest_repos_maintenance import 
list_available_manifest_repos
@@ -77,6 +79,12 @@ class CloneCommand(EdkrepoCommand):
 workspace_dir = os.getcwd()
 else:
 workspace_dir = os.path.abspath(workspace_dir)
+if sys.platform == "win32":
+subst = get_subst_drive_dict()
+drive = os.path.splitdrive(workspace_dir)[0][0].upper()
+if drive in subst:
+workspace_dir = os.path.join(subst[drive], 
os.path.splitdrive(workspace_dir)[1][1:])
+workspace_dir = os.path.normpath(workspace_dir)
 if os.path.isdir(workspace_dir) and os.listdir(workspace_dir):
 raise EdkrepoInvalidParametersException(CLONE_INVALID_WORKSPACE)
 if not os.path.isdir(workspace_dir):
diff --git a/edkrepo/config/config_factory.py b/edkrepo/config/config_factory.py
index a82a438..fe69460 100644
--- a/edkrepo/config/config_factory.py
+++ b/edkrepo/config/config_factory.py
@@ -11,13 +11,16 @@ import os
 import sys
 import configparser
 import collections
-from ctypes import *
+if sys.platform == "win32":
+from ctypes import oledll, c_void_p, c_uint32, c_wchar_p
+from ctypes import create_unicode_buffer
 
 import edkrepo.config.humble.config_factory_humble as humble
 from edkrepo.common.edkrepo_exception import 
EdkrepoGlobalConfigNotFoundException, EdkrepoConfigFileInvalidException
 from edkrepo.common.edkrepo_exception import EdkrepoWorkspaceInvalidException, 
EdkrepoGlobalDataDirectoryNotFoundException
 from edkrepo.common.edkrepo_exception import EdkrepoConfigFileReadOnlyException
 from edkrepo.common.humble import MIRROR_PRIMARY_REPOS_MISSING, 
MIRROR_DECODE_WARNING, MAX_PATCH_SET_INVALID
+from edkrepo.common.pathfix import get_subst_drive_dict
 from edkrepo_manifest_parser import edk_manifest
 from edkrepo.common.pathfix import expanduser
 
@@ -238,6 +241,11 @@ def get_workspace_path():
 while True:
 if os.path.isdir(os.path.join(path, "repo")):
 if os.path.isfile(os.path.join(os.path.join(path, "repo"), 
"Manifest.xml")):
+if sys.platform == "win32":
+subst = get_subst_drive_dict()
+drive = os.path.splitdrive(path)[0][0].upper()
+if drive in subst:
+path = os.path.join(subst[drive], 
os.path.splitdrive(path)[1][1:])
 return path
 if os.path.dirname(path) == path:
 break
-- 
2.27.0.windows.1



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

Re: [edk2-devel] more development process failure [was: UefiPayloadPkg: Runtime MMCONF]

[Guo Dong]

Sorry to have a long email thread since my merge and thanks all for the 
comments.
In general, I still feel current process is a little complicated for the 
maintainers who don't
daily work on EDK2 like me.  I have less than %5 of time spent on open source 
EDK2 
UefiPayloadPkg since I focus on bootloaders.  It would be great if I could 
spend the time
mainly on code review instead of the process as of now.

Even after I read https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-
Development-Process#the-maintainer-process-for-the-edk-ii-project as Liming 
pointed out,
Some info is still not clear for me. E.g. what's the purpose for putting cover 
letter to patch 
set pull request (it looks we could not trace to this PR from code)? is it 
mandatory or optional?
What if there is no cover letter in the patch set in patch #0 summary? For the 
patch I merged, 
I am still not very sure what info I should put there.

I don't know why Laszlo mentioned BZ for my merge since there is no BZ 
mentioned in the patchset.
And I also don't know why Laszlo mentioned to send email after the patch is 
merged since I don't find this 
requirement in the development process. I don't think it is doable to ask all 
the maintainers to monitor EDK2 
mail list on how others are doing since there are so many emails every day, 
especially there is no any patch
for UefiPayloadPkg for several months.

I hope we could simplify the process and have a clear steps in the process 
soon. So that the maintainers could 
focus on the actual code review.

Thanks,
Guo



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

[edk2-devel] Tianocore community page on who we are - please review

[Soumya Guptha]

Dear Community members,



I have drafted a document "who we are", explaining Tianocore community 
structure, members of the community, their role and the current development 
process. I have drafted this document with the help of the Tianocore Stewards.

We view this as a living document, as our development processes evolve, I will 
keep this document updated.



Please review the draft version of the document (link below) and provide your 
feedback. Please send it to me, no need to reply all.

I appreciate your input by Friday, Oct 2. After this, I plan on make it live on 
our TianoCore wiki site.



Link: https://github.com/tianocore/tianocore.github.io/wiki/Who-we-are

Thanks,
Soumya

Soumya Guptha
TianoCore Community Manager






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

[edk2-devel] [Patch V3 1/2] EmulatorPkg: Add CI build for SECURE_BOOT_ENABLE

[Michael D Kinney]

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

Add EmulatorPkg CI builds for SECURE_BOOT_ENABLE=TRUE
for IA32/X64 and DEBUG/RELEASE/NOOPT.  Label these as
FULL builds, so if additional build options are added
in the future, they can be added to these FULL builds.

Cc: Jordan Justen 
Cc: Andrew Fish 
Cc: Ray Ni 
Cc: Divneil Rai Wadhawan 
Cc: Sean Brogan 
Signed-off-by: Michael D Kinney 
Reviewed-by: Sean Brogan 
---
 .../.azurepipelines/Ubuntu-GCC5.yml   | 43 +++
 .../.azurepipelines/Windows-VS2019.yml| 43 +++
 EmulatorPkg/PlatformCI/ReadMe.md  |  4 +-
 3 files changed, 89 insertions(+), 1 deletion(-)

diff --git a/EmulatorPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml 
b/EmulatorPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml
index 12ef8226ff..0e5f4d9961 100644
--- a/EmulatorPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml
+++ b/EmulatorPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml
@@ -6,6 +6,7 @@
 # Toolchain: GCC5
 #
 # Copyright (c) Microsoft Corporation.
+# Copyright (c) 2020, Intel Corporation. All rights reserved.
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 ##
 trigger:
@@ -65,6 +66,48 @@ jobs:
 Build.Target: "NOOPT"
 Run.Flags: $(run_flags)
 Run: $(should_run)
+  EmulatorPkg_X64_FULL_DEBUG:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "X64"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Target: "DEBUG"
+Run.Flags: $(run_flags)
+Run: $(should_run)
+  EmulatorPkg_X64_FULL_RELEASE:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "X64"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Target: "RELEASE"
+Run.Flags: $(run_flags)
+Run: $(should_run)
+  EmulatorPkg_X64_FULL_NOOPT:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "X64"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Target: "NOOPT"
+Run.Flags: $(run_flags)
+Run: $(should_run)
+  EmulatorPkg_IA32_FULL_DEBUG:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "IA32"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Target: "DEBUG"
+Run.Flags: $(run_flags)
+Run: $(should_run)
+  EmulatorPkg_IA32_FULL_RELEASE:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "IA32"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Target: "RELEASE"
+Run.Flags: $(run_flags)
+Run: $(should_run)
+  EmulatorPkg_IA32_FULL_NOOPT:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "IA32"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Target: "NOOPT"
+Run.Flags: $(run_flags)
+Run: $(should_run)
 
 workspace:
   clean: all
diff --git a/EmulatorPkg/PlatformCI/.azurepipelines/Windows-VS2019.yml 
b/EmulatorPkg/PlatformCI/.azurepipelines/Windows-VS2019.yml
index a5baf4b606..2bfce4e0af 100644
--- a/EmulatorPkg/PlatformCI/.azurepipelines/Windows-VS2019.yml
+++ b/EmulatorPkg/PlatformCI/.azurepipelines/Windows-VS2019.yml
@@ -6,6 +6,7 @@
 # Toolchain: VS2019
 #
 # Copyright (c) Microsoft Corporation.
+# Copyright (c) 2020, Intel Corporation. All rights reserved.
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 ##
 trigger:
@@ -66,6 +67,48 @@ jobs:
 Build.Target: "NOOPT"
 Run.Flags: $(run_flags)
 Run: $(should_run)
+  EmulatorPkg_X64_FULL_DEBUG:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "X64"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Target: "DEBUG"
+Run.Flags: $(run_flags)
+Run: $(should_run)
+  EmulatorPkg_X64_FULL_RELEASE:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "X64"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Target: "RELEASE"
+Run.Flags: $(run_flags)
+Run: $(should_run)
+  EmulatorPkg_X64_FULL_NOOPT:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "X64"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Target: "NOOPT"
+Run.Flags: $(run_flags)
+Run: $(should_run)
+  EmulatorPkg_IA32_FULL_DEBUG:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "IA32"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Target: "DEBUG"
+Run.Flags: $(run_flags)
+Run: $(should_run)
+  EmulatorPkg_IA32_FULL_RELEASE:

[edk2-devel] [Patch V3 0/2] EmulatorPkg: Add CI build for SECURE_BOOT_ENABLE

[Michael D Kinney]

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

Add EmulatorPkg CI builds for SECURE_BOOT_ENABLE=TRUE
for IA32/X64 and DEBUG/RELEASE/NOOPT and update add these
to the CI build status in Readme.rst.

Cc: Jordan Justen 
Cc: Andrew Fish 
Cc: Ray Ni 
Cc: Divneil Rai Wadhawan 
Cc: Sean Brogan 
Signed-off-by: Michael D Kinney 

Michael D Kinney (2):
  EmulatorPkg: Add CI build for SECURE_BOOT_ENABLE
  Readme.rst: Add EmulatorPkg SECURE_BOOT_ENABLE CI status

 .../.azurepipelines/Ubuntu-GCC5.yml   | 43 +++
 .../.azurepipelines/Windows-VS2019.yml| 43 +++
 EmulatorPkg/PlatformCI/ReadMe.md  |  4 +-
 ReadMe.rst| 18 
 4 files changed, 107 insertions(+), 1 deletion(-)

-- 
2.21.0.windows.1



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

[edk2-devel] [Patch V3 2/2] Readme.rst: Add EmulatorPkg SECURE_BOOT_ENABLE CI status

[Michael D Kinney]

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

Add CI status badges for the EmulatorPkg CI builds with
SECURE_BOOT_ENABLE=TRUE for IA32/X64 and DEBUG/RELEASE/NOOPT.

Cc: Jordan Justen 
Cc: Andrew Fish 
Cc: Ray Ni 
Cc: Divneil Rai Wadhawan 
Cc: Sean Brogan 
Signed-off-by: Michael D Kinney 
Reviewed-by: Sean Brogan 
---
 ReadMe.rst | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/ReadMe.rst b/ReadMe.rst
index a16e13547f..c3c8178373 100644
--- a/ReadMe.rst
+++ b/ReadMe.rst
@@ -28,6 +28,8 @@ Microsoft Windows VS2019
 = = = = 
==
 EmulatorPkg_Win_VS2019_   | IA32|em32d|   |em32r|   
|em32n|
 | | X64 |em64d|   |em64r|   
|em64n|
+| | IA32 FULL   |em32fd|  |em32fr|  
|em32fn|
+| | X64 FULL|em64fd|  |em64fr|  
|em64fn|
 OvmfPkg_Win_VS2019_   | IA32|op32d|   |op32r|   
|op32n|
 | | X64 |op64d|   |op64r|   
|op64n|
 | | IA32 X64|op3264d| |op3264r| 
|op3264n|
@@ -44,6 +46,8 @@ ArmVirtPkg_Ubuntu_GCC5_   | AARCH64 |avAArch64du| 
|avAArch64ru| |avA
 | | ARM |avArmdu| |avArmru| 
|avArmnu|
 EmulatorPkg_Ubuntu_GCC5_  | IA32|em32du|  |em32ru|  
|em32nu|
 | | X64 |em64du|  |em64ru|  
|em64nu|
+| | IA32 FULL   |em32fdu| |em32fru| 
|em32fnu|
+| | X64 FULL|em64fdu| |em64fru| 
|em64fnu|
 OvmfPkg_Ubuntu_GCC5_  | IA32|op32du|  |op32ru|  
|op32nu|
 | | X64 |op64du|  |op64ru|  
|op64nu|
 | | IA32 X64|op3264du||op3264ru|
|op3264nu|
@@ -319,6 +323,13 @@ use.
 .. |em32n| image:: 
https://dev.azure.com/tianocore/edk2-ci/_apis/build/status/PlatformCI_EmulatorPkg_Windows_VS2019_CI?branchName=master&jobName=Platform_CI&configuration=Platform_CI%20EmulatorPkg_IA32_NOOPT
 .. |em32nu| image:: 
https://dev.azure.com/tianocore/edk2-ci/_apis/build/status/PlatformCI_EmulatorPkg_Ubuntu_GCC5_CI?branchName=master&jobName=Platform_CI&configuration=Platform_CI%20EmulatorPkg_IA32_NOOPT
 
+.. |em32fd| image:: 
https://dev.azure.com/tianocore/edk2-ci/_apis/build/status/PlatformCI_EmulatorPkg_Windows_VS2019_CI?branchName=master&jobName=Platform_CI&configuration=Platform_CI%20EmulatorPkg_IA32_FULL_DEBUG
+.. |em32fdu| image:: 
https://dev.azure.com/tianocore/edk2-ci/_apis/build/status/PlatformCI_EmulatorPkg_Ubuntu_GCC5_CI?branchName=master&jobName=Platform_CI&configuration=Platform_CI%20EmulatorPkg_IA32_FULL_DEBUG
+.. |em32fr| image:: 
https://dev.azure.com/tianocore/edk2-ci/_apis/build/status/PlatformCI_EmulatorPkg_Windows_VS2019_CI?branchName=master&jobName=Platform_CI&configuration=Platform_CI%20EmulatorPkg_IA32_FULL_RELEASE
+.. |em32fru| image:: 
https://dev.azure.com/tianocore/edk2-ci/_apis/build/status/PlatformCI_EmulatorPkg_Ubuntu_GCC5_CI?branchName=master&jobName=Platform_CI&configuration=Platform_CI%20EmulatorPkg_IA32_FULL_RELEASE
+.. |em32fn| image:: 
https://dev.azure.com/tianocore/edk2-ci/_apis/build/status/PlatformCI_EmulatorPkg_Windows_VS2019_CI?branchName=master&jobName=Platform_CI&configuration=Platform_CI%20EmulatorPkg_IA32_FULL_NOOPT
+.. |em32fnu| image:: 
https://dev.azure.com/tianocore/edk2-ci/_apis/build/status/PlatformCI_EmulatorPkg_Ubuntu_GCC5_CI?branchName=master&jobName=Platform_CI&configuration=Platform_CI%20EmulatorPkg_IA32_FULL_NOOPT
+
 .. |em64d| image:: 
https://dev.azure.com/tianocore/edk2-ci/_apis/build/status/PlatformCI_EmulatorPkg_Windows_VS2019_CI?branchName=master&jobName=Platform_CI&configuration=Platform_CI%20EmulatorPkg_X64_DEBUG
 .. |em64du| image:: 
https://dev.azure.com/tianocore/edk2-ci/_apis/build/status/PlatformCI_EmulatorPkg_Ubuntu_GCC5_CI?branchName=master&jobName=Platform_CI&configuration=Platform_CI%20EmulatorPkg_X64_DEBUG
 .. |em64r| image:: 
https://dev.azure.com/tianocore/edk2-ci/_apis/build/status/PlatformCI_EmulatorPkg_Windows_VS2019_CI?branchName=master&jobName=Platform_CI&configuration=Platform_CI%20EmulatorPkg_X64_RELEASE
@@ -326,6 +337,13 @@ use.
 .. |em64n| image:: 
https://dev.azure.com/tianocore/edk2-ci/_apis/build/status/PlatformCI_EmulatorPkg_Windows_VS2019_CI?branchName=master&jobName=Platform_CI&configuration=Platform_CI%20EmulatorPkg_X64_NOOPT
 .. |em64nu| image:: 
https://dev.azure.com/tianocore/edk2-ci/_apis/build/status/PlatformCI_EmulatorPkg_Ubuntu_GCC5_CI?branchName=master&jobName=Platform_CI&configuration=Platform_CI%20EmulatorPkg_X64_NOOPT
 
+.. |em64fd| image:: 
https://dev.azure.com/tianocore/edk2-ci/_apis/build/status/PlatformCI_EmulatorPkg_Windows_VS2019_CI?branchName=master

Re: [edk2-devel] [Patch V2] EmulatorPkg: Add CI build for SECURE_BOOT_ENABLE

[Michael D Kinney]

Hi Sean,

Appreciate the review since the change was based on your suggestions.

I agree.  I can split out the Readme.rst changes into its own commit.

Mike

> -Original Message-
> From: Sean Brogan 
> Sent: Friday, September 25, 2020 1:46 PM
> To: devel@edk2.groups.io; Kinney, Michael D 
> Cc: Justen, Jordan L ; Andrew Fish 
> ; Ni, Ray ; Wadhawan, Divneil R
> ; Sean Brogan 
> Subject: Re: [edk2-devel] [Patch V2] EmulatorPkg: Add CI build for 
> SECURE_BOOT_ENABLE
> 
> 
> I give my reviewed by for all changes for what it is worth.
> 
> One question: Should the EmulatorPkg change be in its own commit just to
> keep sereration.  I know in Mu we split the edk2 repository and it is
> nice that nearly all commits only touch a single "package".
> 
> Reviewed-by: Sean Brogan 
> 
> Thanks
> Sean
> 
> 
> 
> 
> 
> On 9/25/2020 1:20 PM, Michael D Kinney wrote:
> > https://bugzilla.tianocore.org/show_bug.cgi?id=2979
> >
> > Add EmulatorPkg CI builds for SECURE_BOOT_ENABLE=TRUE
> > for IA32/X64 and DEBUG/RELEASE/NOOPT.  Label these as
> > FULL builds, so if additional build options are added
> > in the future, they can be added to these FULL builds.
> >
> > Cc: Jordan Justen 
> > Cc: Andrew Fish 
> > Cc: Ray Ni 
> > Cc: Divneil Rai Wadhawan 
> > Cc: Sean Brogan 
> > Signed-off-by: Michael D Kinney 
> > ---
> >   .../.azurepipelines/Ubuntu-GCC5.yml   | 43 +++
> >   .../.azurepipelines/Windows-VS2019.yml| 43 +++
> >   EmulatorPkg/PlatformCI/ReadMe.md  |  4 +-
> >   ReadMe.rst| 18 
> >   4 files changed, 107 insertions(+), 1 deletion(-)
> >
> > diff --git a/EmulatorPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml 
> > b/EmulatorPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml
> > index 12ef8226ff..0e5f4d9961 100644
> > --- a/EmulatorPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml
> > +++ b/EmulatorPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml
> > @@ -6,6 +6,7 @@
> >   # Toolchain: GCC5
> >   #
> >   # Copyright (c) Microsoft Corporation.
> > +# Copyright (c) 2020, Intel Corporation. All rights reserved.
> >   # SPDX-License-Identifier: BSD-2-Clause-Patent
> >   ##
> >   trigger:
> > @@ -65,6 +66,48 @@ jobs:
> >   Build.Target: "NOOPT"
> >   Run.Flags: $(run_flags)
> >   Run: $(should_run)
> > +  EmulatorPkg_X64_FULL_DEBUG:
> > +Build.File: "$(package)/PlatformCI/PlatformBuild.py"
> > +Build.Arch: "X64"
> > +Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
> > +Build.Target: "DEBUG"
> > +Run.Flags: $(run_flags)
> > +Run: $(should_run)
> > +  EmulatorPkg_X64_FULL_RELEASE:
> > +Build.File: "$(package)/PlatformCI/PlatformBuild.py"
> > +Build.Arch: "X64"
> > +Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
> > +Build.Target: "RELEASE"
> > +Run.Flags: $(run_flags)
> > +Run: $(should_run)
> > +  EmulatorPkg_X64_FULL_NOOPT:
> > +Build.File: "$(package)/PlatformCI/PlatformBuild.py"
> > +Build.Arch: "X64"
> > +Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
> > +Build.Target: "NOOPT"
> > +Run.Flags: $(run_flags)
> > +Run: $(should_run)
> > +  EmulatorPkg_IA32_FULL_DEBUG:
> > +Build.File: "$(package)/PlatformCI/PlatformBuild.py"
> > +Build.Arch: "IA32"
> > +Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
> > +Build.Target: "DEBUG"
> > +Run.Flags: $(run_flags)
> > +Run: $(should_run)
> > +  EmulatorPkg_IA32_FULL_RELEASE:
> > +Build.File: "$(package)/PlatformCI/PlatformBuild.py"
> > +Build.Arch: "IA32"
> > +Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
> > +Build.Target: "RELEASE"
> > +Run.Flags: $(run_flags)
> > +Run: $(should_run)
> > +  EmulatorPkg_IA32_FULL_NOOPT:
> > +Build.File: "$(package)/PlatformCI/PlatformBuild.py"
> > +Build.Arch: "IA32"
> > +Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
> > +Build.Target: "NOOPT"
> > +Run.Flags: $(run_flags)
> > +Run: $(should_run)
> >
> >   workspace:
> > clean: all
> > diff --git a/EmulatorPkg/PlatformCI/.azurepipelines/Windows-VS2019.yml 
> > b/EmulatorPkg/PlatformCI/.azurepipelines/Windows-VS2019.yml
> > index a5baf4b606..2bfce4e0af 100644
> > --- a/EmulatorPkg/PlatformCI/.azurepipelines/Windows-VS2019.yml
> > +++ b/EmulatorPkg/PlatformCI/.azurepipelines/Windows-VS2019.yml
> > @@ -6,6 +6,7 @@
> >   # Toolchain: VS2019
> >   #
> >   # Copyright (c) Microsoft Corporation.
> > +# Copyright (c) 2020, Intel Corporation. All rights reserved.
> >   # SPDX-License-Identifier: BSD-2-Clause-Patent
> >   ##
> >   trigger:
> > @@ -66,6 +67,48 @@ jobs:
> >   Build.Target: "NOO

Re: [edk2-devel] [Patch V2] EmulatorPkg: Add CI build for SECURE_BOOT_ENABLE

[Sean]

I give my reviewed by for all changes for what it is worth.

One question: Should the EmulatorPkg change be in its own commit just to 
keep sereration.  I know in Mu we split the edk2 repository and it is 
nice that nearly all commits only touch a single "package".


Reviewed-by: Sean Brogan 

Thanks
Sean





On 9/25/2020 1:20 PM, Michael D Kinney wrote:

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

Add EmulatorPkg CI builds for SECURE_BOOT_ENABLE=TRUE
for IA32/X64 and DEBUG/RELEASE/NOOPT.  Label these as
FULL builds, so if additional build options are added
in the future, they can be added to these FULL builds.

Cc: Jordan Justen 
Cc: Andrew Fish 
Cc: Ray Ni 
Cc: Divneil Rai Wadhawan 
Cc: Sean Brogan 
Signed-off-by: Michael D Kinney 
---
  .../.azurepipelines/Ubuntu-GCC5.yml   | 43 +++
  .../.azurepipelines/Windows-VS2019.yml| 43 +++
  EmulatorPkg/PlatformCI/ReadMe.md  |  4 +-
  ReadMe.rst| 18 
  4 files changed, 107 insertions(+), 1 deletion(-)

diff --git a/EmulatorPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml 
b/EmulatorPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml
index 12ef8226ff..0e5f4d9961 100644
--- a/EmulatorPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml
+++ b/EmulatorPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml
@@ -6,6 +6,7 @@
  # Toolchain: GCC5
  #
  # Copyright (c) Microsoft Corporation.
+# Copyright (c) 2020, Intel Corporation. All rights reserved.
  # SPDX-License-Identifier: BSD-2-Clause-Patent
  ##
  trigger:
@@ -65,6 +66,48 @@ jobs:
  Build.Target: "NOOPT"
  Run.Flags: $(run_flags)
  Run: $(should_run)
+  EmulatorPkg_X64_FULL_DEBUG:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "X64"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Target: "DEBUG"
+Run.Flags: $(run_flags)
+Run: $(should_run)
+  EmulatorPkg_X64_FULL_RELEASE:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "X64"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Target: "RELEASE"
+Run.Flags: $(run_flags)
+Run: $(should_run)
+  EmulatorPkg_X64_FULL_NOOPT:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "X64"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Target: "NOOPT"
+Run.Flags: $(run_flags)
+Run: $(should_run)
+  EmulatorPkg_IA32_FULL_DEBUG:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "IA32"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Target: "DEBUG"
+Run.Flags: $(run_flags)
+Run: $(should_run)
+  EmulatorPkg_IA32_FULL_RELEASE:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "IA32"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Target: "RELEASE"
+Run.Flags: $(run_flags)
+Run: $(should_run)
+  EmulatorPkg_IA32_FULL_NOOPT:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "IA32"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Target: "NOOPT"
+Run.Flags: $(run_flags)
+Run: $(should_run)
  
  workspace:

clean: all
diff --git a/EmulatorPkg/PlatformCI/.azurepipelines/Windows-VS2019.yml 
b/EmulatorPkg/PlatformCI/.azurepipelines/Windows-VS2019.yml
index a5baf4b606..2bfce4e0af 100644
--- a/EmulatorPkg/PlatformCI/.azurepipelines/Windows-VS2019.yml
+++ b/EmulatorPkg/PlatformCI/.azurepipelines/Windows-VS2019.yml
@@ -6,6 +6,7 @@
  # Toolchain: VS2019
  #
  # Copyright (c) Microsoft Corporation.
+# Copyright (c) 2020, Intel Corporation. All rights reserved.
  # SPDX-License-Identifier: BSD-2-Clause-Patent
  ##
  trigger:
@@ -66,6 +67,48 @@ jobs:
  Build.Target: "NOOPT"
  Run.Flags: $(run_flags)
  Run: $(should_run)
+  EmulatorPkg_X64_FULL_DEBUG:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "X64"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Target: "DEBUG"
+Run.Flags: $(run_flags)
+Run: $(should_run)
+  EmulatorPkg_X64_FULL_RELEASE:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "X64"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Target: "RELEASE"
+Run.Flags: $(run_flags)
+Run: $(should_run)
+  EmulatorPkg_X64_FULL_NOOPT:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "X64"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Ta

[edk2-devel] [Patch V2] EmulatorPkg: Add CI build for SECURE_BOOT_ENABLE

[Michael D Kinney]

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

Add EmulatorPkg CI builds for SECURE_BOOT_ENABLE=TRUE
for IA32/X64 and DEBUG/RELEASE/NOOPT.  Label these as
FULL builds, so if additional build options are added
in the future, they can be added to these FULL builds.

Cc: Jordan Justen 
Cc: Andrew Fish 
Cc: Ray Ni 
Cc: Divneil Rai Wadhawan 
Cc: Sean Brogan 
Signed-off-by: Michael D Kinney 
---
 .../.azurepipelines/Ubuntu-GCC5.yml   | 43 +++
 .../.azurepipelines/Windows-VS2019.yml| 43 +++
 EmulatorPkg/PlatformCI/ReadMe.md  |  4 +-
 ReadMe.rst| 18 
 4 files changed, 107 insertions(+), 1 deletion(-)

diff --git a/EmulatorPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml 
b/EmulatorPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml
index 12ef8226ff..0e5f4d9961 100644
--- a/EmulatorPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml
+++ b/EmulatorPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml
@@ -6,6 +6,7 @@
 # Toolchain: GCC5
 #
 # Copyright (c) Microsoft Corporation.
+# Copyright (c) 2020, Intel Corporation. All rights reserved.
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 ##
 trigger:
@@ -65,6 +66,48 @@ jobs:
 Build.Target: "NOOPT"
 Run.Flags: $(run_flags)
 Run: $(should_run)
+  EmulatorPkg_X64_FULL_DEBUG:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "X64"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Target: "DEBUG"
+Run.Flags: $(run_flags)
+Run: $(should_run)
+  EmulatorPkg_X64_FULL_RELEASE:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "X64"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Target: "RELEASE"
+Run.Flags: $(run_flags)
+Run: $(should_run)
+  EmulatorPkg_X64_FULL_NOOPT:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "X64"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Target: "NOOPT"
+Run.Flags: $(run_flags)
+Run: $(should_run)
+  EmulatorPkg_IA32_FULL_DEBUG:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "IA32"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Target: "DEBUG"
+Run.Flags: $(run_flags)
+Run: $(should_run)
+  EmulatorPkg_IA32_FULL_RELEASE:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "IA32"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Target: "RELEASE"
+Run.Flags: $(run_flags)
+Run: $(should_run)
+  EmulatorPkg_IA32_FULL_NOOPT:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "IA32"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Target: "NOOPT"
+Run.Flags: $(run_flags)
+Run: $(should_run)
 
 workspace:
   clean: all
diff --git a/EmulatorPkg/PlatformCI/.azurepipelines/Windows-VS2019.yml 
b/EmulatorPkg/PlatformCI/.azurepipelines/Windows-VS2019.yml
index a5baf4b606..2bfce4e0af 100644
--- a/EmulatorPkg/PlatformCI/.azurepipelines/Windows-VS2019.yml
+++ b/EmulatorPkg/PlatformCI/.azurepipelines/Windows-VS2019.yml
@@ -6,6 +6,7 @@
 # Toolchain: VS2019
 #
 # Copyright (c) Microsoft Corporation.
+# Copyright (c) 2020, Intel Corporation. All rights reserved.
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 ##
 trigger:
@@ -66,6 +67,48 @@ jobs:
 Build.Target: "NOOPT"
 Run.Flags: $(run_flags)
 Run: $(should_run)
+  EmulatorPkg_X64_FULL_DEBUG:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "X64"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Target: "DEBUG"
+Run.Flags: $(run_flags)
+Run: $(should_run)
+  EmulatorPkg_X64_FULL_RELEASE:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "X64"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Target: "RELEASE"
+Run.Flags: $(run_flags)
+Run: $(should_run)
+  EmulatorPkg_X64_FULL_NOOPT:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "X64"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Target: "NOOPT"
+Run.Flags: $(run_flags)
+Run: $(should_run)
+  EmulatorPkg_IA32_FULL_DEBUG:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "IA32"
+Build.Flags: "BLD_*_SECURE_BOOT_ENABLE=TRUE"
+Build.Target: "DEBUG"
+Run.Flags: $(run_flags)
+Run: $(should_run)
+ 

Re: [edk2-devel] [PATCH v2 1/1] UefiCpuPkg/MpInitLib: Reduce reset vector memory pressure

[Lendacky, Thomas]

On 9/25/20 11:09 AM, Brian J. Johnson wrote:

On 9/24/20 1:22 AM, Laszlo Ersek wrote:

- I don't remember if it's required that the APIC ID space be densely
populated. For example, if we have a topology with 7 possible (=
maximum) logical CPUs, I'm unsure if a spec forbids any of those CPUs
from having APIC ID 7. That could cause a problem in
MpInitLibSevEsAPReset(), I assume.


FWIW, there are many bare metal processors with non-contiguous APIC IDs. 
  Intel puts the socket ID in the upper bits of the APIC ID.  So if the 
socket doesn't have a power-of-two number of cores, there is always a 
gap.  Plus, the BSP doesn't always have APIC ID 0 -- it depends on which 
physical cores passed the manufacturing tests for that particular part. 
That has broken various kernels, BIOSes, and other software at times. So 
it's best not to make assumptions.


I don't know if that applies to VMs, though.  The standards may be 
different (if there are any standards at all in that area.)


Yes, I'm working on a patch to use GetProcessorNumber() instead of using 
the APIC ID.


Thanks,
Tom






-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#65633): https://edk2.groups.io/g/devel/message/65633
Mute This Topic: https://groups.io/mt/77041010/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 1/1] UefiCpuPkg/MpInitLib: Reduce reset vector memory pressure

[Brian J. Johnson]

On 9/24/20 1:22 AM, Laszlo Ersek wrote:

- I don't remember if it's required that the APIC ID space be densely
populated. For example, if we have a topology with 7 possible (=
maximum) logical CPUs, I'm unsure if a spec forbids any of those CPUs
from having APIC ID 7. That could cause a problem in
MpInitLibSevEsAPReset(), I assume.


FWIW, there are many bare metal processors with non-contiguous APIC IDs. 
 Intel puts the socket ID in the upper bits of the APIC ID.  So if the 
socket doesn't have a power-of-two number of cores, there is always a 
gap.  Plus, the BSP doesn't always have APIC ID 0 -- it depends on which 
physical cores passed the manufacturing tests for that particular part. 
That has broken various kernels, BIOSes, and other software at times. 
So it's best not to make assumptions.


I don't know if that applies to VMs, though.  The standards may be 
different (if there are any standards at all in that area.)


--
Brian J. Johnson
Enterprise X86 Lab

Hewlett Packard Enterprise



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

Re: [edk2-devel] who can help restore the iBFT spec link to working state?

[Sean]

I see what i can find.

Thanks for pointing this out.



On 9/25/2020 7:20 AM, Laszlo Ersek wrote:

Hi,

the iBFT spec link

   http://www.microsoft.com/whdc/system/platform/firmware/ibft.mspx

as shown at

   https://uefi.org/acpi

does not work (it has not worked since at least August 13th, which is
when I first reported this issue on the ASWG mailing list).

Who from Microsoft can please help the community with a working, stable
link to the iBFT spec?

Thanks,
Laszlo









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

[edk2-devel] who can help restore the iBFT spec link to working state?

[Laszlo Ersek]

Hi,

the iBFT spec link

  http://www.microsoft.com/whdc/system/platform/firmware/ibft.mspx

as shown at

  https://uefi.org/acpi

does not work (it has not worked since at least August 13th, which is
when I first reported this issue on the ASWG mailing list).

Who from Microsoft can please help the community with a working, stable
link to the iBFT spec?

Thanks,
Laszlo



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

Re: [edk2-devel] [PATCH v1 1/1] UefiCpuPkg: Remove PEI/DXE instances of CpuTimerLib.

[Zeng, Star]

A little surprised by "Average time taken to find CpuCrystalFrequencyHob: about 
2000 ns".
It depends on the hob is built early or late?
Seemingly, NanoSecondDelay() will always have some deviation.

Thanks,
Star
-Original Message-
From: devel@edk2.groups.io  On Behalf Of Laszlo Ersek
Sent: Friday, September 25, 2020 2:46 PM
To: Ni, Ray ; Lou, Yun ; 
devel@edk2.groups.io
Cc: Dong, Eric ; Kumar, Rahul1 
Subject: Re: [edk2-devel] [PATCH v1 1/1] UefiCpuPkg: Remove PEI/DXE instances 
of CpuTimerLib.

On 09/25/20 07:25, Ni, Ray wrote:
> Reviewed-by: Ray Ni 

Acked-by: Laszlo Ersek 

Thanks
Laszlo

> 
>> -Original Message-
>> From: Lou, Yun 
>> Sent: Friday, September 25, 2020 11:58 AM
>> To: devel@edk2.groups.io
>> Cc: Lou, Yun ; Ni, Ray ; Dong, 
>> Eric ; Laszlo Ersek ; Kumar, 
>> Rahul1 
>> Subject: [PATCH v1 1/1] UefiCpuPkg: Remove PEI/DXE instances of CpuTimerLib.
>>
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2832
>>
>> 1. Remove PEI instance(PeiCpuTimerLib).
>> PeiCpuTimerLib is currently designed to save time by getting CPU TSC 
>> frequncy from Hob.
>> BaseCpuTimerLib is designed to calculate TSC frequency by using CPUID[15h] 
>> each time.
>> The time it takes to find CpuCrystalFrequencyHob (about 2000ns) is 
>> much longer than it takes to calculate TSC frequency with CPUID[15h] 
>> (about 450ns), which means using BaseCpuTimerLib to trigger a delay 
>> is more accurate than using PeiCpuTimerLib, recommend to use BaseCpuTimerLib 
>> instead of PeiCpuTimerLib.
>>
>> 2. Remove DXE instance(DxeCpuTimerLib).
>> DxeCpuTimerLib is designed to calculate TSC frequency with CPUID[15h] 
>> in its constructor function, then save it in a global variable. For 
>> this design, once the driver containing this instance is running, the 
>> constructor function is called, it will take extra time to calculate TSC 
>> frequency.
>> The time it takes to get TSC frequncy from global variable is shorter 
>> than it takes to calculate TSC frequency with CPUID[15h], but 450ns is a 
>> short time, the impact on the platform is very limited.
>> In addition, in order to simplify the code, recommend to use BaseCpuTimerLib 
>> instead of DxeCpuTimerLib.
>>
>> I did some experiments on one Intel server platform and collected the 
>> following data:
>> 1. Average time taken to find CpuCrystalFrequencyHob: about 2000 ns.
>> 2. Average time taken to calculate TSC frequency: about 450 ns.
>>
>> Reference code:
>> //
>> // Calculate average time taken to find Hob.
>> //
>> DEBUG((DEBUG_ERROR, "[PeiCpuTimerLib] GetPerformanceCounterFrequency - 
>> GetFirstGuidHob (1000 cycles)\n"));
>> Ticks1 = AsmReadTsc();
>> for (i = 0; i < 1000; i++) {
>>   GuidHob = GetFirstGuidHob (&mCpuCrystalFrequencyHobGuid);
>> }
>> Ticks2 = AsmReadTsc();
>>
>> if (GuidHob == NULL) {
>>   DEBUG((DEBUG_ERROR, "[PeiCpuTimerLib]  - CpuCrystalFrequencyHob can 
>> not be found!\n"));
>> } else {
>>   DEBUG((DEBUG_ERROR, "[PeiCpuTimerLib]  - Average time taken to find 
>> Hob = %d ns\n", \
>>   DivU64x32(DivU64x64Remainder(MultU64x32((Ticks2 - Ticks1), 
>> 10), *CpuCrystalCounterFrequency, NULL), 1000)));
>> }
>>
>> //
>> // Calculate average time taken to calculate CPU frequency.
>> //
>> DEBUG((DEBUG_ERROR, "[PeiCpuTimerLib] 
>> GetPerformanceCounterFrequency - CpuidCoreClockCalculateTscFrequency
>> (1000 cycles)\n"));
>> Ticks1 = AsmReadTsc();
>> for (i = 0; i < 1000; i++) {
>>   Freq = CpuidCoreClockCalculateTscFrequency ();
>> }
>> Ticks2 = AsmReadTsc();
>> DEBUG((DEBUG_ERROR, "[PeiCpuTimerLib]  - Average time taken to calculate 
>> TSC frequency = %d ns\n", \
>> DivU64x32(DivU64x64Remainder(MultU64x32((Ticks2 - Ticks1), 
>> 10), *CpuCrystalCounterFrequency, NULL), 1000)));
>>
>> Signed-off-by: Jason Lou 
>> Cc: Ray Ni 
>> Cc: Eric Dong 
>> Cc: Laszlo Ersek 
>> Cc: Rahul Kumar 
>> ---
>>  UefiCpuPkg/Library/CpuTimerLib/DxeCpuTimerLib.c   | 85 
>>  UefiCpuPkg/Library/CpuTimerLib/PeiCpuTimerLib.c   | 58 -
>>  UefiCpuPkg/Library/CpuTimerLib/DxeCpuTimerLib.inf | 37 -  
>> UefiCpuPkg/Library/CpuTimerLib/DxeCpuTimerLib.uni | 17   
>> UefiCpuPkg/Library/CpuTimerLib/PeiCpuTimerLib.inf | 36 -  
>> UefiCpuPkg/Library/CpuTimerLib/PeiCpuTimerLib.uni | 17 
>>  UefiCpuPkg/UefiCpuPkg.dsc |  4 +-
>>  7 files changed, 1 insertion(+), 253 deletions(-)
>>
>> diff --git a/UefiCpuPkg/Library/CpuTimerLib/DxeCpuTimerLib.c 
>> b/UefiCpuPkg/Library/CpuTimerLib/DxeCpuTimerLib.c
>> deleted file mode 100644
>> index 269e5a3e83d7..
>> --- a/UefiCpuPkg/Library/CpuTimerLib/DxeCpuTimerLib.c
>> +++ /dev/null
>> @@ -1,85 +0,0 @@
>> -/** @file
>>
>> -  CPUID Leaf 0x15 for Core Crystal Clock frequency instance of Timer 
>> Library.
>>
>> -
>>
>> -  Copyright (c) 2019 Intel Corporation. All rights reserved.
>>
>> -  SPDX-License-Identifier: BSD-2-Clause-Pa

[edk2-devel] [PATCH v2 0/2] Support for dynamic CMN-600 AML generation

[Sami Mujawar]

The 'Generic ACPI for Arm Components 1.0 Platform Design Document' provides
a standard description for an 'Arm CoreLink CMN-600 Coherent Mesh Network'.

This patch series adds:
  - definition for extended interrupt flags.
  - support for generating SSDT table(s) describing the CMN-600 mesh(es) using
Dynamic AML. This also demonstrates the use of AML Fixup and AML Codegen
techniques.
  - addresses the review feedback for "[PATCH v1 1/2] MdePkg: Definitions for
Extended Interrupt Flags" and makes corresponding changes to the subsequent
patch.

The changes can be seen at:
https://github.com/samimujawar/edk2/tree/1411_cmn600_generator_v2

Pierre Gondois (1):
  DynamicTablesPkg: Add SSDT CMN-600 Table generator

Sami Mujawar (1):
  MdePkg: Definitions for Extended Interrupt Flags

 DynamicTablesPkg/DynamicTables.dsc.inc
|   2 +
 DynamicTablesPkg/DynamicTablesPkg.ci.yaml 
|   4 +
 DynamicTablesPkg/Include/AcpiTableGenerator.h 
|   5 +
 DynamicTablesPkg/Include/ArmNameSpaceObjects.h
|  64 +-
 DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCmn600LibArm/SsdtCmn600Generator.c  
| 708 
 DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCmn600LibArm/SsdtCmn600Generator.h  
|  51 ++
 DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCmn600LibArm/SsdtCmn600LibArm.inf   
|  34 +
 DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCmn600LibArm/SsdtCmn600Template.asl 
|  81 +++
 MdePkg/Include/IndustryStandard/Acpi10.h  
|  11 +
 9 files changed, 954 insertions(+), 6 deletions(-)
 create mode 100644 
DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCmn600LibArm/SsdtCmn600Generator.c
 create mode 100644 
DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCmn600LibArm/SsdtCmn600Generator.h
 create mode 100644 
DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCmn600LibArm/SsdtCmn600LibArm.inf
 create mode 100644 
DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCmn600LibArm/SsdtCmn600Template.asl

-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'



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

[edk2-devel] [PATCH v2 2/2] DynamicTablesPkg: Add SSDT CMN-600 Table generator

[Sami Mujawar]

From: Pierre Gondois 

The Generic ACPI for Arm Components 1.0 Platform Design
Document, s2.6.4 "ASL code examples" provides information
to describe an Arm CoreLink CMN-600 Coherent Mesh Network
using an ASL definition block table.

The SSDT CMN-600 Table Generator uses the Configuration
Manager protocol to obtain the following information about
the CMN-600 device on the platform:
 - the PERIPHBASE address location and address range;
 - the ROOTNODEBASE address location;
 - the number of Debug and Trace Controller (DTC)
   and their respective interrupt number;

The CMN-600 mesh is described using the CM_ARM_CMN_600_INFO
and CM_ARM_EXTENDED_INTERRUPT structures in the Configuration
Manager.

The SSDT CMN-600 Table generator:
 - gets the CMN-600 hardware information
   from the configuration manager.
 - uses the AmlLib interfaces to parse the AML
   template BLOB and construct an AML tree.
 - uses the AmlLib to update:
   - the "_UID" value;
   - the address location and range of the PERIPHBASE;
   - the address location of the ROOTNODEBASE;
   - the number of Debug and Trace Controller (DTC)
 and their respective interrupt number;
 - serializes the AML tree to an output buffer.
   This output buffer contains the fixed-up AML code,
   which is then installed as an ACPI SSDT table.

Signed-off-by: Pierre Gondois 
Co-authored-by: Sami Mujawar 
---

Notes:
v2:
  - Updates corresponding to changes done in Acpi10.h based on  [SAMI]
review comments for [PATCH v1 1/1].

 DynamicTablesPkg/DynamicTables.dsc.inc
|   2 +
 DynamicTablesPkg/DynamicTablesPkg.ci.yaml 
|   4 +
 DynamicTablesPkg/Include/AcpiTableGenerator.h 
|   5 +
 DynamicTablesPkg/Include/ArmNameSpaceObjects.h
|  64 +-
 DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCmn600LibArm/SsdtCmn600Generator.c  
| 708 
 DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCmn600LibArm/SsdtCmn600Generator.h  
|  51 ++
 DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCmn600LibArm/SsdtCmn600LibArm.inf   
|  34 +
 DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCmn600LibArm/SsdtCmn600Template.asl 
|  81 +++
 8 files changed, 943 insertions(+), 6 deletions(-)

diff --git a/DynamicTablesPkg/DynamicTables.dsc.inc 
b/DynamicTablesPkg/DynamicTables.dsc.inc
index 
7fb14d8d1463f7d4502fd3a7708bc94bc336357d..fa33b7ee67e615e236cb13224c859594566df19f
 100644
--- a/DynamicTablesPkg/DynamicTables.dsc.inc
+++ b/DynamicTablesPkg/DynamicTables.dsc.inc
@@ -34,6 +34,7 @@ [Components.common]
 
   # AML Fixup
   
DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtSerialPortLibArm/SsdtSerialPortLibArm.inf
+  DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCmn600LibArm/SsdtCmn600LibArm.inf
 
   #
   # Dynamic Table Factory Dxe
@@ -53,6 +54,7 @@ [Components.common]
 
   # AML Fixup
   
NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtSerialPortLibArm/SsdtSerialPortLibArm.inf
+  
NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCmn600LibArm/SsdtCmn600LibArm.inf
   }
 
   #
diff --git a/DynamicTablesPkg/DynamicTablesPkg.ci.yaml 
b/DynamicTablesPkg/DynamicTablesPkg.ci.yaml
index 
c0d09e79fdf7f6003b5bbda45abc82a0caf4e53f..52c8c2ab4aefb21bea0289a4fd02209ae937a221
 100644
--- a/DynamicTablesPkg/DynamicTablesPkg.ci.yaml
+++ b/DynamicTablesPkg/DynamicTablesPkg.ci.yaml
@@ -70,6 +70,7 @@
  # in matching files
 "ExtendWords": [
"ARMHB",  # ARMHB000
+   "ARMHC",  # ARMHC600
"ARMLTD",
"EISAID",
"CCIDX",
@@ -81,8 +82,11 @@
"lgreater",
"lless",
"MPIDR",
+   "PERIPHBASE",
"pytool",
"Roadmap",
+   "ROOTNODEBASE",
+   "ssdtcmn",
"ssdtserialporttemplate",
"SMMUV",
"standardised",
diff --git a/DynamicTablesPkg/Include/AcpiTableGenerator.h 
b/DynamicTablesPkg/Include/AcpiTableGenerator.h
index 
ef5018c312c1abbc205a06b037ffd6063cf02f0a..352331d6dc957b664d31d55b50efcce5b90d8ada
 100644
--- a/DynamicTablesPkg/Include/AcpiTableGenerator.h
+++ b/DynamicTablesPkg/Include/AcpiTableGenerator.h
@@ -59,6 +59,10 @@ The Dynamic Tables Framework implements the following ACPI 
table generators:
 The SSDT Serial generator collates the Serial port information
 from the Configuration Manager and patches the SSDT Serial Port
 template to build the SSDT Serial port table.
+  - SSDT CMN-600:
+The SSDT CMN-600 generator collates the CMN-600 information
+from the Configuration Manager and patches the SSDT CMN-600
+template to build the SSDT CMN-600 table.
 */
 
 /** The ACPI_TABLE_GENERATOR_ID type describes ACPI table generator ID.
@@ -83,6 +87,7 @@ typedef enum StdAcpiTableId {
   EStdAcpiTableIdPptt,  ///< PPTT Generator
   EStdAcpiTableIdSrat,  

[edk2-devel] [PATCH v2 1/2] MdePkg: Definitions for Extended Interrupt Flags

[Sami Mujawar]

Add Interrupt Vector Flag definitions for Extended Interrupt
Descriptor, and macros to test the flags.
Ref: ACPI specification 6.4.3.6

Signed-off-by: Sami Mujawar 
---

Notes:
v2
  - Updated based on review comments to just define the bit [SAMI]
locations. Also dropped the IS_xxx macros.

 MdePkg/Include/IndustryStandard/Acpi10.h | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/MdePkg/Include/IndustryStandard/Acpi10.h 
b/MdePkg/Include/IndustryStandard/Acpi10.h
index 
adeb5ae8c219f31d2403fc7aa217bfb4e1e44694..7ac9b967b54dcc92f2c20366bf1ff08d67c4c971
 100644
--- a/MdePkg/Include/IndustryStandard/Acpi10.h
+++ b/MdePkg/Include/IndustryStandard/Acpi10.h
@@ -2,6 +2,7 @@
   ACPI 1.0b definitions from the ACPI Specification, revision 1.0b
 
 Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2020, Arm Limited. All rights reserved.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
 
@@ -377,6 +378,16 @@ typedef struct {
 #define   EFI_ACPI_MEMORY_NON_WRITABLE  0x00
 
 //
+// Interrupt Vector Flags definitions for Extended Interrupt Descriptor
+// Ref ACPI specification 6.4.3.6
+//
+#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_PRODUCER_CONSUMER_MASK   BIT0
+#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_MODE_MASKBIT1
+#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_POLARITY_MASKBIT2
+#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_SHARABLE_MASKBIT3
+#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_WAKE_CAPABLITY_MASK  BIT4
+
+//
 // Ensure proper structure formats
 //
 #pragma pack(1)
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'



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

Re: [edk2-devel] [edk2-platforms 3/4] Silicon/NXP: Implement USB Errata

[Leif Lindholm]

On Tue, Sep 15, 2020 at 21:59:02 +0530, Meenakshi Aggarwal wrote:
> Implement USB errata A009008, A009798, A008997, A009007
> Make USB,SEC and SATA snoopable

Somewhat nitpicking, but for both subject and message - can you say
"errata workarounds" rather than "errata"?

Ideally, I would like to see the addition of the SCFG as a separate
patch from the one that implements the workarounds.

> Signed-off-by: Meenakshi Aggarwal 
> ---
>  Silicon/NXP/NxpQoriqLs.dec |   1 +
>  Silicon/NXP/LS1046A/LS1046A.dsc.inc|   1 +
>  .../NXP/Chassis2/Library/ChassisLib/ChassisLib.inf |   2 +
>  Silicon/NXP/LS1046A/Library/SocLib/SocLib.inf  |   2 +
>  Silicon/NXP/Chassis2/Include/Chassis.h | 112 +++
>  Silicon/NXP/Chassis2/Library/ChassisLib/Erratum.h  |  23 +++
>  Silicon/NXP/Include/Library/ChassisLib.h   |  62 
>  Silicon/NXP/LS1046A/Include/Soc.h  |   2 +
>  .../NXP/Chassis2/Library/ChassisLib/ChassisLib.c   |  63 
>  Silicon/NXP/Chassis2/Library/ChassisLib/Erratum.c  | 159 
> +
>  Silicon/NXP/LS1046A/Library/SocLib/SocLib.c|  66 +
>  11 files changed, 493 insertions(+)
>  create mode 100644 Silicon/NXP/Chassis2/Library/ChassisLib/Erratum.h
>  create mode 100644 Silicon/NXP/Chassis2/Library/ChassisLib/Erratum.c
> 
> diff --git a/Silicon/NXP/NxpQoriqLs.dec b/Silicon/NXP/NxpQoriqLs.dec
> index 3a568c0437e7..90dce69fd472 100644
> --- a/Silicon/NXP/NxpQoriqLs.dec
> +++ b/Silicon/NXP/NxpQoriqLs.dec
> @@ -30,6 +30,7 @@ [PcdsFeatureFlag]
>gNxpQoriqLsTokenSpaceGuid.PcdPciLutBigEndian|FALSE|BOOLEAN|0x0317
>gNxpQoriqLsTokenSpaceGuid.PcdSataErratumA009185|FALSE|BOOLEAN|0x0318
>
> gNxpQoriqLsTokenSpaceGuid.PcdGpioControllerBigEndian|FALSE|BOOLEAN|0x0319
> +  gNxpQoriqLsTokenSpaceGuid.PcdScfgBigEndian|FALSE|BOOLEAN|0x0320
>  
>  [PcdsFixedAtBuild.common]
># Pcds for PCI Express
> diff --git a/Silicon/NXP/LS1046A/LS1046A.dsc.inc 
> b/Silicon/NXP/LS1046A/LS1046A.dsc.inc
> index db110553605f..4e1d6a7ae7a2 100644
> --- a/Silicon/NXP/LS1046A/LS1046A.dsc.inc
> +++ b/Silicon/NXP/LS1046A/LS1046A.dsc.inc
> @@ -34,6 +34,7 @@ [PcdsFixedAtBuild.common]
>  
>  [PcdsFeatureFlag]
>gNxpQoriqLsTokenSpaceGuid.PcdDcfgBigEndian|TRUE
> +  gNxpQoriqLsTokenSpaceGuid.PcdScfgBigEndian|TRUE
>gNxpQoriqLsTokenSpaceGuid.PcdGpioControllerBigEndian|TRUE
>  
>  
> 
> diff --git a/Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.inf 
> b/Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.inf
> index f5dbd1349dc5..d64286b199c6 100644
> --- a/Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.inf
> +++ b/Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.inf
> @@ -28,6 +28,8 @@ [LibraryClasses]
>  
>  [Sources.common]
>ChassisLib.c
> +  Erratum.c
>  
>  [FeaturePcd]
>gNxpQoriqLsTokenSpaceGuid.PcdDcfgBigEndian
> +  gNxpQoriqLsTokenSpaceGuid.PcdScfgBigEndian
> diff --git a/Silicon/NXP/LS1046A/Library/SocLib/SocLib.inf 
> b/Silicon/NXP/LS1046A/Library/SocLib/SocLib.inf
> index 01ed0f6592d2..e2336bb18f29 100644
> --- a/Silicon/NXP/LS1046A/Library/SocLib/SocLib.inf
> +++ b/Silicon/NXP/LS1046A/Library/SocLib/SocLib.inf
> @@ -14,6 +14,7 @@ [Defines]
>LIBRARY_CLASS  = SocLib
>  
>  [Packages]
> +  ArmPkg/ArmPkg.dec
>MdePkg/MdePkg.dec
>Silicon/NXP/Chassis2/Chassis2.dec
>Silicon/NXP/LS1046A/LS1046A.dec
> @@ -25,3 +26,4 @@ [LibraryClasses]
>  
>  [Sources.common]
>SocLib.c
> +
> diff --git a/Silicon/NXP/Chassis2/Include/Chassis.h 
> b/Silicon/NXP/Chassis2/Include/Chassis.h
> index 7e8bf224884b..f8fa7ed67596 100644
> --- a/Silicon/NXP/Chassis2/Include/Chassis.h
> +++ b/Silicon/NXP/Chassis2/Include/Chassis.h
> @@ -11,6 +11,7 @@
>  #include 
>  
>  #define  NXP_LAYERSCAPE_CHASSIS2_DCFG_ADDRESS  0x1EE
> +#define  NXP_LAYERSCAPE_CHASSIS2_SCFG_ADDRESS  0x157
>  
>  #define SVR_SOC_VER(svr)(((svr) >> 8) & 0xFE)
>  #define SVR_MAJOR(svr)  (((svr) >> 4) & 0xf)
> @@ -26,6 +27,10 @@
>  #define SCR0_CLIENTPD_MASK 0x0001
>  #define SACR_PAGESIZE_MASK 0x0001
>  
> +#define USB_PHY1_BASE_ADDRESS  0x084F
> +#define USB_PHY2_BASE_ADDRESS  0x0850
> +#define USB_PHY3_BASE_ADDRESS  0x0851
> +
>  /**
>The Device Configuration Unit provides general purpose configuration and
>status for the device. These registers only support 32-bit accesses.
> @@ -45,4 +50,111 @@ typedef struct {
>  } NXP_LAYERSCAPE_CHASSIS2_DEVICE_CONFIG;
>  #pragma pack()
>  
> +/* Supplemental Configuration Unit (SCFG) */
> +typedef struct {
> +  UINT8  Res000[0x070-0x000];
> +  UINT32 Usb1Prm1Cr;
> +  UINT32 Usb1Prm2Cr;
> +  UINT32 Usb1Prm3Cr;
> +  UINT32 Usb2Prm1Cr;
> +  UINT32 Usb2Prm2Cr;
> +  UINT32 Usb2Prm3Cr;
> +  UINT32 Usb3Prm1Cr;
> +  UINT32 Usb3Prm2Cr;
> +  UINT32 Usb3Prm3Cr;
> +  UINT8  Res094[0x100-0x094];
> +  UINT32 Usb2

Re: [edk2-devel] [edk2-platforms 4/4] LS1046aFrwy: Enable USB support for LS1046AFRWY board.

[Leif Lindholm]

On Tue, Sep 15, 2020 at 21:59:03 +0530, Meenakshi Aggarwal wrote:
> Signed-off-by: Meenakshi Aggarwal 

Reviewed-by: Leif Lindholm 

> ---
>  Silicon/NXP/LS1046A/LS1046A.dsc.inc|  3 +++
>  Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.dsc |  2 ++
>  Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.fdf | 13 +
>  3 files changed, 18 insertions(+)
>  mode change 100644 => 100755 Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.dsc
>  mode change 100644 => 100755 Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.fdf
> 
> diff --git a/Silicon/NXP/LS1046A/LS1046A.dsc.inc 
> b/Silicon/NXP/LS1046A/LS1046A.dsc.inc
> index 4e1d6a7ae7a2..7004533ed5f1 100644
> --- a/Silicon/NXP/LS1046A/LS1046A.dsc.inc
> +++ b/Silicon/NXP/LS1046A/LS1046A.dsc.inc
> @@ -31,6 +31,9 @@ [PcdsFixedAtBuild.common]
>gNxpQoriqLsTokenSpaceGuid.PcdGpioModuleBaseAddress|0x0230
>gNxpQoriqLsTokenSpaceGuid.PcdGpioControllerOffset|0x1
>  
> +  gNxpQoriqLsTokenSpaceGuid.PcdUsbBaseAddr|0x2F0
> +  gNxpQoriqLsTokenSpaceGuid.PcdUsbSize|0x10
> +  gNxpQoriqLsTokenSpaceGuid.PcdNumUsbController|3
>  
>  [PcdsFeatureFlag]
>gNxpQoriqLsTokenSpaceGuid.PcdDcfgBigEndian|TRUE
> diff --git a/Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.dsc 
> b/Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.dsc
> old mode 100644
> new mode 100755
> index 3f29dadd5d1d..266fdbd2b4d3
> --- a/Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.dsc
> +++ b/Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.dsc
> @@ -43,4 +43,6 @@ [Components.common]
>  gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable|TRUE
>}
>  
> +  Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.inf
> +
>  ##
> diff --git a/Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.fdf 
> b/Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.fdf
> old mode 100644
> new mode 100755
> index 24af547729c7..34c4e5a02516
> --- a/Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.fdf
> +++ b/Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.fdf
> @@ -120,6 +120,19 @@ [FV.FvMain]
>INF FatPkg/EnhancedFatDxe/Fat.inf
>INF MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
>  
> +  INF 
> MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf
> +
> +  #
> +  # USB Support
> +  #
> +  INF MdeModulePkg/Bus/Pci/UhciDxe/UhciDxe.inf
> +  INF MdeModulePkg/Bus/Pci/EhciDxe/EhciDxe.inf
> +  INF MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf
> +  INF MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBusDxe.inf
> +  INF MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
> +
> +  INF Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.inf
> +
>#
># UEFI application (Shell Embedded Boot Loader)
>#
> -- 
> 1.9.1
> 


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

Re: [edk2-devel] [edk2-platforms 2/4] Platform/NXP/LS1046aFrwyPkg: GPIO mux changes for USB

[Leif Lindholm]

Same comment as for previous patch:

Commit messages are not optional.

On Tue, Sep 15, 2020 at 21:59:01 +0530, Meenakshi Aggarwal wrote:
> Signed-off-by: Pramod Kumar 
> Signed-off-by: Meenakshi Aggarwal 

And only the poster can sign off.

Please address for all patches in series.

> ---
>  Silicon/NXP/NxpQoriqLs.dec  |  8 
>  Silicon/NXP/LS1046A/LS1046A.dsc.inc |  5 +
>  Silicon/NXP/NxpQoriqLs.dsc.inc  |  2 ++
>  .../Library/ArmPlatformLib/ArmPlatformLib.inf   |  1 +
>  .../Library/ArmPlatformLib/ArmPlatformLib.c | 17 
> +
>  5 files changed, 33 insertions(+)
> 
> diff --git a/Silicon/NXP/NxpQoriqLs.dec b/Silicon/NXP/NxpQoriqLs.dec
> index 0c3608696569..3a568c0437e7 100644
> --- a/Silicon/NXP/NxpQoriqLs.dec
> +++ b/Silicon/NXP/NxpQoriqLs.dec
> @@ -29,6 +29,7 @@ [PcdsFeatureFlag]
>gNxpQoriqLsTokenSpaceGuid.PcdDcfgBigEndian|FALSE|BOOLEAN|0x0316
>gNxpQoriqLsTokenSpaceGuid.PcdPciLutBigEndian|FALSE|BOOLEAN|0x0317
>gNxpQoriqLsTokenSpaceGuid.PcdSataErratumA009185|FALSE|BOOLEAN|0x0318
> +  
> gNxpQoriqLsTokenSpaceGuid.PcdGpioControllerBigEndian|FALSE|BOOLEAN|0x0319
>  
>  [PcdsFixedAtBuild.common]
># Pcds for PCI Express
> @@ -48,6 +49,13 @@ [PcdsFixedAtBuild.common]
>gNxpQoriqLsTokenSpaceGuid.PcdSataSize|0x0|UINT32|0x0351
>gNxpQoriqLsTokenSpaceGuid.PcdNumSataController|0x0|UINT32|0x0352
>  
> +  #
> +  # Pcds for Gpio
> +  #
> +  gNxpQoriqLsTokenSpaceGuid.PcdNumGpioController|0|UINT32|0x0355
> +  gNxpQoriqLsTokenSpaceGuid.PcdGpioModuleBaseAddress|0|UINT64|0x0356
> +  gNxpQoriqLsTokenSpaceGuid.PcdGpioControllerOffset|0|UINT64|0x0357
> +
>  [PcdsDynamic.common]
>gNxpQoriqLsTokenSpaceGuid.PcdPciCfgShiftEnable|FALSE|BOOLEAN|0x0600
>gNxpQoriqLsTokenSpaceGuid.PcdPciLsGen4Ctrl|FALSE|BOOLEAN|0x0601
> diff --git a/Silicon/NXP/LS1046A/LS1046A.dsc.inc 
> b/Silicon/NXP/LS1046A/LS1046A.dsc.inc
> index dbe7f408fce9..db110553605f 100644
> --- a/Silicon/NXP/LS1046A/LS1046A.dsc.inc
> +++ b/Silicon/NXP/LS1046A/LS1046A.dsc.inc
> @@ -27,9 +27,14 @@ [PcdsDynamicDefault.common]
>  
>  [PcdsFixedAtBuild.common]
>gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x021c0500
> +  gNxpQoriqLsTokenSpaceGuid.PcdNumGpioController|0x04
> +  gNxpQoriqLsTokenSpaceGuid.PcdGpioModuleBaseAddress|0x0230
> +  gNxpQoriqLsTokenSpaceGuid.PcdGpioControllerOffset|0x1
> +
>  
>  [PcdsFeatureFlag]
>gNxpQoriqLsTokenSpaceGuid.PcdDcfgBigEndian|TRUE
> +  gNxpQoriqLsTokenSpaceGuid.PcdGpioControllerBigEndian|TRUE
>  
>  
> 
>  #
> diff --git a/Silicon/NXP/NxpQoriqLs.dsc.inc b/Silicon/NXP/NxpQoriqLs.dsc.inc
> index fc600de01d74..21c87df73220 100644
> --- a/Silicon/NXP/NxpQoriqLs.dsc.inc
> +++ b/Silicon/NXP/NxpQoriqLs.dsc.inc
> @@ -103,6 +103,8 @@ [LibraryClasses.common]
>MemoryInitPeiLib|Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf
>UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
>  
> +  GpioLib|Silicon/NXP/Library/GpioLib/GpioLib.inf
> +
>  [LibraryClasses.common.SEC]
>PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
>
> DebugAgentLib|ArmPkg/Library/DebugAgentSymbolsBaseLib/DebugAgentSymbolsBaseLib.inf
> diff --git 
> a/Platform/NXP/LS1046aFrwyPkg/Library/ArmPlatformLib/ArmPlatformLib.inf 
> b/Platform/NXP/LS1046aFrwyPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> index 7802696bf39b..2e755842a714 100644
> --- a/Platform/NXP/LS1046aFrwyPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> +++ b/Platform/NXP/LS1046aFrwyPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> @@ -25,6 +25,7 @@ [Packages]
>  [LibraryClasses]
>ArmLib
>DebugLib
> +  GpioLib
>SocLib
>  
>  [Sources.common]
> diff --git 
> a/Platform/NXP/LS1046aFrwyPkg/Library/ArmPlatformLib/ArmPlatformLib.c 
> b/Platform/NXP/LS1046aFrwyPkg/Library/ArmPlatformLib/ArmPlatformLib.c
> index e1f20da09337..d467992a3e47 100644
> --- a/Platform/NXP/LS1046aFrwyPkg/Library/ArmPlatformLib/ArmPlatformLib.c
> +++ b/Platform/NXP/LS1046aFrwyPkg/Library/ArmPlatformLib/ArmPlatformLib.c
> @@ -8,11 +8,14 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #include 
>  #include 
>  
> +#define USB2_MUX_SEL_GPIO23
> +
>  ARM_CORE_INFO mLS1046aMpCoreInfoTable[] = {
>{
>  // Cluster 0, Core 0
> @@ -89,6 +92,19 @@ NxpPlatformGetClock(
>  }
>  
>  /**
> +  FRWY-LS1046A GPIO 23 use for USB2
> +  mux seclection
> +**/
> +STATIC VOID  MuxSelectUsb2 (VOID)
> +{
> +
> +  SetDir (GPIO3, USB2_MUX_SEL_GPIO, OUTPUT);
> +  SetData (GPIO3, USB2_MUX_SEL_GPIO, HIGH);

Oh, I didn't spot in previous patch that these functions were exported
(my bad). They need more global names then - at least
GpioSetDirection/GpioSetData.

/
Leif

> +
> +  return;
> +}
> +
> +/**
>Initialize controllers that must setup in the normal world
>  
>This function is called by the ArmPlatformPkg/

[edk2-devel] *yet more* development process failure

[Laszlo Ersek]

Hi,

so check out this pull request:

https://github.com/tianocore/edk2/pull/960

- empty PR description
- useless PR title ("pr925")
- merges patches for *five* different BZs
  (2882, 2965, 2880, 2978, 2881) in one go
- all of those BZs are still in CONFIRMED state
- none of those BZs reference the corresponding patches
  in the mailing list archive.

So, all of this, *right after* the last two discussions on edk2-devel:

* development process failure
  https://edk2.groups.io/g/devel/message/65313

* more development process failure
  https://edk2.groups.io/g/devel/message/65315

Why is it that people that utterly disregard the process are allowed to
modify the master branch?

And don't give me the "lacking documentation" excuse. I'm not buying
that. The on-list discussions linked above are NINE DAYS old.

Instead, some maintainers just don't give a shit about the community,
and/or the exchanges on the list, they just do whatever they damn pleae.

Laszlo



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

Re: [edk2-devel] [edk2-platforms 1/4] Silicon/NXP: Add GPIO driver support.

[Leif Lindholm]

Needs an actual commit message.
What GPIO controller? If it does not have an explicit name, what
family of devices is it in?

On Tue, Sep 15, 2020 at 21:59:00 +0530, Meenakshi Aggarwal wrote:
> Signed-off-by: Pramod Kumar 
> Signed-off-by: Meenakshi Aggarwal 

Only the poster can sign off that the post is being submitted in
accordance with the Developer's Certificate of Origin:
https://developercertificate.org/

If the author is different from the poster, that should be described
in the patch metadata (i.e. Author: ).

I have permitted (although I'm not a fan) Co-authored-by: tags, if
that is what this is intended to describe.

> ---
>  Silicon/NXP/Library/GpioLib/GpioLib.inf |  39 +
>  Silicon/NXP/Include/Library/GpioLib.h   | 110 +++
>  Silicon/NXP/Library/GpioLib/GpioLib.c   | 242 
> 
>  3 files changed, 391 insertions(+)
>  create mode 100644 Silicon/NXP/Library/GpioLib/GpioLib.inf
>  create mode 100644 Silicon/NXP/Include/Library/GpioLib.h
>  create mode 100644 Silicon/NXP/Library/GpioLib/GpioLib.c
> 
> diff --git a/Silicon/NXP/Library/GpioLib/GpioLib.inf 
> b/Silicon/NXP/Library/GpioLib/GpioLib.inf
> new file mode 100644
> index ..7878d1d03db2
> --- /dev/null
> +++ b/Silicon/NXP/Library/GpioLib/GpioLib.inf
> @@ -0,0 +1,39 @@
> +/** @file
> +
> + Copyright 2020 NXP
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +[Defines]
> +  INF_VERSION= 0x0001001A
> +  BASE_NAME  = GpioLib
> +  FILE_GUID  = addec2b8-d2e0-43c0-a277-41a8d42f3f4f
> +  MODULE_TYPE= BASE
> +  VERSION_STRING = 1.0
> +  LIBRARY_CLASS  = GpioLib
> +
> +[Sources.common]
> +  GpioLib.c
> +
> +[LibraryClasses]
> +  ArmLib
> +  BaseMemoryLib
> +  BaseLib

Flip order of above two lines.

> +  IoAccessLib
> +  IoLib
> +
> +[Packages]
> +  ArmPlatformPkg/ArmPlatformPkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  Silicon/NXP/NxpQoriqLs.dec
> +
> +[Pcd]
> +  gNxpQoriqLsTokenSpaceGuid.PcdNumGpioController
> +  gNxpQoriqLsTokenSpaceGuid.PcdGpioModuleBaseAddress
> +  gNxpQoriqLsTokenSpaceGuid.PcdGpioControllerOffset
> +
> +[FeaturePcd]
> +  gNxpQoriqLsTokenSpaceGuid.PcdGpioControllerBigEndian
> diff --git a/Silicon/NXP/Include/Library/GpioLib.h 
> b/Silicon/NXP/Include/Library/GpioLib.h
> new file mode 100644
> index ..5821806226ee
> --- /dev/null
> +++ b/Silicon/NXP/Include/Library/GpioLib.h
> @@ -0,0 +1,110 @@
> +/** @file
> +
> + Copyright 2020 NXP
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#ifndef GPIO_H__
> +#define GPIO_H__
> +
> +#include 
> +
> +/* enum for GPIO number */
> +typedef enum _GPIO_BLOCK {
> +  GPIO1,
> +  GPIO2,
> +  GPIO3,
> +  GPIO4,
> +  GPIO_MAX
> +} GPIO_BLOCK;
> +
> +/* enum for GPIO direction */
> +typedef enum _GPIO_DIRECTION {
> +  INPUT,
> +  OUTPUT
> +} GPIO_DIRECTION;
> +
> +/* enum for GPIO state */
> +typedef enum _GPIO_STATE {
> +  LOW,
> +  HIGH
> +} GPIO_VAL;
> +
> +/**
> +   SetDir Set GPIO direction as INPUT or OUTPUT
> +
> +   @param[in] Id  GPIO controller number
> +   @param[in] Bit GPIO number
> +   @param[in] Dir GPIO Direction as INPUT or OUTPUT
> +
> +   @retval EFI_SUCCESS
> + **/
> +EFI_STATUS
> +SetDir (
> +  IN  UINT8Id,
> +  IN  UINT32   Bit,
> +  IN  BOOLEAN  Dir
> +  );
> +
> +/**
> +   GetDir  Retrieve GPIO direction
> +
> +   @param[in] Id  GPIO controller number
> +   @param[in] Bit GPIO number
> +
> +   @retval GPIO Direction as INPUT or OUTPUT
> + **/
> +UINT32
> +GetDir (
> +  IN  UINT8Id,
> +  IN  UINT32   Bit
> +  );
> +
> + /**
> +   GetData  Retrieve GPIO Value
> +
> +   @param[in] Id  GPIO controller number
> +   @param[in] Bit GPIO number
> +
> +   @retval GPIO value  as HIGH or LOW
> + **/
> +UINT32
> +GetData (
> +  IN  UINT8Id,
> +  IN  UINT32   Bit
> +  );
> +
> +/**
> +   SetData  Set GPIO data Value
> +
> +   @param[in] Id  GPIO controller number
> +   @param[in] Bit GPIO number
> +   @param[in] Data GPIO data value to set
> +
> +   @retval GPIO value  as HIGH or LOW
> + **/
> +EFI_STATUS
> +SetData (
> +  IN  UINT8Id,
> +  IN  UINT32   Bit,
> +  IN  BOOLEAN  Data
> +  );
> +
> +/**
> +   SetOpenDrain  Set GPIO as Open drain
> +
> +   @param[in] Id  GPIO controller number
> +   @param[in] Bit GPIO number
> +   @param[in] OpenDrain Set as open drain
> +
> +   @retval EFI_SUCCESS
> + **/
> +EFI_STATUS
> +SetOpenDrain (
> +  IN  UINT8Id,
> +  IN  UINT32   Bit,
> +  IN  BOOLEAN  OpenDrain
> +  );
> +
> +#endif
> diff --git a/Silicon/NXP/Library/GpioLib/GpioLib.c 
> b/Silicon/NXP/Library/GpioLib/GpioLib.c
> new file mode 100644
> index ..33cc45c2152b
> --- /dev/null
> +++ b/Silicon/NXP/Library/GpioLib/GpioLib.c
> @@ -0,0 +1,242 @@
> +/** @file
> +

Which controller is this for. There should be a comment here
sufficient for me to figure out where I should start looking for
docu

[edk2-devel] [PATCH v1 1/1] UefiCpuPkg: Remove PEI/DXE instances of CpuTimerLib.

[jasonlouyun]

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

1. Remove PEI instance(PeiCpuTimerLib).
PeiCpuTimerLib is currently designed to save time by getting CPU TSC frequncy 
from Hob.
BaseCpuTimerLib is designed to calculate TSC frequency by using CPUID[15h] each 
time.
The time it takes to find CpuCrystalFrequencyHob (about 2000ns) is much longer 
than it takes to
calculate TSC frequency with CPUID[15h] (about 450ns), which means using 
BaseCpuTimerLib to
trigger a delay is more accurate than using PeiCpuTimerLib, recommend to use 
BaseCpuTimerLib
instead of PeiCpuTimerLib.

2. Remove DXE instance(DxeCpuTimerLib).
DxeCpuTimerLib is designed to calculate TSC frequency with CPUID[15h] in its 
constructor function,
then save it in a global variable. For this design, once the driver containing 
this instance is
running, the constructor function is called, it will take extra time to 
calculate TSC frequency.
The time it takes to get TSC frequncy from global variable is shorter than it 
takes to calculate
TSC frequency with CPUID[15h], but 450ns is a short time, the impact on the 
platform is very limited.
In addition, in order to simplify the code, recommend to use BaseCpuTimerLib 
instead of DxeCpuTimerLib.

I did some experiments on one Intel server platform and collected the following 
data:
1. Average time taken to find CpuCrystalFrequencyHob: about 2000 ns.
2. Average time taken to calculate TSC frequency: about 450 ns.

Reference code:
//
// Calculate average time taken to find Hob.
//
DEBUG((DEBUG_ERROR, "[PeiCpuTimerLib] GetPerformanceCounterFrequency - 
GetFirstGuidHob (1000 cycles)\n"));
Ticks1 = AsmReadTsc();
for (i = 0; i < 1000; i++) {
  GuidHob = GetFirstGuidHob (&mCpuCrystalFrequencyHobGuid);
}
Ticks2 = AsmReadTsc();

if (GuidHob == NULL) {
  DEBUG((DEBUG_ERROR, "[PeiCpuTimerLib]  - CpuCrystalFrequencyHob can not 
be found!\n"));
} else {
  DEBUG((DEBUG_ERROR, "[PeiCpuTimerLib]  - Average time taken to find Hob = 
%d ns\n", \
  DivU64x32(DivU64x64Remainder(MultU64x32((Ticks2 - Ticks1), 
10), *CpuCrystalCounterFrequency, NULL), 1000)));
}

//
// Calculate average time taken to calculate CPU frequency.
//
DEBUG((DEBUG_ERROR, "[PeiCpuTimerLib] GetPerformanceCounterFrequency - 
CpuidCoreClockCalculateTscFrequency (1000 cycles)\n"));
Ticks1 = AsmReadTsc();
for (i = 0; i < 1000; i++) {
  Freq = CpuidCoreClockCalculateTscFrequency ();
}
Ticks2 = AsmReadTsc();
DEBUG((DEBUG_ERROR, "[PeiCpuTimerLib]  - Average time taken to calculate 
TSC frequency = %d ns\n", \
DivU64x32(DivU64x64Remainder(MultU64x32((Ticks2 - Ticks1), 10), 
*CpuCrystalCounterFrequency, NULL), 1000)));

Signed-off-by: Jason Lou 
Cc: Ray Ni 
Cc: Eric Dong 
Cc: Laszlo Ersek 
Cc: Rahul Kumar 
---
 UefiCpuPkg/Library/CpuTimerLib/DxeCpuTimerLib.c   | 85 
 UefiCpuPkg/Library/CpuTimerLib/PeiCpuTimerLib.c   | 58 -
 UefiCpuPkg/Library/CpuTimerLib/DxeCpuTimerLib.inf | 37 -
 UefiCpuPkg/Library/CpuTimerLib/DxeCpuTimerLib.uni | 17 
 UefiCpuPkg/Library/CpuTimerLib/PeiCpuTimerLib.inf | 36 -
 UefiCpuPkg/Library/CpuTimerLib/PeiCpuTimerLib.uni | 17 
 UefiCpuPkg/UefiCpuPkg.dsc |  4 +-
 7 files changed, 1 insertion(+), 253 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuTimerLib/DxeCpuTimerLib.c 
b/UefiCpuPkg/Library/CpuTimerLib/DxeCpuTimerLib.c
deleted file mode 100644
index 269e5a3e83d7..
--- a/UefiCpuPkg/Library/CpuTimerLib/DxeCpuTimerLib.c
+++ /dev/null
@@ -1,85 +0,0 @@
-/** @file
-  CPUID Leaf 0x15 for Core Crystal Clock frequency instance of Timer Library.
-
-  Copyright (c) 2019 Intel Corporation. All rights reserved.
-  SPDX-License-Identifier: BSD-2-Clause-Patent
-
-**/
-
-#include 
-#include 
-#include 
-#include 
-
-extern GUID mCpuCrystalFrequencyHobGuid;
-
-/**
-  CPUID Leaf 0x15 for Core Crystal Clock Frequency.
-
-  The TSC counting frequency is determined by using CPUID leaf 0x15. Frequency 
in MHz = Core XTAL frequency * EBX/EAX.
-  In newer flavors of the CPU, core xtal frequency is returned in ECX or 0 if 
not supported.
-  @return The number of TSC counts per second.
-
-**/
-UINT64
-CpuidCoreClockCalculateTscFrequency (
-  VOID
-  );
-
-//
-// Cached CPU Crystal counter frequency
-//
-UINT64  mCpuCrystalCounterFrequency = 0;
-
-
-/**
-  Internal function to retrieves the 64-bit frequency in Hz.
-
-  Internal function to retrieves the 64-bit frequency in Hz.
-
-  @return The frequency in Hz.
-
-**/
-UINT64
-InternalGetPerformanceCounterFrequency (
-  VOID
-  )
-{
-  return mCpuCrystalCounterFrequency;
-}
-
-/**
-  The constructor function is to initialize CpuCrystalCounterFrequency.
-
-  @param  ImageHandle   The firmware allocated handle for the EFI image.
-  @param  SystemTable   A pointer to the EFI System Table.
-
-  @retval EFI_SUCCESS   The constructor always returns RETURN_SUCCESS.
-
-**/
-EFI_STATUS
-EFIAPI
-Dx

Re: [edk2-devel][PATCH] EmulatorPkg/Unix Prevents the compiler form optimizing unused variable

[Ni, Ray]

Jordan, Andrew,
I will defer to you to review the patch.

Thanks,
Ray

> -Original Message-
> From: LiuYu 
> Sent: Friday, September 25, 2020 1:49 PM
> To: Justen, Jordan L ; af...@apple.com; Ni, Ray 
> 
> Cc: devel@edk2.groups.io; LiuYu 
> Subject: [edk2-devel][PATCH] EmulatorPkg/Unix Prevents the compiler form 
> optimizing unused variable
> 
> gInXcode is only used by GDB script and if optimization is turned on then 
> compiler
> treats this variable as unused so it can't been linked in the final object.
> 
> Signed-off-by: LiuYu 
> ---
>  EmulatorPkg/Unix/Host/Host.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/EmulatorPkg/Unix/Host/Host.c b/EmulatorPkg/Unix/Host/Host.c
> index b4e5510613..b851264c8e 100644
> --- a/EmulatorPkg/Unix/Host/Host.c
> +++ b/EmulatorPkg/Unix/Host/Host.c
> @@ -54,7 +54,7 @@ IMAGE_CONTEXT_TO_MOD_HANDLE  *mImageContextModHandleArray = 
> NULL;
>  EFI_PEI_PPI_DESCRIPTOR  *gPpiList;
> 
> 
> -int gInXcode = 0;
> +int gInXcode  __attribute__((used)) = 0;
> 
> 
>  /*++
> --
> 2.20.1



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

回复: [edk2-devel] [PATCH v2 0/2] UEFI memmap workaround for hiding page-access caps from OSes hides SP and CRYPTO caps too

[gaoliming]

Malgorzata: 
  How do know OS (Windows or Linux) behavior for SP and CRYPTO attribute? Is
there the public document to describe this behavior?

Thanks
Liming
> -邮件原件-
> 发件人: bounce+27952+65566+4905953+8761...@groups.io
>  代表 Malgorzata
> Kukiello
> 发送时间: 2020年9月24日 18:22
> 收件人: devel@edk2.groups.io
> 抄送: Malgorzata Kukiello ; Michael D Kinney
> ; Jian J Wang ; Hao A
> Wu ; Dandan Bi ; Liming Gao
> ; Zhiguang Liu ;
> Oleksiy Yakovlev ; Ard Biesheuvel
> 
> 主题: [edk2-devel] [PATCH v2 0/2] UEFI memmap workaround for hiding
> page-access caps from OSes hides SP and CRYPTO caps too
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2982
> 
> The workaround in the UEFI memmap construction, near the end of the
> function CoreGetMemoryMap() [MdeModulePkg/Core/Dxe/Mem/Page.c]
> should
> not clear the SP and CRYPTO bits, because OSes do (apparently) correctly
> interpret SP and CRYPTO as capabilities, and not as currently set
> attributes (upon which the OSes should set their page tables). For this
> reason, the SP and CRYPTO bits should be separated from the bitmask that
> we use for hiding the page-access attributes, in the workaround
> 
> Signed-off-by: Malgorzata Kukiello 
> Cc: Michael D Kinney 
> Cc: Jian J Wang 
> Cc: Hao A Wu 
> Cc: Dandan Bi 
> Cc: Liming Gao 
> Cc: Zhiguang Liu 
> Cc: Oleksiy Yakovlev 
> Cc: Ard Biesheuvel (ARM address) 
> 
>  MdeModulePkg/Core/Dxe/Mem/Page.c | 12 ++--
>  MdePkg/Include/Uefi/UefiSpec.h   |  3 ++-
>  2 files changed, 8 insertions(+), 7 deletions(-)
> -
> Intel Technology Poland sp. z o.o.
> ul. Sowackiego 173 | 80-298 Gdask | Sd Rejonowy Gdask Pnoc | VII Wydzia
> Gospodarczy Krajowego Rejestru Sdowego - KRS 101882 | NIP 957-07-52-316
> | Kapita zakadowy 200.000 PLN.
> Ta wiadomo wraz z zacznikami jest przeznaczona dla okrelonego adresata i
> moe zawiera informacje poufne. W razie przypadkowego otrzymania tej
> wiadomoci, prosimy o powiadomienie nadawcy oraz trwae jej usunicie;
> jakiekolwiek przegldanie lub rozpowszechnianie jest zabronione.
> This e-mail and any attachments may contain confidential material for the
> sole use of the intended recipient(s). If you are not the intended
recipient,
> please contact the sender and delete all copies; any review or
distribution by
> others is strictly prohibited.
> 
> 
> 
> 
> 
> 





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

Re: [edk2-devel] [edk2-test] Contributions.txt still mentions the TianoCore Contribution Agreement

[Laszlo Ersek]

On 09/24/20 23:42, Rebecca Cran wrote:
> While working on the SctPkg, I noticed the Contributions.txt file in the
> top of the repo still mentions the TianoCore Contribution Agreement.
> Does it need updated?
> 
> 

(CC'ing the other stewards)

Most likely so.

It's probably best to "replay" (or "port") edk2 commit 3806e1fd1397 to
edk2-test.

The "TianoCore Contribution Agreement" should be mentioned indirectly,
if at all, namely via a reference to edk2's "License-History.txt".


Note that "BaseTools/Scripts/PatchCheck.py" in edk2 already rejects
commit messages that contain "Contributed-under" lines; see the
"check_contributed_under" method.

Thanks
Laszlo



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

Re: [edk2-devel] What is a special documentation block?

[Laszlo Ersek]

On 09/25/20 03:16, Bret Barkelew via groups.io wrote:
> Expect a few of these questions as I update 2000+ lines of test code that was 
> written prior to EccCheck.
> 
> “The function headers should follow Doxygen special documentation blocks in 
> section 2.3.5�
> Doxygen doesn’t have a section 2.3.5.
> Nor does the EDKII coding standard.

It probably refers to the syntax that we use for the leading comment
blocks on functions.

"/**" and "**/" for start and finish, @param[in], @param[out], @retval
VALUE, @return, and so on.

https://www.doxygen.nl/manual/commands.html

Thanks
Laszlo



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

Re: [edk2-devel] ECC: main function entry point in host-based unit tests

[Laszlo Ersek]

On 09/25/20 04:38, Bret Barkelew via groups.io wrote:
> ERROR - EFI coding style error
> ERROR - *Error code: 7001
> ERROR - *There should be no use of int, unsigned, char, void, long in any .c, 
> .h or .asl files
> ERROR - *file: 
> //home/corthon/_uefi/edk2_qemu_ci/edk2/MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicyUnitTest.c
> ERROR - *Line number: 763
> ERROR - *[main] Return type int
> ERROR -
> ERROR - EFI coding style error
> ERROR - *Error code: 8006
> ERROR - *Function name does not follow the rules: 1. First character should 
> be upper case 2. Must contain lower case characters 3. No white space 
> characters
> ERROR - *file: 
> //home/corthon/_uefi/edk2_qemu_ci/edk2/MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicyUnitTest.c
> ERROR - *Line number: 2253
> ERROR - *The function name [main] does not follow the rules
> 
> Currently, the host-based unit tests are using a standard C entry point:
> int
> main ()

"int main()" is not the standard C entry point. Two prototypes for main() are 
standard:

int main(void);
int main(int argc, char *argv[]);

where
- "int" may be replaced with a typedef name that's defined as "int",
- and *argv[] may be spelled as **argv too.


... Of course, this doesn't address your main point.

ECC#7001 can be suppressed perhaps if you use INT32 rather than "int".

ECC#8006 can be suppressed perhaps with a macro that expands to "main".


Another -- likely better -- idea:

(1) replace the current main() function prototype with

INT32
UnitTestMain (
  IN INT32 ArgC,
  IN UINT8 **ArgV
  );

(2) Introduce

  MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/UnitTestMain.h

such that it only declare the above prototype.

(3) Introduce

  MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/UnitTestEntry.c

with the following contents:

  #include "UnitTestMain.h"

  int main(int argc, char **argv)
  {
return UnitTestMain(argc, (UINT8 **)argv);
  }

(4) List both new source files in

  
MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicyUnitTest.inf

in the [Sources] section.

(5) Add

  MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/UnitTestEntry.c

to "EccCheck.IgnoreFiles" in "MdeModulePkg/MdeModulePkg.ci.yaml".

Thanks
Laszlo




> 
> That’s going to break both of these.
> 
> Another thing to override/figure out for host-based tests
> 
> - Bret
> 
> 
> 
> 
> 
> 
> 



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

Re: [edk2-devel] [PATCH v1 1/2] MdePkg: Definitions for Extended Interrupt Flags

[Sami Mujawar]

Hi Mike,

I will send an updated patch with the suggested changes. I will also drop the 
IS_xxx macros.

Regards,

Sami Mujawar

-Original Message-
From: Kinney, Michael D  
Sent: 24 September 2020 11:07 PM
To: Sami Mujawar ; devel@edk2.groups.io; Kinney, Michael 
D 
Cc: gaolim...@byosoft.com.cn; Liu, Zhiguang ; Alexei 
Fedorov ; Pierre Gondois ; 
Matteo Carlini ; Ben Adderson ; 
nd 
Subject: RE: [PATCH v1 1/2] MdePkg: Definitions for Extended Interrupt Flags

Hi Sami,

The set of define values looks more complex than needed for single bit field 
checks.

Perhaps we can simplify to just defining the bit locations and the IS_ macros 
check
for 0 or non-zer0 results?

#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_PRODUCER_CONSUMER_MASK   BIT0
#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_MODE_MASKBIT1
#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_POLARITY_MASKBIT2
#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_SHARABLE_MASKBIT3
#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_WAKE_CAPABLITY_MASK  BIT4

#define IS_EXTENDED_INTERRUPT_CONSUMER(Flag)  \
  (((Flag) & EFI_ACPI_EXTENDED_INTERRUPT_FLAG_PRODUCER_CONSUMER_MASK) != 0)

#define IS_EXTENDED_INTERRUPT_EDGE_TRIGGERED(Flag)\
  (((Flag) & EFI_ACPI_EXTENDED_INTERRUPT_FLAG_MODE_MASK) != 0)

Are the IS_ macros really required?  I do not see those for other bit
fields in ACPI structs.

Thanks,

Mike

> -Original Message-
> From: Sami Mujawar 
> Sent: Tuesday, September 22, 2020 7:08 AM
> To: devel@edk2.groups.io
> Cc: Sami Mujawar ; Kinney, Michael D 
> ; gaolim...@byosoft.com.cn; Liu, Zhiguang
> ; alexei.fedo...@arm.com; pierre.gond...@arm.com; 
> matteo.carl...@arm.com; ben.adder...@arm.com; n...@arm.com
> Subject: [PATCH v1 1/2] MdePkg: Definitions for Extended Interrupt Flags
> 
> Add Interrupt Vector Flag definitions for Extended Interrupt
> Descriptor, and macros to test the flags.
> Ref: ACPI specification 6.4.3.6
> 
> Signed-off-by: Sami Mujawar 
> ---
>  MdePkg/Include/IndustryStandard/Acpi10.h | 85 
>  1 file changed, 85 insertions(+)
> 
> diff --git a/MdePkg/Include/IndustryStandard/Acpi10.h 
> b/MdePkg/Include/IndustryStandard/Acpi10.h
> index 
> adeb5ae8c219f31d2403fc7aa217bfb4e1e44694..fa3f0694b9cf80bf9c1a325099a970b9cf8c1426
>  100644
> --- a/MdePkg/Include/IndustryStandard/Acpi10.h
> +++ b/MdePkg/Include/IndustryStandard/Acpi10.h
> @@ -2,6 +2,7 @@
>ACPI 1.0b definitions from the ACPI Specification, revision 1.0b
> 
>  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
> +Copyright (c) 2020, Arm Limited. All rights reserved.
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>  **/
> 
> @@ -377,6 +378,90 @@ typedef struct {
>  #define   EFI_ACPI_MEMORY_NON_WRITABLE  0x00
> 
>  //
> +// Interrupt Vector Flags definitions for Extended Interrupt Descriptor
> +// Ref ACPI specification 6.4.3.6
> +//
> +#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_PRODUCER_CONSUMER_MASK   BIT0
> +#define   EFI_ACPI_EXTENDED_INTERRUPT_FLAG_PRODUCER   0
> +#define   EFI_ACPI_EXTENDED_INTERRUPT_FLAG_CONSUMER   BIT0
> +
> +#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_MODE_MASKBIT1
> +#define   EFI_ACPI_EXTENDED_INTERRUPT_FLAG_LEVEL_TRIGGERED0
> +#define   EFI_ACPI_EXTENDED_INTERRUPT_FLAG_EDGE_TRIGGERED BIT1
> +
> +#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_POLARITY_MASKBIT2
> +#define   EFI_ACPI_EXTENDED_INTERRUPT_FLAG_ACTIVE_HIGH0
> +#define   EFI_ACPI_EXTENDED_INTERRUPT_FLAG_ACTIVE_LOW BIT2
> +
> +#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_SHARABLE_MASKBIT3
> +#define   EFI_ACPI_EXTENDED_INTERRUPT_FLAG_EXCLUSIVE  0
> +#define   EFI_ACPI_EXTENDED_INTERRUPT_FLAG_SHARED BIT3
> +
> +#define EFI_ACPI_EXTENDED_INTERRUPT_FLAG_WAKE_CAPABLITY_MASK  BIT4
> +#define   EFI_ACPI_EXTENDED_INTERRUPT_FLAG_NOT_WAKE_CAPABLE   0
> +#define   EFI_ACPI_EXTENDED_INTERRUPT_FLAG_WAKE_CAPABLE   BIT4> +
> +/* Helper macros to test Extended Interrupt Resource descriptor flags.
> +*/
> +
> +/** Test the Extended Interrupt flags to determine if the Device
> +is an Interrupt Consumer or Producer.
> +
> +  @param [in]  Flag   Extended Interrupt Resource descriptor flag.
> +
> +  @retval TRUEDevice is Interrupt Consumer.
> +  @retval FALSE   Device is Interrupt Producer.
> +*/
> +#define IS_EXTENDED_INTERRUPT_CONSUMER(Flag)  \
> +  (((Flag) & EFI_ACPI_EXTENDED_INTERRUPT_FLAG_CONSUMER) ==\
> +EFI_ACPI_EXTENDED_INTERRUPT_FLAG_CONSUMER)
> +
> +/** Test if the Extended Interrupt is Edge or Level triggered.
> +
> +  @param [in]  Flag   Extended Interrupt Resource descriptor flag.
> +
> +  @retval TRUEInterrupt is Edge triggered.
> +  @retval FALSE   Interrupt is Level triggered.
> +*/
> +#define IS_EXTENDED_INTERRUPT_EDGE_TRIGGERED(Flag)\
> +  (((Flag) & EFI

[edk2-devel] [PATCH v2 1/1] BaseTools: Copy PACKED definition from MdePkg Base.h

[gaoliming]

From: gaoliming 

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

MdePkg Acpi10.h definition depends on PACKED.
When structure PCD refers to Acpi10.h, build will fail,
because PACKED definition is missing in BaseTools BaseTypes.h.

C source tools include BaseTools BaseTypes.h. They don't include MdePkg Base.h. 
When C source tools include MdePkg Acpi10.h, they also need PACKED definition. 
So, add PACKED definition into BaseTools BaseTypes.h.

Cc: Bob Feng 
Cc: Yuwei Chen 
Cc: Michael Kinney 
Signed-off-by: Liming Gao 
---
 V2: update the commit message.
 BaseTools/Source/C/Include/Common/BaseTypes.h | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/BaseTools/Source/C/Include/Common/BaseTypes.h 
b/BaseTools/Source/C/Include/Common/BaseTypes.h
index 31d0662085a8..150980b4c0bf 100644
--- a/BaseTools/Source/C/Include/Common/BaseTypes.h
+++ b/BaseTools/Source/C/Include/Common/BaseTypes.h
@@ -57,6 +57,16 @@
 #define NULL  ((VOID *) 0)
 #endif
 
+#ifdef __CC_ARM
+  //
+  // Older RVCT ARM compilers don't fully support #pragma pack and require 
__packed
+  // as a prefix for the structure.
+  //
+  #define PACKED  __packed
+#else
+  #define PACKED
+#endif
+
 //
 //  Support for variable length argument lists using the ANSI standard.
 //
-- 
2.27.0.windows.1




GitPatchExtractor 1.1



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

[edk2-devel] [PATCH] BaseTools Build_Rule: Add the missing ASM16_FLAGS for ASM16 source file

[gaoliming]

Signed-off-by: Liming Gao 
Cc: Bob Feng 
Cc: Yuwei Chen 
---
 BaseTools/Conf/build_rule.template | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Conf/build_rule.template 
b/BaseTools/Conf/build_rule.template
index c034869915..1395792cd6 100755
--- a/BaseTools/Conf/build_rule.template
+++ b/BaseTools/Conf/build_rule.template
@@ -515,7 +515,7 @@
 "$(PP)" $(DEPS_FLAGS) $(PP_FLAGS) $(INC) ${src} > 
${d_path}(+)${s_base}.ii
 Trim --source-code --convert-hex --trim-long -o 
${d_path}(+)${s_base}.iii ${d_path}(+)${s_base}.ii
 cd $(OUTPUT_DIR)(+)${s_dir}
-"$(ASM16)" /nologo /c /omf $(INC) 
/Fo$(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj ${d_path}(+)${s_base}.iii
+"$(ASM16)" /nologo /c /omf $(ASM16_FLAGS) $(INC) 
/Fo$(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj ${d_path}(+)${s_base}.iii
 "$(ASMLINK)" $(ASMLINK_FLAGS) 
$(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj,${dst}
 
 
-- 
2.27.0.windows.1




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

Re: [edk2-devel] ECC: Won't somebody PLEASE think of the... test structures.

[Laszlo Ersek]

On 09/25/20 04:25, Bret Barkelew via groups.io wrote:
> So for context, this is a new host-based test that should only run
> within a platform OS, so intrinsics aren't the big deal that they
> would be in FW code.
>
> But we do need to figure out how to simultaneously adhere to the
> coding convention while enabling test authoring.

The EvaluatePolicyMatch() function -- very helpfully -- takes a
const-qualified pointer to VARIABLE_POLICY_ENTRY.

That allows me to see at once that the "MatchCheckPolicy" variable is
not going to be modified, in the PoliciesShouldMatchByNameAndGuid()
function
[MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicyUnitTest.c].

So the solution is to give "MatchCheckPolicy" static storage duration
rather than automatic. For good measure, make it CONST too:

> diff --git 
> a/MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicyUnitTest.c
>  
> b/MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicyUnitTest.c
> index 40e946a73814..ab05989c36e7 100644
> --- 
> a/MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicyUnitTest.c
> +++ 
> b/MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicyUnitTest.c
> @@ -330,7 +330,7 @@ PoliciesShouldMatchByNameAndGuid (
>IN UNIT_TEST_CONTEXT  Context
>)
>  {
> -  SIMPLE_VARIABLE_POLICY_ENTRY   MatchCheckPolicy = {
> +  STATIC CONST SIMPLE_VARIABLE_POLICY_ENTRY   MatchCheckPolicy = {
>  {
>VARIABLE_POLICY_ENTRY_REVISION,
>sizeof(VARIABLE_POLICY_ENTRY) + sizeof(TEST_VAR_1_NAME),

In my opinion, this is an improvement *regardless* of the restrictions
that edk2 places on compiler intrinsics. Namely, even in a hosted C
environment, the actual data content needs to exist *somewhere* in the
executable file, and correspondingly, in the executing process. And so
when the struct with automatic storage duration is being initialized,
there is either a large quick copy (cue the intrinsic), or -- naively --
a bunch of member-wise assignments behind the curtain (where the
initializer values for the scalar fields could even be encoded as
constants in the instruction stream).

Either way, the data comes from *somewhere* -- so why not just *label*
that original data with a name, and refer to the data by that name?

And that's exactly what STATIC CONST does.

(I realize the actual data content might end up with a different
representation and/or in a different section of the on-disk executable,
but that fact does not invalidate my point.)


In case you do need the variable to be writeable and/or to have auto
storage duration (e.g., because you need to fix up a few fields before
use), the common pattern in edk2 is to have a template object (usually
at file scope, not at function scope), such as:

  STATIC CONST SIMPLE_VARIABLE_POLICY_ENTRY mMatchCheckPolicy = { ... };

(note the "m" prefix on the name).

Subsequently, wherever you need a writeable copy (local variable, or
dynamically allocated object), use CopyMem() or AllocateCopyPool(),
respectively. Then fix up any fields that require that.

Thanks,
Laszlo



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

回复: [edk2-devel] ECC: main function entry point in host-based unit tests

[gaoliming]

Bret:

 ECC mainly checks the firmware source coding style. Host based unit test
code may follow the different rule. I suggest to ignore ECC check result for
them. The ignored file or directory can be added into the Package level
ci.yaml file.

 

Thanks

Liming

发件人: bounce+27952+65601+4905953+8761...@groups.io
 代表 Bret Barkelew via
groups.io
发送时间: 2020年9月25日 10:39
收件人: devel@edk2.groups.io
主题: [edk2-devel] ECC: main function entry point in host-based unit tests

 

ERROR - EFI coding style error

ERROR - *Error code: 7001

ERROR - *There should be no use of int, unsigned, char, void, long in any
.c, .h or .asl files

ERROR - *file:
//home/corthon/_uefi/edk2_qemu_ci/edk2/MdeModulePkg/Library/VariablePolicyLi
b/VariablePolicyUnitTest/VariablePolicyUnitTest.c

ERROR - *Line number: 763

ERROR - *[main] Return type int

ERROR - 

ERROR - EFI coding style error

ERROR - *Error code: 8006

ERROR - *Function name does not follow the rules: 1. First character should
be upper case 2. Must contain lower case characters 3. No white space
characters

ERROR - *file:
//home/corthon/_uefi/edk2_qemu_ci/edk2/MdeModulePkg/Library/VariablePolicyLi
b/VariablePolicyUnitTest/VariablePolicyUnitTest.c

ERROR - *Line number: 2253

ERROR - *The function name [main] does not follow the rules

 

Currently, the host-based unit tests are using a standard C entry point:

int

main ()

 

That’s going to break both of these.

 

Another thing to override/figure out for host-based tests 

 

- Bret 

 





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