From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.byosoft.com.cn (mail.byosoft.com.cn [58.240.74.242]) by mx.groups.io with SMTP id smtpd.web12.3668.1615776056793144636 for ; Sun, 14 Mar 2021 19:40:58 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: byosoft.com.cn, ip: 58.240.74.242, mailfrom: gaoliming@byosoft.com.cn) Received: from DESKTOPS6D0PVI ([58.246.60.130]) (envelope-sender ) by 192.168.6.13 with ESMTP for ; Mon, 15 Mar 2021 10:40:54 +0800 X-WM-Sender: gaoliming@byosoft.com.cn X-Originating-IP: 58.246.60.130 X-WM-AuthFlag: YES X-WM-AuthUser: gaoliming@byosoft.com.cn From: "gaoliming" To: , Cc: , , , References: <20210312230554.292964-1-ardb@kernel.org> In-Reply-To: <20210312230554.292964-1-ardb@kernel.org> Subject: =?UTF-8?B?5Zue5aSNOiBbZWRrMi1kZXZlbF0gW1BBVENIIDEvMV0gTWRlTW9kdWxlUGtnL1ZhcmlhYmxlUnVudGltZUR4ZTogYXZvaWQgZG91YmxlIFZBIGNvbnZlcnNpb24gb2YgRlZCIHByb3RvY29s?= Date: Mon, 15 Mar 2021 10:40:53 +0800 Message-ID: <008c01d71944$9e971150$dbc533f0$@byosoft.com.cn> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQF0h9jvF1uStj3iSnT3AW56YAZKmKtJeHWQ Content-Type: text/plain; charset="gb2312" Content-Transfer-Encoding: quoted-printable Content-Language: zh-cn Ard: Thanks for your fix. Reviewed-by: Liming Gao = 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.=20 Thanks Liming > -----=D3=CA=BC=FE=D4=AD=BC=FE----- > =B7=A2=BC=FE=C8=CB: devel@edk2.groups.io = =B4=FA=B1=ED Ard > Biesheuvel > =B7=A2=CB=CD=CA=B1=BC=E4: 2021=C4=EA3=D4=C213=C8=D5 7:06 > =CA=D5=BC=FE=C8=CB: devel@edk2.groups.io > =B3=AD=CB=CD: lersek@redhat.com; liming.gao@intel.com; = jon@solid-run.com; > leif@nuviainc.com; Ard Biesheuvel > =D6=F7=CC=E2: [edk2-devel] [PATCH 1/1] = MdeModulePkg/VariableRuntimeDxe: avoid > double VA conversion of FVB protocol >=20 > 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. >=20 > 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. >=20 > 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. >=20 > 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. >=20 > Signed-off-by: Ard Biesheuvel > --- > Build tested only. >=20 > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c | 50 > +++++++++++++++++--- > 1 file changed, 43 insertions(+), 7 deletions(-) >=20 > 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 =3D { VarCheckRegis >=20 > VarCheckVariablePropertySet, >=20 > VarCheckVariablePropertyGet }; >=20 > +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 =3D mVariableModuleGlobal->FvbInstance; > + if (FvbInstance !=3D NULL && > + FvbInstance->GetBlockSize !=3D = mFvbProtocolShadow.GetBlockSize) > { > + FvbInstance->GetBlockSize =3D > mFvbProtocolShadow.GetBlockSize; > + FvbInstance->GetPhysicalAddress =3D > mFvbProtocolShadow.GetPhysicalAddress; > + FvbInstance->GetAttributes =3D > mFvbProtocolShadow.GetAttributes; > + FvbInstance->SetAttributes =3D > mFvbProtocolShadow.SetAttributes; > + FvbInstance->Read =3D mFvbProtocolShadow.Read; > + FvbInstance->Write =3D mFvbProtocolShadow.Write; > + FvbInstance->EraseBlocks =3D > mFvbProtocolShadow.EraseBlocks; > + } > } > } >=20 > @@ -247,13 +267,22 @@ VariableClassAddressChangeEvent ( > UINTN Index; >=20 > if (mVariableModuleGlobal->FvbInstance !=3D 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 =3D FvbProtocol; >=20 > + // > + // 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 >=20 >=20 >=20 > -=3D-=3D-=3D-=3D-=3D-=3D > 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+owner@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub > [gaoliming@byosoft.com.cn] > -=3D-=3D-=3D-=3D-=3D-=3D >=20