[Patch 0/2] Add missed Depex Protocol/Ppi.

[Patch 0/2] Add missed Depex Protocol/Ppi.

[Eric Dong]

The Protocol/Ppi used in the drivers but the it not add them
in the dependence section, it may cause driver assert. 
This patch series add the missed Protocol/Ppi.

Eric Dong (2):
  UefiCpuPkg/CpuFeaturesDxe.inf: Add missed Depex protocol.
  UefiCpuPkg/CpuFeaturesPei.inf: Add missed Depex Ppi.

 UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.inf | 2 +-
 UefiCpuPkg/CpuFeatures/CpuFeaturesPei.inf | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
2.7.0.windows.1

[Patch 1/2] UefiCpuPkg/CpuFeaturesDxe.inf: Add missed Depex protocol.

[Eric Dong]

The gEfiMpServiceProtocolGuid protocol is used by
CpuFeaturesDxe.inf during feature detection. Should be
included in the [DepEx] section but actually not.
This patch add it.

Cc: Liming Gao <ruiyu.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
---
 UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.inf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.inf b/UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.inf
index b1733be..77c6d80 100644
--- a/UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.inf
+++ b/UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.inf
@@ -48,7 +48,7 @@
   gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesInitAfterSmmRelocation  ## CONSUMES
 
 [Depex]
-  TRUE
+  gEfiMpServiceProtocolGuid
 
 [UserExtensions.TianoCore."ExtraFiles"]
   CpuFeaturesDxeExtra.uni
-- 
2.7.0.windows.1

[Patch 2/2] UefiCpuPkg/CpuFeaturesPei.inf: Add missed Depex Ppi.

[Eric Dong]

The gEfiPeiMpServicesPpiGuid Ppi is used by CpuFeaturesPei.inf
during feature detection. Should be included in the [DepEx]
section but actually not. This patch add it.

Cc: Liming Gao <ruiyu.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
---
 UefiCpuPkg/CpuFeatures/CpuFeaturesPei.inf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/UefiCpuPkg/CpuFeatures/CpuFeaturesPei.inf b/UefiCpuPkg/CpuFeatures/CpuFeaturesPei.inf
index e617c5b..ad3bab3 100644
--- a/UefiCpuPkg/CpuFeatures/CpuFeaturesPei.inf
+++ b/UefiCpuPkg/CpuFeatures/CpuFeaturesPei.inf
@@ -44,7 +44,7 @@
   gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesInitOnS3Resume          ## CONSUMES
 
 [Depex]
-  TRUE
+  gEfiPeiMpServicesPpiGuid
 
 [UserExtensions.TianoCore."ExtraFiles"]
   CpuFeaturesPeiExtra.uni
-- 
2.7.0.windows.1

Re: [Patch 0/2] Add missed Depex Protocol/Ppi.

[Laszlo Ersek]

Hi Eric,

On 11/06/17 13:26, Eric Dong wrote:
> The Protocol/Ppi used in the drivers but the it not add them
> in the dependence section, it may cause driver assert. 
> This patch series add the missed Protocol/Ppi.
> 
> Eric Dong (2):
>   UefiCpuPkg/CpuFeaturesDxe.inf: Add missed Depex protocol.
>   UefiCpuPkg/CpuFeaturesPei.inf: Add missed Depex Ppi.
> 
>  UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.inf | 2 +-
>  UefiCpuPkg/CpuFeatures/CpuFeaturesPei.inf | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 

(hopefully I'm commenting soon enough,)

where exactly are the MP protocol and PPI used in UefiCpuPkg/CpuFeatures/?

I grepped the directory for both protocol and PPI, and I found no matches.

If the dependencies are incurred via library instances, then those
library instances should spell out the depex.

The only library class used by CpuFeaturesDxe.inf and
CpuFeaturesPei.inf, for which multi-processing looks remotely relevant,
is: RegisterCpuFeaturesLib.

For this class, two instances appear to exist:

UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.inf
UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.inf

The DXE instance consumes gEfiMpServiceProtocolGuid alright, but it also
spells out the protocol in the depex:

[Depex]
  gEfiMpServiceProtocolGuid AND gEdkiiCpuFeaturesSetDoneGuid

The PEI instance is similar, wrt. the PPI:

[Depex]
  gEfiPeiMpServicesPpiGuid AND gEdkiiCpuFeaturesSetDoneGuid

So, this series appears unnecessary. What am I missing?

Thanks
Laszlo

Re: [Patch 0/2] Add missed Depex Protocol/Ppi.

[Dong, Eric]

Hi Laszlo,

Agree with you, I'm not aware the consume protocol/Ppi code is in the library till now. I will discard the changes.

Thanks,
Eric
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Wednesday, November 8, 2017 2:18 AM
> To: Dong, Eric <eric.dong@intel.com>
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [Patch 0/2] Add missed Depex Protocol/Ppi.
> 
> Hi Eric,
> 
> On 11/06/17 13:26, Eric Dong wrote:
> > The Protocol/Ppi used in the drivers but the it not add them in the
> > dependence section, it may cause driver assert.
> > This patch series add the missed Protocol/Ppi.
> >
> > Eric Dong (2):
> >   UefiCpuPkg/CpuFeaturesDxe.inf: Add missed Depex protocol.
> >   UefiCpuPkg/CpuFeaturesPei.inf: Add missed Depex Ppi.
> >
> >  UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.inf | 2 +-
> > UefiCpuPkg/CpuFeatures/CpuFeaturesPei.inf | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> 
> (hopefully I'm commenting soon enough,)
> 
> where exactly are the MP protocol and PPI used in
> UefiCpuPkg/CpuFeatures/?
> 
> I grepped the directory for both protocol and PPI, and I found no matches.
> 
> If the dependencies are incurred via library instances, then those library
> instances should spell out the depex.
> 
> The only library class used by CpuFeaturesDxe.inf and CpuFeaturesPei.inf,
> for which multi-processing looks remotely relevant,
> is: RegisterCpuFeaturesLib.
> 
> For this class, two instances appear to exist:
> 
> UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.inf
> UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.inf
> 
> The DXE instance consumes gEfiMpServiceProtocolGuid alright, but it also
> spells out the protocol in the depex:
> 
> [Depex]
>   gEfiMpServiceProtocolGuid AND gEdkiiCpuFeaturesSetDoneGuid
> 
> The PEI instance is similar, wrt. the PPI:
> 
> [Depex]
>   gEfiPeiMpServicesPpiGuid AND gEdkiiCpuFeaturesSetDoneGuid
> 
> So, this series appears unnecessary. What am I missing?
> 
> Thanks
> Laszlo