From: "Zeng, Star" <star.zeng@intel.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>, edk2-devel@lists.01.org
Cc: Hao Wu <hao.a.wu@intel.com>, Liming Gao <liming.gao@intel.com>,
Michael D Kinney <michael.d.kinney@intel.com>,
Laszlo Ersek <lersek@redhat.com>,
star.zeng@intel.com
Subject: Re: [PATCH v2 07/17] MdeModulePkg/VariableRuntimeDxe: factor out boot service accesses
Date: Wed, 16 Jan 2019 12:57:43 +0800 [thread overview]
Message-ID: <69caeeee-827e-c0a2-9563-852240621f47@intel.com> (raw)
In-Reply-To: <20190114132758.24054-8-ard.biesheuvel@linaro.org>
Some minor comments.
With them handled, Reviewed-by: Star Zeng <star.zeng@intel.com>
On 2019/1/14 21:27, Ard Biesheuvel wrote:
> In preparation of providing a standalone MM based variable runtime
> driver, move the existing SMM driver to the new MM services table,
> and factor out some pieces that are specific to the traditional
> driver, mainly related to the use of UEFI boot services, which are
> not accessible to standalone MM drivers.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
> ---
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf | 5 +-
> MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h | 58 +++++++++
> MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c | 18 +--
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c | 59 +++------
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableTraditionalMm.c | 130 ++++++++++++++++++++
> 5 files changed, 211 insertions(+), 59 deletions(-)
>
[Trimmed]
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h b/MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h
> index 7af22a4ad671..eb84589fc404 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h
> @@ -97,4 +97,62 @@ VariableSpeculationBarrier (
> VOID
> );
>
> +/**
> + Notify the system that the SMM variable driver is ready
> +**/
> +VOID
> +VariableNotifySmmReady (
> + VOID
> + );
> +
> +/**
> + Notify the system that the SMM variable write driver is ready
> +**/
> +VOID
> +VariableNotifySmmWriteReady (
> + VOID
> + );
> +
> +/**
> + Variable service MM driver entry point
The block should be matched with the MmVariableServiceInitialize
implementation in VariableSmm.c. But not SmmVariableServiceInitialize
implementation in VariableTraditionalMm.c
> +**/
> +EFI_STATUS
> +EFIAPI
> +MmVariableServiceInitialize (
> + VOID
> + );
> +
> +/**
> + This function checks if the buffer is valid per processor architecture and
> + does not overlap with SMRAM.
> +
> + @param Buffer The buffer start address to be checked.
> + @param Length The buffer length to be checked.
> +
> + @retval TRUE This buffer is valid per processor architecture and does not
> + overlap with SMRAM.
> + @retval FALSE This buffer is not valid per processor architecture or overlaps
> + with SMRAM.
> +**/
> +BOOLEAN
> +VariableSmmIsBufferOutsideSmmValid (
> + IN EFI_PHYSICAL_ADDRESS Buffer,
> + IN UINT64 Length
> + );
> +
> +/**
> + Whether the TCG or TCG2 protocols are installed in the UEFI protocol database.
> + This information is used by the MorLock code to infer whether an existing
> + MOR variable is legitimate or not.
> +
> + @retval TRUE Either the TCG or TCG2 protocol is installed in the UEFI
> + protocol database
> + @retval FALSE Neither the TCG nor the TCG2 protocol is installed in the UEFI
> + protocol database
> +**/
> +BOOLEAN
> +VariableHaveTcgProtocols (
> + VOID
> + );
> +
> #endif
[Trimmed]
> @@ -928,18 +920,11 @@ SmmFtwNotificationEvent (
> runtime services in the EFI System Table and installs arch protocols
> for variable read and write services being available. It also registers
> a notification function for an EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE event.
> -
> - @param[in] ImageHandle The firmware allocated handle for the EFI image.
> - @param[in] SystemTable A pointer to the EFI System Table.
> -
> - @retval EFI_SUCCESS Variable service successfully initialized.
This line needs to be kept.
> -
> **/
> EFI_STATUS
> EFIAPI
> -VariableServiceInitialize (
> - IN EFI_HANDLE ImageHandle,
> - IN EFI_SYSTEM_TABLE *SystemTable
> +MmVariableServiceInitialize (
> + VOID
> )
> {
> EFI_STATUS Status;
> @@ -957,7 +942,7 @@ VariableServiceInitialize (
> // Install the Smm Variable Protocol on a new handle.
> //
> VariableHandle = NULL;
> - Status = gSmst->SmmInstallProtocolInterface (
> + Status = gMmst->MmInstallProtocolInterface (
> &VariableHandle,
> &gEfiSmmVariableProtocolGuid,
> EFI_NATIVE_INTERFACE,
> @@ -965,7 +950,7 @@ VariableServiceInitialize (
> );
> ASSERT_EFI_ERROR (Status);
>
> - Status = gSmst->SmmInstallProtocolInterface (
> + Status = gMmst->MmInstallProtocolInterface (
> &VariableHandle,
> &gEdkiiSmmVarCheckProtocolGuid,
> EFI_NATIVE_INTERFACE,
> @@ -976,7 +961,7 @@ VariableServiceInitialize (
> mVariableBufferPayloadSize = GetMaxVariableSize () +
> OFFSET_OF (SMM_VARIABLE_COMMUNICATE_VAR_CHECK_VARIABLE_PROPERTY, Name) - GetVariableHeaderSize ();
>
> - Status = gSmst->SmmAllocatePool (
> + Status = gMmst->MmAllocatePool (
> EfiRuntimeServicesData,
> mVariableBufferPayloadSize,
> (VOID **)&mVariableBufferPayload
> @@ -987,25 +972,19 @@ VariableServiceInitialize (
> /// Register SMM variable SMI handler
> ///
> VariableHandle = NULL;
> - Status = gSmst->SmiHandlerRegister (SmmVariableHandler, &gEfiSmmVariableProtocolGuid, &VariableHandle);
> + Status = gMmst->MmiHandlerRegister (SmmVariableHandler, &gEfiSmmVariableProtocolGuid, &VariableHandle);
> ASSERT_EFI_ERROR (Status);
>
> //
> // Notify the variable wrapper driver the variable service is ready
> //
> - Status = SystemTable->BootServices->InstallProtocolInterface (
> - &mVariableHandle,
> - &gEfiSmmVariableProtocolGuid,
> - EFI_NATIVE_INTERFACE,
> - &gSmmVariable
> - );
> - ASSERT_EFI_ERROR (Status);
> + VariableNotifySmmReady ();
>
> //
> // Register EFI_SMM_END_OF_DXE_PROTOCOL_GUID notify function.
> //
> - Status = gSmst->SmmRegisterProtocolNotify (
> - &gEfiSmmEndOfDxeProtocolGuid,
> + Status = gMmst->MmRegisterProtocolNotify (
> + &gEfiMmEndOfDxeProtocolGuid,
> SmmEndOfDxeCallback,
> &SmmEndOfDxeRegistration
> );
> @@ -1014,7 +993,7 @@ VariableServiceInitialize (
> //
> // Register FtwNotificationEvent () notify function.
> //
> - Status = gSmst->SmmRegisterProtocolNotify (
> + Status = gMmst->MmRegisterProtocolNotify (
> &gEfiSmmFaultTolerantWriteProtocolGuid,
> SmmFtwNotificationEvent,
> &SmmFtwRegistration
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableTraditionalMm.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableTraditionalMm.c
> new file mode 100644
> index 000000000000..d702c8f8db67
> --- /dev/null
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableTraditionalMm.c
> @@ -0,0 +1,130 @@
[Trimmed]
> +
> +/**
> + Variable service MM driver entry point
The comments block should have description for the two parameters.
Thanks,
Star
> +**/
> +EFI_STATUS
> +EFIAPI
> +VariableServiceInitialize (
> + IN EFI_HANDLE ImageHandle,
> + IN EFI_SYSTEM_TABLE *SystemTable
> + )
> +{
> + return MmVariableServiceInitialize ();
> +}
> +
> +/**
> + Whether the TCG or TCG2 protocols are installed in the UEFI protocol database.
> + This information is used by the MorLock code to infer whether an existing
> + MOR variable is legitimate or not.
> +
> + @retval TRUE Either the TCG or TCG2 protocol is installed in the UEFI
> + protocol database
> + @retval FALSE Neither the TCG nor the TCG2 protocol is installed in the UEFI
> + protocol database
> +**/
> +BOOLEAN
> +VariableHaveTcgProtocols (
> + VOID
> + )
> +{
> + EFI_STATUS Status;
> + VOID *Interface;
> +
> + Status = gBS->LocateProtocol (
> + &gEfiTcg2ProtocolGuid,
> + NULL, // Registration
> + &Interface
> + );
> + if (!EFI_ERROR (Status)) {
> + return TRUE;
> + }
> +
> + Status = gBS->LocateProtocol (
> + &gEfiTcgProtocolGuid,
> + NULL, // Registration
> + &Interface
> + );
> + return !EFI_ERROR (Status);
> +}
>
next prev parent reply other threads:[~2019-01-16 4:58 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-14 13:27 [PATCH v2 00/17] implement standalone MM versions of the variable runtime drivers Ard Biesheuvel
2019-01-14 13:27 ` [PATCH v2 01/17] MdeModulePkg/MdeModulePkg.dsc: add MmServicesTableLib resolution Ard Biesheuvel
2019-01-16 5:02 ` Zeng, Star
2019-01-14 13:27 ` [PATCH v2 02/17] OvmfPkg: " Ard Biesheuvel
2019-01-14 16:40 ` Laszlo Ersek
2019-01-14 17:05 ` Ard Biesheuvel
2019-01-14 13:27 ` [PATCH v2 03/17] QuarkPlatformPkg: " Ard Biesheuvel
2019-01-16 15:14 ` Gao, Liming
2019-01-14 13:27 ` [PATCH v2 04/17] Vlv2TbltDevicePkg: " Ard Biesheuvel
2019-01-16 15:14 ` Gao, Liming
2019-01-16 17:44 ` Ard Biesheuvel
2019-01-16 19:00 ` Kinney, Michael D
2019-01-17 5:20 ` Qian, Yi
2019-01-14 13:27 ` [PATCH v2 05/17] MdeModulePkg/FaultTolerantWriteDxe: factor out boot service accesses Ard Biesheuvel
2019-01-16 3:29 ` Zeng, Star
2019-01-14 13:27 ` [PATCH v2 06/17] MdeModulePkg/FaultTolerantWriteDxe: implement standalone MM version Ard Biesheuvel
2019-01-16 3:32 ` Zeng, Star
2019-01-14 13:27 ` [PATCH v2 07/17] MdeModulePkg/VariableRuntimeDxe: factor out boot service accesses Ard Biesheuvel
2019-01-16 4:57 ` Zeng, Star [this message]
2019-01-14 13:27 ` [PATCH v2 08/17] MdeModulePkg/VariableRuntimeDxe: implement standalone MM version Ard Biesheuvel
2019-01-16 5:00 ` Zeng, Star
2019-01-14 13:27 ` [PATCH v2 09/17] MdePkg: introduce standalone MM entry point library class Ard Biesheuvel
2019-01-16 6:32 ` Zeng, Star
2019-01-16 14:56 ` Gao, Liming
2019-01-16 14:57 ` Ard Biesheuvel
2019-01-14 13:27 ` [PATCH v2 10/17] MdePkg: introduce standalone MM entry point library implementation Ard Biesheuvel
2019-01-14 13:27 ` [PATCH v2 11/17] MdePkg: add MM_STANDALONE implementation of MmServicesTableLib Ard Biesheuvel
2019-01-14 13:27 ` [PATCH v2 12/17] MdeModulePkg: implement NULL instance of HobLib library class Ard Biesheuvel
2019-01-16 5:10 ` Zeng, Star
2019-01-14 13:27 ` [PATCH v2 13/17] MdeModulePkg: implement NULL instance of MemoryAllocationLib " Ard Biesheuvel
2019-01-16 5:09 ` Zeng, Star
2019-01-14 13:27 ` [PATCH v2 14/17] MdeModulePkg/MdeModulePkg/dsc: move DxeDebugSupportDxe to x86 only section Ard Biesheuvel
2019-01-16 5:27 ` Zeng, Star
2019-01-14 13:27 ` [PATCH v2 15/17] MdeModulePkg/AuthVariableLibNull: add MM_STANDALONE support Ard Biesheuvel
2019-01-16 5:03 ` Zeng, Star
2019-01-14 13:27 ` [PATCH v2 16/17] MdeModulePkg/VarCheckLib: " Ard Biesheuvel
2019-01-16 5:03 ` Zeng, Star
2019-01-14 13:27 ` [PATCH v2 17/17] MdeModulePkg/MdeModulePkg.dsc: add MM_STANDALONE FTW and variable modules Ard Biesheuvel
2019-01-16 5:04 ` Zeng, Star
2019-01-15 8:25 ` [PATCH v2 00/17] implement standalone MM versions of the variable runtime drivers Wang, Jian J
2019-01-16 19:28 ` Ard Biesheuvel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=69caeeee-827e-c0a2-9563-852240621f47@intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox