Re: [edk2-devel] [PATCH 01/15] .pytool/Plugin: Add a plugin LicenseCheck

2020-07-28 Thread Zhang, Shenglei
Five sorts of license are accepted in edk2(described in readme). They are BSD 
(2-clause), BSD (3-clause), MIT, Python-2.0 and zlib.
Minus non bsd plus patent license, they are BSD (2-clause) and BSD (3-clause).

Thanks,
Shenglei

> -Original Message-
> From: Kinney, Michael D 
> Sent: Wednesday, July 29, 2020 7:29 AM
> To: Zhang, Shenglei ; devel@edk2.groups.io;
> Kinney, Michael D 
> Cc: Sean Brogan ; Bret Barkelew
> ; Gao, Liming 
> Subject: RE: [PATCH 01/15] .pytool/Plugin: Add a plugin LicenseCheck
> 
> Where did the requirement for BSD-3-Clause-Patent come from?
> 
> Thanks,
> 
> Mike
> 
> > -Original Message-
> > From: Zhang, Shenglei 
> > Sent: Monday, July 20, 2020 1:37 AM
> > To: devel@edk2.groups.io
> > Cc: Sean Brogan ; Bret
> > Barkelew ; Kinney, Michael
> > D ; Gao, Liming
> > 
> > Subject: [PATCH 01/15] .pytool/Plugin: Add a plugin
> > LicenseCheck
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2691
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2833
> > Add a plugin to check license conflict for new added
> > files in a patch. It will report out errors when meeting
> > files which are now contributed under BSD-2-Clause-
> > Patent
> > or BSD-3-Clause-Patent.
> >
> > Cc: Sean Brogan 
> > Cc: Bret Barkelew 
> > Cc: Michael D Kinney 
> > Cc: Liming Gao 
> > Signed-off-by: Shenglei Zhang 
> > ---
> >  .pytool/Plugin/LicenseCheck/LicenseCheck.py   | 118
> > ++
> >  .../LicenseCheck/LicenseCheck_plug_in.yaml|  11 ++
> >  .pytool/Plugin/LicenseCheck/Readme.md |  17 +++
> >  3 files changed, 146 insertions(+)
> >  create mode 100644
> > .pytool/Plugin/LicenseCheck/LicenseCheck.py
> >  create mode 100644
> > .pytool/Plugin/LicenseCheck/LicenseCheck_plug_in.yaml
> >  create mode 100644
> > .pytool/Plugin/LicenseCheck/Readme.md
> >
> > diff --git a/.pytool/Plugin/LicenseCheck/LicenseCheck.py
> > b/.pytool/Plugin/LicenseCheck/LicenseCheck.py
> > new file mode 100644
> > index ..98941ddda758
> > --- /dev/null
> > +++ b/.pytool/Plugin/LicenseCheck/LicenseCheck.py
> > @@ -0,0 +1,118 @@
> > +# @file LicenseCheck.py
> > +#
> > +# Copyright (c) 2020, Intel Corporation. All rights
> > reserved.
> > +# SPDX-License-Identifier: BSD-2-Clause-Patent
> > +##
> > +
> > +import os
> > +import logging
> > +import re
> > +from io import StringIO
> > +from typing import List, Tuple
> > +from
> > edk2toolext.environment.plugintypes.ci_build_plugin
> > import ICiBuildPlugin
> > +from edk2toolext.environment.var_dict import VarDict
> > +from edk2toollib.utility_functions import RunCmd
> > +
> > +
> > +class LicenseCheck(ICiBuildPlugin):
> > +
> > +"""
> > +A CiBuildPlugin to check the license for new added
> > files.
> > +
> > +Configuration options:
> > +"LicenseCheck": {
> > +"IgnoreFiles": []
> > +},
> > +"""
> > +
> > +license_format_preflix = 'SPDX-License-Identifier'
> > +
> > +bsd2_patent = 'BSD-2-Clause-Patent'
> > +
> > +bsd3_patent = 'BSD-3-Clause-Patent'
> > +
> > +Readdedfileformat = re.compile(r'\+\+\+ b\/(.*)')
> > +
> > +file_extension_list = [".c", ".h", ".inf", ".dsc",
> > ".dec", ".py", ".bat", ".sh", ".uni", ".yaml",
> > +   ".fdf", ".inc", "yml",
> > ".asm", ".asm16", ".asl", ".vfr", ".s", ".S", ".aslc",
> > +   ".nasm", ".nasmb", ".idf",
> > ".Vfr", ".H"]
> > +
> > +def GetTestName(self, packagename: str,
> > environment: VarDict) -> tuple:
> > +""" Provide the testcase name and classname for
> > use in reporting
> > +testclassname: a descriptive string for the
> > testcase can include whitespace
> > +classname: should be patterned
> > ..
> > +
> > +Args:
> > +  packagename: string containing name of
> > package to build
> > +  environment: The VarDict for the test to
> > run in
> > +Returns:
> > +a tuple containing the testcase name
> > and the classname
> > +(testcasename, classname)
> > +"""
> > +return ("Check for license for " + packagename,
> > packagename + ".LicenseCheck")
> > +
> > +##
> > +# External function of plugin.  This function is
> > used to perform the task of the ci_build_plugin Plugin
> > +#
> > +#   - package is the edk2 path to package.  This
> > means workspace/packagepath relative.
> > +#   - edk2path object configured with workspace and
> > packages path
> > +#   - PkgConfig Object (dict) for the pkg
> > +#   - EnvConfig Object
> > +#   - Plugin Manager Instance
> > +#   - Plugin Helper Obj Instance
> > +#   - Junit Logger
> > +#   - output_stream the StringIO output stream from
> > this plugin via logging
> > +def RunBuildPlugin(self, packagename, Edk2pathObj,
> > pkgconfig, environment, PLM, PLMHelper, tc,
> > output_stream=None):
> > +return_buffer = StringIO()
> > +params = "diff --unified=0 origin/master

[edk2-devel] [PATCH] SimicsOpenBoardPkg: Update usage of functions to be removed

2020-07-28 Thread Zhang, Shenglei
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2777
With some functions to be deprecated, their usage in platforms
should also be updated.

Cc: Agyeman Prince 
Signed-off-by: Shenglei Zhang 
---
 .../Library/BoardBdsHookLib/BoardBdsHookLib.c|  2 +-
 .../Intel/SimicsOpenBoardPkg/SimicsDxe/Platform.c|  4 ++--
 .../Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c   |  6 +++---
 .../Intel/SimicsOpenBoardPkg/SimicsPei/Platform.c| 12 ++--
 4 files changed, 12 insertions(+), 12 deletions(-)

diff --git 
a/Platform/Intel/SimicsOpenBoardPkg/Library/BoardBdsHookLib/BoardBdsHookLib.c 
b/Platform/Intel/SimicsOpenBoardPkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
index 1058dbf3..ba4d2b02 100644
--- 
a/Platform/Intel/SimicsOpenBoardPkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
+++ 
b/Platform/Intel/SimicsOpenBoardPkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
@@ -1206,7 +1206,7 @@ VisitingFileSystemInstance (
   NULL,
   &mEmuVariableEventReg
   );
-  PcdSet64 (PcdEmuVariableEvent, (UINT64)(UINTN) mEmuVariableEvent);
+  PcdSet64S (PcdEmuVariableEvent, (UINT64)(UINTN) mEmuVariableEvent);
 
   return EFI_SUCCESS;
 }
diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsDxe/Platform.c 
b/Platform/Intel/SimicsOpenBoardPkg/SimicsDxe/Platform.c
index b7fd4d1f..c856ff44 100644
--- a/Platform/Intel/SimicsOpenBoardPkg/SimicsDxe/Platform.c
+++ b/Platform/Intel/SimicsOpenBoardPkg/SimicsDxe/Platform.c
@@ -669,9 +669,9 @@ ExecutePlatformConfig (
 //
 // Pass the preferred resolution to GraphicsConsoleDxe via dynamic PCDs.
 //
-PcdSet32 (PcdVideoHorizontalResolution,
+PcdSet32S (PcdVideoHorizontalResolution,
   PlatformConfig.HorizontalResolution);
-PcdSet32 (PcdVideoVerticalResolution,
+PcdSet32S (PcdVideoVerticalResolution,
   PlatformConfig.VerticalResolution);
   }
 
diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c 
b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
index 60aa54be..127afffc 100644
--- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
+++ b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
@@ -155,7 +155,7 @@ GetFirstNonAddress (
 if (mBootMode != BOOT_ON_S3_RESUME) {
   DEBUG ((EFI_D_INFO, "%a: disabling 64-bit PCI host aperture\n",
 __FUNCTION__));
-  PcdSet64 (PcdPciMmio64Size, 0);
+  PcdSet64S (PcdPciMmio64Size, 0);
 }
 
 //
@@ -187,8 +187,8 @@ GetFirstNonAddress (
 // the GCD memory space map through our PciHostBridgeLib instance; here we
 // only need to set the PCDs.
 //
-PcdSet64 (PcdPciMmio64Base, Pci64Base);
-PcdSet64 (PcdPciMmio64Size, Pci64Size);
+PcdSet64S (PcdPciMmio64Base, Pci64Base);
+PcdSet64S (PcdPciMmio64Size, Pci64Size);
 DEBUG ((EFI_D_INFO, "%a: Pci64Base=0x%Lx Pci64Size=0x%Lx\n",
   __FUNCTION__, Pci64Base, Pci64Size));
   }
diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Platform.c 
b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Platform.c
index 0bec76e4..6963f39a 100644
--- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Platform.c
+++ b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Platform.c
@@ -257,8 +257,8 @@ MemMapInitialization (
   //
   PciSize = 0xFC00 - PciBase;
   AddIoMemoryBaseSizeHob (PciBase, PciSize);
-  PcdSet64 (PcdPciMmio32Base, PciBase);
-  PcdSet64 (PcdPciMmio32Size, PciSize);
+  PcdSet64S (PcdPciMmio32Base, PciBase);
+  PcdSet64S (PcdPciMmio32Size, PciSize);
   AddIoMemoryBaseSizeHob (0xFEC0, SIZE_4KB);
   AddIoMemoryBaseSizeHob (0xFED0, SIZE_1KB);
   if (mHostBridgeDevId == INTEL_ICH10_DEVICE_ID) {
@@ -300,8 +300,8 @@ MemMapInitialization (
 PciIoBase,
 PciIoSize
 );
-  PcdSet64 (PcdPciIoBase, PciIoBase);
-  PcdSet64 (PcdPciIoSize, PciIoSize);
+  PcdSet64S (PcdPciIoBase, PciIoBase);
+  PcdSet64S (PcdPciIoSize, PciIoSize);
 
   //
   // Add flash range.
@@ -367,7 +367,7 @@ MiscInitialization (
   ASSERT (FALSE);
   return;
   }
-  PcdSet16 (PcdSimicsX58HostBridgePciDevId, mHostBridgeDevId);
+  PcdSet16S (PcdSimicsX58HostBridgePciDevId, mHostBridgeDevId);
 
   //
   // If the appropriate IOspace enable bit is set, assume the ACPI PMBA
@@ -483,7 +483,7 @@ ReserveEmuVariableNvStore (
   VariableStore,
   (2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize)) / 1024
 ));
-  PcdSet64 (PcdEmuVariableNvStoreReserved, VariableStore);
+  PcdSet64S (PcdEmuVariableNvStoreReserved, VariableStore);
 }
 
 
-- 
2.18.0.windows.1


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

View/Reply Online (#63429): https://edk2.groups.io/g/devel/message/63429
Mute This Topic: https://groups.io/mt/75858614/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH] BaseTools/PeCoffLoaderEx: Remove the unused local variable

2020-07-28 Thread Liming Gao
Reviewed-by: Liming Gao 

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Abner Chang
Sent: 2020年7月25日 10:35
To: devel@edk2.groups.io
Cc: abner.ch...@hpe.com; Feng, Bob C ; Gao, Liming 
; Daniel Schaefer 
Subject: [edk2-devel] [PATCH] BaseTools/PeCoffLoaderEx: Remove the unused local 
variable

BZ:2864 GCC build fails due to variable self assignment.

This local variable is not used at any where, we can just remove it.

Signed-off-by: Abner Chang 

Cc: Bob Feng 
Cc: Liming Gao 
Cc: Daniel Schaefer 
---
 BaseTools/Source/C/Common/PeCoffLoaderEx.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/BaseTools/Source/C/Common/PeCoffLoaderEx.c 
b/BaseTools/Source/C/Common/PeCoffLoaderEx.c
index 588b3a2f84..799f282970 100644
--- a/BaseTools/Source/C/Common/PeCoffLoaderEx.c
+++ b/BaseTools/Source/C/Common/PeCoffLoaderEx.c
@@ -127,10 +127,7 @@ PeCoffLoaderRelocateRiscVImage (
 {   UINT32 Value;   UINT32 Value2;-  UINT32 OrgValue; -  OrgValue = *(UINT32 
*) Fixup;-  OrgValue = OrgValue;   switch ((*Reloc) >> 12) {   case 
EFI_IMAGE_REL_BASED_RISCV_HI20:   RiscVHi20Fixup = (UINT32 *) Fixup;-- 
2.25.0


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

View/Reply Online (#63269): https://edk2.groups.io/g/devel/message/63269
Mute This Topic: https://groups.io/mt/75779823/1759384
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [liming@intel.com] 
-=-=-=-=-=-=


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

View/Reply Online (#63428): https://edk2.groups.io/g/devel/message/63428
Mute This Topic: https://groups.io/mt/75779823/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH 00/15] Add a plugin LicenseCheck in open ci

2020-07-28 Thread Michael D Kinney
Hi Liming,

There are exception to PatchCheck as well for line endings and tabs.

Seems like a similar problem, and a single CI plugin could
support all these cases.

Mike

> -Original Message-
> From: Gao, Liming 
> Sent: Tuesday, July 28, 2020 6:12 PM
> To: Kinney, Michael D ;
> Zhang, Shenglei ;
> devel@edk2.groups.io
> Cc: Sean Brogan ; Bret
> Barkelew ; Dong, Eric
> ; Laszlo Ersek ;
> Gao, Zhichao ; Yao, Jiewen
> ; Chao Zhang
> ; Justen, Jordan L
> ; Maciej Rabeda
> ; Wu, Jiaxin
> ; Fu, Siyuan ;
> Wang, Jian J ; Wu, Hao A
> ; Andrew Fish ; Ni,
> Ray ; Lu, XiaoyuX
> ; Ard Biesheuvel
> ; Leif Lindholm
> ; Gao, Liming 
> Subject: RE: [PATCH 00/15] Add a plugin LicenseCheck in
> open ci
> 
> Mike:
>   Previous discussion
> (https://edk2.groups.io/g/devel/message/62494) is to
> revert the license check change in PatchCheck, and
> enable license check as plugin. If so, the package
> maintainers can configure the package level exception
> list to allow some special cases, such as autogen file.
> 
> Thanks
> Liming
> -Original Message-
> From: Kinney, Michael D 
> Sent: 2020年7月29日 7:34
> To: Zhang, Shenglei ;
> devel@edk2.groups.io; Kinney, Michael D
> 
> Cc: Sean Brogan ; Bret
> Barkelew ; Dong, Eric
> ; Laszlo Ersek ;
> Gao, Zhichao ; Yao, Jiewen
> ; Chao Zhang
> ; Justen, Jordan L
> ; Maciej Rabeda
> ; Wu, Jiaxin
> ; Fu, Siyuan ;
> Gao, Liming ; Wang, Jian J
> ; Wu, Hao A ;
> Andrew Fish ; Ni, Ray
> ; Lu, XiaoyuX ;
> Ard Biesheuvel ; Leif Lindholm
> 
> Subject: RE: [PATCH 00/15] Add a plugin LicenseCheck in
> open ci
> 
> CI already runs PatchCheck.  If we ported PatchCheck to
> a CI plugin, then the plugin could perform both the
> current PatchCheck features and the license check.
> 
> What this option evaluated?
> 
> If we did provide PatchCheck as a CI plugin, developers
> I believe developers could run a stuart command likely
> for the NOOPT target to run a PatchCheck CI plugin
> locally.
> 
> Thanks,
> 
> Mike
> 
> > -Original Message-
> > From: Zhang, Shenglei 
> > Sent: Monday, July 20, 2020 1:37 AM
> > To: devel@edk2.groups.io
> > Cc: Kinney, Michael D ;
> Sean Brogan
> > ; Bret Barkelew
> > ; Dong, Eric
> ;
> > Laszlo Ersek ; Gao, Zhichao
> > ; Yao, Jiewen
> ; Chao
> > Zhang ; Justen, Jordan L
> > ; Maciej Rabeda
> > ; Wu, Jiaxin
> ; Fu,
> > Siyuan ; Gao, Liming
> ;
> > Wang, Jian J ; Wu, Hao A
> ;
> > Andrew Fish ; Ni, Ray
> ; Lu, XiaoyuX
> > ; Ard Biesheuvel
> ; Leif
> > Lindholm 
> > Subject: [PATCH 00/15] Add a plugin LicenseCheck in
> open ci
> >
> > REF:
> https://bugzilla.tianocore.org/show_bug.cgi?id=2691
> > REF:
> https://bugzilla.tianocore.org/show_bug.cgi?id=2833
> > LicenseCheck is now enabled in PatchCheck.py. But
> there's a patch
> > "Revert 'BaseTools/PatchCheck.py: Add LicenseCheck'"
> > to suggest revert the change.These patch series
> introduce a plugin
> > LicenseCheck into open ci so that license issues can
> still be checked
> > after the checker is disabled in PatchCheck.py.
> > 1/15 is the plugin implementation.
> > 2/15 ~ 15/15 introduce sections "IgnoreFiles" to allow
> developers to
> > skip license check for some files like generated
> files.
> >
> > Only BSD-2-Clause-Patent and BSD-3-Clause-Patent can
> pass this
> > checker.
> >
> > Cc: Michael D Kinney 
> > Cc: Sean Brogan 
> > Cc: Bret Barkelew 
> > Cc: Eric Dong 
> > Cc: Laszlo Ersek 
> > Cc: Zhichao Gao 
> > Cc: Jiewen Yao 
> > Cc: Chao Zhang 
> > Cc: Jordan Justen 
> > Cc: Maciej Rabeda 
> > Cc: Jiaxin Wu 
> > Cc: Siyuan Fu 
> > Cc: Liming Gao 
> > Cc: Jian J Wang 
> > Cc: Hao A Wu 
> > Cc: Jordan Justen 
> > Cc: Andrew Fish 
> > Cc: Ray Ni 
> > Cc: Jian J Wang 
> > Cc: Xiaoyu Lu 
> > Cc: Ard Biesheuvel 
> > Cc: Leif Lindholm 
> > Shenglei Zhang (15):
> >   .pytool/Plugin: Add a plugin LicenseCheck
> >   FatPkg/FatPkg.ci.yaml: Add configuration for
> LicenseCheck
> >   ArmVirtPkg/ArmVirtPkg.ci.yaml: Add configuration for
> LicenseCheck
> >   CryptoPkg/CryptoPkg.ci.yaml: Add configuration for
> LicenseCheck
> >   EmulatorPkg/EmulatorPkg.ci.yaml: Add configuration
> for LicenseCheck
> >   FmpDevicePkg/FmpDevicePkg.ci.yaml: Add configuration
> for
> > LicenseCheck
> >   MdeModulePkg/MdeModulePkg.ci.yaml: Add configuration
> for
> > LicenseCheck
> >   MdePkg/MdePkg.ci.yaml: Add configuration for
> LicenseCheck
> >   NetworkPkg/NetworkPkg.ci.yaml: Add configuration for
> LicenseCheck
> >   OvmfPkg/OvmfPkg.ci.yaml: Add configuration for
> LicenseCheck
> >   PcAtChipsetPkg/PcAtChipsetPkg.ci.yaml: Add
> configuration for
> > LicenseCheck
> >   SecurityPkg/SecurityPkg.ci.yaml: Add configuration
> for LicenseCheck
> >   ShellPkg/ShellPkg.ci.yaml: Add configuration for
> LicenseCheck
> >   UefiCpuPkg/UefiCpuPkg.ci.yaml: Add configuration for
> LicenseCheck
> >   UnitTestFrameworkPkg: Add configuration for
> LicenseCheck in yaml
> > file
> >
> >  .pytool/Plugin/LicenseCheck/LicenseCheck.py   | 118
> > ++
> >  .../LicenseCheck/LicenseCheck_plug_in.yaml|  11
> ++
> >  .pytool/Pl

[edk2-devel] TianoCore Bug Triage - APAC / NAMO - Tue, 07/28/2020 6:30pm-7:30pm #cal-reminder

2020-07-28 Thread devel@edk2.groups.io Calendar
*Reminder:* TianoCore Bug Triage - APAC / NAMO

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

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

View Event ( https://edk2.groups.io/g/devel/viewevent?eventid=816383 )

*Organizer:* Brian Richardson brian.richard...@intel.com ( 
brian.richard...@intel.com?subject=Re:%20Event:%20TianoCore%20Bug%20Triage%20-%20APAC%20%2F%20NAMO
 )

*Description:*

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

Meeting URL

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

Meeting ID

889 357 567

Want to dial in from a phone?

Dial one of the following numbers:

+1.408.740.7256 (US (San Jose))

+1.408.317.9253 (US (Primary, San Jose))

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

Enter the meeting ID and passcode followed by #

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

View/Reply Online (#63426): https://edk2.groups.io/g/devel/message/63426
Mute This Topic: https://groups.io/mt/75856961/21656
Mute #cal-reminder: https://groups.io/g/edk2/mutehashtag/cal-reminder
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT instruction

2020-07-28 Thread Liming Gao
Laszlo:
 Yes. This solution makes sense. 

Thanks
Liming
-Original Message-
From: Laszlo Ersek  
Sent: 2020年7月28日 23:19
To: Tom Lendacky ; Gao, Liming ; 
devel@edk2.groups.io
Cc: Brijesh Singh ; Ard Biesheuvel 
; Dong, Eric ; Justen, Jordan L 
; Kinney, Michael D ; 
Ni, Ray 
Subject: Re: [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT 
instruction

On 07/28/20 16:13, Tom Lendacky wrote:

> IIUC, create a VmgExit.c file in MdePkg/Library/BaseLib/Ia32/ that 
> doesn't actually encode the VMGEXIT instruction, just calls 
> CpuBreakpoint(), e.g.:
>
>   VOID
>   EFIAPI
>   AsmVmgExit (
> VOID
> )
>   {
> CpuBreakpoint();
>   }
>

Yes -- either that, or even just open-code (copy) what we have in

  MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.nasm

now:

global ASM_PFX(AsmVmgExit)
ASM_PFX(AsmVmgExit):
;
; VMGEXIT makes no sense on IA32, and NASM versions before 2.12 cannot ; 
translate the 64-bit "rep vmmcall" instruction into elf32 format. So ; provide 
a stub implementation that is identical to CpuBreakpoint(). In ; practice, 
AsmVmgExit() should never be called on IA32.
;
int  3
ret

Because this assembly-language implementation might be more "true" to the name 
"AsmVmgExit".

Liming, would you be OK with this approach? In other words, the set of files 
changed/introduced in this patch would not change, just the implementation of 
IA32 AsmVmgExit().

> The other alternative is to use a DB-encoded instruction, though I 
> know that isn't the most popular approach.

Right, I've been quite on a quest to eliminate DBs that encode instructions.

> The BITS 64 method to generate the instruction bytes is also used in 
> OvmfPkg/ResetVector/Ia32/PageTables64.asm, but that file is only 
> included when ARCH_X64 is defined, so there shouldn't be an issue 
> there, plus the nasm file format is bin (-f bin).

I confirm that; the following commands all work on RHEL7, with this series 
applied:

$ build -b NOOPT -t GCC48 -m OvmfPkg/ResetVector/ResetVector.inf -a IA32
-p OvmfPkg/OvmfPkgIa32.dsc
$ build -b NOOPT -t GCC48 -m OvmfPkg/ResetVector/ResetVector.inf -a IA32 -a X64 
-p OvmfPkg/OvmfPkgIa32X64.dsc
$ build -b NOOPT -t GCC48 -m OvmfPkg/ResetVector/ResetVector.inf -a X64 
-p OvmfPkg/OvmfPkgX64.dsc

Thanks!
Laszlo


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

View/Reply Online (#63425): https://edk2.groups.io/g/devel/message/63425
Mute This Topic: https://groups.io/mt/75824947/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH 00/15] Add a plugin LicenseCheck in open ci

2020-07-28 Thread Liming Gao
Mike:
  Previous discussion (https://edk2.groups.io/g/devel/message/62494) is to 
revert the license check change in PatchCheck, and enable license check as 
plugin. If so, the package maintainers can configure the package level 
exception list to allow some special cases, such as autogen file. 

Thanks
Liming
-Original Message-
From: Kinney, Michael D  
Sent: 2020年7月29日 7:34
To: Zhang, Shenglei ; devel@edk2.groups.io; Kinney, 
Michael D 
Cc: Sean Brogan ; Bret Barkelew 
; Dong, Eric ; Laszlo Ersek 
; Gao, Zhichao ; Yao, Jiewen 
; Chao Zhang ; Justen, Jordan L 
; Maciej Rabeda ; Wu, 
Jiaxin ; Fu, Siyuan ; Gao, Liming 
; Wang, Jian J ; Wu, Hao A 
; Andrew Fish ; Ni, Ray 
; Lu, XiaoyuX ; Ard Biesheuvel 
; Leif Lindholm 
Subject: RE: [PATCH 00/15] Add a plugin LicenseCheck in open ci

CI already runs PatchCheck.  If we ported PatchCheck to a CI plugin, then the 
plugin could perform both the current PatchCheck features and the license check.

What this option evaluated?

If we did provide PatchCheck as a CI plugin, developers I believe developers 
could run a stuart command likely for the NOOPT target to run a PatchCheck CI 
plugin locally.

Thanks,

Mike

> -Original Message-
> From: Zhang, Shenglei 
> Sent: Monday, July 20, 2020 1:37 AM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D ; Sean Brogan 
> ; Bret Barkelew 
> ; Dong, Eric ; 
> Laszlo Ersek ; Gao, Zhichao 
> ; Yao, Jiewen ; Chao 
> Zhang ; Justen, Jordan L 
> ; Maciej Rabeda 
> ; Wu, Jiaxin ; Fu, 
> Siyuan ; Gao, Liming ; 
> Wang, Jian J ; Wu, Hao A ; 
> Andrew Fish ; Ni, Ray ; Lu, XiaoyuX 
> ; Ard Biesheuvel ; Leif 
> Lindholm 
> Subject: [PATCH 00/15] Add a plugin LicenseCheck in open ci
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2691
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2833
> LicenseCheck is now enabled in PatchCheck.py. But there's a patch 
> "Revert 'BaseTools/PatchCheck.py: Add LicenseCheck'"
> to suggest revert the change.These patch series introduce a plugin 
> LicenseCheck into open ci so that license issues can still be checked 
> after the checker is disabled in PatchCheck.py.
> 1/15 is the plugin implementation.
> 2/15 ~ 15/15 introduce sections "IgnoreFiles" to allow developers to 
> skip license check for some files like generated files.
> 
> Only BSD-2-Clause-Patent and BSD-3-Clause-Patent can pass this 
> checker.
> 
> Cc: Michael D Kinney 
> Cc: Sean Brogan 
> Cc: Bret Barkelew 
> Cc: Eric Dong 
> Cc: Laszlo Ersek 
> Cc: Zhichao Gao 
> Cc: Jiewen Yao 
> Cc: Chao Zhang 
> Cc: Jordan Justen 
> Cc: Maciej Rabeda 
> Cc: Jiaxin Wu 
> Cc: Siyuan Fu 
> Cc: Liming Gao 
> Cc: Jian J Wang 
> Cc: Hao A Wu 
> Cc: Jordan Justen 
> Cc: Andrew Fish 
> Cc: Ray Ni 
> Cc: Jian J Wang 
> Cc: Xiaoyu Lu 
> Cc: Ard Biesheuvel 
> Cc: Leif Lindholm 
> Shenglei Zhang (15):
>   .pytool/Plugin: Add a plugin LicenseCheck
>   FatPkg/FatPkg.ci.yaml: Add configuration for LicenseCheck
>   ArmVirtPkg/ArmVirtPkg.ci.yaml: Add configuration for LicenseCheck
>   CryptoPkg/CryptoPkg.ci.yaml: Add configuration for LicenseCheck
>   EmulatorPkg/EmulatorPkg.ci.yaml: Add configuration for LicenseCheck
>   FmpDevicePkg/FmpDevicePkg.ci.yaml: Add configuration for 
> LicenseCheck
>   MdeModulePkg/MdeModulePkg.ci.yaml: Add configuration for 
> LicenseCheck
>   MdePkg/MdePkg.ci.yaml: Add configuration for LicenseCheck
>   NetworkPkg/NetworkPkg.ci.yaml: Add configuration for LicenseCheck
>   OvmfPkg/OvmfPkg.ci.yaml: Add configuration for LicenseCheck
>   PcAtChipsetPkg/PcAtChipsetPkg.ci.yaml: Add configuration for
> LicenseCheck
>   SecurityPkg/SecurityPkg.ci.yaml: Add configuration for LicenseCheck
>   ShellPkg/ShellPkg.ci.yaml: Add configuration for LicenseCheck
>   UefiCpuPkg/UefiCpuPkg.ci.yaml: Add configuration for LicenseCheck
>   UnitTestFrameworkPkg: Add configuration for LicenseCheck in yaml 
> file
> 
>  .pytool/Plugin/LicenseCheck/LicenseCheck.py   | 118
> ++
>  .../LicenseCheck/LicenseCheck_plug_in.yaml|  11 ++
>  .pytool/Plugin/LicenseCheck/Readme.md |  17 +++
>  ArmVirtPkg/ArmVirtPkg.ci.yaml |   4 +
>  CryptoPkg/CryptoPkg.ci.yaml   |   3 +
>  EmulatorPkg/EmulatorPkg.ci.yaml   |   4 +
>  FatPkg/FatPkg.ci.yaml |   3 +
>  FmpDevicePkg/FmpDevicePkg.ci.yaml |   3 +
>  MdeModulePkg/MdeModulePkg.ci.yaml |   4 +
>  MdePkg/MdePkg.ci.yaml |   4 +
>  NetworkPkg/NetworkPkg.ci.yaml |   3 +
>  OvmfPkg/OvmfPkg.ci.yaml   |   4 +
>  PcAtChipsetPkg/PcAtChipsetPkg.ci.yaml |   3 +
>  SecurityPkg/SecurityPkg.ci.yaml   |   3 +
>  ShellPkg/ShellPkg.ci.yaml |   3 +
>  UefiCpuPkg/UefiCpuPkg.ci.yaml |   3 +
>  .../UnitTestFrameworkPkg.ci.yaml  |   4 +
>  17 files changed, 194 insertions(+)
>  create mode 100644
> .pytool/Plugin/LicenseCheck/LicenseCheck.py
>  create mode 100644
> .pytool/Plugi

Re: [edk2-devel] [PATCH v1 1/2] CryptoPkg: OpensslLib: Use RngLib to generate entropy in rand_pool

2020-07-28 Thread Guomin Jiang
Add comments inline

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of
> Matthew Carlson
> Sent: Tuesday, July 28, 2020 9:53 AM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen ; Wang, Jian J
> ; Lu, XiaoyuX ; Matthew
> Carlson 
> Subject: [edk2-devel] [PATCH v1 1/2] CryptoPkg: OpensslLib: Use RngLib to
> generate entropy in rand_pool
> 
> From: Matthew Carlson 
> 
> Changes OpenSSL to no longer depend on TimerLib and instead use RngLib.
> This allows platforms to decide for themsevles what sort of entropy source
> they provide to OpenSSL and TlsLib.
> 
> Cc: Jiewen Yao 
> Cc: Jian J Wang 
> Cc: Xiaoyu Lu 
> Signed-off-by: Matthew Carlson 
> ---
>  CryptoPkg/Library/OpensslLib/rand_pool.c   | 200 ++--
>  CryptoPkg/Library/OpensslLib/rand_pool_noise.c |  29 ---
>  CryptoPkg/Library/OpensslLib/rand_pool_noise_tsc.c |  43 -
>  CryptoPkg/Library/OpensslLib/OpensslLib.inf|  15 +-
>  CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf  |  15 +-
>  CryptoPkg/Library/OpensslLib/rand_pool_noise.h |  29 ---
>  6 files changed, 20 insertions(+), 311 deletions(-)
> 
> diff --git a/CryptoPkg/Library/OpensslLib/rand_pool.c
> b/CryptoPkg/Library/OpensslLib/rand_pool.c
> index 9e0179b03490..55bf6c9c6950 100644
> --- a/CryptoPkg/Library/OpensslLib/rand_pool.c
> +++ b/CryptoPkg/Library/OpensslLib/rand_pool.c
> @@ -11,44 +11,10 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include 
> 
> 
> 
>  #include 
> 
> -#include 
> 
> +#include 
> 
> 
> 
>  #include "rand_pool_noise.h"

It seem that you delete the rand_pool_noise.h file, but forget to remove the 
header?

> 
> 
> 
> -/**
> 
> -  Get some randomness from low-order bits of GetPerformanceCounter
> results.
> 
> -  And combine them to the 64-bit value
> 
> -
> 
> -  @param[out] RandBuffer pointer to store the 64-bit random value.
> 
> -
> 
> -  @retval TRUERandom number generated successfully.
> 
> -  @retval FALSE   Failed to generate.
> 
> -**/
> 
> -STATIC
> 
> -BOOLEAN
> 
> -EFIAPI
> 
> -GetRandNoise64FromPerformanceCounter(
> 
> -  OUT UINT64  *Rand
> 
> -  )
> 
> -{
> 
> -  UINT32 Index;
> 
> -  UINT32 *RandPtr;
> 
> -
> 
> -  if (NULL == Rand) {
> 
> -return FALSE;
> 
> -  }
> 
> -
> 
> -  RandPtr = (UINT32 *) Rand;
> 
> -
> 
> -  for (Index = 0; Index < 2; Index ++) {
> 
> -*RandPtr = (UINT32) (GetPerformanceCounter () & 0xFF);
> 
> -MicroSecondDelay (10);
> 
> -RandPtr++;
> 
> -  }
> 
> -
> 
> -  return TRUE;
> 
> -}
> 
> -
> 
>  /**
> 
>Calls RandomNumber64 to fill
> 
>a buffer of arbitrary size with random bytes.
> 
> @@ -56,8 +22,8 @@ GetRandNoise64FromPerformanceCounter(
>@param[in]   LengthSize of the buffer, in bytes,  to fill with.
> 
>@param[out]  RandBufferPointer to the buffer to store the random
> result.
> 
> 
> 
> -  @retval EFI_SUCCESSRandom bytes generation succeeded.
> 
> -  @retval EFI_NOT_READY  Failed to request random bytes.
> 
> +  @retval TrueRandom bytes generation succeeded.
> 
> +  @retval False   Failed to request random bytes.
> 
> 
> 
>  **/
> 
>  STATIC
> 
> @@ -73,17 +39,17 @@ RandGetBytes (
> 
> 
>Ret = FALSE;
> 
> 
> 
> +  if (RandBuffer == NULL) {
> 
> +DEBUG((DEBUG_ERROR, "[OPENSSL_RAND_POOL] NULL RandBuffer. No
> random numbers are generated and your system is not secure\n"));
> 
> +ASSERT(FALSE); // Since we can't generate random numbers, we should
> assert. Otherwise we will just blow up later.
> 
> +return Ret;
> 
> +  }
> 
> +
> 
> +
> 
>while (Length > 0) {
> 
> -//
> 
> -// Get random noise from platform.
> 
> -// If it failed, fallback to PerformanceCounter
> 
> -// If you really care about security, you must override
> 
> -// GetRandomNoise64FromPlatform.
> 
> -//
> 
> -Ret = GetRandomNoise64 (&TempRand);
> 
> -if (Ret == FALSE) {
> 
> -  Ret = GetRandNoise64FromPerformanceCounter (&TempRand);
> 
> -}
> 
> +// Use RngLib to get random number
> 
> +Ret = GetRandomNumber64(&TempRand);
> 
> +
> 
>  if (!Ret) {
> 
>return Ret;
> 
>  }
> 
> @@ -100,125 +66,6 @@ RandGetBytes (
>return Ret;
> 
>  }
> 
> 
> 
> -/**
> 
> -  Creates a 128bit random value that is fully forward and backward prediction
> resistant,
> 
> -  suitable for seeding a NIST SP800-90 Compliant.
> 
> -  This function takes multiple random numbers from PerformanceCounter to
> ensure reseeding
> 
> -  and performs AES-CBC-MAC over the data to compute the seed value.
> 
> -
> 
> -  @param[out]  SeedBufferPointer to a 128bit buffer to store the random
> seed.
> 
> -
> 
> -  @retval TRUERandom seed generation succeeded.
> 
> -  @retval FALSE  Failed to request random bytes.
> 
> -
> 
> -**/
> 
> -STATIC
> 
> -BOOLEAN
> 
> -EFIAPI
> 
> -RandGetSeed128 (
> 
> -  OUT UINT8*SeedBuffer
> 
> -  )
> 
> -{
> 
> -  BOOLEAN Ret;
> 
> -  UINT8   RandByte[16];
> 
> -  UINT8   Key[16];
> 
> -  UINT8  

[edk2-devel] [PATCH edk2-platforms 1/1] Silicon/Socionext: rename Emmc.c to fix build error

2020-07-28 Thread Masahisa Kojima
Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/ has both an Emmc.asl
and an Emmc.c file, which the patch(edk2 commit:0a4aa20e8d446 "BaseTools:
Compile AML bytecode arrays into .obj file") both generate an Emmc.obj
in the same output directory.

To fix the build error for Developerbox platform, this patch renames
the Emmc.c.

Suggested-by: Leif Lindholm 
Cc: Pierre Gondois 
Signed-off-by: Masahisa Kojima 
---
 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf   | 2 +-
 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/{Emmc.c => EmmcDxe.c} | 0
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf 
b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf
index aa3cbdf35c73..74bbd2d48b30 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf
+++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf
@@ -18,7 +18,7 @@ [Defines]
 
 [Sources]
   Emmc.asl
-  Emmc.c
+  EmmcDxe.c
   Optee.asl
   Pci.c
   PlatformDxe.c
diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c 
b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/EmmcDxe.c
similarity index 100%
rename from Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c
rename to Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/EmmcDxe.c
-- 
2.17.1


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

View/Reply Online (#63422): https://edk2.groups.io/g/devel/message/63422
Mute This Topic: https://groups.io/mt/75856685/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v5 4/5] BaseTools: Compile AML bytecode arrays into .obj file

2020-07-28 Thread Masahisa Kojima
Hi Pierre, Leif,

Thank you for your comment, I could replicate the issue.
I will submit the patch to rename the Emmc.c.

Regards,
Masahisa

On Tue, 28 Jul 2020 at 20:19, Leif Lindholm  wrote:
>
> Yes,
>
> Do a git clean -fx in your edk2 directory, and delete your Build
> directory completely.
>
> Regards,
>
> Leif
>
> On Tue, Jul 28, 2020 at 09:04:44 +, Pierre Gondois wrote:
> > Hello Masahisa,
> >
> > Maybe you will have to delete/re-generate the build_rule.txt and
> > tools_def.txt files that you have. This patch makes the build system
> > generate a .amli file for each .asl file. If there is no Emmc.amli
> > in your Build directory, this might be the reason.
> >
> > Regards,
> > Pierre
> >
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of Masahisa 
> > Kojima via groups.io
> > Sent: Tuesday, July 28, 2020 2:38 AM
> > To: Leif Lindholm 
> > Cc: devel@edk2.groups.io; Gao, Liming ; Pierre 
> > Gondois ; Sami Mujawar ; 
> > Tomas Pilar ; Feng, Bob C ; Ard 
> > Biesheuvel 
> > Subject: Re: [edk2-devel] [PATCH v5 4/5] BaseTools: Compile AML bytecode 
> > arrays into .obj file
> >
> > Hi Leif,
> >
> > > > Masahisa - since Ard is still on holiday, could you create a patch
> > > > and send out for me to review? Either one of the files needs to be
> > > > renamed, or we need to move the .asl files (Emmc.asl and Optee.asl)
> > > > into a subdirectory.
> >
> > Probably I'm missing something, but my build for Developerbox platform is 
> > successful, with the latest edk2/edk2-platforms as of today.
> >
> > My build option is:
> > ---
> > export TARGET=Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> > export PROFILE=DEBUG
> > export ARCH=AARCH64
> > export TOOL_CHAIN_TAG=GCC5
> >
> > build -n $NUM_CPUS -a $ARCH -b $PROFILE -t $TOOL_CHAIN_TAG -p $TARGET -D 
> > SECURE_BOOT_ENABLE=TRUE -D X64EMU_ENABLE=TRUE -D TPM2_ENABLE=TRUE
> > ---
> >
> > GCC version is 
> > "gcc-linaro-6.4.1-2017.11-x86_64_aarch64-linux-gnu/bin/aarch64-linux-gnu-".
> >
> > Some Output directory information:
> > ~/src/uefi/Build/DeveloperBox$ find .|grep -i emmc 
> > ./DEBUG_GCC5/AARCH64/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe/OUTPUT/Emmc.iii
> > ./DEBUG_GCC5/AARCH64/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe/OUTPUT/Emmc.obj.deps
> > ./DEBUG_GCC5/AARCH64/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe/OUTPUT/Emmc.
> > ./DEBUG_GCC5/AARCH64/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe/OUTPUT/Emmc.obj
> > ./DEBUG_GCC5/AARCH64/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe/OUTPUT/Emmc.aml.deps
> > ./DEBUG_GCC5/AARCH64/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe/OUTPUT/Emmc.aml
> > ./DEBUG_GCC5/AARCH64/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe/OUTPUT/Emmc.i
> >
> > Regards,
> > Masahisa
> >
> > On Tue, 28 Jul 2020 at 09:40, Gao, Liming  wrote:
> > >
> > > Pierre:
> > >   Thanks for your investigation. For now, the module will rename source 
> > > file to avoid the file conflict. Can you submit one BZ for BaseTools to 
> > > enhance the logic for the autogen source file? This needs more discussion.
> > >
> > >   Yes. I agree to place the generated source file into $(DEBUG_DIR). You 
> > > can make another patch for this change.
> > >
> > > Thanks
> > > Liming
> > > -Original Message-
> > > From: devel@edk2.groups.io  On Behalf Of
> > > PierreGondois
> > > Sent: 2020年7月28日 1:10
> > > To: devel@edk2.groups.io; Gao, Liming ; Leif
> > > Lindholm ; Masahisa Kojima
> > > 
> > > Cc: Sami Mujawar ; Tomas Pilar
> > > ; Feng, Bob C ; Ard
> > > Biesheuvel 
> > > Subject: Re: [edk2-devel] [PATCH v5 4/5] BaseTools: Compile AML
> > > bytecode arrays into .obj file
> > >
> > > Hello,
> > >
> > > Liming:
> > > I didn't find anything preventing from having a .vfr and .c file with the 
> > > same name in the same scope.
> > >
> > > Everybody:
> > > The .c files generated (from either .vfr or .asl) are generated in the 
> > > Build//OUTPUT/ directory, with their original subdirectory stripped. 
> > > In the example below, the .c, .vfr and .asl files have been placed in 
> > > subdirectories in edk2/edk2-platforms. The recipes that have been 
> > > generated are:
> > > $(OUTPUT_DIR)/Emmc.obj : $(OUTPUT_DIR)/AslDir/Emmc.c (from Emmc.asl)
> > > $(OUTPUT_DIR)/Ip4Config2.obj : $(DEBUG_DIR)/VfrDir/Ip4Config2.c (from
> > > Ip4Config2.vfr) $(OUTPUT_DIR)/CDir/Emmc.obj :
> > > $(WORKSPACE)/edk2-platforms/Silicon/Socionext/SynQuacer/Drivers/Platfo
> > > rmDxe/CDir/Emmc.c (from Emmc.c)
> > >
> > > The only subdirectory that remains is the one from the .c files. Indeed, 
> > > the build system might detects subdirectories from the .inf file 
> > > location. As the .c file is generated in the OUTPUT directory where there 
> > > is no .inf file, it gets stripped.
> > > This means that if a subdirectory is created, it is for the .c file, but 
> > > I don't think we should modify this.
> > >
> > > Adding a pre/postfix to the genera

Re: [edk2-devel] [PATCH 00/15] Add a plugin LicenseCheck in open ci

2020-07-28 Thread Michael D Kinney
CI already runs PatchCheck.  If we ported PatchCheck to
a CI plugin, then the plugin could perform both the 
current PatchCheck features and the license check.

What this option evaluated?

If we did provide PatchCheck as a CI plugin, developers
I believe developers could run a stuart command likely
for the NOOPT target to run a PatchCheck CI plugin locally.

Thanks,

Mike

> -Original Message-
> From: Zhang, Shenglei 
> Sent: Monday, July 20, 2020 1:37 AM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D ; Sean
> Brogan ; Bret Barkelew
> ; Dong, Eric
> ; Laszlo Ersek ;
> Gao, Zhichao ; Yao, Jiewen
> ; Chao Zhang
> ; Justen, Jordan L
> ; Maciej Rabeda
> ; Wu, Jiaxin
> ; Fu, Siyuan ;
> Gao, Liming ; Wang, Jian J
> ; Wu, Hao A ;
> Andrew Fish ; Ni, Ray
> ; Lu, XiaoyuX ;
> Ard Biesheuvel ; Leif Lindholm
> 
> Subject: [PATCH 00/15] Add a plugin LicenseCheck in open
> ci
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2691
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2833
> LicenseCheck is now enabled in PatchCheck.py. But
> there's
> a patch "Revert 'BaseTools/PatchCheck.py: Add
> LicenseCheck'"
> to suggest revert the change.These patch series
> introduce
> a plugin LicenseCheck into open ci so that license
> issues can
> still be checked after the checker is disabled in
> PatchCheck.py.
> 1/15 is the plugin implementation.
> 2/15 ~ 15/15 introduce sections "IgnoreFiles" to allow
> developers
> to skip license check for some files like generated
> files.
> 
> Only BSD-2-Clause-Patent and BSD-3-Clause-Patent can
> pass this checker.
> 
> Cc: Michael D Kinney 
> Cc: Sean Brogan 
> Cc: Bret Barkelew 
> Cc: Eric Dong 
> Cc: Laszlo Ersek 
> Cc: Zhichao Gao 
> Cc: Jiewen Yao 
> Cc: Chao Zhang 
> Cc: Jordan Justen 
> Cc: Maciej Rabeda 
> Cc: Jiaxin Wu 
> Cc: Siyuan Fu 
> Cc: Liming Gao 
> Cc: Jian J Wang 
> Cc: Hao A Wu 
> Cc: Jordan Justen 
> Cc: Andrew Fish 
> Cc: Ray Ni 
> Cc: Jian J Wang 
> Cc: Xiaoyu Lu 
> Cc: Ard Biesheuvel 
> Cc: Leif Lindholm 
> Shenglei Zhang (15):
>   .pytool/Plugin: Add a plugin LicenseCheck
>   FatPkg/FatPkg.ci.yaml: Add configuration for
> LicenseCheck
>   ArmVirtPkg/ArmVirtPkg.ci.yaml: Add configuration for
> LicenseCheck
>   CryptoPkg/CryptoPkg.ci.yaml: Add configuration for
> LicenseCheck
>   EmulatorPkg/EmulatorPkg.ci.yaml: Add configuration for
> LicenseCheck
>   FmpDevicePkg/FmpDevicePkg.ci.yaml: Add configuration
> for LicenseCheck
>   MdeModulePkg/MdeModulePkg.ci.yaml: Add configuration
> for LicenseCheck
>   MdePkg/MdePkg.ci.yaml: Add configuration for
> LicenseCheck
>   NetworkPkg/NetworkPkg.ci.yaml: Add configuration for
> LicenseCheck
>   OvmfPkg/OvmfPkg.ci.yaml: Add configuration for
> LicenseCheck
>   PcAtChipsetPkg/PcAtChipsetPkg.ci.yaml: Add
> configuration for
> LicenseCheck
>   SecurityPkg/SecurityPkg.ci.yaml: Add configuration for
> LicenseCheck
>   ShellPkg/ShellPkg.ci.yaml: Add configuration for
> LicenseCheck
>   UefiCpuPkg/UefiCpuPkg.ci.yaml: Add configuration for
> LicenseCheck
>   UnitTestFrameworkPkg: Add configuration for
> LicenseCheck in yaml file
> 
>  .pytool/Plugin/LicenseCheck/LicenseCheck.py   | 118
> ++
>  .../LicenseCheck/LicenseCheck_plug_in.yaml|  11 ++
>  .pytool/Plugin/LicenseCheck/Readme.md |  17 +++
>  ArmVirtPkg/ArmVirtPkg.ci.yaml |   4 +
>  CryptoPkg/CryptoPkg.ci.yaml   |   3 +
>  EmulatorPkg/EmulatorPkg.ci.yaml   |   4 +
>  FatPkg/FatPkg.ci.yaml |   3 +
>  FmpDevicePkg/FmpDevicePkg.ci.yaml |   3 +
>  MdeModulePkg/MdeModulePkg.ci.yaml |   4 +
>  MdePkg/MdePkg.ci.yaml |   4 +
>  NetworkPkg/NetworkPkg.ci.yaml |   3 +
>  OvmfPkg/OvmfPkg.ci.yaml   |   4 +
>  PcAtChipsetPkg/PcAtChipsetPkg.ci.yaml |   3 +
>  SecurityPkg/SecurityPkg.ci.yaml   |   3 +
>  ShellPkg/ShellPkg.ci.yaml |   3 +
>  UefiCpuPkg/UefiCpuPkg.ci.yaml |   3 +
>  .../UnitTestFrameworkPkg.ci.yaml  |   4 +
>  17 files changed, 194 insertions(+)
>  create mode 100644
> .pytool/Plugin/LicenseCheck/LicenseCheck.py
>  create mode 100644
> .pytool/Plugin/LicenseCheck/LicenseCheck_plug_in.yaml
>  create mode 100644
> .pytool/Plugin/LicenseCheck/Readme.md
> 
> --
> 2.18.0.windows.1


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

View/Reply Online (#63420): https://edk2.groups.io/g/devel/message/63420
Mute This Topic: https://groups.io/mt/75678207/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH 06/15] FmpDevicePkg/FmpDevicePkg.ci.yaml: Add configuration for LicenseCheck

2020-07-28 Thread Michael D Kinney
Reviewed-by: Michael D Kinney 

> -Original Message-
> From: Zhang, Shenglei 
> Sent: Monday, July 20, 2020 1:37 AM
> To: devel@edk2.groups.io
> Cc: Gao, Liming ; Kinney, Michael
> D 
> Subject: [PATCH 06/15]
> FmpDevicePkg/FmpDevicePkg.ci.yaml: Add configuration for
> LicenseCheck
> 
> Add configuration IgnoreFiles for package config files.
> So users can rely on this to skip license conflict for
> some generated files.
> 
> Cc: Liming Gao 
> Cc: Michael D Kinney 
> Signed-off-by: Shenglei Zhang 
> ---
>  FmpDevicePkg/FmpDevicePkg.ci.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/FmpDevicePkg/FmpDevicePkg.ci.yaml
> b/FmpDevicePkg/FmpDevicePkg.ci.yaml
> index 74a0aefe8e49..d498ad7d21e1 100644
> --- a/FmpDevicePkg/FmpDevicePkg.ci.yaml
> +++ b/FmpDevicePkg/FmpDevicePkg.ci.yaml
> @@ -5,6 +5,9 @@
>  # SPDX-License-Identifier: BSD-2-Clause-Patent
>  ##
>  {
> +"LicenseCheck": {
> +"IgnoreFiles": []
> +},
>  "CompilerPlugin": {
>  "DscPath": "FmpDevicePkg.dsc"
>  },
> --
> 2.18.0.windows.1


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

View/Reply Online (#63419): https://edk2.groups.io/g/devel/message/63419
Mute This Topic: https://groups.io/mt/75678214/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH 15/15] UnitTestFrameworkPkg: Add configuration for LicenseCheck in yaml file

2020-07-28 Thread Michael D Kinney
Reviewed-by: Michael D Kinney 

> -Original Message-
> From: devel@edk2.groups.io  On
> Behalf Of Zhang, Shenglei
> Sent: Monday, July 20, 2020 1:37 AM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D ; Sean
> Brogan ; Bret Barkelew
> 
> Subject: [edk2-devel] [PATCH 15/15]
> UnitTestFrameworkPkg: Add configuration for LicenseCheck
> in yaml file
> 
> Add configuration IgnoreFiles for package config files.
> So users can rely on this to skip license conflict for
> some generated files.
> 
> Cc: Michael D Kinney 
> Cc: Sean Brogan 
> Cc: Bret Barkelew 
> Signed-off-by: Shenglei Zhang 
> ---
>  UnitTestFrameworkPkg/UnitTestFrameworkPkg.ci.yaml | 4
> 
>  1 file changed, 4 insertions(+)
> 
> diff --git
> a/UnitTestFrameworkPkg/UnitTestFrameworkPkg.ci.yaml
> b/UnitTestFrameworkPkg/UnitTestFrameworkPkg.ci.yaml
> index 51e172537f8a..2988e03f763f 100644
> --- a/UnitTestFrameworkPkg/UnitTestFrameworkPkg.ci.yaml
> +++ b/UnitTestFrameworkPkg/UnitTestFrameworkPkg.ci.yaml
> @@ -5,6 +5,10 @@
>  # SPDX-License-Identifier: BSD-2-Clause-Patent
>  ##
>  {
> +## options defined .pytool/Plugin/LicenseCheck
> +"LicenseCheck": {
> +"IgnoreFiles": []
> +},
>  ## options defined .pytool/Plugin/CompilerPlugin
>  "CompilerPlugin": {
>  "DscPath": "UnitTestFrameworkPkg.dsc"
> --
> 2.18.0.windows.1
> 
> 
> 


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

View/Reply Online (#63418): https://edk2.groups.io/g/devel/message/63418
Mute This Topic: https://groups.io/mt/75678226/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH 01/15] .pytool/Plugin: Add a plugin LicenseCheck

2020-07-28 Thread Michael D Kinney
Where did the requirement for BSD-3-Clause-Patent come from?

Thanks,

Mike

> -Original Message-
> From: Zhang, Shenglei 
> Sent: Monday, July 20, 2020 1:37 AM
> To: devel@edk2.groups.io
> Cc: Sean Brogan ; Bret
> Barkelew ; Kinney, Michael
> D ; Gao, Liming
> 
> Subject: [PATCH 01/15] .pytool/Plugin: Add a plugin
> LicenseCheck
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2691
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2833
> Add a plugin to check license conflict for new added
> files in a patch. It will report out errors when meeting
> files which are now contributed under BSD-2-Clause-
> Patent
> or BSD-3-Clause-Patent.
> 
> Cc: Sean Brogan 
> Cc: Bret Barkelew 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Signed-off-by: Shenglei Zhang 
> ---
>  .pytool/Plugin/LicenseCheck/LicenseCheck.py   | 118
> ++
>  .../LicenseCheck/LicenseCheck_plug_in.yaml|  11 ++
>  .pytool/Plugin/LicenseCheck/Readme.md |  17 +++
>  3 files changed, 146 insertions(+)
>  create mode 100644
> .pytool/Plugin/LicenseCheck/LicenseCheck.py
>  create mode 100644
> .pytool/Plugin/LicenseCheck/LicenseCheck_plug_in.yaml
>  create mode 100644
> .pytool/Plugin/LicenseCheck/Readme.md
> 
> diff --git a/.pytool/Plugin/LicenseCheck/LicenseCheck.py
> b/.pytool/Plugin/LicenseCheck/LicenseCheck.py
> new file mode 100644
> index ..98941ddda758
> --- /dev/null
> +++ b/.pytool/Plugin/LicenseCheck/LicenseCheck.py
> @@ -0,0 +1,118 @@
> +# @file LicenseCheck.py
> +#
> +# Copyright (c) 2020, Intel Corporation. All rights
> reserved.
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +##
> +
> +import os
> +import logging
> +import re
> +from io import StringIO
> +from typing import List, Tuple
> +from
> edk2toolext.environment.plugintypes.ci_build_plugin
> import ICiBuildPlugin
> +from edk2toolext.environment.var_dict import VarDict
> +from edk2toollib.utility_functions import RunCmd
> +
> +
> +class LicenseCheck(ICiBuildPlugin):
> +
> +"""
> +A CiBuildPlugin to check the license for new added
> files.
> +
> +Configuration options:
> +"LicenseCheck": {
> +"IgnoreFiles": []
> +},
> +"""
> +
> +license_format_preflix = 'SPDX-License-Identifier'
> +
> +bsd2_patent = 'BSD-2-Clause-Patent'
> +
> +bsd3_patent = 'BSD-3-Clause-Patent'
> +
> +Readdedfileformat = re.compile(r'\+\+\+ b\/(.*)')
> +
> +file_extension_list = [".c", ".h", ".inf", ".dsc",
> ".dec", ".py", ".bat", ".sh", ".uni", ".yaml",
> +   ".fdf", ".inc", "yml",
> ".asm", ".asm16", ".asl", ".vfr", ".s", ".S", ".aslc",
> +   ".nasm", ".nasmb", ".idf",
> ".Vfr", ".H"]
> +
> +def GetTestName(self, packagename: str,
> environment: VarDict) -> tuple:
> +""" Provide the testcase name and classname for
> use in reporting
> +testclassname: a descriptive string for the
> testcase can include whitespace
> +classname: should be patterned
> ..
> +
> +Args:
> +  packagename: string containing name of
> package to build
> +  environment: The VarDict for the test to
> run in
> +Returns:
> +a tuple containing the testcase name
> and the classname
> +(testcasename, classname)
> +"""
> +return ("Check for license for " + packagename,
> packagename + ".LicenseCheck")
> +
> +##
> +# External function of plugin.  This function is
> used to perform the task of the ci_build_plugin Plugin
> +#
> +#   - package is the edk2 path to package.  This
> means workspace/packagepath relative.
> +#   - edk2path object configured with workspace and
> packages path
> +#   - PkgConfig Object (dict) for the pkg
> +#   - EnvConfig Object
> +#   - Plugin Manager Instance
> +#   - Plugin Helper Obj Instance
> +#   - Junit Logger
> +#   - output_stream the StringIO output stream from
> this plugin via logging
> +def RunBuildPlugin(self, packagename, Edk2pathObj,
> pkgconfig, environment, PLM, PLMHelper, tc,
> output_stream=None):
> +return_buffer = StringIO()
> +params = "diff --unified=0 origin/master HEAD"
> +RunCmd("git", params, outstream=return_buffer)
> +p = return_buffer.getvalue().strip()
> +patch = p.split("\n")
> +return_buffer.close()
> +
> +ignore_files = []
> +if "IgnoreFiles" in pkgconfig:
> +ignore_files = pkgconfig["IgnoreFiles"]
> +
> +self.ok = True
> +self.startcheck = False
> +self.license = True
> +self.all_file_pass = True
> +count = len(patch)
> +line_index = 0
> +for line in patch:
> +if line.startswith('--- /dev/null'):
> +nextline = patch[line_index + 1]
> +added_file =
> self.Readdedfileformat.search(nextline).group(1)
> +added_file_extension =
> os.path.splitext(added_file)[1]
> 

[edk2-devel] [edk2-platform][PATCH v1 1/1] Platforms/RaspberryPi: Fix RPi4 GICC PMU PPI

2020-07-28 Thread Samer El-Haj-Mahmoud
Arm SBSA specification section ver 6.0, 4.1.5 defines specific PPI
values for certain standard interrupt IDs. The value for
"Performance Monitors Interrupt" needs to be 23.

REF: https://developer.arm.com/documentation/den0029/latest

This partially fixes SBSA test #11 ("Incorrect PPI value") reported in
https://github.com/pftf/RPi4/issues/74

Cc: Leif Lindholm 
Cc: Pete Batard 
Cc: Andrei Warkentin 
Cc: Ard Biesheuvel 
Signed-off-by: Samer El-Haj-Mahmoud 
---
 Platform/RaspberryPi/RPi4/RPi4.dsc | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc 
b/Platform/RaspberryPi/RPi4/RPi4.dsc
index c481c3534263..00683afe96b9 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -433,10 +433,10 @@ [PcdsFixedAtBuild.common]
   gRaspberryPiTokenSpaceGuid.PcdGicInterruptInterfaceHBase|0xFF844000
   gRaspberryPiTokenSpaceGuid.PcdGicInterruptInterfaceVBase|0xFF846000
   gRaspberryPiTokenSpaceGuid.PcdGicGsivId|0x19
-  gRaspberryPiTokenSpaceGuid.PcdGicPmuIrq0|0x30
-  gRaspberryPiTokenSpaceGuid.PcdGicPmuIrq1|0x31
-  gRaspberryPiTokenSpaceGuid.PcdGicPmuIrq2|0x32
-  gRaspberryPiTokenSpaceGuid.PcdGicPmuIrq3|0x33
+  gRaspberryPiTokenSpaceGuid.PcdGicPmuIrq0|23
+  gRaspberryPiTokenSpaceGuid.PcdGicPmuIrq1|23
+  gRaspberryPiTokenSpaceGuid.PcdGicPmuIrq2|23
+  gRaspberryPiTokenSpaceGuid.PcdGicPmuIrq3|23
 
   #
   # Fixed CPU settings.
-- 
2.17.1


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

View/Reply Online (#63415): https://edk2.groups.io/g/devel/message/63415
Mute This Topic: https://groups.io/mt/75853085/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v4] UefiCpuPkg/MtrrLib/UnitTest: Add host based unit test

2020-07-28 Thread Tim Lewis
Sean --

What I have seen done for fuzz testing is to (a) report the seed used to 
initialize the RNG in the log and then (b) provide an option to force the seed 
to that value. Using a static seed might actually be the default for CI runs, 
but stand-alone runs could use a random value. 

Just a thought.

Tim

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Sean
Sent: Tuesday, July 28, 2020 9:38 AM
To: devel@edk2.groups.io; ray...@intel.com
Cc: Michael D Kinney ; Ming Shao 
; Eric Dong ; Laszlo Ersek 
; Sean Brogan ; Bret Barkelew 
; Jiewen Yao 
Subject: Re: [edk2-devel] [PATCH v4] UefiCpuPkg/MtrrLib/UnitTest: Add host 
based unit test

Ray,

I worry that this style of testing will lead to inconsistant results. 
Generating random test cases means that the test cases on any given run 
could find a bug in this code without this code changing.   I think this 
type of testing (fuzz testing like) is great but I think we might want 
to consider this a different test type and treat it differently.

For unit testing the mtrr lib it would make more sense to identify a few 
unique passing and failing tests and statically add those.  If there are 
edge cases or more cases needed to get full code coverage then 
developing those would be great.

Another point is once we start tracking code coverage your random test 
generation will lead to different results which will make it hard to 
track the metrics reliably.

Finally, if edk2 community wants to support fuzz testing (which i think 
is good) we should add details about how to add fuzz testing to edk2 and 
  how to exclude it from PR/CI test runs.

Thoughts?

Thanks
Sean




On 7/28/2020 1:43 AM, Ni, Ray wrote:
> Add host based unit tests for the MtrrLib services.
> The BaseLib services AsmCpuid(), AsmReadMsr64(), and
> AsmWriteMsr64() are hooked and provide simple emulation
> of the CPUID leafs and MSRs required by the MtrrLib to
> run as a host based unit test.
> 
> Test cases are developed for each of the API.
> 
> For the most important APIs MtrrSetMemoryAttributesInMtrrSettings()
> and MtrrSetMemoryAttributeInMtrrSettings(), random inputs are
> generated and fed to the APIs to make sure the implementation is
> good. The test application accepts an optional parameter which
> specifies how many iterations of feeding random inputs to the two
> APIs. The overall number of test cases increases when the iteration
> increases. Default iteration is 10 when no parameter is specified.
> 
> Signed-off-by: Ray Ni 
> Signed-off-by: Michael D Kinney 
> Signed-off-by: Ming Shao 
> Cc: Michael D Kinney 
> Cc: Eric Dong 
> Cc: Laszlo Ersek 
> Cc: Ming Shao 
> Cc: Sean Brogan 
> Cc: Bret Barkelew 
> Cc: Jiewen Yao 
> ---
>   .../MtrrLib/UnitTest/MtrrLibUnitTest.c| 1139 +
>   .../MtrrLib/UnitTest/MtrrLibUnitTest.h|  182 +++
>   .../MtrrLib/UnitTest/MtrrLibUnitTestHost.inf  |   39 +
>   UefiCpuPkg/Library/MtrrLib/UnitTest/Support.c |  923 +
>   UefiCpuPkg/Test/UefiCpuPkgHostTest.dsc|   31 +
>   UefiCpuPkg/UefiCpuPkg.ci.yaml |   12 +-
>   6 files changed, 2325 insertions(+), 1 deletion(-)
>   create mode 100644 UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTest.c
>   create mode 100644 UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTest.h
>   create mode 100644 
> UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTestHost.inf
>   create mode 100644 UefiCpuPkg/Library/MtrrLib/UnitTest/Support.c
>   create mode 100644 UefiCpuPkg/Test/UefiCpuPkgHostTest.dsc
> 
> diff --git a/UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTest.c 
> b/UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTest.c
> new file mode 100644
> index 00..123e1c741a
> --- /dev/null
> +++ b/UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTest.c
> @@ -0,0 +1,1139 @@
> +/** @file
> 
> +  Unit tests of the MtrrLib instance of the MtrrLib class
> 
> +
> 
> +  Copyright (c) 2020, Intel Corporation. All rights reserved.
> 
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> +
> 
> +**/
> 
> +
> 
> +#include "MtrrLibUnitTest.h"
> 
> +
> 
> +STATIC CONST MTRR_LIB_SYSTEM_PARAMETER mDefaultSystemParameter = {
> 
> +  42, TRUE, TRUE, CacheUncacheable, 12
> 
> +};
> 
> +
> 
> +STATIC MTRR_LIB_SYSTEM_PARAMETER mSystemParameters[] = {
> 
> +  { 38, TRUE, TRUE, CacheUncacheable,12 },
> 
> +  { 38, TRUE, TRUE, CacheWriteBack,  12 },
> 
> +  { 38, TRUE, TRUE, CacheWriteThrough,   12 },
> 
> +  { 38, TRUE, TRUE, CacheWriteProtected, 12 },
> 
> +  { 38, TRUE, TRUE, CacheWriteCombining, 12 },
> 
> +
> 
> +  { 42, TRUE, TRUE, CacheUncacheable,12 },
> 
> +  { 42, TRUE, TRUE, CacheWriteBack,  12 },
> 
> +  { 42, TRUE, TRUE, CacheWriteThrough,   12 },
> 
> +  { 42, TRUE, TRUE, CacheWriteProtected, 12 },
> 
> +  { 42, TRUE, TRUE, CacheWriteCombining, 12 },
> 
> +
> 
> +  { 48, TRUE, TRUE, CacheUncacheable,12 },
> 
> +  { 48, TRUE, TRUE, CacheWriteBack,  12 },
> 
> +  { 48, TRUE, TRUE, CacheWriteThrough,   12 },
> 
> +  { 48, TRU

Re: [edk2-devel] [PATCH v4] UefiCpuPkg/MtrrLib/UnitTest: Add host based unit test

2020-07-28 Thread Sean

Ray,

I worry that this style of testing will lead to inconsistant results. 
Generating random test cases means that the test cases on any given run 
could find a bug in this code without this code changing.   I think this 
type of testing (fuzz testing like) is great but I think we might want 
to consider this a different test type and treat it differently.


For unit testing the mtrr lib it would make more sense to identify a few 
unique passing and failing tests and statically add those.  If there are 
edge cases or more cases needed to get full code coverage then 
developing those would be great.


Another point is once we start tracking code coverage your random test 
generation will lead to different results which will make it hard to 
track the metrics reliably.


Finally, if edk2 community wants to support fuzz testing (which i think 
is good) we should add details about how to add fuzz testing to edk2 and 
 how to exclude it from PR/CI test runs.


Thoughts?

Thanks
Sean




On 7/28/2020 1:43 AM, Ni, Ray wrote:

Add host based unit tests for the MtrrLib services.
The BaseLib services AsmCpuid(), AsmReadMsr64(), and
AsmWriteMsr64() are hooked and provide simple emulation
of the CPUID leafs and MSRs required by the MtrrLib to
run as a host based unit test.

Test cases are developed for each of the API.

For the most important APIs MtrrSetMemoryAttributesInMtrrSettings()
and MtrrSetMemoryAttributeInMtrrSettings(), random inputs are
generated and fed to the APIs to make sure the implementation is
good. The test application accepts an optional parameter which
specifies how many iterations of feeding random inputs to the two
APIs. The overall number of test cases increases when the iteration
increases. Default iteration is 10 when no parameter is specified.

Signed-off-by: Ray Ni 
Signed-off-by: Michael D Kinney 
Signed-off-by: Ming Shao 
Cc: Michael D Kinney 
Cc: Eric Dong 
Cc: Laszlo Ersek 
Cc: Ming Shao 
Cc: Sean Brogan 
Cc: Bret Barkelew 
Cc: Jiewen Yao 
---
  .../MtrrLib/UnitTest/MtrrLibUnitTest.c| 1139 +
  .../MtrrLib/UnitTest/MtrrLibUnitTest.h|  182 +++
  .../MtrrLib/UnitTest/MtrrLibUnitTestHost.inf  |   39 +
  UefiCpuPkg/Library/MtrrLib/UnitTest/Support.c |  923 +
  UefiCpuPkg/Test/UefiCpuPkgHostTest.dsc|   31 +
  UefiCpuPkg/UefiCpuPkg.ci.yaml |   12 +-
  6 files changed, 2325 insertions(+), 1 deletion(-)
  create mode 100644 UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTest.c
  create mode 100644 UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTest.h
  create mode 100644 UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTestHost.inf
  create mode 100644 UefiCpuPkg/Library/MtrrLib/UnitTest/Support.c
  create mode 100644 UefiCpuPkg/Test/UefiCpuPkgHostTest.dsc

diff --git a/UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTest.c 
b/UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTest.c
new file mode 100644
index 00..123e1c741a
--- /dev/null
+++ b/UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTest.c
@@ -0,0 +1,1139 @@
+/** @file

+  Unit tests of the MtrrLib instance of the MtrrLib class

+

+  Copyright (c) 2020, Intel Corporation. All rights reserved.

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

+

+**/

+

+#include "MtrrLibUnitTest.h"

+

+STATIC CONST MTRR_LIB_SYSTEM_PARAMETER mDefaultSystemParameter = {

+  42, TRUE, TRUE, CacheUncacheable, 12

+};

+

+STATIC MTRR_LIB_SYSTEM_PARAMETER mSystemParameters[] = {

+  { 38, TRUE, TRUE, CacheUncacheable,12 },

+  { 38, TRUE, TRUE, CacheWriteBack,  12 },

+  { 38, TRUE, TRUE, CacheWriteThrough,   12 },

+  { 38, TRUE, TRUE, CacheWriteProtected, 12 },

+  { 38, TRUE, TRUE, CacheWriteCombining, 12 },

+

+  { 42, TRUE, TRUE, CacheUncacheable,12 },

+  { 42, TRUE, TRUE, CacheWriteBack,  12 },

+  { 42, TRUE, TRUE, CacheWriteThrough,   12 },

+  { 42, TRUE, TRUE, CacheWriteProtected, 12 },

+  { 42, TRUE, TRUE, CacheWriteCombining, 12 },

+

+  { 48, TRUE, TRUE, CacheUncacheable,12 },

+  { 48, TRUE, TRUE, CacheWriteBack,  12 },

+  { 48, TRUE, TRUE, CacheWriteThrough,   12 },

+  { 48, TRUE, TRUE, CacheWriteProtected, 12 },

+  { 48, TRUE, TRUE, CacheWriteCombining, 12 },

+};

+

+UINT32mFixedMtrrsIndex[] = {

+  MSR_IA32_MTRR_FIX64K_0,

+  MSR_IA32_MTRR_FIX16K_8,

+  MSR_IA32_MTRR_FIX16K_A,

+  MSR_IA32_MTRR_FIX4K_C,

+  MSR_IA32_MTRR_FIX4K_C8000,

+  MSR_IA32_MTRR_FIX4K_D,

+  MSR_IA32_MTRR_FIX4K_D8000,

+  MSR_IA32_MTRR_FIX4K_E,

+  MSR_IA32_MTRR_FIX4K_E8000,

+  MSR_IA32_MTRR_FIX4K_F,

+  MSR_IA32_MTRR_FIX4K_F8000

+};

+STATIC_ASSERT (

+  (ARRAY_SIZE (mFixedMtrrsIndex) == MTRR_NUMBER_OF_FIXED_MTRR),

+  "gFixedMtrrIndex does NOT contain all the fixed MTRRs!"

+  );

+

+//

+// Context structure to be used for most of the test cases.

+//

+typedef struct {

+  CONST MTRR_LIB_SYSTEM_PARAMETER *SystemParameter;

+} MTRR_LIB_TEST_CONTEXT;

+

+//

+// Context structure to be used for GetFirmwareVaria

Re: [edk2-devel] [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT instruction

2020-07-28 Thread Laszlo Ersek
On 07/28/20 16:13, Tom Lendacky wrote:

> IIUC, create a VmgExit.c file in MdePkg/Library/BaseLib/Ia32/ that
> doesn't actually encode the VMGEXIT instruction, just calls
> CpuBreakpoint(), e.g.:
>
>   VOID
>   EFIAPI
>   AsmVmgExit (
> VOID
> )
>   {
> CpuBreakpoint();
>   }
>

Yes -- either that, or even just open-code (copy) what we have in

  MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.nasm

now:

global ASM_PFX(AsmVmgExit)
ASM_PFX(AsmVmgExit):
;
; VMGEXIT makes no sense on IA32, and NASM versions before 2.12 cannot
; translate the 64-bit "rep vmmcall" instruction into elf32 format. So
; provide a stub implementation that is identical to CpuBreakpoint(). In
; practice, AsmVmgExit() should never be called on IA32.
;
int  3
ret

Because this assembly-language implementation might be more "true" to
the name "AsmVmgExit".

Liming, would you be OK with this approach? In other words, the set of
files changed/introduced in this patch would not change, just the
implementation of IA32 AsmVmgExit().

> The other alternative is to use a DB-encoded instruction, though I
> know that isn't the most popular approach.

Right, I've been quite on a quest to eliminate DBs that encode
instructions.

> The BITS 64 method to generate the instruction bytes is also used in
> OvmfPkg/ResetVector/Ia32/PageTables64.asm, but that file is only
> included when ARCH_X64 is defined, so there shouldn't be an issue
> there, plus the nasm file format is bin (-f bin).

I confirm that; the following commands all work on RHEL7, with this
series applied:

$ build -b NOOPT -t GCC48 -m OvmfPkg/ResetVector/ResetVector.inf -a IA32
-p OvmfPkg/OvmfPkgIa32.dsc
$ build -b NOOPT -t GCC48 -m OvmfPkg/ResetVector/ResetVector.inf -a IA32 -a X64 
-p OvmfPkg/OvmfPkgIa32X64.dsc
$ build -b NOOPT -t GCC48 -m OvmfPkg/ResetVector/ResetVector.inf -a X64 
-p OvmfPkg/OvmfPkgX64.dsc

Thanks!
Laszlo


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

View/Reply Online (#63408): https://edk2.groups.io/g/devel/message/63408
Mute This Topic: https://groups.io/mt/75824947/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] TianoCore Bug Triage - APAC / NAMO - Tue, 07/28/2020 6:30pm-7:30pm #cal-reminder

2020-07-28 Thread Liming Gao

The following Bug will be reviewed this week. Welcome the reporter join the 
meeting to introduce the more background and detail.

Thanks
Liming
ID
Product
Comp
Assignee
Status
Summary
Reporter
Changed▼
2865
EDK2 Tes
UEFI-SCT
edhaya.chand...@arm.com
UNCO
uefi-sct/SctPkg: EFI_SUCCESS not accepted for 
QueryVariableInfo
xypron.g...@gmx.de
Mon 13:57
2233
EDK2 Pla
MinPlatf
rangasai.v.chaga...@intel.com
UNCO
Test Point infrastructure should be broken into more granular categories of 
tests
michael.a.kuba...@intel.com
Sun 10:15
2863
EDK2
Test
unassig...@tianocore.org
UNCO
Make MtrrLib unit test failure easily 
reproducable
ray...@intel.com
Fri 13:13
2853
EDK2
Code
unassig...@tianocore.org
UNCO
UT_LOG_xxx() messages are 
truncated
ray...@intel.com
Fri 02:26
2861
EDK2
Code
unassig...@tianocore.org
UNCO
HiiDatabaseDxe, ConfigRouting.c

Re: [edk2-devel] [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT instruction

2020-07-28 Thread Lendacky, Thomas
On 7/28/20 7:04 AM, Laszlo Ersek wrote:
> On 07/28/20 09:39, Gao, Liming wrote:
>> This error is reported from nasm compiler. My nasm compiler version is
>> 2.11.08. It may be a little old. 2.12 should be fine.

I have 2.13.02.

>>
>> This change also requires to update
>> edk2\BaseTools\Conf\tools_def.template and mention nasm compiler
>> version.
> 
> "tools_def.template" says:
> 
>   NASM 2.10 or later for use with the GCC toolchain family
> 
> Bumping the NASM requirement from 2.10 to 2.12 will rule out:
> 
> - Debian "jessie" (oldoldstable),
> - Ubuntu "xenial" (16.04 LTS),
> - and RHEL7,
> 
> as build hosts.
> 
> Debian "jessie" is no longer supported (LTS ended in June 2020), but
> Ubuntu "xenial" and RHEL7 are still supported by their vendors.
> 
> I seem to recall that it was me to recommend "BITS 64" in front of "rep
> vmmcall" in the IA32 NASM source file:
> 
>   
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F48292&data=02%7C01%7Cthomas.lendacky%40amd.com%7Ca8b0f66286b745e78b1d08d832ee6f95%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637315346944536242&sdata=4DBhHd2WYgoEx%2F76YaNRalJlSvd0rAIWxrN1qOFR9Tw%3D&reserved=0
>   
> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmid.mail-archive.com%2Fe8a8e21e-4045-1b2b-f959-13fbe00132d9%40redhat.com&data=02%7C01%7Cthomas.lendacky%40amd.com%7Ca8b0f66286b745e78b1d08d832ee6f95%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637315346944536242&sdata=AV5lDCYWOm2cTOzlaI7OwMjcK9TknVGBNFAkNJhahuM%3D&reserved=0
> 
> I don't understand why my testing worked back then, and now it doesn't.
> (IOW, I can also reproduce the error that Liming reported!) It's likely
> because I didn't specify the elf32 output format back then.
> 
> Indeed: the following command fails:
> 
>> "nasm" \
>>   -I"$WORKSPACE"/MdePkg/Library/BaseLib/Ia32/ \
>>   -I"$WORKSPACE"/MdePkg/Library/BaseLib/Ia32/ \
>>   -I"$WORKSPACE"/MdePkg/Library/BaseLib/ \
>>   
>> -I"$WORKSPACE"/Build/OvmfIa32/NOOPT_GCC48/IA32/MdePkg/Library/BaseLib/BaseLib/DEBUG/
>>  \
>>   -I"$WORKSPACE"/MdePkg/ \
>>   -I"$WORKSPACE"/MdePkg/Include/ \
>>   -I"$WORKSPACE"/MdePkg/Test/UnitTest/Include/ \
>>   -I"$WORKSPACE"/MdePkg/Include/Ia32/ \
>>   -f elf32 \
>>   -o 
>> "$WORKSPACE"/Build/OvmfIa32/NOOPT_GCC48/IA32/MdePkg/Library/BaseLib/BaseLib/OUTPUT/Ia32/VmgExit.obj
>>  \
>>   
>> "$WORKSPACE"/Build/OvmfIa32/NOOPT_GCC48/IA32/MdePkg/Library/BaseLib/BaseLib/OUTPUT/Ia32/VmgExit.iii
> 
> but if I remove "-f elf32", it completes fine. :(

With version 2.13.02, I can successfully build with '-f elf32', so it
seems to be version related.

> 
> The AMD manual says about VMGEXIT:
> 
>> The VMGEXIT opcode is only valid within a guest when run with SEV-ES
>> mode active. If the guest is not run with SEV-ES mode active, the
>> VMGEXIT opcode will be treated as a VMMCALL opcode and will behave
>> exactly like a VMMCALL.
> 
> VMGEXIT is a SEV-ES-only form of guest-host communication. SEV-ES mode
> depends on SEV. A SEV guest can only interact with the host (= decrypt
> its pages for the host to access) if the guest is executing in long
> mode.
> 
> So does it even make sense to *attempt* implementing AsmVmgExit()
> "correctly" for IA32?

We are only enabling SEV-ES support for X64, so this is a valid point. It
would be very hard to enable the Ia32X64 combination (because of how long
it takes to get into long mode) and impossible for the Ia32 alone.

> 
> I don't want to complicate the build dependencies in this series
> further, so I won't suggest that we simply *not* implement AsmVmgExit()
> for IA32 at all. (Purely from a BaseLib perspective, this would be a
> valid approach, but then call sites would have to be *build-time*
> restricted to X64 too. The call sites *are* already restricted to X64,
> AIUI, but that happens at runtime (= dynamic checks), not at build
> time.)
> 
> So here's what I suggest: implement AsmVmgExit() for IA32 in the C
> language, namely as a call to CpuBreakpoint().

IIUC, create a VmgExit.c file in MdePkg/Library/BaseLib/Ia32/ that doesn't
actually encode the VMGEXIT instruction, just calls CpuBreakpoint(), e.g.:

  VOID
  EFIAPI
  AsmVmgExit (
VOID
)
  {
CpuBreakpoint();
  }


The other alternative is to use a DB-encoded instruction, though I know
that isn't the most popular approach.

The BITS 64 method to generate the instruction bytes is also used in
OvmfPkg/ResetVector/Ia32/PageTables64.asm, but that file is only included
when ARCH_X64 is defined, so there shouldn't be an issue there, plus the
nasm file format is bin (-f bin).

Thanks,
Tom

> 
> I wouldn't like to tighten the NASM version requirement for *all* of
> edk2, for the sake of building a BaseLib primitive for IA32 that we
> never *call* on IA32.
> 
> Thanks,
> Laszlo
> 
>>
>> Thanks
>> Liming
>> -Original Message-
>> From: Tom Lendacky 
>> Sent: 2020t728å 12:08
>> To: Gao, Liming ; devel@edk2.groups.io
>> Cc: Brijesh Singh ; Ard Biesheuvel 
>> ; Don

Re: [edk2-devel] [PATCH v12 00/46] SEV-ES guest support

2020-07-28 Thread Laszlo Ersek
On 07/28/20 14:52, Tom Lendacky wrote:
> On 7/28/20 5:07 AM, Laszlo Ersek wrote:
>> On 07/27/20 19:49, Tom Lendacky wrote:
>>
>>> Yes, I always get an error when I reach 40 emails:
>>>
>>> Reported error: 550 5.0.350 Remote server returned an error -> 500 We have
>>> received more than 40 messages in 30 minutes from you. To guard against
>>> autoresponder mail loops, we must reject additional messages from you
>>> temporarily. Please try again later.
>>>
>>> So I usually have to wait a few hours and resend the last ones so they
>>> make it to groups.io.
>>
>> Sigh.
>>
>> Phil found one way to deal with this non-interactively: see the
>> "sendemail.smtpBatchSize" and "sendemail.smtpReloginDelay" git-config
>> options. From git-send-email(1):
>>
>>--batch-size=
>>Some email servers (e.g. smtp.163.com) limit the number
>>emails to be sent per session (connection) and this will
>>lead to a failure when sending many messages. With this
>>option, send-email will disconnect after sending $
>>messages and wait for a few seconds (see --relogin-delay)
>>and reconnect, to work around such a limit. You may want to
>>use some form of credential helper to avoid having to
>>retype your password every time this happens. Defaults to
>>the sendemail.smtpBatchSize configuration variable.
>>
>>--relogin-delay=
>>Waiting $ seconds before reconnecting to SMTP server.
>>Used together with --batch-size option. Defaults to the
>>sendemail.smtpReloginDelay configuration variable.
>>
>> That way the "second batch" should need no manual work, hopefully.
> 
> I've tried with and without batch-size and relogin-delay. My initial
> submissions were without these parameters and I hit the 40 email limit. I
> later tried a batch-size of 1 with a relogin-delay of 1 and that still
> results in the 40 email limit. I haven't tried any combinations of a
> larger batch-size.
> 
> I don't think groups.io has a per-session limit, I think it is actually
> tracking individual emails over a period of time, so I don't think these
> parameters do anything for that.

Thank you for explaining.

I've now filed a request with  to raise the limit (on
this list only) to 200 messages per 30 minutes. That should allow people
to post large series *and* to respond to other (unrelated)
email threads as well, in the normal course of their workday.

Thanks
Laszlo


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

View/Reply Online (#63405): https://edk2.groups.io/g/devel/message/63405
Mute This Topic: https://groups.io/mt/75824926/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT instruction

2020-07-28 Thread Liming Gao
Laszlo:

> -Original Message-
> From: Laszlo Ersek 
> Sent: Tuesday, July 28, 2020 8:04 PM
> To: Gao, Liming ; Tom Lendacky 
> ; devel@edk2.groups.io
> Cc: Brijesh Singh ; Ard Biesheuvel 
> ; Dong, Eric ; Justen,
> Jordan L ; Kinney, Michael D 
> ; Ni, Ray 
> Subject: Re: [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT 
> instruction
> 
> On 07/28/20 09:39, Gao, Liming wrote:
> > This error is reported from nasm compiler. My nasm compiler version is
> > 2.11.08. It may be a little old. 2.12 should be fine.
> >
> > This change also requires to update
> > edk2\BaseTools\Conf\tools_def.template and mention nasm compiler
> > version.
> 
> "tools_def.template" says:
> 
>   NASM 2.10 or later for use with the GCC toolchain family
> 
> Bumping the NASM requirement from 2.10 to 2.12 will rule out:
> 
> - Debian "jessie" (oldoldstable),
> - Ubuntu "xenial" (16.04 LTS),
> - and RHEL7,
> 
> as build hosts.
> 
> Debian "jessie" is no longer supported (LTS ended in June 2020), but
> Ubuntu "xenial" and RHEL7 are still supported by their vendors.
> 
Yes. I also realize nasm version impact. It may also impact Mac OS. 
We should try to avoid new requirement to update nasm version. 

Thanks
Liming
> I seem to recall that it was me to recommend "BITS 64" in front of "rep
> vmmcall" in the IA32 NASM source file:
> 
>   https://edk2.groups.io/g/devel/message/48292
>   http://mid.mail-archive.com/e8a8e21e-4045-1b2b-f959-13fbe00132d9@redhat.com
> 
> I don't understand why my testing worked back then, and now it doesn't.
> (IOW, I can also reproduce the error that Liming reported!) It's likely
> because I didn't specify the elf32 output format back then.
> 
> Indeed: the following command fails:
> 
> > "nasm" \
> >   -I"$WORKSPACE"/MdePkg/Library/BaseLib/Ia32/ \
> >   -I"$WORKSPACE"/MdePkg/Library/BaseLib/Ia32/ \
> >   -I"$WORKSPACE"/MdePkg/Library/BaseLib/ \
> >   
> > -I"$WORKSPACE"/Build/OvmfIa32/NOOPT_GCC48/IA32/MdePkg/Library/BaseLib/BaseLib/DEBUG/
> >  \
> >   -I"$WORKSPACE"/MdePkg/ \
> >   -I"$WORKSPACE"/MdePkg/Include/ \
> >   -I"$WORKSPACE"/MdePkg/Test/UnitTest/Include/ \
> >   -I"$WORKSPACE"/MdePkg/Include/Ia32/ \
> >   -f elf32 \
> >   -o 
> > "$WORKSPACE"/Build/OvmfIa32/NOOPT_GCC48/IA32/MdePkg/Library/BaseLib/BaseLib/OUTPUT/Ia32/VmgExit.obj
> >  \
> >   
> > "$WORKSPACE"/Build/OvmfIa32/NOOPT_GCC48/IA32/MdePkg/Library/BaseLib/BaseLib/OUTPUT/Ia32/VmgExit.iii
> 
> but if I remove "-f elf32", it completes fine. :(
> 
> The AMD manual says about VMGEXIT:
> 
> > The VMGEXIT opcode is only valid within a guest when run with SEV-ES
> > mode active. If the guest is not run with SEV-ES mode active, the
> > VMGEXIT opcode will be treated as a VMMCALL opcode and will behave
> > exactly like a VMMCALL.
> 
> VMGEXIT is a SEV-ES-only form of guest-host communication. SEV-ES mode
> depends on SEV. A SEV guest can only interact with the host (= decrypt
> its pages for the host to access) if the guest is executing in long
> mode.
> 
> So does it even make sense to *attempt* implementing AsmVmgExit()
> "correctly" for IA32?
> 
> I don't want to complicate the build dependencies in this series
> further, so I won't suggest that we simply *not* implement AsmVmgExit()
> for IA32 at all. (Purely from a BaseLib perspective, this would be a
> valid approach, but then call sites would have to be *build-time*
> restricted to X64 too. The call sites *are* already restricted to X64,
> AIUI, but that happens at runtime (= dynamic checks), not at build
> time.)
> 
> So here's what I suggest: implement AsmVmgExit() for IA32 in the C
> language, namely as a call to CpuBreakpoint().
> 
> I wouldn't like to tighten the NASM version requirement for *all* of
> edk2, for the sake of building a BaseLib primitive for IA32 that we
> never *call* on IA32.
> 
> Thanks,
> Laszlo
> 
> >
> > Thanks
> > Liming
> > -Original Message-
> > From: Tom Lendacky 
> > Sent: 2020t728å 12:08
> > To: Gao, Liming ; devel@edk2.groups.io
> > Cc: Brijesh Singh ; Ard Biesheuvel 
> > ; Dong, Eric ; Justen,
> Jordan L ; Laszlo Ersek ; 
> Kinney, Michael D ; Ni, Ray
> 
> > Subject: Re: [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT 
> > instruction
> >
> > On 7/27/20 8:34 PM, Gao, Liming wrote:
> >> Tom:
> >
> > Hi Liming,
> >
> >>I meet with GCC failure on this patch. Can you help check it? If nasm 
> >> doesn't support the vmmcall instruction in 32-bit mode, you
> have to use inline assembly to support it.
> >
> > What version of GCC are you using. I was able to successfully build the
> > Ia32 version with my GCC level. The Ia32 version uses a trick to do switch 
> > to 64-bit just to encode the instruction. Looks like that
> doesn't work with your version of GCC.
> >
> > I can probably switch to defining the instruction as bytes. Let me look 
> > into that and possibly send you a patch to test.
> >
> > Thanks,
> > Tom
> >
> >>
> >> Edk2/Build/IntelFsp2Pkg/DEBUG_GCC5/IA32/MdePkg/Library/BaseLib/BaseLib
> >> /OUTPUT/Ia32/VmgExit.ii

Re: [edk2-devel] [PATCH v12 00/46] SEV-ES guest support

2020-07-28 Thread Lendacky, Thomas
On 7/28/20 5:07 AM, Laszlo Ersek wrote:
> On 07/27/20 19:49, Tom Lendacky wrote:
> 
>> Yes, I always get an error when I reach 40 emails:
>>
>> Reported error: 550 5.0.350 Remote server returned an error -> 500 We have
>> received more than 40 messages in 30 minutes from you. To guard against
>> autoresponder mail loops, we must reject additional messages from you
>> temporarily. Please try again later.
>>
>> So I usually have to wait a few hours and resend the last ones so they
>> make it to groups.io.
> 
> Sigh.
> 
> Phil found one way to deal with this non-interactively: see the
> "sendemail.smtpBatchSize" and "sendemail.smtpReloginDelay" git-config
> options. From git-send-email(1):
> 
>--batch-size=
>Some email servers (e.g. smtp.163.com) limit the number
>emails to be sent per session (connection) and this will
>lead to a failure when sending many messages. With this
>option, send-email will disconnect after sending $
>messages and wait for a few seconds (see --relogin-delay)
>and reconnect, to work around such a limit. You may want to
>use some form of credential helper to avoid having to
>retype your password every time this happens. Defaults to
>the sendemail.smtpBatchSize configuration variable.
> 
>--relogin-delay=
>Waiting $ seconds before reconnecting to SMTP server.
>Used together with --batch-size option. Defaults to the
>sendemail.smtpReloginDelay configuration variable.
> 
> That way the "second batch" should need no manual work, hopefully.

I've tried with and without batch-size and relogin-delay. My initial
submissions were without these parameters and I hit the 40 email limit. I
later tried a batch-size of 1 with a relogin-delay of 1 and that still
results in the 40 email limit. I haven't tried any combinations of a
larger batch-size.

I don't think groups.io has a per-session limit, I think it is actually
tracking individual emails over a period of time, so I don't think these
parameters do anything for that.

Thanks,
Tom

> 
> Thanks
> Laszlo
> 

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

View/Reply Online (#63403): https://edk2.groups.io/g/devel/message/63403
Mute This Topic: https://groups.io/mt/75824926/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v4] UefiCpuPkg/MtrrLib/UnitTest: Add host based unit test

2020-07-28 Thread Laszlo Ersek
Hi Ray,

On 07/28/20 10:43, Ni, Ray wrote:
> Add host based unit tests for the MtrrLib services.
> The BaseLib services AsmCpuid(), AsmReadMsr64(), and
> AsmWriteMsr64() are hooked and provide simple emulation
> of the CPUID leafs and MSRs required by the MtrrLib to
> run as a host based unit test.
> 
> Test cases are developed for each of the API.
> 
> For the most important APIs MtrrSetMemoryAttributesInMtrrSettings()
> and MtrrSetMemoryAttributeInMtrrSettings(), random inputs are
> generated and fed to the APIs to make sure the implementation is
> good. The test application accepts an optional parameter which
> specifies how many iterations of feeding random inputs to the two
> APIs. The overall number of test cases increases when the iteration
> increases. Default iteration is 10 when no parameter is specified.
> 
> Signed-off-by: Ray Ni 
> Signed-off-by: Michael D Kinney 
> Signed-off-by: Ming Shao 
> Cc: Michael D Kinney 
> Cc: Eric Dong 
> Cc: Laszlo Ersek 
> Cc: Ming Shao 
> Cc: Sean Brogan 
> Cc: Bret Barkelew 
> Cc: Jiewen Yao 
> ---
>  .../MtrrLib/UnitTest/MtrrLibUnitTest.c| 1139 +
>  .../MtrrLib/UnitTest/MtrrLibUnitTest.h|  182 +++
>  .../MtrrLib/UnitTest/MtrrLibUnitTestHost.inf  |   39 +
>  UefiCpuPkg/Library/MtrrLib/UnitTest/Support.c |  923 +
>  UefiCpuPkg/Test/UefiCpuPkgHostTest.dsc|   31 +
>  UefiCpuPkg/UefiCpuPkg.ci.yaml |   12 +-
>  6 files changed, 2325 insertions(+), 1 deletion(-)
>  create mode 100644 UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTest.c
>  create mode 100644 UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTest.h
>  create mode 100644 
> UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTestHost.inf
>  create mode 100644 UefiCpuPkg/Library/MtrrLib/UnitTest/Support.c
>  create mode 100644 UefiCpuPkg/Test/UefiCpuPkgHostTest.dsc

I don't have comments on this patch; I've been giving my Acked-by to
confirm that I'm OK with this particular set of files being introduced /
modified under UefiCpuPkg. So I'll defer on further versions to Eric.

For this version, this one last time:

Acked-by: Laszlo Ersek 

Thanks
Laszlo


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

View/Reply Online (#63402): https://edk2.groups.io/g/devel/message/63402
Mute This Topic: https://groups.io/mt/75840226/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT instruction

2020-07-28 Thread Laszlo Ersek
On 07/28/20 09:39, Gao, Liming wrote:
> This error is reported from nasm compiler. My nasm compiler version is
> 2.11.08. It may be a little old. 2.12 should be fine.
>
> This change also requires to update
> edk2\BaseTools\Conf\tools_def.template and mention nasm compiler
> version.

"tools_def.template" says:

  NASM 2.10 or later for use with the GCC toolchain family

Bumping the NASM requirement from 2.10 to 2.12 will rule out:

- Debian "jessie" (oldoldstable),
- Ubuntu "xenial" (16.04 LTS),
- and RHEL7,

as build hosts.

Debian "jessie" is no longer supported (LTS ended in June 2020), but
Ubuntu "xenial" and RHEL7 are still supported by their vendors.

I seem to recall that it was me to recommend "BITS 64" in front of "rep
vmmcall" in the IA32 NASM source file:

  https://edk2.groups.io/g/devel/message/48292
  http://mid.mail-archive.com/e8a8e21e-4045-1b2b-f959-13fbe00132d9@redhat.com

I don't understand why my testing worked back then, and now it doesn't.
(IOW, I can also reproduce the error that Liming reported!) It's likely
because I didn't specify the elf32 output format back then.

Indeed: the following command fails:

> "nasm" \
>   -I"$WORKSPACE"/MdePkg/Library/BaseLib/Ia32/ \
>   -I"$WORKSPACE"/MdePkg/Library/BaseLib/Ia32/ \
>   -I"$WORKSPACE"/MdePkg/Library/BaseLib/ \
>   
> -I"$WORKSPACE"/Build/OvmfIa32/NOOPT_GCC48/IA32/MdePkg/Library/BaseLib/BaseLib/DEBUG/
>  \
>   -I"$WORKSPACE"/MdePkg/ \
>   -I"$WORKSPACE"/MdePkg/Include/ \
>   -I"$WORKSPACE"/MdePkg/Test/UnitTest/Include/ \
>   -I"$WORKSPACE"/MdePkg/Include/Ia32/ \
>   -f elf32 \
>   -o 
> "$WORKSPACE"/Build/OvmfIa32/NOOPT_GCC48/IA32/MdePkg/Library/BaseLib/BaseLib/OUTPUT/Ia32/VmgExit.obj
>  \
>   
> "$WORKSPACE"/Build/OvmfIa32/NOOPT_GCC48/IA32/MdePkg/Library/BaseLib/BaseLib/OUTPUT/Ia32/VmgExit.iii

but if I remove "-f elf32", it completes fine. :(

The AMD manual says about VMGEXIT:

> The VMGEXIT opcode is only valid within a guest when run with SEV-ES
> mode active. If the guest is not run with SEV-ES mode active, the
> VMGEXIT opcode will be treated as a VMMCALL opcode and will behave
> exactly like a VMMCALL.

VMGEXIT is a SEV-ES-only form of guest-host communication. SEV-ES mode
depends on SEV. A SEV guest can only interact with the host (= decrypt
its pages for the host to access) if the guest is executing in long
mode.

So does it even make sense to *attempt* implementing AsmVmgExit()
"correctly" for IA32?

I don't want to complicate the build dependencies in this series
further, so I won't suggest that we simply *not* implement AsmVmgExit()
for IA32 at all. (Purely from a BaseLib perspective, this would be a
valid approach, but then call sites would have to be *build-time*
restricted to X64 too. The call sites *are* already restricted to X64,
AIUI, but that happens at runtime (= dynamic checks), not at build
time.)

So here's what I suggest: implement AsmVmgExit() for IA32 in the C
language, namely as a call to CpuBreakpoint().

I wouldn't like to tighten the NASM version requirement for *all* of
edk2, for the sake of building a BaseLib primitive for IA32 that we
never *call* on IA32.

Thanks,
Laszlo

>
> Thanks
> Liming
> -Original Message-
> From: Tom Lendacky 
> Sent: 2020t728å 12:08
> To: Gao, Liming ; devel@edk2.groups.io
> Cc: Brijesh Singh ; Ard Biesheuvel 
> ; Dong, Eric ; Justen, Jordan L 
> ; Laszlo Ersek ; Kinney, 
> Michael D ; Ni, Ray 
> Subject: Re: [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT 
> instruction
>
> On 7/27/20 8:34 PM, Gao, Liming wrote:
>> Tom:
>
> Hi Liming,
>
>>I meet with GCC failure on this patch. Can you help check it? If nasm 
>> doesn't support the vmmcall instruction in 32-bit mode, you have to use 
>> inline assembly to support it.
>
> What version of GCC are you using. I was able to successfully build the
> Ia32 version with my GCC level. The Ia32 version uses a trick to do switch to 
> 64-bit just to encode the instruction. Looks like that doesn't work with your 
> version of GCC.
>
> I can probably switch to defining the instruction as bytes. Let me look into 
> that and possibly send you a patch to test.
>
> Thanks,
> Tom
>
>>
>> Edk2/Build/IntelFsp2Pkg/DEBUG_GCC5/IA32/MdePkg/Library/BaseLib/BaseLib
>> /OUTPUT/Ia32/VmgExit.iii:33: error: elf32 output format does not
>> support 64-bit code
>> GNUmakefile:741: recipe for target
>>
>> Thanks
>> Liming
>> -Original Message-
>> From: Tom Lendacky 
>> Sent: 2020t727å 23:26
>> To: devel@edk2.groups.io
>> Cc: Brijesh Singh ; Ard Biesheuvel
>> ; Dong, Eric ; Justen,
>> Jordan L ; Laszlo Ersek
>> ; Gao, Liming ; Kinney,
>> Michael D ; Ni, Ray 
>> Subject: [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT
>> instruction
>>
>> From: Tom Lendacky 
>>
>> BZ:
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugz
>> illa.tianocore.org%2Fshow_bug.cgi%3Fid%3D2198&data=02%7C01%7Cthoma
>> s.lendacky%40amd.com%7C77c8250cd9e14f2929a008d832965726%7C3dd8961fe488
>> 4e608e11a82d994e183d%7C0%7C0

Re: [edk2-devel] [PATCH v5 4/5] BaseTools: Compile AML bytecode arrays into .obj file

2020-07-28 Thread Leif Lindholm
Yes,

Do a git clean -fx in your edk2 directory, and delete your Build
directory completely.

Regards,

Leif

On Tue, Jul 28, 2020 at 09:04:44 +, Pierre Gondois wrote:
> Hello Masahisa,
> 
> Maybe you will have to delete/re-generate the build_rule.txt and
> tools_def.txt files that you have. This patch makes the build system
> generate a .amli file for each .asl file. If there is no Emmc.amli
> in your Build directory, this might be the reason.
> 
> Regards,
> Pierre
> 
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Masahisa 
> Kojima via groups.io
> Sent: Tuesday, July 28, 2020 2:38 AM
> To: Leif Lindholm 
> Cc: devel@edk2.groups.io; Gao, Liming ; Pierre Gondois 
> ; Sami Mujawar ; Tomas Pilar 
> ; Feng, Bob C ; Ard Biesheuvel 
> 
> Subject: Re: [edk2-devel] [PATCH v5 4/5] BaseTools: Compile AML bytecode 
> arrays into .obj file
> 
> Hi Leif,
> 
> > > Masahisa - since Ard is still on holiday, could you create a patch
> > > and send out for me to review? Either one of the files needs to be
> > > renamed, or we need to move the .asl files (Emmc.asl and Optee.asl)
> > > into a subdirectory.
> 
> Probably I'm missing something, but my build for Developerbox platform is 
> successful, with the latest edk2/edk2-platforms as of today.
> 
> My build option is:
> ---
> export TARGET=Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> export PROFILE=DEBUG
> export ARCH=AARCH64
> export TOOL_CHAIN_TAG=GCC5
> 
> build -n $NUM_CPUS -a $ARCH -b $PROFILE -t $TOOL_CHAIN_TAG -p $TARGET -D 
> SECURE_BOOT_ENABLE=TRUE -D X64EMU_ENABLE=TRUE -D TPM2_ENABLE=TRUE
> ---
> 
> GCC version is 
> "gcc-linaro-6.4.1-2017.11-x86_64_aarch64-linux-gnu/bin/aarch64-linux-gnu-".
> 
> Some Output directory information:
> ~/src/uefi/Build/DeveloperBox$ find .|grep -i emmc 
> ./DEBUG_GCC5/AARCH64/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe/OUTPUT/Emmc.iii
> ./DEBUG_GCC5/AARCH64/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe/OUTPUT/Emmc.obj.deps
> ./DEBUG_GCC5/AARCH64/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe/OUTPUT/Emmc.
> ./DEBUG_GCC5/AARCH64/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe/OUTPUT/Emmc.obj
> ./DEBUG_GCC5/AARCH64/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe/OUTPUT/Emmc.aml.deps
> ./DEBUG_GCC5/AARCH64/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe/OUTPUT/Emmc.aml
> ./DEBUG_GCC5/AARCH64/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe/OUTPUT/Emmc.i
> 
> Regards,
> Masahisa
> 
> On Tue, 28 Jul 2020 at 09:40, Gao, Liming  wrote:
> >
> > Pierre:
> >   Thanks for your investigation. For now, the module will rename source 
> > file to avoid the file conflict. Can you submit one BZ for BaseTools to 
> > enhance the logic for the autogen source file? This needs more discussion.
> >
> >   Yes. I agree to place the generated source file into $(DEBUG_DIR). You 
> > can make another patch for this change.
> >
> > Thanks
> > Liming
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of
> > PierreGondois
> > Sent: 2020年7月28日 1:10
> > To: devel@edk2.groups.io; Gao, Liming ; Leif
> > Lindholm ; Masahisa Kojima
> > 
> > Cc: Sami Mujawar ; Tomas Pilar
> > ; Feng, Bob C ; Ard
> > Biesheuvel 
> > Subject: Re: [edk2-devel] [PATCH v5 4/5] BaseTools: Compile AML
> > bytecode arrays into .obj file
> >
> > Hello,
> >
> > Liming:
> > I didn't find anything preventing from having a .vfr and .c file with the 
> > same name in the same scope.
> >
> > Everybody:
> > The .c files generated (from either .vfr or .asl) are generated in the 
> > Build//OUTPUT/ directory, with their original subdirectory stripped. In 
> > the example below, the .c, .vfr and .asl files have been placed in 
> > subdirectories in edk2/edk2-platforms. The recipes that have been generated 
> > are:
> > $(OUTPUT_DIR)/Emmc.obj : $(OUTPUT_DIR)/AslDir/Emmc.c (from Emmc.asl)
> > $(OUTPUT_DIR)/Ip4Config2.obj : $(DEBUG_DIR)/VfrDir/Ip4Config2.c (from
> > Ip4Config2.vfr) $(OUTPUT_DIR)/CDir/Emmc.obj :
> > $(WORKSPACE)/edk2-platforms/Silicon/Socionext/SynQuacer/Drivers/Platfo
> > rmDxe/CDir/Emmc.c (from Emmc.c)
> >
> > The only subdirectory that remains is the one from the .c files. Indeed, 
> > the build system might detects subdirectories from the .inf file location. 
> > As the .c file is generated in the OUTPUT directory where there is no .inf 
> > file, it gets stripped.
> > This means that if a subdirectory is created, it is for the .c file, but I 
> > don't think we should modify this.
> >
> > Adding a pre/postfix to the generated .c filename seems a better approach 
> > to me. It would require modifying build_rule.template, and choose which 
> > pre/postfix to add.
> >
> > Another point is that the .c file generated from the .c file is placed in 
> > the $(DEBUG_DIR), when the one coming from the .asl file is placed in 
> > $(OUTPUT_DIR). Maybe it should be modified to $(DEBUG_DIR) aswell.
> >
> > Regards,
> > Pierre
> >
> > -Origina

Re: [edk2-devel] [PATCH v12 00/46] SEV-ES guest support

2020-07-28 Thread Laszlo Ersek
On 07/27/20 19:49, Tom Lendacky wrote:

> Yes, I always get an error when I reach 40 emails:
> 
> Reported error: 550 5.0.350 Remote server returned an error -> 500 We have
> received more than 40 messages in 30 minutes from you. To guard against
> autoresponder mail loops, we must reject additional messages from you
> temporarily. Please try again later.
> 
> So I usually have to wait a few hours and resend the last ones so they
> make it to groups.io.

Sigh.

Phil found one way to deal with this non-interactively: see the
"sendemail.smtpBatchSize" and "sendemail.smtpReloginDelay" git-config
options. From git-send-email(1):

   --batch-size=
   Some email servers (e.g. smtp.163.com) limit the number
   emails to be sent per session (connection) and this will
   lead to a failure when sending many messages. With this
   option, send-email will disconnect after sending $
   messages and wait for a few seconds (see --relogin-delay)
   and reconnect, to work around such a limit. You may want to
   use some form of credential helper to avoid having to
   retype your password every time this happens. Defaults to
   the sendemail.smtpBatchSize configuration variable.

   --relogin-delay=
   Waiting $ seconds before reconnecting to SMTP server.
   Used together with --batch-size option. Defaults to the
   sendemail.smtpReloginDelay configuration variable.

That way the "second batch" should need no manual work, hopefully.

Thanks
Laszlo


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

View/Reply Online (#63399): https://edk2.groups.io/g/devel/message/63399
Mute This Topic: https://groups.io/mt/75824926/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v3] UefiCpuPkg/MtrrLib/UnitTest: Add host based unit test

2020-07-28 Thread Laszlo Ersek
On 07/28/20 04:30, Ray Ni wrote:
> Add host based unit tests for the MtrrLib services.
> The BaseLib services AsmCpuid(), AsmReadMsr64(), and
> AsmWriteMsr64() are hooked and provide simple emulation
> of the CPUID leafs and MSRs required by the MtrrLib to
> run as a host based unit test.
> 
> Test cases are developed for each of the API.
> 
> For the most important APIs MtrrSetMemoryAttributesInMtrrSettings()
> and MtrrSetMemoryAttributeInMtrrSettings(), random inputs are
> generated and fed to the APIs to make sure the implementation is
> good. The test application accepts an optional parameter which
> specifies how many iterations of feeding random inputs to the two
> APIs. The overall number of test cases increases when the iteration
> increases. Default iteration is 10 when no parameter is specified.
> 
> Signed-off-by: Ray Ni 
> Signed-off-by: Michael D Kinney 
> Signed-off-by: Ming Shao 
> Cc: Michael D Kinney 
> Cc: Eric Dong 
> Cc: Laszlo Ersek 
> Cc: Ming Shao 
> Cc: Sean Brogan 
> Cc: Bret Barkelew 
> Cc: Jiewen Yao 
> ---
>  .../MtrrLib/UnitTest/MtrrLibUnitTest.c| 1140 +
>  .../MtrrLib/UnitTest/MtrrLibUnitTest.h|  171 +++
>  .../MtrrLib/UnitTest/MtrrLibUnitTestHost.inf  |   39 +
>  UefiCpuPkg/Library/MtrrLib/UnitTest/Support.c |  913 +
>  UefiCpuPkg/Test/UefiCpuPkgHostTest.dsc|   31 +
>  UefiCpuPkg/UefiCpuPkg.ci.yaml |   12 +-
>  6 files changed, 2305 insertions(+), 1 deletion(-)
>  create mode 100644 UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTest.c
>  create mode 100644 UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTest.h
>  create mode 100644 
> UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTestHost.inf
>  create mode 100644 UefiCpuPkg/Library/MtrrLib/UnitTest/Support.c
>  create mode 100644 UefiCpuPkg/Test/UefiCpuPkgHostTest.dsc

Acked-by: Laszlo Ersek 


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

View/Reply Online (#63398): https://edk2.groups.io/g/devel/message/63398
Mute This Topic: https://groups.io/mt/75837117/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH 10/15] OvmfPkg/OvmfPkg.ci.yaml: Add configuration for LicenseCheck

2020-07-28 Thread Laszlo Ersek
On 07/28/20 03:11, Zhang, Shenglei wrote:
> Hi Laszlo,
> 
>> -Original Message-
>> From: Laszlo Ersek 
>> Sent: Monday, July 27, 2020 5:51 PM
>> To: Zhang, Shenglei ; Rebecca Cran
>> 
>> Cc: devel@edk2.groups.io; Justen, Jordan L ;
>> Ard Biesheuvel 
>> Subject: Re: [PATCH 10/15] OvmfPkg/OvmfPkg.ci.yaml: Add configuration for
>> LicenseCheck
>>
>> On 07/27/20 08:21, Zhang, Shenglei wrote:
>>> Hi Laszlo,
>>>
>>> VbeShim.h is existing in edk2 now. This plugin only checks the patches to
>> be checked in.
>>> So there's no need to add existing files to this section.
>>
>> OK, thanks, we can always extend this stanza later, if needed.
>>
>> Rebecca: once this patch is upstream, please post a separate patch for 
>> listing
>> "OvmfPkg/Bhyve/BhyveRfbDxe/VbeShim.h" in "IgnoreFiles". Otherwise I
>> won't be able to merge your patch at
>> .
>>
>>
>> Shenglei: I have a question regarding IgnoreFiles syntax. In
>> "MdeModulePkg/MdeModulePkg.ci.yaml", there are two syntaxes:
>>
>> - The IgnoreFiles stanza for "CharEncodingCheck" uses pathnames that are
>> relative to the *project* root:
>>
>>> ## options defined ci/Plugin/CharEncodingCheck
>>> "CharEncodingCheck": {
>>> "IgnoreFiles": [
>>>
>> "MdeModulePkg/Universal/RegularExpressionDxe/oniguruma/test/testc.c",
>>>
>> "MdeModulePkg/Universal/RegularExpressionDxe/oniguruma/windows/tes
>> tc.c"
>>> ]
>>> },
>>
>> - The IgnoreFiles stanza for "SpellCheck" uses pathnames that are relative to
>> the *package* (not project) root:
>>
>>> "SpellCheck": {
>>> ...
>>> "IgnoreFiles": [ # use gitignore syntax to ignore 
>>> errors in matching
>> files
>>> "Library/LzmaCustomDecompressLib/Sdk/DOC/*"
>>> ],
>>
>> How do we know whether a particular check's IgnoreFiles stanza requires
>> project-root-relative or package-root-relative pathnames?
> 
> It depends on the designing of the plugins, likes the check scope.
> But looks like all checks' IgnoreFiles stanza only requires 
> package-root-relative pathnames, currently.
> It's recommended to use package-root-relative pathnames because a plugin must 
> support this format.

Thanks!

So we should ignore "Bhyve/BhyveRfbDxe/VbeShim.h".

Thanks
Laszlo


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

View/Reply Online (#63397): https://edk2.groups.io/g/devel/message/63397
Mute This Topic: https://groups.io/mt/75678218/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v5 4/5] BaseTools: Compile AML bytecode arrays into .obj file

2020-07-28 Thread PierreGondois
Hello Masahisa,

Maybe you will have to delete/re-generate the build_rule.txt and tools_def.txt 
files that you have. This patch makes the build system generate a .amli file 
for each .asl file. If there is no Emmc.amli in your Build directory, this 
might be the reason.

Regards,
Pierre

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Masahisa Kojima 
via groups.io
Sent: Tuesday, July 28, 2020 2:38 AM
To: Leif Lindholm 
Cc: devel@edk2.groups.io; Gao, Liming ; Pierre Gondois 
; Sami Mujawar ; Tomas Pilar 
; Feng, Bob C ; Ard Biesheuvel 

Subject: Re: [edk2-devel] [PATCH v5 4/5] BaseTools: Compile AML bytecode arrays 
into .obj file

Hi Leif,

> > Masahisa - since Ard is still on holiday, could you create a patch
> > and send out for me to review? Either one of the files needs to be
> > renamed, or we need to move the .asl files (Emmc.asl and Optee.asl)
> > into a subdirectory.

Probably I'm missing something, but my build for Developerbox platform is 
successful, with the latest edk2/edk2-platforms as of today.

My build option is:
---
export TARGET=Platform/Socionext/DeveloperBox/DeveloperBox.dsc
export PROFILE=DEBUG
export ARCH=AARCH64
export TOOL_CHAIN_TAG=GCC5

build -n $NUM_CPUS -a $ARCH -b $PROFILE -t $TOOL_CHAIN_TAG -p $TARGET -D 
SECURE_BOOT_ENABLE=TRUE -D X64EMU_ENABLE=TRUE -D TPM2_ENABLE=TRUE
---

GCC version is 
"gcc-linaro-6.4.1-2017.11-x86_64_aarch64-linux-gnu/bin/aarch64-linux-gnu-".

Some Output directory information:
~/src/uefi/Build/DeveloperBox$ find .|grep -i emmc 
./DEBUG_GCC5/AARCH64/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe/OUTPUT/Emmc.iii
./DEBUG_GCC5/AARCH64/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe/OUTPUT/Emmc.obj.deps
./DEBUG_GCC5/AARCH64/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe/OUTPUT/Emmc.
./DEBUG_GCC5/AARCH64/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe/OUTPUT/Emmc.obj
./DEBUG_GCC5/AARCH64/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe/OUTPUT/Emmc.aml.deps
./DEBUG_GCC5/AARCH64/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe/OUTPUT/Emmc.aml
./DEBUG_GCC5/AARCH64/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe/OUTPUT/Emmc.i

Regards,
Masahisa

On Tue, 28 Jul 2020 at 09:40, Gao, Liming  wrote:
>
> Pierre:
>   Thanks for your investigation. For now, the module will rename source file 
> to avoid the file conflict. Can you submit one BZ for BaseTools to enhance 
> the logic for the autogen source file? This needs more discussion.
>
>   Yes. I agree to place the generated source file into $(DEBUG_DIR). You can 
> make another patch for this change.
>
> Thanks
> Liming
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of
> PierreGondois
> Sent: 2020年7月28日 1:10
> To: devel@edk2.groups.io; Gao, Liming ; Leif
> Lindholm ; Masahisa Kojima
> 
> Cc: Sami Mujawar ; Tomas Pilar
> ; Feng, Bob C ; Ard
> Biesheuvel 
> Subject: Re: [edk2-devel] [PATCH v5 4/5] BaseTools: Compile AML
> bytecode arrays into .obj file
>
> Hello,
>
> Liming:
> I didn't find anything preventing from having a .vfr and .c file with the 
> same name in the same scope.
>
> Everybody:
> The .c files generated (from either .vfr or .asl) are generated in the 
> Build//OUTPUT/ directory, with their original subdirectory stripped. In 
> the example below, the .c, .vfr and .asl files have been placed in 
> subdirectories in edk2/edk2-platforms. The recipes that have been generated 
> are:
> $(OUTPUT_DIR)/Emmc.obj : $(OUTPUT_DIR)/AslDir/Emmc.c (from Emmc.asl)
> $(OUTPUT_DIR)/Ip4Config2.obj : $(DEBUG_DIR)/VfrDir/Ip4Config2.c (from
> Ip4Config2.vfr) $(OUTPUT_DIR)/CDir/Emmc.obj :
> $(WORKSPACE)/edk2-platforms/Silicon/Socionext/SynQuacer/Drivers/Platfo
> rmDxe/CDir/Emmc.c (from Emmc.c)
>
> The only subdirectory that remains is the one from the .c files. Indeed, the 
> build system might detects subdirectories from the .inf file location. As the 
> .c file is generated in the OUTPUT directory where there is no .inf file, it 
> gets stripped.
> This means that if a subdirectory is created, it is for the .c file, but I 
> don't think we should modify this.
>
> Adding a pre/postfix to the generated .c filename seems a better approach to 
> me. It would require modifying build_rule.template, and choose which 
> pre/postfix to add.
>
> Another point is that the .c file generated from the .c file is placed in the 
> $(DEBUG_DIR), when the one coming from the .asl file is placed in 
> $(OUTPUT_DIR). Maybe it should be modified to $(DEBUG_DIR) aswell.
>
> Regards,
> Pierre
>
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Liming
> Gao via groups.io
> Sent: Monday, July 27, 2020 3:21 PM
> To: Leif Lindholm ; devel@edk2.groups.io; Pierre
> Gondois ; Masahisa Kojima
> 
> Cc: Sami Mujawar ; Tomas Pilar
> ; Feng, Bob C ; Ard
> Biesheuvel 
> Subject: Re: [edk2-devel] [PATCH v5 4/5] BaseTools: Compile AML
> bytecode arrays into .obj file
>
> Leif:
>   VFR file has the similar

Re: [edk2-devel] [PATCH v4] UefiCpuPkg/MtrrLib/UnitTest: Add host based unit test

2020-07-28 Thread Ni, Ray
Eric,
I just found another GCC build failure with my original C code.

Can you please review this patch again?

Thanks,
Ray

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Ni, Ray
> Sent: Tuesday, July 28, 2020 4:43 PM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D ; Shao, Ming 
> ; Dong, Eric
> ; Laszlo Ersek ; Sean Brogan 
> ; Bret Barkelew
> ; Yao, Jiewen 
> Subject: [edk2-devel] [PATCH v4] UefiCpuPkg/MtrrLib/UnitTest: Add host based 
> unit test
> 
> Add host based unit tests for the MtrrLib services.
> The BaseLib services AsmCpuid(), AsmReadMsr64(), and
> AsmWriteMsr64() are hooked and provide simple emulation
> of the CPUID leafs and MSRs required by the MtrrLib to
> run as a host based unit test.
> 
> Test cases are developed for each of the API.
> 
> For the most important APIs MtrrSetMemoryAttributesInMtrrSettings()
> and MtrrSetMemoryAttributeInMtrrSettings(), random inputs are
> generated and fed to the APIs to make sure the implementation is
> good. The test application accepts an optional parameter which
> specifies how many iterations of feeding random inputs to the two
> APIs. The overall number of test cases increases when the iteration
> increases. Default iteration is 10 when no parameter is specified.
> 
> Signed-off-by: Ray Ni 
> Signed-off-by: Michael D Kinney 
> Signed-off-by: Ming Shao 
> Cc: Michael D Kinney 
> Cc: Eric Dong 
> Cc: Laszlo Ersek 
> Cc: Ming Shao 
> Cc: Sean Brogan 
> Cc: Bret Barkelew 
> Cc: Jiewen Yao 
> ---
>  .../MtrrLib/UnitTest/MtrrLibUnitTest.c| 1139 +
>  .../MtrrLib/UnitTest/MtrrLibUnitTest.h|  182 +++
>  .../MtrrLib/UnitTest/MtrrLibUnitTestHost.inf  |   39 +
>  UefiCpuPkg/Library/MtrrLib/UnitTest/Support.c |  923 +
>  UefiCpuPkg/Test/UefiCpuPkgHostTest.dsc|   31 +
>  UefiCpuPkg/UefiCpuPkg.ci.yaml |   12 +-
>  6 files changed, 2325 insertions(+), 1 deletion(-)
>  create mode 100644 UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTest.c
>  create mode 100644 UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTest.h
>  create mode 100644 
> UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTestHost.inf
>  create mode 100644 UefiCpuPkg/Library/MtrrLib/UnitTest/Support.c
>  create mode 100644 UefiCpuPkg/Test/UefiCpuPkgHostTest.dsc
> 
> diff --git a/UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTest.c
> b/UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTest.c
> new file mode 100644
> index 00..123e1c741a
> --- /dev/null
> +++ b/UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTest.c
> @@ -0,0 +1,1139 @@
> +/** @file
> 
> +  Unit tests of the MtrrLib instance of the MtrrLib class
> 
> +
> 
> +  Copyright (c) 2020, Intel Corporation. All rights reserved.
> 
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> +
> 
> +**/
> 
> +
> 
> +#include "MtrrLibUnitTest.h"
> 
> +
> 
> +STATIC CONST MTRR_LIB_SYSTEM_PARAMETER mDefaultSystemParameter = {
> 
> +  42, TRUE, TRUE, CacheUncacheable, 12
> 
> +};
> 
> +
> 
> +STATIC MTRR_LIB_SYSTEM_PARAMETER mSystemParameters[] = {
> 
> +  { 38, TRUE, TRUE, CacheUncacheable,12 },
> 
> +  { 38, TRUE, TRUE, CacheWriteBack,  12 },
> 
> +  { 38, TRUE, TRUE, CacheWriteThrough,   12 },
> 
> +  { 38, TRUE, TRUE, CacheWriteProtected, 12 },
> 
> +  { 38, TRUE, TRUE, CacheWriteCombining, 12 },
> 
> +
> 
> +  { 42, TRUE, TRUE, CacheUncacheable,12 },
> 
> +  { 42, TRUE, TRUE, CacheWriteBack,  12 },
> 
> +  { 42, TRUE, TRUE, CacheWriteThrough,   12 },
> 
> +  { 42, TRUE, TRUE, CacheWriteProtected, 12 },
> 
> +  { 42, TRUE, TRUE, CacheWriteCombining, 12 },
> 
> +
> 
> +  { 48, TRUE, TRUE, CacheUncacheable,12 },
> 
> +  { 48, TRUE, TRUE, CacheWriteBack,  12 },
> 
> +  { 48, TRUE, TRUE, CacheWriteThrough,   12 },
> 
> +  { 48, TRUE, TRUE, CacheWriteProtected, 12 },
> 
> +  { 48, TRUE, TRUE, CacheWriteCombining, 12 },
> 
> +};
> 
> +
> 
> +UINT32mFixedMtrrsIndex[] = {
> 
> +  MSR_IA32_MTRR_FIX64K_0,
> 
> +  MSR_IA32_MTRR_FIX16K_8,
> 
> +  MSR_IA32_MTRR_FIX16K_A,
> 
> +  MSR_IA32_MTRR_FIX4K_C,
> 
> +  MSR_IA32_MTRR_FIX4K_C8000,
> 
> +  MSR_IA32_MTRR_FIX4K_D,
> 
> +  MSR_IA32_MTRR_FIX4K_D8000,
> 
> +  MSR_IA32_MTRR_FIX4K_E,
> 
> +  MSR_IA32_MTRR_FIX4K_E8000,
> 
> +  MSR_IA32_MTRR_FIX4K_F,
> 
> +  MSR_IA32_MTRR_FIX4K_F8000
> 
> +};
> 
> +STATIC_ASSERT (
> 
> +  (ARRAY_SIZE (mFixedMtrrsIndex) == MTRR_NUMBER_OF_FIXED_MTRR),
> 
> +  "gFixedMtrrIndex does NOT contain all the fixed MTRRs!"
> 
> +  );
> 
> +
> 
> +//
> 
> +// Context structure to be used for most of the test cases.
> 
> +//
> 
> +typedef struct {
> 
> +  CONST MTRR_LIB_SYSTEM_PARAMETER *SystemParameter;
> 
> +} MTRR_LIB_TEST_CONTEXT;
> 
> +
> 
> +//
> 
> +// Context structure to be used for GetFirmwareVariableMtrrCount() test.
> 
> +//
> 
> +typedef struct {
> 
> +  UINT32  NumberOfReservedVariableMtrrs;
> 
> +  CONST MTRR_LIB_SYSTEM_PARAMETER *SystemParameter;
> 
> +} MTRR_LIB_GET_FIRMWARE_VARIABLE_MTRR_

[edk2-devel] [PATCH v4] UefiCpuPkg/MtrrLib/UnitTest: Add host based unit test

2020-07-28 Thread Ni, Ray
Add host based unit tests for the MtrrLib services.
The BaseLib services AsmCpuid(), AsmReadMsr64(), and
AsmWriteMsr64() are hooked and provide simple emulation
of the CPUID leafs and MSRs required by the MtrrLib to
run as a host based unit test.

Test cases are developed for each of the API.

For the most important APIs MtrrSetMemoryAttributesInMtrrSettings()
and MtrrSetMemoryAttributeInMtrrSettings(), random inputs are
generated and fed to the APIs to make sure the implementation is
good. The test application accepts an optional parameter which
specifies how many iterations of feeding random inputs to the two
APIs. The overall number of test cases increases when the iteration
increases. Default iteration is 10 when no parameter is specified.

Signed-off-by: Ray Ni 
Signed-off-by: Michael D Kinney 
Signed-off-by: Ming Shao 
Cc: Michael D Kinney 
Cc: Eric Dong 
Cc: Laszlo Ersek 
Cc: Ming Shao 
Cc: Sean Brogan 
Cc: Bret Barkelew 
Cc: Jiewen Yao 
---
 .../MtrrLib/UnitTest/MtrrLibUnitTest.c| 1139 +
 .../MtrrLib/UnitTest/MtrrLibUnitTest.h|  182 +++
 .../MtrrLib/UnitTest/MtrrLibUnitTestHost.inf  |   39 +
 UefiCpuPkg/Library/MtrrLib/UnitTest/Support.c |  923 +
 UefiCpuPkg/Test/UefiCpuPkgHostTest.dsc|   31 +
 UefiCpuPkg/UefiCpuPkg.ci.yaml |   12 +-
 6 files changed, 2325 insertions(+), 1 deletion(-)
 create mode 100644 UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTest.c
 create mode 100644 UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTest.h
 create mode 100644 UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTestHost.inf
 create mode 100644 UefiCpuPkg/Library/MtrrLib/UnitTest/Support.c
 create mode 100644 UefiCpuPkg/Test/UefiCpuPkgHostTest.dsc

diff --git a/UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTest.c 
b/UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTest.c
new file mode 100644
index 00..123e1c741a
--- /dev/null
+++ b/UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTest.c
@@ -0,0 +1,1139 @@
+/** @file
+  Unit tests of the MtrrLib instance of the MtrrLib class
+
+  Copyright (c) 2020, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "MtrrLibUnitTest.h"
+
+STATIC CONST MTRR_LIB_SYSTEM_PARAMETER mDefaultSystemParameter = {
+  42, TRUE, TRUE, CacheUncacheable, 12
+};
+
+STATIC MTRR_LIB_SYSTEM_PARAMETER mSystemParameters[] = {
+  { 38, TRUE, TRUE, CacheUncacheable,12 },
+  { 38, TRUE, TRUE, CacheWriteBack,  12 },
+  { 38, TRUE, TRUE, CacheWriteThrough,   12 },
+  { 38, TRUE, TRUE, CacheWriteProtected, 12 },
+  { 38, TRUE, TRUE, CacheWriteCombining, 12 },
+
+  { 42, TRUE, TRUE, CacheUncacheable,12 },
+  { 42, TRUE, TRUE, CacheWriteBack,  12 },
+  { 42, TRUE, TRUE, CacheWriteThrough,   12 },
+  { 42, TRUE, TRUE, CacheWriteProtected, 12 },
+  { 42, TRUE, TRUE, CacheWriteCombining, 12 },
+
+  { 48, TRUE, TRUE, CacheUncacheable,12 },
+  { 48, TRUE, TRUE, CacheWriteBack,  12 },
+  { 48, TRUE, TRUE, CacheWriteThrough,   12 },
+  { 48, TRUE, TRUE, CacheWriteProtected, 12 },
+  { 48, TRUE, TRUE, CacheWriteCombining, 12 },
+};
+
+UINT32mFixedMtrrsIndex[] = {
+  MSR_IA32_MTRR_FIX64K_0,
+  MSR_IA32_MTRR_FIX16K_8,
+  MSR_IA32_MTRR_FIX16K_A,
+  MSR_IA32_MTRR_FIX4K_C,
+  MSR_IA32_MTRR_FIX4K_C8000,
+  MSR_IA32_MTRR_FIX4K_D,
+  MSR_IA32_MTRR_FIX4K_D8000,
+  MSR_IA32_MTRR_FIX4K_E,
+  MSR_IA32_MTRR_FIX4K_E8000,
+  MSR_IA32_MTRR_FIX4K_F,
+  MSR_IA32_MTRR_FIX4K_F8000
+};
+STATIC_ASSERT (
+  (ARRAY_SIZE (mFixedMtrrsIndex) == MTRR_NUMBER_OF_FIXED_MTRR),
+  "gFixedMtrrIndex does NOT contain all the fixed MTRRs!"
+  );
+
+//
+// Context structure to be used for most of the test cases.
+//
+typedef struct {
+  CONST MTRR_LIB_SYSTEM_PARAMETER *SystemParameter;
+} MTRR_LIB_TEST_CONTEXT;
+
+//
+// Context structure to be used for GetFirmwareVariableMtrrCount() test.
+//
+typedef struct {
+  UINT32  NumberOfReservedVariableMtrrs;
+  CONST MTRR_LIB_SYSTEM_PARAMETER *SystemParameter;
+} MTRR_LIB_GET_FIRMWARE_VARIABLE_MTRR_COUNT_CONTEXT;
+
+STATIC CHAR8 *mCacheDescription[] = { "UC", "WC", "N/A", "N/A", "WT", "WP", 
"WB" };
+
+/**
+  Compare the actual memory ranges against expected memory ranges and return 
PASS when they match.
+
+  @param ExpectedMemoryRanges Expected memory ranges.
+  @param ExpectedMemoryRangeCount Count of expected memory ranges.
+  @param ActualRanges Actual memory ranges.
+  @param ActualRangeCount Count of actual memory ranges.
+
+  @retval UNIT_TEST_PASSED  Test passed.
+  @retval othersTest failed.
+**/
+UNIT_TEST_STATUS
+VerifyMemoryRanges (
+  IN MTRR_MEMORY_RANGE  *ExpectedMemoryRanges,
+  IN UINTN  ExpectedMemoryRangeCount,
+  IN MTRR_MEMORY_RANGE  *ActualRanges,
+  IN UINTN  ActualRangeCount
+  )
+{
+  UINTN  Index;
+  UT_ASSERT_EQUAL (ExpectedMemoryRangeCount, ActualRangeCount);
+  for (Index = 0; Index < ExpectedMemoryRan

[edk2-devel] [PATCH v4 2/2] SecurityPkg/Tcg2Config: remove TPM2_ChangEPS if it is not supported.

2020-07-28 Thread Qi Zhang
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2793

In current implementation TPM2_ChangeEPS command is always available
in the TPM2 operation pull down list in TCG2 Configuration, which
is confusing when the command is not supported by specific TPM chip.
As a user experience improvement, TPM2_ChangeEPS command should be
removed from the list when it is not supported.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Chao Zhang 
Cc: Rahul Kumar 
Signed-off-by: Qi Zhang 
---
 SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr | 2 ++
 SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c   | 7 +++
 SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h | 1 +
 3 files changed, 10 insertions(+)

diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr 
b/SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr
index 91a463997c..47d63b009d 100644
--- a/SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr
+++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr
@@ -144,7 +144,9 @@ formset
 option text = STRING_TOKEN(STR_TCG2_DISABLE), value = 
TCG2_PHYSICAL_PRESENCE_DISABLE, flags = RESET_REQUIRED;
 option text = STRING_TOKEN(STR_TCG2_CLEAR), value = 
TCG2_PHYSICAL_PRESENCE_CLEAR, flags = RESET_REQUIRED;
 option text = STRING_TOKEN(STR_TCG2_SET_PCD_BANKS), value = 
TCG2_PHYSICAL_PRESENCE_SET_PCR_BANKS, flags = RESET_REQUIRED;
+suppressif ideqval TCG2_CONFIGURATION_INFO.ChangeEPSSupported == 0;
 option text = STRING_TOKEN(STR_TCG2_CHANGE_EPS), value = 
TCG2_PHYSICAL_PRESENCE_CHANGE_EPS, flags = RESET_REQUIRED;
+endif
 option text = STRING_TOKEN(STR_TCG2_LOG_ALL_DIGESTS), value = 
TCG2_PHYSICAL_PRESENCE_LOG_ALL_DIGESTS, flags = RESET_REQUIRED;
 option text = 
STRING_TOKEN(STR_TCG2_DISABLE_ENDORSEMENT_ENABLE_STORAGE_HIERARCHY), value = 
TCG2_PHYSICAL_PRESENCE_DISABLE_ENDORSEMENT_ENABLE_STORAGE_HIERARCHY, flags = 
RESET_REQUIRED;
 endoneof;
diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c 
b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
index baa8fcd08d..2946f95db0 100644
--- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
+++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
@@ -788,6 +788,7 @@ InstallTcg2ConfigForm (
   CHAR16  TempBuffer[1024];
   TCG2_CONFIGURATION_INFO Tcg2ConfigInfo;
   TPM2_PTP_INTERFACE_TYPE TpmDeviceInterfaceDetected;
+  BOOLEAN IsCmdImp = FALSE;
 
   DriverHandle = NULL;
   ConfigAccess = &PrivateData->ConfigAccess;
@@ -870,6 +871,12 @@ InstallTcg2ConfigForm (
 HiiSetString (PrivateData->HiiHandle, STRING_TOKEN 
(STR_TPM2_SUPPORTED_HASH_ALGO_CONTENT), TempBuffer, NULL);
   }
 
+  Status = Tpm2GetCapabilityIsCommandImplemented (TPM_CC_ChangeEPS, &IsCmdImp);
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Tpm2GetCapabilityIsCmdImpl fails %r\n", Status));
+  }
+  Tcg2ConfigInfo.ChangeEPSSupported = IsCmdImp;
+
   FillBufferWithBootHashAlg (TempBuffer, sizeof(TempBuffer), PcdGet32 
(PcdTcg2HashAlgorithmBitmap));
   HiiSetString (PrivateData->HiiHandle, STRING_TOKEN 
(STR_BIOS_HASH_ALGO_CONTENT), TempBuffer, NULL);
 
diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h 
b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h
index a91c052850..b84af40a04 100644
--- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h
+++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h
@@ -70,6 +70,7 @@ typedef struct {
   UINT8TpmDeviceInterfaceAttempt;
   BOOLEAN  TpmDeviceInterfacePtpFifoSupported;
   BOOLEAN  TpmDeviceInterfacePtpCrbSupported;
+  BOOLEAN  ChangeEPSSupported;
 } TCG2_CONFIGURATION_INFO;
 
 //
-- 
2.26.2.windows.1


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

View/Reply Online (#63393): https://edk2.groups.io/g/devel/message/63393
Mute This Topic: https://groups.io/mt/75840054/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/2] refine TPM2 operation pull down list

2020-07-28 Thread Qi Zhang
v2 change: separate the change into two patches
v3 change: rename Tpm2GetCapabilityIsCmdImpl to 
Tpm2GetCapabilityIsCommandImplemented
v4 change: fix two build errors reported by CI

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Chao Zhang 
Cc: Rahul Kumar 
Signed-off-by: Qi Zhang 

Qi Zhang (1):
  SecurityPkg/Tcg2Config: remove TPM2_ChangEPS if it is not supported.

Zhang, Qi (1):
  SecurityPkg/Tpm2CommandLib: add a new function

 SecurityPkg/Include/Library/Tpm2CommandLib.h  | 16 
 .../Library/Tpm2CommandLib/Tpm2Capability.c   | 40 +++
 SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr |  2 +
 SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c   |  7 
 SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h |  1 +
 5 files changed, 66 insertions(+)

-- 
2.26.2.windows.1


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

View/Reply Online (#63391): https://edk2.groups.io/g/devel/message/63391
Mute This Topic: https://groups.io/mt/75840051/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] [PATCH v4 1/2] SecurityPkg/Tpm2CommandLib: add a new function

2020-07-28 Thread Qi Zhang
From: "Zhang, Qi" 

 Tpm2GetCapabilityIsCommandImplemented

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

check if the commad is supported by comparing the command code with
command index.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Chao Zhang 
Cc: Rahul Kumar 
Signed-off-by: Qi Zhang 
---
 SecurityPkg/Include/Library/Tpm2CommandLib.h  | 16 
 .../Library/Tpm2CommandLib/Tpm2Capability.c   | 40 +++
 2 files changed, 56 insertions(+)

diff --git a/SecurityPkg/Include/Library/Tpm2CommandLib.h 
b/SecurityPkg/Include/Library/Tpm2CommandLib.h
index ce381e786b..ee8eb62295 100644
--- a/SecurityPkg/Include/Library/Tpm2CommandLib.h
+++ b/SecurityPkg/Include/Library/Tpm2CommandLib.h
@@ -790,6 +790,22 @@ Tpm2GetCapabilityAlgorithmSet (
   OUT UINT32  *AlgorithmSet
   );
 
+/**
+  This function will query if the command is supported.
+
+  @param[In]  Command TPM_CC command starts from TPM_CC_FIRST.
+  @param[out] IsCmdImpl   The command is supported or not.
+
+  @retval EFI_SUCCESSOperation completed successfully.
+  @retval EFI_DEVICE_ERROR   The command was unsuccessful.
+**/
+EFI_STATUS
+EFIAPI
+Tpm2GetCapabilityIsCommandImplemented (
+  IN  TPM_CC  Command,
+  OUT BOOLEAN *IsCmdImpl
+  );
+
 /**
   This command is used to check to see if specific combinations of algorithm 
parameters are supported.
 
diff --git a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c 
b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
index 85b11c7715..17c0c3a151 100644
--- a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
+++ b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
@@ -39,6 +39,8 @@ typedef struct {
 
 #pragma pack()
 
+#define TPMA_CC_COMMANDINDEX_MASK  0x2000
+
 /**
   This command returns various information regarding the TPM and its current 
state.
 
@@ -628,6 +630,44 @@ Tpm2GetCapabilityAlgorithmSet (
   return EFI_SUCCESS;
 }
 
+/**
+  This function will query if the command is supported.
+
+  @param[In]  Command TPM_CC command starts from TPM_CC_FIRST.
+  @param[out] IsCmdImpl   The command is supported or not.
+
+  @retval EFI_SUCCESSOperation completed successfully.
+  @retval EFI_DEVICE_ERROR   The command was unsuccessful.
+**/
+EFI_STATUS
+EFIAPI
+Tpm2GetCapabilityIsCommandImplemented (
+  IN  TPM_CC  Command,
+  OUT BOOLEAN *IsCmdImpl
+  )
+{
+  TPMS_CAPABILITY_DATATpmCap;
+  TPMI_YES_NO MoreData;
+  EFI_STATUS  Status;
+  UINT32  Attribute;
+
+  Status = Tpm2GetCapability (
+ TPM_CAP_COMMANDS,
+ Command,
+ 1,
+ &MoreData,
+ &TpmCap
+ );
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  CopyMem (&Attribute, &TpmCap.data.command.commandAttributes[0], sizeof 
(UINT32));
+  *IsCmdImpl = (Command == (SwapBytes32(Attribute) & 
TPMA_CC_COMMANDINDEX_MASK));
+
+  return EFI_SUCCESS;
+}
+
 /**
   This command is used to check to see if specific combinations of algorithm 
parameters are supported.
 
-- 
2.26.2.windows.1


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

View/Reply Online (#63392): https://edk2.groups.io/g/devel/message/63392
Mute This Topic: https://groups.io/mt/75840053/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT instruction

2020-07-28 Thread Liming Gao
This error is reported from nasm compiler. My nasm compiler version is 2.11.08. 
It may be a little old. 2.12 should be fine. 

This change also requires to update edk2\BaseTools\Conf\tools_def.template and 
mention nasm compiler version. 

Thanks
Liming
-Original Message-
From: Tom Lendacky  
Sent: 2020年7月28日 12:08
To: Gao, Liming ; devel@edk2.groups.io
Cc: Brijesh Singh ; Ard Biesheuvel 
; Dong, Eric ; Justen, Jordan L 
; Laszlo Ersek ; Kinney, Michael 
D ; Ni, Ray 
Subject: Re: [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT 
instruction

On 7/27/20 8:34 PM, Gao, Liming wrote:
> Tom:

Hi Liming,

>I meet with GCC failure on this patch. Can you help check it? If nasm 
> doesn't support the vmmcall instruction in 32-bit mode, you have to use 
> inline assembly to support it.

What version of GCC are you using. I was able to successfully build the
Ia32 version with my GCC level. The Ia32 version uses a trick to do switch to 
64-bit just to encode the instruction. Looks like that doesn't work with your 
version of GCC.

I can probably switch to defining the instruction as bytes. Let me look into 
that and possibly send you a patch to test.

Thanks,
Tom

> 
> Edk2/Build/IntelFsp2Pkg/DEBUG_GCC5/IA32/MdePkg/Library/BaseLib/BaseLib
> /OUTPUT/Ia32/VmgExit.iii:33: error: elf32 output format does not 
> support 64-bit code
> GNUmakefile:741: recipe for target
> 
> Thanks
> Liming
> -Original Message-
> From: Tom Lendacky 
> Sent: 2020年7月27日 23:26
> To: devel@edk2.groups.io
> Cc: Brijesh Singh ; Ard Biesheuvel 
> ; Dong, Eric ; Justen, 
> Jordan L ; Laszlo Ersek 
> ; Gao, Liming ; Kinney, 
> Michael D ; Ni, Ray 
> Subject: [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT 
> instruction
> 
> From: Tom Lendacky 
> 
> BZ: 
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugz
> illa.tianocore.org%2Fshow_bug.cgi%3Fid%3D2198&data=02%7C01%7Cthoma
> s.lendacky%40amd.com%7C77c8250cd9e14f2929a008d832965726%7C3dd8961fe488
> 4e608e11a82d994e183d%7C0%7C0%7C637314968570901400&sdata=6zqseI3tVm
> aw351w9mfEymMnDcjDzjvcBrhARU6r3Ho%3D&reserved=0
> 
> VMGEXIT is a new instruction used for Hypervisor/Guest communication when 
> running as an SEV-ES guest. A VMGEXIT will cause an automatic exit (AE) to 
> occur, resulting in a #VMEXIT with an exit code value of 0x403.
> 
> Provide the necessary support to execute the VMGEXIT instruction, which is 
> "rep; vmmcall".
> 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Signed-off-by: Tom Lendacky 
> ---
>   MdePkg/Library/BaseLib/BaseLib.inf   |  2 ++
>   MdePkg/Include/Library/BaseLib.h | 14 +
>   MdePkg/Library/BaseLib/Ia32/VmgExit.nasm | 37   
> MdePkg/Library/BaseLib/X64/VmgExit.nasm  | 32 
>   4 files changed, 85 insertions(+)
>   create mode 100644 MdePkg/Library/BaseLib/Ia32/VmgExit.nasm
>   create mode 100644 MdePkg/Library/BaseLib/X64/VmgExit.nasm
> 
> diff --git a/MdePkg/Library/BaseLib/BaseLib.inf 
> b/MdePkg/Library/BaseLib/BaseLib.inf
> index 3b93b5db8d24..3b85c56c3c03 100644
> --- a/MdePkg/Library/BaseLib/BaseLib.inf
> +++ b/MdePkg/Library/BaseLib/BaseLib.inf
> @@ -184,6 +184,7 @@ [Sources.Ia32]
> Ia32/DisableCache.nasm| GCC
> Ia32/RdRand.nasm
> Ia32/XGetBv.nasm
> +  Ia32/VmgExit.nasm
>   
> Ia32/DivS64x64Remainder.c
> Ia32/InternalSwitchStack.c | MSFT
> @@ -317,6 +318,7 @@ [Sources.X64]
> X64/DisablePaging64.nasm
> X64/RdRand.nasm
> X64/XGetBv.nasm
> +  X64/VmgExit.nasm
> ChkStkGcc.c  | GCC
>   
>   [Sources.EBC]
> diff --git a/MdePkg/Include/Library/BaseLib.h 
> b/MdePkg/Include/Library/BaseLib.h
> index 7edf0051a0a0..04fb329eaabb 100644
> --- a/MdePkg/Include/Library/BaseLib.h
> +++ b/MdePkg/Include/Library/BaseLib.h
> @@ -7848,6 +7848,20 @@ AsmXGetBv (
> );
>   
>   
> +/**
> +  Executes a VMGEXIT instruction (VMMCALL with a REP prefix)
> +
> +  Executes a VMGEXIT instruction. This function is only available on
> + IA-32 and  x64.
> +
> +**/
> +VOID
> +EFIAPI
> +AsmVmgExit (
> +  VOID
> +  );
> +
> +
>   /**
> Patch the immediate operand of an IA32 or X64 instruction such that the 
> byte,
> word, dword or qword operand is encoded at the end of the 
> instruction's diff --git a/MdePkg/Library/BaseLib/Ia32/VmgExit.nasm 
> b/MdePkg/Library/BaseLib/Ia32/VmgExit.nasm
> new file mode 100644
> index ..a4b37385cc7a
> --- /dev/null
> +++ b/MdePkg/Library/BaseLib/Ia32/VmgExit.nasm
> @@ -0,0 +1,37 @@
> +;
> +--
> +
> +;
> +; Copyright (C) 2020, Advanced Micro Devices, Inc. All rights 
> +reserved. ; SPDX-License-Identifier: BSD-2-Clause-Patent ; ; 
> +Module
> +Name:
> +;
> +;   VmgExit.Asm
> +;
> +; Abstract:
> +;
> +;   AsmVmgExit function
> +;
> +; Notes:
> +;
> +;
> +--
> +
> +
> +SECTION .text
> +
> +;---