On 08/02/16 10:59, Jeff Fan wrote:
> PiSmmCpuDxeSmm driver's entry point will get some PCDs supported dynamic type.
> In case those PCDs are set as DynamicHii type in platform DSC File, it implies
> that EFI Variable Arch protocol is required.
> 
> This fix is to add gEfiVariableArchProtocolGuid dependency on PiSmmCpuDxeSmm
> driver to make sure those DynamicHii PCDs could be read correctly.
> 
> Cc: Michael Kinney <michael.d.kin...@intel.com>
> Cc: Feng Tian <feng.t...@intel.com>
> Cc: Giri P Mudusuru <giri.p.mudus...@intel.com>
> Cc: Laszlo Ersek <ler...@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan <jeff....@intel.com>
> Reviewed-by: Giri P Mudusuru <giri.p.mudus...@intel.com>
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf 
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> index d7e6e07..83e3c55 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> @@ -4,7 +4,7 @@
>  # This SMM driver performs SMM initialization, deploy SMM Entry Vector,
>  # provides CPU specific services in SMM.
>  #
> -# Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
>  #
>  # This program and the accompanying materials
>  # are licensed and made available under the terms and conditions of the BSD 
> License
> @@ -155,7 +155,7 @@ [Pcd]
>    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode                      ## 
> CONSUMES
>  
>  [Depex]
> -  gEfiMpServiceProtocolGuid
> +  gEfiMpServiceProtocolGuid AND gEfiVariableArchProtocolGuid
>  
>  [UserExtensions.TianoCore."ExtraFiles"]
>    PiSmmCpuDxeSmmExtra.uni
> 

This patch (commit 7503cd70fb86) breaks OVMF. From the OVMF log:

Loading SMM driver at 0x0007FFDA000 EntryPoint=0x0007FFDA271 FvbServicesSmm.efi
QEMU Flash: Attempting flash detection at FFE00010
QemuFlashDetected => FD behaves as ROM
QemuFlashDetected => No
ASSERT OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c(248): 
!_gPcd_FixedAtBuild_PcdSmmSmramRequire

This symptom means that the SMM build of the QemuFlashFvbServicesRuntimeDxe 
driver is not actually running in SMM when it tries to access the pflash chip 
at startup. Therefore QEMU prevents the access, and then OVMF gives up.

Here's the bisection log:

git bisect start
# good: [de74668f5ea713b7e91e01318f0d15d2bf0effce] MdeModulePkg/PeiCore: Fix 
ConverSinglePpiPointer () typo.
git bisect good de74668f5ea713b7e91e01318f0d15d2bf0effce
# bad: [7822a1d91d53e80525f571183a24d54488f5437f] NetworkPkg/IpSecDxe: Fix 
wrong IKE header "FLAG" update
git bisect bad 7822a1d91d53e80525f571183a24d54488f5437f
# good: [8a2d564b2a811b8dbc85f90e14a67ae4ade21201] UefiCpuPkg/MpInitLib: Sort 
processor by ascending order of APIC ID
git bisect good 8a2d564b2a811b8dbc85f90e14a67ae4ade21201
# good: [7fadaacd50d716e8e054a94c20db56cca98e961e] UefiCpuPkg/CpuDxe: Consume 
MpInitLib to produce CPU MP Protocol services
git bisect good 7fadaacd50d716e8e054a94c20db56cca98e961e
# bad: [a012df5ec643a0c08c2b723a02919a5c9373ca74] PcAtChipsetPkg AcpiTimerLib: 
Wait 363 ACPI timer counts to get TSC Freq
git bisect bad a012df5ec643a0c08c2b723a02919a5c9373ca74
# good: [51d4779d7bfb74bfdbb0e196846de78567127348] MdePkg/MpService.h: Fixed 
typo in function header to match PI spec
git bisect good 51d4779d7bfb74bfdbb0e196846de78567127348
# good: [f3b91fa04adea2389c5a6d0fbd9a584d149bae09] UefiCpuPkg/CpuDxe: Fixed 
typo in function header to match PI spec
git bisect good f3b91fa04adea2389c5a6d0fbd9a584d149bae09
# bad: [7503cd70fb864a5663edb121c9b2488b4c69e7f5] UefiCpuPkg/PiSmmCpuDxeSmm: 
Add gEfiVariableArchProtocolGuid dependency
git bisect bad 7503cd70fb864a5663edb121c9b2488b4c69e7f5
# first bad commit: [7503cd70fb864a5663edb121c9b2488b4c69e7f5] 
UefiCpuPkg/PiSmmCpuDxeSmm: Add gEfiVariableArchProtocolGuid dependency

I see that this patch appeared in the final, v5 version of the series, as the 
last patch in the series. I never got around testing v5. I request that we 
please revert it for now, and then we investigate the problem.

>From the time we worked on SMM enablement for OVMF, I recall that pulling in 
>the SMM variable driver stack was tricky. The three layers (Firmware Volume 
>Block (2), Fault Tolerant Write, and Variable) have apparent / implicit 
>dependencies between them,  but these are not actually expressed in DepExes. I 
>don't know / don't remember why that is the case, but I recall that I had to 
>fiddle with their inclusion order in the FDF files. Following the  inclusion 
>order of the preexistent, SMM-less variable driver stack made it all work, if 
>I remember correctly.

I don't know why those depexes are not spelled out explicitly in those drivers, 
but at this point I think that the new DepEx on PiSmmCpuDxeSmm.inf upsets the 
dispatch order, and things break. I fully agree that this should hopefully be 
fixed by spelling out all DepExes explicitly everywhere, and I can also imagine 
it is actually a bug in OVMF somewhere, but for now, until we figure it out, 
can we please revert the patch?

The commit carries Mike's Tested-by -- I wonder what the differences are 
between Mike's test platform and OVMF.

Thank you,
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to