[edk2] [Patch V2 0/1] Document: Update DEC spec to remove EDK related contents

[Feng, Bob C]

V2:
Update top level README to add update history.

Feng, Bob C (1):
  Document: Update DEC spec to remove EDK related contents

 2_dec_file_overview/210_pcd_usage.md | 4 +---
 2_dec_file_overview/25_[includes]_usage.md   | 6 +-
 2_dec_file_overview/26_[guids]_usage.md  | 6 +-
 2_dec_file_overview/27_[protocols]_usage.md  | 6 +-
 2_dec_file_overview/28_[ppis]_usage.md   | 6 +-
 2_dec_file_overview/29_[libraryclasses]_usage.md | 4 +---
 3_edk_ii_dec_file_format/310_pcd_sections.md | 8 ++--
 3_edk_ii_dec_file_format/35_[includes]_sections.md   | 5 +
 3_edk_ii_dec_file_format/39_[libraryclasses]_sections.md | 5 +
 README.md| 1 +
 10 files changed, 11 insertions(+), 40 deletions(-)

-- 
2.20.1.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch V2 1/1] Document: Update DEC spec to remove EDK related contents

[Feng, Bob C]

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

Remove EDK related contents from DEC spec.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Bob Feng 
Cc: Liming Gao 
Cc: Jaben Carsey 
---
 2_dec_file_overview/210_pcd_usage.md | 4 +---
 2_dec_file_overview/25_[includes]_usage.md   | 6 +-
 2_dec_file_overview/26_[guids]_usage.md  | 6 +-
 2_dec_file_overview/27_[protocols]_usage.md  | 6 +-
 2_dec_file_overview/28_[ppis]_usage.md   | 6 +-
 2_dec_file_overview/29_[libraryclasses]_usage.md | 4 +---
 3_edk_ii_dec_file_format/310_pcd_sections.md | 8 ++--
 3_edk_ii_dec_file_format/35_[includes]_sections.md   | 5 +
 3_edk_ii_dec_file_format/39_[libraryclasses]_sections.md | 5 +
 README.md| 1 +
 10 files changed, 11 insertions(+), 40 deletions(-)

diff --git a/2_dec_file_overview/210_pcd_usage.md 
b/2_dec_file_overview/210_pcd_usage.md
index 9b554be..e600db2 100644
--- a/2_dec_file_overview/210_pcd_usage.md
+++ b/2_dec_file_overview/210_pcd_usage.md
@@ -1,9 +1,9 @@
 

[edk2] [Patch V2 1/1] Document: Update Build spec to remove EDK related contents

[Feng, Bob C]

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

Remove EDK related contents from Build spec.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Bob Feng 
Cc: Liming Gao 
Cc: Jaben Carsey 
---
 10_post-build_imagegen_stage_-_flash/103_build_intermediate_images.md |  3 +--
 12_build_changes_and_customizations/README.md |  4 ++--
 4_edk_ii_build_process_overview/42_build_process_overview.md  |  6 
++
 4_edk_ii_build_process_overview/46_file_specifications.md | 20 
+---
 6_quick_start/61_environment_variables.md | 24 
+---
 8_pre-build_autogen_stage/82_auto-generation_process.md   | 76 
+++-
 8_pre-build_autogen_stage/83_auto-generated_code.md   | 43 
+++
 8_pre-build_autogen_stage/85_auto-generated_makefiles.md  | 23 
+--
 9_build_or_make_stage/README.md   |  9 
-
 README.md |  1 +
 appendix_a_variables.md   |  3 +--
 11 files changed, 44 insertions(+), 168 deletions(-)

diff --git 
a/10_post-build_imagegen_stage_-_flash/103_build_intermediate_images.md 
b/10_post-build_imagegen_stage_-_flash/103_build_intermediate_images.md
index 5f5aefc..9253cde 100644
--- a/10_post-build_imagegen_stage_-_flash/103_build_intermediate_images.md
+++ b/10_post-build_imagegen_stage_-_flash/103_build_intermediate_images.md
@@ -1,9 +1,9 @@
 
 
 # 12 Build Changes and Customizations
 
 This chapter deals with customizing a build, including options and settings for
-debugging, using custom tools as well as how to customize EDK component builds
+debugging, using custom tools.
diff --git a/4_edk_ii_build_process_overview/42_build_process_overview.md 
b/4_edk_ii_build_process_overview/42_build_process_overview.md
index d0725d3..17ed278 100644
--- a/4_edk_ii_build_process_overview/42_build_process_overview.md
+++ b/4_edk_ii_build_process_overview/42_build_process_overview.md
@@ -1,9 +1,9 @@
 
 
 ## 4.2 Build Process Overview
 
 Prior to executing a build command, specific system environment variables must
-be initialized: `WORKSPACE`, `EDK_TOOLS_PATH` are required for all builds,
-while `ECP_SOURCE`, `EFI_SOURCE` and `EDK_SOURCE` are only required to build
-EDK II platforms that contain EDK components and EDK libraries. Additionally,
+be initialized: `WORKSPACE`, `EDK_TOOLS_PATH` are required for all builds. 
Additionally,
 the provided EDK II tool set must be present in a directory that is in the
 system environment variable: PATH. The edksetup scripts provided in the root
 directory of the EDK II development tree will set the `WORKSPACE` and
 `EDK_TOOLS_PATH`, as well as modify the system environment variable, PATH to
 ensure that the tools can execute. Refer to "_Build Environment_" for more
diff --git a/4_edk_ii_build_process_overview/46_file_specifications.md 
b/4_edk_ii_build_process_overview/46_file_specifications.md
index a606488..f30f806 100644
--- a/4_edk_ii_build_process_overview/46_file_specifications.md
+++ b/4_edk_ii_build_process_overview/46_file_specifications.md
@@ -1,9 +1,9 @@
 

[edk2] [Patch V2 0/1] Document: Update Build spec to remove EDK related

[Feng, Bob C]

V2:
Update the top level README to add the update history.

Feng, Bob C (1):
  Document: Update Build spec to remove EDK related contents

 10_post-build_imagegen_stage_-_flash/103_build_intermediate_images.md |  3 +--
 12_build_changes_and_customizations/README.md |  4 ++--
 4_edk_ii_build_process_overview/42_build_process_overview.md  |  6 
++
 4_edk_ii_build_process_overview/46_file_specifications.md | 20 
+---
 6_quick_start/61_environment_variables.md | 24 
+---
 8_pre-build_autogen_stage/82_auto-generation_process.md   | 76 
+++-
 8_pre-build_autogen_stage/83_auto-generated_code.md   | 43 
+++
 8_pre-build_autogen_stage/85_auto-generated_makefiles.md  | 23 
+--
 9_build_or_make_stage/README.md   |  9 
-
 README.md |  1 +
 appendix_a_variables.md   |  3 +--
 11 files changed, 44 insertions(+), 168 deletions(-)

-- 
2.20.1.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Does ARM platform produce MP protocol?

[Ard Biesheuvel]

On Wed, 6 Mar 2019 at 06:44, Ni, Ray  wrote:
>
> Ard, Leif,
> I am a bit interested in how ARM platform supports the MP?
> PI Spec defines below protocol but I failed to find a driver in ARM platform 
> producing this protocol.
> Or did I miss anything?
>

No you are right. We don't expose that on ARM, since UEFI only runs on
a single core. Bringing up and taking down cores is done via a
protocol called PSCI, which is implemented by firmware running at a
higher privilege level.

So while it would be possible to implement the MP protocol on top of
PSCI, we haven't identified a use case for it yet. (The OS calls PSCI
directly to boot the secondary cores)

> typedef struct _EFI_MP_SERVICES_PROTOCOL {
> EFI_MP_SERVICES_GET_NUMBER_OF_PROCESSORS GetNumberOfProcessors;
> EFI_MP_SERVICES_GET_PROCESSOR_INFO GetProcessorInfo;
> EFI_MP_SERVICES_STARTUP_ALL_APS StartupAllAPs;
> EFI_MP_SERVICES_STARTUP_THIS_AP StartupThisAP;
> EFI_MP_SERVICES_SWITCH_BSP SwitchBSP;
> EFI_MP_SERVICES_ENABLEDISABLEAP EnableDisableAP;
> EFI_MP_SERVICES_WHOAMI WhoAmI;
> } EFI_MP_SERVICES_PROTOCOL;
>
> Thanks,
> Ray
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] ShellPkg/UefiShellAcpiViewCommandLib: Change the note in uni

[Zhichao Gao]

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

Add DSDT and SSDT description in the help information. Depend
on the implement of UefiShellAcpiViewCommandLib, the "acpiview"
command support to show all present type in the system not only
support the listed type in the help information. So change the
help information of this command.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhichao Gao 
Cc: Ruiyu Ni 
Cc: Jaben Carsey 
Cc: Liming Gao 
---
 .../UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni   | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git 
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni
index 0762eeba53..f2a2400b91 100644
--- 
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni
+++ 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni
@@ -74,10 +74,15 @@
 "  1. The AcpiTable parameter can match any installed table type.\r\n"
 " Tables without specific handling will be displayed as a raw hex dump 
(or\r\n"
 " dumped to a file if -d is used).\r\n"
-"  2. Formatted display and checking is provided for these signature 
types:\r\n"
+"  2. -s option supports to display the specified AcpiTable type that is 
present\r\n"
+" in the system. For normal type AcpiTable, it would display the data of 
the\r\n"
+" AcpiTable and AcpiTable header. The following type may contain header 
type\r\n"
+" other than AcpiTable header. The acual header can refer to the ACPI spec 
6.2\r\n"
+" Extra A. Particual types:\r\n"
 "   APIC  - Multiple APIC Description Table (MADT)\r\n"
 "   BGRT  - Boot Graphics Resource Table\r\n"
 "   DBG2  - Debug Port Table 2\r\n"
+"   DSDT  - Differentiated System Description Table\r\n"
 "   FACP  - Fixed ACPI Description Table (FADT)\r\n"
 "   GTDT  - Generic Timer Description Table\r\n"
 "   IORT  - IO Remapping Table\r\n"
@@ -87,6 +92,7 @@
 "   SLIT  - System Locality Information Table\r\n"
 "   SPCR  - Serial Port Console Redirection Table\r\n"
 "   SRAT  - System Resource Affinity Table\r\n"
+"   SSDT  - Secondary SystemDescription Table\r\n"
 "   XSDT  - Extended System Description Table\r\n"
 " \r\n"
 ".SH STANDARDS\r\n"
-- 
2.16.2.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] ShellPkg/UefiShellAcpiViewCommandLib: Clarify explain of '-s'

[Zhichao Gao]

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

The '-s' option of 'acpiview' do not support multiply
invocation options. So clarify it for users.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhichao Gao 
Cc: Ruiyu Ni 
Cc: Jaben Carsey 
Cc: Liming Gao 
---
 .../UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni| 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git 
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni
index 0762eeba53..634485131b 100644
--- 
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni
+++ 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni
@@ -41,7 +41,8 @@
 ".SH OPTIONS\r\n"
 " \r\n"
 "  -l - Display list of installed ACPI Tables.\r\n"
-"  -s - Display only the specified AcpiTable type.\r\n"
+"  -s - Display only the specified AcpiTable type and only support single\r\n"
+"   invocation option.\r\n"
 " AcpiTable: The required ACPI Table type.\r\n"
 "  -d - Generate a binary file dump of the specified AcpiTable.\r\n"
 "  -c - Consistency checking (enabled by default).\r\n"
-- 
2.16.2.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] FW: [PATCH] BaseTools:Guid.xref will change after increment build

[Fan, ZhijuX]

Hi:

The items to be added to the PcdList are  "(PcdCName, TokenSpaceGuid, SkuName, 
DefaultStore, Dummy5)"
"Dummy5" can distinguish PCDS with the same name, So the results are consistent 
except for the order change.



Any question, please let me know. Thanks.

Best Regards
Fan Zhiju



-Original Message-
From: Feng, Bob C 
Sent: Tuesday, March 5, 2019 5:48 PM
To: Fan, ZhijuX ; edk2-devel@lists.01.org
Cc: Gao, Liming 
Subject: RE: [edk2][PATCH] BaseTools:Guid.xref will change after increment build

Hi Zhiju,

Since you changed a set() to a list,  I think you need to check if the item is 
already in the list before appending it.

Thanks,
Bob

-Original Message-
From: Fan, ZhijuX
Sent: Tuesday, March 5, 2019 4:48 PM
To: edk2-devel@lists.01.org
Cc: Gao, Liming ; Feng, Bob C 
Subject: [edk2][PATCH] BaseTools:Guid.xref will change after increment build

the order of the data may change if set() is used

Cc: Bob Feng 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhiju.Fan 
---
 BaseTools/Source/Python/Workspace/DscBuildData.py | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py 
b/BaseTools/Source/Python/Workspace/DscBuildData.py
index 5e7d7dcd63..342b9472a2 100644
--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
@@ -1654,7 +1654,7 @@ class DscBuildData(PlatformBuildClassObject):
 AvailableSkuIdSet = copy.copy(self.SkuIds)
 
 PcdDict = tdict(True, 4)
-PcdSet = set()
+PcdList = []
 # Find out all possible PCD candidates for self._Arch
 RecordList = self._RawData[Type, self._Arch]
 PcdValueDict = OrderedDict()
@@ -1666,10 +1666,10 @@ class DscBuildData(PlatformBuildClassObject):
 File=self.MetaFile, Line=Dummy5)
 if SkuName in (self.SkuIdMgr.SystemSkuId, TAB_DEFAULT, TAB_COMMON):
 if "." not in TokenSpaceGuid and "[" not in PcdCName:
-PcdSet.add((PcdCName, TokenSpaceGuid, SkuName, Dummy5))
+PcdList.append((PcdCName, TokenSpaceGuid, SkuName,
+ Dummy5))
 PcdDict[Arch, PcdCName, TokenSpaceGuid, SkuName] = Setting
 
-for PcdCName, TokenSpaceGuid, SkuName, Dummy4 in PcdSet:
+for PcdCName, TokenSpaceGuid, SkuName, Dummy4 in PcdList:
 Setting = PcdDict[self._Arch, PcdCName, TokenSpaceGuid, SkuName]
 if Setting is None:
 continue
@@ -2874,7 +2874,7 @@ class DscBuildData(PlatformBuildClassObject):
 # PCD settings for certain ARCH and SKU
 #
 PcdDict = tdict(True, 5)
-PcdSet = set()
+PcdList = []
 RecordList = self._RawData[Type, self._Arch]
 # Find out all possible PCD candidates for self._Arch
 AvailableSkuIdSet = copy.copy(self.SkuIds) @@ -2896,12 +2896,12 @@ 
class DscBuildData(PlatformBuildClassObject):
 EdkLogger.error('build', PARAMETER_INVALID, 'DefaultStores %s 
is not defined in [DefaultStores] section' % DefaultStore,
 File=self.MetaFile, Line=Dummy5)
 if "." not in TokenSpaceGuid and "[" not in PcdCName:
-PcdSet.add((PcdCName, TokenSpaceGuid, SkuName, DefaultStore, 
Dummy5))
+PcdList.append((PcdCName, TokenSpaceGuid, SkuName, 
+ DefaultStore, Dummy5))
 PcdDict[Arch, SkuName, PcdCName, TokenSpaceGuid, DefaultStore] = 
Setting
 
 
 # Remove redundant PCD candidates, per the ARCH and SKU
-for PcdCName, TokenSpaceGuid, SkuName, DefaultStore, Dummy4 in PcdSet:
+for PcdCName, TokenSpaceGuid, SkuName, DefaultStore, Dummy4 in PcdList:
 
 Setting = PcdDict[self._Arch, SkuName, PcdCName, TokenSpaceGuid, 
DefaultStore]
 if Setting is None:
--
2.14.1.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] UefiCpuPkg/Microcode.c: Add verification before calculate CheckSum32

[Ni, Ray]

Pushed @ 219e560c20034843ac9917146c60db99bd01b6f4.

> -Original Message-
> From: Gao, Liming 
> Sent: Wednesday, March 6, 2019 1:38 PM
> To: Ni, Ray ; edk2-devel@lists.01.org
> Cc: Dong, Eric ; Chen, Chen A
> 
> Subject: RE: [edk2] [PATCH] UefiCpuPkg/Microcode.c: Add verification
> before calculate CheckSum32
> 
> This is a bug. I agree to add it into Q1 stable tag.
> 
> Thanks
> Liming
> > -Original Message-
> > From: Ni, Ray
> > Sent: Tuesday, March 5, 2019 9:35 PM
> > To: edk2-devel@lists.01.org; Gao, Liming 
> > Cc: Dong, Eric ; Ni, Ray ;
> > Chen, Chen A 
> > Subject: RE: [edk2] [PATCH] UefiCpuPkg/Microcode.c: Add verification
> > before calculate CheckSum32
> >
> > Liming,
> > Do I need any approval from you side before pushing the commit?
> >
> > Thanks,
> > Ray
> >
> > > -Original Message-
> > > From: edk2-devel  On Behalf Of Ni,
> > > Ray
> > > Sent: Wednesday, March 6, 2019 10:15 AM
> > > To: Chen, Chen A ; edk2-devel@lists.01.org
> > > Cc: Dong, Eric 
> > > Subject: Re: [edk2] [PATCH] UefiCpuPkg/Microcode.c: Add verification
> > > before calculate CheckSum32
> > >
> > > Reviewed-by: Ray Ni 
> > >
> > > > -Original Message-
> > > > From: edk2-devel  On Behalf Of
> > > > Chen A Chen
> > > > Sent: Tuesday, March 5, 2019 8:21 AM
> > > > To: edk2-devel@lists.01.org
> > > > Cc: Dong, Eric 
> > > > Subject: [edk2] [PATCH] UefiCpuPkg/Microcode.c: Add verification
> > > > before calculate CheckSum32
> > > >
> > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1020
> > > >
> > > > Should make sure the TotalSize of Microcode is aligned with 4
> > > > bytes before calling CalculateSum32 function.
> > > >
> > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > Signed-off-by: Chen A Chen 
> > > > Cc: Ray Ni 
> > > > Cc: Eric Dong 
> > > > ---
> > > >  UefiCpuPkg/Library/MpInitLib/Microcode.c | 31
> > > --
> > > > -
> > > >  1 file changed, 20 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > > > b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > > > index 5f9ae22794..643a6f94f4 100644
> > > > --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > > > +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > > > @@ -166,20 +166,29 @@ MicrocodeDetect (
> > > >  //
> > > >  CorrectMicrocode = FALSE;
> > > >
> > > > -//
> > > > -// Save an in-complete CheckSum32 from CheckSum Part1 for
> common
> > > > parts.
> > > > -//
> > > >  if (MicrocodeEntryPoint->DataSize == 0) {
> > > > -  InCompleteCheckSum32 = CalculateSum32 (
> > > > -   (UINT32 *) MicrocodeEntryPoint,
> > > > -   sizeof (CPU_MICROCODE_HEADER) + 2000
> > > > -   );
> > > > +  TotalSize = sizeof (CPU_MICROCODE_HEADER) + 2000;
> > > >  } else {
> > > > -  InCompleteCheckSum32 = CalculateSum32 (
> > > > -   (UINT32 *) MicrocodeEntryPoint,
> > > > -   sizeof (CPU_MICROCODE_HEADER) +
> > > MicrocodeEntryPoint-
> > > > >DataSize
> > > > -   );
> > > > +  TotalSize = sizeof (CPU_MICROCODE_HEADER) +
> > > > + MicrocodeEntryPoint-
> > > > >DataSize;
> > > >  }
> > > > +
> > > > +///
> > > > +/// Check overflow and whether TotalSize is aligned with 4 bytes.
> > > > +///
> > > > +if ( ((UINTN)MicrocodeEntryPoint + TotalSize) > MicrocodeEnd ||
> > > > + (TotalSize & 0x3) != 0
> > > > +   ) {
> > > > +  MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN)
> > > > MicrocodeEntryPoint) + SIZE_1KB);
> > > > +  continue;
> > > > +}
> > > > +
> > > > +//
> > > > +// Save an in-complete CheckSum32 from CheckSum Part1 for
> > > > + common
> > > > parts.
> > > > +//
> > > > +InCompleteCheckSum32 = CalculateSum32 (
> > > > + (UINT32 *) MicrocodeEntryPoint,
> > > > + TotalSize
> > > > + );
> > > >  InCompleteCheckSum32 -= MicrocodeEntryPoint-
> > > > >ProcessorSignature.Uint32;
> > > >  InCompleteCheckSum32 -= MicrocodeEntryPoint->ProcessorFlags;
> > > >  InCompleteCheckSum32 -= MicrocodeEntryPoint->Checksum;
> > > > --
> > > > 2.16.2.windows.1
> > > >
> > > > ___
> > > > edk2-devel mailing list
> > > > edk2-devel@lists.01.org
> > > > https://lists.01.org/mailman/listinfo/edk2-devel
> > > ___
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] Does ARM platform produce MP protocol?

[Ni, Ray]

Ard, Leif,
I am a bit interested in how ARM platform supports the MP?
PI Spec defines below protocol but I failed to find a driver in ARM platform 
producing this protocol.
Or did I miss anything?

typedef struct _EFI_MP_SERVICES_PROTOCOL {
EFI_MP_SERVICES_GET_NUMBER_OF_PROCESSORS GetNumberOfProcessors;
EFI_MP_SERVICES_GET_PROCESSOR_INFO GetProcessorInfo;
EFI_MP_SERVICES_STARTUP_ALL_APS StartupAllAPs;
EFI_MP_SERVICES_STARTUP_THIS_AP StartupThisAP;
EFI_MP_SERVICES_SWITCH_BSP SwitchBSP;
EFI_MP_SERVICES_ENABLEDISABLEAP EnableDisableAP;
EFI_MP_SERVICES_WHOAMI WhoAmI;
} EFI_MP_SERVICES_PROTOCOL;

Thanks,
Ray
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] UefiCpuPkg/Microcode.c: Add verification before calculate CheckSum32

[Gao, Liming]

This is a bug. I agree to add it into Q1 stable tag. 

Thanks
Liming
> -Original Message-
> From: Ni, Ray
> Sent: Tuesday, March 5, 2019 9:35 PM
> To: edk2-devel@lists.01.org; Gao, Liming 
> Cc: Dong, Eric ; Ni, Ray ; Chen, Chen 
> A 
> Subject: RE: [edk2] [PATCH] UefiCpuPkg/Microcode.c: Add verification before 
> calculate CheckSum32
> 
> Liming,
> Do I need any approval from you side before pushing the commit?
> 
> Thanks,
> Ray
> 
> > -Original Message-
> > From: edk2-devel  On Behalf Of Ni, Ray
> > Sent: Wednesday, March 6, 2019 10:15 AM
> > To: Chen, Chen A ; edk2-devel@lists.01.org
> > Cc: Dong, Eric 
> > Subject: Re: [edk2] [PATCH] UefiCpuPkg/Microcode.c: Add verification
> > before calculate CheckSum32
> >
> > Reviewed-by: Ray Ni 
> >
> > > -Original Message-
> > > From: edk2-devel  On Behalf Of Chen A
> > > Chen
> > > Sent: Tuesday, March 5, 2019 8:21 AM
> > > To: edk2-devel@lists.01.org
> > > Cc: Dong, Eric 
> > > Subject: [edk2] [PATCH] UefiCpuPkg/Microcode.c: Add verification
> > > before calculate CheckSum32
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1020
> > >
> > > Should make sure the TotalSize of Microcode is aligned with 4 bytes
> > > before calling CalculateSum32 function.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Chen A Chen 
> > > Cc: Ray Ni 
> > > Cc: Eric Dong 
> > > ---
> > >  UefiCpuPkg/Library/MpInitLib/Microcode.c | 31
> > --
> > > -
> > >  1 file changed, 20 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > > b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > > index 5f9ae22794..643a6f94f4 100644
> > > --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > > +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > > @@ -166,20 +166,29 @@ MicrocodeDetect (
> > >  //
> > >  CorrectMicrocode = FALSE;
> > >
> > > -//
> > > -// Save an in-complete CheckSum32 from CheckSum Part1 for common
> > > parts.
> > > -//
> > >  if (MicrocodeEntryPoint->DataSize == 0) {
> > > -  InCompleteCheckSum32 = CalculateSum32 (
> > > -   (UINT32 *) MicrocodeEntryPoint,
> > > -   sizeof (CPU_MICROCODE_HEADER) + 2000
> > > -   );
> > > +  TotalSize = sizeof (CPU_MICROCODE_HEADER) + 2000;
> > >  } else {
> > > -  InCompleteCheckSum32 = CalculateSum32 (
> > > -   (UINT32 *) MicrocodeEntryPoint,
> > > -   sizeof (CPU_MICROCODE_HEADER) +
> > MicrocodeEntryPoint-
> > > >DataSize
> > > -   );
> > > +  TotalSize = sizeof (CPU_MICROCODE_HEADER) +
> > > + MicrocodeEntryPoint-
> > > >DataSize;
> > >  }
> > > +
> > > +///
> > > +/// Check overflow and whether TotalSize is aligned with 4 bytes.
> > > +///
> > > +if ( ((UINTN)MicrocodeEntryPoint + TotalSize) > MicrocodeEnd ||
> > > + (TotalSize & 0x3) != 0
> > > +   ) {
> > > +  MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN)
> > > MicrocodeEntryPoint) + SIZE_1KB);
> > > +  continue;
> > > +}
> > > +
> > > +//
> > > +// Save an in-complete CheckSum32 from CheckSum Part1 for common
> > > parts.
> > > +//
> > > +InCompleteCheckSum32 = CalculateSum32 (
> > > + (UINT32 *) MicrocodeEntryPoint,
> > > + TotalSize
> > > + );
> > >  InCompleteCheckSum32 -= MicrocodeEntryPoint-
> > > >ProcessorSignature.Uint32;
> > >  InCompleteCheckSum32 -= MicrocodeEntryPoint->ProcessorFlags;
> > >  InCompleteCheckSum32 -= MicrocodeEntryPoint->Checksum;
> > > --
> > > 2.16.2.windows.1
> > >
> > > ___
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] UefiCpuPkg/Microcode.c: Add verification before calculate CheckSum32

[Ni, Ray]

Liming,
Do I need any approval from you side before pushing the commit?

Thanks,
Ray

> -Original Message-
> From: edk2-devel  On Behalf Of Ni, Ray
> Sent: Wednesday, March 6, 2019 10:15 AM
> To: Chen, Chen A ; edk2-devel@lists.01.org
> Cc: Dong, Eric 
> Subject: Re: [edk2] [PATCH] UefiCpuPkg/Microcode.c: Add verification
> before calculate CheckSum32
> 
> Reviewed-by: Ray Ni 
> 
> > -Original Message-
> > From: edk2-devel  On Behalf Of Chen A
> > Chen
> > Sent: Tuesday, March 5, 2019 8:21 AM
> > To: edk2-devel@lists.01.org
> > Cc: Dong, Eric 
> > Subject: [edk2] [PATCH] UefiCpuPkg/Microcode.c: Add verification
> > before calculate CheckSum32
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1020
> >
> > Should make sure the TotalSize of Microcode is aligned with 4 bytes
> > before calling CalculateSum32 function.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Chen A Chen 
> > Cc: Ray Ni 
> > Cc: Eric Dong 
> > ---
> >  UefiCpuPkg/Library/MpInitLib/Microcode.c | 31
> --
> > -
> >  1 file changed, 20 insertions(+), 11 deletions(-)
> >
> > diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > index 5f9ae22794..643a6f94f4 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > @@ -166,20 +166,29 @@ MicrocodeDetect (
> >  //
> >  CorrectMicrocode = FALSE;
> >
> > -//
> > -// Save an in-complete CheckSum32 from CheckSum Part1 for common
> > parts.
> > -//
> >  if (MicrocodeEntryPoint->DataSize == 0) {
> > -  InCompleteCheckSum32 = CalculateSum32 (
> > -   (UINT32 *) MicrocodeEntryPoint,
> > -   sizeof (CPU_MICROCODE_HEADER) + 2000
> > -   );
> > +  TotalSize = sizeof (CPU_MICROCODE_HEADER) + 2000;
> >  } else {
> > -  InCompleteCheckSum32 = CalculateSum32 (
> > -   (UINT32 *) MicrocodeEntryPoint,
> > -   sizeof (CPU_MICROCODE_HEADER) +
> MicrocodeEntryPoint-
> > >DataSize
> > -   );
> > +  TotalSize = sizeof (CPU_MICROCODE_HEADER) +
> > + MicrocodeEntryPoint-
> > >DataSize;
> >  }
> > +
> > +///
> > +/// Check overflow and whether TotalSize is aligned with 4 bytes.
> > +///
> > +if ( ((UINTN)MicrocodeEntryPoint + TotalSize) > MicrocodeEnd ||
> > + (TotalSize & 0x3) != 0
> > +   ) {
> > +  MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN)
> > MicrocodeEntryPoint) + SIZE_1KB);
> > +  continue;
> > +}
> > +
> > +//
> > +// Save an in-complete CheckSum32 from CheckSum Part1 for common
> > parts.
> > +//
> > +InCompleteCheckSum32 = CalculateSum32 (
> > + (UINT32 *) MicrocodeEntryPoint,
> > + TotalSize
> > + );
> >  InCompleteCheckSum32 -= MicrocodeEntryPoint-
> > >ProcessorSignature.Uint32;
> >  InCompleteCheckSum32 -= MicrocodeEntryPoint->ProcessorFlags;
> >  InCompleteCheckSum32 -= MicrocodeEntryPoint->Checksum;
> > --
> > 2.16.2.windows.1
> >
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support

[Ashish Singhal]

Hi Hao,

That is right. Also, I have addressed your comments and have submitted v2 patch.

Thanks
Ashish

-Original Message-
From: Wu, Hao A  
Sent: Tuesday, March 5, 2019 8:01 PM
To: Ashish Singhal ; edk2-devel@lists.01.org
Cc: eug...@hp.com
Subject: RE: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support

Hi Ashish,

One thing to confirm, for the updated checks within
SdMmcPciHcDriverBindingStart():

>  if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
>   Private->Capability[Slot].SysBus64V3 == 0) ||
>  (Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400 &&
>   Private->Capability[Slot].SysBus64V3 == 0) ||
>  (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410 &&
>   Private->Capability[Slot].SysBus64V4 == 0)) {
>Support64BitDma = FALSE;
>  }

When the SDHC with version greater than 4.10, the check is only performed 
against the 'SysBus64V4' bit. My understanding of the purpose is that:

1. For SDHC with version 4.00, the support of V3 mode and V4 mode of 64-bit 
System Address are reflect by bit 'SysBus64V3'. Thus, I can infer that the 
possible support case is both or neither.

2. The spec states that SDHC with version greater than 4.10 divides the V3 mode 
and V4 mode support into 2 bits (SysBus64V3, SysBus64V4) so that the
V3 mode support can be optional.

So based on 1 & 2, we do not even bother to check the 'SysBus64V3' bit when HC 
version >= 4.10. Is that right?


I verified the patch on SDHC version 3.00 with 64-bit System Address Support. 
Hope more configurations are available for testing on Eugene's side.


Besides, some minor comments below:

> -Original Message-
> From: Ashish Singhal [mailto:ashishsin...@nvidia.com]
> Sent: Saturday, March 02, 2019 2:30 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A; eug...@hp.com; Ashish Singhal
> Subject: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support

Please help to add the below Bugzilla tracker for reference:
https://bugzilla.tianocore.org/show_bug.cgi?id=1583

I have updated the above tracker to match the purpose of the proposed patch.


> 
> Driver was supporting only 32b DMA support for V3 controllers. Add 
> support for 64b DMA as well for completeness.
> 
> For V4.0 64b support, driver was looking at incorrect capability 
> register bit. Fix for that is present as well.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ashish Singhal 
> ---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c |  10 +-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h |   6 +-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   | 199
> ++---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h   |  29 ++-
>  4 files changed, 170 insertions(+), 74 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> index b474f8d..9b7b88c 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> @@ -6,7 +6,7 @@
> 
>It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
> 
> -  Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> +  Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
>Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of 
> the BSD License @@ -666,8 +666,12 @@ SdMmcPciHcDriverBindingStart (
>  // If any of the slots does not support 64b system bus
>  // do not enable 64b DMA in the PCI layer.
>  //
> -if (Private->Capability[Slot].SysBus64V3 == 0 &&
> -Private->Capability[Slot].SysBus64V4 == 0) {
> +if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
> + Private->Capability[Slot].SysBus64V3 == 0) ||
> +(Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400 &&
> + Private->Capability[Slot].SysBus64V3 == 0) ||
> +(Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410 &&
> + Private->Capability[Slot].SysBus64V4 == 0)) {
>Support64BitDma = FALSE;
>  }
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> index 1bb701a..68d8a5c 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> @@ -2,7 +2,7 @@
> 
>Provides some data structure definitions used by the SD/MMC host 
> controller driver.
> 
> -Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> +Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
>  Copyright (c) 2015, Intel Corporation. All rights reserved.  This 
> program and the accompanying materials  are licensed and made 
> available under the terms and conditions of the BSD License @@ -145,13 
> +145,15 @@ typedef struct {

Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support

[Wu, Hao A]

Hi Ashish,

One thing to confirm, for the updated checks within
SdMmcPciHcDriverBindingStart():

>  if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
>   Private->Capability[Slot].SysBus64V3 == 0) ||
>  (Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400 &&
>   Private->Capability[Slot].SysBus64V3 == 0) ||
>  (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410 &&
>   Private->Capability[Slot].SysBus64V4 == 0)) {
>Support64BitDma = FALSE;
>  }

When the SDHC with version greater than 4.10, the check is only performed
against the 'SysBus64V4' bit. My understanding of the purpose is that:

1. For SDHC with version 4.00, the support of V3 mode and V4 mode of
64-bit System Address are reflect by bit 'SysBus64V3'. Thus, I can infer
that the possible support case is both or neither.

2. The spec states that SDHC with version greater than 4.10 divides the V3
mode and V4 mode support into 2 bits (SysBus64V3, SysBus64V4) so that the
V3 mode support can be optional.

So based on 1 & 2, we do not even bother to check the 'SysBus64V3' bit
when HC version >= 4.10. Is that right?


I verified the patch on SDHC version 3.00 with 64-bit System Address
Support. Hope more configurations are available for testing on Eugene's
side.


Besides, some minor comments below:

> -Original Message-
> From: Ashish Singhal [mailto:ashishsin...@nvidia.com]
> Sent: Saturday, March 02, 2019 2:30 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A; eug...@hp.com; Ashish Singhal
> Subject: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA
> Support

Please help to add the below Bugzilla tracker for reference:
https://bugzilla.tianocore.org/show_bug.cgi?id=1583

I have updated the above tracker to match the purpose of the proposed
patch.


> 
> Driver was supporting only 32b DMA support for V3 controllers. Add
> support for 64b DMA as well for completeness.
> 
> For V4.0 64b support, driver was looking at incorrect capability
> register bit. Fix for that is present as well.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ashish Singhal 
> ---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c |  10 +-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h |   6 +-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   | 199
> ++---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h   |  29 ++-
>  4 files changed, 170 insertions(+), 74 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> index b474f8d..9b7b88c 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> @@ -6,7 +6,7 @@
> 
>It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
> 
> -  Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> +  Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
>Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the BSD
> License
> @@ -666,8 +666,12 @@ SdMmcPciHcDriverBindingStart (
>  // If any of the slots does not support 64b system bus
>  // do not enable 64b DMA in the PCI layer.
>  //
> -if (Private->Capability[Slot].SysBus64V3 == 0 &&
> -Private->Capability[Slot].SysBus64V4 == 0) {
> +if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
> + Private->Capability[Slot].SysBus64V3 == 0) ||
> +(Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400 &&
> + Private->Capability[Slot].SysBus64V3 == 0) ||
> +(Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410 &&
> + Private->Capability[Slot].SysBus64V4 == 0)) {
>Support64BitDma = FALSE;
>  }
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> index 1bb701a..68d8a5c 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> @@ -2,7 +2,7 @@
> 
>Provides some data structure definitions used by the SD/MMC host
> controller driver.
> 
> -Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> +Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
>  Copyright (c) 2015, Intel Corporation. All rights reserved.
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
> @@ -145,13 +145,15 @@ typedef struct {
>EFI_PHYSICAL_ADDRESSDataPhy;
>VOID*DataMap;
>SD_MMC_HC_TRANSFER_MODE Mode;
> +  SD_MMC_HC_ADMA_LEGTHLength;

Maybe:
SD_MMC_HC_ADMA_LENGTH_MODEAdmaLengthMode;

is better to avoid confusion.


> 
>EFI_EVENT   

Re: [edk2] [PATCH 0/3] Xen PCI passthrough fixes

[Konrad Rzeszutek Wilk]

On Mon, Mar 04, 2019 at 03:04:05PM +, Igor Druzhinin wrote:
> Igor Druzhinin (3):

Could you also CC xen-devel please?
>   OvmfPkg/XenSupport: remove usage of prefetchable PCI host bridge
> aperture
>   OvmfPkg/XenSupport: use a correct PCI host bridge aperture for BAR64
>   OvmfPkg/XenSupport: turn off address decoding before BAR sizing
> 
>  OvmfPkg/Library/PciHostBridgeLib/XenSupport.c | 44 
> ++-
>  1 file changed, 37 insertions(+), 7 deletions(-)
> 
> -- 
> 2.7.4
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] EmulatorPkg/Sec: Don't install TemporaryRamSupport PPI

[Ni, Ray]

> -Original Message-
> From: edk2-devel  On Behalf Of Jordan
> Justen
> Sent: Wednesday, March 6, 2019 5:48 AM
> To: Ni, Ray ; edk2-devel@lists.01.org
> Cc: Kinney, Michael D ; Laszlo Ersek
> 
> Subject: Re: [edk2] [PATCH] EmulatorPkg/Sec: Don't install
> TemporaryRamSupport PPI
> 
> On 2019-03-04 18:37:03, Ni, Ray wrote:
> >
> > > -Original Message-
> > > From: edk2-devel  On Behalf Of
> > > Jordan Justen
> > > Sent: Sunday, March 3, 2019 5:45 AM
> > > To: Ni, Ray ; edk2-devel@lists.01.org
> > > Subject: Re: [edk2] [PATCH] EmulatorPkg/Sec: Don't install
> > > TemporaryRamSupport PPI
> > >
> > > On 2019-03-02 04:11:11, Ray Ni wrote:
> > > >
> > > > So this patch removes TemporaryRamSupport PPI implementation from
> > > > EmulatorPkg/Sec module to fix the boot failure when using GCC5.
> > >
> > > I don't think we should just hide the known bug with the
> > > TemporaryRamSupport PPI implementation in the PEI Core.
> > >
> > > I would agree that we should make this change after addressing that.
> >
> > Jordan,
> > I have a goal to replace Nt32 with EmulatorPkg before Q2 stable tag release.
> > I understand we need more discussions on how to fix the PeiCore bug.
> > So I don't want this blocks the Nt32 deletion.
> 
> Given the choice of fixing a known bug in PEI Core relating to the PI spec
> TemporaryRamSupport PPI, or deleting Nt32, I would choose fixing the bug.
> 
> But, why not fix the bug, and then make this change?
> 
Mike, Liming, Jian,
Can you please review Jordan's patch to fix the code calling 
TemporaryRamSupport PPI
in PeiCore with some assembly code?
Personally I don't like the assembly code.
But you are the experts and maintainers, I am ok if you give a R-b.

Ard, Leif,
Jordan is fixing a bug in PeiCore but only fixing x86. I am not sure whether 
ARM code
also contains such TemporaryRamSupport PPI issue.

Thanks,
Ray
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] UefiCpuPkg/Microcode.c: Add verification before calculate CheckSum32

[Ni, Ray]

Reviewed-by: Ray Ni 

> -Original Message-
> From: edk2-devel  On Behalf Of Chen A
> Chen
> Sent: Tuesday, March 5, 2019 8:21 AM
> To: edk2-devel@lists.01.org
> Cc: Dong, Eric 
> Subject: [edk2] [PATCH] UefiCpuPkg/Microcode.c: Add verification before
> calculate CheckSum32
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1020
> 
> Should make sure the TotalSize of Microcode is aligned with 4 bytes
> before calling CalculateSum32 function.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chen A Chen 
> Cc: Ray Ni 
> Cc: Eric Dong 
> ---
>  UefiCpuPkg/Library/MpInitLib/Microcode.c | 31 --
> -
>  1 file changed, 20 insertions(+), 11 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> index 5f9ae22794..643a6f94f4 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> @@ -166,20 +166,29 @@ MicrocodeDetect (
>  //
>  CorrectMicrocode = FALSE;
> 
> -//
> -// Save an in-complete CheckSum32 from CheckSum Part1 for common
> parts.
> -//
>  if (MicrocodeEntryPoint->DataSize == 0) {
> -  InCompleteCheckSum32 = CalculateSum32 (
> -   (UINT32 *) MicrocodeEntryPoint,
> -   sizeof (CPU_MICROCODE_HEADER) + 2000
> -   );
> +  TotalSize = sizeof (CPU_MICROCODE_HEADER) + 2000;
>  } else {
> -  InCompleteCheckSum32 = CalculateSum32 (
> -   (UINT32 *) MicrocodeEntryPoint,
> -   sizeof (CPU_MICROCODE_HEADER) + 
> MicrocodeEntryPoint-
> >DataSize
> -   );
> +  TotalSize = sizeof (CPU_MICROCODE_HEADER) + MicrocodeEntryPoint-
> >DataSize;
>  }
> +
> +///
> +/// Check overflow and whether TotalSize is aligned with 4 bytes.
> +///
> +if ( ((UINTN)MicrocodeEntryPoint + TotalSize) > MicrocodeEnd ||
> + (TotalSize & 0x3) != 0
> +   ) {
> +  MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN)
> MicrocodeEntryPoint) + SIZE_1KB);
> +  continue;
> +}
> +
> +//
> +// Save an in-complete CheckSum32 from CheckSum Part1 for common
> parts.
> +//
> +InCompleteCheckSum32 = CalculateSum32 (
> + (UINT32 *) MicrocodeEntryPoint,
> + TotalSize
> + );
>  InCompleteCheckSum32 -= MicrocodeEntryPoint-
> >ProcessorSignature.Uint32;
>  InCompleteCheckSum32 -= MicrocodeEntryPoint->ProcessorFlags;
>  InCompleteCheckSum32 -= MicrocodeEntryPoint->Checksum;
> --
> 2.16.2.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 3/3] MdePkg/BaseSynchronizationLib: Remove inline X86 assembly code

[Andrew Fish via edk2-devel]

> On Mar 5, 2019, at 1:32 PM, Gao, Liming  wrote:
> 
> Andrew:
>  BZ 1163 is to remove inline X86 assembly code in C source file. But, this 
> patch is wrong. I have gave my comments to update this patch.
> 

Why do we want to remove inline X86 assembly. As I mentioned it will lead to 
larger binaries, slower binaries, and less optimized code.

For example take this simple inline assembly function:

VOID
EFIAPI
CpuBreakpoint (
  VOID
  )
{
  __asm__ __volatile__ ("int $3");
}

Today with clang LTO turned on calling CpuBreakpoint() looks like this from the 
callers point of view. 

a.out[0x1fa5] <+6>:  cc  int3   

But if we move that to NASM:

a.out[0x1fa6] <+7>:  e8 07 00 00 00  calll  0x1fb2; 
CpuBreakpoint

plus:
a.out`CpuBreakpoint:
a.out[0x1fb2] <+0>: cc int3   
a.out[0x1fb3] <+1>: c3 retl   

And there is also an extra push and pop on the stack. The other issue is the 
call to the function that is unknown to the compiler will act like a 
_ReadWriteBarrier (Yikes I see _ReadWriteBarrier is deprecated in VC++ 2017). 
Is the depreciation of some of these intrinsics in VC++ driving the removal of 
inline assembly? For GCC inline assembly works great for local compile, and for 
clang LTO it works in entire program optimization.

The LTO bitcode includes inline assembly and constraints so that the optimizer 
knows what to do so it can get optimized just like the abstract bitcode during 
the Link Time Optimization. 

This is CpuBreakpoint():
; Function Attrs: noinline nounwind optnone ssp uwtable
define void @CpuBreakpoint() #0 {
  call void asm sideeffect "int $$3", "~{dirflag},~{fpsr},~{flags}"() #2, 
!srcloc !3
  ret void
}

This is AsmReadMsr64():
; Function Attrs: noinline nounwind optnone ssp uwtable
define i64 @AsmReadMsr64(i32) #0 {
  %2 = alloca i32, align 4
  %3 = alloca i64, align 8
  store i32 %0, i32* %2, align 4
  %4 = load i32, i32* %2, align 4
  %5 = call i64 asm sideeffect "rdmsr", 
"=A,{cx},~{dirflag},~{fpsr},~{flags}"(i32 %4) #2, !srcloc !4
  store i64 %5, i64* %3, align 8
  %6 = load i64, i64* %3, align 8
  ret i64 %6
}


I agree it make sense to remove .S and .asm files and only have .nasm files. 

Thanks,

Andrew Fish

PS For the Xcode clang since it emits frame pointers by default we need to add 
the extra 4 bytes to save the assembly functions so the stack can get unwound. 

a.out`CpuBreakpoint:
a.out[0x1f99] <+0>: 55 pushl  %ebp
a.out[0x1f9a] <+1>: 89 e5  movl   %esp, %ebp
a.out[0x1f9c] <+3>: cc int3   
a.out[0x1f9d] <+4>: 5d popl   %ebp
a.out[0x1f9e] <+5>: c3 retl   

So breakpoint goes from 1 byte to 11 bytes if we get rid of the intrinsics. 

>  The change is to reduce the duplicated implementation. It will be good on 
> the code maintain. Recently, one patch has to update .c and .nasm both. 
> Please see 
> https://lists.01.org/pipermail/edk2-devel/2019-February/037165.html 
> . Beside 
> this change, I propose to remove all GAS assembly code for IA32 and X64 arch. 
> After those change, the patch owner will collect the impact of the image 
> size. 
> 
> Thanks
> Liming

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] EmulatorPkg/Sec: Don't install TemporaryRamSupport PPI

[Jordan Justen]

On 2019-03-04 18:37:03, Ni, Ray wrote:
> 
> > -Original Message-
> > From: edk2-devel  On Behalf Of Jordan
> > Justen
> > Sent: Sunday, March 3, 2019 5:45 AM
> > To: Ni, Ray ; edk2-devel@lists.01.org
> > Subject: Re: [edk2] [PATCH] EmulatorPkg/Sec: Don't install
> > TemporaryRamSupport PPI
> > 
> > On 2019-03-02 04:11:11, Ray Ni wrote:
> > >
> > > So this patch removes TemporaryRamSupport PPI implementation from
> > > EmulatorPkg/Sec module to fix the boot failure when using GCC5.
> > 
> > I don't think we should just hide the known bug with the
> > TemporaryRamSupport PPI implementation in the PEI Core.
> > 
> > I would agree that we should make this change after addressing that.
> 
> Jordan,
> I have a goal to replace Nt32 with EmulatorPkg before Q2 stable tag release.
> I understand we need more discussions on how to fix the PeiCore bug.
> So I don't want this blocks the Nt32 deletion.

Given the choice of fixing a known bug in PEI Core relating to the PI
spec TemporaryRamSupport PPI, or deleting Nt32, I would choose fixing
the bug.

But, why not fix the bug, and then make this change?

> And in my opinion, the TemporaryRamSupport PPI that requires being called
> from assembly code is a design bug.

I agree. It's unfortunate, but it is part of the PI spec, and unlikely
to be changed very soon.

Even if a new PPI is added, I would guess that the old PPI might
remain in the PI spec. Maybe it would be marked as deprecated at some
point and then removed later?

I am no fan adding assembly language unnecessarily. But, I can't see a
way around it in this case.

> I suggest to change the PI spec instead of changing PeiCore by introducing
> more assembly code to hide this design bug.

I did prototype a TemporaryRamSupport2 PPI under:

https://github.com/jljusten/edk2/tree/temp-ram-support2

My prototype still used assembly, as I don't think the
TemporaryRamSupport PPI would be quickly removed from the PI spec.
(And, therefore, I think edk2's PEI Core should support it without
known bugs.)

But, I think the new TemporaryRamSupport2 PPI should be possible to
safely implement in C code for both the PEI Core and in the PPI
implementation.

-Jordan
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 3/3] MdePkg/BaseSynchronizationLib: Remove inline X86 assembly code

[Kinney, Michael D]

Liming,

I agree .nasm files replacing .S/.asm files.  But the use of inline
assembly in C files for some primitives does produce much smaller/faster
code.

I would prefer that this BZ identify the specific functions that you
think would provide better maintainability with no impact to size
or performance.

Thanks,

Mike

> -Original Message-
> From: Gao, Liming
> Sent: Tuesday, March 5, 2019 1:33 PM
> To: af...@apple.com; Zhang, Shenglei
> 
> Cc: edk2-devel-01 ; Kinney,
> Michael D 
> Subject: RE: [edk2] [PATCH 3/3]
> MdePkg/BaseSynchronizationLib: Remove inline X86
> assembly code
> 
> Andrew:
>   BZ 1163 is to remove inline X86 assembly code in C
> source file. But, this patch is wrong. I have gave my
> comments to update this patch.
> 
>   The change is to reduce the duplicated
> implementation. It will be good on the code maintain.
> Recently, one patch has to update .c and .nasm both.
> Please see https://lists.01.org/pipermail/edk2-
> devel/2019-February/037165.html. Beside this change, I
> propose to remove all GAS assembly code for IA32 and
> X64 arch. After those change, the patch owner will
> collect the impact of the image size.
> 
> Thanks
> Liming
> > -Original Message-
> > From: af...@apple.com [mailto:af...@apple.com]
> > Sent: Tuesday, March 5, 2019 12:58 PM
> > To: Zhang, Shenglei 
> > Cc: edk2-devel-01 ; Kinney,
> Michael D ; Gao, Liming
> 
> > Subject: Re: [edk2] [PATCH 3/3]
> MdePkg/BaseSynchronizationLib: Remove inline X86
> assembly code
> >
> > Shenglei,
> >
> > I was confused by the names of these patches. From a
> C point of view this is inline assembly:
> >
> > VOID
> > EFIAPI
> > CpuBreakpoint (
> > VOID
> > )
> > {
> > __asm__ __volatile__ ("int $3");
> > }
> >
> > These patches seem to be removing GAS and MASM
> assembly files, but not the inline assembly *.c files?
> >
> > We don't want to remove the inline assembly from the
> BaseLib as that could have size, performance, and
> compiler optimization impact.
> >
> > For example CpuBreakpoint() for clang with LTO would
> end up inlining a single byte. For i385 a call to
> assembler would be 5 bytes at each
> > call location plus the overhead of the function. So
> that is a size increase and a performance overhead. For
> a C complier calling an assembly
> > function is a serializing event an that can restrict
> optimizations. Thus having some limited inline assembly
> support is very useful.
> >
> > Thanks,
> >
> > Andrew Fish
> >
> > > On Mar 4, 2019, at 5:40 PM, Shenglei Zhang
>  wrote:
> > >
> > > MdePkg BaseSynchronizationLib still uses the inline
> X86 assembly code
> > > in C code files.It should be updated to consume
> nasm only.
> > > https://bugzilla.tianocore.org/show_bug.cgi?id=1163
> > >
> > > Cc: Michael D Kinney 
> > > Cc: Liming Gao 
> > > Contributed-under: TianoCore Contribution Agreement
> 1.1
> > > Signed-off-by: Shenglei Zhang
> 
> > > ---
> > >
> .../Library/BaseSynchronizationLib/BaseSynchronizationL
> ib.inf   | 2 --
> > > 1 file changed, 2 deletions(-)
> > >
> > > diff --git
> a/MdePkg/Library/BaseSynchronizationLib/BaseSynchroniza
> tionLib.inf
> >
> b/MdePkg/Library/BaseSynchronizationLib/BaseSynchroniza
> tionLib.inf
> > > index 32414b29fa..719dc1938d 100755
> > > ---
> a/MdePkg/Library/BaseSynchronizationLib/BaseSynchroniza
> tionLib.inf
> > > +++
> b/MdePkg/Library/BaseSynchronizationLib/BaseSynchroniza
> tionLib.inf
> > > @@ -75,13 +75,11 @@
> > >
> > > [Sources.ARM]
> > > Synchronization.c
> > > -  Arm/Synchronization.asm   | RVCT
> > > Arm/Synchronization.S | GCC
> > >
> > > [Sources.AARCH64]
> > > Synchronization.c
> > > AArch64/Synchronization.S | GCC
> > > -  AArch64/Synchronization.asm   | MSFT
> > >
> > > [Packages]
> > > MdePkg/MdePkg.dec
> > > --
> > > 2.18.0.windows.1
> > >
> > > ___
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 3/3] MdePkg/BaseSynchronizationLib: Remove inline X86 assembly code

[Gao, Liming]

Andrew:
  BZ 1163 is to remove inline X86 assembly code in C source file. But, this 
patch is wrong. I have gave my comments to update this patch.

  The change is to reduce the duplicated implementation. It will be good on the 
code maintain. Recently, one patch has to update .c and .nasm both. Please see 
https://lists.01.org/pipermail/edk2-devel/2019-February/037165.html. Beside 
this change, I propose to remove all GAS assembly code for IA32 and X64 arch. 
After those change, the patch owner will collect the impact of the image size. 

Thanks
Liming
> -Original Message-
> From: af...@apple.com [mailto:af...@apple.com]
> Sent: Tuesday, March 5, 2019 12:58 PM
> To: Zhang, Shenglei 
> Cc: edk2-devel-01 ; Kinney, Michael D 
> ; Gao, Liming 
> Subject: Re: [edk2] [PATCH 3/3] MdePkg/BaseSynchronizationLib: Remove inline 
> X86 assembly code
> 
> Shenglei,
> 
> I was confused by the names of these patches. From a C point of view this is 
> inline assembly:
> 
> VOID
> EFIAPI
> CpuBreakpoint (
> VOID
> )
> {
> __asm__ __volatile__ ("int $3");
> }
> 
> These patches seem to be removing GAS and MASM assembly files, but not the 
> inline assembly *.c files?
> 
> We don't want to remove the inline assembly from the BaseLib as that could 
> have size, performance, and compiler optimization impact.
> 
> For example CpuBreakpoint() for clang with LTO would end up inlining a single 
> byte. For i385 a call to assembler would be 5 bytes at each
> call location plus the overhead of the function. So that is a size increase 
> and a performance overhead. For a C complier calling an assembly
> function is a serializing event an that can restrict optimizations. Thus 
> having some limited inline assembly support is very useful.
> 
> Thanks,
> 
> Andrew Fish
> 
> > On Mar 4, 2019, at 5:40 PM, Shenglei Zhang  wrote:
> >
> > MdePkg BaseSynchronizationLib still uses the inline X86 assembly code
> > in C code files.It should be updated to consume nasm only.
> > https://bugzilla.tianocore.org/show_bug.cgi?id=1163
> >
> > Cc: Michael D Kinney 
> > Cc: Liming Gao 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Shenglei Zhang 
> > ---
> > .../Library/BaseSynchronizationLib/BaseSynchronizationLib.inf   | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git 
> > a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> > index 32414b29fa..719dc1938d 100755
> > --- a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> > +++ b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> > @@ -75,13 +75,11 @@
> >
> > [Sources.ARM]
> > Synchronization.c
> > -  Arm/Synchronization.asm   | RVCT
> > Arm/Synchronization.S | GCC
> >
> > [Sources.AARCH64]
> > Synchronization.c
> > AArch64/Synchronization.S | GCC
> > -  AArch64/Synchronization.asm   | MSFT
> >
> > [Packages]
> > MdePkg/MdePkg.dec
> > --
> > 2.18.0.windows.1
> >
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 3/3] MdePkg/BaseSynchronizationLib: Remove inline X86 assembly code

[Andrew Fish via edk2-devel]

Shenglei,

I was confused by the names of these patches. From a C point of view this is 
inline assembly:

VOID
EFIAPI
CpuBreakpoint (
VOID
)
{
__asm__ __volatile__ ("int $3");
}

These patches seem to be removing GAS and MASM assembly files, but not the 
inline assembly *.c files?

We don't want to remove the inline assembly from the BaseLib as that could have 
size, performance, and compiler optimization impact. 

For example CpuBreakpoint() for clang with LTO would end up inlining a single 
byte. For i385 a call to assembler would be 5 bytes at each call location plus 
the overhead of the function. So that is a size increase and a performance 
overhead. For a C complier calling an assembly function is a serializing event 
an that can restrict optimizations. Thus having some limited inline assembly 
support is very useful. 

Thanks,

Andrew Fish

> On Mar 4, 2019, at 5:40 PM, Shenglei Zhang  wrote:
> 
> MdePkg BaseSynchronizationLib still uses the inline X86 assembly code
> in C code files.It should be updated to consume nasm only.
> https://bugzilla.tianocore.org/show_bug.cgi?id=1163
> 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Shenglei Zhang 
> ---
> .../Library/BaseSynchronizationLib/BaseSynchronizationLib.inf   | 2 --
> 1 file changed, 2 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf 
> b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> index 32414b29fa..719dc1938d 100755
> --- a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> +++ b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> @@ -75,13 +75,11 @@
> 
> [Sources.ARM]
> Synchronization.c
> -  Arm/Synchronization.asm   | RVCT
> Arm/Synchronization.S | GCC
> 
> [Sources.AARCH64]
> Synchronization.c
> AArch64/Synchronization.S | GCC
> -  AArch64/Synchronization.asm   | MSFT
> 
> [Packages]
> MdePkg/MdePkg.dec
> -- 
> 2.18.0.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [RFC] Change EDK II to BSD+Patent License

[Kevin D Davis]

Mr. Kinney,
Wow.  Of all the many licenses I’ve read, this one takes the cake at having the 
highest confusion to words ratio for my reading comprehension level.  I’ll 
admit my level is a lot lower than some on this reflector. 
Maybe if I knew the intent of this license when reading it I would find it 
clear.  Is there an opinion about the intentions around these two questions?
A) am I granting patent rights if I add patentable/patented code?
B) do I need to get a patent license from all of the copyright holders to use 
this code for technology covered by their code?


Thanks,Kevin  

  



On Tue, Mar 5, 2019 at 1:10 PM -0600, "Kinney, Michael D" 
 wrote:










Hello,

This RFC follows up on the proposal from Mark Doran to change the 
EDK II Project to a BSD+Patent License.

https://lists.01.org/pipermail/edk2-devel/2019-February/036260.html

The review period for this license change is 30 days.  If there is no
unresolved feedback on April 9, 2019, then commits of the license change
patches will begin on April 9, 2019.  

  ** Please provide feedback on the proposal by Monday April 8, 2019. **

Feedback can be sent to edk2-devel@lists.01.org, the EDK II community
manager or any of the EDK II stewards.

  * Stephano CetolaCommunity Manager
  * Leif Lindholm   Steward
  * Andrew Fish  Steward
  * Laszlo Ersek   Steward
  * Michael KinneySteward

The goal is to convert all of the files in the edk2 repository that are
currently covered by the BSD 2-Clause License and the TianoCore
Contribution Agreement to a BSD+Patent License.  

I will be following up with pointers to public GitHub branches that
contain the set of changes to the edk2 repository for review.

The proposal is to perform this change to edk2/master in the steps listed
below. The license change will not be applied to any of the other existing
branches in the edk2 repository.

1) Add a License-History.txt file to the root of the edk2 repository that
   contains the BSD 2-Clause License and the TianoCore Contribution
   Agreement along with the details on the license change to BSD+Patent.

2) Change all files currently covered by a BSD 2-Clause license and the 
   TianoCore Contribution Agreement to a BSD+Patent license and add an 
   SPDX-License-Identifier statement.  The link to the BSD+Patent license
   and the text for file headers is listed below. 

   https://opensource.org/licenses/BSDplusPatent

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

   Redistribution and use in source and binary forms, with or without
   modification, are permitted provided that the following conditions are met:

   1. Redistributions of source code must retain the above copyright notice,
  this list of conditions and the following disclaimer.

   2. Redistributions in binary form must reproduce the above copyright notice,
  this list of conditions and the following disclaimer in the documentation
  and/or other materials provided with the distribution.

   Subject to the terms and conditions of this license, each copyright holder
   and contributor hereby grants to those receiving rights under this license
   a perpetual, worldwide, non-exclusive, no-charge, royalty-free, irrevocable
   (except for failure to satisfy the conditions of this license) patent
   license to make, have made, use, offer to sell, sell, import, and otherwise
   transfer this software, where such license applies only to those patent
   claims, already acquired or hereafter acquired, licensable by such copyright
   holder or contributor that are necessarily infringed by:

   (a) their Contribution(s) (the licensed copyrights of copyright holders and
   non-copyrightable additions of contributors, in source or binary form)
   alone; or

   (b) combination of their Contribution(s) with the work of authorship to
   which such Contribution(s) was added by such copyright holder or
   contributor, if, at the time the Contribution is added, such addition
   causes such combination to be necessarily infringed. The patent license
   shall not apply to any other combinations which include the
   Contribution.

   Except as expressly stated above, no rights or licenses from any copyright
   holder or contributor is granted under this license, whether expressly, by
   implication, estoppel or otherwise.

   DISCLAIMER

   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
   AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
   IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
   ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE
   LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
   CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
   SUBSTITUTE GOODS OR 

[edk2] [RFC] Change EDK II to BSD+Patent License

[Kinney, Michael D]

Hello,

This RFC follows up on the proposal from Mark Doran to change the 
EDK II Project to a BSD+Patent License.

https://lists.01.org/pipermail/edk2-devel/2019-February/036260.html

The review period for this license change is 30 days.  If there is no
unresolved feedback on April 9, 2019, then commits of the license change
patches will begin on April 9, 2019.  

  ** Please provide feedback on the proposal by Monday April 8, 2019. **

Feedback can be sent to edk2-devel@lists.01.org, the EDK II community
manager or any of the EDK II stewards.

  * Stephano CetolaCommunity Manager
  * Leif Lindholm   Steward
  * Andrew Fish  Steward
  * Laszlo Ersek   Steward
  * Michael KinneySteward

The goal is to convert all of the files in the edk2 repository that are
currently covered by the BSD 2-Clause License and the TianoCore
Contribution Agreement to a BSD+Patent License.  

I will be following up with pointers to public GitHub branches that
contain the set of changes to the edk2 repository for review.

The proposal is to perform this change to edk2/master in the steps listed
below. The license change will not be applied to any of the other existing
branches in the edk2 repository.

1) Add a License-History.txt file to the root of the edk2 repository that
   contains the BSD 2-Clause License and the TianoCore Contribution
   Agreement along with the details on the license change to BSD+Patent.

2) Change all files currently covered by a BSD 2-Clause license and the 
   TianoCore Contribution Agreement to a BSD+Patent license and add an 
   SPDX-License-Identifier statement.  The link to the BSD+Patent license
   and the text for file headers is listed below. 

   https://opensource.org/licenses/BSDplusPatent

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

   Redistribution and use in source and binary forms, with or without
   modification, are permitted provided that the following conditions are met:

   1. Redistributions of source code must retain the above copyright notice,
  this list of conditions and the following disclaimer.

   2. Redistributions in binary form must reproduce the above copyright notice,
  this list of conditions and the following disclaimer in the documentation
  and/or other materials provided with the distribution.

   Subject to the terms and conditions of this license, each copyright holder
   and contributor hereby grants to those receiving rights under this license
   a perpetual, worldwide, non-exclusive, no-charge, royalty-free, irrevocable
   (except for failure to satisfy the conditions of this license) patent
   license to make, have made, use, offer to sell, sell, import, and otherwise
   transfer this software, where such license applies only to those patent
   claims, already acquired or hereafter acquired, licensable by such copyright
   holder or contributor that are necessarily infringed by:

   (a) their Contribution(s) (the licensed copyrights of copyright holders and
   non-copyrightable additions of contributors, in source or binary form)
   alone; or

   (b) combination of their Contribution(s) with the work of authorship to
   which such Contribution(s) was added by such copyright holder or
   contributor, if, at the time the Contribution is added, such addition
   causes such combination to be necessarily infringed. The patent license
   shall not apply to any other combinations which include the
   Contribution.

   Except as expressly stated above, no rights or licenses from any copyright
   holder or contributor is granted under this license, whether expressly, by
   implication, estoppel or otherwise.

   DISCLAIMER

   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
   AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
   IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
   ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE
   LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
   CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
   SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
   INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
   CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
   ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
   POSSIBILITY OF SUCH DAMAGE.
   ==

3) Update Readme.md and License.txt in the root of the edk2 repository to
   state that content is covered by a BSD+Patent license.  Also state that
   BSD+Patent is the preferred license for the EDK II project.

4) Remove the Contributions.txt file in the root of the edk2 repository
   That contiants the TianoCore Contribution 

Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context

[Ard Biesheuvel]

On Tue, 5 Mar 2019 at 17:53, Felix Polyudov  wrote:
>
> There is an architectural difference between EndOfDxe and SmmReadyToLock 
> events.
> It's important to have both of them.
> Here is what PI specification says:
> ==
> Transition from the environment where all of the components are under the 
> authority of the platform manufacturer to the environment where third party 
> modules are executed is a two-step process:
> 1.End of DXE Event is signaled. This event presents the last opportunity to 
> use resources or interfaces that are going to be locked or disabled in 
> anticipation of the invocation of 3rd party extensible modules.
> 2.DXE SMM Ready to Lock Protocol is installed. PI modules that need to lock 
> or protect their resources in anticipation of the invocation of 3rd party 
> extensible modules should register for notification on installation of this 
> protocol and effect the appropriate protections in their notification handlers

Thanks for pointing that out, Felix. I will add SmmReadyToLock as well.


> ==
>
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Yao, 
> Jiewen
> Sent: Tuesday, March 05, 2019 11:19 AM
> To: Ard Biesheuvel
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal 
> architected PI events into MM context
>
> For current X86 world, we do use both SmmReadyToLock and EndOfDxe, for almost 
> all X86 platform.
>
> So, let me clarify:
> If we try to align with PI spec, we can add 
> EndOfDxe/ReadyToBoot/ExitBootService.
> If we try to align with security, we can add EndOfDxe/SmmReadyToLock.
>
> It depends upon the what goal we want to achieve. That is why I ask if we 
> have specific use case.
>
> Anyway, I think we can add when it is really needed later.
> So I am OK with current patch.
>
> Thank you
> Yao Jiewen
>
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Ard Biesheuvel
> > Sent: Tuesday, March 5, 2019 8:07 AM
> > To: Yao, Jiewen 
> > Cc: edk2-devel@lists.01.org
> > Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal
> > architected PI events into MM context
> >
> > On Tue, 5 Mar 2019 at 17:04, Yao, Jiewen  wrote:
> > >
> > > OK. To keep the compatibility of existing MM driver. That makes sense.
> > >
> > > If it is for security, I think EndOfDxe is the only point.
> > > ReadyToBoot and ExitBootService cannot be used for security purpose.
> > >
> >
> > OK, good to know. I will keep them for the time being - MM drivers may
> > be able to release resources or do other useful things when the
> > non-secure side enters runtime mode.
> >
> > > Then do we need SmmReadyToLock ? :-)
> > >
> >
> > Good point. It looked fairly x86 specific to me. Do you think it is
> > likely to be used in OEM code running in MM mode?
> >
> >
> >
> >
> > > > -Original Message-
> > > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
> > Of
> > > > Ard Biesheuvel
> > > > Sent: Tuesday, March 5, 2019 7:58 AM
> > > > To: Yao, Jiewen 
> > > > Cc: edk2-devel@lists.01.org
> > > > Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe:
> > signal
> > > > architected PI events into MM context
> > > >
> > > > On Tue, 5 Mar 2019 at 16:56, Yao, Jiewen 
> > wrote:
> > > > >
> > > > > Hi
> > > > > In original SMM infrastructure, there are lots of interaction that SMM
> > has
> > > > to know the DXE status.
> > > > >
> > > > > In StandaloneMm, I don't expect we have many interaction. Is there
> > any
> > > > specific example?
> > > > >
> > > > > I am totally OK to add those. And I just want to know more usage.
> > > > >
> > > > > Reviewed-by: jiewen@intel.com
> > > > >
> > > >
> > > > Jiewen,
> > > >
> > > > Thanks for the review.
> > > >
> > > > It is not 100% clear at the moment, but since existing third party
> > > > software designed to run in MM context may make assumptions about
> > > > security of the platform (e.g., before vs after end of dxe) based on
> > > > these events, we should at least signal the common ones added in this
> > > > patch.
> > > >
> > > >
> > > >
> > > >
> > > > > > -Original Message-
> > > > > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > > > > Sent: Tuesday, March 5, 2019 5:33 AM
> > > > > > To: edk2-devel@lists.01.org
> > > > > > Cc: Ard Biesheuvel ; Achin Gupta
> > > > > > ; Supreeth Venkatesh
> > > > > > ; Yao, Jiewen
> > ;
> > > > > > Leif Lindholm ; Jagadeesh Ujja
> > > > > > 
> > > > > > Subject: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal
> > > > architected
> > > > > > PI events into MM context
> > > > > >
> > > > > > PI defines a few architected events that have significance in the MM
> > > > > > context as well as in the non-secure DXE context. So register notify
> > > > > > handlers for these events, and relay them into the standalone MM
> > world.
> > > > > >
> > > > > > Contributed-under: TianoCore Contribution Agreement 1.1

Re: [edk2] [PATCH] UefiCpuPkg\PiSmmCpuDxeSmm: Save and restore CR2 only on-demand paging in SMM BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1593 For every SMI occurrence, save and restore CR2 r

[Yao, Jiewen]

Hey
Thanks for the patch.

Would you please follow the EDKII patch format?

Please refer to 
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process

And https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format


Thank you
Yao Jiewen

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> nkvangup
> Sent: Tuesday, March 5, 2019 9:17 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH] UefiCpuPkg\PiSmmCpuDxeSmm: Save and restore
> CR2 only on-demand paging in SMM BZ:
> https://bugzilla.tianocore.org/show_bug.cgi?id=1593 For every SMI
> occurrence, save and restore CR2 register only when SMM on-demand
> paging support is enable...
> 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 20 
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> index 3b0b3b52ac..5be4a2b020 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> @@ -,10 +,12 @@ SmiRendezvous (
> 
>ASSERT(CpuIndex < mMaxNumberOfCpus);
> 
> -  //
> -  // Save Cr2 because Page Fault exception in SMM may override its value
> -  //
> -  Cr2 = AsmReadCr2 ();
> +  if ((sizeof (UINTN) == sizeof (UINT64)) && (!PcdGetBool
> (PcdCpuSmmStaticPageTable))) {
> +//
> +// Save Cr2 because Page Fault exception in SMM may override its
> value
> +//
> +Cr2 = AsmReadCr2 ();
> +  }
> 
>//
>// Perform CPU specific entry hooks
> @@ -1253,10 +1255,12 @@ SmiRendezvous (
> 
>  Exit:
>SmmCpuFeaturesRendezvousExit (CpuIndex);
> -  //
> -  // Restore Cr2
> -  //
> -  AsmWriteCr2 (Cr2);
> +  if ((sizeof (UINTN) == sizeof (UINT64)) && (!PcdGetBool
> (PcdCpuSmmStaticPageTable))) {
> +//
> +// Restore Cr2
> +//
> +AsmWriteCr2 (Cr2);
> +  }
>  }
> 
>  /**
> --
> 2.16.2.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] UefiCpuPkg\PiSmmCpuDxeSmm: Save and restore CR2 only on-demand paging in SMM BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1593 For every SMI occurrence, save and restore CR2 regis

[nkvangup]

---
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index 3b0b3b52ac..5be4a2b020 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -,10 +,12 @@ SmiRendezvous (
 
   ASSERT(CpuIndex < mMaxNumberOfCpus);
 
-  //
-  // Save Cr2 because Page Fault exception in SMM may override its value
-  //
-  Cr2 = AsmReadCr2 ();
+  if ((sizeof (UINTN) == sizeof (UINT64)) && (!PcdGetBool 
(PcdCpuSmmStaticPageTable))) {
+//
+// Save Cr2 because Page Fault exception in SMM may override its value
+//
+Cr2 = AsmReadCr2 ();
+  }
 
   //
   // Perform CPU specific entry hooks
@@ -1253,10 +1255,12 @@ SmiRendezvous (
 
 Exit:
   SmmCpuFeaturesRendezvousExit (CpuIndex);
-  //
-  // Restore Cr2
-  //
-  AsmWriteCr2 (Cr2);
+  if ((sizeof (UINTN) == sizeof (UINT64)) && (!PcdGetBool 
(PcdCpuSmmStaticPageTable))) {
+//
+// Restore Cr2
+//
+AsmWriteCr2 (Cr2);
+  }
 }
 
 /**
-- 
2.16.2.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context

[Felix Polyudov]

There is an architectural difference between EndOfDxe and SmmReadyToLock events.
It's important to have both of them.
Here is what PI specification says:
==
Transition from the environment where all of the components are under the 
authority of the platform manufacturer to the environment where third party 
modules are executed is a two-step process:
1.End of DXE Event is signaled. This event presents the last opportunity to use 
resources or interfaces that are going to be locked or disabled in anticipation 
of the invocation of 3rd party extensible modules.
2.DXE SMM Ready to Lock Protocol is installed. PI modules that need to lock or 
protect their resources in anticipation of the invocation of 3rd party 
extensible modules should register for notification on installation of this 
protocol and effect the appropriate protections in their notification handlers
==

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Yao, 
Jiewen
Sent: Tuesday, March 05, 2019 11:19 AM
To: Ard Biesheuvel
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected 
PI events into MM context

For current X86 world, we do use both SmmReadyToLock and EndOfDxe, for almost 
all X86 platform.

So, let me clarify:
If we try to align with PI spec, we can add 
EndOfDxe/ReadyToBoot/ExitBootService.
If we try to align with security, we can add EndOfDxe/SmmReadyToLock.

It depends upon the what goal we want to achieve. That is why I ask if we have 
specific use case.

Anyway, I think we can add when it is really needed later.
So I am OK with current patch.

Thank you
Yao Jiewen

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Tuesday, March 5, 2019 8:07 AM
> To: Yao, Jiewen 
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal
> architected PI events into MM context
> 
> On Tue, 5 Mar 2019 at 17:04, Yao, Jiewen  wrote:
> >
> > OK. To keep the compatibility of existing MM driver. That makes sense.
> >
> > If it is for security, I think EndOfDxe is the only point.
> > ReadyToBoot and ExitBootService cannot be used for security purpose.
> >
> 
> OK, good to know. I will keep them for the time being - MM drivers may
> be able to release resources or do other useful things when the
> non-secure side enters runtime mode.
> 
> > Then do we need SmmReadyToLock ? :-)
> >
> 
> Good point. It looked fairly x86 specific to me. Do you think it is
> likely to be used in OEM code running in MM mode?
> 
> 
> 
> 
> > > -Original Message-
> > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
> Of
> > > Ard Biesheuvel
> > > Sent: Tuesday, March 5, 2019 7:58 AM
> > > To: Yao, Jiewen 
> > > Cc: edk2-devel@lists.01.org
> > > Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe:
> signal
> > > architected PI events into MM context
> > >
> > > On Tue, 5 Mar 2019 at 16:56, Yao, Jiewen 
> wrote:
> > > >
> > > > Hi
> > > > In original SMM infrastructure, there are lots of interaction that SMM
> has
> > > to know the DXE status.
> > > >
> > > > In StandaloneMm, I don't expect we have many interaction. Is there
> any
> > > specific example?
> > > >
> > > > I am totally OK to add those. And I just want to know more usage.
> > > >
> > > > Reviewed-by: jiewen@intel.com
> > > >
> > >
> > > Jiewen,
> > >
> > > Thanks for the review.
> > >
> > > It is not 100% clear at the moment, but since existing third party
> > > software designed to run in MM context may make assumptions about
> > > security of the platform (e.g., before vs after end of dxe) based on
> > > these events, we should at least signal the common ones added in this
> > > patch.
> > >
> > >
> > >
> > >
> > > > > -Original Message-
> > > > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > > > Sent: Tuesday, March 5, 2019 5:33 AM
> > > > > To: edk2-devel@lists.01.org
> > > > > Cc: Ard Biesheuvel ; Achin Gupta
> > > > > ; Supreeth Venkatesh
> > > > > ; Yao, Jiewen
> ;
> > > > > Leif Lindholm ; Jagadeesh Ujja
> > > > > 
> > > > > Subject: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal
> > > architected
> > > > > PI events into MM context
> > > > >
> > > > > PI defines a few architected events that have significance in the MM
> > > > > context as well as in the non-secure DXE context. So register notify
> > > > > handlers for these events, and relay them into the standalone MM
> world.
> > > > >
> > > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > > Signed-off-by: Ard Biesheuvel 
> > > > > ---
> > > > >  ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf |
> 5
> > > +++
> > > > >  ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c   |
> 47
> > > > > +++-
> > > > >  2 files changed, 50 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git
> > > 

Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context

[Yao, Jiewen]

For current X86 world, we do use both SmmReadyToLock and EndOfDxe, for almost 
all X86 platform.

So, let me clarify:
If we try to align with PI spec, we can add 
EndOfDxe/ReadyToBoot/ExitBootService.
If we try to align with security, we can add EndOfDxe/SmmReadyToLock.

It depends upon the what goal we want to achieve. That is why I ask if we have 
specific use case.

Anyway, I think we can add when it is really needed later.
So I am OK with current patch.

Thank you
Yao Jiewen

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Tuesday, March 5, 2019 8:07 AM
> To: Yao, Jiewen 
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal
> architected PI events into MM context
> 
> On Tue, 5 Mar 2019 at 17:04, Yao, Jiewen  wrote:
> >
> > OK. To keep the compatibility of existing MM driver. That makes sense.
> >
> > If it is for security, I think EndOfDxe is the only point.
> > ReadyToBoot and ExitBootService cannot be used for security purpose.
> >
> 
> OK, good to know. I will keep them for the time being - MM drivers may
> be able to release resources or do other useful things when the
> non-secure side enters runtime mode.
> 
> > Then do we need SmmReadyToLock ? :-)
> >
> 
> Good point. It looked fairly x86 specific to me. Do you think it is
> likely to be used in OEM code running in MM mode?
> 
> 
> 
> 
> > > -Original Message-
> > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
> Of
> > > Ard Biesheuvel
> > > Sent: Tuesday, March 5, 2019 7:58 AM
> > > To: Yao, Jiewen 
> > > Cc: edk2-devel@lists.01.org
> > > Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe:
> signal
> > > architected PI events into MM context
> > >
> > > On Tue, 5 Mar 2019 at 16:56, Yao, Jiewen 
> wrote:
> > > >
> > > > Hi
> > > > In original SMM infrastructure, there are lots of interaction that SMM
> has
> > > to know the DXE status.
> > > >
> > > > In StandaloneMm, I don't expect we have many interaction. Is there
> any
> > > specific example?
> > > >
> > > > I am totally OK to add those. And I just want to know more usage.
> > > >
> > > > Reviewed-by: jiewen@intel.com
> > > >
> > >
> > > Jiewen,
> > >
> > > Thanks for the review.
> > >
> > > It is not 100% clear at the moment, but since existing third party
> > > software designed to run in MM context may make assumptions about
> > > security of the platform (e.g., before vs after end of dxe) based on
> > > these events, we should at least signal the common ones added in this
> > > patch.
> > >
> > >
> > >
> > >
> > > > > -Original Message-
> > > > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > > > Sent: Tuesday, March 5, 2019 5:33 AM
> > > > > To: edk2-devel@lists.01.org
> > > > > Cc: Ard Biesheuvel ; Achin Gupta
> > > > > ; Supreeth Venkatesh
> > > > > ; Yao, Jiewen
> ;
> > > > > Leif Lindholm ; Jagadeesh Ujja
> > > > > 
> > > > > Subject: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal
> > > architected
> > > > > PI events into MM context
> > > > >
> > > > > PI defines a few architected events that have significance in the MM
> > > > > context as well as in the non-secure DXE context. So register notify
> > > > > handlers for these events, and relay them into the standalone MM
> world.
> > > > >
> > > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > > Signed-off-by: Ard Biesheuvel 
> > > > > ---
> > > > >  ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf |
> 5
> > > +++
> > > > >  ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c   |
> 47
> > > > > +++-
> > > > >  2 files changed, 50 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git
> > > a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > > > index 88beafa39c05..8bf269270f9d 100644
> > > > > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > > > +++
> b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > > > @@ -48,6 +48,11 @@ [LibraryClasses]
> > > > >  [Protocols]
> > > > >gEfiMmCommunicationProtocolGuid  ##
> PRODUCES
> > > > >
> > > > > +[Guids]
> > > > > +  gEfiEndOfDxeEventGroupGuid
> > > > > +  gEfiEventExitBootServicesGuid
> > > > > +  gEfiEventReadyToBootGuid
> > > > > +
> > > > >  [Pcd.common]
> > > > >gArmTokenSpaceGuid.PcdMmBufferBase
> > > > >gArmTokenSpaceGuid.PcdMmBufferSize
> > > > > diff --git
> > > a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > > > index feb9fa9f4ead..3203cf801a19 100644
> > > > > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > > > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > > > @@ -265,6 +265,43 @@ GetMmCompatibility ()
> > > > >return Status;
> > > > >  }
> > > > >
> > > > > +STATIC EFI_GUID* CONST mGuidedEventGuid[] = {

Re: [edk2] [PATCH v3] StandaloneMmPkg/Library: Install Variable Arch Protocol

[Ard Biesheuvel]

On Tue, 5 Mar 2019 at 17:15, Yao, Jiewen  wrote:
>
> I look at the patch. I don’t have concern.
>
> Please go ahead.
>

Thank you Jiewen,

I will take that as a reviewed-by and proceed with merging the patch
once the hard freeze is over.


> > -Original Message-
> > From: Jagadeesh Ujja [mailto:jagadeesh.u...@arm.com]
> > Sent: Tuesday, March 5, 2019 6:09 AM
> > To: Ard Biesheuvel 
> > Cc: Achin Gupta ; Yao, Jiewen
> > ; Gao, Liming ;
> > edk2-devel@lists.01.org; Zhang, Chao B ; Kinney,
> > Michael D ; Zeng, Star 
> > Subject: Re: [edk2] [PATCH v3] StandaloneMmPkg/Library: Install Variable
> > Arch Protocol
> >
> > hi Jiewen, Achin
> >
> > On Mon, Mar 4, 2019 at 4:16 PM Ard Biesheuvel 
> > wrote:
> > >
> > > (add StandaloneMmPkg maintainers)
> > >
> > Please let me know if you have any comments on this patch
> >
> > > On Mon, 4 Mar 2019 at 09:54, Jagadeesh Ujja 
> > wrote:
> > > >
> > > > In a system implementing the variable store in MM, there are no variable
> > > > arch protocol and variable write arch protocol installed into the
> > > > DXE_SMM protocol database. On such systems, it is not required to
> > > > locate these protocols by the DXE runtime variable drivers because
> > > > it can be assumed that these protocols are already installed in the
> > > > MM context. But then such an implementation will deviate from the
> > > > existing traditional MM based variable driver implementation.
> > > >
> > > > So in order to maintain consistency with the traditional MM variable
> > > > driver implementation, allow platforms to install these protocols into
> > > > the DXE protocol database but these protocol will not be consumed
> > > > by non-secure variable service runtime driver.
> > > >
> > > > The Platform which uses StandaloneMM based secure variable storage
> > > > have to include this library
> > > >
> > > > Example
> > > > In edk2-platforms/Platform/ARM/SgiPkg/SgiPlatform.dsc
> > > >
> > > >   ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf {
> > > > 
> > > >
> > NULL|StandaloneMmPkg/Library/VariableMmDependency/VariableMmDep
> > endency.inf
> > > >   }
> > > >
> > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > Signed-off-by: Jagadeesh Ujja 
> > >
> > > Reviewed-by: Ard Biesheuvel 
> > >
> > > > ---
> > > > Changes since v2:
> > > > - Addressed the comments from Ard Biesheuvel and Zeng Star
> > > >
> > > > Changes since v1:
> > > > - This is a next version of patch
> > > >“MdeModulePkg/VariableSmmRuntimeDxe: Refactor locating
> > Variable Arch Protocol”.
> > > >
> > [https://lists.01.org/pipermail/edk2-devel/2019-February/036885.html]
> > > > - Addressed the comments from Ard Biesheuvel and Zeng Star
> > > > - Can this library be placed in MdePkg rather then the StandaloneMmPkg?
> > > >
> > > >
> > StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependenc
> > y.c   | 54 
> > > >
> > StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependenc
> > y.inf | 46 +
> > > >  2 files changed, 100 insertions(+)
> > > >
> > > > diff --git
> > a/StandaloneMmPkg/Library/VariableMmDependency/VariableMmDepende
> > ncy.c
> > b/StandaloneMmPkg/Library/VariableMmDependency/VariableMmDepende
> > ncy.c
> > > > new file mode 100644
> > > > index 000..7e0f31b
> > > > --- /dev/null
> > > > +++
> > b/StandaloneMmPkg/Library/VariableMmDependency/VariableMmDepende
> > ncy.c
> > > > @@ -0,0 +1,54 @@
> > > > +/** @file
> > > > +  Runtime DXE part corresponding to StanaloneMM variable module.
> > > > +
> > > > +This module installs variable arch protocol and variable write arch
> > protocol
> > > > +to StandaloneMM runtime variable service.
> > > > +
> > > > +Copyright (c) 2019, ARM Ltd. All rights reserved.
> > > > +
> > > > +This program and the accompanying materials
> > > > +are licensed and made available under the terms and conditions of the
> > BSD License
> > > > +which accompanies this distribution.  The full text of the license may
> > be found at
> > > > +http://opensource.org/licenses/bsd-license.php.
> > > > +
> > > > +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> > BASIS,
> > > > +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> > EXPRESS OR IMPLIED.
> > > > +
> > > > +**/
> > > > +
> > > > +#include 
> > > > +#include 
> > > > +
> > > > +/**
> > > > +  The constructor function installs variable arch protocol and variable
> > > > +  write arch protocol to StandaloneMM runtime variable service
> > > > +
> > > > +  @param  ImageHandle   The firmware allocated handle for the EFI
> > image.
> > > > +  @param  SystemTable   A pointer to the Management mode
> > System Table.
> > > > +
> > > > +  @retval EFI_SUCCESS   The constructor always returns
> > EFI_SUCCESS.
> > > > +
> > > > +**/
> > > > +EFI_STATUS
> > > > +EFIAPI
> > > > +VariableMmDependencyLibConstructor (
> > > > +  IN EFI_HANDLE   ImageHandle,
> > > > +  IN EFI_SYSTEM_TABLE *SystemTable
> 

Re: [edk2] [PATCH v3] StandaloneMmPkg/Library: Install Variable Arch Protocol

[Yao, Jiewen]

I look at the patch. I don’t have concern.

Please go ahead.

Thank you
Yao Jiewen


> -Original Message-
> From: Jagadeesh Ujja [mailto:jagadeesh.u...@arm.com]
> Sent: Tuesday, March 5, 2019 6:09 AM
> To: Ard Biesheuvel 
> Cc: Achin Gupta ; Yao, Jiewen
> ; Gao, Liming ;
> edk2-devel@lists.01.org; Zhang, Chao B ; Kinney,
> Michael D ; Zeng, Star 
> Subject: Re: [edk2] [PATCH v3] StandaloneMmPkg/Library: Install Variable
> Arch Protocol
> 
> hi Jiewen, Achin
> 
> On Mon, Mar 4, 2019 at 4:16 PM Ard Biesheuvel 
> wrote:
> >
> > (add StandaloneMmPkg maintainers)
> >
> Please let me know if you have any comments on this patch
> 
> > On Mon, 4 Mar 2019 at 09:54, Jagadeesh Ujja 
> wrote:
> > >
> > > In a system implementing the variable store in MM, there are no variable
> > > arch protocol and variable write arch protocol installed into the
> > > DXE_SMM protocol database. On such systems, it is not required to
> > > locate these protocols by the DXE runtime variable drivers because
> > > it can be assumed that these protocols are already installed in the
> > > MM context. But then such an implementation will deviate from the
> > > existing traditional MM based variable driver implementation.
> > >
> > > So in order to maintain consistency with the traditional MM variable
> > > driver implementation, allow platforms to install these protocols into
> > > the DXE protocol database but these protocol will not be consumed
> > > by non-secure variable service runtime driver.
> > >
> > > The Platform which uses StandaloneMM based secure variable storage
> > > have to include this library
> > >
> > > Example
> > > In edk2-platforms/Platform/ARM/SgiPkg/SgiPlatform.dsc
> > >
> > >   ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf {
> > > 
> > >
> NULL|StandaloneMmPkg/Library/VariableMmDependency/VariableMmDep
> endency.inf
> > >   }
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Jagadeesh Ujja 
> >
> > Reviewed-by: Ard Biesheuvel 
> >
> > > ---
> > > Changes since v2:
> > > - Addressed the comments from Ard Biesheuvel and Zeng Star
> > >
> > > Changes since v1:
> > > - This is a next version of patch
> > >“MdeModulePkg/VariableSmmRuntimeDxe: Refactor locating
> Variable Arch Protocol”.
> > >
> [https://lists.01.org/pipermail/edk2-devel/2019-February/036885.html]
> > > - Addressed the comments from Ard Biesheuvel and Zeng Star
> > > - Can this library be placed in MdePkg rather then the StandaloneMmPkg?
> > >
> > >
> StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependenc
> y.c   | 54 
> > >
> StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependenc
> y.inf | 46 +
> > >  2 files changed, 100 insertions(+)
> > >
> > > diff --git
> a/StandaloneMmPkg/Library/VariableMmDependency/VariableMmDepende
> ncy.c
> b/StandaloneMmPkg/Library/VariableMmDependency/VariableMmDepende
> ncy.c
> > > new file mode 100644
> > > index 000..7e0f31b
> > > --- /dev/null
> > > +++
> b/StandaloneMmPkg/Library/VariableMmDependency/VariableMmDepende
> ncy.c
> > > @@ -0,0 +1,54 @@
> > > +/** @file
> > > +  Runtime DXE part corresponding to StanaloneMM variable module.
> > > +
> > > +This module installs variable arch protocol and variable write arch
> protocol
> > > +to StandaloneMM runtime variable service.
> > > +
> > > +Copyright (c) 2019, ARM Ltd. All rights reserved.
> > > +
> > > +This program and the accompanying materials
> > > +are licensed and made available under the terms and conditions of the
> BSD License
> > > +which accompanies this distribution.  The full text of the license may
> be found at
> > > +http://opensource.org/licenses/bsd-license.php.
> > > +
> > > +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> > > +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> > > +
> > > +**/
> > > +
> > > +#include 
> > > +#include 
> > > +
> > > +/**
> > > +  The constructor function installs variable arch protocol and variable
> > > +  write arch protocol to StandaloneMM runtime variable service
> > > +
> > > +  @param  ImageHandle   The firmware allocated handle for the EFI
> image.
> > > +  @param  SystemTable   A pointer to the Management mode
> System Table.
> > > +
> > > +  @retval EFI_SUCCESS   The constructor always returns
> EFI_SUCCESS.
> > > +
> > > +**/
> > > +EFI_STATUS
> > > +EFIAPI
> > > +VariableMmDependencyLibConstructor (
> > > +  IN EFI_HANDLE   ImageHandle,
> > > +  IN EFI_SYSTEM_TABLE *SystemTable
> > > +  )
> > > +{
> > > +  EFI_STATUSStatus;
> > > +  EFI_HANDLEHandle;
> > > +
> > > +  Handle = NULL;
> > > +  Status = gBS->InstallMultipleProtocolInterfaces (
> > > +  ,
> > > +  ,
> > > +  NULL,
> > > +  ,
> > > +  NULL,
> > > +  NULL
> > > +  );
> > > +  ASSERT_EFI_ERROR 

Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context

[Ard Biesheuvel]

On Tue, 5 Mar 2019 at 17:04, Yao, Jiewen  wrote:
>
> OK. To keep the compatibility of existing MM driver. That makes sense.
>
> If it is for security, I think EndOfDxe is the only point.
> ReadyToBoot and ExitBootService cannot be used for security purpose.
>

OK, good to know. I will keep them for the time being - MM drivers may
be able to release resources or do other useful things when the
non-secure side enters runtime mode.

> Then do we need SmmReadyToLock ? :-)
>

Good point. It looked fairly x86 specific to me. Do you think it is
likely to be used in OEM code running in MM mode?




> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Ard Biesheuvel
> > Sent: Tuesday, March 5, 2019 7:58 AM
> > To: Yao, Jiewen 
> > Cc: edk2-devel@lists.01.org
> > Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal
> > architected PI events into MM context
> >
> > On Tue, 5 Mar 2019 at 16:56, Yao, Jiewen  wrote:
> > >
> > > Hi
> > > In original SMM infrastructure, there are lots of interaction that SMM has
> > to know the DXE status.
> > >
> > > In StandaloneMm, I don't expect we have many interaction. Is there any
> > specific example?
> > >
> > > I am totally OK to add those. And I just want to know more usage.
> > >
> > > Reviewed-by: jiewen@intel.com
> > >
> >
> > Jiewen,
> >
> > Thanks for the review.
> >
> > It is not 100% clear at the moment, but since existing third party
> > software designed to run in MM context may make assumptions about
> > security of the platform (e.g., before vs after end of dxe) based on
> > these events, we should at least signal the common ones added in this
> > patch.
> >
> >
> >
> >
> > > > -Original Message-
> > > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > > Sent: Tuesday, March 5, 2019 5:33 AM
> > > > To: edk2-devel@lists.01.org
> > > > Cc: Ard Biesheuvel ; Achin Gupta
> > > > ; Supreeth Venkatesh
> > > > ; Yao, Jiewen ;
> > > > Leif Lindholm ; Jagadeesh Ujja
> > > > 
> > > > Subject: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal
> > architected
> > > > PI events into MM context
> > > >
> > > > PI defines a few architected events that have significance in the MM
> > > > context as well as in the non-secure DXE context. So register notify
> > > > handlers for these events, and relay them into the standalone MM world.
> > > >
> > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > Signed-off-by: Ard Biesheuvel 
> > > > ---
> > > >  ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf |  5
> > +++
> > > >  ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c   | 47
> > > > +++-
> > > >  2 files changed, 50 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git
> > a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > > index 88beafa39c05..8bf269270f9d 100644
> > > > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > > @@ -48,6 +48,11 @@ [LibraryClasses]
> > > >  [Protocols]
> > > >gEfiMmCommunicationProtocolGuid  ## PRODUCES
> > > >
> > > > +[Guids]
> > > > +  gEfiEndOfDxeEventGroupGuid
> > > > +  gEfiEventExitBootServicesGuid
> > > > +  gEfiEventReadyToBootGuid
> > > > +
> > > >  [Pcd.common]
> > > >gArmTokenSpaceGuid.PcdMmBufferBase
> > > >gArmTokenSpaceGuid.PcdMmBufferSize
> > > > diff --git
> > a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > > index feb9fa9f4ead..3203cf801a19 100644
> > > > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > > @@ -265,6 +265,43 @@ GetMmCompatibility ()
> > > >return Status;
> > > >  }
> > > >
> > > > +STATIC EFI_GUID* CONST mGuidedEventGuid[] = {
> > > > +  ,
> > > > +  ,
> > > > +  ,
> > > > +};
> > > > +
> > > > +STATIC EFI_EVENT mGuidedEvent[ARRAY_SIZE (mGuidedEventGuid)];
> > > > +
> > > > +/**
> > > > +  Event notification that is fired when GUIDed Event Group is signaled.
> > > > +
> > > > +  @param  Event The Event that is being
> > processed,
> > > > not used.
> > > > +  @param  Context   Event Context, not used.
> > > > +
> > > > +**/
> > > > +STATIC
> > > > +VOID
> > > > +EFIAPI
> > > > +MmGuidedEventNotify (
> > > > +  IN EFI_EVENT  Event,
> > > > +  IN VOID   *Context
> > > > +  )
> > > > +{
> > > > +  EFI_MM_COMMUNICATE_HEADER   Header;
> > > > +  UINTN   Size;
> > > > +
> > > > +  //
> > > > +  // Use Guid to initialize EFI_SMM_COMMUNICATE_HEADER structure
> > > > +  //
> > > > +  CopyGuid (, Context);
> > > > +  Header.MessageLength = 1;
> > > > +  Header.Data[0] = 0;
> > > > +
> > > > +  Size = sizeof (Header);
> > > > +  MmCommunicationCommunicate (, ,
> > > > );
> > > > +}
> > > > +
> > > >  /**
> > > >

Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context

[Yao, Jiewen]

OK. To keep the compatibility of existing MM driver. That makes sense.

If it is for security, I think EndOfDxe is the only point.
ReadyToBoot and ExitBootService cannot be used for security purpose.

Then do we need SmmReadyToLock ? :-)


Thank you
Yao Jiewen

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Tuesday, March 5, 2019 7:58 AM
> To: Yao, Jiewen 
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal
> architected PI events into MM context
> 
> On Tue, 5 Mar 2019 at 16:56, Yao, Jiewen  wrote:
> >
> > Hi
> > In original SMM infrastructure, there are lots of interaction that SMM has
> to know the DXE status.
> >
> > In StandaloneMm, I don't expect we have many interaction. Is there any
> specific example?
> >
> > I am totally OK to add those. And I just want to know more usage.
> >
> > Reviewed-by: jiewen@intel.com
> >
> 
> Jiewen,
> 
> Thanks for the review.
> 
> It is not 100% clear at the moment, but since existing third party
> software designed to run in MM context may make assumptions about
> security of the platform (e.g., before vs after end of dxe) based on
> these events, we should at least signal the common ones added in this
> patch.
> 
> 
> 
> 
> > > -Original Message-
> > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > Sent: Tuesday, March 5, 2019 5:33 AM
> > > To: edk2-devel@lists.01.org
> > > Cc: Ard Biesheuvel ; Achin Gupta
> > > ; Supreeth Venkatesh
> > > ; Yao, Jiewen ;
> > > Leif Lindholm ; Jagadeesh Ujja
> > > 
> > > Subject: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal
> architected
> > > PI events into MM context
> > >
> > > PI defines a few architected events that have significance in the MM
> > > context as well as in the non-secure DXE context. So register notify
> > > handlers for these events, and relay them into the standalone MM world.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Ard Biesheuvel 
> > > ---
> > >  ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf |  5
> +++
> > >  ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c   | 47
> > > +++-
> > >  2 files changed, 50 insertions(+), 2 deletions(-)
> > >
> > > diff --git
> a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > index 88beafa39c05..8bf269270f9d 100644
> > > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > @@ -48,6 +48,11 @@ [LibraryClasses]
> > >  [Protocols]
> > >gEfiMmCommunicationProtocolGuid  ## PRODUCES
> > >
> > > +[Guids]
> > > +  gEfiEndOfDxeEventGroupGuid
> > > +  gEfiEventExitBootServicesGuid
> > > +  gEfiEventReadyToBootGuid
> > > +
> > >  [Pcd.common]
> > >gArmTokenSpaceGuid.PcdMmBufferBase
> > >gArmTokenSpaceGuid.PcdMmBufferSize
> > > diff --git
> a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > index feb9fa9f4ead..3203cf801a19 100644
> > > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > @@ -265,6 +265,43 @@ GetMmCompatibility ()
> > >return Status;
> > >  }
> > >
> > > +STATIC EFI_GUID* CONST mGuidedEventGuid[] = {
> > > +  ,
> > > +  ,
> > > +  ,
> > > +};
> > > +
> > > +STATIC EFI_EVENT mGuidedEvent[ARRAY_SIZE (mGuidedEventGuid)];
> > > +
> > > +/**
> > > +  Event notification that is fired when GUIDed Event Group is signaled.
> > > +
> > > +  @param  Event The Event that is being
> processed,
> > > not used.
> > > +  @param  Context   Event Context, not used.
> > > +
> > > +**/
> > > +STATIC
> > > +VOID
> > > +EFIAPI
> > > +MmGuidedEventNotify (
> > > +  IN EFI_EVENT  Event,
> > > +  IN VOID   *Context
> > > +  )
> > > +{
> > > +  EFI_MM_COMMUNICATE_HEADER   Header;
> > > +  UINTN   Size;
> > > +
> > > +  //
> > > +  // Use Guid to initialize EFI_SMM_COMMUNICATE_HEADER structure
> > > +  //
> > > +  CopyGuid (, Context);
> > > +  Header.MessageLength = 1;
> > > +  Header.Data[0] = 0;
> > > +
> > > +  Size = sizeof (Header);
> > > +  MmCommunicationCommunicate (, ,
> > > );
> > > +}
> > > +
> > >  /**
> > >The Entry Point for MM Communication
> > >
> > > @@ -287,6 +324,7 @@ MmCommunicationInitialize (
> > >)
> > >  {
> > >EFI_STATUS Status;
> > > +  UINTN  Index;
> > >
> > >// Check if we can make the MM call
> > >Status = GetMmCompatibility ();
> > > @@ -351,8 +389,13 @@ MmCommunicationInitialize (
> > >NULL,
> > >
> > >);
> > > -  if (Status == EFI_SUCCESS) {
> > > -return Status;
> > > +  ASSERT_EFI_ERROR (Status);
> > > +
> > > +  for (Index = 0; Index < ARRAY_SIZE 

Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context

[Ard Biesheuvel]

On Tue, 5 Mar 2019 at 16:56, Yao, Jiewen  wrote:
>
> Hi
> In original SMM infrastructure, there are lots of interaction that SMM has to 
> know the DXE status.
>
> In StandaloneMm, I don't expect we have many interaction. Is there any 
> specific example?
>
> I am totally OK to add those. And I just want to know more usage.
>
> Reviewed-by: jiewen@intel.com
>

Jiewen,

Thanks for the review.

It is not 100% clear at the moment, but since existing third party
software designed to run in MM context may make assumptions about
security of the platform (e.g., before vs after end of dxe) based on
these events, we should at least signal the common ones added in this
patch.




> > -Original Message-
> > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > Sent: Tuesday, March 5, 2019 5:33 AM
> > To: edk2-devel@lists.01.org
> > Cc: Ard Biesheuvel ; Achin Gupta
> > ; Supreeth Venkatesh
> > ; Yao, Jiewen ;
> > Leif Lindholm ; Jagadeesh Ujja
> > 
> > Subject: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected
> > PI events into MM context
> >
> > PI defines a few architected events that have significance in the MM
> > context as well as in the non-secure DXE context. So register notify
> > handlers for these events, and relay them into the standalone MM world.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel 
> > ---
> >  ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf |  5 +++
> >  ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c   | 47
> > +++-
> >  2 files changed, 50 insertions(+), 2 deletions(-)
> >
> > diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > index 88beafa39c05..8bf269270f9d 100644
> > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > @@ -48,6 +48,11 @@ [LibraryClasses]
> >  [Protocols]
> >gEfiMmCommunicationProtocolGuid  ## PRODUCES
> >
> > +[Guids]
> > +  gEfiEndOfDxeEventGroupGuid
> > +  gEfiEventExitBootServicesGuid
> > +  gEfiEventReadyToBootGuid
> > +
> >  [Pcd.common]
> >gArmTokenSpaceGuid.PcdMmBufferBase
> >gArmTokenSpaceGuid.PcdMmBufferSize
> > diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > index feb9fa9f4ead..3203cf801a19 100644
> > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > @@ -265,6 +265,43 @@ GetMmCompatibility ()
> >return Status;
> >  }
> >
> > +STATIC EFI_GUID* CONST mGuidedEventGuid[] = {
> > +  ,
> > +  ,
> > +  ,
> > +};
> > +
> > +STATIC EFI_EVENT mGuidedEvent[ARRAY_SIZE (mGuidedEventGuid)];
> > +
> > +/**
> > +  Event notification that is fired when GUIDed Event Group is signaled.
> > +
> > +  @param  Event The Event that is being processed,
> > not used.
> > +  @param  Context   Event Context, not used.
> > +
> > +**/
> > +STATIC
> > +VOID
> > +EFIAPI
> > +MmGuidedEventNotify (
> > +  IN EFI_EVENT  Event,
> > +  IN VOID   *Context
> > +  )
> > +{
> > +  EFI_MM_COMMUNICATE_HEADER   Header;
> > +  UINTN   Size;
> > +
> > +  //
> > +  // Use Guid to initialize EFI_SMM_COMMUNICATE_HEADER structure
> > +  //
> > +  CopyGuid (, Context);
> > +  Header.MessageLength = 1;
> > +  Header.Data[0] = 0;
> > +
> > +  Size = sizeof (Header);
> > +  MmCommunicationCommunicate (, ,
> > );
> > +}
> > +
> >  /**
> >The Entry Point for MM Communication
> >
> > @@ -287,6 +324,7 @@ MmCommunicationInitialize (
> >)
> >  {
> >EFI_STATUS Status;
> > +  UINTN  Index;
> >
> >// Check if we can make the MM call
> >Status = GetMmCompatibility ();
> > @@ -351,8 +389,13 @@ MmCommunicationInitialize (
> >NULL,
> >
> >);
> > -  if (Status == EFI_SUCCESS) {
> > -return Status;
> > +  ASSERT_EFI_ERROR (Status);
> > +
> > +  for (Index = 0; Index < ARRAY_SIZE (mGuidedEventGuid); Index++) {
> > +Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
> > +MmGuidedEventNotify, mGuidedEventGuid[Index],
> > +mGuidedEventGuid[Index],
> > [Index]);
> > +ASSERT_EFI_ERROR (Status);
> >}
> >
> >gBS->UninstallProtocolInterface (
> > --
> > 2.20.1
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context

[Yao, Jiewen]

Hi
In original SMM infrastructure, there are lots of interaction that SMM has to 
know the DXE status.

In StandaloneMm, I don't expect we have many interaction. Is there any specific 
example?

I am totally OK to add those. And I just want to know more usage.

Reviewed-by: jiewen@intel.com


Thank you
Yao Jiewen

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, March 5, 2019 5:33 AM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel ; Achin Gupta
> ; Supreeth Venkatesh
> ; Yao, Jiewen ;
> Leif Lindholm ; Jagadeesh Ujja
> 
> Subject: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected
> PI events into MM context
> 
> PI defines a few architected events that have significance in the MM
> context as well as in the non-secure DXE context. So register notify
> handlers for these events, and relay them into the standalone MM world.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf |  5 +++
>  ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c   | 47
> +++-
>  2 files changed, 50 insertions(+), 2 deletions(-)
> 
> diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> index 88beafa39c05..8bf269270f9d 100644
> --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> @@ -48,6 +48,11 @@ [LibraryClasses]
>  [Protocols]
>gEfiMmCommunicationProtocolGuid  ## PRODUCES
> 
> +[Guids]
> +  gEfiEndOfDxeEventGroupGuid
> +  gEfiEventExitBootServicesGuid
> +  gEfiEventReadyToBootGuid
> +
>  [Pcd.common]
>gArmTokenSpaceGuid.PcdMmBufferBase
>gArmTokenSpaceGuid.PcdMmBufferSize
> diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> index feb9fa9f4ead..3203cf801a19 100644
> --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> @@ -265,6 +265,43 @@ GetMmCompatibility ()
>return Status;
>  }
> 
> +STATIC EFI_GUID* CONST mGuidedEventGuid[] = {
> +  ,
> +  ,
> +  ,
> +};
> +
> +STATIC EFI_EVENT mGuidedEvent[ARRAY_SIZE (mGuidedEventGuid)];
> +
> +/**
> +  Event notification that is fired when GUIDed Event Group is signaled.
> +
> +  @param  Event The Event that is being processed,
> not used.
> +  @param  Context   Event Context, not used.
> +
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +MmGuidedEventNotify (
> +  IN EFI_EVENT  Event,
> +  IN VOID   *Context
> +  )
> +{
> +  EFI_MM_COMMUNICATE_HEADER   Header;
> +  UINTN   Size;
> +
> +  //
> +  // Use Guid to initialize EFI_SMM_COMMUNICATE_HEADER structure
> +  //
> +  CopyGuid (, Context);
> +  Header.MessageLength = 1;
> +  Header.Data[0] = 0;
> +
> +  Size = sizeof (Header);
> +  MmCommunicationCommunicate (, ,
> );
> +}
> +
>  /**
>The Entry Point for MM Communication
> 
> @@ -287,6 +324,7 @@ MmCommunicationInitialize (
>)
>  {
>EFI_STATUS Status;
> +  UINTN  Index;
> 
>// Check if we can make the MM call
>Status = GetMmCompatibility ();
> @@ -351,8 +389,13 @@ MmCommunicationInitialize (
>NULL,
>
>);
> -  if (Status == EFI_SUCCESS) {
> -return Status;
> +  ASSERT_EFI_ERROR (Status);
> +
> +  for (Index = 0; Index < ARRAY_SIZE (mGuidedEventGuid); Index++) {
> +Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
> +MmGuidedEventNotify, mGuidedEventGuid[Index],
> +mGuidedEventGuid[Index],
> [Index]);
> +ASSERT_EFI_ERROR (Status);
>}
> 
>gBS->UninstallProtocolInterface (
> --
> 2.20.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 06/10] StandaloneMmPkg/Core: permit encapsulated firmware volumes

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, March 5, 2019 5:33 AM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel ; Achin Gupta
> ; Supreeth Venkatesh
> ; Yao, Jiewen ;
> Leif Lindholm ; Jagadeesh Ujja
> 
> Subject: [PATCH 06/10] StandaloneMmPkg/Core: permit encapsulated
> firmware volumes
> 
> Standalone MM requires 4 KB section alignment for all images, so that
> strict permissions can be applied. Unfortunately, this results in a
> lot of wasted space, which is usually costly in the secure world
> environment that standalone MM is expected to operate in.
> 
> So let's permit the standalone MM drivers (but not the core) to be
> delivered in a compressed firmware volume.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  StandaloneMmPkg/Core/StandaloneMmCore.inf |  1 +
>  StandaloneMmPkg/Core/FwVol.c  | 99
> ++--
>  2 files changed, 91 insertions(+), 9 deletions(-)
> 
> diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.inf
> b/StandaloneMmPkg/Core/StandaloneMmCore.inf
> index ff2b8b9cef03..83d31e2d92c5 100644
> --- a/StandaloneMmPkg/Core/StandaloneMmCore.inf
> +++ b/StandaloneMmPkg/Core/StandaloneMmCore.inf
> @@ -49,6 +49,7 @@ [LibraryClasses]
>BaseMemoryLib
>CacheMaintenanceLib
>DebugLib
> +  ExtractGuidedSectionLib
>FvLib
>HobLib
>MemoryAllocationLib
> diff --git a/StandaloneMmPkg/Core/FwVol.c
> b/StandaloneMmPkg/Core/FwVol.c
> index 5abf98c24797..d95491f252f9 100644
> --- a/StandaloneMmPkg/Core/FwVol.c
> +++ b/StandaloneMmPkg/Core/FwVol.c
> @@ -14,6 +14,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> ANY KIND, EITHER EXPRESS OR IMPLIED.
> 
>  #include "StandaloneMmCore.h"
>  #include 
> +#include 
> 
>  //
>  // List of file types supported by dispatcher
> @@ -65,15 +66,25 @@ Returns:
> 
>  --*/
>  {
> -  EFI_STATUS  Status;
> -  EFI_STATUS  DepexStatus;
> -  EFI_FFS_FILE_HEADER *FileHeader;
> -  EFI_FV_FILETYPE FileType;
> -  VOID*Pe32Data;
> -  UINTN   Pe32DataSize;
> -  VOID*Depex;
> -  UINTN   DepexSize;
> -  UINTN   Index;
> +  EFI_STATUS  Status;
> +  EFI_STATUS  DepexStatus;
> +  EFI_FFS_FILE_HEADER *FileHeader;
> +  EFI_FV_FILETYPE FileType;
> +  VOID*Pe32Data;
> +  UINTN   Pe32DataSize;
> +  VOID*Depex;
> +  UINTN   DepexSize;
> +  UINTN   Index;
> +  EFI_COMMON_SECTION_HEADER   *Section;
> +  VOID*SectionData;
> +  UINTN   SectionDataSize;
> +  UINT32  DstBufferSize;
> +  VOID*ScratchBuffer;
> +  UINT32  ScratchBufferSize;
> +  VOID*DstBuffer;
> +  UINT16  SectionAttribute;
> +  UINT32  AuthenticationStatus;
> +  EFI_FIRMWARE_VOLUME_HEADER  *InnerFvHeader;
> 
>DEBUG ((DEBUG_INFO, "MmCoreFfsFindMmDriver - 0x%x\n",
> FwVolHeader));
> 
> @@ -83,6 +94,71 @@ Returns:
> 
>FvIsBeingProcesssed (FwVolHeader);
> 
> +  //
> +  // First check for encapsulated compressed firmware volumes
> +  //
> +  FileHeader = NULL;
> +  do {
> +Status = FfsFindNextFile
> (EFI_FV_FILETYPE_FIRMWARE_VOLUME_IMAGE,
> +   FwVolHeader, );
> +if (EFI_ERROR (Status)) {
> +  break;
> +}
> +Status = FfsFindSectionData (EFI_SECTION_GUID_DEFINED,
> FileHeader,
> +   , );
> +if (EFI_ERROR (Status)) {
> +  break;
> +}
> +Section = (EFI_COMMON_SECTION_HEADER *)(FileHeader + 1);
> +Status = ExtractGuidedSectionGetInfo (Section, ,
> +   , );
> +if (EFI_ERROR (Status)) {
> +  break;
> +}
> +
> +//
> +// Allocate scratch buffer
> +//
> +ScratchBuffer = (VOID *)(UINTN)AllocatePages (EFI_SIZE_TO_PAGES
> (ScratchBufferSize));
> +if (ScratchBuffer == NULL) {
> +  return EFI_OUT_OF_RESOURCES;
> +}
> +
> +//
> +// Allocate destination buffer, extra one page for adjustment
> +//
> +DstBuffer = (VOID *)(UINTN)AllocatePages (EFI_SIZE_TO_PAGES
> (DstBufferSize));
> +if (DstBuffer == NULL) {
> +  return EFI_OUT_OF_RESOURCES;
> +}
> +
> +//
> +// Call decompress function
> +//
> +Status = ExtractGuidedSectionDecode (Section, ,
> ScratchBuffer,
> +);
> +FreePages (ScratchBuffer, EFI_SIZE_TO_PAGES (ScratchBufferSize));
> +if (EFI_ERROR (Status)) {
> +  goto FreeDstBuffer;
> +}
> +
> +DEBUG ((DEBUG_INFO,
> +  

Re: [edk2] [PATCH 08/10] StandaloneMmPkg/Core: drop support for dispatching FVs into MM

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, March 5, 2019 5:33 AM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel ; Achin Gupta
> ; Supreeth Venkatesh
> ; Yao, Jiewen ;
> Leif Lindholm ; Jagadeesh Ujja
> 
> Subject: [PATCH 08/10] StandaloneMmPkg/Core: drop support for
> dispatching FVs into MM
> 
> Remove the support that permits calls into the MM context to dispatch
> firmware volumes that are not part of the initial standalone MM firmware
> volume.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  StandaloneMmPkg/Core/StandaloneMmCore.h | 22 --
>  StandaloneMmPkg/Core/Dispatcher.c   | 46 
>  StandaloneMmPkg/Core/StandaloneMmCore.c |  1 -
>  3 files changed, 69 deletions(-)
> 
> diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.h
> b/StandaloneMmPkg/Core/StandaloneMmCore.h
> index 0d20bcaa6be5..74338dc9da0d 100644
> --- a/StandaloneMmPkg/Core/StandaloneMmCore.h
> +++ b/StandaloneMmPkg/Core/StandaloneMmCore.h
> @@ -635,28 +635,6 @@ MmDriverDispatchHandler (
> 
>@return Status Code
> 
> -**/
> -EFI_STATUS
> -EFIAPI
> -MmFvDispatchHandler (
> -  IN EFI_HANDLE   DispatchHandle,
> -  IN CONST VOID   *Context,OPTIONAL
> -  IN OUT VOID *CommBuffer, OPTIONAL
> -  IN OUT UINTN*CommBufferSize  OPTIONAL
> -  );
> -
> -/**
> -  This function is the main entry point for an MM handler dispatch
> -  or communicate-based callback.
> -
> -  @param  DispatchHandle  The unique handle assigned to this handler
> by MmiHandlerRegister().
> -  @param  Context Points to an optional handler context
> which was specified when the handler was registered.
> -  @param  CommBuffer  A pointer to a collection of data in
> memory that will
> -  be conveyed from a non-MM environment
> into an MM environment.
> -  @param  CommBufferSize  The size of the CommBuffer.
> -
> -  @return Status Code
> -
>  **/
>  EFI_STATUS
>  EFIAPI
> diff --git a/StandaloneMmPkg/Core/Dispatcher.c
> b/StandaloneMmPkg/Core/Dispatcher.c
> index bede4832cfb7..4b2f38f700a0 100644
> --- a/StandaloneMmPkg/Core/Dispatcher.c
> +++ b/StandaloneMmPkg/Core/Dispatcher.c
> @@ -883,52 +883,6 @@ MmAddToDriverList (
>return EFI_SUCCESS;
>  }
> 
> -/**
> -  This function is the main entry point for an MM handler dispatch
> -  or communicate-based callback.
> -
> -  @param  DispatchHandle  The unique handle assigned to this handler
> by SmiHandlerRegister().
> -  @param  Context Points to an optional handler context
> which was specified when the handler was registered.
> -  @param  CommBuffer  A pointer to a collection of data in
> memory that will
> -  be conveyed from a non-MM environment
> into an MM environment.
> -  @param  CommBufferSize  The size of the CommBuffer.
> -
> -  @return Status Code
> -
> -**/
> -EFI_STATUS
> -EFIAPI
> -MmFvDispatchHandler (
> -  IN EFI_HANDLE   DispatchHandle,
> -  IN CONST VOID   *Context,OPTIONAL
> -  IN OUT VOID *CommBuffer, OPTIONAL
> -  IN OUT UINTN*CommBufferSize  OPTIONAL
> -  )
> -{
> -  EFI_STATUSStatus;
> -  EFI_MM_COMMUNICATE_FV_DISPATCH_DATA
> *CommunicationFvDispatchData;
> -  EFI_FIRMWARE_VOLUME_HEADER*FwVolHeader;
> -
> -  DEBUG ((DEBUG_INFO, "MmFvDispatchHandler\n"));
> -
> -  CommunicationFvDispatchData = CommBuffer;
> -
> -  DEBUG ((DEBUG_INFO, "  Dispatch - 0x%016lx - 0x%016lx\n",
> CommunicationFvDispatchData->Address,
> -  CommunicationFvDispatchData->Size));
> -
> -  FwVolHeader = (EFI_FIRMWARE_VOLUME_HEADER
> *)(UINTN)CommunicationFvDispatchData->Address;
> -
> -  MmCoreFfsFindMmDriver (FwVolHeader);
> -
> -  //
> -  // Execute the MM Dispatcher on any newly discovered FVs and
> previously
> -  // discovered MM drivers that have been discovered but not dispatched.
> -  //
> -  Status = MmDispatcher ();
> -
> -  return Status;
> -}
> -
>  /**
>Traverse the discovered list for any drivers that were discovered but not
> loaded
>because the dependency experessions evaluated to false.
> diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.c
> b/StandaloneMmPkg/Core/StandaloneMmCore.c
> index ec53b8d8bec4..766cdb5c848c 100644
> --- a/StandaloneMmPkg/Core/StandaloneMmCore.c
> +++ b/StandaloneMmPkg/Core/StandaloneMmCore.c
> @@ -99,7 +99,6 @@ BOOLEAN  mInLegacyBoot = FALSE;
>  // Table of MMI Handlers that are registered by the MM Core when it is
> initialized
>  //
>  MM_CORE_MMI_HANDLERS  mMmCoreMmiHandlers[] = {
> -  { MmFvDispatchHandler, ,
> NULL, TRUE  },
>{ MmReadyToLockHandler,,
> NULL, TRUE  },
>{ MmEndOfDxeHandler,   ,
> NULL, FALSE },
>{ MmLegacyBootHandler, ,
> NULL, FALSE },
> --
> 2.20.1

Re: [edk2] [PATCH 07/10] StandaloneMmPkg/Core: dispatch all drivers at init time

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com


> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, March 5, 2019 5:33 AM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel ; Achin Gupta
> ; Supreeth Venkatesh
> ; Yao, Jiewen ;
> Leif Lindholm ; Jagadeesh Ujja
> 
> Subject: [PATCH 07/10] StandaloneMmPkg/Core: dispatch all drivers at init
> time
> 
> Instead of deferring dispatch of the remaining MM drivers once the
> CPU driver has been dispatched, proceed and dispatch all drivers.
> This makes sense for standalone MM, since all dispatchable drivers
> should be present in the initial firmware volume anyway: dispatch
> of additional FVs originating in the non-secure side is not supported.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  StandaloneMmPkg/Core/Dispatcher.c   | 92 
>  StandaloneMmPkg/Core/StandaloneMmCore.c |  1 -
>  2 files changed, 93 deletions(-)
> 
> diff --git a/StandaloneMmPkg/Core/Dispatcher.c
> b/StandaloneMmPkg/Core/Dispatcher.c
> index 8a2ad5118d92..bede4832cfb7 100644
> --- a/StandaloneMmPkg/Core/Dispatcher.c
> +++ b/StandaloneMmPkg/Core/Dispatcher.c
> @@ -575,7 +575,6 @@ MmDispatcher (
>LIST_ENTRY*Link;
>EFI_MM_DRIVER_ENTRY  *DriverEntry;
>BOOLEAN   ReadyToRun;
> -  BOOLEAN   PreviousMmEntryPointRegistered;
> 
>DEBUG ((DEBUG_INFO, "MmDispatcher\n"));
> 
> @@ -639,11 +638,6 @@ MmDispatcher (
>DriverEntry->Initialized  = TRUE;
>RemoveEntryList (>ScheduledLink);
> 
> -  //
> -  // Cache state of MmEntryPointRegistered before calling entry point
> -  //
> -  PreviousMmEntryPointRegistered =
> gMmCorePrivate->MmEntryPointRegistered;
> -
>//
>// For each MM driver, pass NULL as ImageHandle
>//
> @@ -661,20 +655,6 @@ MmDispatcher (
>  DEBUG ((DEBUG_INFO, "StartImage Status - %r\n", Status));
>  MmFreePages(DriverEntry->ImageBuffer,
> DriverEntry->NumberOfPage);
>}
> -
> -  if (!PreviousMmEntryPointRegistered &&
> gMmCorePrivate->MmEntryPointRegistered) {
> -//
> -// Return immediately if the MM Entry Point was registered by the
> MM
> -// Driver that was just dispatched.  The MM IPL will reinvoke the
> MM
> -// Core Dispatcher.  This is required so MM Mode may be
> enabled as soon
> -// as all the dependent MM Drivers for MM Mode have been
> dispatched.
> -// Once the MM Entry Point has been registered, then MM Mode
> will be
> -// used.
> -//
> -gRequestDispatch = TRUE;
> -gDispatcherRunning = FALSE;
> -return EFI_NOT_READY;
> -  }
>  }
> 
>  //
> @@ -903,78 +883,6 @@ MmAddToDriverList (
>return EFI_SUCCESS;
>  }
> 
> -/**
> -  This function is the main entry point for an MM handler dispatch
> -  or communicate-based callback.
> -
> -  Event notification that is fired every time a FV dispatch protocol is 
> added.
> -  More than one protocol may have been added when this event is fired, so
> you
> -  must loop on MmLocateHandle () to see how many protocols were added
> and
> -  do the following to each FV:
> -  If the Fv has already been processed, skip it. If the Fv has not been
> -  processed then mark it as being processed, as we are about to process it.
> -  Read the Fv and add any driver in the Fv to the mDiscoveredList.The
> -  mDiscoveredList is never free'ed and contains variables that define
> -  the other states the MM driver transitions to..
> -  While you are at it read the A Priori file into memory.
> -  Place drivers in the A Priori list onto the mScheduledQueue.
> -
> -  @param  DispatchHandle  The unique handle assigned to this handler
> by SmiHandlerRegister().
> -  @param  Context Points to an optional handler context
> which was specified when the handler was registered.
> -  @param  CommBuffer  A pointer to a collection of data in
> memory that will
> -  be conveyed from a non-MM environment
> into an MM environment.
> -  @param  CommBufferSize  The size of the CommBuffer.
> -
> -  @return Status Code
> -
> -**/
> -EFI_STATUS
> -EFIAPI
> -MmDriverDispatchHandler (
> -  IN EFI_HANDLE  DispatchHandle,
> -  IN CONST VOID  *Context,OPTIONAL
> -  IN OUT VOID*CommBuffer, OPTIONAL
> -  IN OUT UINTN   *CommBufferSize  OPTIONAL
> -  )
> -{
> -  EFI_STATUSStatus;
> -
> -  DEBUG ((DEBUG_INFO, "MmDriverDispatchHandler\n"));
> -
> -  //
> -  // Execute the MM Dispatcher on any newly discovered FVs and
> previously
> -  // discovered MM drivers that have been discovered but not dispatched.
> -  //
> -  Status = MmDispatcher ();
> -
> -  //
> -  // Check to see if CommBuffer and CommBufferSize are valid
> -  //
> -  if (CommBuffer != NULL && CommBufferSize != NULL) {
> -if (*CommBufferSize > 0) {
> -  if (Status == EFI_NOT_READY) 

Re: [edk2] [Patch] Document: Update Build spec to remove EDK related contents

[Carsey, Jaben]

Reviewed-by: Jaben Carsey 

> -Original Message-
> From: Feng, Bob C
> Sent: Monday, March 04, 2019 9:28 PM
> To: edk2-devel@lists.01.org
> Cc: Feng, Bob C ; Gao, Liming
> ; Carsey, Jaben 
> Subject: [Patch] Document: Update Build spec to remove EDK related
> contents
> Importance: High
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1453
> 
> Remove EDK related contents from Build spec.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Bob Feng 
> Cc: Liming Gao 
> Cc: Jaben Carsey 
> ---
>  .../103_build_intermediate_images.md  |  3 +-
>  12_build_changes_and_customizations/README.md |  4 +-
>  .../42_build_process_overview.md  |  6 +-
>  .../46_file_specifications.md | 20 +
>  6_quick_start/61_environment_variables.md | 24 +-
>  .../82_auto-generation_process.md | 76 +++
>  .../83_auto-generated_code.md | 43 +++
>  .../85_auto-generated_makefiles.md| 23 +++---
>  9_build_or_make_stage/README.md   |  9 +--
>  appendix_a_variables.md   |  3 +-
>  10 files changed, 43 insertions(+), 168 deletions(-)
> 
> diff --git a/10_post-build_imagegen_stage_-
> _flash/103_build_intermediate_images.md b/10_post-
> build_imagegen_stage_-_flash/103_build_intermediate_images.md
> index 5f5aefc..9253cde 100644
> --- a/10_post-build_imagegen_stage_-
> _flash/103_build_intermediate_images.md
> +++ b/10_post-build_imagegen_stage_-
> _flash/103_build_intermediate_images.md
> @@ -1,9 +1,9 @@
>  
> 
>  # 12 Build Changes and Customizations
> 
>  This chapter deals with customizing a build, including options and settings 
> for
> -debugging, using custom tools as well as how to customize EDK component
> builds
> +debugging, using custom tools.
> diff --git
> a/4_edk_ii_build_process_overview/42_build_process_overview.md
> b/4_edk_ii_build_process_overview/42_build_process_overview.md
> index d0725d3..17ed278 100644
> --- a/4_edk_ii_build_process_overview/42_build_process_overview.md
> +++ b/4_edk_ii_build_process_overview/42_build_process_overview.md
> @@ -1,9 +1,9 @@
>  
> 
>  ## 4.2 Build Process Overview
> 
>  Prior to executing a build command, specific system environment variables
> must
> -be initialized: `WORKSPACE`, `EDK_TOOLS_PATH` are required for all builds,
> -while `ECP_SOURCE`, `EFI_SOURCE` and `EDK_SOURCE` are only required to
> build
> -EDK II platforms that contain EDK components and EDK libraries.
> Additionally,
> +be initialized: `WORKSPACE`, `EDK_TOOLS_PATH` are required for all builds.
> Additionally,
>  the provided EDK II tool set must be present in a directory that is in the
>  system environment variable: PATH. The edksetup scripts provided in the
> root
>  directory of the EDK II development tree will set the `WORKSPACE` and
>  `EDK_TOOLS_PATH`, as well as modify the system environment variable,
> PATH to
>  ensure that the tools can execute. Refer to "_Build Environment_" for more
> diff --git a/4_edk_ii_build_process_overview/46_file_specifications.md
> b/4_edk_ii_build_process_overview/46_file_specifications.md
> index a606488..f30f806 100644
> --- a/4_edk_ii_build_process_overview/46_file_specifications.md
> +++ b/4_edk_ii_build_process_overview/46_file_specifications.md
> @@ -1,9 +1,9 @@
>   ---:|
>  | `$(WORKSPACE)`  | `%WORKSPACE%`| 
> `$WORKSPACE`
> |
> -| `$(EFI_SOURCE)` | `%EFI_SOURCE%`   | 
> `$EFI_SOURCE`
> |
> -| `$(EDK_SOURCE)` | `%EDK_SOURCE%`   | 
> `$EDK_SOURCE`
> |
>  | `$(EDK_TOOLS_PATH)` | `%EDK_TOOLS_PATH%`   |
> `$EDK_TOOLS_PATH` |
> -| `$(ECP_SOURCE)` | `%ECP_SOURCE%`   | 
> `$ECP_SOURCE`
> |
> 
>  **
>  **Note:** The `PACKAGES_PATH` and `EDK_TOOLS_BIN` system
> environment variables
>  shall not be referenced in EDK II meta-data files.
>  **
> @@ -413,13 +407,10 @@ conditional directives. Macros can also be defined
> or used in the `[Defines]`,
>  `[LibraryClasses]`, `[Libraries]`, `[Components]` and all PCD sections.
> 
>  Macros defined by the user may be used in the !include statements in DSC
> and
>  FDF files.
> 
> -`EDK_GLOBAL` type macros defined in the DSC file can be used in later
> sections
> -of the DSC, FDF and any of the included EDK INF files.
> -
>  Macro values must be defined prior to using them in directive statements or
> for
>  PCD values. The following provides the precedence (high to low) for
> obtaining
>  macro values.
> 
>  * Command-line, `-D` flags (left most has higher priority)
> @@ -480,15 +471,11 @@ internal build tools. These macros must not be
> redefined.
>  | - | 
> ---
> --
> 

Re: [edk2] [Patch] Document: Update DSC spec to remove EDK related contents

[Carsey, Jaben]

Reviewed-by: Jaben Carsey 

It's a lot of changes. It looks good to me.

> -Original Message-
> From: Feng, Bob C
> Sent: Monday, March 04, 2019 7:49 PM
> To: edk2-devel@lists.01.org
> Cc: Feng, Bob C ; Gao, Liming
> ; Carsey, Jaben 
> Subject: [Patch] Document: Update DSC spec to remove EDK related
> contents
> Importance: High
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1453
> 
> Remove EDK related contents inf Dsc spec.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Bob Feng 
> Cc: Liming Gao 
> Cc: Jaben Carsey 
> ---
>  1_introduction/11_overview.md | 14 +--
>  ...=> 210_[components]_section_processing.md} | 27 +-
>  ...ion.md => 211_[userextensions]_section.md} |  4 +-
>  ...212_[defaultstores]_section_processing.md} |  4 +-
>  .../22_build_description_file_format.md   | 50 ++
>  .../23_[defines]_section_processing.md| 12 +--
>  2_dsc_overview/24_[buildoptions]_section.md   | 72 ++
>  .../26_[libraries]_section_processing.md  | 69 --
>  ...26_[libraryclasses]_section_processing.md} |  4 +-
>  ...essing.md => 27_pcd_section_processing.md} | 34 +++
>  ...{29_pcd_sections.md => 28_pcd_sections.md} | 26 ++---
>  ...210_pcd_database.md => 29_pcd_database.md} |  4 +-
>  ...ctions.md => 310_[components]_sections.md} | 62 +---
>  ...ns.md => 311_[userextensions]_sections.md} |  4 +-
>  ...tion.md => 312_[defaultstores]_section.md} |  4 +-
>  3_edk_ii_dsc_file_format/32_general_rules.md  | 13 +--
>  .../33_platform_dsc_definition.md | 17 +---
>  .../35_[defines]_section.md   | 12 +--
>  .../36_[buildoptions]_sections.md | 19 ++--
>  .../38_[libraries]_sections.md| 94 ---
>  ...ons.md => 38_[libraryclasses]_sections.md} |  4 +-
>  ...310_pcd_sections.md => 39_pcd_sections.md} | 14 +--
>  22 files changed, 101 insertions(+), 462 deletions(-)
>  rename 2_dsc_overview/{211_[components]_section_processing.md =>
> 210_[components]_section_processing.md} (84%)
>  rename 2_dsc_overview/{212_[userextensions]_section.md =>
> 211_[userextensions]_section.md} (93%)
>  rename 2_dsc_overview/{213_[defaultstores]_section_processing.md =>
> 212_[defaultstores]_section_processing.md} (93%)
>  delete mode 100644 2_dsc_overview/26_[libraries]_section_processing.md
>  rename 2_dsc_overview/{27_[libraryclasses]_section_processing.md =>
> 26_[libraryclasses]_section_processing.md} (96%)
>  rename 2_dsc_overview/{28_pcd_section_processing.md =>
> 27_pcd_section_processing.md} (94%)
>  rename 2_dsc_overview/{29_pcd_sections.md => 28_pcd_sections.md}
> (93%)
>  rename 2_dsc_overview/{210_pcd_database.md => 29_pcd_database.md}
> (96%)
>  rename 3_edk_ii_dsc_file_format/{311_[components]_sections.md =>
> 310_[components]_sections.md} (81%)
>  rename 3_edk_ii_dsc_file_format/{312_[userextensions]_sections.md =>
> 311_[userextensions]_sections.md} (94%)
>  rename 3_edk_ii_dsc_file_format/{313_[defaultstores]_section.md =>
> 312_[defaultstores]_section.md} (93%)
>  delete mode 100644 3_edk_ii_dsc_file_format/38_[libraries]_sections.md
>  rename 3_edk_ii_dsc_file_format/{39_[libraryclasses]_sections.md =>
> 38_[libraryclasses]_sections.md} (95%)
>  rename 3_edk_ii_dsc_file_format/{310_pcd_sections.md =>
> 39_pcd_sections.md} (97%)
> 
> diff --git a/1_introduction/11_overview.md
> b/1_introduction/11_overview.md
> index d9006df..ff2b517 100644
> --- a/1_introduction/11_overview.md
> +++ b/1_introduction/11_overview.md
> @@ -1,9 +1,9 @@
>   |:-:|::|:-:|::|:-:|:
> :|
>  | EDK Build Tools| YES   | NO   | YES   | NO   | 
> YES   | NO   |
> -| EDK II Build Tools | NO| YES  | NO| YES  | 
> YES   | YES
> |
> +| EDK II Build Tools | NO| YES  | NO| YES  | 
> NO| YES
> |
> 
>  **
>  **Note:** This document is intended for persons doing EFI development
> and
>  support for different platforms. It is most likely only of interest in the
>  event that there is a problem with a build, or if a developer needs to
> perform
> @@ -90,9 +87,6 @@ contain information on the EFI format for FFS or FV file
> creation. The
>  Makefiles will support third party compilation tools - Microsoft, Intel and
> GCC
>  tool chains - and at least one EDK II tool, GenFw. The GenFw tool is used to
>  manipulate the files emitted from the compilation tools.
> 
>  The EDK II build provides UEFI and PI (Unified EFI, Inc.)
> -specification-compliant images. Use of the tools in the EDK Compatibility
> -package can be used for creating earlier versions of EFI 1.10 and/or UEFI 2.0
> -compliant components. To be clear, EDK II tools do not have the limitation of
> -ECP tools, which can only do EFI 1.10 and UEFI 2.0 images.
> +specification-compliant images.
> diff --git 

Re: [edk2] [PATCH] ShellPkg: Correct a parameter's name

[Carsey, Jaben]

Reviewed-by: Jaben Carsey 

> -Original Message-
> From: Zhang, Shenglei
> Sent: Monday, March 04, 2019 6:06 PM
> To: edk2-devel@lists.01.org
> Cc: Carsey, Jaben ; Ni, Ray 
> Subject: [PATCH] ShellPkg: Correct a parameter's name
> Importance: High
> 
> The parameter FilePath of ShellOpenFileByName defined in
> ShellLib.h is incorrect. It should be FileName.
> https://bugzilla.tianocore.org/show_bug.cgi?id=1221
> 
> Cc: Jaben Carsey 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Shenglei Zhang 
> ---
>  ShellPkg/Include/Library/ShellLib.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/ShellPkg/Include/Library/ShellLib.h
> b/ShellPkg/Include/Library/ShellLib.h
> index 2ecc5ee006..78bdcc8c53 100644
> --- a/ShellPkg/Include/Library/ShellLib.h
> +++ b/ShellPkg/Include/Library/ShellLib.h
> @@ -161,7 +161,7 @@ ShellOpenFileByDevicePath(
>otherwise, the Filehandle is NULL. Attributes is valid only for
>EFI_FILE_MODE_CREATE.
> 
> -  @param[in] FilePath   The pointer to file name.
> +  @param[in] FileName   The pointer to file name.
>@param[out] FileHandleThe pointer to the file handle.
>@param[in] OpenMode   The mode to open the file with.
>@param[in] Attributes The file's file attributes.
> @@ -186,7 +186,7 @@ ShellOpenFileByDevicePath(
>  EFI_STATUS
>  EFIAPI
>  ShellOpenFileByName(
> -  IN CONST CHAR16   *FilePath,
> +  IN CONST CHAR16   *FileName,
>OUT SHELL_FILE_HANDLE *FileHandle,
>IN UINT64 OpenMode,
>IN UINT64 Attributes
> --
> 2.18.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v3] StandaloneMmPkg/Library: Install Variable Arch Protocol

[Jagadeesh Ujja]

On Tue, Mar 5, 2019 at 7:39 PM Jagadeesh Ujja  wrote:
>
Adding Achin, Jiewen
> hi Jiewen, Achin
>
> On Mon, Mar 4, 2019 at 4:16 PM Ard Biesheuvel  
> wrote:
> >
> > (add StandaloneMmPkg maintainers)
> >
> Please let me know if you have any comments on this patch
>
> > On Mon, 4 Mar 2019 at 09:54, Jagadeesh Ujja  wrote:
> > >
> > > In a system implementing the variable store in MM, there are no variable
> > > arch protocol and variable write arch protocol installed into the
> > > DXE_SMM protocol database. On such systems, it is not required to
> > > locate these protocols by the DXE runtime variable drivers because
> > > it can be assumed that these protocols are already installed in the
> > > MM context. But then such an implementation will deviate from the
> > > existing traditional MM based variable driver implementation.
> > >
> > > So in order to maintain consistency with the traditional MM variable
> > > driver implementation, allow platforms to install these protocols into
> > > the DXE protocol database but these protocol will not be consumed
> > > by non-secure variable service runtime driver.
> > >
> > > The Platform which uses StandaloneMM based secure variable storage
> > > have to include this library
> > >
> > > Example
> > > In edk2-platforms/Platform/ARM/SgiPkg/SgiPlatform.dsc
> > >
> > >   ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf {
> > > 
> > >   
> > > NULL|StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.inf
> > >   }
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Jagadeesh Ujja 
> >
> > Reviewed-by: Ard Biesheuvel 
> >
> > > ---
> > > Changes since v2:
> > > - Addressed the comments from Ard Biesheuvel and Zeng Star
> > >
> > > Changes since v1:
> > > - This is a next version of patch
> > >“MdeModulePkg/VariableSmmRuntimeDxe: Refactor locating Variable Arch 
> > > Protocol”.
> > >[https://lists.01.org/pipermail/edk2-devel/2019-February/036885.html]
> > > - Addressed the comments from Ard Biesheuvel and Zeng Star
> > > - Can this library be placed in MdePkg rather then the StandaloneMmPkg?
> > >
> > >  StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.c   | 
> > > 54 
> > >  StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.inf | 
> > > 46 +
> > >  2 files changed, 100 insertions(+)
> > >
> > > diff --git 
> > > a/StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.c 
> > > b/StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.c
> > > new file mode 100644
> > > index 000..7e0f31b
> > > --- /dev/null
> > > +++ b/StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.c
> > > @@ -0,0 +1,54 @@
> > > +/** @file
> > > +  Runtime DXE part corresponding to StanaloneMM variable module.
> > > +
> > > +This module installs variable arch protocol and variable write arch 
> > > protocol
> > > +to StandaloneMM runtime variable service.
> > > +
> > > +Copyright (c) 2019, ARM Ltd. All rights reserved.
> > > +
> > > +This program and the accompanying materials
> > > +are licensed and made available under the terms and conditions of the 
> > > BSD License
> > > +which accompanies this distribution.  The full text of the license may 
> > > be found at
> > > +http://opensource.org/licenses/bsd-license.php.
> > > +
> > > +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > > +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> > > IMPLIED.
> > > +
> > > +**/
> > > +
> > > +#include 
> > > +#include 
> > > +
> > > +/**
> > > +  The constructor function installs variable arch protocol and variable
> > > +  write arch protocol to StandaloneMM runtime variable service
> > > +
> > > +  @param  ImageHandle   The firmware allocated handle for the EFI image.
> > > +  @param  SystemTable   A pointer to the Management mode System Table.
> > > +
> > > +  @retval EFI_SUCCESS   The constructor always returns EFI_SUCCESS.
> > > +
> > > +**/
> > > +EFI_STATUS
> > > +EFIAPI
> > > +VariableMmDependencyLibConstructor (
> > > +  IN EFI_HANDLE   ImageHandle,
> > > +  IN EFI_SYSTEM_TABLE *SystemTable
> > > +  )
> > > +{
> > > +  EFI_STATUSStatus;
> > > +  EFI_HANDLEHandle;
> > > +
> > > +  Handle = NULL;
> > > +  Status = gBS->InstallMultipleProtocolInterfaces (
> > > +  ,
> > > +  ,
> > > +  NULL,
> > > +  ,
> > > +  NULL,
> > > +  NULL
> > > +  );
> > > +  ASSERT_EFI_ERROR (Status);
> > > +  return EFI_SUCCESS;
> > > +}
> > > +
> > > diff --git 
> > > a/StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.inf 
> > > b/StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.inf
> > > new file mode 100644
> > > index 000..e71c44d
> > > --- /dev/null
> > > +++ 
> > > 

Re: [edk2] [PATCH 04/10] StandaloneMmPkg: remove redundant StandaloneMmDriverEntryPoint driver

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, March 5, 2019 5:33 AM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel ; Achin Gupta
> ; Supreeth Venkatesh
> ; Yao, Jiewen ;
> Leif Lindholm ; Jagadeesh Ujja
> 
> Subject: [PATCH 04/10] StandaloneMmPkg: remove redundant
> StandaloneMmDriverEntryPoint driver
> 
> StandaloneMmDriverEntryPoint is implemented in MdePkg now, so let's
> drop the redundant StandaloneMmPkg version.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
> 
> StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmD
> riverEntryPoint.inf | 41 
> 
> StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmD
> riverEntryPoint.c   | 99 
>  2 files changed, 140 deletions(-)
> 
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneM
> mDriverEntryPoint.inf
> b/StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneM
> mDriverEntryPoint.inf
> deleted file mode 100644
> index 4d1896db10ba..
> ---
> a/StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneM
> mDriverEntryPoint.inf
> +++ /dev/null
> @@ -1,41 +0,0 @@
> -## @file
> -# Module entry point library for Standalone MM driver.
> -#
> -# Copyright (c) 2015, Intel Corporation. All rights reserved.
> -# Copyright (c) 2016-2018, ARM Ltd. All rights reserved.
> -#
> -#  This program and the accompanying materials
> -#  are licensed and made available under the terms and conditions of the
> BSD License
> -#  which accompanies this distribution. The full text of the license may be
> found at
> -#  http://opensource.org/licenses/bsd-license.php.
> -#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> -#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> -#
> -#
> -##
> -
> -[Defines]
> -  INF_VERSION= 0x0001001A
> -  BASE_NAME  = StandaloneMmDriverEntryPoint
> -  FILE_GUID  =
> BBC33478-98F8-4B78-B29D-574D681B7E43
> -  MODULE_TYPE= MM_STANDALONE
> -  VERSION_STRING = 1.0
> -  PI_SPECIFICATION_VERSION   = 0x00010032
> -  LIBRARY_CLASS  =
> StandaloneMmDriverEntryPoint|MM_STANDALONE
> -
> -#
> -# The following information is for reference only and not required by the
> build tools.
> -#
> -#  VALID_ARCHITECTURES   = IA32 X64 IPF EBC
> -#
> -
> -[Sources]
> -  StandaloneMmDriverEntryPoint.c
> -
> -[Packages]
> -  MdePkg/MdePkg.dec
> -
> -[LibraryClasses]
> -  BaseLib
> -  DebugLib
> -
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneM
> mDriverEntryPoint.c
> b/StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneM
> mDriverEntryPoint.c
> deleted file mode 100644
> index 64bffcfccc8a..
> ---
> a/StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneM
> mDriverEntryPoint.c
> +++ /dev/null
> @@ -1,99 +0,0 @@
> -/** @file
> -  Entry point to a Standalone MM driver.
> -
> -Copyright (c) 2015, Intel Corporation. All rights reserved.
> -Copyright (c) 2016 - 2018, ARM Ltd. All rights reserved.
> -
> -This program and the accompanying materials
> -are licensed and made available under the terms and conditions of the BSD
> License
> -which accompanies this distribution.  The full text of the license may be
> found at
> -http://opensource.org/licenses/bsd-license.php
> -
> -THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> -WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> -
> -**/
> -
> -#include 
> -
> -#include 
> -#include 
> -
> -VOID
> -EFIAPI
> -ProcessLibraryConstructorList (
> -  IN EFI_HANDLE   ImageHandle,
> -  IN IN EFI_MM_SYSTEM_TABLE   *MmSystemTable
> -  );
> -
> -EFI_STATUS
> -EFIAPI
> -ProcessModuleEntryPointList (
> -  IN EFI_HANDLE   ImageHandle,
> -  IN IN EFI_MM_SYSTEM_TABLE   *MmSystemTable
> -  );
> -
> -VOID
> -EFIAPI
> -ProcessLibraryDestructorList (
> -  IN EFI_HANDLE   ImageHandle,
> -  IN IN EFI_MM_SYSTEM_TABLE   *MmSystemTable
> -  );
> -
> -/**
> -  The entry point of PE/COFF Image for a Standalone MM Driver.
> -
> -  This function is the entry point for a Standalone MM Driver.
> -  This function must call ProcessLibraryConstructorList() and
> -  ProcessModuleEntryPointList().
> -  If the return status from ProcessModuleEntryPointList()
> -  is an error status, then ProcessLibraryDestructorList() must be called.
> -  The return value from ProcessModuleEntryPointList() is returned.
> -  If _gDriverUnloadImageCount is greater than zero, then an unload
> -  handler must be registered for this image
> -  and the unload handler must invoke ProcessModuleUnloadList().
> -  If _gUefiDriverRevision is not zero and SystemTable->Hdr.Revision is less
> -  than 

Re: [edk2] [PATCH 03/10] StandaloneMmPkg: switch to NULL DebugLib resolution

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, March 5, 2019 5:33 AM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel ; Achin Gupta
> ; Supreeth Venkatesh
> ; Yao, Jiewen ;
> Leif Lindholm ; Jagadeesh Ujja
> 
> Subject: [PATCH 03/10] StandaloneMmPkg: switch to NULL DebugLib
> resolution
> 
> Building StandaloneMmPkg from its .DSC is mainly intended for build
> coverage, and so platform specific configuration such as UART addresses
> don't belong here.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  StandaloneMmPkg/StandaloneMmPkg.dsc | 11 +--
>  1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/StandaloneMmPkg/StandaloneMmPkg.dsc
> b/StandaloneMmPkg/StandaloneMmPkg.dsc
> index f279d9e7e5c7..8def9688fe7d 100644
> --- a/StandaloneMmPkg/StandaloneMmPkg.dsc
> +++ b/StandaloneMmPkg/StandaloneMmPkg.dsc
> @@ -43,7 +43,7 @@ [LibraryClasses]
>#
>BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
>BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> -
> DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.
> inf
> +  DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
> 
> DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/Bas
> eDebugPrintErrorLevelLib.inf
>FvLib|StandaloneMmPkg/Library/FvLib/FvLib.inf
> 
> HobLib|StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneM
> mCoreHobLib.inf
> @@ -66,10 +66,6 @@ [LibraryClasses.AARCH64]
>ArmSvcLib|ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf
> 
> CacheMaintenanceLib|ArmPkg/Library/ArmCacheMaintenanceLib/ArmCach
> eMaintenanceLib.inf
> 
> PeCoffExtraActionLib|StandaloneMmPkg/Library/StandaloneMmPeCoffExtr
> aActionLib/StandaloneMmPeCoffExtraActionLib.inf
> -  PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
> -
> PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011Uart
> ClockLib.inf
> -  # ARM PL011 UART Driver
> -
> SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLi
> b.inf
> 
> 
> StandaloneMmCoreEntryPoint|StandaloneMmPkg/Library/StandaloneMmC
> oreEntryPoint/StandaloneMmCoreEntryPoint.inf
> 
> @@ -83,11 +79,6 @@ [PcdsFixedAtBuild]
>gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xff
>gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x0f
> 
> -[PcdsFixedAtBuild.AARCH64]
> -  ## PL011 - Serial Terminal
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x1c0b
> -  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
> -
> 
> ##
> #
>  #
>  # Components Section - list of the modules and components that will be
> processed by compilation
> --
> 2.20.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v3] StandaloneMmPkg/Library: Install Variable Arch Protocol

[Jagadeesh Ujja]

hi Jiewen, Achin

On Mon, Mar 4, 2019 at 4:16 PM Ard Biesheuvel  wrote:
>
> (add StandaloneMmPkg maintainers)
>
Please let me know if you have any comments on this patch

> On Mon, 4 Mar 2019 at 09:54, Jagadeesh Ujja  wrote:
> >
> > In a system implementing the variable store in MM, there are no variable
> > arch protocol and variable write arch protocol installed into the
> > DXE_SMM protocol database. On such systems, it is not required to
> > locate these protocols by the DXE runtime variable drivers because
> > it can be assumed that these protocols are already installed in the
> > MM context. But then such an implementation will deviate from the
> > existing traditional MM based variable driver implementation.
> >
> > So in order to maintain consistency with the traditional MM variable
> > driver implementation, allow platforms to install these protocols into
> > the DXE protocol database but these protocol will not be consumed
> > by non-secure variable service runtime driver.
> >
> > The Platform which uses StandaloneMM based secure variable storage
> > have to include this library
> >
> > Example
> > In edk2-platforms/Platform/ARM/SgiPkg/SgiPlatform.dsc
> >
> >   ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf {
> > 
> >   
> > NULL|StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.inf
> >   }
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Jagadeesh Ujja 
>
> Reviewed-by: Ard Biesheuvel 
>
> > ---
> > Changes since v2:
> > - Addressed the comments from Ard Biesheuvel and Zeng Star
> >
> > Changes since v1:
> > - This is a next version of patch
> >“MdeModulePkg/VariableSmmRuntimeDxe: Refactor locating Variable Arch 
> > Protocol”.
> >[https://lists.01.org/pipermail/edk2-devel/2019-February/036885.html]
> > - Addressed the comments from Ard Biesheuvel and Zeng Star
> > - Can this library be placed in MdePkg rather then the StandaloneMmPkg?
> >
> >  StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.c   | 54 
> > 
> >  StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.inf | 46 
> > +
> >  2 files changed, 100 insertions(+)
> >
> > diff --git 
> > a/StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.c 
> > b/StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.c
> > new file mode 100644
> > index 000..7e0f31b
> > --- /dev/null
> > +++ b/StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.c
> > @@ -0,0 +1,54 @@
> > +/** @file
> > +  Runtime DXE part corresponding to StanaloneMM variable module.
> > +
> > +This module installs variable arch protocol and variable write arch 
> > protocol
> > +to StandaloneMM runtime variable service.
> > +
> > +Copyright (c) 2019, ARM Ltd. All rights reserved.
> > +
> > +This program and the accompanying materials
> > +are licensed and made available under the terms and conditions of the BSD 
> > License
> > +which accompanies this distribution.  The full text of the license may be 
> > found at
> > +http://opensource.org/licenses/bsd-license.php.
> > +
> > +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> > IMPLIED.
> > +
> > +**/
> > +
> > +#include 
> > +#include 
> > +
> > +/**
> > +  The constructor function installs variable arch protocol and variable
> > +  write arch protocol to StandaloneMM runtime variable service
> > +
> > +  @param  ImageHandle   The firmware allocated handle for the EFI image.
> > +  @param  SystemTable   A pointer to the Management mode System Table.
> > +
> > +  @retval EFI_SUCCESS   The constructor always returns EFI_SUCCESS.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +VariableMmDependencyLibConstructor (
> > +  IN EFI_HANDLE   ImageHandle,
> > +  IN EFI_SYSTEM_TABLE *SystemTable
> > +  )
> > +{
> > +  EFI_STATUSStatus;
> > +  EFI_HANDLEHandle;
> > +
> > +  Handle = NULL;
> > +  Status = gBS->InstallMultipleProtocolInterfaces (
> > +  ,
> > +  ,
> > +  NULL,
> > +  ,
> > +  NULL,
> > +  NULL
> > +  );
> > +  ASSERT_EFI_ERROR (Status);
> > +  return EFI_SUCCESS;
> > +}
> > +
> > diff --git 
> > a/StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.inf 
> > b/StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.inf
> > new file mode 100644
> > index 000..e71c44d
> > --- /dev/null
> > +++ b/StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.inf
> > @@ -0,0 +1,46 @@
> > +## @file
> > +#  Runtime DXE part corresponding to StanaloneMM variable module.
> > +#
> > +#  This module installs variable arch protocol and variable write arch 
> > protocol
> > +#  to StandaloneMM runtime variable service.
> > +#
> > +# Copyright (c) 2019, ARM Ltd. 

Re: [edk2] [PATCH v2 0/1] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems

[Ashish Singhal]

Ok with me.

Get Outlook for iOS


From: Ard Biesheuvel 
Sent: Tuesday, March 5, 2019 6:39 AM
To: Wu, Hao A
Cc: edk2-devel@lists.01.org; Eugene Cohen; Ashish Singhal
Subject: Re: [PATCH v2 0/1] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 
64-bit systems

On Tue, 5 Mar 2019 at 03:18, Wu, Hao A  wrote:
>
> > -Original Message-
> > From: Wu, Hao A
> > Sent: Tuesday, March 05, 2019 9:14 AM
> > To: edk2-devel@lists.01.org
> > Cc: Wu, Hao A; Eugene Cohen; Ard Biesheuvel; Ashish Singhal
> > Subject: [PATCH v2 0/1] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC
> > v3 64-bit systems
>
> Since Ashish already posted a patch to add the 64-bit System Address
> support for V3 mode SDHC:
> https://www.mail-archive.com/edk2-devel@lists.01.org/msg52057.html
>
> I think this patch can be dropped.
>
> But since Ashish's patch above is considered as a new feature addition, it
> will be pushed (if passes the review process) after the 19`Q1 release tag.
>
> So Eugene, Ard and Ashish, do you have concern on this?
>

That works for me.

---
This email message is for the sole use of the intended recipient(s) and may 
contain
confidential information.  Any unauthorized review, use, disclosure or 
distribution
is prohibited.  If you are not the intended recipient, please contact the 
sender by
reply email and destroy all copies of the original message.
---
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 02/10] StandaloneMmPkg: drop unused PCD PcdStandaloneMmEnable

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Tuesday, March 5, 2019 5:33 AM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen 
> Subject: [edk2] [PATCH 02/10] StandaloneMmPkg: drop unused PCD
> PcdStandaloneMmEnable
> 
> The PCD PcdStandaloneMmEnable is unused, and shouldn't exist in the
> first place since the value is implied by the context (it is never
> valid to set it to FALSE for standalone MM or TRUE for traditional
> MM). So drop it.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  StandaloneMmPkg/StandaloneMmPkg.dec
> | 3 ---
>  StandaloneMmPkg/StandaloneMmPkg.dsc
> | 3 ---
> 
> StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/Standalone
> MmPeCoffExtraActionLib.inf | 3 ---
>  3 files changed, 9 deletions(-)
> 
> diff --git a/StandaloneMmPkg/StandaloneMmPkg.dec
> b/StandaloneMmPkg/StandaloneMmPkg.dec
> index 0b5fbf9e1767..2d178c5e20a6 100644
> --- a/StandaloneMmPkg/StandaloneMmPkg.dec
> +++ b/StandaloneMmPkg/StandaloneMmPkg.dec
> @@ -39,6 +39,3 @@ [Guids]
>gEfiStandaloneMmNonSecureBufferGuid  = { 0xf00497e3, 0xbfa2,
> 0x41a1, { 0x9d, 0x29, 0x54, 0xc2, 0xe9, 0x37, 0x21, 0xc5 }}
>gEfiArmTfCpuDriverEpDescriptorGuid   = { 0x6ecbd5a1, 0xc0f8,
> 0x4702, { 0x83, 0x01, 0x4f, 0xc2, 0xc5, 0x47, 0x0a, 0x51 }}
> 
> -[PcdsFeatureFlag]
> -
> gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable|FALSE|BOOL
> EAN|0x0001
> -
> diff --git a/StandaloneMmPkg/StandaloneMmPkg.dsc
> b/StandaloneMmPkg/StandaloneMmPkg.dsc
> index e8d71ad56f52..f279d9e7e5c7 100644
> --- a/StandaloneMmPkg/StandaloneMmPkg.dsc
> +++ b/StandaloneMmPkg/StandaloneMmPkg.dsc
> @@ -78,9 +78,6 @@ [LibraryClasses.AARCH64]
>  # Pcd Section - list of all EDK II PCD Entries defined by this Platform
>  #
> 
> ##
> ##
> -[PcdsFeatureFlag]
> -  gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable|TRUE
> -
>  [PcdsFixedAtBuild]
>gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x80CF
>gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xff
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/Standalo
> neMmPeCoffExtraActionLib.inf
> b/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/Standalo
> neMmPeCoffExtraActionLib.inf
> index eef3d7c6e253..181c15b9345a 100644
> ---
> a/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/Standalo
> neMmPeCoffExtraActionLib.inf
> +++
> b/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/Standalo
> neMmPeCoffExtraActionLib.inf
> @@ -37,9 +37,6 @@ [Packages]
>MdePkg/MdePkg.dec
>StandaloneMmPkg/StandaloneMmPkg.dec
> 
> -[FeaturePcd]
> -  gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable
> -
>  [LibraryClasses]
>StandaloneMmMmuLib
>PcdLib
> --
> 2.20.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 01/10] StandaloneMmPkg: drop redundant definition of gEfiMmConfigurationProtocolGuid

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Tuesday, March 5, 2019 5:33 AM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen 
> Subject: [edk2] [PATCH 01/10] StandaloneMmPkg: drop redundant definition
> of gEfiMmConfigurationProtocolGuid
> 
> gEfiMmConfigurationProtocolGuid is already defined in MdePkg, so drop
> the duplicate definition from StandaloneMmPkg.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  StandaloneMmPkg/StandaloneMmPkg.dec | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/StandaloneMmPkg/StandaloneMmPkg.dec
> b/StandaloneMmPkg/StandaloneMmPkg.dec
> index 34108376233d..0b5fbf9e1767 100644
> --- a/StandaloneMmPkg/StandaloneMmPkg.dec
> +++ b/StandaloneMmPkg/StandaloneMmPkg.dec
> @@ -42,6 +42,3 @@ [Guids]
>  [PcdsFeatureFlag]
> 
> gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable|FALSE|BOOL
> EAN|0x0001
> 
> -[Protocols]
> -  gEfiMmConfigurationProtocolGuid  = { 0xc109319, 0xc149,
> 0x450e,  { 0xa3, 0xe3, 0xb9, 0xba, 0xdd, 0x9d, 0xc3, 0xa4 }}
> -
> --
> 2.20.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 09/10] StandaloneMmPkg/Core: remove legacy boot support

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Tuesday, March 5, 2019 5:33 AM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen 
> Subject: [edk2] [PATCH 09/10] StandaloneMmPkg/Core: remove legacy boot
> support
> 
> Remove the support for booting 'legacy' (i.e., non-UEFI boot) OSes.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  StandaloneMmPkg/Core/StandaloneMmCore.h |  22 
>  StandaloneMmPkg/Core/StandaloneMmCore.c | 124 ++--
>  2 files changed, 33 insertions(+), 113 deletions(-)
> 
> diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.h
> b/StandaloneMmPkg/Core/StandaloneMmCore.h
> index 74338dc9da0d..5d336b3dbbf6 100644
> --- a/StandaloneMmPkg/Core/StandaloneMmCore.h
> +++ b/StandaloneMmPkg/Core/StandaloneMmCore.h
> @@ -635,28 +635,6 @@ MmDriverDispatchHandler (
> 
>@return Status Code
> 
> -**/
> -EFI_STATUS
> -EFIAPI
> -MmLegacyBootHandler (
> -  IN EFI_HANDLE   DispatchHandle,
> -  IN CONST VOID   *Context,OPTIONAL
> -  IN OUT VOID *CommBuffer, OPTIONAL
> -  IN OUT UINTN*CommBufferSize  OPTIONAL
> -  );
> -
> -/**
> -  This function is the main entry point for an MM handler dispatch
> -  or communicate-based callback.
> -
> -  @param  DispatchHandle  The unique handle assigned to this handler
> by MmiHandlerRegister().
> -  @param  Context Points to an optional handler context
> which was specified when the handler was registered.
> -  @param  CommBuffer  A pointer to a collection of data in
> memory that will
> -  be conveyed from a non-MM environment
> into an MM environment.
> -  @param  CommBufferSize  The size of the CommBuffer.
> -
> -  @return Status Code
> -
>  **/
>  EFI_STATUS
>  EFIAPI
> diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.c
> b/StandaloneMmPkg/Core/StandaloneMmCore.c
> index 766cdb5c848c..fb6b3055e9c6 100644
> --- a/StandaloneMmPkg/Core/StandaloneMmCore.c
> +++ b/StandaloneMmPkg/Core/StandaloneMmCore.c
> @@ -87,21 +87,12 @@ EFI_MM_SYSTEM_TABLE  gMmCoreMmst = {
>MmiHandlerUnRegister
>  };
> 
> -//
> -// Flag to determine if the platform has performed a legacy boot.
> -// If this flag is TRUE, then the runtime code and runtime data associated
> with the
> -// MM IPL are converted to free memory, so the MM Core must guarantee
> that is
> -// does not touch of the code/data associated with the MM IPL if this flag is
> TRUE.
> -//
> -BOOLEAN  mInLegacyBoot = FALSE;
> -
>  //
>  // Table of MMI Handlers that are registered by the MM Core when it is
> initialized
>  //
>  MM_CORE_MMI_HANDLERS  mMmCoreMmiHandlers[] = {
>{ MmReadyToLockHandler,,
> NULL, TRUE  },
>{ MmEndOfDxeHandler,   ,
> NULL, FALSE },
> -  { MmLegacyBootHandler, ,
> NULL, FALSE },
>{ MmExitBootServiceHandler,,
> NULL, FALSE },
>{ MmReadyToBootHandler,,
> NULL, FALSE },
>{ NULL,NULL,
> NULL, FALSE },
> @@ -142,47 +133,6 @@ MmEfiNotAvailableYetArg5 (
>return EFI_NOT_AVAILABLE_YET;
>  }
> 
> -/**
> -  Software MMI handler that is called when a Legacy Boot event is signaled.
> The MM
> -  Core uses this signal to know that a Legacy Boot has been performed and
> that
> -  gMmCorePrivate that is shared between the UEFI and MM execution
> environments can
> -  not be accessed from MM anymore since that structure is considered
> free memory by
> -  a legacy OS.
> -
> -  @param  DispatchHandle  The unique handle assigned to this handler
> by MmiHandlerRegister().
> -  @param  Context Points to an optional handler context
> which was specified when the handler was registered.
> -  @param  CommBuffer  A pointer to a collection of data in
> memory that will
> -  be conveyed from a non-MM environment
> into an MM environment.
> -  @param  CommBufferSize  The size of the CommBuffer.
> -
> -  @return Status Code
> -
> -**/
> -EFI_STATUS
> -EFIAPI
> -MmLegacyBootHandler (
> -  IN EFI_HANDLE  DispatchHandle,
> -  IN CONST VOID  *Context,OPTIONAL
> -  IN OUT VOID*CommBuffer, OPTIONAL
> -  IN OUT UINTN   *CommBufferSize  OPTIONAL
> -  )
> -{
> -  EFI_HANDLE  MmHandle;
> -  EFI_STATUS  Status = EFI_SUCCESS;
> -
> -  if (!mInLegacyBoot) {
> -MmHandle = NULL;
> -Status = MmInstallProtocolInterface (
> -   ,
> -   ,
> -   EFI_NATIVE_INTERFACE,
> -   NULL
> -   );
> -  }
> -  mInLegacyBoot = TRUE;
> -  return Status;
> -}
> -
>  /**
>Software MMI handler that is called when a ExitBoot Service event is
> signaled.
> 
> @@ -396,7 +346,6 @@ MmEntryPoint (
>  {
>EFI_STATUS  Status;
>EFI_MM_COMMUNICATE_HEADER  *CommunicateHeader;
> -  BOOLEAN InLegacyBoot;
> 
>DEBUG ((DEBUG_INFO, 

Re: [edk2] [PATCH 05/10] StandaloneMmPkg/StandaloneMmCoreEntryPoint: drop explicit SerialPortLib call

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, March 5, 2019 5:33 AM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel ; Achin Gupta
> ; Supreeth Venkatesh
> ; Yao, Jiewen ;
> Leif Lindholm ; Jagadeesh Ujja
> 
> Subject: [PATCH 05/10] StandaloneMmPkg/StandaloneMmCoreEntryPoint:
> drop explicit SerialPortLib call
> 
> Sending DEBUG output to the serial port should only be done via
> DebugLib calls, which is in charge of initializing the serial
> port when appropriate. So drop the explicit SerialPortInitialize ()
> invocation, and rely on normal constructor ordering to get the
> serial port into the appropriate state at the right time.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
> 
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standal
> oneMmCoreEntryPoint.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Stand
> aloneMmCoreEntryPoint.c
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Stand
> aloneMmCoreEntryPoint.c
> index 5cca532456fd..c8e11a253d24 100644
> ---
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Stand
> aloneMmCoreEntryPoint.c
> +++
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Stand
> aloneMmCoreEntryPoint.c
> @@ -232,9 +232,6 @@ _ModuleEntryPoint (
>VOID*TeData;
>UINTN   TeDataSize;
> 
> -  Status = SerialPortInitialize ();
> -  ASSERT_EFI_ERROR (Status);
> -
>// Get Secure Partition Manager Version Information
>Status = GetSpmVersion ();
>if (EFI_ERROR (Status)) {
> --
> 2.20.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 0/1] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems

[Ard Biesheuvel]

On Tue, 5 Mar 2019 at 03:18, Wu, Hao A  wrote:
>
> > -Original Message-
> > From: Wu, Hao A
> > Sent: Tuesday, March 05, 2019 9:14 AM
> > To: edk2-devel@lists.01.org
> > Cc: Wu, Hao A; Eugene Cohen; Ard Biesheuvel; Ashish Singhal
> > Subject: [PATCH v2 0/1] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC
> > v3 64-bit systems
>
> Since Ashish already posted a patch to add the 64-bit System Address
> support for V3 mode SDHC:
> https://www.mail-archive.com/edk2-devel@lists.01.org/msg52057.html
>
> I think this patch can be dropped.
>
> But since Ashish's patch above is considered as a new feature addition, it
> will be pushed (if passes the review process) after the 19`Q1 release tag.
>
> So Eugene, Ard and Ashish, do you have concern on this?
>

That works for me.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context

[Ard Biesheuvel]

PI defines a few architected events that have significance in the MM
context as well as in the non-secure DXE context. So register notify
handlers for these events, and relay them into the standalone MM world.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf |  5 +++
 ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c   | 47 +++-
 2 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf 
b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
index 88beafa39c05..8bf269270f9d 100644
--- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
@@ -48,6 +48,11 @@ [LibraryClasses]
 [Protocols]
   gEfiMmCommunicationProtocolGuid  ## PRODUCES
 
+[Guids]
+  gEfiEndOfDxeEventGroupGuid
+  gEfiEventExitBootServicesGuid
+  gEfiEventReadyToBootGuid
+
 [Pcd.common]
   gArmTokenSpaceGuid.PcdMmBufferBase
   gArmTokenSpaceGuid.PcdMmBufferSize
diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c 
b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
index feb9fa9f4ead..3203cf801a19 100644
--- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
@@ -265,6 +265,43 @@ GetMmCompatibility ()
   return Status;
 }
 
+STATIC EFI_GUID* CONST mGuidedEventGuid[] = {
+  ,
+  ,
+  ,
+};
+
+STATIC EFI_EVENT mGuidedEvent[ARRAY_SIZE (mGuidedEventGuid)];
+
+/**
+  Event notification that is fired when GUIDed Event Group is signaled.
+
+  @param  Event The Event that is being processed, not used.
+  @param  Context   Event Context, not used.
+
+**/
+STATIC
+VOID
+EFIAPI
+MmGuidedEventNotify (
+  IN EFI_EVENT  Event,
+  IN VOID   *Context
+  )
+{
+  EFI_MM_COMMUNICATE_HEADER   Header;
+  UINTN   Size;
+
+  //
+  // Use Guid to initialize EFI_SMM_COMMUNICATE_HEADER structure
+  //
+  CopyGuid (, Context);
+  Header.MessageLength = 1;
+  Header.Data[0] = 0;
+
+  Size = sizeof (Header);
+  MmCommunicationCommunicate (, , );
+}
+
 /**
   The Entry Point for MM Communication
 
@@ -287,6 +324,7 @@ MmCommunicationInitialize (
   )
 {
   EFI_STATUS Status;
+  UINTN  Index;
 
   // Check if we can make the MM call
   Status = GetMmCompatibility ();
@@ -351,8 +389,13 @@ MmCommunicationInitialize (
   NULL,
   
   );
-  if (Status == EFI_SUCCESS) {
-return Status;
+  ASSERT_EFI_ERROR (Status);
+
+  for (Index = 0; Index < ARRAY_SIZE (mGuidedEventGuid); Index++) {
+Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
+MmGuidedEventNotify, mGuidedEventGuid[Index],
+mGuidedEventGuid[Index], [Index]);
+ASSERT_EFI_ERROR (Status);
   }
 
   gBS->UninstallProtocolInterface (
-- 
2.20.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 09/10] StandaloneMmPkg/Core: remove legacy boot support

[Ard Biesheuvel]

Remove the support for booting 'legacy' (i.e., non-UEFI boot) OSes.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 StandaloneMmPkg/Core/StandaloneMmCore.h |  22 
 StandaloneMmPkg/Core/StandaloneMmCore.c | 124 ++--
 2 files changed, 33 insertions(+), 113 deletions(-)

diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.h 
b/StandaloneMmPkg/Core/StandaloneMmCore.h
index 74338dc9da0d..5d336b3dbbf6 100644
--- a/StandaloneMmPkg/Core/StandaloneMmCore.h
+++ b/StandaloneMmPkg/Core/StandaloneMmCore.h
@@ -635,28 +635,6 @@ MmDriverDispatchHandler (
 
   @return Status Code
 
-**/
-EFI_STATUS
-EFIAPI
-MmLegacyBootHandler (
-  IN EFI_HANDLE   DispatchHandle,
-  IN CONST VOID   *Context,OPTIONAL
-  IN OUT VOID *CommBuffer, OPTIONAL
-  IN OUT UINTN*CommBufferSize  OPTIONAL
-  );
-
-/**
-  This function is the main entry point for an MM handler dispatch
-  or communicate-based callback.
-
-  @param  DispatchHandle  The unique handle assigned to this handler by 
MmiHandlerRegister().
-  @param  Context Points to an optional handler context which was 
specified when the handler was registered.
-  @param  CommBuffer  A pointer to a collection of data in memory that will
-  be conveyed from a non-MM environment into an MM 
environment.
-  @param  CommBufferSize  The size of the CommBuffer.
-
-  @return Status Code
-
 **/
 EFI_STATUS
 EFIAPI
diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.c 
b/StandaloneMmPkg/Core/StandaloneMmCore.c
index 766cdb5c848c..fb6b3055e9c6 100644
--- a/StandaloneMmPkg/Core/StandaloneMmCore.c
+++ b/StandaloneMmPkg/Core/StandaloneMmCore.c
@@ -87,21 +87,12 @@ EFI_MM_SYSTEM_TABLE  gMmCoreMmst = {
   MmiHandlerUnRegister
 };
 
-//
-// Flag to determine if the platform has performed a legacy boot.
-// If this flag is TRUE, then the runtime code and runtime data associated 
with the
-// MM IPL are converted to free memory, so the MM Core must guarantee that is
-// does not touch of the code/data associated with the MM IPL if this flag is 
TRUE.
-//
-BOOLEAN  mInLegacyBoot = FALSE;
-
 //
 // Table of MMI Handlers that are registered by the MM Core when it is 
initialized
 //
 MM_CORE_MMI_HANDLERS  mMmCoreMmiHandlers[] = {
   { MmReadyToLockHandler,,  NULL, TRUE  },
   { MmEndOfDxeHandler,   ,NULL, FALSE },
-  { MmLegacyBootHandler, ,   NULL, FALSE },
   { MmExitBootServiceHandler,, NULL, FALSE },
   { MmReadyToBootHandler,,  NULL, FALSE },
   { NULL,NULL,   NULL, FALSE },
@@ -142,47 +133,6 @@ MmEfiNotAvailableYetArg5 (
   return EFI_NOT_AVAILABLE_YET;
 }
 
-/**
-  Software MMI handler that is called when a Legacy Boot event is signaled.  
The MM
-  Core uses this signal to know that a Legacy Boot has been performed and that
-  gMmCorePrivate that is shared between the UEFI and MM execution environments 
can
-  not be accessed from MM anymore since that structure is considered free 
memory by
-  a legacy OS.
-
-  @param  DispatchHandle  The unique handle assigned to this handler by 
MmiHandlerRegister().
-  @param  Context Points to an optional handler context which was 
specified when the handler was registered.
-  @param  CommBuffer  A pointer to a collection of data in memory that will
-  be conveyed from a non-MM environment into an MM 
environment.
-  @param  CommBufferSize  The size of the CommBuffer.
-
-  @return Status Code
-
-**/
-EFI_STATUS
-EFIAPI
-MmLegacyBootHandler (
-  IN EFI_HANDLE  DispatchHandle,
-  IN CONST VOID  *Context,OPTIONAL
-  IN OUT VOID*CommBuffer, OPTIONAL
-  IN OUT UINTN   *CommBufferSize  OPTIONAL
-  )
-{
-  EFI_HANDLE  MmHandle;
-  EFI_STATUS  Status = EFI_SUCCESS;
-
-  if (!mInLegacyBoot) {
-MmHandle = NULL;
-Status = MmInstallProtocolInterface (
-   ,
-   ,
-   EFI_NATIVE_INTERFACE,
-   NULL
-   );
-  }
-  mInLegacyBoot = TRUE;
-  return Status;
-}
-
 /**
   Software MMI handler that is called when a ExitBoot Service event is 
signaled.
 
@@ -396,7 +346,6 @@ MmEntryPoint (
 {
   EFI_STATUS  Status;
   EFI_MM_COMMUNICATE_HEADER  *CommunicateHeader;
-  BOOLEAN InLegacyBoot;
 
   DEBUG ((DEBUG_INFO, "MmEntryPoint ...\n"));
 
@@ -413,44 +362,42 @@ MmEntryPoint (
   //
   // If a legacy boot has occured, then make sure gMmCorePrivate is not 
accessed
   //
-  InLegacyBoot = mInLegacyBoot;
-  if (!InLegacyBoot) {
-//
-// TBD: Mark the InMm flag as TRUE
-//
-gMmCorePrivate->InMm = TRUE;
 
+  //
+  // TBD: Mark the InMm flag as TRUE
+  //
+  gMmCorePrivate->InMm = TRUE;
+
+  //
+  // Check to see if this is a Synchronous MMI sent through the MM 
Communication
+  // Protocol or an Asynchronous MMI
+  //
+  if 

[edk2] [PATCH 02/10] StandaloneMmPkg: drop unused PCD PcdStandaloneMmEnable

[Ard Biesheuvel]

The PCD PcdStandaloneMmEnable is unused, and shouldn't exist in the
first place since the value is implied by the context (it is never
valid to set it to FALSE for standalone MM or TRUE for traditional
MM). So drop it.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 StandaloneMmPkg/StandaloneMmPkg.dec
   | 3 ---
 StandaloneMmPkg/StandaloneMmPkg.dsc
   | 3 ---
 
StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf
 | 3 ---
 3 files changed, 9 deletions(-)

diff --git a/StandaloneMmPkg/StandaloneMmPkg.dec 
b/StandaloneMmPkg/StandaloneMmPkg.dec
index 0b5fbf9e1767..2d178c5e20a6 100644
--- a/StandaloneMmPkg/StandaloneMmPkg.dec
+++ b/StandaloneMmPkg/StandaloneMmPkg.dec
@@ -39,6 +39,3 @@ [Guids]
   gEfiStandaloneMmNonSecureBufferGuid  = { 0xf00497e3, 0xbfa2, 0x41a1, { 
0x9d, 0x29, 0x54, 0xc2, 0xe9, 0x37, 0x21, 0xc5 }}
   gEfiArmTfCpuDriverEpDescriptorGuid   = { 0x6ecbd5a1, 0xc0f8, 0x4702, { 
0x83, 0x01, 0x4f, 0xc2, 0xc5, 0x47, 0x0a, 0x51 }}
 
-[PcdsFeatureFlag]
-  gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable|FALSE|BOOLEAN|0x0001
-
diff --git a/StandaloneMmPkg/StandaloneMmPkg.dsc 
b/StandaloneMmPkg/StandaloneMmPkg.dsc
index e8d71ad56f52..f279d9e7e5c7 100644
--- a/StandaloneMmPkg/StandaloneMmPkg.dsc
+++ b/StandaloneMmPkg/StandaloneMmPkg.dsc
@@ -78,9 +78,6 @@ [LibraryClasses.AARCH64]
 # Pcd Section - list of all EDK II PCD Entries defined by this Platform
 #
 

-[PcdsFeatureFlag]
-  gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable|TRUE
-
 [PcdsFixedAtBuild]
   gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x80CF
   gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xff
diff --git 
a/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf
 
b/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf
index eef3d7c6e253..181c15b9345a 100644
--- 
a/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf
+++ 
b/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf
@@ -37,9 +37,6 @@ [Packages]
   MdePkg/MdePkg.dec
   StandaloneMmPkg/StandaloneMmPkg.dec
 
-[FeaturePcd]
-  gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable
-
 [LibraryClasses]
   StandaloneMmMmuLib
   PcdLib
-- 
2.20.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 07/10] StandaloneMmPkg/Core: dispatch all drivers at init time

[Ard Biesheuvel]

Instead of deferring dispatch of the remaining MM drivers once the
CPU driver has been dispatched, proceed and dispatch all drivers.
This makes sense for standalone MM, since all dispatchable drivers
should be present in the initial firmware volume anyway: dispatch
of additional FVs originating in the non-secure side is not supported.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 StandaloneMmPkg/Core/Dispatcher.c   | 92 
 StandaloneMmPkg/Core/StandaloneMmCore.c |  1 -
 2 files changed, 93 deletions(-)

diff --git a/StandaloneMmPkg/Core/Dispatcher.c 
b/StandaloneMmPkg/Core/Dispatcher.c
index 8a2ad5118d92..bede4832cfb7 100644
--- a/StandaloneMmPkg/Core/Dispatcher.c
+++ b/StandaloneMmPkg/Core/Dispatcher.c
@@ -575,7 +575,6 @@ MmDispatcher (
   LIST_ENTRY*Link;
   EFI_MM_DRIVER_ENTRY  *DriverEntry;
   BOOLEAN   ReadyToRun;
-  BOOLEAN   PreviousMmEntryPointRegistered;
 
   DEBUG ((DEBUG_INFO, "MmDispatcher\n"));
 
@@ -639,11 +638,6 @@ MmDispatcher (
   DriverEntry->Initialized  = TRUE;
   RemoveEntryList (>ScheduledLink);
 
-  //
-  // Cache state of MmEntryPointRegistered before calling entry point
-  //
-  PreviousMmEntryPointRegistered = gMmCorePrivate->MmEntryPointRegistered;
-
   //
   // For each MM driver, pass NULL as ImageHandle
   //
@@ -661,20 +655,6 @@ MmDispatcher (
 DEBUG ((DEBUG_INFO, "StartImage Status - %r\n", Status));
 MmFreePages(DriverEntry->ImageBuffer, DriverEntry->NumberOfPage);
   }
-
-  if (!PreviousMmEntryPointRegistered && 
gMmCorePrivate->MmEntryPointRegistered) {
-//
-// Return immediately if the MM Entry Point was registered by the MM
-// Driver that was just dispatched.  The MM IPL will reinvoke the MM
-// Core Dispatcher.  This is required so MM Mode may be enabled as soon
-// as all the dependent MM Drivers for MM Mode have been dispatched.
-// Once the MM Entry Point has been registered, then MM Mode will be
-// used.
-//
-gRequestDispatch = TRUE;
-gDispatcherRunning = FALSE;
-return EFI_NOT_READY;
-  }
 }
 
 //
@@ -903,78 +883,6 @@ MmAddToDriverList (
   return EFI_SUCCESS;
 }
 
-/**
-  This function is the main entry point for an MM handler dispatch
-  or communicate-based callback.
-
-  Event notification that is fired every time a FV dispatch protocol is added.
-  More than one protocol may have been added when this event is fired, so you
-  must loop on MmLocateHandle () to see how many protocols were added and
-  do the following to each FV:
-  If the Fv has already been processed, skip it. If the Fv has not been
-  processed then mark it as being processed, as we are about to process it.
-  Read the Fv and add any driver in the Fv to the mDiscoveredList.The
-  mDiscoveredList is never free'ed and contains variables that define
-  the other states the MM driver transitions to..
-  While you are at it read the A Priori file into memory.
-  Place drivers in the A Priori list onto the mScheduledQueue.
-
-  @param  DispatchHandle  The unique handle assigned to this handler by 
SmiHandlerRegister().
-  @param  Context Points to an optional handler context which was 
specified when the handler was registered.
-  @param  CommBuffer  A pointer to a collection of data in memory that will
-  be conveyed from a non-MM environment into an MM 
environment.
-  @param  CommBufferSize  The size of the CommBuffer.
-
-  @return Status Code
-
-**/
-EFI_STATUS
-EFIAPI
-MmDriverDispatchHandler (
-  IN EFI_HANDLE  DispatchHandle,
-  IN CONST VOID  *Context,OPTIONAL
-  IN OUT VOID*CommBuffer, OPTIONAL
-  IN OUT UINTN   *CommBufferSize  OPTIONAL
-  )
-{
-  EFI_STATUSStatus;
-
-  DEBUG ((DEBUG_INFO, "MmDriverDispatchHandler\n"));
-
-  //
-  // Execute the MM Dispatcher on any newly discovered FVs and previously
-  // discovered MM drivers that have been discovered but not dispatched.
-  //
-  Status = MmDispatcher ();
-
-  //
-  // Check to see if CommBuffer and CommBufferSize are valid
-  //
-  if (CommBuffer != NULL && CommBufferSize != NULL) {
-if (*CommBufferSize > 0) {
-  if (Status == EFI_NOT_READY) {
-//
-// If a the MM Core Entry Point was just registered, then set flag to
-// request the MM Dispatcher to be restarted.
-//
-*(UINT8 *)CommBuffer = COMM_BUFFER_MM_DISPATCH_RESTART;
-  } else if (!EFI_ERROR (Status)) {
-//
-// Set the flag to show that the MM Dispatcher executed without errors
-//
-*(UINT8 *)CommBuffer = COMM_BUFFER_MM_DISPATCH_SUCCESS;
-  } else {
-//
-// Set the flag to show that the MM Dispatcher encountered an error
-//
-*(UINT8 *)CommBuffer = COMM_BUFFER_MM_DISPATCH_ERROR;
-  }
-}
-  }
-

[edk2] [PATCH 04/10] StandaloneMmPkg: remove redundant StandaloneMmDriverEntryPoint driver

[Ard Biesheuvel]

StandaloneMmDriverEntryPoint is implemented in MdePkg now, so let's
drop the redundant StandaloneMmPkg version.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 
StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.inf
 | 41 
 
StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.c
   | 99 
 2 files changed, 140 deletions(-)

diff --git 
a/StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.inf
 
b/StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.inf
deleted file mode 100644
index 4d1896db10ba..
--- 
a/StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.inf
+++ /dev/null
@@ -1,41 +0,0 @@
-## @file
-# Module entry point library for Standalone MM driver.
-#
-# Copyright (c) 2015, Intel Corporation. All rights reserved.
-# Copyright (c) 2016-2018, ARM Ltd. All rights reserved.
-#
-#  This program and the accompanying materials
-#  are licensed and made available under the terms and conditions of the BSD 
License
-#  which accompanies this distribution. The full text of the license may be 
found at
-#  http://opensource.org/licenses/bsd-license.php.
-#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
-#
-#
-##
-
-[Defines]
-  INF_VERSION= 0x0001001A
-  BASE_NAME  = StandaloneMmDriverEntryPoint
-  FILE_GUID  = BBC33478-98F8-4B78-B29D-574D681B7E43
-  MODULE_TYPE= MM_STANDALONE
-  VERSION_STRING = 1.0
-  PI_SPECIFICATION_VERSION   = 0x00010032
-  LIBRARY_CLASS  = StandaloneMmDriverEntryPoint|MM_STANDALONE
-
-#
-# The following information is for reference only and not required by the 
build tools.
-#
-#  VALID_ARCHITECTURES   = IA32 X64 IPF EBC
-#
-
-[Sources]
-  StandaloneMmDriverEntryPoint.c
-
-[Packages]
-  MdePkg/MdePkg.dec
-
-[LibraryClasses]
-  BaseLib
-  DebugLib
-
diff --git 
a/StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.c
 
b/StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.c
deleted file mode 100644
index 64bffcfccc8a..
--- 
a/StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.c
+++ /dev/null
@@ -1,99 +0,0 @@
-/** @file
-  Entry point to a Standalone MM driver.
-
-Copyright (c) 2015, Intel Corporation. All rights reserved.
-Copyright (c) 2016 - 2018, ARM Ltd. All rights reserved.
-
-This program and the accompanying materials
-are licensed and made available under the terms and conditions of the BSD 
License
-which accompanies this distribution.  The full text of the license may be 
found at
-http://opensource.org/licenses/bsd-license.php
-
-THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-
-**/
-
-#include 
-
-#include 
-#include 
-
-VOID
-EFIAPI
-ProcessLibraryConstructorList (
-  IN EFI_HANDLE   ImageHandle,
-  IN IN EFI_MM_SYSTEM_TABLE   *MmSystemTable
-  );
-
-EFI_STATUS
-EFIAPI
-ProcessModuleEntryPointList (
-  IN EFI_HANDLE   ImageHandle,
-  IN IN EFI_MM_SYSTEM_TABLE   *MmSystemTable
-  );
-
-VOID
-EFIAPI
-ProcessLibraryDestructorList (
-  IN EFI_HANDLE   ImageHandle,
-  IN IN EFI_MM_SYSTEM_TABLE   *MmSystemTable
-  );
-
-/**
-  The entry point of PE/COFF Image for a Standalone MM Driver.
-
-  This function is the entry point for a Standalone MM Driver.
-  This function must call ProcessLibraryConstructorList() and
-  ProcessModuleEntryPointList().
-  If the return status from ProcessModuleEntryPointList()
-  is an error status, then ProcessLibraryDestructorList() must be called.
-  The return value from ProcessModuleEntryPointList() is returned.
-  If _gDriverUnloadImageCount is greater than zero, then an unload
-  handler must be registered for this image
-  and the unload handler must invoke ProcessModuleUnloadList().
-  If _gUefiDriverRevision is not zero and SystemTable->Hdr.Revision is less
-  than _gUefiDriverRevison, then return EFI_INCOMPATIBLE_VERSION.
-
-
-  @param  ImageHandle  The image handle of the Standalone MM Driver.
-  @param  SystemTable  A pointer to the EFI System Table.
-
-  @retval  EFI_SUCCESS   The Standalone MM Driver exited normally.
-  @retval  EFI_INCOMPATIBLE_VERSION  _gUefiDriverRevision is greater than
-SystemTable->Hdr.Revision.
-  @retval  Other Return value from 
ProcessModuleEntryPointList().
-
-**/
-EFI_STATUS
-EFIAPI
-_ModuleEntryPoint (
-  IN EFI_HANDLE   ImageHandle,
-  IN IN EFI_MM_SYSTEM_TABLE   *MmSystemTable
-  )
-{
-  EFI_STATUS Status;
-
-  //
-  // Call 

[edk2] [PATCH 08/10] StandaloneMmPkg/Core: drop support for dispatching FVs into MM

[Ard Biesheuvel]

Remove the support that permits calls into the MM context to dispatch
firmware volumes that are not part of the initial standalone MM firmware
volume.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 StandaloneMmPkg/Core/StandaloneMmCore.h | 22 --
 StandaloneMmPkg/Core/Dispatcher.c   | 46 
 StandaloneMmPkg/Core/StandaloneMmCore.c |  1 -
 3 files changed, 69 deletions(-)

diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.h 
b/StandaloneMmPkg/Core/StandaloneMmCore.h
index 0d20bcaa6be5..74338dc9da0d 100644
--- a/StandaloneMmPkg/Core/StandaloneMmCore.h
+++ b/StandaloneMmPkg/Core/StandaloneMmCore.h
@@ -635,28 +635,6 @@ MmDriverDispatchHandler (
 
   @return Status Code
 
-**/
-EFI_STATUS
-EFIAPI
-MmFvDispatchHandler (
-  IN EFI_HANDLE   DispatchHandle,
-  IN CONST VOID   *Context,OPTIONAL
-  IN OUT VOID *CommBuffer, OPTIONAL
-  IN OUT UINTN*CommBufferSize  OPTIONAL
-  );
-
-/**
-  This function is the main entry point for an MM handler dispatch
-  or communicate-based callback.
-
-  @param  DispatchHandle  The unique handle assigned to this handler by 
MmiHandlerRegister().
-  @param  Context Points to an optional handler context which was 
specified when the handler was registered.
-  @param  CommBuffer  A pointer to a collection of data in memory that will
-  be conveyed from a non-MM environment into an MM 
environment.
-  @param  CommBufferSize  The size of the CommBuffer.
-
-  @return Status Code
-
 **/
 EFI_STATUS
 EFIAPI
diff --git a/StandaloneMmPkg/Core/Dispatcher.c 
b/StandaloneMmPkg/Core/Dispatcher.c
index bede4832cfb7..4b2f38f700a0 100644
--- a/StandaloneMmPkg/Core/Dispatcher.c
+++ b/StandaloneMmPkg/Core/Dispatcher.c
@@ -883,52 +883,6 @@ MmAddToDriverList (
   return EFI_SUCCESS;
 }
 
-/**
-  This function is the main entry point for an MM handler dispatch
-  or communicate-based callback.
-
-  @param  DispatchHandle  The unique handle assigned to this handler by 
SmiHandlerRegister().
-  @param  Context Points to an optional handler context which was 
specified when the handler was registered.
-  @param  CommBuffer  A pointer to a collection of data in memory that will
-  be conveyed from a non-MM environment into an MM 
environment.
-  @param  CommBufferSize  The size of the CommBuffer.
-
-  @return Status Code
-
-**/
-EFI_STATUS
-EFIAPI
-MmFvDispatchHandler (
-  IN EFI_HANDLE   DispatchHandle,
-  IN CONST VOID   *Context,OPTIONAL
-  IN OUT VOID *CommBuffer, OPTIONAL
-  IN OUT UINTN*CommBufferSize  OPTIONAL
-  )
-{
-  EFI_STATUSStatus;
-  EFI_MM_COMMUNICATE_FV_DISPATCH_DATA  *CommunicationFvDispatchData;
-  EFI_FIRMWARE_VOLUME_HEADER*FwVolHeader;
-
-  DEBUG ((DEBUG_INFO, "MmFvDispatchHandler\n"));
-
-  CommunicationFvDispatchData = CommBuffer;
-
-  DEBUG ((DEBUG_INFO, "  Dispatch - 0x%016lx - 0x%016lx\n", 
CommunicationFvDispatchData->Address,
-  CommunicationFvDispatchData->Size));
-
-  FwVolHeader = (EFI_FIRMWARE_VOLUME_HEADER 
*)(UINTN)CommunicationFvDispatchData->Address;
-
-  MmCoreFfsFindMmDriver (FwVolHeader);
-
-  //
-  // Execute the MM Dispatcher on any newly discovered FVs and previously
-  // discovered MM drivers that have been discovered but not dispatched.
-  //
-  Status = MmDispatcher ();
-
-  return Status;
-}
-
 /**
   Traverse the discovered list for any drivers that were discovered but not 
loaded
   because the dependency experessions evaluated to false.
diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.c 
b/StandaloneMmPkg/Core/StandaloneMmCore.c
index ec53b8d8bec4..766cdb5c848c 100644
--- a/StandaloneMmPkg/Core/StandaloneMmCore.c
+++ b/StandaloneMmPkg/Core/StandaloneMmCore.c
@@ -99,7 +99,6 @@ BOOLEAN  mInLegacyBoot = FALSE;
 // Table of MMI Handlers that are registered by the MM Core when it is 
initialized
 //
 MM_CORE_MMI_HANDLERS  mMmCoreMmiHandlers[] = {
-  { MmFvDispatchHandler, , NULL, TRUE  },
   { MmReadyToLockHandler,,  NULL, TRUE  },
   { MmEndOfDxeHandler,   ,NULL, FALSE },
   { MmLegacyBootHandler, ,   NULL, FALSE },
-- 
2.20.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 06/10] StandaloneMmPkg/Core: permit encapsulated firmware volumes

[Ard Biesheuvel]

Standalone MM requires 4 KB section alignment for all images, so that
strict permissions can be applied. Unfortunately, this results in a
lot of wasted space, which is usually costly in the secure world
environment that standalone MM is expected to operate in.

So let's permit the standalone MM drivers (but not the core) to be
delivered in a compressed firmware volume.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 StandaloneMmPkg/Core/StandaloneMmCore.inf |  1 +
 StandaloneMmPkg/Core/FwVol.c  | 99 ++--
 2 files changed, 91 insertions(+), 9 deletions(-)

diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.inf 
b/StandaloneMmPkg/Core/StandaloneMmCore.inf
index ff2b8b9cef03..83d31e2d92c5 100644
--- a/StandaloneMmPkg/Core/StandaloneMmCore.inf
+++ b/StandaloneMmPkg/Core/StandaloneMmCore.inf
@@ -49,6 +49,7 @@ [LibraryClasses]
   BaseMemoryLib
   CacheMaintenanceLib
   DebugLib
+  ExtractGuidedSectionLib
   FvLib
   HobLib
   MemoryAllocationLib
diff --git a/StandaloneMmPkg/Core/FwVol.c b/StandaloneMmPkg/Core/FwVol.c
index 5abf98c24797..d95491f252f9 100644
--- a/StandaloneMmPkg/Core/FwVol.c
+++ b/StandaloneMmPkg/Core/FwVol.c
@@ -14,6 +14,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 
 #include "StandaloneMmCore.h"
 #include 
+#include 
 
 //
 // List of file types supported by dispatcher
@@ -65,15 +66,25 @@ Returns:
 
 --*/
 {
-  EFI_STATUS  Status;
-  EFI_STATUS  DepexStatus;
-  EFI_FFS_FILE_HEADER *FileHeader;
-  EFI_FV_FILETYPE FileType;
-  VOID*Pe32Data;
-  UINTN   Pe32DataSize;
-  VOID*Depex;
-  UINTN   DepexSize;
-  UINTN   Index;
+  EFI_STATUS  Status;
+  EFI_STATUS  DepexStatus;
+  EFI_FFS_FILE_HEADER *FileHeader;
+  EFI_FV_FILETYPE FileType;
+  VOID*Pe32Data;
+  UINTN   Pe32DataSize;
+  VOID*Depex;
+  UINTN   DepexSize;
+  UINTN   Index;
+  EFI_COMMON_SECTION_HEADER   *Section;
+  VOID*SectionData;
+  UINTN   SectionDataSize;
+  UINT32  DstBufferSize;
+  VOID*ScratchBuffer;
+  UINT32  ScratchBufferSize;
+  VOID*DstBuffer;
+  UINT16  SectionAttribute;
+  UINT32  AuthenticationStatus;
+  EFI_FIRMWARE_VOLUME_HEADER  *InnerFvHeader;
 
   DEBUG ((DEBUG_INFO, "MmCoreFfsFindMmDriver - 0x%x\n", FwVolHeader));
 
@@ -83,6 +94,71 @@ Returns:
 
   FvIsBeingProcesssed (FwVolHeader);
 
+  //
+  // First check for encapsulated compressed firmware volumes
+  //
+  FileHeader = NULL;
+  do {
+Status = FfsFindNextFile (EFI_FV_FILETYPE_FIRMWARE_VOLUME_IMAGE,
+   FwVolHeader, );
+if (EFI_ERROR (Status)) {
+  break;
+}
+Status = FfsFindSectionData (EFI_SECTION_GUID_DEFINED, FileHeader,
+   , );
+if (EFI_ERROR (Status)) {
+  break;
+}
+Section = (EFI_COMMON_SECTION_HEADER *)(FileHeader + 1);
+Status = ExtractGuidedSectionGetInfo (Section, ,
+   , );
+if (EFI_ERROR (Status)) {
+  break;
+}
+
+//
+// Allocate scratch buffer
+//
+ScratchBuffer = (VOID *)(UINTN)AllocatePages (EFI_SIZE_TO_PAGES 
(ScratchBufferSize));
+if (ScratchBuffer == NULL) {
+  return EFI_OUT_OF_RESOURCES;
+}
+
+//
+// Allocate destination buffer, extra one page for adjustment
+//
+DstBuffer = (VOID *)(UINTN)AllocatePages (EFI_SIZE_TO_PAGES 
(DstBufferSize));
+if (DstBuffer == NULL) {
+  return EFI_OUT_OF_RESOURCES;
+}
+
+//
+// Call decompress function
+//
+Status = ExtractGuidedSectionDecode (Section, , ScratchBuffer,
+);
+FreePages (ScratchBuffer, EFI_SIZE_TO_PAGES (ScratchBufferSize));
+if (EFI_ERROR (Status)) {
+  goto FreeDstBuffer;
+}
+
+DEBUG ((DEBUG_INFO,
+  "Processing compressed firmware volume (AuthenticationStatus == %x)\n",
+  AuthenticationStatus));
+
+Status = FindFfsSectionInSections (DstBuffer, DstBufferSize,
+   EFI_SECTION_FIRMWARE_VOLUME_IMAGE, );
+if (EFI_ERROR (Status)) {
+  goto FreeDstBuffer;
+}
+
+InnerFvHeader = (VOID *)(Section + 1);
+Status = MmCoreFfsFindMmDriver (InnerFvHeader);
+if (EFI_ERROR (Status)) {
+  goto FreeDstBuffer;
+}
+  } while (TRUE);
+
   for (Index = 0; Index < sizeof (mMmFileTypes) / sizeof (mMmFileTypes[0]); 
Index++) {
 DEBUG ((DEBUG_INFO, "Check MmFileTypes - 0x%x\n", mMmFileTypes[Index]));
 FileType = 

[edk2] [PATCH 00/10] StandaloneMmPkg, ArmPkg: cleanups and improvements

[Ard Biesheuvel]

This series is a further cleanup of the StandaloneMmPkg infrastructure
used to implement UEFI secure boot on ARM systems.

The first 5 patches are simple cleanups.

Patch #6 adds support for dispatching a compressed firmware volume in the
standalone MM context, so that all drivers except the core can be delivered
in an encapsulated compressed FV, which saves quite some space.

Patch #7 modifies the driver dispatch logic in the MM context so that the
dispatcher continues until all drivers are dispatched, rather than waiting
for a nudge from the non-secure side once the CPU driver has been loaded.

Patch #8 removes support for the FV dispatch MM call.

Patch #9 removes support for legacy boot handling.

Patch #10 implements relaying architected PI events from DXE into MM by
the MM communicate driver.

Cc: Achin Gupta 
Cc: Supreeth Venkatesh 
Cc: Jiewen Yao 
Cc: Leif Lindholm 
Cc: Jagadeesh Ujja 

Ard Biesheuvel (10):
  StandaloneMmPkg: drop redundant definition of
gEfiMmConfigurationProtocolGuid
  StandaloneMmPkg: drop unused PCD PcdStandaloneMmEnable
  StandaloneMmPkg: switch to NULL DebugLib resolution
  StandaloneMmPkg: remove redundant StandaloneMmDriverEntryPoint driver
  StandaloneMmPkg/StandaloneMmCoreEntryPoint: drop explicit
SerialPortLib call
  StandaloneMmPkg/Core: permit encapsulated firmware volumes
  StandaloneMmPkg/Core: dispatch all drivers at init time
  StandaloneMmPkg/Core: drop support for dispatching FVs into MM
  StandaloneMmPkg/Core: remove legacy boot support
  ArmPkg/MmCommunicationDxe: signal architected PI events into MM
context

 StandaloneMmPkg/StandaloneMmPkg.dec
   |   6 -
 StandaloneMmPkg/StandaloneMmPkg.dsc
   |  14 +-
 ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf  
   |   5 +
 StandaloneMmPkg/Core/StandaloneMmCore.inf  
   |   1 +
 
StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.inf
 |  41 --
 
StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf
 |   3 -
 StandaloneMmPkg/Core/StandaloneMmCore.h
   |  44 ---
 ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
   |  47 ++-
 StandaloneMmPkg/Core/Dispatcher.c  
   | 138 
 StandaloneMmPkg/Core/FwVol.c   
   |  99 --
 StandaloneMmPkg/Core/StandaloneMmCore.c
   | 126 +-
 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
   |   3 -
 
StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.c
   |  99 --
 13 files changed, 175 insertions(+), 451 deletions(-)
 delete mode 100644 
StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.inf
 delete mode 100644 
StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.c

-- 
2.20.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 05/10] StandaloneMmPkg/StandaloneMmCoreEntryPoint: drop explicit SerialPortLib call

[Ard Biesheuvel]

Sending DEBUG output to the serial port should only be done via
DebugLib calls, which is in charge of initializing the serial
port when appropriate. So drop the explicit SerialPortInitialize ()
invocation, and rely on normal constructor ordering to get the
serial port into the appropriate state at the right time.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
 | 3 ---
 1 file changed, 3 deletions(-)

diff --git 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
index 5cca532456fd..c8e11a253d24 100644
--- 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
+++ 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
@@ -232,9 +232,6 @@ _ModuleEntryPoint (
   VOID*TeData;
   UINTN   TeDataSize;
 
-  Status = SerialPortInitialize ();
-  ASSERT_EFI_ERROR (Status);
-
   // Get Secure Partition Manager Version Information
   Status = GetSpmVersion ();
   if (EFI_ERROR (Status)) {
-- 
2.20.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 01/10] StandaloneMmPkg: drop redundant definition of gEfiMmConfigurationProtocolGuid

[Ard Biesheuvel]

gEfiMmConfigurationProtocolGuid is already defined in MdePkg, so drop
the duplicate definition from StandaloneMmPkg.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 StandaloneMmPkg/StandaloneMmPkg.dec | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/StandaloneMmPkg/StandaloneMmPkg.dec 
b/StandaloneMmPkg/StandaloneMmPkg.dec
index 34108376233d..0b5fbf9e1767 100644
--- a/StandaloneMmPkg/StandaloneMmPkg.dec
+++ b/StandaloneMmPkg/StandaloneMmPkg.dec
@@ -42,6 +42,3 @@ [Guids]
 [PcdsFeatureFlag]
   gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable|FALSE|BOOLEAN|0x0001
 
-[Protocols]
-  gEfiMmConfigurationProtocolGuid  = { 0xc109319, 0xc149, 0x450e,  { 
0xa3, 0xe3, 0xb9, 0xba, 0xdd, 0x9d, 0xc3, 0xa4 }}
-
-- 
2.20.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 03/10] StandaloneMmPkg: switch to NULL DebugLib resolution

[Ard Biesheuvel]

Building StandaloneMmPkg from its .DSC is mainly intended for build
coverage, and so platform specific configuration such as UART addresses
don't belong here.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 StandaloneMmPkg/StandaloneMmPkg.dsc | 11 +--
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/StandaloneMmPkg/StandaloneMmPkg.dsc 
b/StandaloneMmPkg/StandaloneMmPkg.dsc
index f279d9e7e5c7..8def9688fe7d 100644
--- a/StandaloneMmPkg/StandaloneMmPkg.dsc
+++ b/StandaloneMmPkg/StandaloneMmPkg.dsc
@@ -43,7 +43,7 @@ [LibraryClasses]
   #
   BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
   BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
-  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
+  DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
   
DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
   FvLib|StandaloneMmPkg/Library/FvLib/FvLib.inf
   
HobLib|StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf
@@ -66,10 +66,6 @@ [LibraryClasses.AARCH64]
   ArmSvcLib|ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf
   
CacheMaintenanceLib|ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.inf
   
PeCoffExtraActionLib|StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf
-  PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
-  
PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf
-  # ARM PL011 UART Driver
-  
SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
 
   
StandaloneMmCoreEntryPoint|StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
 
@@ -83,11 +79,6 @@ [PcdsFixedAtBuild]
   gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xff
   gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x0f
 
-[PcdsFixedAtBuild.AARCH64]
-  ## PL011 - Serial Terminal
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x1c0b
-  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
-
 
###
 #
 # Components Section - list of the modules and components that will be 
processed by compilation
-- 
2.20.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 0/3] Xen PCI passthrough fixes

[Laszlo Ersek]

Hi Igor,

On 03/04/19 16:04, Igor Druzhinin wrote:
> Igor Druzhinin (3):
>   OvmfPkg/XenSupport: remove usage of prefetchable PCI host bridge
> aperture
>   OvmfPkg/XenSupport: use a correct PCI host bridge aperture for BAR64
>   OvmfPkg/XenSupport: turn off address decoding before BAR sizing
> 
>  OvmfPkg/Library/PciHostBridgeLib/XenSupport.c | 44 
> ++-
>  1 file changed, 37 insertions(+), 7 deletions(-)
> 

thank you for the patches! I'll let our Xen reviewers check and approve
them.

For patch#3, I have one suggestion: please don't #define macros that
start with EFI_ but are not directly related to the UEFI, PI, PCI etc
standards. If you think the helper macro is suitable for general use and
it actually corresponds to ideas from e.g. the PCI standard(s), then
yoou might want to suggest the macro in a separate patch for e.g.
"MdePkg/Include/IndustryStandard/Pci22.h". (Adding Ray.)

Thanks,
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems

[Cohen, Eugene]

Ashish,

Thanks - I haven't forgotten.  We are still doing tests with the 32-bit ADMA 
driver and running into issues on our newer platform - once we have those 
resolved we will test the patch.  (We are seeing a strange issue where Read 
Multiple Block works but Write Multiple Block does not.)

Eugene

From: Ashish Singhal 
Sent: Sunday, March 3, 2019 9:00 PM
To: Wu, Hao A ; Cohen, Eugene ; Ard 
Biesheuvel 
Cc: edk2-devel@lists.01.org; Kim, Sangwoo (김상우 SW1Lab.) 
Subject: RE: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit 
systems

Hi Hao,

I agree that there has been a bug all along which got exposed just now. We 
should submit the patch as proposed by Eugene.

Also, I have submitted the patch for enabling 64b DMA for V3. Please take that 
into consideration once the freeze is over so that we can fix the issue in real 
sense.

Eugene,

Please let me know once you have tried my patch on your board.

Thanks
Ashish

-Original Message-
From: Wu, Hao A mailto:hao.a...@intel.com>>
Sent: Sunday, March 3, 2019 7:39 PM
To: Cohen, Eugene mailto:eug...@hp.com>>; Ashish Singhal 
mailto:ashishsin...@nvidia.com>>; Ard Biesheuvel 
mailto:ard.biesheu...@linaro.org>>
Cc: edk2-devel@lists.01.org; Kim, Sangwoo (??? 
SW1Lab.) mailto:sangwoo@hp.com>>
Subject: RE: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit 
systems

Hi Eugene, Ashish and Ard

Sorry for the delayed response, I was out of office in the previous several 
days.

According to the discussion, my understanding is that (quote the comments from
Ard):

> Driver should not set the EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE
> attribute 1. If the device does not support it; 2. If the driver does
> not implement the 64-bit DMA mode that the device does
> support.

Thus, for the current implementation of the SdMmcPciHcDxe driver (including the
V4 ADMA descriptor support from Ashish):

* The driver should set the DUAL_ADDRESS_CYCLE attribute only when 'SysBus64V4'
bit set, because of the statement 2 above.

And for the previous implementation (before the change from Ashish):

* The driver should not set the DUAL_ADDRESS_CYCLE attribute at all, since the
implementation was written to support only the 32b ADMA descriptor.

If this is true, I am fine with your proposed fix.


Eugene,

Could you help to state the reason for the fix a bit more clear in the commit 
log?

Also, I have filed a Bugzilla tracker for this one:
https://bugzilla.tianocore.org/show_bug.cgi?id=1583

Could you help to add this information into the commit log as well? Thanks.

Best Regards,
Hao Wu

> -Original Message-
> From: Ashish Singhal [mailto:ashishsin...@nvidia.com]
> Sent: Friday, March 01, 2019 11:25 PM
> To: Ard Biesheuvel; Cohen, Eugene
> Cc: Wu, Hao A; edk2-devel@lists.01.org; Kim, 
> Sangwoo (김상우 SW1Lab.)
> Subject: RE: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3
> 64-bit systems
>
> Acked-by: Ashish Singhal 
> mailto:ashishsin...@nvidia.com>>
>
> -Original Message-
> From: Ard Biesheuvel 
> mailto:ard.biesheu...@linaro.org>>
> Sent: Friday, March 1, 2019 4:39 AM
> To: Cohen, Eugene mailto:eug...@hp.com>>
> Cc: Ashish Singhal mailto:ashishsin...@nvidia.com>>; 
> Wu, Hao A
> mailto:hao.a...@intel.com>>; 
> edk2-devel@lists.01.org; Kim, Sangwoo (김상우
> SW1Lab.) mailto:sangwoo@hp.com>>
> Subject: Re: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3
> 64-bit systems
>
> On Fri, 1 Mar 2019 at 11:54, Cohen, Eugene 
> mailto:eug...@hp.com>> wrote:
> >
> > Ard,
> >
> > > So before these changes, we were in the exact same situation, but
> > > since PC platforms never enable DMA above 4 GB in the first place,
> > > nobody ever noticed until we started running this code on arm64
> > > platforms that have no 32-bit addressable DRAM to begin with.
> >
> > Interesting - I did not realize that there were designs that were
> > crazy
> enough to have no addressable DRAM below 4G.
> >
>
> You must be new here :-)
>
> But seriously, it does make sense for an implementation to, say, put
> all peripherals, PCIe resource windows etc in the bottom half and all
> DRAM in the top half of a 40-bit address space, which is how the AMD
> Seattle SoC ended with its system memory at address 0x80__.
> Note that on this platform, we can still use 32-bit DMA if we want to
> with the help of the SMMUs, but we haven't wired those up in UEFI (and
> the generic host bridge driver did not have the IOMMU hooks at the
> time)
>
> > > The obvious conclusion is that the driver should not set the
> > > EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE attribute if the device
> does
> > > not support it, or, which seems to be our case, if the driver does
> > > not implement the 64-bit DMA mode that the driver does support.
> > > However, since there are platforms for which bounce buffering is
> > > not an option 

Re: [edk2] [Patch v2 2/4] UefiCpuPkg/RegisterCpuFeaturesLib: Optimize PCD PcdCpuFeaturesUserConfiguration.

[Zeng, Star]

And suggest update the title to be like "UefiCpuPkg: Retire 
PcdCpuFeaturesUserConfiguration".

Thanks,
Star
-Original Message-
From: Zeng, Star 
Sent: Tuesday, March 5, 2019 5:55 PM
To: Dong, Eric ; edk2-devel@lists.01.org
Cc: Ni, Ray ; Laszlo Ersek (ler...@redhat.com) 
; Gao, Liming ; Zeng, Star 

Subject: RE: [edk2] [Patch v2 2/4] UefiCpuPkg/RegisterCpuFeaturesLib: Optimize 
PCD PcdCpuFeaturesUserConfiguration.

It should be an incompatible change to remove PcdCpuFeaturesUserConfiguration.
Please add upgrade notes for it. I know upgrade notes for 2019 Q1 stable tag is 
at https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Notes. 
I do not know where is the link for 2019 Q1 stable tag. Liming may know it.


Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Eric Dong
Sent: Friday, March 1, 2019 1:40 PM
To: edk2-devel@lists.01.org
Subject: [edk2] [Patch v2 2/4] UefiCpuPkg/RegisterCpuFeaturesLib: Optimize PCD 
PcdCpuFeaturesUserConfiguration.

Merge PcdCpuFeaturesUserConfiguration into PcdCpuFeaturesSetting.
Use PcdCpuFeaturesSetting as input for the user input feature setting Use 
PcdCpuFeaturesSetting as output for the final CPU feature setting

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

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 71 +-
 .../DxeRegisterCpuFeaturesLib.inf  |  3 +-
 .../PeiRegisterCpuFeaturesLib.inf  |  3 +-
 .../RegisterCpuFeaturesLib/RegisterCpuFeatures.h   |  1 -
 UefiCpuPkg/UefiCpuPkg.dec  |  9 +--
 5 files changed, 21 insertions(+), 66 deletions(-)

diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c 
b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
index bae92b89a6..d877caff74 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
@@ -21,16 +21,21 @@ CHAR16 *mRegisterTypeStr[] = {L"MSR", L"CR", L"MMIO", 
L"CACHE", L"SEMAP", L"INVA
   Worker function to save PcdCpuFeaturesCapability.
 
   @param[in]  SupportedFeatureMask  The pointer to CPU feature bits mask buffer
+  @param[in]  FeatureMaskSize   CPU feature bits mask buffer size.
+
 **/
 VOID
 SetCapabilityPcd (
-  IN UINT8   *SupportedFeatureMask
+  IN UINT8   *SupportedFeatureMask,
+  IN UINT32  FeatureMaskSize
   )
 {
   EFI_STATUS Status;
   UINTN  BitMaskSize;
 
   BitMaskSize = PcdGetSize (PcdCpuFeaturesCapability);
+  ASSERT (FeatureMaskSize == BitMaskSize);
+
   Status = PcdSetPtrS (PcdCpuFeaturesCapability, , 
SupportedFeatureMask);
   ASSERT_EFI_ERROR (Status);
 }
@@ -53,48 +58,6 @@ SetSettingPcd (
   ASSERT_EFI_ERROR (Status);
 }
 
-/**
-  Worker function to get PcdCpuFeaturesSupport.
-
-  @return  The pointer to CPU feature bits mask buffer.
-**/
-UINT8 *
-GetSupportPcd (
-  VOID
-  )
-{
-  UINT8  *SupportBitMask;
-
-  SupportBitMask = AllocateCopyPool (
-  PcdGetSize (PcdCpuFeaturesSupport),
-  PcdGetPtr (PcdCpuFeaturesSupport)
-  );
-  ASSERT (SupportBitMask != NULL);
-
-  return SupportBitMask;
-}
-
-/**
-  Worker function to get PcdCpuFeaturesUserConfiguration.
-
-  @return  The pointer to CPU feature bits mask buffer.
-**/
-UINT8 *
-GetConfigurationPcd (
-  VOID
-  )
-{
-  UINT8  *SupportBitMask;
-
-  SupportBitMask = AllocateCopyPool (
-  PcdGetSize (PcdCpuFeaturesUserConfiguration),
-  PcdGetPtr (PcdCpuFeaturesUserConfiguration)
-  );
-  ASSERT (SupportBitMask != NULL);
-
-  return SupportBitMask;
-}
-
 /**
   Collects CPU type and feature information.
 
@@ -287,7 +250,6 @@ CpuInitDataInitialize (
   // Get support and configuration PCDs
   //
   CpuFeaturesData->SupportPcd   = GetSupportPcd ();
-  CpuFeaturesData->ConfigurationPcd = GetConfigurationPcd ();  }
 
 /**
@@ -610,16 +572,9 @@ AnalysisProcessorFeatures (
   //
   // Calculate the last setting
   //
-
   CpuFeaturesData->SettingPcd = AllocateCopyPool 
(CpuFeaturesData->BitMaskSize, CpuFeaturesData->CapabilityPcd);
   ASSERT (CpuFeaturesData->SettingPcd != NULL);
-  SupportedMaskAnd (CpuFeaturesData->SettingPcd, 
CpuFeaturesData->ConfigurationPcd);
-
-  //
-  // Save PCDs and display CPU PCDs
-  //
-  SetCapabilityPcd (CpuFeaturesData->CapabilityPcd);
-  SetSettingPcd (CpuFeaturesData->SettingPcd);
+  SupportedMaskAnd (CpuFeaturesData->SettingPcd, PcdGetPtr 
+ (PcdCpuFeaturesSetting));
 
   //
   // Dump the last CPU feature list
@@ -643,14 +598,20 @@ AnalysisProcessorFeatures (
 }
 DEBUG ((DEBUG_INFO, "PcdCpuFeaturesSupport:\n"));
 DumpCpuFeatureMask (CpuFeaturesData->SupportPcd);
-DEBUG ((DEBUG_INFO, "PcdCpuFeaturesUserConfiguration:\n"));
-DumpCpuFeatureMask 

Re: [edk2] [Patch v2 2/4] UefiCpuPkg/RegisterCpuFeaturesLib: Optimize PCD PcdCpuFeaturesUserConfiguration.

[Zeng, Star]

It should be an incompatible change to remove PcdCpuFeaturesUserConfiguration.
Please add upgrade notes for it. I know upgrade notes for 2019 Q1 stable tag is 
at https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Notes. 
I do not know where is the link for 2019 Q1 stable tag. Liming may know it.


Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Eric Dong
Sent: Friday, March 1, 2019 1:40 PM
To: edk2-devel@lists.01.org
Subject: [edk2] [Patch v2 2/4] UefiCpuPkg/RegisterCpuFeaturesLib: Optimize PCD 
PcdCpuFeaturesUserConfiguration.

Merge PcdCpuFeaturesUserConfiguration into PcdCpuFeaturesSetting.
Use PcdCpuFeaturesSetting as input for the user input feature setting Use 
PcdCpuFeaturesSetting as output for the final CPU feature setting

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

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 71 +-
 .../DxeRegisterCpuFeaturesLib.inf  |  3 +-
 .../PeiRegisterCpuFeaturesLib.inf  |  3 +-
 .../RegisterCpuFeaturesLib/RegisterCpuFeatures.h   |  1 -
 UefiCpuPkg/UefiCpuPkg.dec  |  9 +--
 5 files changed, 21 insertions(+), 66 deletions(-)

diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c 
b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
index bae92b89a6..d877caff74 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
@@ -21,16 +21,21 @@ CHAR16 *mRegisterTypeStr[] = {L"MSR", L"CR", L"MMIO", 
L"CACHE", L"SEMAP", L"INVA
   Worker function to save PcdCpuFeaturesCapability.
 
   @param[in]  SupportedFeatureMask  The pointer to CPU feature bits mask buffer
+  @param[in]  FeatureMaskSize   CPU feature bits mask buffer size.
+
 **/
 VOID
 SetCapabilityPcd (
-  IN UINT8   *SupportedFeatureMask
+  IN UINT8   *SupportedFeatureMask,
+  IN UINT32  FeatureMaskSize
   )
 {
   EFI_STATUS Status;
   UINTN  BitMaskSize;
 
   BitMaskSize = PcdGetSize (PcdCpuFeaturesCapability);
+  ASSERT (FeatureMaskSize == BitMaskSize);
+
   Status = PcdSetPtrS (PcdCpuFeaturesCapability, , 
SupportedFeatureMask);
   ASSERT_EFI_ERROR (Status);
 }
@@ -53,48 +58,6 @@ SetSettingPcd (
   ASSERT_EFI_ERROR (Status);
 }
 
-/**
-  Worker function to get PcdCpuFeaturesSupport.
-
-  @return  The pointer to CPU feature bits mask buffer.
-**/
-UINT8 *
-GetSupportPcd (
-  VOID
-  )
-{
-  UINT8  *SupportBitMask;
-
-  SupportBitMask = AllocateCopyPool (
-  PcdGetSize (PcdCpuFeaturesSupport),
-  PcdGetPtr (PcdCpuFeaturesSupport)
-  );
-  ASSERT (SupportBitMask != NULL);
-
-  return SupportBitMask;
-}
-
-/**
-  Worker function to get PcdCpuFeaturesUserConfiguration.
-
-  @return  The pointer to CPU feature bits mask buffer.
-**/
-UINT8 *
-GetConfigurationPcd (
-  VOID
-  )
-{
-  UINT8  *SupportBitMask;
-
-  SupportBitMask = AllocateCopyPool (
-  PcdGetSize (PcdCpuFeaturesUserConfiguration),
-  PcdGetPtr (PcdCpuFeaturesUserConfiguration)
-  );
-  ASSERT (SupportBitMask != NULL);
-
-  return SupportBitMask;
-}
-
 /**
   Collects CPU type and feature information.
 
@@ -287,7 +250,6 @@ CpuInitDataInitialize (
   // Get support and configuration PCDs
   //
   CpuFeaturesData->SupportPcd   = GetSupportPcd ();
-  CpuFeaturesData->ConfigurationPcd = GetConfigurationPcd ();  }
 
 /**
@@ -610,16 +572,9 @@ AnalysisProcessorFeatures (
   //
   // Calculate the last setting
   //
-
   CpuFeaturesData->SettingPcd = AllocateCopyPool 
(CpuFeaturesData->BitMaskSize, CpuFeaturesData->CapabilityPcd);
   ASSERT (CpuFeaturesData->SettingPcd != NULL);
-  SupportedMaskAnd (CpuFeaturesData->SettingPcd, 
CpuFeaturesData->ConfigurationPcd);
-
-  //
-  // Save PCDs and display CPU PCDs
-  //
-  SetCapabilityPcd (CpuFeaturesData->CapabilityPcd);
-  SetSettingPcd (CpuFeaturesData->SettingPcd);
+  SupportedMaskAnd (CpuFeaturesData->SettingPcd, PcdGetPtr 
+ (PcdCpuFeaturesSetting));
 
   //
   // Dump the last CPU feature list
@@ -643,14 +598,20 @@ AnalysisProcessorFeatures (
 }
 DEBUG ((DEBUG_INFO, "PcdCpuFeaturesSupport:\n"));
 DumpCpuFeatureMask (CpuFeaturesData->SupportPcd);
-DEBUG ((DEBUG_INFO, "PcdCpuFeaturesUserConfiguration:\n"));
-DumpCpuFeatureMask (CpuFeaturesData->ConfigurationPcd);
 DEBUG ((DEBUG_INFO, "PcdCpuFeaturesCapability:\n"));
 DumpCpuFeatureMask (CpuFeaturesData->CapabilityPcd);
-DEBUG ((DEBUG_INFO, "PcdCpuFeaturesSetting:\n"));
+DEBUG ((DEBUG_INFO, "Origin PcdCpuFeaturesSetting:\n"));
+DumpCpuFeatureMask (PcdGetPtr (PcdCpuFeaturesSetting));
+DEBUG ((DEBUG_INFO, "Final PcdCpuFeaturesSetting:\n"));
 DumpCpuFeatureMask (CpuFeaturesData->SettingPcd);

Re: [edk2] [PATCH] BaseTools:Guid.xref will change after increment build

[Feng, Bob C]

Hi Zhiju,

Since you changed a set() to a list,  I think you need to check if the item is 
already in the list before appending it.

Thanks,
Bob

-Original Message-
From: Fan, ZhijuX 
Sent: Tuesday, March 5, 2019 4:48 PM
To: edk2-devel@lists.01.org
Cc: Gao, Liming ; Feng, Bob C 
Subject: [edk2][PATCH] BaseTools:Guid.xref will change after increment build

the order of the data may change if set() is used

Cc: Bob Feng 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhiju.Fan 
---
 BaseTools/Source/Python/Workspace/DscBuildData.py | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py 
b/BaseTools/Source/Python/Workspace/DscBuildData.py
index 5e7d7dcd63..342b9472a2 100644
--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
@@ -1654,7 +1654,7 @@ class DscBuildData(PlatformBuildClassObject):
 AvailableSkuIdSet = copy.copy(self.SkuIds)
 
 PcdDict = tdict(True, 4)
-PcdSet = set()
+PcdList = []
 # Find out all possible PCD candidates for self._Arch
 RecordList = self._RawData[Type, self._Arch]
 PcdValueDict = OrderedDict()
@@ -1666,10 +1666,10 @@ class DscBuildData(PlatformBuildClassObject):
 File=self.MetaFile, Line=Dummy5)
 if SkuName in (self.SkuIdMgr.SystemSkuId, TAB_DEFAULT, TAB_COMMON):
 if "." not in TokenSpaceGuid and "[" not in PcdCName:
-PcdSet.add((PcdCName, TokenSpaceGuid, SkuName, Dummy5))
+PcdList.append((PcdCName, TokenSpaceGuid, SkuName, 
+ Dummy5))
 PcdDict[Arch, PcdCName, TokenSpaceGuid, SkuName] = Setting
 
-for PcdCName, TokenSpaceGuid, SkuName, Dummy4 in PcdSet:
+for PcdCName, TokenSpaceGuid, SkuName, Dummy4 in PcdList:
 Setting = PcdDict[self._Arch, PcdCName, TokenSpaceGuid, SkuName]
 if Setting is None:
 continue
@@ -2874,7 +2874,7 @@ class DscBuildData(PlatformBuildClassObject):
 # PCD settings for certain ARCH and SKU
 #
 PcdDict = tdict(True, 5)
-PcdSet = set()
+PcdList = []
 RecordList = self._RawData[Type, self._Arch]
 # Find out all possible PCD candidates for self._Arch
 AvailableSkuIdSet = copy.copy(self.SkuIds) @@ -2896,12 +2896,12 @@ 
class DscBuildData(PlatformBuildClassObject):
 EdkLogger.error('build', PARAMETER_INVALID, 'DefaultStores %s 
is not defined in [DefaultStores] section' % DefaultStore,
 File=self.MetaFile, Line=Dummy5)
 if "." not in TokenSpaceGuid and "[" not in PcdCName:
-PcdSet.add((PcdCName, TokenSpaceGuid, SkuName, DefaultStore, 
Dummy5))
+PcdList.append((PcdCName, TokenSpaceGuid, SkuName, 
+ DefaultStore, Dummy5))
 PcdDict[Arch, SkuName, PcdCName, TokenSpaceGuid, DefaultStore] = 
Setting
 
 
 # Remove redundant PCD candidates, per the ARCH and SKU
-for PcdCName, TokenSpaceGuid, SkuName, DefaultStore, Dummy4 in PcdSet:
+for PcdCName, TokenSpaceGuid, SkuName, DefaultStore, Dummy4 in PcdList:
 
 Setting = PcdDict[self._Arch, SkuName, PcdCName, TokenSpaceGuid, 
DefaultStore]
 if Setting is None:
--
2.14.1.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] BaseTools:Guid.xref will change after increment build

[Fan, ZhijuX]

the order of the data may change if set() is used

Cc: Bob Feng 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhiju.Fan 
---
 BaseTools/Source/Python/Workspace/DscBuildData.py | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py 
b/BaseTools/Source/Python/Workspace/DscBuildData.py
index 5e7d7dcd63..342b9472a2 100644
--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
@@ -1654,7 +1654,7 @@ class DscBuildData(PlatformBuildClassObject):
 AvailableSkuIdSet = copy.copy(self.SkuIds)
 
 PcdDict = tdict(True, 4)
-PcdSet = set()
+PcdList = []
 # Find out all possible PCD candidates for self._Arch
 RecordList = self._RawData[Type, self._Arch]
 PcdValueDict = OrderedDict()
@@ -1666,10 +1666,10 @@ class DscBuildData(PlatformBuildClassObject):
 File=self.MetaFile, Line=Dummy5)
 if SkuName in (self.SkuIdMgr.SystemSkuId, TAB_DEFAULT, TAB_COMMON):
 if "." not in TokenSpaceGuid and "[" not in PcdCName:
-PcdSet.add((PcdCName, TokenSpaceGuid, SkuName, Dummy5))
+PcdList.append((PcdCName, TokenSpaceGuid, SkuName, Dummy5))
 PcdDict[Arch, PcdCName, TokenSpaceGuid, SkuName] = Setting
 
-for PcdCName, TokenSpaceGuid, SkuName, Dummy4 in PcdSet:
+for PcdCName, TokenSpaceGuid, SkuName, Dummy4 in PcdList:
 Setting = PcdDict[self._Arch, PcdCName, TokenSpaceGuid, SkuName]
 if Setting is None:
 continue
@@ -2874,7 +2874,7 @@ class DscBuildData(PlatformBuildClassObject):
 # PCD settings for certain ARCH and SKU
 #
 PcdDict = tdict(True, 5)
-PcdSet = set()
+PcdList = []
 RecordList = self._RawData[Type, self._Arch]
 # Find out all possible PCD candidates for self._Arch
 AvailableSkuIdSet = copy.copy(self.SkuIds)
@@ -2896,12 +2896,12 @@ class DscBuildData(PlatformBuildClassObject):
 EdkLogger.error('build', PARAMETER_INVALID, 'DefaultStores %s 
is not defined in [DefaultStores] section' % DefaultStore,
 File=self.MetaFile, Line=Dummy5)
 if "." not in TokenSpaceGuid and "[" not in PcdCName:
-PcdSet.add((PcdCName, TokenSpaceGuid, SkuName, DefaultStore, 
Dummy5))
+PcdList.append((PcdCName, TokenSpaceGuid, SkuName, 
DefaultStore, Dummy5))
 PcdDict[Arch, SkuName, PcdCName, TokenSpaceGuid, DefaultStore] = 
Setting
 
 
 # Remove redundant PCD candidates, per the ARCH and SKU
-for PcdCName, TokenSpaceGuid, SkuName, DefaultStore, Dummy4 in PcdSet:
+for PcdCName, TokenSpaceGuid, SkuName, DefaultStore, Dummy4 in PcdList:
 
 Setting = PcdDict[self._Arch, SkuName, PcdCName, TokenSpaceGuid, 
DefaultStore]
 if Setting is None:
-- 
2.14.1.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch] UefiCpuPkg/MpInitLib: Direct allocate buffer for Wake up Buffer.

[Ni, Ray]

Eric,
I agree with the code change. It fixes the memtest86 problem through
a very simple code change.
But I think we do need a very meaningful comments to describe a such simple
code change.

Comments below:
1. Please make sure each line of commit message doesn't exceed 70.

> -Original Message-
> From: Dong, Eric 
> Sent: Tuesday, March 5, 2019 10:07 AM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ray 
> Subject: [Patch] UefiCpuPkg/MpInitLib: Direct allocate buffer for Wake up
> Buffer.
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=1538
> 
> Current CpuDxe driver "borrows" Wakeup Buffer (through Allocate & free
> to get the buffer pointer, backup the buffer data before using it and
> restore it after using).  Now this logic met a problem described in
> the above BZ because the test tool and the CpuDxe both use the same
> memory at the same time.

2. Can you describe the issue more clearly in the commit message?
What problem is met? What's the tool?

> 
> In order to fix the above issue, CpuDxe changed to allocate the buffer
> below 1M instead of borrow it. After investigation, we found below
> 0x88000 is the possible space which can be used. 

3. What investigation was made to choose 0x88000?

> For now, range
> 0x6 ~ 0x88000 used for Legacy OPROMs by LegacyBios driver. And it
> tries to allocate these range page(4K size) by page. It just reports
> warning message if specific page been used by others already.
> 
> Also CpuDxe driver will produce CPU arch protocol and LegacyBios driver
> has dependency for this protocol. So CpuDxe driver will start before
> LegacyBios driver and CpuDxe driver can allocate that space successful.
> 
> With this change, CpuDxe driver will use 0x87000 ~ 0x88000 as wakeup
> buffer.

4. What if someone allocates the exact page? Will CpuDxe driver fail to start?

> 
> Cc: Ray Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> ---
>  UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 30 +++
> ---
>  1 file changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> index b2307cbb61..5bc9a47efb 100644
> --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> @@ -76,7 +76,7 @@ SaveCpuMpData (
>  }
> 
>  /**
> -  Get available system memory below 1MB by specified size.
> +  Get available system memory below 0x88000 by specified size.
> 
>@param[in] WakeupBufferSize   Wakeup buffer size required
> 
> @@ -91,7 +91,19 @@ GetWakeupBuffer (
>EFI_STATUS  Status;
>EFI_PHYSICAL_ADDRESSStartAddress;
> 
> -  StartAddress = BASE_1MB;
> +  //
> +  // Current "Borrow" space mechanism caused potential race condition if
> both
> +  // AP and the original owner use the share space.
> +  //

5. future code reader cannot understand what "current borrow mechanism" because
you change the implementation.

> +  // LegacyBios driver tries to allocate 4K pages between 0x6 ~ 0x88000
> +  // space. It will just report an waring message if the page has been 
> allocate
> +  // by other drivers.

6. The above wording describes the behavior of LegacyBios driver. What's the
relation ship between the LegacyBios driver behavior and the new implementation?

> +  // LagacyBios driver depends on CPU Arch protocol, so it will start after
> +  // CpuDxe driver which produce Cpu Arch Protocol and use this library.
> +  // So below allocate logic will be trigged before LegacyBios driver and it
> +  // will always return success.
> +  //
> +  StartAddress = BASE_512KB + BASE_32KB;

7. If 0x88000 is chosen, why use BASE_512KB + BASE_32KB instead of 0x88000?
Will you remove the hard-code 0x88000 and use BASE_1MB after CSM is EOL?

>Status = gBS->AllocatePages (
>AllocateMaxAddress,
>EfiBootServicesData,
> @@ -99,17 +111,13 @@ GetWakeupBuffer (
>
>);
>ASSERT_EFI_ERROR (Status);
> -  if (!EFI_ERROR (Status)) {
> -Status = gBS->FreePages(
> -   StartAddress,
> -   EFI_SIZE_TO_PAGES (WakeupBufferSize)
> -   );
> -ASSERT_EFI_ERROR (Status);
> -DEBUG ((DEBUG_INFO, "WakeupBufferStart = %x, WakeupBufferSize
> = %x\n",
> -(UINTN) StartAddress, WakeupBufferSize));
> -  } else {
> +  if (EFI_ERROR (Status)) {
>  StartAddress = (EFI_PHYSICAL_ADDRESS) -1;
>}
> +
> +  DEBUG ((DEBUG_INFO, "WakeupBufferStart = %x, WakeupBufferSize
> = %x\n",
> +  (UINTN) StartAddress, WakeupBufferSize));
> +
>return (UINTN) StartAddress;
>  }
> 
> --
> 2.15.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch] Document: Add PCD flexible format value EBNF in Fdf.

[Feng, Bob C]

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

This patch is to add flexible PCD value format EBNF into Fdf spec.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Bob Feng 
Cc: Liming Gao 
Cc: Yonghong Zhu 
---
 3_edk_ii_fdf_file_format/32_fdf_definition.md | 25 ---
 3_edk_ii_fdf_file_format/35_[fd]_sections.md  |  4 +--
 3_edk_ii_fdf_file_format/36_[fv]_sections.md  |  4 +--
 .../37_[capsule]_sections.md  |  4 +--
 4 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/3_edk_ii_fdf_file_format/32_fdf_definition.md 
b/3_edk_ii_fdf_file_format/32_fdf_definition.md
index db098cf..2b044ab 100644
--- a/3_edk_ii_fdf_file_format/32_fdf_definition.md
+++ b/3_edk_ii_fdf_file_format/32_fdf_definition.md
@@ -1,9 +1,9 @@