Hi Eric, On 10/25/17 09:58, Eric Dong wrote: > In current implementation, CPU initialized can be done in PEI > or DXE phase. PEI uses CpuFeaturesPie and Dxe uses CpuFeaturesDxe.
(1) "CpuFeaturesPie" has a typo > If CPU initialized in PEI phase, CpuFeaturesDxe driver will > not be used. This driver will install gEdkiiCpuFeaturesInitDoneGuid > protocol after it initializes the CPU. > > Some drivers depend on this protocol to dispatch themselves. If > CpuFeaturesDxe not been used, these drivers will not be dispatched. > > This patch fix the above issue. If Cpu initialized in PEI > phase, it also report a guid HOB for CpuFeaturesDxe. > CpuFeaturesDxe will check this HOB first. If it found this > HOB, it just install gEdkiiCpuFeaturesInitDoneGuid protocol, > else it will also do the CPU initialization. > > Cc: Ruiyu Ni <[email protected]> > Cc: Laszlo Ersek <[email protected]> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Eric Dong <[email protected]> > --- > UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.c | 19 +++++++++++++++++++ > UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.inf | 1 + > UefiCpuPkg/CpuFeatures/CpuFeaturesPei.c | 6 ++++++ > UefiCpuPkg/CpuFeatures/CpuFeaturesPei.inf | 1 + > 4 files changed, 27 insertions(+) > > diff --git a/UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.c > b/UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.c > index b0b186d..6c0ae9d 100644 > --- a/UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.c > +++ b/UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.c > @@ -19,6 +19,7 @@ > #include <Library/UefiLib.h> > #include <Library/UefiBootServicesTableLib.h> > #include <Library/RegisterCpuFeaturesLib.h> > +#include <Library/HobLib.h> > > #include <Protocol/SmmConfiguration.h> > #include <Guid/CpuFeaturesInitDone.h> > @@ -101,6 +102,24 @@ CpuFeaturesDxeInitialize ( > ) > { > VOID *Registration; > + EFI_STATUS Status; > + EFI_HANDLE Handle; > + > + if (GetFirstGuidHob (&gEdkiiCpuFeaturesInitDoneGuid) != NULL) { > + // > + // Try to find HOB first. This HOB exist means CPU features have > + // been initialized by CpuFeaturesPei driver, just install > gEdkiiCpuFeaturesInitDoneGuid. (2) I think this line is too long; I suggest to wrap it to 80 characters. > + // > + Handle = NULL; > + Status = gBS->InstallProtocolInterface ( > + &Handle, > + &gEdkiiCpuFeaturesInitDoneGuid, > + EFI_NATIVE_INTERFACE, > + NULL > + ); > + ASSERT_EFI_ERROR (Status); > + return EFI_SUCCESS; > + } (3) Can we remove the ASSERT_EFI_ERROR() and just return Status here? No resources would be leaked, and the driver exit status would reflect the failure. (4) Just a question (not even a suggestion): can we perhaps install the protocol on ImageHandle as well? This way we could avoid allocating a new handle. > > if (PcdGetBool (PcdCpuFeaturesInitAfterSmmRelocation)) { > // > diff --git a/UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.inf > b/UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.inf > index 175e8a9..b1733be 100644 > --- a/UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.inf > +++ b/UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.inf > @@ -33,6 +33,7 @@ > UefiDriverEntryPoint > UefiBootServicesTableLib > RegisterCpuFeaturesLib > + HobLib > > [Sources] > CpuFeaturesDxe.c > diff --git a/UefiCpuPkg/CpuFeatures/CpuFeaturesPei.c > b/UefiCpuPkg/CpuFeatures/CpuFeaturesPei.c > index b052d55..72ee19b 100644 > --- a/UefiCpuPkg/CpuFeatures/CpuFeaturesPei.c > +++ b/UefiCpuPkg/CpuFeatures/CpuFeaturesPei.c > @@ -18,6 +18,7 @@ > #include <Library/DebugLib.h> > #include <Library/PeiServicesLib.h> > #include <Library/RegisterCpuFeaturesLib.h> > +#include <Library/HobLib.h> > > #include <Guid/CpuFeaturesInitDone.h> > > @@ -70,6 +71,11 @@ CpuFeaturesPeimInitialize ( > Status = PeiServicesInstallPpi(&mPeiCpuFeaturesInitDonePpiDesc); > ASSERT_EFI_ERROR (Status); > > + // > + // Build HOB to let CpuFeatureDxe driver skip the initialization process. > + // > + BuildGuidHob (&gEdkiiCpuFeaturesInitDoneGuid, 0); > + > return EFI_SUCCESS; > } > > diff --git a/UefiCpuPkg/CpuFeatures/CpuFeaturesPei.inf > b/UefiCpuPkg/CpuFeatures/CpuFeaturesPei.inf > index dd4b388..e617c5b 100644 > --- a/UefiCpuPkg/CpuFeatures/CpuFeaturesPei.inf > +++ b/UefiCpuPkg/CpuFeatures/CpuFeaturesPei.inf > @@ -32,6 +32,7 @@ > PeimEntryPoint > PeiServicesLib > RegisterCpuFeaturesLib > + HobLib > > [Sources] > CpuFeaturesPei.c > I think (1) and (2) should really be fixed -- but you can fix those when you push the patch. I don't insist on (3) and (4) -- if you don't want to change those parts, I'm fine with that too. With (1) and (2) updated: Reviewed-by: Laszlo Ersek <[email protected]> Thanks! Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
