From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: philmd@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Wed, 25 Sep 2019 09:02:05 -0700 Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9089512A2 for ; Wed, 25 Sep 2019 16:02:04 +0000 (UTC) Received: by mail-wr1-f71.google.com with SMTP id z1so2568839wrw.21 for ; Wed, 25 Sep 2019 09:02:04 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:openpgp:message-id :date:user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=2J2c0DUU74x1uX7lubBZiaWnME0aaVInMrkBE4HdhpM=; b=EI7EtzMDstwx2WjqN6+IEe+axxbNO6dpxZ3LKgjeYXUbK5tjpb2KGiXC7pP7Th8NCM NF+X/I5G8IpmVCswFZs0YiyusI2zBjiiu2Z5vucLJav7cWhCIsrACLlVK//TmBsn0MMO AoOR14t26QtSeSGqbbRtBftZIcNBgWx/xa87dI+bCtFTEUeaBn7DIGAvintcTO7O76IQ mz1BmZDeGbXBoyWR5mgT4TJRYEzXmkqtgNplBXmofrlWJUV5wRPnbGKvZZVv8z5WIK4k adxqSPQAL8rnZN0NXk9d4YWoiBcRnbuSeCgKfINlhbu2wsx+BC0xBSgkftwUCOSy63Yk XCVg== X-Gm-Message-State: APjAAAXttqAP7krVFTX/5gA18vl4zLyNiYxZSI6C0XLZuqgdPO+JyNzG g2WB2YEfX4bBNHhTd1pvjLsM5FuKaQir/P4TUrECH+coFOCx/p5uFq+AMW3kg51hvA3PH0IeGql +jAN75NzxrL/TPQ== X-Received: by 2002:a1c:3b06:: with SMTP id i6mr8484493wma.6.1569427322937; Wed, 25 Sep 2019 09:02:02 -0700 (PDT) X-Google-Smtp-Source: APXvYqw6qjOKaIYizC6A3SN/vcSfM4piMr1T/LJb0dArrQMblzwd4paojRTqMVflvu8DeUb+fZb8pQ== X-Received: by 2002:a1c:3b06:: with SMTP id i6mr8484457wma.6.1569427322662; Wed, 25 Sep 2019 09:02:02 -0700 (PDT) Received: from [10.201.33.84] ([195.166.127.210]) by smtp.gmail.com with ESMTPSA id x5sm6493059wrt.75.2019.09.25.09.02.01 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 25 Sep 2019 09:02:01 -0700 (PDT) Subject: Re: [edk2-devel] [PATCH 09/35] MdeModulePkg: stop abusing EFI_EVENT for protocol notify registration To: devel@edk2.groups.io, lersek@redhat.com Cc: Hao A Wu , Jian J Wang , Liming Gao , Ray Ni , Zhichao Gao References: <20190917194935.24322-1-lersek@redhat.com> <20190917194935.24322-10-lersek@redhat.com> From: =?UTF-8?B?UGhpbGlwcGUgTWF0aGlldS1EYXVkw6k=?= Openpgp: id=89C1E78F601EE86C867495CBA2A3FD6EDEADC0DE; url=http://pgp.mit.edu/pks/lookup?op=get&search=0xA2A3FD6EDEADC0DE Message-ID: <44c7d155-647f-ce11-d006-8071b97d6e8a@redhat.com> Date: Wed, 25 Sep 2019 18:02:00 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.0 MIME-Version: 1.0 In-Reply-To: <20190917194935.24322-10-lersek@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 9/17/19 9:49 PM, Laszlo Ersek wrote: > EfiCreateProtocolNotifyEvent() takes a (VOID**) for "Registration", > similarly to gBS->RegisterProtocolNotify(). We should pass the address of > an actual pointer-to-VOID, and not the address of an EFI_EVENT. EFI_EVENT > just happens to be specified as (VOID*), and has nothing to do with the > registration. > > The same applies to gMmst->MmRegisterProtocolNotify(). > > "mFtwRegistration", "mFvRegistration", and "mFvbRegistration" are used for > nothing else. > > This change is a no-op in practice; it's a semantic improvement. > > Cc: Hao A Wu > Cc: Jian J Wang > Cc: Liming Gao > Cc: Ray Ni > Cc: Zhichao Gao > Signed-off-by: Laszlo Ersek > --- > > Notes: > lightly tested, as these modules (except LoadFileOnFv2) are part of the > ArmVirt and/or OVMF platforms > > MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.c | 2 +- > MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c | 2 +- > MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c | 2 +- > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c | 2 +- > 4 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.c b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.c > index ae8f117905cd..de38ea028af1 100644 > --- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.c > +++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.c > @@ -47,7 +47,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > #include > #include "FaultTolerantWrite.h" > -EFI_EVENT mFvbRegistration = NULL; > +VOID *mFvbRegistration = NULL; > > > /** > diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c > index e8e935a85b5b..9612b394865b 100644 > --- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c > +++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c > @@ -56,7 +56,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > #include "FaultTolerantWriteSmmCommon.h" > #include > > -EFI_EVENT mFvbRegistration = NULL; > +VOID *mFvbRegistration = NULL; > EFI_FTW_DEVICE *mFtwDevice = NULL; > > /// > diff --git a/MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c b/MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c > index 4af2da05e145..43fa6ce12875 100644 > --- a/MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c > +++ b/MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c > @@ -36,7 +36,7 @@ typedef struct { > #define LOAD_FILE_ON_FV2_PRIVATE_DATA_FROM_THIS(a) CR (a, LOAD_FILE_ON_FV2_PRIVATE_DATA, LoadFile, LOAD_FILE_ON_FV2_PRIVATE_DATA_SIGNATURE) > #define LOAD_FILE_ON_FV2_PRIVATE_DATA_FROM_LINK(a) CR (a, LOAD_FILE_ON_FV2_PRIVATE_DATA, Link, LOAD_FILE_ON_FV2_PRIVATE_DATA_SIGNATURE) > > -EFI_EVENT mFvRegistration; > +VOID *mFvRegistration; > LIST_ENTRY mPrivateDataList; > > /** > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c > index 3d232bb36cb4..7d2b6c8e1fad 100644 > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c > @@ -13,7 +13,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > EFI_HANDLE mHandle = NULL; > EFI_EVENT mVirtualAddressChangeEvent = NULL; > -EFI_EVENT mFtwRegistration = NULL; > +VOID *mFtwRegistration = NULL; > VOID ***mVarCheckAddressPointer = NULL; > UINTN mVarCheckAddressPointerCount = 0; > EDKII_VARIABLE_LOCK_PROTOCOL mVariableLock = { VariableLockRequestToLock }; Surprisingly the one declared in MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c is correct :) Reviewed-by: Philippe Mathieu-Daude