Ard:
Thanks for your fix. Reviewed-by: Liming Gao <gaolim...@byosoft.com.cn>
Can you include BZ into the commit message? BZ can mention the correct
conversion solution. Other modules should follow the same way to convert FVB
protocol VA.
Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Ard
> Biesheuvel
> 发送时间: 2021年3月13日 7:06
> 收件人: devel@edk2.groups.io
> 抄送: ler...@redhat.com; liming....@intel.com; j...@solid-run.com;
> l...@nuviainc.com; Ard Biesheuvel <a...@kernel.org>
> 主题: [edk2-devel] [PATCH 1/1] MdeModulePkg/VariableRuntimeDxe: avoid
> double VA conversion of FVB protocol
>
> For historical reasons, the VariableRuntimeDxe performs virtual address
> conversion on the FVB protocol member pointers of the protocol instance
> that backs the EFI variable store. However, the driver that produces the
> actual instance should be doing this, as it is the owner, and provides
> the actual implementation of those methods.
>
> Unfortunately, we cannot simply change this: existing drivers may rely
> on this behavior, and so the variable driver should take care to only
> convert the pointers when necessary, but leave them alone when the owner
> is taking care of this.
>
> The virtual address switch event can be delivered in arbitrary order, and
> so we should take care of this in a way that does not rely on whether this
> driver converts its pointers either before or after the owner of the FVB
> protocol receives the event.
>
> So let's not convert the pointers at all when the event is delivered, but
> record the converted addresses in a shadow FVB protocol. This way, we can
> check when the variable driver is invoked at runtime whether the switch
> has occurred or not, and perform the switch if it hasn't.
>
> Signed-off-by: Ard Biesheuvel <a...@kernel.org>
> ---
> Build tested only.
>
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c | 50
> +++++++++++++++++---
> 1 file changed, 43 insertions(+), 7 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> index 0fca0bb2a9b5..3d83ac4452ee 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> @@ -37,6 +37,8 @@ EDKII_VAR_CHECK_PROTOCOL
> mVarCheck = { VarCheckRegis
>
> VarCheckVariablePropertySet,
>
> VarCheckVariablePropertyGet };
>
> +STATIC EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL mFvbProtocolShadow;
> +
> /**
> Some Secure Boot Policy Variable may update following other variable
> changes(SecureBoot follows PK change, etc).
> Record their initial State when variable write service is ready.
> @@ -105,8 +107,26 @@ AcquireLockOnlyAtBootTime (
> IN EFI_LOCK *Lock
> )
> {
> + EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *FvbInstance;
> +
> if (!AtRuntime ()) {
> EfiAcquireLock (Lock);
> + } else {
> + //
> + // Check whether we need to swap in the converted pointer values for
> the
> + // FVB protocol methods
> + //
> + FvbInstance = mVariableModuleGlobal->FvbInstance;
> + if (FvbInstance != NULL &&
> + FvbInstance->GetBlockSize != mFvbProtocolShadow.GetBlockSize)
> {
> + FvbInstance->GetBlockSize =
> mFvbProtocolShadow.GetBlockSize;
> + FvbInstance->GetPhysicalAddress =
> mFvbProtocolShadow.GetPhysicalAddress;
> + FvbInstance->GetAttributes =
> mFvbProtocolShadow.GetAttributes;
> + FvbInstance->SetAttributes =
> mFvbProtocolShadow.SetAttributes;
> + FvbInstance->Read = mFvbProtocolShadow.Read;
> + FvbInstance->Write = mFvbProtocolShadow.Write;
> + FvbInstance->EraseBlocks =
> mFvbProtocolShadow.EraseBlocks;
> + }
> }
> }
>
> @@ -247,13 +267,22 @@ VariableClassAddressChangeEvent (
> UINTN Index;
>
> if (mVariableModuleGlobal->FvbInstance != NULL) {
> - EfiConvertPointer (0x0, (VOID **)
> &mVariableModuleGlobal->FvbInstance->GetBlockSize);
> - EfiConvertPointer (0x0, (VOID **)
> &mVariableModuleGlobal->FvbInstance->GetPhysicalAddress);
> - EfiConvertPointer (0x0, (VOID **)
> &mVariableModuleGlobal->FvbInstance->GetAttributes);
> - EfiConvertPointer (0x0, (VOID **)
> &mVariableModuleGlobal->FvbInstance->SetAttributes);
> - EfiConvertPointer (0x0, (VOID **)
> &mVariableModuleGlobal->FvbInstance->Read);
> - EfiConvertPointer (0x0, (VOID **)
> &mVariableModuleGlobal->FvbInstance->Write);
> - EfiConvertPointer (0x0, (VOID **)
> &mVariableModuleGlobal->FvbInstance->EraseBlocks);
> + //
> + // This module did not produce the FVB protocol instance that
provides
> the
> + // variable store, so it should not be the one performing the pointer
> + // conversion on its members. However, some FVB protocol
> implementations
> + // may rely on this behavior, which was present in older versions of
this
> + // driver, so we need to perform the conversion if the protocol
producer
> + // fails to do so. So let's record the converted values now, and swap
> them
> + // in later if they haven't changed values.
> + //
> + EfiConvertPointer (0x0, (VOID **)&mFvbProtocolShadow.GetBlockSize);
> + EfiConvertPointer (0x0, (VOID
> **)&mFvbProtocolShadow.GetPhysicalAddress);
> + EfiConvertPointer (0x0, (VOID
> **)&mFvbProtocolShadow.GetAttributes);
> + EfiConvertPointer (0x0, (VOID **)&mFvbProtocolShadow.SetAttributes);
> + EfiConvertPointer (0x0, (VOID **)&mFvbProtocolShadow.Read);
> + EfiConvertPointer (0x0, (VOID **)&mFvbProtocolShadow.Write);
> + EfiConvertPointer (0x0, (VOID **)&mFvbProtocolShadow.EraseBlocks);
> EfiConvertPointer (0x0, (VOID **)
> &mVariableModuleGlobal->FvbInstance);
> }
> EfiConvertPointer (0x0, (VOID **)
> &mVariableModuleGlobal->PlatformLangCodes);
> @@ -454,6 +483,13 @@ FtwNotificationEvent (
> }
> mVariableModuleGlobal->FvbInstance = FvbProtocol;
>
> + //
> + // Store the boot time values of the function pointers so we can
compare
> + // them later. This is needed to avoid double conversion during the
call
> + // to SetVirtualAddressMap.
> + //
> + CopyMem (&mFvbProtocolShadow, FvbProtocol, sizeof
> mFvbProtocolShadow);
> +
> //
> // Mark the variable storage region of the FLASH as RUNTIME.
> //
> --
> 2.30.1
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#72734): https://edk2.groups.io/g/devel/message/72734
> Mute This Topic: https://groups.io/mt/81292874/4905953
> Group Owner: devel+ow...@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [gaolim...@byosoft.com.cn]
> -=-=-=-=-=-=
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72752): https://edk2.groups.io/g/devel/message/72752
Mute This Topic: https://groups.io/mt/81340892/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
