On 08/18/16 22:57, Zeng, Star wrote:
On 2016/8/19 10:45, Zeng, Star wrote:
On 2016/8/19 10:26, Laszlo Ersek wrote:
On 08/19/16 04:00, Fan, Jeff wrote:
Laszlo,
I could revert this patch firstly.
Thank you, that would be very kind.
Could you please dig out the OVMF to address the potential issue,
then I could re-commit it for those platforms required this patch.
The problem is that this week (what remains of it) and the next week
I won't really have time for this -- tomorrow I'm hoping to finish
up something else in a mortal hurry. It was actually in preparation
for rebasing / pushing that work that I pulled "master", and found
out about the regression.
Can we perhaps get help from the variable stack maintainers? What's
the design of the (lack of) depexes on those drivers?
Variable driver consumes
PcdFlashNvStorageVariableBase(64)/PcdFlashNvStorageVariableSize to
produce *gEfiVariableArchProtocolGuid* protocol. Variable driver
registers (SMM)FTW protocol notification function
SmmFtwNotificationEvent() or FtwNotificationEvent() to produce
*gEfiVariableWriteArchProtocolGuid* protocol. (SMM)FTW driver has
dependency on gEfiSmmFirmwareVolumeBlockProtocolGuid or
gEfiFirmwareVolumeBlockProtocolGuid.
I am not so sure what you said about the (lack of) depexes on those
drivers.
Did you see variable driver loaded and started when ASSERT appeared
on OVMF?
You may could compare the serial logs to get if there is some driver
execution flow differences for the images built without and with this
patch.
The only difference is that in the working case, PiSmmCpuDxeSmm.efi is
dispatched immediately before FvbServicesSmm.efi, while in the broken
case, PiSmmCpuDxeSmm.efi is not dispatched (it is delayed due to the new
depex) and FvbServicesSmm.efi is dispatched without PiSmmCpuDxeSmm.efi
being present, and it breaks.
I see that PiSmmCpuDxeSmm.efi installs:
- gEfiSmmConfigurationProtocolGuid,
- EfiSmmCpuProtocol,
- EfiSmmCpuServiceProtocol,
so it might not be hard to add a depex on one of those (DXE/SMM)
protocols to FvbServicesSmm.efi. In particular, the PI 1.4a spec says in
Volume 4, "1.6.1 SMM Drivers":
the dependency expression in the file section EFI_SECTION_SMM_DEPEX
[...] can refer to both UEFI and SMM protocols
so we could easily make FvbServicesSmm.efi dependent on either of those
protocols, I think.
However, is that the official way to delay the entry point function of a
DXE_SMM_DRIVER module until the code would actually run in SMM? Section
"1.7 SMM Driver Initialization" says:
An SMM Driver's initialization phase begins when the driver has been
loaded into SMRAM and its entry point is called. An SMM Driver's
initialization phase ends when the entry point returns.
During SMM Driver initialization, SMM Drivers have access to two
sets of protocols: UEFI and SMM. UEFI protocols are those which are
installed and discovered using the UEFI Boot Services. UEFI
protocols can be located and used by SMM drivers only during SMM
Initialization. SMM protocols are those which are installed and
discovered using the System Management Services Table (SMST). SMM
protocols can be discovered by SMM drivers during initialization
time and accessed while inside of SMM.
These paragraphs seem to imply that the processor will execute the entry
point function of a DXE_SMM_DRIVER module from SMRAM, without the
processor necessarily being in SMM just yet. (Which further implies that
SMRAM will not have been closed and locked at that point, but that's a
side remark only.)
This seems reasonable to me, but in the entry point of OVMF's
FvbServicesSmm.efi, we specifically need to test a write access to the
pflash chip, to make sure that the QEMU configuration is suitable and
secure. For this, FvbServicesSmm.efi must be dispatched in SMM. And,
apparently, in the current tree, when PiSmmCpuDxe launches first,
FvbServicesSmm *is* dispatched in SMM.
I find it quite non-intuitive that a DXE_SMM_DRIVER's entry point may or
may not be executed in SMM, but more importantly, what's the best way to
delay the driver dispatch until after PiSmmCpuDxe has been dispatched?
The PiSmmIpl driver (a DXE_RUNTIME_DRIVER) produces the
EFI_SMM_BASE2_PROTOCOL, which has a member function called InSmm().
However:
- according to the documentation, it doesn't test for SMM, it tests for
being executed from SMRAM (which are two different things!), and it
only really makes sense for SMM/DXE combined drivers,
- in fact I don't need a query-like function here, for the code, but
likely a new depex for FvbServicesSmm that delays its dispatch until
after PiSmmCpuDxe.
Actually, I think the simplest way to solve this in OVMF is to add a
depex on EFI_SMM_CPU_PROTOCOL to FvbServicesSmm. From the spec:
Provides access to CPU-related information while in SMM.
[...]
This protocol allows SMM drivers to access architecture-standard
registers from any of the CPU save state areas. [...]
I think a DXE_SMM_DRIVER might perfectly want to use this protocol in
its entry point function (in the general case, for whatever reason), so
it seems very suitable for delaying FvbServicesSmm.
For example, in
"QuarkPlatformPkg/Acpi/DxeSmm/SmmPowerManagement/SmmPowerManagement.inf",
I think we see the same pattern:
- MODULE_TYPE = DXE_SMM_DRIVER
- Depex on gEfiSmmCpuProtocolGuid
.... Okay, I tried to test this patch (in combination with re-adding the
gEfiVariableArchProtocolGuid dependency to PiSmmCpuDxeSmm):
diff --git
a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesSmm.inf
b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesSmm.inf
index ba2d3679a46d..a241f7702ca2 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesSmm.inf
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesSmm.inf
@@ -88,4 +88,4 @@ [FeaturePcd]
gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
[Depex]
- TRUE
+ gEfiSmmCpuProtocolGuid
With this patch, in the build report I get:
Module Name: FvbServicesSmm
Module INF Path:
OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesSmm.inf
[...]
----------------------------------------------------------------------------------------------------------------------<
Final Dependency Expression (DEPEX) Instructions
PUSH gEfiSmmCpuProtocolGuid
PUSH gEfiPcdProtocolGuid
PUSH gEfiSmmBase2ProtocolGuid
PUSH gEfiSmmAccess2ProtocolGuid
PUSH gEfiDevicePathUtilitiesProtocolGuid
AND
AND
AND
AND
END
------------------------------------------------------------------------------------------------------------------------
Dependency Expression (DEPEX) from INF
(gEfiSmmCpuProtocolGuid) AND (gEfiPcdProtocolGuid) AND
(gEfiSmmBase2ProtocolGuid) AND (gEfiSmmAccess2ProtocolGuid) AND
(gEfiDevicePathUtilitiesProtocolGuid)
------------------------------------------------------------------------------------------------------------------------
From Module INF: gEfiSmmCpuProtocolGuid
From Library INF: (gEfiPcdProtocolGuid) AND
(gEfiSmmBase2ProtocolGuid) AND (gEfiSmmAccess2ProtocolGuid) AND
(gEfiDevicePathUtilitiesProtocolGuid)
<---------------------------------------------------------------------------------------------------------------------->
and FvbServicesSmm *is* appropriately delayed. However, the variable
driver blows up:
Loading SMM driver at 0x0007FF7D000 EntryPoint=0x0007FF7D271
VariableSmm.efi
mSmmMemLibInternalMaximumSupportAddress = 0xFFFFFFFFF
VarCheckLibRegisterSetVariableCheckHandler - 0x7FF89334 Success
Firmware Volume for Variable Store is corrupted
ASSERT_EFI_ERROR (Status = Volume Corrupt)
ASSERT MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c(927):
!EFI_ERROR (Status)
This is the call tree that fails:
VariableServiceInitialize()
[MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c]
VariableCommonInitialize()
[MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c]
InitNonVolatileVariableStore()
[MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c]
However, the variable store is definitely not corrupted; this failure
reproduces with the pristine "OVMF_VARS.fd" varstore template that falls
right out of the OVMF build.
