Date   

Re: Progress on getting Uncrustify working for EDK2?

Michael D Kinney
 

Submodule within which repo? What would be the proposed location?

Would a fork of uncrustify maintained as a repo under TianoCore work as well?

There are CI checks (including extensive unit tests) and release generation that are built into uncrustify repo and I do not know if those will be functional if it is maintained as a submodule.

Thanks,

Mike

-----Original Message-----
From: Leif Lindholm <leif@...>
Sent: Tuesday, November 9, 2021 4:37 AM
To: Gerd Hoffmann <kraxel@...>
Cc: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>; Andrew Fish <afish@...>; Marvin Häuser
<mhaeuser@...>; Michael Kubacki <Michael.Kubacki@...>; mikuback@...; rebecca@...;
Bret Barkelew <Bret.Barkelew@...>
Subject: Re: [edk2-devel] Progress on getting Uncrustify working for EDK2?

On Tue, Nov 09, 2021 at 09:40:02 +0100, Gerd Hoffmann wrote:
Hi,

3. Require use of uncrustify tool before submitting patch review emails or PRs.
* The required version would be a formally released version from the fork maintained by Michael Kubacki until
the changes can be upstreamed.
* https://dev.azure.com/projectmu/Uncrustify
Can we please *first* get the changes merged to upstream uncrustify?

That'll make the whole process much less painful because the usual
software repositories (linux distro packages, macos homebrew, ...)
can be used to install uncrustify then, and it's also less confusing if
developers don't have to juggle with different uncrustify variants
(upstream vs. edk2).
Whilst I agree in principle...

This means postponing automated coding style changes until 2023
(Debian stable), 2025 (Ubuntu LTS), ??? (RHEL10), or even later
... and I'd rather not.

I like Marvin's suggestion of a submodule. Which we could drop once
no longer needed.

/
Leif


[PATCH EDK2 v1 0/1] CryptoPkg/BaseCryptLib: Support PEM certification

wenyi,xie
 

Main Changes :
1.add support for PEM-encoded certificate.


Wenyi Xie (1):
CryptoPkg/BaseCryptLib: Support PEM certification

CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c | 33 ++++++++++++++++++--
1 file changed, 31 insertions(+), 2 deletions(-)

--
2.20.1.windows.1


[PATCH EDK2 v1 1/1] CryptoPkg/BaseCryptLib: Support PEM certification

wenyi,xie
 

As PEM-encoded certificate is also necessary, add
support for PEM-encoded certificate in
X509ConstructCertificate.

Cc: Jiewen Yao <jiewen.yao@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Xiaoyu Lu <xiaoyux.lu@...>
Cc: Guomin Jiang <guomin.jiang@...>
Signed-off-by: Jiaxia Xu <xujiaxia@...>
Signed-off-by: Wenyi Xie <xiewenyi2@...>
---
CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c | 33 ++++++++++++++++++--
1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
index b1393a89c5ab..db122cd574fa 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
@@ -9,6 +9,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include "InternalCryptLib.h"
#include <openssl/x509.h>
#include <openssl/rsa.h>
+#include <openssl/pem.h>

/**
Construct a X509 object from DER-encoded certificate data.
@@ -33,7 +34,12 @@ X509ConstructCertificate (
)
{
X509 *X509Cert;
+ BIO *BioCert;
CONST UINT8 *Temp;
+ BOOLEAN CertFlag;
+
+ BioCert = NULL;
+ CertFlag = TRUE;

//
// Check input parameters.
@@ -48,12 +54,35 @@ X509ConstructCertificate (
Temp = Cert;
X509Cert = d2i_X509 (NULL, &Temp, (long) CertSize);
if (X509Cert == NULL) {
- return FALSE;
+ BioCert = BIO_new (BIO_s_mem ());
+ if (BioCert == NULL) {
+ CertFlag = FALSE;
+ goto ON_EXIT;
+ }
+
+ if (BIO_write (BioCert, Temp, (UINT32) CertSize) <= 0) {
+ CertFlag = FALSE;
+ goto ON_EXIT;
+ }
+
+ //
+ // Read PEM-encoded X509 Certificate and Construct X509 object.
+ //
+ X509Cert = PEM_read_bio_X509 (BioCert, NULL, NULL, NULL);
+ if (X509Cert == NULL) {
+ CertFlag = FALSE;
+ goto ON_EXIT;
+ }
}

*SingleX509Cert = (UINT8 *) X509Cert;

- return TRUE;
+ON_EXIT:
+ if (BioCert != NULL) {
+ BIO_free (BioCert);
+ }
+
+ return CertFlag;
}

/**
--
2.20.1.windows.1


Re: Progress on getting Uncrustify working for EDK2?

Leif Lindholm
 

On Tue, Nov 09, 2021 at 09:40:02 +0100, Gerd Hoffmann wrote:
Hi,

3. Require use of uncrustify tool before submitting patch review emails or PRs.
* The required version would be a formally released version from the fork maintained by Michael Kubacki until the changes can be upstreamed.
* https://dev.azure.com/projectmu/Uncrustify
Can we please *first* get the changes merged to upstream uncrustify?

That'll make the whole process much less painful because the usual
software repositories (linux distro packages, macos homebrew, ...)
can be used to install uncrustify then, and it's also less confusing if
developers don't have to juggle with different uncrustify variants
(upstream vs. edk2).
Whilst I agree in principle...

This means postponing automated coding style changes until 2023
(Debian stable), 2025 (Ubuntu LTS), ??? (RHEL10), or even later
... and I'd rather not.

I like Marvin's suggestion of a submodule. Which we could drop once
no longer needed.

/
Leif


Re: [PATCH v2 1/2] ArmPkg: Add SMC helper functions

Ard Biesheuvel
 

On Tue, 9 Nov 2021 at 12:57, Leif Lindholm <leif@...> wrote:

On Mon, Nov 08, 2021 at 18:56:09 -0700, Rebecca Cran wrote:
Could I have some reviews on this please?
I'm all for it. It's clunky, but less clunky than the situation
without, and it improves readability at call sites.

Ard had some reservations for v1 not actually adding any users.
Ard - do you like it any better now Rebecca's added some?
Yeah, this is fine. TBH, I am not going to have time to look into this
in detail again, so if you're both happy, then I am too.

Acked-by: Ard Biesheuvel <ardb@...>



On 11/1/21 4:11 PM, Rebecca Cran wrote:
Add functions ArmCallSmc0/1/2/3 to do SMC calls with 0, 1, 2 or 3
arguments.
The functions return up to 3 values.

Signed-off-by: Rebecca Cran <rebecca@...>
---
ArmPkg/Include/Library/ArmSmcLib.h | 73 ++++++++++++
ArmPkg/Library/ArmSmcLib/ArmSmc.c | 122 ++++++++++++++++++++
ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf | 3 +
ArmPkg/Library/ArmSmcLibNull/ArmSmcLibNull.c | 85 ++++++++++++++
4 files changed, 283 insertions(+)

diff --git a/ArmPkg/Include/Library/ArmSmcLib.h b/ArmPkg/Include/Library/ArmSmcLib.h
index ced60b3c1147..343ae7f40ad2 100644
--- a/ArmPkg/Include/Library/ArmSmcLib.h
+++ b/ArmPkg/Include/Library/ArmSmcLib.h
@@ -1,5 +1,6 @@
/** @file
*
+* Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
* Copyright (c) 2012-2014, ARM Limited. All rights reserved.
*
* SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -37,4 +38,76 @@ ArmCallSmc (
IN OUT ARM_SMC_ARGS *Args
);
+/** Trigger an SMC call with 3 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Argument/result.
+ @param Arg3 Argument/result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc3 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1,
+ IN OUT UINTN *Arg2,
+ IN OUT UINTN *Arg3
+ );
+
+/** Trigger an SMC call with 2 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Argument/result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc2 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1,
+ IN OUT UINTN *Arg2,
+ OUT UINTN *Arg3
+ );
+
+/** Trigger an SMC call with 1 argument.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc1 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1,
+ OUT UINTN *Arg2,
+ OUT UINTN *Arg3
+ );
+
+/** Trigger an SMC call with 0 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Result.
+ @param Arg2 Result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc0 (
+ IN UINTN Function,
+ OUT UINTN *Arg1,
+ OUT UINTN *Arg2,
+ OUT UINTN *Arg3
+ );
+
#endif // ARM_SMC_LIB_H_
diff --git a/ArmPkg/Library/ArmSmcLib/ArmSmc.c b/ArmPkg/Library/ArmSmcLib/ArmSmc.c
new file mode 100644
index 000000000000..d596003a857e
--- /dev/null
+++ b/ArmPkg/Library/ArmSmcLib/ArmSmc.c
@@ -0,0 +1,122 @@
+/** @file
+ SMC helper functions.
+
+ Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/ArmSmcLib.h>
+
+/** Triggers an SMC call with 3 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Argument/result.
+ @param Arg3 Argument/result.
+
+ @return The SMC error code.
+**/
+UINTN
+ArmCallSmc3 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1,
+ IN OUT UINTN *Arg2,
+ IN OUT UINTN *Arg3
+ )
+{
+ ARM_SMC_ARGS Args;
+ UINTN ErrorCode;
+
+ Args.Arg0 = Function;
+
+ if (Arg1 != NULL) {
+ Args.Arg1 = *Arg1;
+ }
+ if (Arg2 != NULL) {
+ Args.Arg2 = *Arg2;
+ }
+ if (Arg3 != NULL) {
+ Args.Arg3 = *Arg3;
+ }
+
+ ArmCallSmc (&Args);
+
+ ErrorCode = Args.Arg0;
+
+ if (Arg1 != NULL) {
+ *Arg1 = Args.Arg1;
+ }
+ if (Arg2 != NULL) {
+ *Arg2 = Args.Arg2;
+ }
+ if (Arg3 != NULL) {
+ *Arg3 = Args.Arg3;
+ }
+
+ return ErrorCode;
+}
+
+/** Trigger an SMC call with 2 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Argument/result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc2 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1,
+ IN OUT UINTN *Arg2,
+ OUT UINTN *Arg3
+ )
+{
+ return ArmCallSmc3 (Function, Arg1, Arg2, Arg3);
+}
+
+/** Trigger an SMC call with 1 argument.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc1 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1,
+ OUT UINTN *Arg2,
+ OUT UINTN *Arg3
+ )
+{
+ return ArmCallSmc3 (Function, Arg1, Arg2, Arg3);
+}
+
+/** Trigger an SMC call with 0 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Result.
+ @param Arg2 Result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc0 (
+ IN UINTN Function,
+ OUT UINTN *Arg1,
+ OUT UINTN *Arg2,
+ OUT UINTN *Arg3
+ )
+{
+ return ArmCallSmc3 (Function, Arg1, Arg2, Arg3);
+}
diff --git a/ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf b/ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
index 4f4b09f4528a..a89f9203fb7e 100644
--- a/ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
+++ b/ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
@@ -20,6 +20,9 @@
[Sources.AARCH64]
AArch64/ArmSmc.S
+[Sources]
+ ArmSmc.c
+
[Packages]
MdePkg/MdePkg.dec
ArmPkg/ArmPkg.dec
diff --git a/ArmPkg/Library/ArmSmcLibNull/ArmSmcLibNull.c b/ArmPkg/Library/ArmSmcLibNull/ArmSmcLibNull.c
index 2d79aadaf1fa..ca1b8830a119 100644
--- a/ArmPkg/Library/ArmSmcLibNull/ArmSmcLibNull.c
+++ b/ArmPkg/Library/ArmSmcLibNull/ArmSmcLibNull.c
@@ -1,4 +1,5 @@
//
+// Copyright (c) 2021, NUVIA Inc. All rights reserved.
// Copyright (c) 2016, Linaro Limited. All rights reserved.
//
// SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -7,6 +8,7 @@
#include <Base.h>
#include <Library/ArmSmcLib.h>
+#include <IndustryStandard/ArmStdSmc.h>
VOID
ArmCallSmc (
@@ -14,3 +16,86 @@ ArmCallSmc (
)
{
}
+
+/** Triggers an SMC call with 3 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Argument/result.
+ @param Arg3 Argument/result.
+
+ @return The SMC error code.
+**/
+UINTN
+ArmCallSmc3 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1,
+ IN OUT UINTN *Arg2,
+ IN OUT UINTN *Arg3
+ )
+{
+ return SMC_ARCH_CALL_NOT_SUPPORTED;
+}
+
+/** Trigger an SMC call with 2 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Argument/result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc2 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1,
+ IN OUT UINTN *Arg2,
+ OUT UINTN *Arg3
+ )
+{
+ return SMC_ARCH_CALL_NOT_SUPPORTED;
+}
+
+/** Trigger an SMC call with 1 argument.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc1 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1,
+ OUT UINTN *Arg2,
+ OUT UINTN *Arg3
+ )
+{
+ return SMC_ARCH_CALL_NOT_SUPPORTED;
+}
+
+/** Trigger an SMC call with 0 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Result.
+ @param Arg2 Result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc0 (
+ IN UINTN Function,
+ OUT UINTN *Arg1,
+ OUT UINTN *Arg2,
+ OUT UINTN *Arg3
+ )
+{
+ return SMC_ARCH_CALL_NOT_SUPPORTED;
+}


Re: [PATCH] UefiCpuPkg/UefiCpuLib: Add GetCpuFamilyModel and GetCpuSteppingId

Dong, Eric
 

Reviewed-by: Eric Dong <eric.dong@...>

-----Original Message-----
From: Ni, Ray <ray.ni@...>
Sent: Tuesday, October 19, 2021 2:42 PM
To: devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@...>; Kumar, Rahul1 <rahul1.kumar@...>
Subject: [PATCH] UefiCpuPkg/UefiCpuLib: Add GetCpuFamilyModel and GetCpuSteppingId

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

Lots of code relies on CPU Family/Model/Stepping for different logics.

The change adds two APIs for such needs.

Signed-off-by: Ray Ni <ray.ni@...>
Cc: Eric Dong <eric.dong@...>
Cc: Rahul Kumar <rahul1.kumar@...>
---
UefiCpuPkg/Include/Library/UefiCpuLib.h | 23 +++++++++-
.../Library/BaseUefiCpuLib/BaseUefiCpuLib.c | 43 +++++++++++++++++++
2 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Include/Library/UefiCpuLib.h b/UefiCpuPkg/Include/Library/UefiCpuLib.h
index 5326e72463..092c1d2116 100644
--- a/UefiCpuPkg/Include/Library/UefiCpuLib.h
+++ b/UefiCpuPkg/Include/Library/UefiCpuLib.h
@@ -4,7 +4,7 @@
This library class defines some routines that are generic for IA32 family CPU

to be UEFI specification compliant.



- Copyright (c) 2009, Intel Corporation. All rights reserved.<BR>

+ Copyright (c) 2009 - 2021, Intel Corporation. All rights reserved.<BR>

Copyright (c) 2020, AMD Inc. All rights reserved.<BR>

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



@@ -43,4 +43,25 @@ StandardSignatureIsAuthenticAMD (
VOID

);



+/**

+ Return the 32bit CPU family and model value.

+

+ @return CPUID[01h].EAX with Processor Type and Stepping ID cleared.

+**/

+UINT32

+EFIAPI

+GetCpuFamilyModel (

+ VOID

+ );

+

+/**

+ Return the CPU stepping ID.

+ @return CPU stepping ID value in CPUID[01h].EAX.

+**/

+UINT8

+EFIAPI

+GetCpuSteppingId (

+ VOID

+ );

+

#endif

diff --git a/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c b/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c
index c2cc3ff9a7..50891618c4 100644
--- a/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c
+++ b/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c
@@ -4,6 +4,7 @@
The library routines are UEFI specification compliant.



Copyright (c) 2020, AMD Inc. All rights reserved.<BR>

+ Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>

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



**/

@@ -36,3 +37,45 @@ StandardSignatureIsAuthenticAMD (
RegEcx == CPUID_SIGNATURE_AUTHENTIC_AMD_ECX &&

RegEdx == CPUID_SIGNATURE_AUTHENTIC_AMD_EDX);

}

+

+/**

+ Return the 32bit CPU family and model value.

+

+ @return CPUID[01h].EAX with Processor Type and Stepping ID cleared.

+**/

+UINT32

+EFIAPI

+GetCpuFamilyModel (

+ VOID

+ )

+{

+ CPUID_VERSION_INFO_EAX Eax;

+

+ AsmCpuid (CPUID_VERSION_INFO, &Eax.Uint32, NULL, NULL, NULL);

+

+ //

+ // Mask other fields than Family and Model.

+ //

+ Eax.Bits.SteppingId = 0;

+ Eax.Bits.ProcessorType = 0;

+ Eax.Bits.Reserved1 = 0;

+ Eax.Bits.Reserved2 = 0;

+ return Eax.Uint32;

+}

+

+/**

+ Return the CPU stepping ID.

+ @return CPU stepping ID value in CPUID[01h].EAX.

+**/

+UINT8

+EFIAPI

+GetCpuSteppingId (

+ VOID

+ )

+{

+ CPUID_VERSION_INFO_EAX Eax;

+

+ AsmCpuid (CPUID_VERSION_INFO, &Eax.Uint32, NULL, NULL, NULL);

+

+ return (UINT8) Eax.Bits.SteppingId;

+}

--
2.32.0.windows.1


Re: [PATCH v2 1/2] ArmPkg: Add SMC helper functions

Leif Lindholm
 

On Mon, Nov 08, 2021 at 18:56:09 -0700, Rebecca Cran wrote:
Could I have some reviews on this please?
I'm all for it. It's clunky, but less clunky than the situation
without, and it improves readability at call sites.

Ard had some reservations for v1 not actually adding any users.
Ard - do you like it any better now Rebecca's added some?

/
Leif

Rebecca Cran


On 11/1/21 4:11 PM, Rebecca Cran wrote:
Add functions ArmCallSmc0/1/2/3 to do SMC calls with 0, 1, 2 or 3
arguments.
The functions return up to 3 values.

Signed-off-by: Rebecca Cran <rebecca@...>
---
ArmPkg/Include/Library/ArmSmcLib.h | 73 ++++++++++++
ArmPkg/Library/ArmSmcLib/ArmSmc.c | 122 ++++++++++++++++++++
ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf | 3 +
ArmPkg/Library/ArmSmcLibNull/ArmSmcLibNull.c | 85 ++++++++++++++
4 files changed, 283 insertions(+)

diff --git a/ArmPkg/Include/Library/ArmSmcLib.h b/ArmPkg/Include/Library/ArmSmcLib.h
index ced60b3c1147..343ae7f40ad2 100644
--- a/ArmPkg/Include/Library/ArmSmcLib.h
+++ b/ArmPkg/Include/Library/ArmSmcLib.h
@@ -1,5 +1,6 @@
/** @file
*
+* Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
* Copyright (c) 2012-2014, ARM Limited. All rights reserved.
*
* SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -37,4 +38,76 @@ ArmCallSmc (
IN OUT ARM_SMC_ARGS *Args
);
+/** Trigger an SMC call with 3 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Argument/result.
+ @param Arg3 Argument/result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc3 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1,
+ IN OUT UINTN *Arg2,
+ IN OUT UINTN *Arg3
+ );
+
+/** Trigger an SMC call with 2 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Argument/result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc2 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1,
+ IN OUT UINTN *Arg2,
+ OUT UINTN *Arg3
+ );
+
+/** Trigger an SMC call with 1 argument.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc1 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1,
+ OUT UINTN *Arg2,
+ OUT UINTN *Arg3
+ );
+
+/** Trigger an SMC call with 0 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Result.
+ @param Arg2 Result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc0 (
+ IN UINTN Function,
+ OUT UINTN *Arg1,
+ OUT UINTN *Arg2,
+ OUT UINTN *Arg3
+ );
+
#endif // ARM_SMC_LIB_H_
diff --git a/ArmPkg/Library/ArmSmcLib/ArmSmc.c b/ArmPkg/Library/ArmSmcLib/ArmSmc.c
new file mode 100644
index 000000000000..d596003a857e
--- /dev/null
+++ b/ArmPkg/Library/ArmSmcLib/ArmSmc.c
@@ -0,0 +1,122 @@
+/** @file
+ SMC helper functions.
+
+ Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/ArmSmcLib.h>
+
+/** Triggers an SMC call with 3 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Argument/result.
+ @param Arg3 Argument/result.
+
+ @return The SMC error code.
+**/
+UINTN
+ArmCallSmc3 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1,
+ IN OUT UINTN *Arg2,
+ IN OUT UINTN *Arg3
+ )
+{
+ ARM_SMC_ARGS Args;
+ UINTN ErrorCode;
+
+ Args.Arg0 = Function;
+
+ if (Arg1 != NULL) {
+ Args.Arg1 = *Arg1;
+ }
+ if (Arg2 != NULL) {
+ Args.Arg2 = *Arg2;
+ }
+ if (Arg3 != NULL) {
+ Args.Arg3 = *Arg3;
+ }
+
+ ArmCallSmc (&Args);
+
+ ErrorCode = Args.Arg0;
+
+ if (Arg1 != NULL) {
+ *Arg1 = Args.Arg1;
+ }
+ if (Arg2 != NULL) {
+ *Arg2 = Args.Arg2;
+ }
+ if (Arg3 != NULL) {
+ *Arg3 = Args.Arg3;
+ }
+
+ return ErrorCode;
+}
+
+/** Trigger an SMC call with 2 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Argument/result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc2 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1,
+ IN OUT UINTN *Arg2,
+ OUT UINTN *Arg3
+ )
+{
+ return ArmCallSmc3 (Function, Arg1, Arg2, Arg3);
+}
+
+/** Trigger an SMC call with 1 argument.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc1 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1,
+ OUT UINTN *Arg2,
+ OUT UINTN *Arg3
+ )
+{
+ return ArmCallSmc3 (Function, Arg1, Arg2, Arg3);
+}
+
+/** Trigger an SMC call with 0 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Result.
+ @param Arg2 Result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc0 (
+ IN UINTN Function,
+ OUT UINTN *Arg1,
+ OUT UINTN *Arg2,
+ OUT UINTN *Arg3
+ )
+{
+ return ArmCallSmc3 (Function, Arg1, Arg2, Arg3);
+}
diff --git a/ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf b/ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
index 4f4b09f4528a..a89f9203fb7e 100644
--- a/ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
+++ b/ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
@@ -20,6 +20,9 @@
[Sources.AARCH64]
AArch64/ArmSmc.S
+[Sources]
+ ArmSmc.c
+
[Packages]
MdePkg/MdePkg.dec
ArmPkg/ArmPkg.dec
diff --git a/ArmPkg/Library/ArmSmcLibNull/ArmSmcLibNull.c b/ArmPkg/Library/ArmSmcLibNull/ArmSmcLibNull.c
index 2d79aadaf1fa..ca1b8830a119 100644
--- a/ArmPkg/Library/ArmSmcLibNull/ArmSmcLibNull.c
+++ b/ArmPkg/Library/ArmSmcLibNull/ArmSmcLibNull.c
@@ -1,4 +1,5 @@
//
+// Copyright (c) 2021, NUVIA Inc. All rights reserved.
// Copyright (c) 2016, Linaro Limited. All rights reserved.
//
// SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -7,6 +8,7 @@
#include <Base.h>
#include <Library/ArmSmcLib.h>
+#include <IndustryStandard/ArmStdSmc.h>
VOID
ArmCallSmc (
@@ -14,3 +16,86 @@ ArmCallSmc (
)
{
}
+
+/** Triggers an SMC call with 3 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Argument/result.
+ @param Arg3 Argument/result.
+
+ @return The SMC error code.
+**/
+UINTN
+ArmCallSmc3 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1,
+ IN OUT UINTN *Arg2,
+ IN OUT UINTN *Arg3
+ )
+{
+ return SMC_ARCH_CALL_NOT_SUPPORTED;
+}
+
+/** Trigger an SMC call with 2 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Argument/result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc2 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1,
+ IN OUT UINTN *Arg2,
+ OUT UINTN *Arg3
+ )
+{
+ return SMC_ARCH_CALL_NOT_SUPPORTED;
+}
+
+/** Trigger an SMC call with 1 argument.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc1 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1,
+ OUT UINTN *Arg2,
+ OUT UINTN *Arg3
+ )
+{
+ return SMC_ARCH_CALL_NOT_SUPPORTED;
+}
+
+/** Trigger an SMC call with 0 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Result.
+ @param Arg2 Result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc0 (
+ IN UINTN Function,
+ OUT UINTN *Arg1,
+ OUT UINTN *Arg2,
+ OUT UINTN *Arg3
+ )
+{
+ return SMC_ARCH_CALL_NOT_SUPPORTED;
+}


Re: Progress on getting Uncrustify working for EDK2?

Michael Brown
 

On 09/11/2021 08:40, Gerd Hoffmann wrote:
3. Require use of uncrustify tool before submitting patch review emails or PRs.
* The required version would be a formally released version from the fork maintained by Michael Kubacki until the changes can be upstreamed.
* https://dev.azure.com/projectmu/Uncrustify
Can we please *first* get the changes merged to upstream uncrustify?
That'll make the whole process much less painful because the usual
software repositories (linux distro packages, macos homebrew, ...)
can be used to install uncrustify then, and it's also less confusing if
developers don't have to juggle with different uncrustify variants
(upstream vs. edk2).
I very strongly agree with this. It's always a bad sign when a project requires installation of a custom version of a widely used tool.

Michael


Re: [PATCH 1/2] Reconfigure OpensslLib to add elliptic curve chipher algorithms

Gerd Hoffmann
 

2. Remove unnecessary algo in openssl config
* Do you really want to enable all those algorithms? Such as SM2? Maybe revisit them again to see if they are really needed. I could see it might break other platform potentially.
Enabling only those algorithms which are actually used by tianocore
certainly makes sense ...

3. Provide 2 profiles – with ECC and without ECC.
... and if it gets down the size enough would be better than yet another
compile time option.

take care,
Gerd


Re: [PATCH v2 5/5] OvmfPkg/Microvm: add README

Philippe Mathieu-Daudé <philmd@...>
 

On 11/3/21 16:06, Gerd Hoffmann wrote:
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3599
Signed-off-by: Gerd Hoffmann <kraxel@...>
Acked-by: Jiewen Yao <Jiewen.yao@...>
---
OvmfPkg/Microvm/README | 50 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
create mode 100644 OvmfPkg/Microvm/README

diff --git a/OvmfPkg/Microvm/README b/OvmfPkg/Microvm/README
new file mode 100644
index 000000000000..82e2bfe03d37
--- /dev/null
+++ b/OvmfPkg/Microvm/README
@@ -0,0 +1,50 @@
+
+This is an *experimental* port of OVMF for the qemu microvm
+machine type.
'QEMU' uppercase?

+microvm background info
+-----------------------
+
+microvm is designed for modern, virtio-based workloads. Most legacy
+lpc/isa devices like pit and pic can be turned off. virtio-mmio
+(i.e. '-device virtio-{blk,net,scsi,...}-device') is used for
+storage/network/etc.
+
+Optional pcie support is available and any pcie device supported by
+qemu can be plugged in (including virtio-pci if you prefer that over
+virtio-mmio).
Ditto.

+https://qemu.readthedocs.io/en/latest/system/i386/microvm.html
+https://www.kraxel.org/blog/2020/10/qemu-microvm-acpi/
+
+design issues
+-------------
+
+Not fully clear yet how to do hardware detection best. Right now
+using device tree to find virtio-mmio devices and pcie host bridge,
+can reuse existing ArmVirtPkg code that way. Needs patched qemu.
Ditto.


Re: [PATCH 1/2] Reconfigure OpensslLib to add elliptic curve chipher algorithms

Gerd Hoffmann
 

Hi,

* OvmfPkg_Win_VS2019 - Passed
* OvmfPkg_Ubuntu_GCC5 – Failed
* INFO - GenFv: ERROR 3000: Invalid
* INFO -   the required fv image size 0xcb2ac0 exceeds the set fv image size 0xc00000
Wow. That is a quite significant increase.
Is this the OVMF_IA32X64_FULL_NOOPT build?

That one is disabled on windows already, probably because turning off
compiler optimizations increases the build size too much. We could do
the same for ubuntu as short-term solution. Long-term we probably need
options to build 8M and 16M OVMF binaries.

While being at it: have you by chance also looked at switching tianocore
over to openssl 3.0?

take care,
Gerd


Re: [PATCH v2 3/5] OvmfPkg/Microvm/fdt: add empty fdt

Philippe Mathieu-Daudé <philmd@...>
 

On 11/3/21 16:06, Gerd Hoffmann wrote:
FdtClient is unhappy without a device tree, so add an empty fdt
which we can use in case etc/fdt is not present in fw_cfg.

On ARM machines a device tree is mandatory for hardware detection,
thats why FdtClient fails hard.

On microvm the device tree is only used to detect virtio-mmio devices
(this patch series) and the pcie host (future series). So edk2 can
continue with limited functionality in case no device tree is present:
no storage, no network, but serial console and direct kernel boot
works.

qemu release 6.2 & newer will provide a device tree for microvm.

https://bugzilla.tianocore.org/show_bug.cgi?id=3689
Signed-off-by: Gerd Hoffmann <kraxel@...>
---
OvmfPkg/PlatformPei/Platform.c | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)
Reviewed-by: Philippe Mathieu-Daude <philmd@...>


Re: [PATCH v3 5/5] OvmfPkg: rework TPM configuration

Philippe Mathieu-Daudé <philmd@...>
 

On 10/28/21 13:09, Gerd Hoffmann wrote:
Rename TPM_ENABLE to TPM2_ENABLE so naming is in line with the
ArmVirtPkg config option name.

Add separate TPM1_ENABLE option for TPM 1.2 support.

Signed-off-by: Gerd Hoffmann <kraxel@...>
Tested-by: Stefan Berger <stefanb@...>
---
OvmfPkg/OvmfTpmComponentsDxe.dsc.inc | 4 +++-
OvmfPkg/OvmfTpmComponentsPei.dsc.inc | 6 +++++-
OvmfPkg/OvmfTpmDefines.dsc.inc | 5 ++++-
OvmfPkg/OvmfTpmLibs.dsc.inc | 4 +++-
OvmfPkg/OvmfTpmLibsDxe.dsc.inc | 4 +++-
OvmfPkg/OvmfTpmLibsPeim.dsc.inc | 4 +++-
OvmfPkg/OvmfTpmPcds.dsc.inc | 2 +-
OvmfPkg/OvmfTpmPcdsHii.dsc.inc | 2 +-
OvmfPkg/OvmfTpmSecurityStub.dsc.inc | 4 +++-
OvmfPkg/OvmfTpmDxe.fdf.inc | 4 +++-
OvmfPkg/OvmfTpmPei.fdf.inc | 6 +++++-
OvmfPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml | 6 +++---
OvmfPkg/PlatformCI/.azurepipelines/Windows-VS2019.yml | 6 +++---
OvmfPkg/PlatformCI/ReadMe.md | 2 +-
14 files changed, 41 insertions(+), 18 deletions(-)
Reviewed-by: Philippe Mathieu-Daude <philmd@...>


Re: [PATCH v3 3/5] OvmfPkg: drop TPM_CONFIG_ENABLE

Philippe Mathieu-Daudé <philmd@...>
 

On 10/28/21 13:09, Gerd Hoffmann wrote:
Drop TPM_CONFIG_ENABLE config option. Including TPM support in the
build without also including the TPM configuration menu is not useful.

Suggested-by: Stefan Berger <stefanb@...>
Signed-off-by: Gerd Hoffmann <kraxel@...>
Tested-by: Stefan Berger <stefanb@...>
---
OvmfPkg/OvmfTpmComponentsDxe.dsc.inc | 2 --
OvmfPkg/OvmfTpmDefines.dsc.inc | 1 -
OvmfPkg/OvmfTpmPcdsHii.dsc.inc | 2 +-
OvmfPkg/OvmfTpmDxe.fdf.inc | 2 --
OvmfPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml | 6 +++---
OvmfPkg/PlatformCI/.azurepipelines/Windows-VS2019.yml | 6 +++---
OvmfPkg/PlatformCI/ReadMe.md | 2 +-
7 files changed, 8 insertions(+), 13 deletions(-)
Reviewed-by: Philippe Mathieu-Daude <philmd@...>


Re: [PATCH v3 1/5] OvmfPkg: remove unused TPM options from MicrovmX64.dsc

Philippe Mathieu-Daudé <philmd@...>
 

On 10/28/21 13:09, Gerd Hoffmann wrote:
Signed-off-by: Gerd Hoffmann <kraxel@...>
---
OvmfPkg/Microvm/MicrovmX64.dsc | 2 --
1 file changed, 2 deletions(-)
Reviewed-by: Philippe Mathieu-Daude <philmd@...>


Re: [PATCH v3 4/5] OvmfPkg: create Tcg12ConfigPei.inf

Philippe Mathieu-Daudé <philmd@...>
 

On 10/28/21 13:09, Gerd Hoffmann wrote:
Split Tcg2ConfigPei.inf into two variants: Tcg12ConfigPei.inf with
TPM 1.2 support included and Tcg2ConfigPei.inf supporting TPM 2.0 only.
This allows x86 builds to choose whenever TPM 1.2 support should be
included or not by picking the one or the other inf file.

Switch x86 builds to Tcg12ConfigPei.inf, so they continue to
have TPM 1.2 support.

No functional change.

Signed-off-by: Gerd Hoffmann <kraxel@...>
Reviewed-by: Stefan Berger <stefanb@...>
Tested-by: Stefan Berger <stefanb@...>
---
OvmfPkg/OvmfTpmComponentsPei.dsc.inc | 2 +-
.../{Tcg2ConfigPei.inf => Tcg12ConfigPei.inf} | 11 ++---------
OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf | 11 +----------
OvmfPkg/OvmfTpmPei.fdf.inc | 2 +-
4 files changed, 5 insertions(+), 21 deletions(-)
copy OvmfPkg/Tcg/Tcg2Config/{Tcg2ConfigPei.inf => Tcg12ConfigPei.inf} (82%)
Reviewed-by: Philippe Mathieu-Daude <philmd@...>


Re: Progress on getting Uncrustify working for EDK2?

Gerd Hoffmann
 

Hi,

3. Require use of uncrustify tool before submitting patch review emails or PRs.
* The required version would be a formally released version from the fork maintained by Michael Kubacki until the changes can be upstreamed.
* https://dev.azure.com/projectmu/Uncrustify
Can we please *first* get the changes merged to upstream uncrustify?

That'll make the whole process much less painful because the usual
software repositories (linux distro packages, macos homebrew, ...)
can be used to install uncrustify then, and it's also less confusing if
developers don't have to juggle with different uncrustify variants
(upstream vs. edk2).

thanks,
Gerd


Re: [PATCH 1/2] Reconfigure OpensslLib to add elliptic curve chipher algorithms

Yao, Jiewen
 

Some options for your consideration.

  1. Enlarge OVMF size
    1. I have seen discussion to 8M to 16M, but it seems not concluded.
  2. Remove unnecessary algo in openssl config
    1. Do you really want to enable all those algorithms? Such as SM2? Maybe revisit them again to see if they are really needed. I could see it might break other platform potentially.
    2. Do you have any evaluation on binary size difference before or after your patch ? Please provide the data to help other people make decision.
  3. Provide 2 profiles – with ECC and without ECC.
    1. As such, we can let platform decide which one it wants to take, if there is significant size difference.
    2. This would be the best way to keep the compatibility.

Thank you

Yao Jiewen

 

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Vineel Kovvuri via groups.io
Sent: Tuesday, November 9, 2021 6:30 AM
To: Vineel Kovvuri <vineel.kovvuri@...>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH 1/2] Reconfigure OpensslLib to add elliptic curve chipher algorithms

 

Hi Folks,

 

We are able to resolve the __ModuleEntryPoint error and was able to run below build configurations locally.

  • Windows_VS2019 - Passed
    • EmulatorPkg_Win_VS2019 - Passed
    • OvmfPkg_Win_VS2019 - Passed
  • Ubuntu_GCC5 - Passed
    • ArmVirtPkg_Ubuntu_GCC5 - Passed
    • EmulatorPkg_Ubuntu_GCC5 - Passed
    • OvmfPkg_Ubuntu_GCC5 – Failed
      • INFO - GenFv: ERROR 3000: Invalid
      • INFO -   the required fv image size 0xcb2ac0 exceeds the set fv image size 0xc00000

Is it okay to increase the fv image size if so need some guidance with respected to that as it may effect other projects like QEMU etc. Any inputs here are much appreciated

 

For Reference: https://github.com/vineelkovvuri/edk2/pull/2

 


Re: [PATCH v3 6/7] OvmfPkg/PlatformCI: dummy grub.efi for AmdSev

Dov Murik
 

Thanks for this addition to CI!


On 03/11/2021 11:11, Gerd Hoffmann wrote:
Building grub.efi for AmdSev is difficult because it depends on patches
not yet merged to upstream grub. So shortcut the grub build by simply
creating an empty grub.efi file. That allows to at least build-test the
AmdSev variant.
Note that it will also allow (later) to test with QEMU with -kernel (AKA
direct measured Linux boot), which doesn't reach the grub part. (if the
CI supports such tests.)



Acked-by: Jiewen Yao <Jiewen.yao@...>
Acked-by: Ard Biesheuvel <ardb@...>
Signed-off-by: Gerd Hoffmann <kraxel@...>
Reviewed-by: Dov Murik <dovmurik@...>




---
OvmfPkg/PlatformCI/AmdSevBuild.py | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/OvmfPkg/PlatformCI/AmdSevBuild.py b/OvmfPkg/PlatformCI/AmdSevBuild.py
index 2dd72cfe80d9..816caafb0084 100644
--- a/OvmfPkg/PlatformCI/AmdSevBuild.py
+++ b/OvmfPkg/PlatformCI/AmdSevBuild.py
@@ -6,6 +6,7 @@
##
import os
import sys
+import subprocess

sys.path.append(os.path.dirname(os.path.abspath(__file__)))
from PlatformBuildLib import SettingsManager
@@ -35,3 +36,7 @@ class CommonPlatform():

import PlatformBuildLib
PlatformBuildLib.CommonPlatform = CommonPlatform
+
+# hack alert -- create dummy grub.efi
+subprocess.run(['touch', 'OvmfPkg/AmdSev/Grub/grub.efi'])
+subprocess.run(['ls', '-l', '--sort=time', 'OvmfPkg/AmdSev/Grub'])


Re: Progress on getting Uncrustify working for EDK2?

Marvin Häuser <mhaeuser@...>
 

Hey all,

Thanks for the effort!

1. If virtually everyone will need Uncrustify, why cannot it be built along with BaseTools from a submodule? Especially with the fork that makes sense, after that it depends on the upstream (it does not look too nice to me).

2. Does this cover CRLF->LF?

3. Will there be a list of implicit changes from the current code style?

4. I feel like if the other code style changes are not tackled now, they will never be, because who wants to deal with continuous cosmetic commits. But as long as there is a tool now to enforce the current one, that's not a dealbreaker at all. :)

Best regards,
Marvin

09.11.2021 04:03:06 Kinney, Michael D <michael.d.kinney@...>:

Andrew,
 
I think Michael Kubacki started with a nuget feed because that can be easily used by EDK II CI agents.
 
However, that does not work as easily for all development environments using Windows, Linux, and MacOS.  Creating releases that can be easily installed by developers is critical for success.
 
For MacOS, there is a homebrew recipe.  You should just have to update the URL to the uncrustify fork.
 
https://macappstore.org/uncrustify/
 
Adding the installation information to the EDK II Getting Started page would be a good place to capture the developer install.
 
Mike
 
*From:* Andrew Fish <afish@...>
*Sent:* Monday, November 8, 2021 6:47 PM
*To:* Kinney, Michael D <michael.d.kinney@...>
*Cc:* devel@edk2.groups.io; Marvin Häuser <mhaeuser@...>; Michael Kubacki <Michael.Kubacki@...>; Leif Lindholm <leif@...>; mikuback@...; rebecca@...; Bret Barkelew <Bret.Barkelew@...>
*Subject:* Re: [edk2-devel] Progress on getting Uncrustify working for EDK2?
 
 


On Nov 8, 2021, at 5:13 PM, Kinney, Michael D <michael.d.kinney@...> wrote:
 
HI Andrew,
 
Great feedback.
 
What your preferred way to install a tool like this?  If we collect that data from the community, we can make sure the pipeline generates the right type of installers.
 
 
I could not figure out how to download an installer from the links. 
 
If I go to http://uncrustify.sourceforge.net I see https://sourceforge.net/projects/uncrustify/files/ and a button to download binaries. Not ideal that it is only for Windows but at least the workflow was obvious. Looks like I need to build my own version for macOS, not ideal but I can at least figure that out. 
 
Worst case we can have a Tianocore.org[http://Tianocore.org] page to describe a simple recipe to install the tool. With step by step instructions some one can just type in without thinking too much. 
 
Thanks,
 
Andrew Fish


Thanks,
 
Mike
 
*From:* Andrew Fish <afish@...> 
*Sent:* Monday, November 8, 2021 5:09 PM
*To:* devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>
*Cc:* Marvin Häuser <mhaeuser@...>; Michael Kubacki <Michael.Kubacki@...>; Leif Lindholm <leif@...>; mikuback@...; rebecca@...; Bret Barkelew <Bret.Barkelew@...>
*Subject:* Re: [edk2-devel] Progress on getting Uncrustify working for EDK2?
 
MIke,
 
I could not figure out how to download uncrustify tool from the provided link. 99% of the people are just going to want to install the tool, not be a developer of the fork. We should have some simple instructions on how to download the tool. 
 
The link points to something git web view looking Azure DevOps page and talks about this Nuget thing I know nothing about. I ran out of time and had to give up trying to download the tool. 
 
Thanks,
 
Andrew Fish



On Nov 8, 2021, at 4:23 PM, Michael D Kinney <michael.d.kinney@...> wrote:
 
Hello,
 
Good information in this thread on code style.
 
Some of the topics apply to uncrustify and some are out of scope for what uncrustify can fix on its own.
 
I would like to focus on a date to convert all source code in edk2 repo using the uncrustify tool and to capture the other code style topics into their own thread andbugzillas.
 
I would like to propose a conversion date for uncrustify immediately after the edk2-stable202111 release on 2021-11-26.
 
I have been working with Michael Kubacki on a build comparison tool that verifies that the build generate the same obj/lib/dll/efi/fv/fd files before and after the uncrustify changes.  We would run and publish the results from this tool before committing the changes.
 
We need TianoCore community approval of the following:
 
1. > Approve format of C source generated by the uncrustify.
2. > Approve uncrustify changes right after edk2-stable-202111 release. 
1. > Extend code freeze until these changes are committed.
3. > Require use of uncrustify tool before submitting patch review emails or PRs.
1. > The required version would be a formally released version  from the fork maintained by Michael Kubacki until the changes can be upstreamed.
2. > https://dev.azure.com/projectmu/Uncrustify
4. > Add EDK II CI check to verify that all PRs submitted exactly match uncrustified version.  Reject PRs that do not match exactly.
5. > Implement a git hook available that would automatically run uncristufy before committing changes to a local branch of an edk2 repo.
 
Thanks,
 
Mike
 
*From:* Andrew Fish <afish@...> 
*Sent:* Thursday, October 7, 2021 2:09 PM
*To:* Marvin Häuser <mhaeuser@...>
*Cc:* edk2-devel-groups-io <devel@edk2.groups.io>; Kinney, Michael D <michael.d.kinney@...>; Leif Lindholm <leif@...>; mikuback@...; rebecca@...; Michael Kubacki <Michael.Kubacki@...>; Bret Barkelew <Bret.Barkelew@...>
*Subject:* Re: [edk2-devel] Progress on getting Uncrustify working for EDK2?
 
 




On Oct 7, 2021, at 1:43 PM, Marvin Häuser <mhaeuser@...> wrote:
 
Hey Mike,
Hey Andrew,

I'll just reply to both mails at once :)

On 07/10/2021 19:36, Andrew Fish wrote:










Thanks! I'd keep STATIC actually just for the sake of not doing no-op changes that do not really do anything and for consistency with CONST, but whatever works really.






Yes.






This issue is independent of CONST. I’m not sure a coding style tool is smart enough to catch this generically? You need an understanding of C types to know if the local variable assignment is going to trigger a memcpy().

What I’ve seen in the real world is the firmware compiles with -Os or LTO to fit int he ROM for DEBUG and RELEASE, and the optimizer optimizes away the call to memcpy. Then if you try to build NOOPT (or over ride the compiler flags on an individual driver/lib) you fail to link as only the NOOPT build injects the memcpy.

+1





Thus I think the best way to enforce this rule is to compile a project NOOPT. I’m trying to remember are there flags to built to tell it to compile and skip the FD construction? Maybe we should advocate platforms add a NOOPT build target that just compiles the code, but does not create the FD?

I know there were stability concerns with intrinsics in the past, but memcpy() is in the standard, and the rest remained stable to my knowledge. Maybe it's time to fix the issues at the root? Works for us:
https://github.com/acidanthera/OpenCorePkg/tree/master/Library/OcCompilerIntrinsicsLib


 
Marvin,
 
Good point. This would make the rule moot. So maybe just removing the requirement would be the easiest long term fix. 
 
Other embedded projects I know of do this too, and as you point out the compilers keep these APIs standard for folks the provide their own runtimes.
 
Thanks,
 
Andrew Fish




Best regards,
Marvin





Thanks,

Andrew Fish





 

7541 - 7560 of 90967