Re: [edk2] [PATCH v1 0/2] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg

2019-03-15 Thread Jordan Justen
On 2019-03-15 04:09:56, Ard Biesheuvel wrote:
> On Fri, 15 Mar 2019 at 08:16, Hao Wu  wrote:
> >
> > This series will update the OVMF to stop using the ISA drivers within
> > IntelFrameworkModulePkg.
> >
> > As the replacement, a new OVMF Super I/O bus driver has been add which
> > will install the Super I/O protocol for ISA serial and PS2 keyboard
> > devices. By doing so, these devices can be managed by:
> >
> >   MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
> >   MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf
> >
> > respectively.
> >
> 
> Just a couple of notes from my side - I'm sure Laszlo will have a much
> longer list :-)
> 
> - Dropping the floppy driver is fine with me.
> - What is OVMF specific about this driver? Is it only the hardcoded
> list of COM1/COM2/PS2 keyboard? If so, should we split this into a
> driver and a library class, where the driver lives in MdeModulePkg,
> and the library is implemented in the context of OVMF?
> - Regardless of the previous, adding the new driver should be a patch
> of its own.

Yeah. I think the first patch should add the new driver, and update
the .dsc files to build the new driver. But, don't update the .fdf, so
there shouldn't be a functional change to the OVMF output after the
first patch.

Then the second patch can modify the .dsc and .fdf files to swap out
the drivers.

-Jordan

> - I spotted an incorrect reference to PCI_IO_DEV in a comment (patch #2)
> 
> 
> 
> >
> > Tests done:
> > A. GCC5 & VS2015x86 tool chains build pass
> > B. Launch QEMU (2.4.50, Windows) with command:
> >> qemu-system-x86_64.exe -pflash \OVMF.fd -serial file:1.txt 
> > -serial file:2.txt
> >
> >Able to see the ISA COM1/COM2 UART and PS2Keyboard devices under Shell
> >using command 'devtree';
> >
> >Both the serials and PS2 keyboard are working fine;
> >
> > Cc: Jordan Justen 
> > Cc: Laszlo Ersek 
> > Cc: Ard Biesheuvel 
> > Cc: Ray Ni 
> >
> > Hao Wu (2):
> >   OvmfPkg: Drop the ISA Floppy device support
> >   OvmfPkg: Add an Super IO bus driver to replace the current ISA support
> >
> >  OvmfPkg/OvmfPkgIa32.dsc   |  10 +-
> >  OvmfPkg/OvmfPkgIa32X64.dsc|  10 +-
> >  OvmfPkg/OvmfPkgX64.dsc|  10 +-
> >  OvmfPkg/OvmfPkgIa32.fdf   |  10 +-
> >  OvmfPkg/OvmfPkgIa32X64.fdf|  10 +-
> >  OvmfPkg/OvmfPkgX64.fdf|  10 +-
> >  OvmfPkg/SioBusDxe/SioBusDxe.inf   |  54 ++
> >  OvmfPkg/SioBusDxe/SioBusDxe.h | 332 +++
> >  OvmfPkg/SioBusDxe/SioService.h| 221 +++
> >  OvmfPkg/SioBusDxe/ComponentName.c | 167 ++
> >  OvmfPkg/SioBusDxe/SioBusDxe.c | 622 
> >  OvmfPkg/SioBusDxe/SioService.c| 405 +
> >  OvmfPkg/SioBusDxe/SioBusDxe.uni   |  21 +
> >  13 files changed, 1846 insertions(+), 36 deletions(-)
> >  create mode 100644 OvmfPkg/SioBusDxe/SioBusDxe.inf
> >  create mode 100644 OvmfPkg/SioBusDxe/SioBusDxe.h
> >  create mode 100644 OvmfPkg/SioBusDxe/SioService.h
> >  create mode 100644 OvmfPkg/SioBusDxe/ComponentName.c
> >  create mode 100644 OvmfPkg/SioBusDxe/SioBusDxe.c
> >  create mode 100644 OvmfPkg/SioBusDxe/SioService.c
> >  create mode 100644 OvmfPkg/SioBusDxe/SioBusDxe.uni
> >
> > --
> > 2.12.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] Change EDK II to BSD+Patent License

2019-03-15 Thread Julien Grall

Hi Lars,

On 15/03/2019 17:04, Lars Kurth wrote:



On 15/03/2019, 02:35, "Julien Grall"  wrote:

 Hi Lars,
 
 On 14/03/2019 20:04, Lars Kurth wrote:

 >
 >
 > On 14/03/2019, 12:07, "Julien Grall"  wrote:
 >
 >  (+ Lars)
 >
 >  On 3/14/19 10:55 AM, Laszlo Ersek wrote:
 >  > On 03/13/19 18:54, Kinney, Michael D wrote:
 >  > (2.2.2) Files that seem to be covered by the MIT license.
 >  >
 >  >OvmfPkg/Include/IndustryStandard/Xen/arch-arm/xen.h
 > ...
 >  >OvmfPkg/XenBusDxe/XenStore.h
 >  >
 >  >  It's OK to leave these untouched, for now. Later, we 
should
 >  >  probably replace their license blocks with
 >  >  "SPDX-License-Identifier: MIT" (as appropriate). It 
might make
 >  >  sense to file a TianoCore BZ about them immediately, 
with a
 >  >  BZ-dependency on BZ#1373.
 >
 >  I have added Lars Kurth to confirm the license.
 >
 > Can you give me some context? It's not clear to me what you expect me to 
do.
 
 EDK2 is converting the full copyright in each file to SDPX identifier. While the

 copyright looks like an MIT license, it has never been confirmed. Andrew 
Cooper
 suggested you might be able to confirm.
 
Is there a web-link to the files/repos such that I don’t have to clone the repo

Lars


Here an example of files from Xen public headers:

https://xenbits.xen.org/gitweb/?p=xen.git;a=tree;f=xen/include/public;h=0618b0134d2b9babcba71a3f0f86be5a84468b50;hb=HEAD

Cheers,

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


[edk2] [Patch] BaseTools: Remove EDKI related logic from Trim tool

2019-03-15 Thread Feng, Bob C
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1350

Remove EDKI related logic from Trim tool.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Bob Feng 
Cc: Liming Gao 
---
 BaseTools/Source/Python/Trim/Trim.py | 160 ---
 1 file changed, 160 deletions(-)

diff --git a/BaseTools/Source/Python/Trim/Trim.py 
b/BaseTools/Source/Python/Trim/Trim.py
index 825ed3e5d5..228779b5a9 100644
--- a/BaseTools/Source/Python/Trim/Trim.py
+++ b/BaseTools/Source/Python/Trim/Trim.py
@@ -59,74 +59,10 @@ gLongNumberPattern = 
re.compile("(?<=[^a-zA-Z0-9_])(0[xX][0-9a-fA-F]+|[0-9]+)U?L
 ## Regular expression for matching "Include ()" in asl file
 gAslIncludePattern = re.compile("^(\s*)[iI]nclude\s*\(\"?([^\"\(\)]+)\"\)", 
re.MULTILINE)
 ## Regular expression for matching C style #include "XXX.asl" in asl file
 gAslCIncludePattern = 
re.compile(r'^(\s*)#include\s*[<"]\s*([-\\/\w.]+)\s*([>"])', re.MULTILINE)
 ## Patterns used to convert EDK conventions to EDK2 ECP conventions
-gImportCodePatterns = [
-[
-re.compile('^(\s*)\(\*\*PeiServices\)\.PciCfg\s*=\s*([^;\s]+);', 
re.MULTILINE),
-'''\\1{
-\\1  STATIC EFI_PEI_PPI_DESCRIPTOR gEcpPeiPciCfgPpiList = {
-\\1(EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
-\\1,
-\\1\\2
-\\1  };
-\\1  (**PeiServices).InstallPpi (PeiServices, );
-\\1}'''
-],
-
-[
-re.compile('^(\s*)\(\*PeiServices\)->PciCfg\s*=\s*([^;\s]+);', 
re.MULTILINE),
-'''\\1{
-\\1  STATIC EFI_PEI_PPI_DESCRIPTOR gEcpPeiPciCfgPpiList = {
-\\1(EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
-\\1,
-\\1\\2
-\\1  };
-\\1  (**PeiServices).InstallPpi (PeiServices, );
-\\1}'''
-],
-
-[
-re.compile("(\s*).+->Modify[\s\n]*\(", re.MULTILINE),
-'\\1PeiLibPciCfgModify ('
-],
-
-[
-re.compile("(\W*)gRT->ReportStatusCode[\s\n]*\(", re.MULTILINE),
-'\\1EfiLibReportStatusCode ('
-],
-
-[
-re.compile('#include\s+EFI_GUID_DEFINITION\s*\(FirmwareFileSystem\)', 
re.MULTILINE),
-'#include EFI_GUID_DEFINITION (FirmwareFileSystem)\n#include 
EFI_GUID_DEFINITION (FirmwareFileSystem2)'
-],
-
-[
-re.compile('gEfiFirmwareFileSystemGuid', re.MULTILINE),
-'gEfiFirmwareFileSystem2Guid'
-],
-
-[
-re.compile('EFI_FVH_REVISION', re.MULTILINE),
-'EFI_FVH_PI_REVISION'
-],
-
-[
-
re.compile("(\s*)\S*CreateEvent\s*\([\s\n]*EFI_EVENT_SIGNAL_READY_TO_BOOT[^,]*,((?:[^;]+\n)+)(\s*\));",
 re.MULTILINE),
-'\\1EfiCreateEventReadyToBoot (\\2\\3;'
-],
-
-[
-
re.compile("(\s*)\S*CreateEvent\s*\([\s\n]*EFI_EVENT_SIGNAL_LEGACY_BOOT[^,]*,((?:[^;]+\n)+)(\s*\));",
 re.MULTILINE),
-'\\1EfiCreateEventLegacyBoot (\\2\\3;'
-],
-#[
-#re.compile("(\W)(PEI_PCI_CFG_PPI)(\W)", re.MULTILINE),
-#'\\1ECP_\\2\\3'
-#]
-]
 
 ## file cache to avoid circular include in ASL file
 gIncludedAslFile = []
 
 ## Trim preprocessed source code
@@ -492,101 +428,10 @@ def GenerateVfrBinSec(ModuleName, DebugDir, OutputFile):
 EdkLogger.error("Trim", FILE_WRITE_FAILURE, "Write data to file %s 
failed, please check whether the file been locked or using by other 
applications." %OutputFile, None)
 
 fStringIO.close ()
 fInputfile.close ()
 
-## Trim EDK source code file(s)
-#
-#
-# @param  SourceFile or directory to be trimmed
-# @param  TargetFile or directory to store the trimmed content
-#
-def TrimEdkSources(Source, Target):
-if os.path.isdir(Source):
-for CurrentDir, Dirs, Files in os.walk(Source):
-if '.svn' in Dirs:
-Dirs.remove('.svn')
-elif "CVS" in Dirs:
-Dirs.remove("CVS")
-
-for FileName in Files:
-Dummy, Ext = os.path.splitext(FileName)
-if Ext.upper() not in ['.C', '.H']: continue
-if Target is None or Target == '':
-TrimEdkSourceCode(
-os.path.join(CurrentDir, FileName),
-os.path.join(CurrentDir, FileName)
-)
-else:
-TrimEdkSourceCode(
-os.path.join(CurrentDir, FileName),
-os.path.join(Target, CurrentDir[len(Source)+1:], 
FileName)
-)
-else:
-TrimEdkSourceCode(Source, Target)
-
-## Trim one EDK source code file
-#
-# Do following replacement:
-#
-#   (**PeiServices\).PciCfg = <*>;
-#   =>  {
-# STATIC EFI_PEI_PPI_DESCRIPTOR gEcpPeiPciCfgPpiList = {
-# (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
-# ,
-# <*>
-#   };
-#   (**PeiServices).InstallPpi (PeiServices, );
-#
-#   <*>Modify(<*>)
-#   =>  PeiLibPciCfgModify (<*>)
-#
-#   gRT->ReportStatusCode (<*>)
-#   => EfiLibReportStatusCode (<*>)
-#
-#   #include 
-#   =>  

[edk2] [Patch] BaseTools: Remove the logic SourceOverridePath

2019-03-15 Thread Feng, Bob C
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1350
SOURCE_OVERRIDE_PATH is for EDK component INF files.
The corresponding logic should be removed.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Bob Feng 
Cc: Liming Gao 
---
 BaseTools/Source/Python/AutoGen/AutoGen.py| 13 +
 BaseTools/Source/Python/Common/DataType.py|  2 --
 BaseTools/Source/Python/Common/GlobalData.py  |  1 -
 .../Python/CommonDataClass/DataClass.py   |  2 --
 .../Ecc/MetaFileWorkspace/MetaFileParser.py   |  8 
 BaseTools/Source/Python/Eot/EotMain.py|  2 +-
 BaseTools/Source/Python/Eot/InfParserLite.py  |  6 +-
 .../Source/Python/UPT/Library/DataType.py |  3 ---
 .../Python/Workspace/BuildClassObject.py  |  1 -
 .../Source/Python/Workspace/DscBuildData.py   | 19 ---
 .../Source/Python/Workspace/InfBuildData.py   | 11 +--
 .../Source/Python/Workspace/MetaFileParser.py |  8 
 12 files changed, 4 insertions(+), 72 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 568d535754..8c7c20a386 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -1139,11 +1139,10 @@ class PlatformAutoGen(AutoGen):
 self.WorkspaceDir = Workspace.WorkspaceDir
 self.ToolChain = Toolchain
 self.BuildTarget = Target
 self.Arch = Arch
 self.SourceDir = PlatformFile.SubDir
-self.SourceOverrideDir = None
 self.FdTargetList = self.Workspace.FdTargetList
 self.FvTargetList = self.Workspace.FvTargetList
 # get the original module/package/platform objects
 self.BuildDatabase = Workspace.BuildDatabase
 self.DscBuildDataObj = Workspace.Platform
@@ -2557,15 +2556,10 @@ class ModuleAutoGen(AutoGen):
 self.PlatformInfo = PlatformAutoGen(Workspace, PlatformFile, Target, 
Toolchain, Arch)
 
 self.SourceDir = self.MetaFile.SubDir
 self.SourceDir = mws.relpath(self.SourceDir, self.WorkspaceDir)
 
-self.SourceOverrideDir = None
-# use overridden path defined in DSC file
-if self.MetaFile.Key in GlobalData.gOverrideDir:
-self.SourceOverrideDir = GlobalData.gOverrideDir[self.MetaFile.Key]
-
 self.ToolChain = Toolchain
 self.BuildTarget = Target
 self.Arch = Arch
 self.ToolChainFamily = self.PlatformInfo.ToolChainFamily
 self.BuildRuleFamily = self.PlatformInfo.BuildRuleFamily
@@ -2766,16 +2760,11 @@ class ModuleAutoGen(AutoGen):
 @cached_property
 def CustomMakefile(self):
 RetVal = {}
 for Type in self.Module.CustomMakefile:
 MakeType = gMakeTypeMap[Type] if Type in gMakeTypeMap else 'nmake'
-if self.SourceOverrideDir is not None:
-File = os.path.join(self.SourceOverrideDir, 
self.Module.CustomMakefile[Type])
-if not os.path.exists(File):
-File = os.path.join(self.SourceDir, 
self.Module.CustomMakefile[Type])
-else:
-File = os.path.join(self.SourceDir, 
self.Module.CustomMakefile[Type])
+File = os.path.join(self.SourceDir, 
self.Module.CustomMakefile[Type])
 RetVal[MakeType] = File
 return RetVal
 
 ## Return the directory of the makefile
 #
diff --git a/BaseTools/Source/Python/Common/DataType.py 
b/BaseTools/Source/Python/Common/DataType.py
index 798c0e353d..685f428862 100644
--- a/BaseTools/Source/Python/Common/DataType.py
+++ b/BaseTools/Source/Python/Common/DataType.py
@@ -304,12 +304,10 @@ TAB_COMPONENTS_IA32 = TAB_COMPONENTS + TAB_SPLIT + 
TAB_ARCH_IA32
 TAB_COMPONENTS_X64 = TAB_COMPONENTS + TAB_SPLIT + TAB_ARCH_X64
 TAB_COMPONENTS_ARM = TAB_COMPONENTS + TAB_SPLIT + TAB_ARCH_ARM
 TAB_COMPONENTS_EBC = TAB_COMPONENTS + TAB_SPLIT + TAB_ARCH_EBC
 TAB_COMPONENTS_AARCH64 = TAB_COMPONENTS + TAB_SPLIT + TAB_ARCH_AARCH64
 
-TAB_COMPONENTS_SOURCE_OVERRIDE_PATH = 'SOURCE_OVERRIDE_PATH'
-
 TAB_BUILD_OPTIONS = 'BuildOptions'
 
 TAB_DEFINE = 'DEFINE'
 TAB_NMAKE = 'Nmake'
 TAB_USER_EXTENSIONS = 'UserExtensions'
diff --git a/BaseTools/Source/Python/Common/GlobalData.py 
b/BaseTools/Source/Python/Common/GlobalData.py
index f117998b0b..79b21324de 100644
--- a/BaseTools/Source/Python/Common/GlobalData.py
+++ b/BaseTools/Source/Python/Common/GlobalData.py
@@ -27,11 +27,10 @@ gPlatformPcds = {}
 # PCDs with type that are not fixed at build and feature flag
 gPlatformOtherPcds = {}
 gActivePlatform = None
 gCommandLineDefines = {}
 gEdkGlobal = {}
-gOverrideDir = {}
 gCommandMaxLength = 4096
 # for debug trace purpose when problem occurs
 gProcessingFile = ''
 gBuildingModule = ''
 gSkuids = []
diff --git a/BaseTools/Source/Python/CommonDataClass/DataClass.py 
b/BaseTools/Source/Python/CommonDataClass/DataClass.py
index 2d93f79b09..5d0c664f6d 100644
--- a/BaseTools/Source/Python/CommonDataClass/DataClass.py
+++ 

Re: [edk2] [PATCH edk2-platforms 0/2] Platforms/ARM/SgiPkg: apply product names for sgiclarka and sgiclarkh platforms

2019-03-15 Thread Thomas Abraham
On Fri, Mar 15, 2019 at 6:32 PM Ard Biesheuvel
 wrote:
>
> On Fri, 15 Mar 2019 at 13:44, Thomas Abraham  wrote:
> >
> > On Fri, Mar 15, 2019 at 6:04 PM Ard Biesheuvel
> >  wrote:
> > >
> > > On Fri, 15 Mar 2019 at 13:25, Thomas Abraham  
> > > wrote:
> > > >
> > > > On Fri, Mar 15, 2019 at 5:27 PM Ard Biesheuvel
> > > >  wrote:
> > > > >
> > > > > On Fri, 15 Mar 2019 at 12:40, Jagadeesh Ujja  
> > > > > wrote:
> > > > > >
> > > > > > On Fri, Mar 15, 2019 at 4:55 PM Ard Biesheuvel
> > > > > >  wrote:
> > > > > > >
> > > > > > > On Fri, 15 Mar 2019 at 12:17, Jagadeesh Ujja 
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > hi Ard,
> > > > > > > >
> > > > > > > > On Fri, Mar 15, 2019 at 4:14 PM Ard Biesheuvel
> > > > > > > >  wrote:
> > > > > > > > >
> > > > > > > > > On Fri, 15 Mar 2019 at 09:21, Jagadeesh Ujja 
> > > > > > > > >  wrote:
> > > > > > > > > >
> > > > > > > > > > hi Ard/Leif
> > > > > > > > > >
> > > > > > > > > > Please let me know if you have any comments on this patch 
> > > > > > > > > > set
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > HI Jagadeesh,
> > > > > > > > >
> > > > > > > > > What does RdE1Edge or RdN1Edge mean?
> > > > > > > >
> > > > > > > > RdE1Edge/RdN1Edge are new product name
> > > > > > > > Rd stands for reference design.
> > > > > > > >
> > > > > > > > The Neoverse E1 Edge Reference Design
> > > > > > > > The Neoverse N1 Edge Reference Design
> > > > > > > >
> > > > > > >
> > > > > > > So 'reference design' is the name of the platform based on the
> > > > > > > Neoverse E1 and N1, respectively?
> > > > > >
> > > > > > yes ,
> > > > > > 'reference design' is a class of platform, in which we have Neoverse
> > > > > > E1/N1 cores support. Going forward other cores can come in
> > > > > >
> > > > >
> > > > > Could we drop the 'edge'? It seems redundant.
> > > >
> > > > Hi Ard,
> > > >
> > > > 'edge' here would be required because there is also a 'hyperscale'
> > > > class of platform based on N1 cores.
> > > > (https://www.anandtech.com/show/13959/arm-announces-neoverse-n1-platform/3).
> > > >
> > > > >
> > > > > I'd prefer it if we could stick with
> > > > >
> > > > > NeoverseE1
> > > > > NeoverseN1
> > > > >
> > > > > or if the reference design part is relevant
> > > > >
> > > > > NeoverseE1ReferenceDesign
> > > > > NeoverseN1ReferenceDesign
> > > >
> > > > The ReferenceDesign part is relevant and without which the name would
> > > > only indicate the CPU name (NeoverseN1). And so is 'edge' because
> > > > there is 'hyperscale' variant as of today and there could be other
> > > > variants as well.
> > > >
> > >
> > > Hi Thomas,
> > >
> > > Thanks for the clarification.
> > >
> > > So could we go with
> > >
> > > > > NeoverseE1EdgeReferenceDesign
> > > > > NeoverseN1EdgeReferenceDesign
> > >
> > > instead?
> >
> > The official product names being used for these two platforms are
> > 'RD-N1-Edge' and 'RD-E1-Edge'. So to keep to platform name in line
> > with the product name and also to keep it short, 'RdN1Edge' and
> > 'RdE1Edge' names are being used in this patch series. Are there any
> > limitations in using this shorter name for the platforms.
> >
>
> No, I just found them a bit cryptic. But if they are an exact match
> with the product names, I don't have any objections.
>
> Reviewed-by: Ard Biesheuvel 
>
> Pushed as 68cc99303e38..a8f34e065815

Thank you Ard.

> ___
> 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 edk2-platforms 0/2] Platforms/ARM/SgiPkg: apply product names for sgiclarka and sgiclarkh platforms

2019-03-15 Thread Ard Biesheuvel
On Fri, 15 Mar 2019 at 13:44, Thomas Abraham  wrote:
>
> On Fri, Mar 15, 2019 at 6:04 PM Ard Biesheuvel
>  wrote:
> >
> > On Fri, 15 Mar 2019 at 13:25, Thomas Abraham  wrote:
> > >
> > > On Fri, Mar 15, 2019 at 5:27 PM Ard Biesheuvel
> > >  wrote:
> > > >
> > > > On Fri, 15 Mar 2019 at 12:40, Jagadeesh Ujja  
> > > > wrote:
> > > > >
> > > > > On Fri, Mar 15, 2019 at 4:55 PM Ard Biesheuvel
> > > > >  wrote:
> > > > > >
> > > > > > On Fri, 15 Mar 2019 at 12:17, Jagadeesh Ujja 
> > > > > >  wrote:
> > > > > > >
> > > > > > > hi Ard,
> > > > > > >
> > > > > > > On Fri, Mar 15, 2019 at 4:14 PM Ard Biesheuvel
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > On Fri, 15 Mar 2019 at 09:21, Jagadeesh Ujja 
> > > > > > > >  wrote:
> > > > > > > > >
> > > > > > > > > hi Ard/Leif
> > > > > > > > >
> > > > > > > > > Please let me know if you have any comments on this patch set
> > > > > > > > >
> > > > > > > >
> > > > > > > > HI Jagadeesh,
> > > > > > > >
> > > > > > > > What does RdE1Edge or RdN1Edge mean?
> > > > > > >
> > > > > > > RdE1Edge/RdN1Edge are new product name
> > > > > > > Rd stands for reference design.
> > > > > > >
> > > > > > > The Neoverse E1 Edge Reference Design
> > > > > > > The Neoverse N1 Edge Reference Design
> > > > > > >
> > > > > >
> > > > > > So 'reference design' is the name of the platform based on the
> > > > > > Neoverse E1 and N1, respectively?
> > > > >
> > > > > yes ,
> > > > > 'reference design' is a class of platform, in which we have Neoverse
> > > > > E1/N1 cores support. Going forward other cores can come in
> > > > >
> > > >
> > > > Could we drop the 'edge'? It seems redundant.
> > >
> > > Hi Ard,
> > >
> > > 'edge' here would be required because there is also a 'hyperscale'
> > > class of platform based on N1 cores.
> > > (https://www.anandtech.com/show/13959/arm-announces-neoverse-n1-platform/3).
> > >
> > > >
> > > > I'd prefer it if we could stick with
> > > >
> > > > NeoverseE1
> > > > NeoverseN1
> > > >
> > > > or if the reference design part is relevant
> > > >
> > > > NeoverseE1ReferenceDesign
> > > > NeoverseN1ReferenceDesign
> > >
> > > The ReferenceDesign part is relevant and without which the name would
> > > only indicate the CPU name (NeoverseN1). And so is 'edge' because
> > > there is 'hyperscale' variant as of today and there could be other
> > > variants as well.
> > >
> >
> > Hi Thomas,
> >
> > Thanks for the clarification.
> >
> > So could we go with
> >
> > > > NeoverseE1EdgeReferenceDesign
> > > > NeoverseN1EdgeReferenceDesign
> >
> > instead?
>
> The official product names being used for these two platforms are
> 'RD-N1-Edge' and 'RD-E1-Edge'. So to keep to platform name in line
> with the product name and also to keep it short, 'RdN1Edge' and
> 'RdE1Edge' names are being used in this patch series. Are there any
> limitations in using this shorter name for the platforms.
>

No, I just found them a bit cryptic. But if they are an exact match
with the product names, I don't have any objections.

Reviewed-by: Ard Biesheuvel 

Pushed as 68cc99303e38..a8f34e065815
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH edk2-platforms v2 3/3] Platform/ARM/SgiPkg: add MM based UEFI secure boot support

2019-03-15 Thread Thomas Abraham
On Fri, Mar 15, 2019 at 6:12 PM Ard Biesheuvel
 wrote:
>
> On Fri, 15 Mar 2019 at 13:30, Thomas Abraham  wrote:
> >
> > On Fri, Mar 15, 2019 at 5:51 PM Ard Biesheuvel
> >  wrote:
> > >
> > > On Tue, 12 Mar 2019 at 17:06, Jagadeesh Ujja  
> > > wrote:
> > > >
> > > > This implements support for UEFI secure boot on SGI platforms using
> > > > the standalone MM framework. This moves all of the software handling
> > > > of the UEFI authenticated variable store into the standalone MM
> > > > context residing in a secure partition.
> > > >
> > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > Signed-off-by: Jagadeesh Ujja 
> > > > ---
> > > >  Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc | 34 +++-
> > > >  Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf |  5 +++
> > > >  Platform/ARM/SgiPkg/SgiPlatform.dsc  | 18 ++-
> > > >  Platform/ARM/SgiPkg/SgiPlatform.fdf  |  7 +++-
> > > >  4 files changed, 61 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc 
> > > > b/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc
> > > > index 49fc919..b6aa90b 100644
> > > > --- a/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc
> > > > +++ b/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc
> > > > @@ -26,6 +26,7 @@
> > > >SKUID_IDENTIFIER   = DEFAULT
> > > >FLASH_DEFINITION   = 
> > > > Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf
> > > >DEFINE DEBUG_MESSAGE   = TRUE
> > > > +  DEFINE SECURE_BOOT_ENABLE  = FALSE
> > > >
> > >
> > > Maybe I wasn't clear before, but I don't see the point of building the
> > > MM component without secure boot enabled. So can we drop this from
> > > this side?
> >
> > Hi Ard,
> >
> > On the SGI platforms, the MM component is used for platform RAS error
> > handling as well and secure boot is not mandatory in such a build. So
> > the build of MM component is being kept independent of secure boot.
> >
>
> Hi Thomas,
>
> When building the MM side of the platform without secure boot, the
> only MM modules that are included are
>
> > > >INF StandaloneMmPkg/Core/StandaloneMmCore.inf
> > > >INF 
> > > > StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
>
> neither of which implement RAS handling. So are you saying this is
> functionality that runs in MM context, but it has not been upstreamed
> yet?

Hi Ard,

Yes, this functionality is yet to be upstreamed and there is work
happening in that direction. So the MM build is being kept independent
of secure boot feature.

Thanks,
Thomas.


>
>
>
> >
> > >
> > > For the non-secure side, it is a different matter, obviously.
> > >
> > > ># LzmaF86
> > > >DEFINE COMPRESSION_TOOL_GUID   = D42AE6BD-1352-4bfb-909A-CA72A6EAE889
> > > > @@ -83,7 +84,17 @@
> > > >
> > > > HobLib|StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf
> > > >
> > > > MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf
> > > >
> > > > MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf
> > > > -
> > > > +!if $(SECURE_BOOT_ENABLE) == TRUE
> > > > +  
> > > > AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> > > > +  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
> > > > +  IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> > > > +  
> > > > NorFlashPlatformLib|Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.inf
> > > > +  OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
> > > > +  
> > > > PlatformSecureLib|SecurityPkg/Library/PlatformSecureLibNull/PlatformSecureLibNull.inf
> > > > +  
> > > > SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> > > > +  
> > > > TimerLib|MdePkg/Library/BaseTimerLibNullTemplate/BaseTimerLibNullTemplate.inf
> > > > +  VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
> > > > +!endif
> > > >  
> > > > 
> > > >  #
> > > >  # Pcd Section - list of all EDK II PCD Entries defined by this Platform
> > > > @@ -100,6 +111,21 @@
> > > >
> > > >gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x2
> > > >
> > > > +!if $(SECURE_BOOT_ENABLE) == TRUE
> > > > +  #Secure Storage
> > > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
> > > > +  gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|TRUE
> > > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
> > > > +
> > > > +  ## NV Storage - 1MB*3 in NOR2 Flash
> > > > +  
> > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x1040
> > > > +  
> > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x0010
> > > > +  
> > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x1050
> > > > +  
> > > > 

Re: [edk2] [PATCH edk2-platforms v2 3/3] Platform/ARM/SgiPkg: add MM based UEFI secure boot support

2019-03-15 Thread Ard Biesheuvel
On Fri, 15 Mar 2019 at 13:47, Thomas Abraham  wrote:
>
> On Fri, Mar 15, 2019 at 6:12 PM Ard Biesheuvel
>  wrote:
> >
> > On Fri, 15 Mar 2019 at 13:30, Thomas Abraham  wrote:
> > >
> > > On Fri, Mar 15, 2019 at 5:51 PM Ard Biesheuvel
> > >  wrote:
> > > >
> > > > On Tue, 12 Mar 2019 at 17:06, Jagadeesh Ujja  
> > > > wrote:
> > > > >
> > > > > This implements support for UEFI secure boot on SGI platforms using
> > > > > the standalone MM framework. This moves all of the software handling
> > > > > of the UEFI authenticated variable store into the standalone MM
> > > > > context residing in a secure partition.
> > > > >
> > > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > > Signed-off-by: Jagadeesh Ujja 
> > > > > ---
> > > > >  Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc | 34 
> > > > > +++-
> > > > >  Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf |  5 +++
> > > > >  Platform/ARM/SgiPkg/SgiPlatform.dsc  | 18 ++-
> > > > >  Platform/ARM/SgiPkg/SgiPlatform.fdf  |  7 +++-
> > > > >  4 files changed, 61 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc 
> > > > > b/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc
> > > > > index 49fc919..b6aa90b 100644
> > > > > --- a/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc
> > > > > +++ b/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc
> > > > > @@ -26,6 +26,7 @@
> > > > >SKUID_IDENTIFIER   = DEFAULT
> > > > >FLASH_DEFINITION   = 
> > > > > Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf
> > > > >DEFINE DEBUG_MESSAGE   = TRUE
> > > > > +  DEFINE SECURE_BOOT_ENABLE  = FALSE
> > > > >
> > > >
> > > > Maybe I wasn't clear before, but I don't see the point of building the
> > > > MM component without secure boot enabled. So can we drop this from
> > > > this side?
> > >
> > > Hi Ard,
> > >
> > > On the SGI platforms, the MM component is used for platform RAS error
> > > handling as well and secure boot is not mandatory in such a build. So
> > > the build of MM component is being kept independent of secure boot.
> > >
> >
> > Hi Thomas,
> >
> > When building the MM side of the platform without secure boot, the
> > only MM modules that are included are
> >
> > > > >INF StandaloneMmPkg/Core/StandaloneMmCore.inf
> > > > >INF 
> > > > > StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
> >
> > neither of which implement RAS handling. So are you saying this is
> > functionality that runs in MM context, but it has not been upstreamed
> > yet?
>
> Hi Ard,
>
> Yes, this functionality is yet to be upstreamed and there is work
> happening in that direction. So the MM build is being kept independent
> of secure boot feature.
>

OK, fair enough.

I will look in more detail once the NorFlashDxe changes are reviewed and merged.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH edk2-platforms 0/2] Platforms/ARM/SgiPkg: apply product names for sgiclarka and sgiclarkh platforms

2019-03-15 Thread Thomas Abraham
On Fri, Mar 15, 2019 at 6:04 PM Ard Biesheuvel
 wrote:
>
> On Fri, 15 Mar 2019 at 13:25, Thomas Abraham  wrote:
> >
> > On Fri, Mar 15, 2019 at 5:27 PM Ard Biesheuvel
> >  wrote:
> > >
> > > On Fri, 15 Mar 2019 at 12:40, Jagadeesh Ujja  
> > > wrote:
> > > >
> > > > On Fri, Mar 15, 2019 at 4:55 PM Ard Biesheuvel
> > > >  wrote:
> > > > >
> > > > > On Fri, 15 Mar 2019 at 12:17, Jagadeesh Ujja  
> > > > > wrote:
> > > > > >
> > > > > > hi Ard,
> > > > > >
> > > > > > On Fri, Mar 15, 2019 at 4:14 PM Ard Biesheuvel
> > > > > >  wrote:
> > > > > > >
> > > > > > > On Fri, 15 Mar 2019 at 09:21, Jagadeesh Ujja 
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > hi Ard/Leif
> > > > > > > >
> > > > > > > > Please let me know if you have any comments on this patch set
> > > > > > > >
> > > > > > >
> > > > > > > HI Jagadeesh,
> > > > > > >
> > > > > > > What does RdE1Edge or RdN1Edge mean?
> > > > > >
> > > > > > RdE1Edge/RdN1Edge are new product name
> > > > > > Rd stands for reference design.
> > > > > >
> > > > > > The Neoverse E1 Edge Reference Design
> > > > > > The Neoverse N1 Edge Reference Design
> > > > > >
> > > > >
> > > > > So 'reference design' is the name of the platform based on the
> > > > > Neoverse E1 and N1, respectively?
> > > >
> > > > yes ,
> > > > 'reference design' is a class of platform, in which we have Neoverse
> > > > E1/N1 cores support. Going forward other cores can come in
> > > >
> > >
> > > Could we drop the 'edge'? It seems redundant.
> >
> > Hi Ard,
> >
> > 'edge' here would be required because there is also a 'hyperscale'
> > class of platform based on N1 cores.
> > (https://www.anandtech.com/show/13959/arm-announces-neoverse-n1-platform/3).
> >
> > >
> > > I'd prefer it if we could stick with
> > >
> > > NeoverseE1
> > > NeoverseN1
> > >
> > > or if the reference design part is relevant
> > >
> > > NeoverseE1ReferenceDesign
> > > NeoverseN1ReferenceDesign
> >
> > The ReferenceDesign part is relevant and without which the name would
> > only indicate the CPU name (NeoverseN1). And so is 'edge' because
> > there is 'hyperscale' variant as of today and there could be other
> > variants as well.
> >
>
> Hi Thomas,
>
> Thanks for the clarification.
>
> So could we go with
>
> > > NeoverseE1EdgeReferenceDesign
> > > NeoverseN1EdgeReferenceDesign
>
> instead?

The official product names being used for these two platforms are
'RD-N1-Edge' and 'RD-E1-Edge'. So to keep to platform name in line
with the product name and also to keep it short, 'RdN1Edge' and
'RdE1Edge' names are being used in this patch series. Are there any
limitations in using this shorter name for the platforms.

Thanks,
Thomas.

> ___
> 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 edk2-platforms v2 3/3] Platform/ARM/SgiPkg: add MM based UEFI secure boot support

2019-03-15 Thread Ard Biesheuvel
On Fri, 15 Mar 2019 at 13:30, Thomas Abraham  wrote:
>
> On Fri, Mar 15, 2019 at 5:51 PM Ard Biesheuvel
>  wrote:
> >
> > On Tue, 12 Mar 2019 at 17:06, Jagadeesh Ujja  wrote:
> > >
> > > This implements support for UEFI secure boot on SGI platforms using
> > > the standalone MM framework. This moves all of the software handling
> > > of the UEFI authenticated variable store into the standalone MM
> > > context residing in a secure partition.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Jagadeesh Ujja 
> > > ---
> > >  Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc | 34 +++-
> > >  Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf |  5 +++
> > >  Platform/ARM/SgiPkg/SgiPlatform.dsc  | 18 ++-
> > >  Platform/ARM/SgiPkg/SgiPlatform.fdf  |  7 +++-
> > >  4 files changed, 61 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc 
> > > b/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc
> > > index 49fc919..b6aa90b 100644
> > > --- a/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc
> > > +++ b/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc
> > > @@ -26,6 +26,7 @@
> > >SKUID_IDENTIFIER   = DEFAULT
> > >FLASH_DEFINITION   = 
> > > Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf
> > >DEFINE DEBUG_MESSAGE   = TRUE
> > > +  DEFINE SECURE_BOOT_ENABLE  = FALSE
> > >
> >
> > Maybe I wasn't clear before, but I don't see the point of building the
> > MM component without secure boot enabled. So can we drop this from
> > this side?
>
> Hi Ard,
>
> On the SGI platforms, the MM component is used for platform RAS error
> handling as well and secure boot is not mandatory in such a build. So
> the build of MM component is being kept independent of secure boot.
>

Hi Thomas,

When building the MM side of the platform without secure boot, the
only MM modules that are included are

> > >INF StandaloneMmPkg/Core/StandaloneMmCore.inf
> > >INF StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf

neither of which implement RAS handling. So are you saying this is
functionality that runs in MM context, but it has not been upstreamed
yet?



>
> >
> > For the non-secure side, it is a different matter, obviously.
> >
> > ># LzmaF86
> > >DEFINE COMPRESSION_TOOL_GUID   = D42AE6BD-1352-4bfb-909A-CA72A6EAE889
> > > @@ -83,7 +84,17 @@
> > >
> > > HobLib|StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf
> > >
> > > MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf
> > >
> > > MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf
> > > -
> > > +!if $(SECURE_BOOT_ENABLE) == TRUE
> > > +  AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> > > +  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
> > > +  IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> > > +  
> > > NorFlashPlatformLib|Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.inf
> > > +  OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
> > > +  
> > > PlatformSecureLib|SecurityPkg/Library/PlatformSecureLibNull/PlatformSecureLibNull.inf
> > > +  
> > > SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> > > +  
> > > TimerLib|MdePkg/Library/BaseTimerLibNullTemplate/BaseTimerLibNullTemplate.inf
> > > +  VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
> > > +!endif
> > >  
> > > 
> > >  #
> > >  # Pcd Section - list of all EDK II PCD Entries defined by this Platform
> > > @@ -100,6 +111,21 @@
> > >
> > >gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x2
> > >
> > > +!if $(SECURE_BOOT_ENABLE) == TRUE
> > > +  #Secure Storage
> > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
> > > +  gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|TRUE
> > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
> > > +
> > > +  ## NV Storage - 1MB*3 in NOR2 Flash
> > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x1040
> > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x0010
> > > +  
> > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x1050
> > > +  
> > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x0010
> > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x1060
> > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x0010
> > > +!endif
> > > +
> > >  
> > > ###
> > >  #
> > >  # Components Section - list of the modules and components that will be 
> > > processed by compilation
> > > @@ -125,6 +151,12 @@
> > >

Re: [edk2] [PATCH edk2-platforms v2 3/3] Platform/ARM/SgiPkg: add MM based UEFI secure boot support

2019-03-15 Thread Thomas Abraham
On Fri, Mar 15, 2019 at 5:51 PM Ard Biesheuvel
 wrote:
>
> On Tue, 12 Mar 2019 at 17:06, Jagadeesh Ujja  wrote:
> >
> > This implements support for UEFI secure boot on SGI platforms using
> > the standalone MM framework. This moves all of the software handling
> > of the UEFI authenticated variable store into the standalone MM
> > context residing in a secure partition.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Jagadeesh Ujja 
> > ---
> >  Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc | 34 +++-
> >  Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf |  5 +++
> >  Platform/ARM/SgiPkg/SgiPlatform.dsc  | 18 ++-
> >  Platform/ARM/SgiPkg/SgiPlatform.fdf  |  7 +++-
> >  4 files changed, 61 insertions(+), 3 deletions(-)
> >
> > diff --git a/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc 
> > b/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc
> > index 49fc919..b6aa90b 100644
> > --- a/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc
> > +++ b/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc
> > @@ -26,6 +26,7 @@
> >SKUID_IDENTIFIER   = DEFAULT
> >FLASH_DEFINITION   = 
> > Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf
> >DEFINE DEBUG_MESSAGE   = TRUE
> > +  DEFINE SECURE_BOOT_ENABLE  = FALSE
> >
>
> Maybe I wasn't clear before, but I don't see the point of building the
> MM component without secure boot enabled. So can we drop this from
> this side?

Hi Ard,

On the SGI platforms, the MM component is used for platform RAS error
handling as well and secure boot is not mandatory in such a build. So
the build of MM component is being kept independent of secure boot.

Thanks,
Thomas.

>
> For the non-secure side, it is a different matter, obviously.
>
> ># LzmaF86
> >DEFINE COMPRESSION_TOOL_GUID   = D42AE6BD-1352-4bfb-909A-CA72A6EAE889
> > @@ -83,7 +84,17 @@
> >HobLib|StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf
> >
> > MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf
> >
> > MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf
> > -
> > +!if $(SECURE_BOOT_ENABLE) == TRUE
> > +  AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> > +  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
> > +  IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> > +  
> > NorFlashPlatformLib|Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.inf
> > +  OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
> > +  
> > PlatformSecureLib|SecurityPkg/Library/PlatformSecureLibNull/PlatformSecureLibNull.inf
> > +  
> > SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> > +  
> > TimerLib|MdePkg/Library/BaseTimerLibNullTemplate/BaseTimerLibNullTemplate.inf
> > +  VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
> > +!endif
> >  
> > 
> >  #
> >  # Pcd Section - list of all EDK II PCD Entries defined by this Platform
> > @@ -100,6 +111,21 @@
> >
> >gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x2
> >
> > +!if $(SECURE_BOOT_ENABLE) == TRUE
> > +  #Secure Storage
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
> > +  gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|TRUE
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
> > +
> > +  ## NV Storage - 1MB*3 in NOR2 Flash
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x1040
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x0010
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x1050
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x0010
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x1060
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x0010
> > +!endif
> > +
> >  
> > ###
> >  #
> >  # Components Section - list of the modules and components that will be 
> > processed by compilation
> > @@ -125,6 +151,12 @@
> >StandaloneMmPkg/Core/StandaloneMmCore.inf
> >
> >  [Components.AARCH64]
> > +!if $(SECURE_BOOT_ENABLE) == TRUE
> > +  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
> > +  
> > MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
> > +  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> > +!endif
> > +
> >StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
> >
> >  
> > ###
> > diff --git a/Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf 
> > b/Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf
> > 

Re: [edk2] [PATCH edk2-platforms 0/2] Platforms/ARM/SgiPkg: apply product names for sgiclarka and sgiclarkh platforms

2019-03-15 Thread Ard Biesheuvel
On Fri, 15 Mar 2019 at 13:25, Thomas Abraham  wrote:
>
> On Fri, Mar 15, 2019 at 5:27 PM Ard Biesheuvel
>  wrote:
> >
> > On Fri, 15 Mar 2019 at 12:40, Jagadeesh Ujja  wrote:
> > >
> > > On Fri, Mar 15, 2019 at 4:55 PM Ard Biesheuvel
> > >  wrote:
> > > >
> > > > On Fri, 15 Mar 2019 at 12:17, Jagadeesh Ujja  
> > > > wrote:
> > > > >
> > > > > hi Ard,
> > > > >
> > > > > On Fri, Mar 15, 2019 at 4:14 PM Ard Biesheuvel
> > > > >  wrote:
> > > > > >
> > > > > > On Fri, 15 Mar 2019 at 09:21, Jagadeesh Ujja 
> > > > > >  wrote:
> > > > > > >
> > > > > > > hi Ard/Leif
> > > > > > >
> > > > > > > Please let me know if you have any comments on this patch set
> > > > > > >
> > > > > >
> > > > > > HI Jagadeesh,
> > > > > >
> > > > > > What does RdE1Edge or RdN1Edge mean?
> > > > >
> > > > > RdE1Edge/RdN1Edge are new product name
> > > > > Rd stands for reference design.
> > > > >
> > > > > The Neoverse E1 Edge Reference Design
> > > > > The Neoverse N1 Edge Reference Design
> > > > >
> > > >
> > > > So 'reference design' is the name of the platform based on the
> > > > Neoverse E1 and N1, respectively?
> > >
> > > yes ,
> > > 'reference design' is a class of platform, in which we have Neoverse
> > > E1/N1 cores support. Going forward other cores can come in
> > >
> >
> > Could we drop the 'edge'? It seems redundant.
>
> Hi Ard,
>
> 'edge' here would be required because there is also a 'hyperscale'
> class of platform based on N1 cores.
> (https://www.anandtech.com/show/13959/arm-announces-neoverse-n1-platform/3).
>
> >
> > I'd prefer it if we could stick with
> >
> > NeoverseE1
> > NeoverseN1
> >
> > or if the reference design part is relevant
> >
> > NeoverseE1ReferenceDesign
> > NeoverseN1ReferenceDesign
>
> The ReferenceDesign part is relevant and without which the name would
> only indicate the CPU name (NeoverseN1). And so is 'edge' because
> there is 'hyperscale' variant as of today and there could be other
> variants as well.
>

Hi Thomas,

Thanks for the clarification.

So could we go with

> > NeoverseE1EdgeReferenceDesign
> > NeoverseN1EdgeReferenceDesign

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


Re: [edk2] [PATCH edk2-platforms 0/2] Platforms/ARM/SgiPkg: apply product names for sgiclarka and sgiclarkh platforms

2019-03-15 Thread Thomas Abraham
On Fri, Mar 15, 2019 at 5:27 PM Ard Biesheuvel
 wrote:
>
> On Fri, 15 Mar 2019 at 12:40, Jagadeesh Ujja  wrote:
> >
> > On Fri, Mar 15, 2019 at 4:55 PM Ard Biesheuvel
> >  wrote:
> > >
> > > On Fri, 15 Mar 2019 at 12:17, Jagadeesh Ujja  
> > > wrote:
> > > >
> > > > hi Ard,
> > > >
> > > > On Fri, Mar 15, 2019 at 4:14 PM Ard Biesheuvel
> > > >  wrote:
> > > > >
> > > > > On Fri, 15 Mar 2019 at 09:21, Jagadeesh Ujja  
> > > > > wrote:
> > > > > >
> > > > > > hi Ard/Leif
> > > > > >
> > > > > > Please let me know if you have any comments on this patch set
> > > > > >
> > > > >
> > > > > HI Jagadeesh,
> > > > >
> > > > > What does RdE1Edge or RdN1Edge mean?
> > > >
> > > > RdE1Edge/RdN1Edge are new product name
> > > > Rd stands for reference design.
> > > >
> > > > The Neoverse E1 Edge Reference Design
> > > > The Neoverse N1 Edge Reference Design
> > > >
> > >
> > > So 'reference design' is the name of the platform based on the
> > > Neoverse E1 and N1, respectively?
> >
> > yes ,
> > 'reference design' is a class of platform, in which we have Neoverse
> > E1/N1 cores support. Going forward other cores can come in
> >
>
> Could we drop the 'edge'? It seems redundant.

Hi Ard,

'edge' here would be required because there is also a 'hyperscale'
class of platform based on N1 cores.
(https://www.anandtech.com/show/13959/arm-announces-neoverse-n1-platform/3).

>
> I'd prefer it if we could stick with
>
> NeoverseE1
> NeoverseN1
>
> or if the reference design part is relevant
>
> NeoverseE1ReferenceDesign
> NeoverseN1ReferenceDesign

The ReferenceDesign part is relevant and without which the name would
only indicate the CPU name (NeoverseN1). And so is 'edge' because
there is 'hyperscale' variant as of today and there could be other
variants as well.

Thanks,
Thomas.

> ___
> 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 edk2-platforms v2 3/3] Platform/ARM/SgiPkg: add MM based UEFI secure boot support

2019-03-15 Thread Ard Biesheuvel
On Tue, 12 Mar 2019 at 17:06, Jagadeesh Ujja  wrote:
>
> This implements support for UEFI secure boot on SGI platforms using
> the standalone MM framework. This moves all of the software handling
> of the UEFI authenticated variable store into the standalone MM
> context residing in a secure partition.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jagadeesh Ujja 
> ---
>  Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc | 34 +++-
>  Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf |  5 +++
>  Platform/ARM/SgiPkg/SgiPlatform.dsc  | 18 ++-
>  Platform/ARM/SgiPkg/SgiPlatform.fdf  |  7 +++-
>  4 files changed, 61 insertions(+), 3 deletions(-)
>
> diff --git a/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc 
> b/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc
> index 49fc919..b6aa90b 100644
> --- a/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc
> +++ b/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc
> @@ -26,6 +26,7 @@
>SKUID_IDENTIFIER   = DEFAULT
>FLASH_DEFINITION   = 
> Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf
>DEFINE DEBUG_MESSAGE   = TRUE
> +  DEFINE SECURE_BOOT_ENABLE  = FALSE
>

Maybe I wasn't clear before, but I don't see the point of building the
MM component without secure boot enabled. So can we drop this from
this side?

For the non-secure side, it is a different matter, obviously.

># LzmaF86
>DEFINE COMPRESSION_TOOL_GUID   = D42AE6BD-1352-4bfb-909A-CA72A6EAE889
> @@ -83,7 +84,17 @@
>HobLib|StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf
>
> MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf
>
> MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf
> -
> +!if $(SECURE_BOOT_ENABLE) == TRUE
> +  AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> +  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
> +  IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> +  
> NorFlashPlatformLib|Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.inf
> +  OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
> +  
> PlatformSecureLib|SecurityPkg/Library/PlatformSecureLibNull/PlatformSecureLibNull.inf
> +  
> SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> +  
> TimerLib|MdePkg/Library/BaseTimerLibNullTemplate/BaseTimerLibNullTemplate.inf
> +  VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
> +!endif
>  
> 
>  #
>  # Pcd Section - list of all EDK II PCD Entries defined by this Platform
> @@ -100,6 +111,21 @@
>
>gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x2
>
> +!if $(SECURE_BOOT_ENABLE) == TRUE
> +  #Secure Storage
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
> +  gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|TRUE
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
> +
> +  ## NV Storage - 1MB*3 in NOR2 Flash
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x1040
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x0010
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x1050
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x0010
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x1060
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x0010
> +!endif
> +
>  
> ###
>  #
>  # Components Section - list of the modules and components that will be 
> processed by compilation
> @@ -125,6 +151,12 @@
>StandaloneMmPkg/Core/StandaloneMmCore.inf
>
>  [Components.AARCH64]
> +!if $(SECURE_BOOT_ENABLE) == TRUE
> +  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
> +  
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
> +  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> +!endif
> +
>StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
>
>  
> ###
> diff --git a/Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf 
> b/Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf
> index 810460c..8c05a03 100644
> --- a/Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf
> +++ b/Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf
> @@ -55,6 +55,11 @@ READ_LOCK_CAP  = TRUE
>  READ_LOCK_STATUS   = TRUE
>
>INF StandaloneMmPkg/Core/StandaloneMmCore.inf
> +!if $(SECURE_BOOT_ENABLE) == TRUE
> +  INF ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
> +  INF 
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
> +  INF 

Re: [edk2] [PATCH edk2-platforms 0/2] Platforms/ARM/SgiPkg: apply product names for sgiclarka and sgiclarkh platforms

2019-03-15 Thread Ard Biesheuvel
On Fri, 15 Mar 2019 at 12:40, Jagadeesh Ujja  wrote:
>
> On Fri, Mar 15, 2019 at 4:55 PM Ard Biesheuvel
>  wrote:
> >
> > On Fri, 15 Mar 2019 at 12:17, Jagadeesh Ujja  wrote:
> > >
> > > hi Ard,
> > >
> > > On Fri, Mar 15, 2019 at 4:14 PM Ard Biesheuvel
> > >  wrote:
> > > >
> > > > On Fri, 15 Mar 2019 at 09:21, Jagadeesh Ujja  
> > > > wrote:
> > > > >
> > > > > hi Ard/Leif
> > > > >
> > > > > Please let me know if you have any comments on this patch set
> > > > >
> > > >
> > > > HI Jagadeesh,
> > > >
> > > > What does RdE1Edge or RdN1Edge mean?
> > >
> > > RdE1Edge/RdN1Edge are new product name
> > > Rd stands for reference design.
> > >
> > > The Neoverse E1 Edge Reference Design
> > > The Neoverse N1 Edge Reference Design
> > >
> >
> > So 'reference design' is the name of the platform based on the
> > Neoverse E1 and N1, respectively?
>
> yes ,
> 'reference design' is a class of platform, in which we have Neoverse
> E1/N1 cores support. Going forward other cores can come in
>

Could we drop the 'edge'? It seems redundant.

I'd prefer it if we could stick with

NeoverseE1
NeoverseN1

or if the reference design part is relevant

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


Re: [edk2] [PATCH edk2-platforms 0/2] Platforms/ARM/SgiPkg: apply product names for sgiclarka and sgiclarkh platforms

2019-03-15 Thread Jagadeesh Ujja
On Fri, Mar 15, 2019 at 4:55 PM Ard Biesheuvel
 wrote:
>
> On Fri, 15 Mar 2019 at 12:17, Jagadeesh Ujja  wrote:
> >
> > hi Ard,
> >
> > On Fri, Mar 15, 2019 at 4:14 PM Ard Biesheuvel
> >  wrote:
> > >
> > > On Fri, 15 Mar 2019 at 09:21, Jagadeesh Ujja  
> > > wrote:
> > > >
> > > > hi Ard/Leif
> > > >
> > > > Please let me know if you have any comments on this patch set
> > > >
> > >
> > > HI Jagadeesh,
> > >
> > > What does RdE1Edge or RdN1Edge mean?
> >
> > RdE1Edge/RdN1Edge are new product name
> > Rd stands for reference design.
> >
> > The Neoverse E1 Edge Reference Design
> > The Neoverse N1 Edge Reference Design
> >
>
> So 'reference design' is the name of the platform based on the
> Neoverse E1 and N1, respectively?

yes ,
'reference design' is a class of platform, in which we have Neoverse
E1/N1 cores support. Going forward other cores can come in

> ___
> 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 edk2-platforms v2 0/3] Platform/ARM/SgiPkg: Implement StandaloneMm based secure boot

2019-03-15 Thread Ard Biesheuvel
On Fri, 15 Mar 2019 at 09:19, Jagadeesh Ujja  wrote:
>
> hi Ard/Leif
>
> Please let me know if you have any comments on this patch set
>

I'll have a look, but we need the updated NorFlashDxe in
ArmPlatformPkg before we can merge this anyway.

>
> On Tue, Mar
> On Tue, Mar 12, 2019 at 9:45 PM Jagadeesh Ujja  wrote:
> >
> > Changes since v1:
> > - Addressed all the comments from Ard Biesheuvel.
> >
> > Integrating various pieces together so that the authenticated variable store
> > runs entirely in standalone MM context residing in a secure partition.
> > This primarily involves adding all required library and drivers to platform
> > specific .DSC and .FDF files. This creates separate Nor flash region which
> > is visible to only StandaoneMm drivers, this Nor Flash will co-exist along
> > with general Nor flash region.
> >
> > Jagadeesh Ujja (3):
> >   Platform/ARM/Sgi: define nor2 flash controller memory map
> >   Platform/ARM/Sgi: allow MM_STANDALONE modules to use
> > NorFlashPlatformLib
> >   Platform/ARM/SgiPkg: add MM based UEFI secure boot support
> >
> >  Platform/ARM/SgiPkg/Include/SgiPlatform.h   |  4 ++
> >  Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.c   | 63 
> > 
> >  Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.inf | 33 
> > ++
> >  Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc| 34 
> > ++-
> >  Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf|  5 ++
> >  Platform/ARM/SgiPkg/SgiPlatform.dsc | 18 
> > +-
> >  Platform/ARM/SgiPkg/SgiPlatform.fdf |  7 
> > ++-
> >  7 files changed, 161 insertions(+), 3 deletions(-)
> >  create mode 100644 
> > Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.c
> >  create mode 100644 
> > Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.inf
> >
> > --
> > 2.7.4
> >
> > In-Reply-To:
> >
> > ___
> > 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 1/1] SD : Continue setting up sd even if SD_HIGH_SPEED is not supported

2019-03-15 Thread Ard Biesheuvel
On Fri, 22 Feb 2019 at 08:58,  wrote:
>
> From: "Loh, Tien Hock" 
>
> If SD doesn't support SD_HIGH_SPEED, function should still continue to
> setup SD to go into 4 bits more if it is supported.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Loh Tien Hock 

Thanks Tien Hock

Reviewed-by: Ard Biesheuvel 

I tweaked your patch slightly, to fix some whitespace errors, and
downgrade the severity of the 'high speed not supported' message to
DEBUG_INFO

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


Re: [edk2] [PATCH edk2-platforms 0/2] Platforms/ARM/SgiPkg: apply product names for sgiclarka and sgiclarkh platforms

2019-03-15 Thread Jagadeesh Ujja
hi Ard,

On Fri, Mar 15, 2019 at 4:14 PM Ard Biesheuvel
 wrote:
>
> On Fri, 15 Mar 2019 at 09:21, Jagadeesh Ujja  wrote:
> >
> > hi Ard/Leif
> >
> > Please let me know if you have any comments on this patch set
> >
>
> HI Jagadeesh,
>
> What does RdE1Edge or RdN1Edge mean?

RdE1Edge/RdN1Edge are new product name
Rd stands for reference design.

The Neoverse E1 Edge Reference Design
The Neoverse N1 Edge Reference Design

More details can be found in the below links

https://pcfiend.com/2019/02/20/arm-announces-neoverse-n1-e1-platforms-cpus-enabling-a-huge-jump-in-infrastructure-performance/
https://community.arm.com/developer/ip-products/processors/b/processors-ip-blog/posts/arm-neoverse-n1-platform-accelerating-the-transformation-to-a-scalable-cloud-to-edge-infrastructure
https://community.arm.com/developer/ip-products/processors/b/processors-ip-blog/posts/arm-neoverse-e1-platform-empowering-the-infrastructure-to-meet-next-generation-throughput-demands

thanks
Jagadeesh
> ___
> 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 edk2-platforms 0/2] Platforms/ARM/SgiPkg: apply product names for sgiclarka and sgiclarkh platforms

2019-03-15 Thread Ard Biesheuvel
On Fri, 15 Mar 2019 at 12:17, Jagadeesh Ujja  wrote:
>
> hi Ard,
>
> On Fri, Mar 15, 2019 at 4:14 PM Ard Biesheuvel
>  wrote:
> >
> > On Fri, 15 Mar 2019 at 09:21, Jagadeesh Ujja  wrote:
> > >
> > > hi Ard/Leif
> > >
> > > Please let me know if you have any comments on this patch set
> > >
> >
> > HI Jagadeesh,
> >
> > What does RdE1Edge or RdN1Edge mean?
>
> RdE1Edge/RdN1Edge are new product name
> Rd stands for reference design.
>
> The Neoverse E1 Edge Reference Design
> The Neoverse N1 Edge Reference Design
>

So 'reference design' is the name of the platform based on the
Neoverse E1 and N1, respectively?
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH v1 0/2] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg

2019-03-15 Thread Ard Biesheuvel
On Fri, 15 Mar 2019 at 08:16, Hao Wu  wrote:
>
> This series will update the OVMF to stop using the ISA drivers within
> IntelFrameworkModulePkg.
>
> As the replacement, a new OVMF Super I/O bus driver has been add which
> will install the Super I/O protocol for ISA serial and PS2 keyboard
> devices. By doing so, these devices can be managed by:
>
>   MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
>   MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf
>
> respectively.
>

Just a couple of notes from my side - I'm sure Laszlo will have a much
longer list :-)

- Dropping the floppy driver is fine with me.
- What is OVMF specific about this driver? Is it only the hardcoded
list of COM1/COM2/PS2 keyboard? If so, should we split this into a
driver and a library class, where the driver lives in MdeModulePkg,
and the library is implemented in the context of OVMF?
- Regardless of the previous, adding the new driver should be a patch
of its own.
- I spotted an incorrect reference to PCI_IO_DEV in a comment (patch #2)



>
> Tests done:
> A. GCC5 & VS2015x86 tool chains build pass
> B. Launch QEMU (2.4.50, Windows) with command:
>> qemu-system-x86_64.exe -pflash \OVMF.fd -serial file:1.txt 
> -serial file:2.txt
>
>Able to see the ISA COM1/COM2 UART and PS2Keyboard devices under Shell
>using command 'devtree';
>
>Both the serials and PS2 keyboard are working fine;
>
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
> Cc: Ard Biesheuvel 
> Cc: Ray Ni 
>
> Hao Wu (2):
>   OvmfPkg: Drop the ISA Floppy device support
>   OvmfPkg: Add an Super IO bus driver to replace the current ISA support
>
>  OvmfPkg/OvmfPkgIa32.dsc   |  10 +-
>  OvmfPkg/OvmfPkgIa32X64.dsc|  10 +-
>  OvmfPkg/OvmfPkgX64.dsc|  10 +-
>  OvmfPkg/OvmfPkgIa32.fdf   |  10 +-
>  OvmfPkg/OvmfPkgIa32X64.fdf|  10 +-
>  OvmfPkg/OvmfPkgX64.fdf|  10 +-
>  OvmfPkg/SioBusDxe/SioBusDxe.inf   |  54 ++
>  OvmfPkg/SioBusDxe/SioBusDxe.h | 332 +++
>  OvmfPkg/SioBusDxe/SioService.h| 221 +++
>  OvmfPkg/SioBusDxe/ComponentName.c | 167 ++
>  OvmfPkg/SioBusDxe/SioBusDxe.c | 622 
>  OvmfPkg/SioBusDxe/SioService.c| 405 +
>  OvmfPkg/SioBusDxe/SioBusDxe.uni   |  21 +
>  13 files changed, 1846 insertions(+), 36 deletions(-)
>  create mode 100644 OvmfPkg/SioBusDxe/SioBusDxe.inf
>  create mode 100644 OvmfPkg/SioBusDxe/SioBusDxe.h
>  create mode 100644 OvmfPkg/SioBusDxe/SioService.h
>  create mode 100644 OvmfPkg/SioBusDxe/ComponentName.c
>  create mode 100644 OvmfPkg/SioBusDxe/SioBusDxe.c
>  create mode 100644 OvmfPkg/SioBusDxe/SioService.c
>  create mode 100644 OvmfPkg/SioBusDxe/SioBusDxe.uni
>
> --
> 2.12.0.windows.1
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH edk2-platforms 0/2] Platforms/ARM/SgiPkg: apply product names for sgiclarka and sgiclarkh platforms

2019-03-15 Thread Ard Biesheuvel
On Fri, 15 Mar 2019 at 09:21, Jagadeesh Ujja  wrote:
>
> hi Ard/Leif
>
> Please let me know if you have any comments on this patch set
>

HI Jagadeesh,

What does RdE1Edge or RdN1Edge mean?
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


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

2019-03-15 Thread Julien Grall

Hi Lars,

On 14/03/2019 20:04, Lars Kurth wrote:



On 14/03/2019, 12:07, "Julien Grall"  wrote:

 (+ Lars)
 
 On 3/14/19 10:55 AM, Laszlo Ersek wrote:

 > On 03/13/19 18:54, Kinney, Michael D wrote:
 > (2.2.2) Files that seem to be covered by the MIT license.
 >
 >OvmfPkg/Include/IndustryStandard/Xen/arch-arm/xen.h
...
 >OvmfPkg/XenBusDxe/XenStore.h
 >
 >  It's OK to leave these untouched, for now. Later, we should
 >  probably replace their license blocks with
 >  "SPDX-License-Identifier: MIT" (as appropriate). It might make
 >  sense to file a TianoCore BZ about them immediately, with a
 >  BZ-dependency on BZ#1373.
 
 I have added Lars Kurth to confirm the license.


Can you give me some context? It's not clear to me what you expect me to do.


EDK2 is converting the full copyright in each file to SDPX identifier. While the 
copyright looks like an MIT license, it has never been confirmed. Andrew Cooper 
suggested you might be able to confirm.


Cheers,

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


[edk2] [patch] BaseTools: Add missing license and copyright info

2019-03-15 Thread Dandan Bi
Cc: Bob Feng 
Cc: Liming Gao 
Cc: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 .../Conf/Empty_Package_Information_Data_File.ini | 12 
 1 file changed, 12 insertions(+)

diff --git a/BaseTools/Conf/Empty_Package_Information_Data_File.ini 
b/BaseTools/Conf/Empty_Package_Information_Data_File.ini
index fe162568c4..230d858ef9 100644
--- a/BaseTools/Conf/Empty_Package_Information_Data_File.ini
+++ b/BaseTools/Conf/Empty_Package_Information_Data_File.ini
@@ -1,5 +1,17 @@
+;@file
+; Example ini file used for UPT.
+;
+; Copyright (c) 2011 - 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
+; 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.
+;
 [DistributionHeader]
 Name = 
 GUID = 
 Version = 
 Vendor = 
-- 
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 edk2-platforms 0/2] Platforms/ARM/SgiPkg: apply product names for sgiclarka and sgiclarkh platforms

2019-03-15 Thread Jagadeesh Ujja
hi Ard/Leif

Please let me know if you have any comments on this patch set

thanks
Jagadeesh

On Tue, Mar
On Wed, Mar 13, 2019 at 2:18 PM Jagadeesh Ujja  wrote:
>
> hi Ard/Leif
>
> Please let me know if you have any comments on this patch
>
> thanks
> Jagadeesh
> On Tue, Mar 5, 2019 at 12:20 PM Jagadeesh Ujja  wrote:
> >
> > This patchset updates the product names for SGI-Clark.Ares and
> > SGI-Clark.Helios platforms.
> > The first patch replaces all uses of sgiclarka with rdn1edge.
> > The second patch replaces all use of sgiclarkh with rde1edge.
> >
> > Jagadeesh Ujja (2):
> >   Platforms/ARM/SgiPkg: Rename SgiClarkAres to RdN1Edge
> >   Platforms/ARM/SgiPkg: Rename SgiClarkHelios to RdE1Edge
> >
> >  Platform/ARM/SgiPkg/AcpiTables/{SgiClarkHelios => RdE1Edge}/Dsdt.asl   
> >  | 66 ++--
> >  Platform/ARM/SgiPkg/AcpiTables/{SgiClarkHelios => RdE1Edge}/Madt.aslc  
> >  |  0
> >  Platform/ARM/SgiPkg/AcpiTables/{SgiClarkHeliosAcpiTables.inf => 
> > RdE1EdgeAcpiTables.inf} |  6 +-
> >  Platform/ARM/SgiPkg/AcpiTables/{SgiClarkAres => RdN1Edge}/Dsdt.asl 
> >  | 16 ++---
> >  Platform/ARM/SgiPkg/AcpiTables/{SgiClarkAres => RdN1Edge}/Madt.aslc
> >  | 16 ++---
> >  Platform/ARM/SgiPkg/AcpiTables/{SgiClarkAresAcpiTables.inf => 
> > RdN1EdgeAcpiTables.inf}   |  6 +-
> >  Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c  
> >  | 12 ++--
> >  Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
> >  |  4 +-
> >  Platform/ARM/SgiPkg/Include/SgiPlatform.h  
> >  |  8 +--
> >  Platform/ARM/SgiPkg/SgiPlatform.dec
> >  |  4 +-
> >  Platform/ARM/SgiPkg/SgiPlatform.dsc
> >  |  4 +-
> >  Platform/ARM/SgiPkg/SgiPlatform.fdf
> >  |  4 +-
> >  12 files changed, 73 insertions(+), 73 deletions(-)
> >  rename Platform/ARM/SgiPkg/AcpiTables/{SgiClarkHelios => 
> > RdE1Edge}/Dsdt.asl (68%)
> >  rename Platform/ARM/SgiPkg/AcpiTables/{SgiClarkHelios => 
> > RdE1Edge}/Madt.aslc (100%)
> >  rename Platform/ARM/SgiPkg/AcpiTables/{SgiClarkHeliosAcpiTables.inf => 
> > RdE1EdgeAcpiTables.inf} (91%)
> >  rename Platform/ARM/SgiPkg/AcpiTables/{SgiClarkAres => RdN1Edge}/Dsdt.asl 
> > (85%)
> >  rename Platform/ARM/SgiPkg/AcpiTables/{SgiClarkAres => RdN1Edge}/Madt.aslc 
> > (93%)
> >  rename Platform/ARM/SgiPkg/AcpiTables/{SgiClarkAresAcpiTables.inf => 
> > RdN1EdgeAcpiTables.inf} (92%)
> >
> > --
> > 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] BaseTools: Add missing license and copyright info

2019-03-15 Thread Feng, Bob C
Reviewed-by: Bob Feng 

-Original Message-
From: Bi, Dandan 
Sent: Friday, March 15, 2019 4:23 PM
To: edk2-devel@lists.01.org
Cc: Feng, Bob C ; Gao, Liming ; 
Kinney, Michael D 
Subject: [patch] BaseTools: Add missing license and copyright info

Cc: Bob Feng 
Cc: Liming Gao 
Cc: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 .../Conf/Empty_Package_Information_Data_File.ini | 12 
 1 file changed, 12 insertions(+)

diff --git a/BaseTools/Conf/Empty_Package_Information_Data_File.ini 
b/BaseTools/Conf/Empty_Package_Information_Data_File.ini
index fe162568c4..230d858ef9 100644
--- a/BaseTools/Conf/Empty_Package_Information_Data_File.ini
+++ b/BaseTools/Conf/Empty_Package_Information_Data_File.ini
@@ -1,5 +1,17 @@
+;@file
+; Example ini file used for UPT.
+;
+; Copyright (c) 2011 - 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 ; 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.
+;
 [DistributionHeader]
 Name =
 GUID =
 Version =
 Vendor =
--
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 edk2-platforms v2 0/3] Platform/ARM/SgiPkg: Implement StandaloneMm based secure boot

2019-03-15 Thread Jagadeesh Ujja
hi Ard/Leif

Please let me know if you have any comments on this patch set

thanks
Jagadeesh

On Tue, Mar
On Tue, Mar 12, 2019 at 9:45 PM Jagadeesh Ujja  wrote:
>
> Changes since v1:
> - Addressed all the comments from Ard Biesheuvel.
>
> Integrating various pieces together so that the authenticated variable store
> runs entirely in standalone MM context residing in a secure partition.
> This primarily involves adding all required library and drivers to platform
> specific .DSC and .FDF files. This creates separate Nor flash region which
> is visible to only StandaoneMm drivers, this Nor Flash will co-exist along
> with general Nor flash region.
>
> Jagadeesh Ujja (3):
>   Platform/ARM/Sgi: define nor2 flash controller memory map
>   Platform/ARM/Sgi: allow MM_STANDALONE modules to use
> NorFlashPlatformLib
>   Platform/ARM/SgiPkg: add MM based UEFI secure boot support
>
>  Platform/ARM/SgiPkg/Include/SgiPlatform.h   |  4 ++
>  Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.c   | 63 
> 
>  Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.inf | 33 
> ++
>  Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc| 34 
> ++-
>  Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf|  5 ++
>  Platform/ARM/SgiPkg/SgiPlatform.dsc | 18 
> +-
>  Platform/ARM/SgiPkg/SgiPlatform.fdf |  7 ++-
>  7 files changed, 161 insertions(+), 3 deletions(-)
>  create mode 100644 
> Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.c
>  create mode 100644 
> Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.inf
>
> --
> 2.7.4
>
> In-Reply-To:
>
> ___
> 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 0/4] MdeModulePkg, StandaloneMmPkg: work around VA vs PA ambiguity

2019-03-15 Thread Ard Biesheuvel
On Fri, 15 Mar 2019 at 03:17, Wu, Hao A  wrote:
>
> > -Original Message-
> > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > Sent: Monday, March 11, 2019 11:36 PM
> > To: edk2-devel@lists.01.org
> > Cc: Ard Biesheuvel; Wang, Jian J; Wu, Hao A; Zeng, Star; Kinney, Michael D;
> > Gao, Liming; Achin Gupta; Yao, Jiewen; Supreeth Venkatesh; Jagadeesh Ujja
> > Subject: [PATCH 0/4] MdeModulePkg, StandaloneMmPkg: work around VA
> > vs PA ambiguity
> >
> > This series proposes one possible approach to work around the issue that the
> > traditional MM and standalone MM implement versions of the communicate
> > protocol
> > that are fundamentally incompatible from the point of view of the caller.
> >
> > In traditional MM, the MM communicate protocol takes a physical pointer for
> > the buffer, so that the SMM execution context can access the memory
> > directly
> > without having to translate it according to the translation regime of the
> > caller.
> >
> > In standalone MM, the buffer that is shared with the MM context is
> > preallocated,
> > and so it is up to the implementation of the MM communicate protocol to
> > copy the
> > data from the caller allocated buffer into the preallocated shared buffer. 
> > In
> > order to be able to do so, the DXE driver needs to copy the contents, and 
> > for
> > this it needs to know the virtual address not the physical address.
> >
> > So this means we have two incompatible versions of the same protocol, and
> > given
> > that we have even re-used the EFI_SMM_COMMUNICATE_PROTOCOL GUID
> > for the new
> > EFI_MM_COMMUNICATE_PROTOCOL, we cannot distinguish
> > programmatically between a
> > MM context that takes physical addresses vs one that takes virtual ones.
> >
> > Since this is known at build time, one way to deal with this is to have two
> > different implementations of a library that defines an abstract
> > MmCommunicate()
> > function, allowing the correct implementation to be selected at integration
> > time.
>
> Hello Ard,
>
> It seems to me that for platforms that include the VariableSmmRuntimeDxe
> driver, they need to add the 'MmCommunicateLib' dependency.
>
> Please grant us some time to evaluate this proposal and its impact. We
> will inform you as soon as there is a result. Thanks.
>

Thank you Hao.

Note that we intend to discuss the issue addressed by this series in
the PIWG call, but the next one is at least two weeks away.

So there is no urgency to reviewing this patch for inclusion, but any
feedback you can give is appreciated.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH V2 11/17] IntelFspPkg/BaseFspDebugLibSerialPort: Add a new api DebugVPrint

2019-03-15 Thread Chiu, Chasel


Reviewed-by: Chasel Chiu 

> -Original Message-
> From: Gao, Zhichao
> Sent: Friday, March 15, 2019 1:18 PM
> To: edk2-devel@lists.01.org
> Cc: Chiu, Chasel ; Desimone, Nathaniel L
> ; Zeng, Star ; Gao,
> Liming ; Sean Brogan ;
> Michael Turner ; Bret Barkelew
> 
> Subject: [PATCH V2 11/17] IntelFspPkg/BaseFspDebugLibSerialPort: Add a new
> api DebugVPrint
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395
> 
> Add a new api DebugVPrint implementation in the DebugLib instance. This api
> would expose a print routine with VaList parameter.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Zhichao Gao 
> Cc: Chasel Chiu 
> Cc: Nate DeSimone 
> Cc: Star Zeng 
> Cc: Liming Gao 
> Cc: Sean Brogan 
> Cc: Michael Turner 
> Cc: Bret Barkelew 
> ---
>  .../Library/BaseFspDebugLibSerialPort/DebugLib.c   | 42
> +++---
>  1 file changed, 37 insertions(+), 5 deletions(-)
> 
> diff --git a/IntelFspPkg/Library/BaseFspDebugLibSerialPort/DebugLib.c
> b/IntelFspPkg/Library/BaseFspDebugLibSerialPort/DebugLib.c
> index 73bb08e357..57b6020a58 100644
> --- a/IntelFspPkg/Library/BaseFspDebugLibSerialPort/DebugLib.c
> +++ b/IntelFspPkg/Library/BaseFspDebugLibSerialPort/DebugLib.c
> @@ -1,6 +1,6 @@
>  /** @file
> 
> -  Copyright (c) 2014 - 2015, Intel Corporation. All rights reserved.
> +  Copyright (c) 2014 - 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
>which accompanies this distribution.  The full text of the license may be 
> found
> at @@ -62,9 +62,38 @@ DebugPrint (
>IN  CONST CHAR8  *Format,
>...
>)
> +{
> +  VA_LIST Marker;
> +
> +  VA_START (Marker, Format);
> +  DebugVPrint (ErrorLevel, Format, Marker);
> +  VA_END (Marker);
> +}
> +
> +
> +/**
> +  Prints a debug message to the debug output device if the specified error 
> level
> is enabled.
> +
> +  If any bit in ErrorLevel is also set in DebugPrintErrorLevelLib
> + function  GetDebugPrintErrorLevel (), then print the message specified
> + by Format and the  associated variable argument list to the debug output
> device.
> +
> +  If Format is NULL, then ASSERT().
> +
> +  @param  ErrorLevelThe error level of the debug message.
> +  @param  FormatFormat string for the debug message to print.
> +  @param  VaListMarker  VA_LIST marker for the variable argument list.
> +
> +**/
> +VOID
> +EFIAPI
> +DebugVPrint (
> +  IN  UINTNErrorLevel,
> +  IN  CONST CHAR8  *Format,
> +  IN  VA_LIST   VaListMarker
> +  )
>  {
>CHAR8Buffer[MAX_DEBUG_MESSAGE_LENGTH];
> -  VA_LIST  Marker;
> 
>//
>// If Format is NULL, then ASSERT().
> @@ -88,9 +117,7 @@ DebugPrint (
>//
>// Convert the DEBUG() message to an ASCII String
>//
> -  VA_START (Marker, Format);
> -  AsciiVSPrint (Buffer, sizeof (Buffer), Format, Marker);
> -  VA_END (Marker);
> +  AsciiVSPrint (Buffer, sizeof (Buffer), Format, VaListMarker);
> 
>//
>// Send the print string to a Serial Port @@ -98,6 +125,7 @@ DebugPrint (
>SerialPortWrite ((UINT8 *)Buffer, AsciiStrLen (Buffer));  }
> 
> +
>  /**
>Convert an UINT32 value into HEX string sepcified by Buffer.
> 
> @@ -118,6 +146,7 @@ FillHex (
>}
>  }
> 
> +
>  /**
>Prints an assert message containing a filename, line number, and 
> description.
>This may be followed by a breakpoint or a dead loop.
> @@ -172,6 +201,7 @@ DebugAssertInternal (
>CpuDeadLoop ();
>  }
> 
> +
>  /**
>Prints an assert message containing a filename, line number, and 
> description.
>This may be followed by a breakpoint or a dead loop.
> @@ -270,6 +300,7 @@ DebugPrintEnabled (
>return (BOOLEAN) ((PcdGet8(PcdDebugPropertyMask) &
> DEBUG_PROPERTY_DEBUG_PRINT_ENABLED) != 0);  }
> 
> +
>  /**
>Returns TRUE if DEBUG_CODE() macros are enabled.
> 
> @@ -309,6 +340,7 @@ DebugClearMemoryEnabled (
>return (BOOLEAN) ((PcdGet8(PcdDebugPropertyMask) &
> DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED) != 0);  }
> 
> +
>  /**
>Returns TRUE if any one of the bit is set both in ErrorLevel and
> PcdFixedDebugPrintErrorLevel.
> 
> --
> 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 V2 10/17] IntelFsp2Pkg/BaseFspDebugLibSerialPort: Add a new api DebugVPrint

2019-03-15 Thread Chiu, Chasel


Reviewed-by: Chasel Chiu 

> -Original Message-
> From: Gao, Zhichao
> Sent: Friday, March 15, 2019 1:18 PM
> To: edk2-devel@lists.01.org
> Cc: Chiu, Chasel ; Desimone, Nathaniel L
> ; Zeng, Star ; Gao,
> Liming ; Sean Brogan ;
> Michael Turner ; Bret Barkelew
> 
> Subject: [PATCH V2 10/17] IntelFsp2Pkg/BaseFspDebugLibSerialPort: Add a new
> api DebugVPrint
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395
> 
> Add a new api DebugVPrint implementation in the DebugLib instance. This api
> would expose a print routine with VaList parameter.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Zhichao Gao 
> Cc: Chasel Chiu 
> Cc: Nate DeSimone 
> Cc: Star Zeng 
> Cc: Liming Gao 
> Cc: Sean Brogan 
> Cc: Michael Turner 
> Cc: Bret Barkelew 
> ---
>  .../Library/BaseFspDebugLibSerialPort/DebugLib.c   | 42
> +++---
>  1 file changed, 37 insertions(+), 5 deletions(-)
> 
> diff --git a/IntelFsp2Pkg/Library/BaseFspDebugLibSerialPort/DebugLib.c
> b/IntelFsp2Pkg/Library/BaseFspDebugLibSerialPort/DebugLib.c
> index 73bb08e357..57b6020a58 100644
> --- a/IntelFsp2Pkg/Library/BaseFspDebugLibSerialPort/DebugLib.c
> +++ b/IntelFsp2Pkg/Library/BaseFspDebugLibSerialPort/DebugLib.c
> @@ -1,6 +1,6 @@
>  /** @file
> 
> -  Copyright (c) 2014 - 2015, Intel Corporation. All rights reserved.
> +  Copyright (c) 2014 - 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
>which accompanies this distribution.  The full text of the license may be 
> found
> at @@ -62,9 +62,38 @@ DebugPrint (
>IN  CONST CHAR8  *Format,
>...
>)
> +{
> +  VA_LIST Marker;
> +
> +  VA_START (Marker, Format);
> +  DebugVPrint (ErrorLevel, Format, Marker);
> +  VA_END (Marker);
> +}
> +
> +
> +/**
> +  Prints a debug message to the debug output device if the specified error 
> level
> is enabled.
> +
> +  If any bit in ErrorLevel is also set in DebugPrintErrorLevelLib
> + function  GetDebugPrintErrorLevel (), then print the message specified
> + by Format and the  associated variable argument list to the debug output
> device.
> +
> +  If Format is NULL, then ASSERT().
> +
> +  @param  ErrorLevelThe error level of the debug message.
> +  @param  FormatFormat string for the debug message to print.
> +  @param  VaListMarker  VA_LIST marker for the variable argument list.
> +
> +**/
> +VOID
> +EFIAPI
> +DebugVPrint (
> +  IN  UINTNErrorLevel,
> +  IN  CONST CHAR8  *Format,
> +  IN  VA_LIST   VaListMarker
> +  )
>  {
>CHAR8Buffer[MAX_DEBUG_MESSAGE_LENGTH];
> -  VA_LIST  Marker;
> 
>//
>// If Format is NULL, then ASSERT().
> @@ -88,9 +117,7 @@ DebugPrint (
>//
>// Convert the DEBUG() message to an ASCII String
>//
> -  VA_START (Marker, Format);
> -  AsciiVSPrint (Buffer, sizeof (Buffer), Format, Marker);
> -  VA_END (Marker);
> +  AsciiVSPrint (Buffer, sizeof (Buffer), Format, VaListMarker);
> 
>//
>// Send the print string to a Serial Port @@ -98,6 +125,7 @@ DebugPrint (
>SerialPortWrite ((UINT8 *)Buffer, AsciiStrLen (Buffer));  }
> 
> +
>  /**
>Convert an UINT32 value into HEX string sepcified by Buffer.
> 
> @@ -118,6 +146,7 @@ FillHex (
>}
>  }
> 
> +
>  /**
>Prints an assert message containing a filename, line number, and 
> description.
>This may be followed by a breakpoint or a dead loop.
> @@ -172,6 +201,7 @@ DebugAssertInternal (
>CpuDeadLoop ();
>  }
> 
> +
>  /**
>Prints an assert message containing a filename, line number, and 
> description.
>This may be followed by a breakpoint or a dead loop.
> @@ -270,6 +300,7 @@ DebugPrintEnabled (
>return (BOOLEAN) ((PcdGet8(PcdDebugPropertyMask) &
> DEBUG_PROPERTY_DEBUG_PRINT_ENABLED) != 0);  }
> 
> +
>  /**
>Returns TRUE if DEBUG_CODE() macros are enabled.
> 
> @@ -309,6 +340,7 @@ DebugClearMemoryEnabled (
>return (BOOLEAN) ((PcdGet8(PcdDebugPropertyMask) &
> DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED) != 0);  }
> 
> +
>  /**
>Returns TRUE if any one of the bit is set both in ErrorLevel and
> PcdFixedDebugPrintErrorLevel.
> 
> --
> 2.16.2.windows.1

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


[edk2] [PATCH v1 0/2] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg

2019-03-15 Thread Hao Wu
This series will update the OVMF to stop using the ISA drivers within
IntelFrameworkModulePkg.

As the replacement, a new OVMF Super I/O bus driver has been add which
will install the Super I/O protocol for ISA serial and PS2 keyboard
devices. By doing so, these devices can be managed by:

  MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
  MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf

respectively.


Tests done:
A. GCC5 & VS2015x86 tool chains build pass
B. Launch QEMU (2.4.50, Windows) with command:
   > qemu-system-x86_64.exe -pflash \OVMF.fd -serial file:1.txt 
-serial file:2.txt

   Able to see the ISA COM1/COM2 UART and PS2Keyboard devices under Shell
   using command 'devtree';

   Both the serials and PS2 keyboard are working fine;

Cc: Jordan Justen 
Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Ray Ni 

Hao Wu (2):
  OvmfPkg: Drop the ISA Floppy device support
  OvmfPkg: Add an Super IO bus driver to replace the current ISA support

 OvmfPkg/OvmfPkgIa32.dsc   |  10 +-
 OvmfPkg/OvmfPkgIa32X64.dsc|  10 +-
 OvmfPkg/OvmfPkgX64.dsc|  10 +-
 OvmfPkg/OvmfPkgIa32.fdf   |  10 +-
 OvmfPkg/OvmfPkgIa32X64.fdf|  10 +-
 OvmfPkg/OvmfPkgX64.fdf|  10 +-
 OvmfPkg/SioBusDxe/SioBusDxe.inf   |  54 ++
 OvmfPkg/SioBusDxe/SioBusDxe.h | 332 +++
 OvmfPkg/SioBusDxe/SioService.h| 221 +++
 OvmfPkg/SioBusDxe/ComponentName.c | 167 ++
 OvmfPkg/SioBusDxe/SioBusDxe.c | 622 
 OvmfPkg/SioBusDxe/SioService.c| 405 +
 OvmfPkg/SioBusDxe/SioBusDxe.uni   |  21 +
 13 files changed, 1846 insertions(+), 36 deletions(-)
 create mode 100644 OvmfPkg/SioBusDxe/SioBusDxe.inf
 create mode 100644 OvmfPkg/SioBusDxe/SioBusDxe.h
 create mode 100644 OvmfPkg/SioBusDxe/SioService.h
 create mode 100644 OvmfPkg/SioBusDxe/ComponentName.c
 create mode 100644 OvmfPkg/SioBusDxe/SioBusDxe.c
 create mode 100644 OvmfPkg/SioBusDxe/SioService.c
 create mode 100644 OvmfPkg/SioBusDxe/SioBusDxe.uni

-- 
2.12.0.windows.1

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


[edk2] [PATCH v1 1/2] OvmfPkg: Drop the ISA Floppy device support

2019-03-15 Thread Hao Wu
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1495

There is a plan to remove the IntelFrameworkModulePkg:
https://bugzilla.tianocore.org/show_bug.cgi?id=1605

And for driver:
IntelFrameworkModulePkg/Bus/Isa/IsaFloppyDxe

This patch proposes to drop the ISA Floppy device support in OVMF.

Cc: Jordan Justen 
Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Ray Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 OvmfPkg/OvmfPkgIa32.dsc| 3 +--
 OvmfPkg/OvmfPkgIa32X64.dsc | 3 +--
 OvmfPkg/OvmfPkgX64.dsc | 3 +--
 OvmfPkg/OvmfPkgIa32.fdf| 3 +--
 OvmfPkg/OvmfPkgIa32X64.fdf | 3 +--
 OvmfPkg/OvmfPkgX64.fdf | 3 +--
 6 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 5b885590b2..1710ab5a88 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -1,7 +1,7 @@
 ## @file
 #  EFI/Framework Open Virtual Machine Firmware (OVMF) platform
 #
-#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+#  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
 #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP
 #
 #  This program and the accompanying materials
@@ -756,7 +756,6 @@
   IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
   IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
   IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
-  IntelFrameworkModulePkg/Bus/Isa/IsaFloppyDxe/IsaFloppyDxe.inf
 
   #
   # SMBIOS Support
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index bbf0853ee6..5bceef3116 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -1,7 +1,7 @@
 ## @file
 #  EFI/Framework Open Virtual Machine Firmware (OVMF) platform
 #
-#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+#  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
 #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP
 #
 #  This program and the accompanying materials
@@ -765,7 +765,6 @@
   IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
   IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
   IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
-  IntelFrameworkModulePkg/Bus/Isa/IsaFloppyDxe/IsaFloppyDxe.inf
 
   #
   # SMBIOS Support
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index d81460f520..3f5d948dbb 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -1,7 +1,7 @@
 ## @file
 #  EFI/Framework Open Virtual Machine Firmware (OVMF) platform
 #
-#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+#  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
 #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP
 #
 #  This program and the accompanying materials
@@ -763,7 +763,6 @@
   IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
   IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
   IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
-  IntelFrameworkModulePkg/Bus/Isa/IsaFloppyDxe/IsaFloppyDxe.inf
 
   #
   # SMBIOS Support
diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
index 4999403ad7..54d7f06a70 100644
--- a/OvmfPkg/OvmfPkgIa32.fdf
+++ b/OvmfPkg/OvmfPkgIa32.fdf
@@ -1,7 +1,7 @@
 ## @file
 #  Open Virtual Machine Firmware: FDF
 #
-#  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
+#  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
 #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP
 #
 #  This program and the accompanying materials
@@ -273,7 +273,6 @@ INF  
IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
 !endif
 
 INF  IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
-INF  IntelFrameworkModulePkg/Bus/Isa/IsaFloppyDxe/IsaFloppyDxe.inf
 
 INF  MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
 INF  OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
index d0cc107928..7519b53a9b 100644
--- a/OvmfPkg/OvmfPkgIa32X64.fdf
+++ b/OvmfPkg/OvmfPkgIa32X64.fdf
@@ -1,7 +1,7 @@
 ## @file
 #  Open Virtual Machine Firmware: FDF
 #
-#  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
+#  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
 #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP
 #
 #  This program and the accompanying materials
@@ -274,7 +274,6 @@ INF  
IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
 !endif
 
 INF  IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
-INF  IntelFrameworkModulePkg/Bus/Isa/IsaFloppyDxe/IsaFloppyDxe.inf
 
 INF  MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
 INF  OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index d0cc107928..7519b53a9b 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf

[edk2] [PATCH v1 2/2] OvmfPkg: Add an Super IO bus driver to replace the current ISA support

2019-03-15 Thread Hao Wu
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1495

There is a plan to remove the IntelFrameworkModulePkg:
https://bugzilla.tianocore.org/show_bug.cgi?id=1605

This patch will replace the current ISA support:

  PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
  IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
  IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
  IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf

with a new OVMF Super I/O bus driver and the serial & PS2 keyboard
drviers:

  MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
  MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf

The new OVMF Super IO bus driver will create the below child devices:

* COM 1 UART
* COM 2 UART
* PS/2 Keyboard

and installs the Super I/O Protocol on them.

Cc: Jordan Justen 
Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Ray Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 OvmfPkg/OvmfPkgIa32.dsc   |   7 +-
 OvmfPkg/OvmfPkgIa32X64.dsc|   7 +-
 OvmfPkg/OvmfPkgX64.dsc|   7 +-
 OvmfPkg/OvmfPkgIa32.fdf   |   7 +-
 OvmfPkg/OvmfPkgIa32X64.fdf|   7 +-
 OvmfPkg/OvmfPkgX64.fdf|   7 +-
 OvmfPkg/SioBusDxe/SioBusDxe.inf   |  54 ++
 OvmfPkg/SioBusDxe/SioBusDxe.h | 332 +++
 OvmfPkg/SioBusDxe/SioService.h| 221 +++
 OvmfPkg/SioBusDxe/ComponentName.c | 167 ++
 OvmfPkg/SioBusDxe/SioBusDxe.c | 622 
 OvmfPkg/SioBusDxe/SioService.c| 405 +
 OvmfPkg/SioBusDxe/SioBusDxe.uni   |  21 +
 13 files changed, 1840 insertions(+), 24 deletions(-)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 1710ab5a88..3caaeab2f3 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -752,10 +752,9 @@
   #
   # ISA Support
   #
-  PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
-  IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
-  IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
-  IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
+  OvmfPkg/SioBusDxe/SioBusDxe.inf
+  MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
+  MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf
 
   #
   # SMBIOS Support
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 5bceef3116..21bf4eeb5a 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -761,10 +761,9 @@
   #
   # ISA Support
   #
-  PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
-  IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
-  IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
-  IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
+  OvmfPkg/SioBusDxe/SioBusDxe.inf
+  MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
+  MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf
 
   #
   # SMBIOS Support
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 3f5d948dbb..6102e8370d 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -759,10 +759,9 @@
   #
   # ISA Support
   #
-  PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
-  IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
-  IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
-  IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
+  OvmfPkg/SioBusDxe/SioBusDxe.inf
+  MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
+  MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf
 
   #
   # SMBIOS Support
diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
index 54d7f06a70..05d9477fc8 100644
--- a/OvmfPkg/OvmfPkgIa32.fdf
+++ b/OvmfPkg/OvmfPkgIa32.fdf
@@ -265,14 +265,13 @@ INF  
MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
 INF  MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
 INF  MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
 
-INF  PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
-INF  IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
+INF  OvmfPkg/SioBusDxe/SioBusDxe.inf
 
 !ifndef $(SOURCE_DEBUG_ENABLE)
-INF  IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
+INF  MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
 !endif
 
-INF  IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
+INF  MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf
 
 INF  MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
 INF  OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
index 7519b53a9b..53b29ddd64 100644
--- a/OvmfPkg/OvmfPkgIa32X64.fdf
+++ b/OvmfPkg/OvmfPkgIa32X64.fdf
@@ -266,14 +266,13 @@ INF  
MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
 INF  MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
 INF  MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
 
-INF  PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
-INF  IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
+INF  

Re: [edk2] [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api DebugVPrint for DebugLib

2019-03-15 Thread Jordan Justen
On 2019-03-14 22:17:33, Zhichao Gao wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395
> 
> Add a new api DebugVPrint prototype definition in the
> DebugLib header file. This api would expose a print
> routine with VaList parameter.

These lines seem to be fairly short, with the longest be 54 chars. I
guess not a problem, but by the recommendation says they could be up
to 75 in length.

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

> 
> diff --git a/MdePkg/Include/Library/DebugLib.h 
> b/MdePkg/Include/Library/DebugLib.h
> index e6a7a357b2..51d89bbd52 100644
> --- a/MdePkg/Include/Library/DebugLib.h
> +++ b/MdePkg/Include/Library/DebugLib.h
> @@ -8,7 +8,7 @@
>of size reduction when compiler optimization is disabled. If MDEPKG_NDEBUG 
> is
>defined, then debug and assert related macros wrapped by it are the NULL 
> implementations.
>  
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
> +Copyright (c) 2006 - 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 that accompanies this 
> distribution.
>  The full text of the license may be found at
> @@ -101,6 +101,29 @@ DebugPrint (
>);
>  
> +/**
> +  Prints a debug message to the debug output device if the specified error 
> level is enabled.

According to the style guide:

  "Preferably, limit line lengths to 80 columns or less."

https://github.com/tianocore/tianocore.github.io/wiki/Code-Style-C

https://github.com/tianocore-docs/Docs/raw/master/Specifications/CCS_2_1_Draft.pdf

But, this line uses 92 columns.

I think there are similar cases in other patches.

> +
> +  If any bit in ErrorLevel is also set in DebugPrintErrorLevelLib function
> +  GetDebugPrintErrorLevel (), then print the message specified by Format and 
> the
> +  associated variable argument list to the debug output device.
> +
> +  If Format is NULL, then ASSERT().
> +
> +  @param  ErrorLevelThe error level of the debug message.
> +  @param  FormatFormat string for the debug message to print.
> +  @param  VaListMarker  VA_LIST marker for the variable argument list.
> +
> +**/
> +VOID
> +EFIAPI
> +DebugVPrint (
> +  IN  UINTNErrorLevel,
> +  IN  CONST CHAR8  *Format,
> +  IN  VA_LIST   VaListMarker
> +  );
> +
> +
>  /**
>Prints an assert message containing a filename, line number, and 
> description.
>This may be followed by a breakpoint or a dead loop.
> @@ -221,6 +244,7 @@ DebugClearMemoryEnabled (
>VOID
>);
>  
> +

What's going on here? It seems like maybe extra lines are added that
have nothing to do with this patch.

-Jordan

>  /**
>Returns TRUE if any one of the bit is set both in ErrorLevel and 
> PcdFixedDebugPrintErrorLevel.
>  
> @@ -236,6 +260,7 @@ DebugPrintLevelEnabled (
>IN  CONST UINTNErrorLevel
>);
>  
> +
>  /**
>Internal worker macro that calls DebugAssert().
>  
> @@ -273,6 +298,7 @@ DebugPrintLevelEnabled (
>  #define _DEBUG(Expression)   DebugPrint Expression
>  #endif
>  
> +
>  /**
>Macro that calls DebugAssert() if an expression evaluates to FALSE.
>  
> @@ -299,6 +325,7 @@ DebugPrintLevelEnabled (
>#define ASSERT(Expression)
>  #endif
>  
> +
>  /**
>Macro that calls DebugPrint().
>  
> @@ -322,6 +349,7 @@ DebugPrintLevelEnabled (
>#define DEBUG(Expression)
>  #endif
>  
> +
>  /**
>Macro that calls DebugAssert() if an EFI_STATUS evaluates to an error code.
>  
> @@ -348,6 +376,7 @@ DebugPrintLevelEnabled (
>#define ASSERT_EFI_ERROR(StatusParameter)
>  #endif
>  
> +
>  /**
>Macro that calls DebugAssert() if a RETURN_STATUS evaluates to an error 
> code.
>  
> @@ -375,6 +404,7 @@ DebugPrintLevelEnabled (
>#define ASSERT_RETURN_ERROR(StatusParameter)
>  #endif
>  
> +
>  /**
>Macro that calls DebugAssert() if a protocol is already installed in the
>handle database.
> @@ -418,6 +448,7 @@ DebugPrintLevelEnabled (
>#define ASSERT_PROTOCOL_ALREADY_INSTALLED(Handle, Guid)
>  #endif
>  
> +
>  /**
>Macro that marks the beginning of debug source code.
>  
> -- 
> 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