Re: [edk2-devel] [Patch] BaseTools/DscBuildData: Fix PCD autogen include file conflict

2020-02-02 Thread Liming Gao
Mike:
  Yes. Build tool needs to know host OS behavior and get the full include path. 
For this patch, it skips some include paths. If this include path also includes 
other required header file, it will cause the build break. Can we have the 
assumption that all sys header files and other non-sys header files are not in 
the same include directory?

Thanks
Liming
> -Original Message-
> From: Kinney, Michael D 
> Sent: Monday, February 3, 2020 2:40 PM
> To: Gao, Liming ; devel@edk2.groups.io; Kinney, Michael 
> D 
> Cc: Feng, Bob C 
> Subject: RE: [Patch] BaseTools/DscBuildData: Fix PCD autogen include file 
> conflict
> 
> Liming,
> 
> The build tools would need to know which env var to query for
> all OS/host tool chain combinations and how to parse that
> information for full paths in an OS specific manner. We should
> not build that type of information into the build tools.
> 
> The fix I have provided does not need this information.
> 
> Mike
> 
> > -Original Message-
> > From: Gao, Liming 
> > Sent: Sunday, February 2, 2020 4:59 PM
> > To: Kinney, Michael D ;
> > devel@edk2.groups.io
> > Cc: Feng, Bob C 
> > Subject: RE: [Patch] BaseTools/DscBuildData: Fix PCD
> > autogen include file conflict
> >
> > Mike:
> >   Can we consider to parse INCLUDE env value and add
> > those path to -I options as the first priority?
> >
> > Thanks
> > Liming
> > > -Original Message-
> > > From: Kinney, Michael D 
> > > Sent: Thursday, January 30, 2020 8:46 AM
> > > To: devel@edk2.groups.io
> > > Cc: Feng, Bob C ; Gao, Liming
> > 
> > > Subject: [Patch] BaseTools/DscBuildData: Fix PCD
> > autogen include file conflict
> > >
> > > https://bugzilla.tianocore.org/show_bug.cgi?id=2494
> > >
> > > When using structured PCDs, a C application is auto
> > generated
> > > to fill in the structured PCD value.  The C
> > application uses
> > > the standard include files , , and
> > .
> > > This C application also supports include paths from
> > package DEC
> > > files when a structured PCD declaration provides a
> > 
> > > list.  The complete list of include paths are -I
> > options for
> > > include paths from package DEC files and the
> > compiler's standard
> > > include paths.
> > >
> > > -I include paths are higher priority than the
> > standard include
> > > paths.  If the -I included paths from package DEC
> > files contain
> > > , , or  the wrong
> > include files are
> > > used to compile the C application for the structured
> > PCD value.
> > >
> > > Update GenerateByteArrayValue() to skip a package DEC
> > include
> > > paths that contain , , or
> > .
> > >
> > > Build failures were observed when adding a structured
> > PCD to
> > > CryptoPkg.  CryptoPkg contains , ,
> > and
> > >  in the path CryptoPkg/Library/Include to
> > support
> > > building Open SSL.  The Library/Include path is
> > listed as a
> > > private include path in CryptoPkg.dec.  Without this
> > change, the
> > > standard include files designed to support build
> > OpenSLL are
> > > used to build the structured PCD C application, and
> > that build
> > > fails.
> > >
> > > Other packages that provide a standard C lib or a
> > gasket for
> > > a subset of the standard C lib will run into this
> > same issue
> > > if they also define and use a Structured PCD.  So
> > this issue
> > > is not limited to the CryptoPkg.
> > >
> > > Cc: Bob Feng 
> > > Cc: Liming Gao 
> > > Signed-off-by: Michael D Kinney
> > 
> > > ---
> > >  .../Source/Python/Workspace/DscBuildData.py| 18
> > +-
> > >  1 file changed, 17 insertions(+), 1 deletion(-)
> > >
> > > diff --git
> > a/BaseTools/Source/Python/Workspace/DscBuildData.py
> > b/BaseTools/Source/Python/Workspace/DscBuildData.py
> > > index c65a0dd346..be6688dc75 100644
> > > ---
> > a/BaseTools/Source/Python/Workspace/DscBuildData.py
> > > +++
> > b/BaseTools/Source/Python/Workspace/DscBuildData.py
> > > @@ -1,7 +1,7 @@
> > >  ## @file
> > >  # This file is used to create a database used by
> > build tool
> > >  #
> > > -# Copyright (c) 2008 - 2019, Intel Corporation. All
> > rights reserved.
> > > +# Copyright (c) 2008 - 2020, Intel Corporation. All
> > rights reserved.
> > >  # (C) Copyright 2016 Hewlett Packard Enterprise
> > Development LP
> > >  # SPDX-License-Identifier: BSD-2-Clause-Patent
> > >  #
> > > @@ -2667,6 +2667,22 @@ class
> > DscBuildData(PlatformBuildClassObject):
> > >  for pkg in PcdDependDEC:
> > >  if pkg in PlatformInc:
> > >  for inc in PlatformInc[pkg]:
> > > +#
> > > +# Get list of files in
> > potential -I include path
> > > +#
> > > +FileList = os.listdir
> > (str(inc))
> > > +#
> > > +# Skip -I include path if
> > one of the include files required
> > > +# by PcdValueInit.c are
> > present in the include paths from
> > > +

Re: [edk2-devel] [Patch] BaseTools/DscBuildData: Fix PCD autogen include file conflict

2020-02-02 Thread Michael D Kinney
Liming,

The build tools would need to know which env var to query for 
all OS/host tool chain combinations and how to parse that
information for full paths in an OS specific manner. We should
not build that type of information into the build tools.

The fix I have provided does not need this information.

Mike

> -Original Message-
> From: Gao, Liming 
> Sent: Sunday, February 2, 2020 4:59 PM
> To: Kinney, Michael D ;
> devel@edk2.groups.io
> Cc: Feng, Bob C 
> Subject: RE: [Patch] BaseTools/DscBuildData: Fix PCD
> autogen include file conflict
> 
> Mike:
>   Can we consider to parse INCLUDE env value and add
> those path to -I options as the first priority?
> 
> Thanks
> Liming
> > -Original Message-
> > From: Kinney, Michael D 
> > Sent: Thursday, January 30, 2020 8:46 AM
> > To: devel@edk2.groups.io
> > Cc: Feng, Bob C ; Gao, Liming
> 
> > Subject: [Patch] BaseTools/DscBuildData: Fix PCD
> autogen include file conflict
> >
> > https://bugzilla.tianocore.org/show_bug.cgi?id=2494
> >
> > When using structured PCDs, a C application is auto
> generated
> > to fill in the structured PCD value.  The C
> application uses
> > the standard include files , , and
> .
> > This C application also supports include paths from
> package DEC
> > files when a structured PCD declaration provides a
> 
> > list.  The complete list of include paths are -I
> options for
> > include paths from package DEC files and the
> compiler's standard
> > include paths.
> >
> > -I include paths are higher priority than the
> standard include
> > paths.  If the -I included paths from package DEC
> files contain
> > , , or  the wrong
> include files are
> > used to compile the C application for the structured
> PCD value.
> >
> > Update GenerateByteArrayValue() to skip a package DEC
> include
> > paths that contain , , or
> .
> >
> > Build failures were observed when adding a structured
> PCD to
> > CryptoPkg.  CryptoPkg contains , ,
> and
> >  in the path CryptoPkg/Library/Include to
> support
> > building Open SSL.  The Library/Include path is
> listed as a
> > private include path in CryptoPkg.dec.  Without this
> change, the
> > standard include files designed to support build
> OpenSLL are
> > used to build the structured PCD C application, and
> that build
> > fails.
> >
> > Other packages that provide a standard C lib or a
> gasket for
> > a subset of the standard C lib will run into this
> same issue
> > if they also define and use a Structured PCD.  So
> this issue
> > is not limited to the CryptoPkg.
> >
> > Cc: Bob Feng 
> > Cc: Liming Gao 
> > Signed-off-by: Michael D Kinney
> 
> > ---
> >  .../Source/Python/Workspace/DscBuildData.py| 18
> +-
> >  1 file changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git
> a/BaseTools/Source/Python/Workspace/DscBuildData.py
> b/BaseTools/Source/Python/Workspace/DscBuildData.py
> > index c65a0dd346..be6688dc75 100644
> > ---
> a/BaseTools/Source/Python/Workspace/DscBuildData.py
> > +++
> b/BaseTools/Source/Python/Workspace/DscBuildData.py
> > @@ -1,7 +1,7 @@
> >  ## @file
> >  # This file is used to create a database used by
> build tool
> >  #
> > -# Copyright (c) 2008 - 2019, Intel Corporation. All
> rights reserved.
> > +# Copyright (c) 2008 - 2020, Intel Corporation. All
> rights reserved.
> >  # (C) Copyright 2016 Hewlett Packard Enterprise
> Development LP
> >  # SPDX-License-Identifier: BSD-2-Clause-Patent
> >  #
> > @@ -2667,6 +2667,22 @@ class
> DscBuildData(PlatformBuildClassObject):
> >  for pkg in PcdDependDEC:
> >  if pkg in PlatformInc:
> >  for inc in PlatformInc[pkg]:
> > +#
> > +# Get list of files in
> potential -I include path
> > +#
> > +FileList = os.listdir
> (str(inc))
> > +#
> > +# Skip -I include path if
> one of the include files required
> > +# by PcdValueInit.c are
> present in the include paths from
> > +# the DEC file.
> PcdValueInit.c must use the standard include
> > +# files from the host
> compiler.
> > +#
> > +if 'stdio.h' in FileList:
> > +  continue
> > +if 'stdlib.h' in FileList:
> > +  continue
> > +if 'string.h' in FileList:
> > +  continue
> >  MakeApp += '-I'  + str(inc)
> + ' '
> >  IncSearchList.append(inc)
> >  MakeApp = MakeApp + '\n'
> > --
> > 2.21.0.windows.1


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

View/Reply Online (#53642): https://edk2.groups.io/g/devel/message/53642
Mute This Topic: https://groups.io/mt/70261740/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 

Re: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/CpuFeature: Introduce First to indicate 1st unit.

2020-02-02 Thread Zeng, Star
Got the point.

With the typo fixed, Reviewed-by: Star Zeng 

> -Original Message-
> From: Ni, Ray
> Sent: Thursday, January 2, 2020 11:15 AM
> To: Zeng, Star ; Ray Ni
> ; devel@edk2.groups.io
> Cc: Dong, Eric ; Kinney, Michael D
> 
> Subject: RE: [PATCH v2 3/3] UefiCpuPkg/CpuFeature: Introduce First to indicate
> 1st unit.
> 
> >
> > Need some patches to update individual InitializeFunc() for features.
> > These patches can be a separated patch series.
> >
> Yes.
> 
> > >
> > > The patch adds a new field Fist to indicate the CPU's location in
> >
> > "Firt" should be "First".
> Will fix the typo in next version of patch or pushing.
> 
> > > +  //
> > > +  // Set First.Die/Tile/Module for each thread assuming:
> > > +  //  single Die under each package, single Tile under each Die, single
> > Module
> > > under each Tile
> >
> > This assumption needs to be addressed in this or a separated patch series.
> 
> The assumption will be fixed after the below changes are merged to trunk.
> https://github.com/tianocore/edk2-staging/tree/cpu/6-level
> 
> 
> > > +  for (PackageIndex = 0; PackageIndex < CpuStatus->PackageCount;
> > > PackageIndex++) {
> > > +//
> > > +// Set First.Core for each thread in the first core of each package.
> > > +//
> > > +First = MAX_UINT32;
> > > +for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus;
> > > ProcessorNumber++) {
> > > +  Location = 
> > > >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
> > > +  if (Location->Package == PackageIndex) {
> >
> > Here the code is assuming Location->Package starts from 0 and consecutive.
> 
> CpuStatus->PackageCount is assigned in CpuInitDataInitialize():
> >  CpuStatus->PackageCount= Package + 1;
> >  CpuStatus->MaxCoreCount= Core + 1;
> >  CpuStatus->MaxThreadCount  = Thread + 1;
> So PackageCount actually is the value of max package ID + 1.
> With that, the code change isn't assuming Location->Package starts from 0 and
> consecutive.
> 
> >
> > Here the code is assuming Location->Package and Location->Core start from
> > 0 and consecutive.
> > We could not have this assumption, this patch is to resolve this assumption.
> Similarly, The code change above isn't assuming Location->Core starts from 0
> and consecutive.
> 
> >
> >
> > Thanks,
> > Star
> >


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

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



Re: [edk2-devel] [Patch] BaseTools/DscBuildData: Fix PCD autogen include file conflict

2020-02-02 Thread Liming Gao
Mike:
  Another way is to parse INCLUDE env value and add the system PATH into -I 
options as the first priority. 

Thanks
Liming
> -Original Message-
> From: Kinney, Michael D 
> Sent: Thursday, January 30, 2020 8:46 AM
> To: devel@edk2.groups.io
> Cc: Feng, Bob C ; Gao, Liming 
> Subject: [Patch] BaseTools/DscBuildData: Fix PCD autogen include file conflict
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=2494
> 
> When using structured PCDs, a C application is auto generated
> to fill in the structured PCD value.  The C application uses
> the standard include files , , and .
> This C application also supports include paths from package DEC
> files when a structured PCD declaration provides a 
> list.  The complete list of include paths are -I options for
> include paths from package DEC files and the compiler's standard
> include paths.
> 
> -I include paths are higher priority than the standard include
> paths.  If the -I included paths from package DEC files contain
> , , or  the wrong include files are
> used to compile the C application for the structured PCD value.
> 
> Update GenerateByteArrayValue() to skip a package DEC include
> paths that contain , , or .
> 
> Build failures were observed when adding a structured PCD to
> CryptoPkg.  CryptoPkg contains , , and
>  in the path CryptoPkg/Library/Include to support
> building Open SSL.  The Library/Include path is listed as a
> private include path in CryptoPkg.dec.  Without this change, the
> standard include files designed to support build OpenSLL are
> used to build the structured PCD C application, and that build
> fails.
> 
> Other packages that provide a standard C lib or a gasket for
> a subset of the standard C lib will run into this same issue
> if they also define and use a Structured PCD.  So this issue
> is not limited to the CryptoPkg.
> 
> Cc: Bob Feng 
> Cc: Liming Gao 
> Signed-off-by: Michael D Kinney 
> ---
>  .../Source/Python/Workspace/DscBuildData.py| 18 +-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py 
> b/BaseTools/Source/Python/Workspace/DscBuildData.py
> index c65a0dd346..be6688dc75 100644
> --- a/BaseTools/Source/Python/Workspace/DscBuildData.py
> +++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
> @@ -1,7 +1,7 @@
>  ## @file
>  # This file is used to create a database used by build tool
>  #
> -# Copyright (c) 2008 - 2019, Intel Corporation. All rights reserved.
> +# Copyright (c) 2008 - 2020, Intel Corporation. All rights reserved.
>  # (C) Copyright 2016 Hewlett Packard Enterprise Development LP
>  # SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -2667,6 +2667,22 @@ class DscBuildData(PlatformBuildClassObject):
>  for pkg in PcdDependDEC:
>  if pkg in PlatformInc:
>  for inc in PlatformInc[pkg]:
> +#
> +# Get list of files in potential -I include path
> +#
> +FileList = os.listdir (str(inc))
> +#
> +# Skip -I include path if one of the include files 
> required
> +# by PcdValueInit.c are present in the include paths 
> from
> +# the DEC file.  PcdValueInit.c must use the 
> standard include
> +# files from the host compiler.
> +#
> +if 'stdio.h' in FileList:
> +  continue
> +if 'stdlib.h' in FileList:
> +  continue
> +if 'string.h' in FileList:
> +  continue
>  MakeApp += '-I'  + str(inc) + ' '
>  IncSearchList.append(inc)
>  MakeApp = MakeApp + '\n'
> --
> 2.21.0.windows.1


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

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



Re: [edk2-devel] [Patch] BaseTools/Build: Fix Structured PCD app host env issues

2020-02-02 Thread Liming Gao
Mike:
  Sorry. This edk2\BaseTools\get_vsvars.bat can set VS host env. 

Thanks
Liming
> -Original Message-
> From: Kinney, Michael D 
> Sent: Saturday, February 1, 2020 12:43 AM
> To: Gao, Liming ; devel@edk2.groups.io; Kinney, Michael 
> D 
> Cc: Feng, Bob C 
> Subject: RE: [Patch] BaseTools/Build: Fix Structured PCD app host env issues
> 
> Liming,
> 
> That script does not set everything up to build
> host apps for all VS20xx tool chains.  It works
> for VS2017, VS2019, but not any of the others.
> It sets env vars for those other tool chains but
> Does not run the VS environment setup script that
> updates PATH and sets LIBS and INC to support
> build of POSIX apps.
> 
> Mike
> 
> > -Original Message-
> > From: Gao, Liming 
> > Sent: Friday, January 31, 2020 12:27 AM
> > To: Kinney, Michael D ;
> > devel@edk2.groups.io
> > Cc: Feng, Bob C 
> > Subject: RE: [Patch] BaseTools/Build: Fix Structured
> > PCD app host env issues
> >
> > Mike:
> >   I suggest to call BaseTools\set_vsprefix_envs.bat to
> > setup VS environment. Its input parameter is VS2012,
> > VS2013, VS2015, VS2017, VS2019. If so, we don't need to
> > add VS installation path in Build python script.
> >
> > Thanks
> > Liming
> > > -Original Message-
> > > From: Kinney, Michael D 
> > > Sent: Thursday, January 30, 2020 10:38 AM
> > > To: devel@edk2.groups.io
> > > Cc: Feng, Bob C ; Gao, Liming
> > 
> > > Subject: [Patch] BaseTools/Build: Fix Structured PCD
> > app host env issues
> > >
> > > https://bugzilla.tianocore.org/show_bug.cgi?id=2495
> > > https://bugzilla.tianocore.org/show_bug.cgi?id=2496
> > >
> > > Structured PCD processing requires a host POSIX build
> > > environment.  If the Structure PCD application can
> > not
> > > be built using the default environment under Windows,
> > then
> > > retry the build after setting up the host environment
> > for
> > > the current tool chain tag.
> > >
> > > Also reduce the build dependencies for the Structured
> > PCD
> > > application to increase compiler compatibility.
> > >
> > > Cc: Bob Feng 
> > > Cc: Liming Gao 
> > > Signed-off-by: Michael D Kinney
> > 
> > > ---
> > >  .../Source/Python/Workspace/DscBuildData.py   | 35
> > +++
> > >  1 file changed, 28 insertions(+), 7 deletions(-)
> > >
> > > diff --git
> > a/BaseTools/Source/Python/Workspace/DscBuildData.py
> > b/BaseTools/Source/Python/Workspace/DscBuildData.py
> > > index c65a0dd346..bc3f32bb1d 100644
> > > ---
> > a/BaseTools/Source/Python/Workspace/DscBuildData.py
> > > +++
> > b/BaseTools/Source/Python/Workspace/DscBuildData.py
> > > @@ -91,9 +91,6 @@ WindowsCFLAGS = 'CFLAGS = $(CFLAGS)
> > /wd4200 /wd4034 /wd4101 '
> > >  LinuxCFLAGS = 'BUILD_CFLAGS += -Wno-pointer-to-int-
> > cast -Wno-unused-variable '
> > >  PcdMakefileEnd = '''
> > >  !INCLUDE
> > $(BASE_TOOLS_PATH)\Source\C\Makefiles\ms.common
> > > -
> > > -LIBS = $(LIB_PATH)\Common.lib
> > > -
> > >  !INCLUDE
> > $(BASE_TOOLS_PATH)\Source\C\Makefiles\ms.app
> > >  '''
> > >
> > > @@ -2637,10 +2634,10 @@ class
> > DscBuildData(PlatformBuildClassObject):
> > >
> > >  MakeApp = PcdMakefileHeader
> > >  if sys.platform == "win32":
> > > -MakeApp = MakeApp + 'APPFILE =
> > %s\%s.exe\n' % (self.OutputPath, PcdValueInitName) +
> > 'APPNAME = %s\n' %
> > > (PcdValueInitName) + 'OBJECTS = %s\%s.obj\n' %
> > (self.OutputPath, PcdValueInitName) + 'INC = '
> > > +MakeApp = MakeApp + 'APPFILE =
> > %s\%s.exe\n' % (self.OutputPath, PcdValueInitName) +
> > 'APPNAME = %s\n' %
> > > (PcdValueInitName) + 'OBJECTS = %s\%s.obj %s.obj\n' %
> > (self.OutputPath, PcdValueInitName,
> > >
> > os.path.normpath(mws.join(GlobalData.gGlobalDefines["ED
> > K_TOOLS_PATH"], "Source/C/Common/PcdValueCommon"))) +
> > 'INC = '
> > >  else:
> > >  MakeApp = MakeApp + PcdGccMakefile
> > > -MakeApp = MakeApp + 'APPFILE = %s/%s\n'
> > % (self.OutputPath, PcdValueInitName) + 'APPNAME =
> > %s\n' % (PcdValueInitName)
> > > + 'OBJECTS = %s/%s.o\n' % (self.OutputPath,
> > PcdValueInitName) + \
> > > +MakeApp = MakeApp + 'APPFILE = %s/%s\n'
> > % (self.OutputPath, PcdValueInitName) + 'APPNAME =
> > %s\n' %
> > > (PcdValueInitName) + 'OBJECTS = %s/%s.o %s.o\n' %
> > (self.OutputPath, PcdValueInitName,
> > >
> > os.path.normpath(mws.join(GlobalData.gGlobalDefines["ED
> > K_TOOLS_PATH"], "Source/C/Common/PcdValueCommon"))) + \
> > >'include
> > $(MAKEROOT)/Makefiles/app.makefile\n' + 'INCLUDE +='
> > >
> > >  IncSearchList = []
> > > @@ -2723,8 +2720,8 @@ class
> > DscBuildData(PlatformBuildClassObject):
> > >
> > IncludeFileFullPaths.append(os.path.normpath(includeful
> > lpath))
> > >  break
> > >  SearchPathList = []
> > > -
> > SearchPathList.append(os.path.normpath(mws.join(GlobalD
> > ata.gWorkspace, "BaseTools/Source/C/Include")))
> > > -
> > SearchPathList.append(os.path.normpath(mws.join(GlobalD
> > ata.gWorkspace, 

Re: [edk2-devel] [PATCH 2/5] MdePkg: Allow PcdFSBClock to by Dynamic

2020-02-02 Thread Liming Gao
Anthony:
  This change is OK to me. But if this PCD is configured as Dynamic, its value 
will be got from PCD service. This operation will take some time and cause the 
inaccurate time delay. Have you measured its impact?

Thanks
Liming
> -Original Message-
> From: Laszlo Ersek 
> Sent: Thursday, January 30, 2020 12:11 AM
> To: Anthony PERARD ; devel@edk2.groups.io
> Cc: Kinney, Michael D ; Ard Biesheuvel 
> ; xen-de...@lists.xenproject.org;
> Gao, Liming ; Justen, Jordan L 
> ; Julien Grall ; Feng, Bob C
> 
> Subject: Re: [PATCH 2/5] MdePkg: Allow PcdFSBClock to by Dynamic
> 
> On 01/29/20 13:12, Anthony PERARD wrote:
> > We are going to want to change the value of PcdFSBClock at run time in
> > OvmfXen, so move it to the PcdsDynamic section.
> >
> > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2490
> > Signed-off-by: Anthony PERARD 
> > ---
> > CC: Bob Feng 
> > CC: Liming Gao 
> > ---
> >  MdePkg/MdePkg.dec | 8 
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> > index d022cc5e3ef2..8f5a48346e50 100644
> > --- a/MdePkg/MdePkg.dec
> > +++ b/MdePkg/MdePkg.dec
> > @@ -2194,10 +2194,6 @@ [PcdsFixedAtBuild,PcdsPatchableInModule]
> ># @ValidList  0x8001 | 8, 16, 32
> >gEfiMdePkgTokenSpaceGuid.PcdPort80DataWidth|8|UINT8|0x002d
> >
> > -  ## This value is used to configure X86 Processor FSB clock.
> > -  # @Prompt FSB Clock.
> > -  gEfiMdePkgTokenSpaceGuid.PcdFSBClock|2|UINT32|0x000c
> > -
> >## The maximum printable number of characters. UefLib functions: 
> > AsciiPrint(), AsciiErrorPrint(),
> >#  PrintXY(), AsciiPrintXY(), Print(), ErrorPrint() base on this PCD 
> > value to print characters.
> ># @Prompt Maximum Printable Number of Characters.
> > @@ -2297,5 +2293,9 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, 
> > PcdsDynamic, PcdsDynamicEx]
> ># @Prompt Boot Timeout (s)
> >gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0x|UINT16|0x002c
> >
> > +  ## This value is used to configure X86 Processor FSB clock.
> > +  # @Prompt FSB Clock.
> > +  gEfiMdePkgTokenSpaceGuid.PcdFSBClock|2|UINT32|0x000c
> > +
> >  [UserExtensions.TianoCore."ExtraFiles"]
> >MdePkgExtra.uni
> >
> 
> Looks good to me:
> 
> Reviewed-by: Laszlo Ersek 
> 
> Mike or Liming will have to ACK.
> 
> Thanks!
> Laszlo


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

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



Re: [edk2-devel] [PATCH v1] UefiCpuPkg/MpInitLib: Always get CPUID & PlatformID in MicrocodeDetect()

2020-02-02 Thread Siyuan, Fu
Got it.

Reviewed-by: Siyuan Fu 

> -Original Message-
> From: Wu, Hao A 
> Sent: 2020年2月3日 9:03
> To: Fu, Siyuan ; devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ; Laszlo
> Ersek ; Kinney, Michael D
> 
> Subject: RE: [edk2-devel] [PATCH v1] UefiCpuPkg/MpInitLib: Always get
> CPUID & PlatformID in MicrocodeDetect()
> 
> > -Original Message-
> > From: Fu, Siyuan
> > Sent: Monday, February 03, 2020 8:55 AM
> > To: devel@edk2.groups.io; Wu, Hao A
> > Cc: Dong, Eric; Ni, Ray; Laszlo Ersek; Kinney, Michael D
> > Subject: RE: [edk2-devel] [PATCH v1] UefiCpuPkg/MpInitLib: Always get
> CPUID
> > & PlatformID in MicrocodeDetect()
> >
> > Hi, Hao
> >
> > If CPUID and PlatformID will always be collected in MicrocodeDetect(), is
> the
> > change in 999463 redundant now? Should we remove that code?
> 
> 
> I think the changes are still needed.
> 
> The CPUID and PlatformID information stored in the 'CPU_AP_DATA'
> structure are
> needed during the pre-process of the microcode patch headers for deciding
> whether the microcode patch data should be loaded into memory.
> 
> Best Regards,
> Hao Wu
> 
> 
> >
> > Best Regards
> > Siyuan
> >
> > > -Original Message-
> > > From: devel@edk2.groups.io  On Behalf Of Wu,
> Hao
> > > A
> > > Sent: 2020年2月3日 8:35
> > > To: devel@edk2.groups.io
> > > Cc: Wu, Hao A ; Dong, Eric ;
> Ni,
> > > Ray ; Laszlo Ersek ; Fu, Siyuan
> > > ; Kinney, Michael D 
> > > Subject: [edk2-devel] [PATCH v1] UefiCpuPkg/MpInitLib: Always get
> CPUID &
> > > PlatformID in MicrocodeDetect()
> > >
> > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2498
> > >
> > > Commit fd30b00707 updated the logic in function MicrocodeDetect()
> that
> > > will directly use the CPUID and PlatformID information from the
> 'CpuData'
> > > field in the CPU_MP_DATA structure, instead of collecting these
> > > information for each processor via AsmCpuid() and AsmReadMsr64() calls
> > > respectively.
> > >
> > > At that moment, this approach worked fine for APs. Since:
> > > a) When the APs are waken up for the 1st time (1st MpInitLibInitialize()
> > >entry at PEI phase), the function InitializeApData() will be called for
> > >each AP and the CPUID and PlatformID information will be collected.
> > >
> > > b) During the 2nd entry of MpInitLibInitialize() at DXE phase, when the
> > >APs are waken up again, the function InitializeApData() will not be
> > >called, which means the CPUID and PlatformID information will not be
> > >collected. However, the below logics in MicrocodeDetect() function:
> > >
> > >   CurrentRevision = GetCurrentMicrocodeSignature ();
> > >   IsBspCallIn = (ProcessorNumber == (UINTN)CpuMpData-
> >BspNumber) ?
> > > TRUE : FALSE;
> > >   if (CurrentRevision != 0 && !IsBspCallIn) {
> > > //
> > > // Skip loading microcode if it has been loaded successfully
> > > //
> > > return;
> > >   }
> > >
> > >will ensure that the microcode detection and application will be
> > >skipped due to the fact that such process has already been done in the
> > >PEI phase.
> > >
> > > But after commit 396e791059, which removes the above skip loading
> logic,
> > > the CPUID and PlatformID information on APs will be used upon the 2nd
> > > entry of the MpInitLibInitialize(). But since the CPUID and PlatformID
> > > information has not been collected, it will bring issue to the microcode
> > > detection process.
> > >
> > > This commit will update the logic in MicrocodeDetect() back to always
> > > collecting the CPUID and PlatformID information explicitly.
> > >
> > > Cc: Eric Dong 
> > > Cc: Ray Ni 
> > > Cc: Laszlo Ersek 
> > > Cc: Siyuan Fu 
> > > Cc: Michael D Kinney 
> > > Signed-off-by: Hao A Wu 
> > > ---
> > >  UefiCpuPkg/Library/MpInitLib/Microcode.c | 15 +--
> > >  1 file changed, 13 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > > b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > > index b6675b9a60..67e214d463 100644
> > > --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > > +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > > @@ -93,6 +93,7 @@ MicrocodeDetect (
> > >UINT32  InCompleteCheckSum32;
> > >BOOLEAN CorrectMicrocode;
> > >VOID*MicrocodeData;
> > > +  MSR_IA32_PLATFORM_ID_REGISTER   PlatformIdMsr;
> > >UINT32  ThreadId;
> > >BOOLEAN IsBspCallIn;
> > >
> > > @@ -115,8 +116,18 @@ MicrocodeDetect (
> > >}
> > >
> > >ExtendedTableLength = 0;
> > > -  Eax.Uint32 = CpuMpData-
> >CpuData[ProcessorNumber].ProcessorSignature;
> > > -  PlatformId = CpuMpData->CpuData[ProcessorNumber].PlatformId;
> > > +  //
> > > +  // Here data of CPUID leafs have not been collected into context 
> > > buffer,
> > so
> > > +  // GetProcessorCpuid() cannot be used here to retrieve CPUID data.
> > > 

Re: [edk2-devel] [PATCH v1] UefiCpuPkg/MpInitLib: Always get CPUID & PlatformID in MicrocodeDetect()

2020-02-02 Thread Wu, Hao A
> -Original Message-
> From: Fu, Siyuan
> Sent: Monday, February 03, 2020 8:55 AM
> To: devel@edk2.groups.io; Wu, Hao A
> Cc: Dong, Eric; Ni, Ray; Laszlo Ersek; Kinney, Michael D
> Subject: RE: [edk2-devel] [PATCH v1] UefiCpuPkg/MpInitLib: Always get CPUID
> & PlatformID in MicrocodeDetect()
> 
> Hi, Hao
> 
> If CPUID and PlatformID will always be collected in MicrocodeDetect(), is the
> change in 999463 redundant now? Should we remove that code?


I think the changes are still needed.

The CPUID and PlatformID information stored in the 'CPU_AP_DATA' structure are
needed during the pre-process of the microcode patch headers for deciding
whether the microcode patch data should be loaded into memory.

Best Regards,
Hao Wu


> 
> Best Regards
> Siyuan
> 
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of Wu, Hao
> > A
> > Sent: 2020年2月3日 8:35
> > To: devel@edk2.groups.io
> > Cc: Wu, Hao A ; Dong, Eric ; Ni,
> > Ray ; Laszlo Ersek ; Fu, Siyuan
> > ; Kinney, Michael D 
> > Subject: [edk2-devel] [PATCH v1] UefiCpuPkg/MpInitLib: Always get CPUID &
> > PlatformID in MicrocodeDetect()
> >
> > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2498
> >
> > Commit fd30b00707 updated the logic in function MicrocodeDetect() that
> > will directly use the CPUID and PlatformID information from the 'CpuData'
> > field in the CPU_MP_DATA structure, instead of collecting these
> > information for each processor via AsmCpuid() and AsmReadMsr64() calls
> > respectively.
> >
> > At that moment, this approach worked fine for APs. Since:
> > a) When the APs are waken up for the 1st time (1st MpInitLibInitialize()
> >entry at PEI phase), the function InitializeApData() will be called for
> >each AP and the CPUID and PlatformID information will be collected.
> >
> > b) During the 2nd entry of MpInitLibInitialize() at DXE phase, when the
> >APs are waken up again, the function InitializeApData() will not be
> >called, which means the CPUID and PlatformID information will not be
> >collected. However, the below logics in MicrocodeDetect() function:
> >
> >   CurrentRevision = GetCurrentMicrocodeSignature ();
> >   IsBspCallIn = (ProcessorNumber == (UINTN)CpuMpData->BspNumber) ?
> > TRUE : FALSE;
> >   if (CurrentRevision != 0 && !IsBspCallIn) {
> > //
> > // Skip loading microcode if it has been loaded successfully
> > //
> > return;
> >   }
> >
> >will ensure that the microcode detection and application will be
> >skipped due to the fact that such process has already been done in the
> >PEI phase.
> >
> > But after commit 396e791059, which removes the above skip loading logic,
> > the CPUID and PlatformID information on APs will be used upon the 2nd
> > entry of the MpInitLibInitialize(). But since the CPUID and PlatformID
> > information has not been collected, it will bring issue to the microcode
> > detection process.
> >
> > This commit will update the logic in MicrocodeDetect() back to always
> > collecting the CPUID and PlatformID information explicitly.
> >
> > Cc: Eric Dong 
> > Cc: Ray Ni 
> > Cc: Laszlo Ersek 
> > Cc: Siyuan Fu 
> > Cc: Michael D Kinney 
> > Signed-off-by: Hao A Wu 
> > ---
> >  UefiCpuPkg/Library/MpInitLib/Microcode.c | 15 +--
> >  1 file changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > index b6675b9a60..67e214d463 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > @@ -93,6 +93,7 @@ MicrocodeDetect (
> >UINT32  InCompleteCheckSum32;
> >BOOLEAN CorrectMicrocode;
> >VOID*MicrocodeData;
> > +  MSR_IA32_PLATFORM_ID_REGISTER   PlatformIdMsr;
> >UINT32  ThreadId;
> >BOOLEAN IsBspCallIn;
> >
> > @@ -115,8 +116,18 @@ MicrocodeDetect (
> >}
> >
> >ExtendedTableLength = 0;
> > -  Eax.Uint32 = CpuMpData->CpuData[ProcessorNumber].ProcessorSignature;
> > -  PlatformId = CpuMpData->CpuData[ProcessorNumber].PlatformId;
> > +  //
> > +  // Here data of CPUID leafs have not been collected into context buffer,
> so
> > +  // GetProcessorCpuid() cannot be used here to retrieve CPUID data.
> > +  //
> > +  AsmCpuid (CPUID_VERSION_INFO, , NULL, NULL, NULL);
> > +
> > +  //
> > +  // The index of platform information resides in bits 50:52 of MSR
> > IA32_PLATFORM_ID
> > +  //
> > +  PlatformIdMsr.Uint64 = AsmReadMsr64 (MSR_IA32_PLATFORM_ID);
> > +  PlatformId = (UINT8) PlatformIdMsr.Bits.PlatformId;
> > +
> >
> >//
> >// Check whether AP has same processor with BSP.
> > --
> > 2.12.0.windows.1
> >
> >
> > 


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

View/Reply Online (#53636): 

Re: [edk2-devel] [Patch] BaseTools/DscBuildData: Fix PCD autogen include file conflict

2020-02-02 Thread Liming Gao
Mike:
  Can we consider to parse INCLUDE env value and add those path to -I options 
as the first priority?

Thanks
Liming
> -Original Message-
> From: Kinney, Michael D 
> Sent: Thursday, January 30, 2020 8:46 AM
> To: devel@edk2.groups.io
> Cc: Feng, Bob C ; Gao, Liming 
> Subject: [Patch] BaseTools/DscBuildData: Fix PCD autogen include file conflict
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=2494
> 
> When using structured PCDs, a C application is auto generated
> to fill in the structured PCD value.  The C application uses
> the standard include files , , and .
> This C application also supports include paths from package DEC
> files when a structured PCD declaration provides a 
> list.  The complete list of include paths are -I options for
> include paths from package DEC files and the compiler's standard
> include paths.
> 
> -I include paths are higher priority than the standard include
> paths.  If the -I included paths from package DEC files contain
> , , or  the wrong include files are
> used to compile the C application for the structured PCD value.
> 
> Update GenerateByteArrayValue() to skip a package DEC include
> paths that contain , , or .
> 
> Build failures were observed when adding a structured PCD to
> CryptoPkg.  CryptoPkg contains , , and
>  in the path CryptoPkg/Library/Include to support
> building Open SSL.  The Library/Include path is listed as a
> private include path in CryptoPkg.dec.  Without this change, the
> standard include files designed to support build OpenSLL are
> used to build the structured PCD C application, and that build
> fails.
> 
> Other packages that provide a standard C lib or a gasket for
> a subset of the standard C lib will run into this same issue
> if they also define and use a Structured PCD.  So this issue
> is not limited to the CryptoPkg.
> 
> Cc: Bob Feng 
> Cc: Liming Gao 
> Signed-off-by: Michael D Kinney 
> ---
>  .../Source/Python/Workspace/DscBuildData.py| 18 +-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py 
> b/BaseTools/Source/Python/Workspace/DscBuildData.py
> index c65a0dd346..be6688dc75 100644
> --- a/BaseTools/Source/Python/Workspace/DscBuildData.py
> +++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
> @@ -1,7 +1,7 @@
>  ## @file
>  # This file is used to create a database used by build tool
>  #
> -# Copyright (c) 2008 - 2019, Intel Corporation. All rights reserved.
> +# Copyright (c) 2008 - 2020, Intel Corporation. All rights reserved.
>  # (C) Copyright 2016 Hewlett Packard Enterprise Development LP
>  # SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -2667,6 +2667,22 @@ class DscBuildData(PlatformBuildClassObject):
>  for pkg in PcdDependDEC:
>  if pkg in PlatformInc:
>  for inc in PlatformInc[pkg]:
> +#
> +# Get list of files in potential -I include path
> +#
> +FileList = os.listdir (str(inc))
> +#
> +# Skip -I include path if one of the include files 
> required
> +# by PcdValueInit.c are present in the include paths 
> from
> +# the DEC file.  PcdValueInit.c must use the 
> standard include
> +# files from the host compiler.
> +#
> +if 'stdio.h' in FileList:
> +  continue
> +if 'stdlib.h' in FileList:
> +  continue
> +if 'string.h' in FileList:
> +  continue
>  MakeApp += '-I'  + str(inc) + ' '
>  IncSearchList.append(inc)
>  MakeApp = MakeApp + '\n'
> --
> 2.21.0.windows.1


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

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



Re: [edk2-devel] [PATCH v1] UefiCpuPkg/MpInitLib: Always get CPUID & PlatformID in MicrocodeDetect()

2020-02-02 Thread Siyuan, Fu
Hi, Hao

If CPUID and PlatformID will always be collected in MicrocodeDetect(), is the 
change in 999463 redundant now? Should we remove that code?

Best Regards
Siyuan 

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Wu, Hao
> A
> Sent: 2020年2月3日 8:35
> To: devel@edk2.groups.io
> Cc: Wu, Hao A ; Dong, Eric ; Ni,
> Ray ; Laszlo Ersek ; Fu, Siyuan
> ; Kinney, Michael D 
> Subject: [edk2-devel] [PATCH v1] UefiCpuPkg/MpInitLib: Always get CPUID &
> PlatformID in MicrocodeDetect()
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2498
> 
> Commit fd30b00707 updated the logic in function MicrocodeDetect() that
> will directly use the CPUID and PlatformID information from the 'CpuData'
> field in the CPU_MP_DATA structure, instead of collecting these
> information for each processor via AsmCpuid() and AsmReadMsr64() calls
> respectively.
> 
> At that moment, this approach worked fine for APs. Since:
> a) When the APs are waken up for the 1st time (1st MpInitLibInitialize()
>entry at PEI phase), the function InitializeApData() will be called for
>each AP and the CPUID and PlatformID information will be collected.
> 
> b) During the 2nd entry of MpInitLibInitialize() at DXE phase, when the
>APs are waken up again, the function InitializeApData() will not be
>called, which means the CPUID and PlatformID information will not be
>collected. However, the below logics in MicrocodeDetect() function:
> 
>   CurrentRevision = GetCurrentMicrocodeSignature ();
>   IsBspCallIn = (ProcessorNumber == (UINTN)CpuMpData->BspNumber) ?
> TRUE : FALSE;
>   if (CurrentRevision != 0 && !IsBspCallIn) {
> //
> // Skip loading microcode if it has been loaded successfully
> //
> return;
>   }
> 
>will ensure that the microcode detection and application will be
>skipped due to the fact that such process has already been done in the
>PEI phase.
> 
> But after commit 396e791059, which removes the above skip loading logic,
> the CPUID and PlatformID information on APs will be used upon the 2nd
> entry of the MpInitLibInitialize(). But since the CPUID and PlatformID
> information has not been collected, it will bring issue to the microcode
> detection process.
> 
> This commit will update the logic in MicrocodeDetect() back to always
> collecting the CPUID and PlatformID information explicitly.
> 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Cc: Siyuan Fu 
> Cc: Michael D Kinney 
> Signed-off-by: Hao A Wu 
> ---
>  UefiCpuPkg/Library/MpInitLib/Microcode.c | 15 +--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> index b6675b9a60..67e214d463 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> @@ -93,6 +93,7 @@ MicrocodeDetect (
>UINT32  InCompleteCheckSum32;
>BOOLEAN CorrectMicrocode;
>VOID*MicrocodeData;
> +  MSR_IA32_PLATFORM_ID_REGISTER   PlatformIdMsr;
>UINT32  ThreadId;
>BOOLEAN IsBspCallIn;
> 
> @@ -115,8 +116,18 @@ MicrocodeDetect (
>}
> 
>ExtendedTableLength = 0;
> -  Eax.Uint32 = CpuMpData->CpuData[ProcessorNumber].ProcessorSignature;
> -  PlatformId = CpuMpData->CpuData[ProcessorNumber].PlatformId;
> +  //
> +  // Here data of CPUID leafs have not been collected into context buffer, so
> +  // GetProcessorCpuid() cannot be used here to retrieve CPUID data.
> +  //
> +  AsmCpuid (CPUID_VERSION_INFO, , NULL, NULL, NULL);
> +
> +  //
> +  // The index of platform information resides in bits 50:52 of MSR
> IA32_PLATFORM_ID
> +  //
> +  PlatformIdMsr.Uint64 = AsmReadMsr64 (MSR_IA32_PLATFORM_ID);
> +  PlatformId = (UINT8) PlatformIdMsr.Bits.PlatformId;
> +
> 
>//
>// Check whether AP has same processor with BSP.
> --
> 2.12.0.windows.1
> 
> 
> 


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

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



[edk2-devel] [PATCH v1] UefiCpuPkg/MpInitLib: Always get CPUID & PlatformID in MicrocodeDetect()

2020-02-02 Thread Wu, Hao A
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2498

Commit fd30b00707 updated the logic in function MicrocodeDetect() that
will directly use the CPUID and PlatformID information from the 'CpuData'
field in the CPU_MP_DATA structure, instead of collecting these
information for each processor via AsmCpuid() and AsmReadMsr64() calls
respectively.

At that moment, this approach worked fine for APs. Since:
a) When the APs are waken up for the 1st time (1st MpInitLibInitialize()
   entry at PEI phase), the function InitializeApData() will be called for
   each AP and the CPUID and PlatformID information will be collected.

b) During the 2nd entry of MpInitLibInitialize() at DXE phase, when the
   APs are waken up again, the function InitializeApData() will not be
   called, which means the CPUID and PlatformID information will not be
   collected. However, the below logics in MicrocodeDetect() function:

  CurrentRevision = GetCurrentMicrocodeSignature ();
  IsBspCallIn = (ProcessorNumber == (UINTN)CpuMpData->BspNumber) ? TRUE : 
FALSE;
  if (CurrentRevision != 0 && !IsBspCallIn) {
//
// Skip loading microcode if it has been loaded successfully
//
return;
  }

   will ensure that the microcode detection and application will be
   skipped due to the fact that such process has already been done in the
   PEI phase.

But after commit 396e791059, which removes the above skip loading logic,
the CPUID and PlatformID information on APs will be used upon the 2nd
entry of the MpInitLibInitialize(). But since the CPUID and PlatformID
information has not been collected, it will bring issue to the microcode
detection process.

This commit will update the logic in MicrocodeDetect() back to always
collecting the CPUID and PlatformID information explicitly.

Cc: Eric Dong 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Siyuan Fu 
Cc: Michael D Kinney 
Signed-off-by: Hao A Wu 
---
 UefiCpuPkg/Library/MpInitLib/Microcode.c | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c 
b/UefiCpuPkg/Library/MpInitLib/Microcode.c
index b6675b9a60..67e214d463 100644
--- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
+++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
@@ -93,6 +93,7 @@ MicrocodeDetect (
   UINT32  InCompleteCheckSum32;
   BOOLEAN CorrectMicrocode;
   VOID*MicrocodeData;
+  MSR_IA32_PLATFORM_ID_REGISTER   PlatformIdMsr;
   UINT32  ThreadId;
   BOOLEAN IsBspCallIn;
 
@@ -115,8 +116,18 @@ MicrocodeDetect (
   }
 
   ExtendedTableLength = 0;
-  Eax.Uint32 = CpuMpData->CpuData[ProcessorNumber].ProcessorSignature;
-  PlatformId = CpuMpData->CpuData[ProcessorNumber].PlatformId;
+  //
+  // Here data of CPUID leafs have not been collected into context buffer, so
+  // GetProcessorCpuid() cannot be used here to retrieve CPUID data.
+  //
+  AsmCpuid (CPUID_VERSION_INFO, , NULL, NULL, NULL);
+
+  //
+  // The index of platform information resides in bits 50:52 of MSR 
IA32_PLATFORM_ID
+  //
+  PlatformIdMsr.Uint64 = AsmReadMsr64 (MSR_IA32_PLATFORM_ID);
+  PlatformId = (UINT8) PlatformIdMsr.Bits.PlatformId;
+
 
   //
   // Check whether AP has same processor with BSP.
-- 
2.12.0.windows.1


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

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