From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM10-MW2-obe.outbound.protection.outlook.com (NAM10-MW2-obe.outbound.protection.outlook.com [40.107.94.122]) by mx.groups.io with SMTP id smtpd.web10.488.1591116947440662690 for ; Tue, 02 Jun 2020 09:55:47 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@microsoft.com header.s=selector2 header.b=GlFCiMA8; spf=pass (domain: microsoft.com, ip: 40.107.94.122, mailfrom: bret.barkelew@microsoft.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=NmTkshmThxXsIGuvPsRv7G3xlJ1RykMllwocZwdr9lT0XNqSsnPvrf7kcnbXMBrlYFuKD4l+bAZRp2eC/2MJbhYPUt+iVdRKZJO23ezpzfuMYcn8/qaGBUC4A/wjLYzGsWgBN1x88MLQ0YIL36swtC6fwAG6Z8DG2qoUDamZM4n/9Go0tj6GbMNhPtC8s9LgTxEPVEECytgWRsCWEmowKr8BpUITV2tMO97koyBpTHIxyhwy/ihiX7ix45Hv3h4aK32UC/hDnNEbOAYZwd9P0boDcPnf7CR2tD+llUxWNN554VVXqApvLayCu8tTisQyDZSKKO5A6FP9O4b5/pqH5w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=uY4/YWOBWIz6mCe+fYkvavQwhLcDl/IGzUWq63t1knc=; b=KOkK6OmFbAIXNL3blVakzjnUZu2gR0c9Abyw3A3mBYcqBbeoYdzNQ6APYKg29SIOHNst1IhZgdV/ofrUKMJKpohkG8XF+QudqMnYIh0fEv60JyMbTM9P/TvlZY7kqvZNvLlghNMmjXonzvR9LugLJB22W0WlJQUociUrfpru+dFEM7+VFB+ZlILtF63A0EctIdr6MK6980mnk/c05CFYGNHLLy0G6pLJhAI/j7Kpq5hRVJ9RW5KsykQ4KqcU738/4qEtHlEeBqTpSWDno4SOxbT5ma98n8xXX0TNlt7CUXysTDV6RBBs/qol/oatJ53Qq7MPhE6/40WbWsvxbIu3mw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=microsoft.com; dmarc=pass action=none header.from=microsoft.com; dkim=pass header.d=microsoft.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=uY4/YWOBWIz6mCe+fYkvavQwhLcDl/IGzUWq63t1knc=; b=GlFCiMA8GhSFyNsqcnhtn0H7CTFQXW/2WG5HWOy5DcqtsjcDeqy0q6ybZQfxdBXvtOjN+yOt6GSLcR/nHUokYdE9OkXlqTkWj2s2/pt4tfpGBXWzmEMC0RC8yQSFv5bGUosjpq60hfmFFt3x1M28cg3BsbZPZgmPH5Mui50TC94= Received: from CY4PR21MB0743.namprd21.prod.outlook.com (2603:10b6:903:b2::9) by CY4PR21MB0693.namprd21.prod.outlook.com (2603:10b6:903:13b::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3066.6; Tue, 2 Jun 2020 16:55:44 +0000 Received: from CY4PR21MB0743.namprd21.prod.outlook.com ([fe80::f112:82fb:d4fd:f7dd]) by CY4PR21MB0743.namprd21.prod.outlook.com ([fe80::f112:82fb:d4fd:f7dd%11]) with mapi id 15.20.3088.000; Tue, 2 Jun 2020 16:55:44 +0000 From: "Bret Barkelew" To: "devel@edk2.groups.io" , "lersek@redhat.com" , "bret@corthon.com" CC: Jian J Wang , Hao A Wu , liming.gao Subject: Re: [EXTERNAL] Re: [edk2-devel] [PATCH v4 09/14] MdeModulePkg: Connect VariablePolicy business logic to VariableServices Thread-Topic: [EXTERNAL] Re: [edk2-devel] [PATCH v4 09/14] MdeModulePkg: Connect VariablePolicy business logic to VariableServices Thread-Index: AQHWOPVOI3JI/xdPXkyLb7oy9DXEoqjFivcR Date: Tue, 2 Jun 2020 16:55:44 +0000 Message-ID: References: <20200601163310.1718-1-brbarkel@microsoft.com> <20200601163310.1718-10-brbarkel@microsoft.com>,<76b6a35d-bb89-e208-18ae-4be2fb2649c2@redhat.com> In-Reply-To: <76b6a35d-bb89-e208-18ae-4be2fb2649c2@redhat.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: msip_labels: MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Enabled=True;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SiteId=72f988bf-86f1-41af-91ab-2d7cd011db47;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SetDate=2020-06-02T16:54:12.1418470Z;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_ContentBits=0;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Method=Privileged authentication-results: edk2.groups.io; dkim=none (message not signed) header.d=none;edk2.groups.io; dmarc=none action=none header.from=microsoft.com; x-originating-ip: [71.212.144.72] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: b5a8d445-e0f5-4788-2e57-08d80715ca59 x-ms-traffictypediagnostic: CY4PR21MB0693: x-ld-processed: 72f988bf-86f1-41af-91ab-2d7cd011db47,ExtAddr x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:8882; x-forefront-prvs: 0422860ED4 x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: M/j6qAgWtoSChoJffhqEH0jUVhO/LnCAkrfRXzb5YsAOnFfT7COaAzyeCYOy3HUc/fxd5P9UqvwBKH92Ctr91pp1rdT271U32CE48f8xK7WkrosxE0S8E5OPELhjNVwF/+3T06lEGIJzpcvLvdZxYdrHkVXffSaEAM64RS+z5Oo4HU55Gi40HNihQDb0HDFv6i0cbmiUMwVQc8CE1CP5zFQeV/6WJGZt8sOGrU7HMiTjgZ+p8MqphQRWjK2WE43LoLt2rwig36sgMXm3wdehGN24UMLpienUGdG1td+ssfj2ukmuEJlvZcn+27ZHeSFmTe/+2B5dPVp+xvTBUi29tC3giqWf6xXBbKE991PZ2eYUXuqxoN6KiUwMC0w8UofCz3j7+lqt/CArgaxUOGgQK/LyxtlPJLf9sO+YE1xmDeoKhCuY1hV4k8sR8Z42OiPFwK4TeN4yezqZczdAVQdSgg== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:CY4PR21MB0743.namprd21.prod.outlook.com;PTR:;CAT:NONE;SFTY:;SFS:(4636009)(136003)(396003)(366004)(376002)(39860400002)(346002)(2906002)(76116006)(66946007)(91956017)(64756008)(110136005)(166002)(8990500004)(66446008)(54906003)(66556008)(33656002)(82960400001)(82950400001)(52536014)(66476007)(9686003)(86362001)(966005)(55016002)(10290500003)(186003)(83380400001)(53546011)(26005)(5660300002)(6506007)(7696005)(30864003)(71200400001)(8676002)(4326008)(8936002)(478600001)(316002)(559001)(579004)(44824005);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata: wMi8m+RNrP0Ioqeqi1drSVjimLDYJURmvkHNXxAHEJvYpMd+tEu51IIWEPBPdSNwT3DXh8b0hJn5RK3Npoo8NIlgM/05ltw8pMuBZZ7OO/HDv8ylQb8XW/57RRUyPhHUE351Y9VREkXJPXAptNzxGaeUYtlecxclhHTmGTYbED0OV3TPAJ4C8vDlLdd4D4txeX+tYeajPXNy+lZSek+JNMKc1TWpvKkWvZybLnulEJOvewvlLcL2Zpx+NEPoApBmFw3soTVzBamhw/SNOwUzeYJbMRt/3puVh11bGflArMc9iQOQUtf0+quk1Q07910XpJmJhvr2DwtScCP0mJKNjDE37mmrNFgRIIv3L3J6GK8BjzvKXn0YVMJhzSBLJAQ/mUwDqV82EZrXwu4JRiR/mkLiXH5BIRAd9mxMBqOzeVlpko7kQD+BK+5A2jbVByq6u3xGYL4j2VSlpc2Ij47pzABsuYdUcBA3METIAO9tb5FjP//BANFldZJz2Ug59LWl x-ms-exchange-transport-forked: True MIME-Version: 1.0 X-OriginatorOrg: microsoft.com X-MS-Exchange-CrossTenant-Network-Message-Id: b5a8d445-e0f5-4788-2e57-08d80715ca59 X-MS-Exchange-CrossTenant-originalarrivaltime: 02 Jun 2020 16:55:44.2115 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 72f988bf-86f1-41af-91ab-2d7cd011db47 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: bFPZQxqYIujy+wb8Zk4cJDA5X5LBLZn7wWWBYcYEL64+la8MUX9jEIOtzF6RewaQfnUzRaoEDn8gomQOBTl80w== X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR21MB0693 Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_CY4PR21MB07438938D2371F37C6DEE6B1EF8B0CY4PR21MB0743namp_" --_000_CY4PR21MB07438938D2371F37C6DEE6B1EF8B0CY4PR21MB0743namp_ Content-Type: text/plain; charset="Windows-1252" Content-Transfer-Encoding: quoted-printable I=92m also puzzled by the lack of error in CI. I just tried to leverage it = to iterate on the problem and resolve the issue you=92re seeing, but it was= n=92t of any use. Will poke around a little, but also keen to hear from any= one with more GCC5 experience. - Bret From: Laszlo Ersek via groups.io Sent: Tuesday, June 2, 2020 8:48 AM To: devel@edk2.groups.io; bret@corthon.com Cc: Jian J Wang; Hao A Wu; liming.gao Subject: [EXTERNAL] Re: [edk2-devel] [PATCH v4 09/14] MdeModulePkg: Connec= t VariablePolicy business logic to VariableServices Hi Bret, this patch causes a build failure with the GCC48 toolchain (actual version: gcc 4.8.5-36 in RHEL7), and also with the GCC5 toolchain (actual version: "Red Hat Cross 6.1.1-2"): On 06/01/20 18:33, Bret Barkelew wrote: > https://nam06.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fbugz= illa.tianocore.org%2Fshow_bug.cgi%3Fid%3D2522&data=3D02%7C01%7CBret.Bar= kelew%40microsoft.com%7C40552c2a4896475ed9da08d8070c6f51%7C72f988bf86f141af= 91ab2d7cd011db47%7C1%7C0%7C637267097285334746&sdata=3DUEd%2FyUeixp5cq2j= XwUQ68tIws9pbGBl0m9cpnrijaGc%3D&reserved=3D0 > > VariablePolicy is an updated interface to > replace VarLock and VarCheckProtocol. > > Add connective code to publish the VariablePolicy protocol > and wire it to either the SMM communication interface > or directly into the VariablePolicyLib business logic. > > Cc: Jian J Wang > Cc: Hao A Wu > Cc: Liming Gao > Cc: Bret Barkelew > Signed-off-by: Bret Barkelew > --- > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c | = 53 ++ > MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c | = 642 ++++++++++++++++++++ > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c | = 14 + > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf | = 2 + > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf | = 3 + > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf | = 10 + > 6 files changed, 724 insertions(+) > > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c b/= MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c > index 7d2b6c8e1fad..d404d4763e54 100644 > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c > @@ -5,18 +5,34 @@ > Copyright (C) 2013, Red Hat, Inc. > Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
> (C) Copyright 2015 Hewlett Packard Enterprise Development LP
> +Copyright (c) Microsoft Corporation. > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > > #include "Variable.h" > > +#include > +#include > + > +EFI_STATUS > +EFIAPI > +ProtocolIsVariablePolicyEnabled ( > + OUT BOOLEAN *State > + ); > + > EFI_HANDLE mHandle =3D NULL= ; > EFI_EVENT mVirtualAddressChangeEvent =3D NULL= ; > VOID *mFtwRegistration =3D NULL= ; > VOID ***mVarCheckAddressPointer =3D NULL= ; > UINTN mVarCheckAddressPointerCount =3D 0; > EDKII_VARIABLE_LOCK_PROTOCOL mVariableLock =3D { Va= riableLockRequestToLock }; > +EDKII_VARIABLE_POLICY_PROTOCOL mVariablePolicyProtocol =3D { ED= KII_VARIABLE_POLICY_PROTOCOL_REVISION, > + Dis= ableVariablePolicy, > + Pro= tocolIsVariablePolicyEnabled, > + Reg= isterVariablePolicy, (1) "error: initialization from incompatible pointer type [-Werror]" That's because RegisterVariablePolicy() has the following prototype (from "MdeModulePkg/Include/Library/VariablePolicyLib.h"): EFI_STATUS EFIAPI RegisterVariablePolicy ( IN CONST VARIABLE_POLICY_ENTRY *NewPolicy ); whereas "EDKII_VARIABLE_POLICY_PROTOCOL.RegisterVariablePolicy" has the following type (from "MdeModulePkg/Include/Protocol/VariablePolicy.h"): typedef EFI_STATUS (EFIAPI *REGISTER_VARIABLE_POLICY)( IN VARIABLE_POLICY_ENTRY *PolicyEntry ); The latter does not take a pointer to CONST. Now, assuming that "CONST is good", I locally modified the REGISTER_VARIABLE_POLICY typedef, just to see if that would allow the build to complete. It fixes problem (1), but it triggers a different problem: > + Dum= pVariablePolicy, > + Loc= kVariablePolicy }; > EDKII_VAR_CHECK_PROTOCOL mVarCheck =3D { Va= rCheckRegisterSetVariableCheckHandler, > Var= CheckVariablePropertySet, > Var= CheckVariablePropertyGet }; > @@ -303,6 +319,8 @@ OnReadyToBoot ( > } > } > > + ASSERT_EFI_ERROR (LockVariablePolicy ()); > + > gBS->CloseEvent (Event); > } > > @@ -466,6 +484,28 @@ FtwNotificationEvent ( > } > > > +/** > + This API function returns whether or not the policy engine is > + currently being enforced. > + > + @param[out] State Pointer to a return value for whether the p= olicy enforcement > + is currently enabled. > + > + @retval EFI_SUCCESS > + @retval Others An error has prevented this command from co= mpleting. > + > +**/ > +EFI_STATUS > +EFIAPI > +ProtocolIsVariablePolicyEnabled ( > + OUT BOOLEAN *State > + ) > +{ > + *State =3D IsVariablePolicyEnabled (); > + return EFI_SUCCESS; > +} > + > + > /** > Variable Driver main entry point. The Variable driver places the 4 EF= I > runtime services in the EFI System Table and installs arch protocols > @@ -576,6 +616,19 @@ VariableServiceInitialize ( > ); > ASSERT_EFI_ERROR (Status); > > + // Register and initialize the VariablePolicy engine. > + Status =3D InitVariablePolicyLib (VariableServiceGetVariable); > + ASSERT_EFI_ERROR (Status); > + Status =3D VarCheckRegisterSetVariableCheckHandler (ValidateSetVariab= le); > + ASSERT_EFI_ERROR (Status); > + Status =3D gBS->InstallMultipleProtocolInterfaces ( > + &mHandle, > + &gEdkiiVariablePolicyProtocolGuid, > + &mVariablePolicyProtocol, > + NULL > + ); > + ASSERT_EFI_ERROR (Status); > + > return EFI_SUCCESS; > } > > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySm= mDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c > new file mode 100644 > index 000000000000..3d799025983a > --- /dev/null > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c > @@ -0,0 +1,642 @@ > +/** @file -- VariablePolicySmmDxe.c > +This protocol allows communication with Variable Policy Engine. > + > +Copyright (c) Microsoft Corporation. > +SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > + > +#include > + > +#include "Variable.h" > + > +EDKII_VARIABLE_POLICY_PROTOCOL mVariablePolicyProtocol; > +EFI_MM_COMMUNICATION2_PROTOCOL *mMmCommunication; > + > +VOID *mMmCommunicationBuffer; > +UINTN mMmCommunicationBufferSize; > +EFI_LOCK mMmCommunicationLock; > + > +/** > + Internal helper function to consolidate communication method. > + > + @param[in,out] CommBuffer > + @param[in,out] CommSize Size of the CommBuffer. > + > + @retval EFI_STATUS Result from communication method. > + > +**/ > +STATIC > +EFI_STATUS > +InternalMmCommunicate ( > + IN OUT VOID *CommBuffer, > + IN OUT UINTN *CommSize > + ) > +{ > + EFI_STATUS Status; > + if (CommBuffer =3D=3D NULL || CommSize =3D=3D NULL) { > + return EFI_INVALID_PARAMETER; > + } > + Status =3D mMmCommunication->Communicate (mMmCommunication, CommBuffe= r, CommBuffer, CommSize); > + return Status; > +} > + > + > +/** > + This API function disables the variable policy enforcement. If it's > + already been called once, will return EFI_ALREADY_STARTED. > + > + @retval EFI_SUCCESS > + @retval EFI_ALREADY_STARTED Has already been called once this b= oot. > + @retval EFI_WRITE_PROTECTED Interface has been locked until reb= oot. > + @retval EFI_WRITE_PROTECTED Interface option is disabled by pla= tform PCD. > + > +**/ > +STATIC > +EFI_STATUS > +EFIAPI > +ProtocolDisableVariablePolicy ( > + VOID > + ) > +{ > + EFI_STATUS Status; > + EFI_MM_COMMUNICATE_HEADER *CommHeader; > + VAR_CHECK_POLICY_COMM_HEADER *PolicyHeader; > + UINTN BufferSize; > + > + // Check the PCD for convenience. > + // This would also be rejected by the lib, but why go to MM if we don= 't have to? > + if (!PcdGetBool (PcdAllowVariablePolicyEnforcementDisable)) { > + return EFI_WRITE_PROTECTED; > + } > + > + AcquireLockOnlyAtBootTime (&mMmCommunicationLock); > + > + // Set up the MM communication. > + BufferSize =3D mMmCommunicationBufferSize; > + CommHeader =3D mMmCommunicationBuffer; > + PolicyHeader =3D (VAR_CHECK_POLICY_COMM_HEADER*)&CommHeader->Data; > + CopyGuid( &CommHeader->HeaderGuid, &gVarCheckPolicyLibMmiHandlerGuid = ); > + CommHeader->MessageLength =3D BufferSize; > + PolicyHeader->Signature =3D VAR_CHECK_POLICY_COMM_SIG; > + PolicyHeader->Revision =3D VAR_CHECK_POLICY_COMM_REVISION; > + PolicyHeader->Command =3D VAR_CHECK_POLICY_COMMAND_DISABLE; > + > + Status =3D InternalMmCommunicate (CommHeader, &BufferSize); > + DEBUG(( DEBUG_VERBOSE, "%a - MmCommunication returned %r.\n", __FUNCT= ION__, Status )); > + > + ReleaseLockOnlyAtBootTime (&mMmCommunicationLock); > + > + return (EFI_ERROR( Status )) ? Status : PolicyHeader->Result; > +} > + > + > +/** > + This API function returns whether or not the policy engine is > + currently being enforced. > + > + @param[out] State Pointer to a return value for whether the p= olicy enforcement > + is currently enabled. > + > + @retval EFI_SUCCESS > + @retval Others An error has prevented this command from co= mpleting. > + > +**/ > +STATIC > +EFI_STATUS > +EFIAPI > +ProtocolIsVariablePolicyEnabled ( > + OUT BOOLEAN *State > + ) > +{ > + EFI_STATUS Status; > + EFI_MM_COMMUNICATE_HEADER *CommHeader; > + VAR_CHECK_POLICY_COMM_HEADER *PolicyHeader; > + VAR_CHECK_POLICY_COMM_IS_ENABLED_PARAMS *CommandParams; > + UINTN BufferSize; > + > + if (State =3D=3D NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > + AcquireLockOnlyAtBootTime (&mMmCommunicationLock); > + > + // Set up the MM communication. > + BufferSize =3D mMmCommunicationBufferSize; > + CommHeader =3D mMmCommunicationBuffer; > + PolicyHeader =3D (VAR_CHECK_POLICY_COMM_HEADER*)&CommHeader->Data; > + CommandParams =3D (VAR_CHECK_POLICY_COMM_IS_ENABLED_PARAMS*)(PolicyHe= ader + 1); > + CopyGuid( &CommHeader->HeaderGuid, &gVarCheckPolicyLibMmiHandlerGuid = ); > + CommHeader->MessageLength =3D BufferSize; > + PolicyHeader->Signature =3D VAR_CHECK_POLICY_COMM_SIG; > + PolicyHeader->Revision =3D VAR_CHECK_POLICY_COMM_REVISION; > + PolicyHeader->Command =3D VAR_CHECK_POLICY_COMMAND_IS_ENABLED; > + > + Status =3D InternalMmCommunicate (CommHeader, &BufferSize); > + DEBUG(( DEBUG_VERBOSE, "%a - MmCommunication returned %r.\n", __FUNCT= ION__, Status )); > + > + if (!EFI_ERROR( Status )) { > + Status =3D PolicyHeader->Result; > + *State =3D CommandParams->State; > + } > + > + ReleaseLockOnlyAtBootTime (&mMmCommunicationLock); > + > + return Status; > +} > + > + > +/** > + This API function validates and registers a new policy with > + the policy enforcement engine. > + > + @param[in] NewPolicy Pointer to the incoming policy structure. > + > + @retval EFI_SUCCESS > + @retval EFI_INVALID_PARAMETER NewPolicy is NULL or is internall= y inconsistent. > + @retval EFI_ALREADY_STARTED An identical matching policy alre= ady exists. > + @retval EFI_WRITE_PROTECTED The interface has been locked unt= il the next reboot. > + @retval EFI_UNSUPPORTED Policy enforcement has been disab= led. No reason to add more policies. > + @retval EFI_ABORTED A calculation error has prevented= this function from completing. > + @retval EFI_OUT_OF_RESOURCES Cannot grow the table to hold any= more policies. > + > +**/ > +STATIC > +EFI_STATUS > +EFIAPI > +ProtocolRegisterVariablePolicy ( > + IN VARIABLE_POLICY_ENTRY *NewPolicy > + ) > +{ > + EFI_STATUS Status; > + EFI_MM_COMMUNICATE_HEADER *CommHeader; > + VAR_CHECK_POLICY_COMM_HEADER *PolicyHeader; > + VOID *PolicyBuffer; > + UINTN BufferSize; > + UINTN RequiredSize; > + > + if (NewPolicy =3D=3D NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > + // First, make sure that the required size does not exceed the capabi= lities > + // of the MmCommunication buffer. > + RequiredSize =3D OFFSET_OF(EFI_MM_COMMUNICATE_HEADER, Data) + sizeof(= VAR_CHECK_POLICY_COMM_HEADER); > + Status =3D SafeUintnAdd( RequiredSize, NewPolicy->Size, &RequiredSize= ); > + if (EFI_ERROR( Status ) || RequiredSize > mMmCommunicationBufferSize)= { > + DEBUG(( DEBUG_ERROR, "%a - Policy too large for buffer! %r, %d > %d= \n", __FUNCTION__, > + Status, RequiredSize, mMmCommunicationBufferSize )); > + return EFI_OUT_OF_RESOURCES; > + } > + > + AcquireLockOnlyAtBootTime (&mMmCommunicationLock); > + > + // Set up the MM communication. > + BufferSize =3D mMmCommunicationBufferSize; > + CommHeader =3D mMmCommunicationBuffer; > + PolicyHeader =3D (VAR_CHECK_POLICY_COMM_HEADER*)&CommHeader->Data; > + PolicyBuffer =3D (VOID*)(PolicyHeader + 1); > + CopyGuid( &CommHeader->HeaderGuid, &gVarCheckPolicyLibMmiHandlerGuid = ); > + CommHeader->MessageLength =3D BufferSize; > + PolicyHeader->Signature =3D VAR_CHECK_POLICY_COMM_SIG; > + PolicyHeader->Revision =3D VAR_CHECK_POLICY_COMM_REVISION; > + PolicyHeader->Command =3D VAR_CHECK_POLICY_COMMAND_REGISTER; > + > + // Copy the policy into place. This copy is safe because we've alread= y tested above. > + CopyMem( PolicyBuffer, NewPolicy, NewPolicy->Size ); > + > + Status =3D InternalMmCommunicate (CommHeader, &BufferSize); > + DEBUG(( DEBUG_VERBOSE, "%a - MmCommunication returned %r.\n", __FUNCT= ION__, Status )); > + > + ReleaseLockOnlyAtBootTime (&mMmCommunicationLock); > + > + return (EFI_ERROR( Status )) ? Status : PolicyHeader->Result; > +} > + > + > +/** > + This helper function takes care of the overhead of formatting, sendin= g, and interpreting > + the results for a single DumpVariablePolicy request. > + > + @param[in] PageRequested The page of the paginated results fro= m MM. 0 for metadata. > + @param[out] TotalSize The total size of the entire buffer. = Returned as part of metadata. > + @param[out] PageSize The size of the current page being re= turned. Not valid as part of metadata. > + @param[out] HasMore A flag indicating whether there are m= ore pages after this one. > + @param[out] Buffer The start of the current page from MM= . > + > + @retval EFI_SUCCESS Output params have been updated (= either metadata or dump page). > + @retval EFI_INVALID_PARAMETER One of the output params is NULL. > + @retval Others Response from MM handler. > + > +**/ > +STATIC > +EFI_STATUS > +DumpVariablePolicyHelper ( > + IN UINT32 PageRequested, > + OUT UINT32 *TotalSize, > + OUT UINT32 *PageSize, > + OUT BOOLEAN *HasMore, > + OUT UINT8 **Buffer > + ) > +{ > + EFI_STATUS Status; > + EFI_MM_COMMUNICATE_HEADER *CommHeader; > + VAR_CHECK_POLICY_COMM_HEADER *PolicyHeader; > + VAR_CHECK_POLICY_COMM_DUMP_PARAMS *CommandParams; > + UINTN BufferSize; > + > + if (TotalSize =3D=3D NULL || PageSize =3D=3D NULL || HasMore =3D=3D N= ULL || Buffer =3D=3D NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > + // Set up the MM communication. > + BufferSize =3D mMmCommunicationBufferSize; > + CommHeader =3D mMmCommunicationBuffer; > + PolicyHeader =3D (VAR_CHECK_POLICY_COMM_HEADER*)&CommHeader->Data; > + CommandParams =3D (VAR_CHECK_POLICY_COMM_DUMP_PARAMS*)(PolicyHeader += 1); > + CopyGuid( &CommHeader->HeaderGuid, &gVarCheckPolicyLibMmiHandlerGuid = ); > + CommHeader->MessageLength =3D BufferSize; > + PolicyHeader->Signature =3D VAR_CHECK_POLICY_COMM_SIG; > + PolicyHeader->Revision =3D VAR_CHECK_POLICY_COMM_REVISION; > + PolicyHeader->Command =3D VAR_CHECK_POLICY_COMMAND_DUMP; > + > + CommandParams->PageRequested =3D PageRequested; > + > + Status =3D InternalMmCommunicate (CommHeader, &BufferSize); > + DEBUG(( DEBUG_VERBOSE, "%a - MmCommunication returned %r.\n", __FUNCT= ION__, Status )); > + > + if (!EFI_ERROR( Status )) { > + Status =3D PolicyHeader->Result; > + *TotalSize =3D CommandParams->TotalSize; > + *PageSize =3D CommandParams->PageSize; > + *HasMore =3D CommandParams->HasMore; > + *Buffer =3D (UINT8*)(CommandParams + 1); > + } > + > + return Status; > +} > + > + > +/** > + This API function will dump the entire contents of the variable polic= y table. > + > + Similar to GetVariable, the first call can be made with a 0 size and = it will return > + the size of the buffer required to hold the entire table. > + > + @param[out] Policy Pointer to the policy buffer. Can be NULL if = Size is 0. > + @param[in,out] Size On input, the size of the output buffer. On o= utput, the size > + of the data returned. > + > + @retval EFI_SUCCESS Policy data is in the output buff= er and Size has been updated. > + @retval EFI_INVALID_PARAMETER Size is NULL, or Size is non-zero= and Policy is NULL. > + @retval EFI_BUFFER_TOO_SMALL Size is insufficient to hold poli= cy. Size updated with required size. > + > +**/ > +STATIC > +EFI_STATUS > +EFIAPI > +ProtocolDumpVariablePolicy ( > + OUT UINT8 *Policy OPTIONAL, > + IN OUT UINT32 *Size > + ) > +{ > + EFI_STATUS Status; > + UINT8 *Source; > + UINT8 *Destination; > + UINT32 PolicySize; > + UINT32 PageSize; > + BOOLEAN HasMore; > + UINT32 PageIndex; > + > + if (Size =3D=3D NULL || (*Size > 0 && Policy =3D=3D NULL)) { > + return EFI_INVALID_PARAMETER; > + } > + > + AcquireLockOnlyAtBootTime (&mMmCommunicationLock); > + > + // Repeat this whole process until we either have a failure case or g= et the entire buffer. > + do { > + // First, we must check the zero page to determine the buffer size = and > + // reset the internal state. > + PolicySize =3D 0; > + PageSize =3D 0; > + HasMore =3D FALSE; > + Status =3D DumpVariablePolicyHelper (0, &PolicySize, &PageSize, &Ha= sMore, &Source); > + if (EFI_ERROR (Status)) { > + break; > + } > + > + // If we're good, we can at least check the required size now. > + if (*Size < PolicySize) { > + *Size =3D PolicySize; > + Status =3D EFI_BUFFER_TOO_SMALL; > + break; > + } > + > + // On further thought, let's update the size either way. > + *Size =3D PolicySize; > + // And get ready to ROCK. > + Destination =3D Policy; > + > + // Keep looping and copying until we're either done or freak out. > + for (PageIndex =3D 1; !EFI_ERROR (Status) && HasMore && PageIndex <= MAX_UINT32; PageIndex++) { > + Status =3D DumpVariablePolicyHelper (PageIndex, &PolicySize, &Pag= eSize, &HasMore, &Source); > + if (!EFI_ERROR (Status)) { > + CopyMem (Destination, Source, PageSize); > + Destination +=3D PageSize; > + } > + } > + > + // Next, we check to see whether > + } while (Status =3D=3D EFI_TIMEOUT); > + > + ReleaseLockOnlyAtBootTime (&mMmCommunicationLock); > + > + // There's currently no use for this, but it shouldn't be hard to imp= lement. > + return Status; > +} > + > + > +/** > + This API function locks the interface so that no more policy updates > + can be performed or changes made to the enforcement until the next bo= ot. > + > + @retval EFI_SUCCESS > + @retval Others An error has prevented this command from co= mpleting. > + > +**/ > +STATIC > +EFI_STATUS > +EFIAPI > +ProtocolLockVariablePolicy ( > + VOID > + ) > +{ > + EFI_STATUS Status; > + EFI_MM_COMMUNICATE_HEADER *CommHeader; > + VAR_CHECK_POLICY_COMM_HEADER *PolicyHeader; > + UINTN BufferSize; > + > + AcquireLockOnlyAtBootTime (&mMmCommunicationLock); > + > + // Set up the MM communication. > + BufferSize =3D mMmCommunicationBufferSize; > + CommHeader =3D mMmCommunicationBuffer; > + PolicyHeader =3D (VAR_CHECK_POLICY_COMM_HEADER*)&CommHeader->Data; > + CopyGuid( &CommHeader->HeaderGuid, &gVarCheckPolicyLibMmiHandlerGuid = ); > + CommHeader->MessageLength =3D BufferSize; > + PolicyHeader->Signature =3D VAR_CHECK_POLICY_COMM_SIG; > + PolicyHeader->Revision =3D VAR_CHECK_POLICY_COMM_REVISION; > + PolicyHeader->Command =3D VAR_CHECK_POLICY_COMMAND_LOCK; > + > + Status =3D InternalMmCommunicate (CommHeader, &BufferSize); > + DEBUG(( DEBUG_VERBOSE, "%a - MmCommunication returned %r.\n", __FUNCT= ION__, Status )); > + > + ReleaseLockOnlyAtBootTime (&mMmCommunicationLock); > + > + return (EFI_ERROR( Status )) ? Status : PolicyHeader->Result; > +} > + > + > +/** > + This helper function locates the shared comm buffer and assigns it to= input pointers. > + > + @param[in,out] BufferSize On input, the minimum buffer size req= uired INCLUDING the MM communicate header. > + On output, the size of the matching b= uffer found. > + @param[out] LocatedBuffer A pointer to the matching buffer. > + > + @retval EFI_SUCCESS > + @retval EFI_INVALID_PARAMETER One of the output pointers was NU= LL. > + @retval EFI_OUT_OF_RESOURCES Not enough memory to allocate a c= omm buffer. > + > +**/ > +STATIC > +EFI_STATUS > +InitMmCommonCommBuffer ( > + IN OUT UINTN *BufferSize, > + OUT VOID **LocatedBuffer > + ) > +{ > + EFI_STATUS Status; > + > + Status =3D EFI_SUCCESS; > + > + // Make sure that we're working with good pointers. > + if (BufferSize =3D=3D NULL || LocatedBuffer =3D=3D NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > + // Allocate the runtime memory for the comm buffer. > + *LocatedBuffer =3D AllocateRuntimePool (*BufferSize); > + if (*LocatedBuffer =3D=3D NULL) { > + Status =3D EFI_OUT_OF_RESOURCES; > + *BufferSize =3D 0; > + } > + > + EfiInitializeLock (&mMmCommunicationLock, TPL_NOTIFY); > + > + return Status; > +} > + > + > +/** > + This helper is responsible for telemetry and any other actions that > + need to be taken if the VariablePolicy fails to lock. > + > + NOTE: It's possible that parts of this handling will need to become > + part of a platform policy. > + > + @param[in] FailureStatus The failure that was reported by LockVari= ablePolicy > + > +**/ > +STATIC > +VOID > +VariablePolicyHandleFailureToLock ( > + IN EFI_STATUS FailureStatus > + ) > +{ > + // For now, there's no agreed-upon policy for this. > + return; > +} > + > + > +/** > + ReadyToBoot Callback > + Lock the VariablePolicy interface if it hasn't already been locked. > + > + @param[in] Event Event whose notification function is being invo= ked > + @param[in] Context Pointer to the notification function's context > + > +**/ > +STATIC > +VOID > +EFIAPI > +LockPolicyInterfaceAtReadyToBoot ( > + IN EFI_EVENT Event, > + IN VOID *Context > + ) > +{ > + EFI_STATUS Status; > + > + Status =3D ProtocolLockVariablePolicy(); > + > + if (EFI_ERROR( Status )) { > + VariablePolicyHandleFailureToLock( Status ); > + } > + else { > + gBS->CloseEvent( Event ); > + } > + > +} > + > + > +/** > + Convert internal pointer addresses to virtual addresses. > + > + @param[in] Event Event whose notification function is being invo= ked. > + @param[in] Context The pointer to the notification function's cont= ext, which > + is implementation-dependent. > +**/ > +STATIC > +VOID > +EFIAPI > +VariablePolicyVirtualAddressCallback ( > + IN EFI_EVENT Event, > + IN VOID *Context > + ) > +{ > + EfiConvertPointer (0, (VOID **)&mMmCommunication); > + EfiConvertPointer (0, (VOID **)&mMmCommunicationBuffer); > +} > + > + > +/** > + The driver's entry point. > + > + @param[in] ImageHandle The firmware allocated handle for the EFI ima= ge. > + @param[in] SystemTable A pointer to the EFI System Table. > + > + @retval EFI_SUCCESS The entry point executed successfully. > + @retval other Some error occured when executing this entry = point. > + > +**/ > +EFI_STATUS > +EFIAPI > +VariablePolicySmmDxeMain ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + EFI_STATUS Status; > + BOOLEAN ProtocolInstalled; > + BOOLEAN CallbackRegistered; > + BOOLEAN VirtualAddressChangeRegistered; > + EFI_EVENT ReadyToBootEvent; > + EFI_EVENT VirtualAddressChangeEvent; > + > + Status =3D EFI_SUCCESS; > + ProtocolInstalled =3D FALSE; > + CallbackRegistered =3D FALSE; > + VirtualAddressChangeRegistered =3D FALSE; > + > + // Update the minimum buffer size. > + mMmCommunicationBufferSize =3D VAR_CHECK_POLICY_MM_COMM_BUFFER_SIZE; > + // Locate the shared comm buffer to use for sending MM commands. > + Status =3D InitMmCommonCommBuffer( &mMmCommunicationBufferSize, &mMmC= ommunicationBuffer ); > + if (EFI_ERROR( Status )) { > + DEBUG((DEBUG_ERROR, "%a - Failed to locate a viable MM comm buffer!= %r\n", __FUNCTION__, Status)); > + ASSERT_EFI_ERROR( Status ); > + return Status; > + } > + > + // Locate the MmCommunication protocol. > + Status =3D gBS->LocateProtocol( &gEfiMmCommunication2ProtocolGuid, NU= LL, (VOID**)&mMmCommunication ); > + if (EFI_ERROR( Status )) { > + DEBUG((DEBUG_ERROR, "%a - Failed to locate MmCommunication protocol= ! %r\n", __FUNCTION__, Status)); > + ASSERT_EFI_ERROR( Status ); > + return Status; > + } > + > + // Configure the VariablePolicy protocol structure. > + mVariablePolicyProtocol.Revision =3D EDKII_VARIABLE_PO= LICY_PROTOCOL_REVISION; > + mVariablePolicyProtocol.DisableVariablePolicy =3D ProtocolDisableVa= riablePolicy; > + mVariablePolicyProtocol.IsVariablePolicyEnabled =3D ProtocolIsVariabl= ePolicyEnabled; > + mVariablePolicyProtocol.RegisterVariablePolicy =3D ProtocolRegisterV= ariablePolicy; (2) "error: assignment from incompatible pointer type [-Werror]" Because, with my local modification for (1), the LHS now takes a pointer-to-CONST, but the RHS is declared (in the present patch) as STATIC EFI_STATUS EFIAPI ProtocolRegisterVariablePolicy ( IN VARIABLE_POLICY_ENTRY *NewPolicy ) So I modified this too, to take a pointer-to-CONST. That allowed the builds to complete. Therefore I suggest one of the following: (a) If CONST is not important in the RegisterVariablePolicy() lib class API, then please remove CONST from there (and the implementation(s) of that function). (b) Alternatively, if CONST is preferred in the library, then (b1) please squash the following hunk: > diff --git a/MdeModulePkg/Include/Protocol/VariablePolicy.h b/MdeModuleP= kg/Include/Protocol/VariablePolicy.h > index 30d6c155ae6a..83b6a999df07 100644 > --- a/MdeModulePkg/Include/Protocol/VariablePolicy.h > +++ b/MdeModulePkg/Include/Protocol/VariablePolicy.h > @@ -102,7 +102,7 @@ EFI_STATUS > typedef > EFI_STATUS > (EFIAPI *REGISTER_VARIABLE_POLICY)( > - IN VARIABLE_POLICY_ENTRY *PolicyEntry > + IN CONST VARIABLE_POLICY_ENTRY *PolicyEntry > ); > > /** into patch "MdeModulePkg: Define the VariablePolicy protocol interface". (b2) And please squash the following hunk: > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySm= mDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c > index 3d799025983a..e2d4cf4cec1a 100644 > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c > @@ -176,7 +176,7 @@ STATIC > EFI_STATUS > EFIAPI > ProtocolRegisterVariablePolicy ( > - IN VARIABLE_POLICY_ENTRY *NewPolicy > + IN CONST VARIABLE_POLICY_ENTRY *NewPolicy > ) > { > EFI_STATUS Status; into the present patch. I've used (b1)+(b2) locally, and those together allow the builds to complete. Now, I wonder why the GCC / OVMF build(s) in the github.com CI did not report this problem. According to the C99 standard, "6.7.5.3 Function declarators (including prototypes)", paragraph 15: For two function types to be compatible, both shall specify compatible return types. [...] Moreover, the parameter type lists, if both are present, shall agree in the number of parameters and in use of the ellipsis terminator; corresponding parameters shall have compatible types. [...] Furthermore, from "6.7.5.1 Pointer declarators", paragraph 2: For two pointer types to be compatible, both shall be identically qualified and both shall be pointers to compatible types. Yet further, from "6.5.16.1 Simple assignment", paragraph 1: One of the following shall hold: [...] - both operands are pointers to qualified or unqualified versions of compatible types, and the type pointed to by the left has all the qualifiers of the type pointed to by the right; [...] So the thought process is: - "mVariablePolicyProtocol.RegisterVariablePolicy" has type EFI_STATUS (EFIAPI *)( VARIABLE_POLICY_ENTRY *) while a pointer to RegisterVariablePolicy(), from the library, has type EFI_STATUS (EFIAPI *)(const VARIABLE_POLICY_ENTRY *) For the assignment, these need to be compatible, per 6.5.16.1p1. Are they compatible? - Per 6.7.5.1p2, that depends on whether the pointed-to function types are compatible. Are the pointed-to function types compatible? - Per 6.7.5.3p15, that depends on whether (VARIABLE_POLICY_ENTRY *) and (const VARIABLE_POLICY_ENTRY *) are compatible. Are those pointer types compatible? - They're not, based on 6.7.5.1p2 -- they are not identically qualified. So IMO (according to the language standard), the code should have been flagged by MSVC too; but more importantly, I don't understand why the GCC5 / Ubuntu builds succeeded in CI. Thanks! Laszlo > + mVariablePolicyProtocol.DumpVariablePolicy =3D ProtocolDumpVaria= blePolicy; > + mVariablePolicyProtocol.LockVariablePolicy =3D ProtocolLockVaria= blePolicy; > + > + // Register all the protocols and return the status. > + Status =3D gBS->InstallMultipleProtocolInterfaces( &ImageHandle, > + &gEdkiiVariablePolic= yProtocolGuid, &mVariablePolicyProtocol, > + NULL ); > + if (EFI_ERROR( Status )) { > + DEBUG(( DEBUG_ERROR, "%a - Failed to install protocol! %r\n", __FUN= CTION__, Status )); > + goto Exit; > + } > + else { > + ProtocolInstalled =3D TRUE; > + } > + > + // > + // Register a callback for ReadyToBoot so that the interface is at le= ast locked before > + // dispatching any bootloaders or UEFI apps. > + Status =3D gBS->CreateEventEx( EVT_NOTIFY_SIGNAL, > + TPL_CALLBACK, > + LockPolicyInterfaceAtReadyToBoot, > + NULL, > + &gEfiEventReadyToBootGuid, > + &ReadyToBootEvent ); > + if (EFI_ERROR( Status )) { > + DEBUG(( DEBUG_ERROR, "%a - Failed to create ReadyToBoot event! %r\n= ", __FUNCTION__, Status )); > + goto Exit; > + } > + else { > + CallbackRegistered =3D TRUE; > + } > + > + // > + // Register a VirtualAddressChange callback for the MmComm protocol a= nd Comm buffer. > + Status =3D gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, > + TPL_NOTIFY, > + VariablePolicyVirtualAddressCallback, > + NULL, > + &gEfiEventVirtualAddressChangeGuid, > + &VirtualAddressChangeEvent); > + if (EFI_ERROR( Status )) { > + DEBUG(( DEBUG_ERROR, "%a - Failed to create VirtualAddressChange ev= ent! %r\n", __FUNCTION__, Status )); > + goto Exit; > + } > + else { > + VirtualAddressChangeRegistered =3D TRUE; > + } > + > + > +Exit: > + // > + // If we're about to return a failed status (and unload this driver),= we must first undo anything that > + // has been successfully done. > + if (EFI_ERROR( Status )) { > + if (ProtocolInstalled) { > + gBS->UninstallProtocolInterface( &ImageHandle, &gEdkiiVariablePol= icyProtocolGuid, &mVariablePolicyProtocol ); > + } > + if (CallbackRegistered) { > + gBS->CloseEvent( ReadyToBootEvent ); > + } > + if (VirtualAddressChangeRegistered) { > + gBS->CloseEvent( VirtualAddressChangeEvent ); > + } > + } > + > + return Status; > +} > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRunti= meDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.= c > index 663a1aaa128f..c47e614d81f4 100644 > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c > @@ -65,6 +65,17 @@ EFI_LOCK mVariableServicesLoc= k; > EDKII_VARIABLE_LOCK_PROTOCOL mVariableLock; > EDKII_VAR_CHECK_PROTOCOL mVarCheck; > > +/** > + The logic to initialize the VariablePolicy engine is in its own file. > + > +**/ > +EFI_STATUS > +EFIAPI > +VariablePolicySmmDxeMain ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ); > + > /** > 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. > @@ -1796,6 +1807,9 @@ VariableSmmRuntimeInitialize ( > &mVirtualAddressChangeEvent > ); > > + // Initialize the VariablePolicy protocol and engine. > + VariablePolicySmmDxeMain (ImageHandle, SystemTable); > + > return EFI_SUCCESS; > } > > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeD= xe.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf > index ceea5d1ff9ac..48ac167906f7 100644 > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf > @@ -10,6 +10,7 @@ > # buffer overflow or integer overflow. > # > # Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved. > +# Copyright (c) Microsoft Corporation. > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > ## > @@ -69,6 +70,7 @@ [LibraryClasses] > TpmMeasurementLib > AuthVariableLib > VarCheckLib > + VariablePolicyLib > > [Protocols] > gEfiFirmwareVolumeBlockProtocolGuid ## CONSUMES > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf = b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf > index bc3033588d40..bbc8d2080193 100644 > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf > @@ -19,6 +19,7 @@ > # the authentication service provided in this driver will be broken, a= nd the behavior is undefined. > # > # Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved. > +# Copyright (c) Microsoft Corporation. > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > ## > @@ -78,6 +79,8 @@ [LibraryClasses] > AuthVariableLib > VarCheckLib > UefiBootServicesTableLib > + VariablePolicyLib > + VariablePolicyHelperLib > > [Protocols] > gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRunti= meDxe.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx= e.inf > index 01564e4c5068..f217530b2985 100644 > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.i= nf > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.i= nf > @@ -14,6 +14,7 @@ > # the authentication service provided in this driver will be broken, a= nd the behavior is undefined. > # > # Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved. > +# Copyright (c) Microsoft Corporation.
> # SPDX-License-Identifier: BSD-2-Clause-Patent > # > ## > @@ -42,6 +43,7 @@ [Sources] > VariableParsing.c > VariableParsing.h > Variable.h > + VariablePolicySmmDxe.c > > [Packages] > MdePkg/MdePkg.dec > @@ -56,6 +58,8 @@ [LibraryClasses] > DxeServicesTableLib > UefiDriverEntryPoint > TpmMeasurementLib > + SafeIntLib > + PcdLib > > [Protocols] > gEfiVariableWriteArchProtocolGuid ## PRODUCES > @@ -67,11 +71,15 @@ [Protocols] > gEfiSmmVariableProtocolGuid > gEdkiiVariableLockProtocolGuid ## PRODUCES > gEdkiiVarCheckProtocolGuid ## PRODUCES > + gEdkiiVariablePolicyProtocolGuid ## PRODUCES > > [FeaturePcd] > gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache = ## CONSUMES > gEfiMdeModulePkgTokenSpaceGuid.PcdVariableCollectStatistics = ## CONSUMES > > +[Pcd] > + gEfiMdeModulePkgTokenSpaceGuid.PcdAllowVariablePolicyEnforcementDisab= le ## CONSUMES > + > [Guids] > ## PRODUCES ## GUID # Signature of Variable store header > ## CONSUMES ## GUID # Signature of Variable store header > @@ -99,6 +107,8 @@ [Guids] > ## SOMETIMES_CONSUMES ## Variable:L"dbt" > gEfiImageSecurityDatabaseGuid > > + gVarCheckPolicyLibMmiHandlerGuid > + > [Depex] > gEfiMmCommunication2ProtocolGuid > > --_000_CY4PR21MB07438938D2371F37C6DEE6B1EF8B0CY4PR21MB0743namp_ Content-Type: text/html; charset="Windows-1252" Content-Transfer-Encoding: quoted-printable

I=92m also puzzled by the lack of error in CI. I ju= st tried to leverage it to iterate on the problem and resolve the issue you= = =92re seeing, but it wasn=92t of any use. Will poke around a little, but a= lso keen to hear from anyone with more GCC5 experience.

 

- Bret

 

From: Laszlo Ersek via groups.io=
Sent: Tuesday, June 2, 2020 8:48 AM
To: devel@edk2.groups.io; bret@corthon.com
Cc: Jian J Wang; Hao A Wu; liming.gao
Subject: [EXTERNAL] Re: [edk2-devel] [PATCH v4 09/14] MdeModulePkg:= Connect VariablePolicy business logic to VariableServices

 

Hi Bret,

this patch causes a build failure with the GCC48 toolchain (actual
version: gcc 4.8.5-36 in RHEL7), and also with the GCC5 toolchain
(actual version: "Red Hat Cross 6.1.1-2"):

On 06/01/20 18:33, Bret Barkelew wrote:
> https://nam06.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fbugzil= la.tianocore.org%2Fshow_bug.cgi%3Fid%3D2522&amp;data=3D02%7C01%7CBret.B= arkelew%40microsoft.com%7C40552c2a4896475ed9da08d8070c6f51%7C72f988bf86f141= af91ab2d7cd011db47%7C1%7C0%7C637267097285334746&amp;sdata=3DUEd%2FyUeix= p5cq2jXwUQ68tIws9pbGBl0m9cpnrijaGc%3D&amp;reserved=3D0
>
> VariablePolicy is an updated interface to
> replace VarLock and VarCheckProtocol.
>
> Add connective code to publish the VariablePolicy protocol
> and wire it to either the SMM communication interface
> or directly into the VariablePolicyLib business logic.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Bret Barkelew <brbarkel@microsoft.com>
> Signed-off-by: Bret Barkelew <brbarkel@microsoft.com>
> ---
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c &= nbsp;           |  5= 3 ++
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe= .c    | 642 +++++++++= 3;++++++++++
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx= e.c   |  14 +
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.i= nf    |   2 +
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf = ;          |   3 = 3;
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx= e.inf |  10 +
>  6 files changed, 724 insertions(+)
>
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c= b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> index 7d2b6c8e1fad..d404d4763e54 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable= Dxe.c
> @@ -5,18 +5,34 @@
>  Copyright (C) 2013, Red Hat, Inc.
>  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserv= ed.<BR>
>  (C) Copyright 2015 Hewlett Packard Enterprise Development LP<= ;BR>
> +Copyright (c) Microsoft Corporation.
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>
>  **/
>
>  #include "Variable.h"
>
> +#include <Protocol/VariablePolicy.h>
> +#include <Library/VariablePolicyLib.h>
> +
> +EFI_STATUS
> +EFIAPI
> +ProtocolIsVariablePolicyEnabled (
> +  OUT BOOLEAN *State
> +  );
> +
>  EFI_HANDLE         = ;            &n= bsp;    mHandle       &nb= sp;            =3D N= ULL;
>  EFI_EVENT         =             &nb= sp;     mVirtualAddressChangeEvent =3D NULL;
>  VOID          = ;            &n= bsp;         *mFtwRegistration = ;         =3D NULL;
>  VOID          = ;            &n= bsp;         ***mVarCheckAddressPoi= nter =3D NULL;
>  UINTN         &nbs= p;            &= nbsp;        mVarCheckAddressPointerCoun= t =3D 0;
>  EDKII_VARIABLE_LOCK_PROTOCOL      = ;  mVariableLock         =      =3D { VariableLockRequestToLock };
> +EDKII_VARIABLE_POLICY_PROTOCOL      mVa= riablePolicyProtocol    =3D { EDKII_VARIABLE_POLICY_PROTOCOL= _REVISION,
> +          &nbs= p;            &= nbsp;           &nbs= p;            &= nbsp;           &nbs= p;       DisableVariablePolicy,
> +          &nbs= p;            &= nbsp;           &nbs= p;            &= nbsp;           &nbs= p;       ProtocolIsVariablePolicyEnabled,
> +          &nbs= p;            &= nbsp;           &nbs= p;            &= nbsp;           &nbs= p;       RegisterVariablePolicy,

(1) "error: initialization from incompatible pointer type [-Werror]&q= uot;

That's because RegisterVariablePolicy() has the following prototype
(from "MdeModulePkg/Include/Library/VariablePolicyLib.h"):

  EFI_STATUS
  EFIAPI
  RegisterVariablePolicy (
    IN CONST VARIABLE_POLICY_ENTRY    *NewPo= licy
    );

whereas "EDKII_VARIABLE_POLICY_PROTOCOL.RegisterVariablePolicy" = has the
following type (from "MdeModulePkg/Include/Protocol/VariablePolicy.h&= quot;):

  typedef
  EFI_STATUS
  (EFIAPI *REGISTER_VARIABLE_POLICY)(
    IN VARIABLE_POLICY_ENTRY *PolicyEntry
    );

The latter does not take a pointer to CONST.

Now, assuming that "CONST is good", I locally modified the
REGISTER_VARIABLE_POLICY typedef, just to see if that would allow the
build to complete. It fixes problem (1), but it triggers a different
problem:


> +          &nbs= p;            &= nbsp;           &nbs= p;            &= nbsp;           &nbs= p;       DumpVariablePolicy,
> +          &nbs= p;            &= nbsp;           &nbs= p;            &= nbsp;           &nbs= p;       LockVariablePolicy };
>  EDKII_VAR_CHECK_PROTOCOL      &nb= sp;     mVarCheck      &n= bsp;           =3D { VarC= heckRegisterSetVariableCheckHandler,
>           &nbs= p;            &= nbsp;           &nbs= p;            &= nbsp;           &nbs= p;        VarCheckVariablePropertySet, >           &nbs= p;            &= nbsp;           &nbs= p;            &= nbsp;           &nbs= p;        VarCheckVariablePropertyGet };=
> @@ -303,6 +319,8 @@ OnReadyToBoot (
>      }
>    }
>
> +  ASSERT_EFI_ERROR (LockVariablePolicy ());
> +
>    gBS->CloseEvent (Event);
>  }
>
> @@ -466,6 +484,28 @@ FtwNotificationEvent (
>  }
>
>
> +/**
> +  This API function returns whether or not the policy engin= e is
> +  currently being enforced.
> +
> +  @param[out]   State    &nbs= p;  Pointer to a return value for whether the policy enforcement
> +          &nbs= p;            &= nbsp;    is currently enabled.
> +
> +  @retval     EFI_SUCCESS
> +  @retval     Others   &= nbsp;    An error has prevented this command from completing= .
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +ProtocolIsVariablePolicyEnabled (
> +  OUT BOOLEAN *State
> +  )
> +{
> +  *State =3D IsVariablePolicyEnabled ();
> +  return EFI_SUCCESS;
> +}
> +
> +
>  /**
>    Variable Driver main entry point. The Variable driv= er places the 4 EFI
>    runtime services in the EFI System Table and instal= ls arch protocols
> @@ -576,6 +616,19 @@ VariableServiceInitialize (
>           &nbs= p;        );
>    ASSERT_EFI_ERROR (Status);
>
> +  // Register and initialize the VariablePolicy engine.
> +  Status =3D InitVariablePolicyLib (VariableServiceGetVaria= ble);
> +  ASSERT_EFI_ERROR (Status);
> +  Status =3D VarCheckRegisterSetVariableCheckHandler (Valid= ateSetVariable);
> +  ASSERT_EFI_ERROR (Status);
> +  Status =3D gBS->InstallMultipleProtocolInterfaces ( > +          &nbs= p;         &mHandle,
> +          &nbs= p;         &gEdkiiVariablePolic= yProtocolGuid,
> +          &nbs= p;         &mVariablePolicyProt= ocol,
> +          &nbs= p;         NULL
> +          &nbs= p;         );
> +  ASSERT_EFI_ERROR (Status);
> +
>    return EFI_SUCCESS;
>  }
>
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolic= ySmmDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe= .c
> new file mode 100644
> index 000000000000..3d799025983a
> --- /dev/null
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable= PolicySmmDxe.c
> @@ -0,0 +1,642 @@
> +/** @file -- VariablePolicySmmDxe.c
> +This protocol allows communication with Variable Policy Engine.<= br> > +
> +Copyright (c) Microsoft Corporation.
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Library/BaseLib.h>
> +#include <Library/UefiLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/SafeIntLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +
> +#include <Protocol/VariablePolicy.h>
> +#include <Protocol/MmCommunication2.h>
> +
> +#include <Guid/VarCheckPolicyMmi.h>
> +
> +#include "Variable.h"
> +
> +EDKII_VARIABLE_POLICY_PROTOCOL  mVariablePolicyProtocol; > +EFI_MM_COMMUNICATION2_PROTOCOL  *mMmCommunication;
> +
> +VOID      *mMmCommunicationBuffer;
> +UINTN     mMmCommunicationBufferSize;
> +EFI_LOCK  mMmCommunicationLock;
> +
> +/**
> +  Internal helper function to consolidate communication met= hod.
> +
> +  @param[in,out]  CommBuffer
> +  @param[in,out]  CommSize    Size of t= he CommBuffer.
> +
> +  @retval   EFI_STATUS    Result f= rom communication method.
> +
> +**/
> +STATIC
> +EFI_STATUS
> +InternalMmCommunicate (
> +  IN OUT VOID       &nbs= p;     *CommBuffer,
> +  IN OUT UINTN       &nb= sp;    *CommSize
> +  )
> +{
> +  EFI_STATUS    Status;
> +  if (CommBuffer =3D=3D NULL || CommSize =3D=3D NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +  Status =3D mMmCommunication->Communicate (mMmCommunica= tion, CommBuffer, CommBuffer, CommSize);
> +  return Status;
> +}
> +
> +
> +/**
> +  This API function disables the variable policy enforcemen= t. If it's
> +  already been called once, will return EFI_ALREADY_STARTED= .
> +
> +  @retval     EFI_SUCCESS
> +  @retval     EFI_ALREADY_STARTED =   Has already been called once this boot.
> +  @retval     EFI_WRITE_PROTECTED =   Interface has been locked until reboot.
> +  @retval     EFI_WRITE_PROTECTED =   Interface option is disabled by platform PCD.
> +
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +ProtocolDisableVariablePolicy (
> +  VOID
> +  )
> +{
> +  EFI_STATUS        = ;            Status;=
> +  EFI_MM_COMMUNICATE_HEADER     *CommHe= ader;
> +  VAR_CHECK_POLICY_COMM_HEADER  *PolicyHeader;
> +  UINTN        &nbs= p;            &= nbsp;   BufferSize;
> +
> +  // Check the PCD for convenience.
> +  // This would also be rejected by the lib, but why go to = MM if we don't have to?
> +  if (!PcdGetBool (PcdAllowVariablePolicyEnforcementDisable= )) {
> +    return EFI_WRITE_PROTECTED;
> +  }
> +
> +  AcquireLockOnlyAtBootTime (&mMmCommunicationLock); > +
> +  // Set up the MM communication.
> +  BufferSize    =3D mMmCommunicationBufferSi= ze;
> +  CommHeader    =3D mMmCommunicationBuffer;<= br> > +  PolicyHeader  =3D (VAR_CHECK_POLICY_COMM_HEADER*)&am= p;CommHeader->Data;
> +  CopyGuid( &CommHeader->HeaderGuid, &gVarCheckP= olicyLibMmiHandlerGuid );
> +  CommHeader->MessageLength =3D BufferSize;
> +  PolicyHeader->Signature   =3D VAR_CHECK_POLI= CY_COMM_SIG;
> +  PolicyHeader->Revision    =3D VAR_CHECK= _POLICY_COMM_REVISION;
> +  PolicyHeader->Command     =3D VAR_= CHECK_POLICY_COMMAND_DISABLE;
> +
> +  Status =3D InternalMmCommunicate (CommHeader, &Buffer= Size);
> +  DEBUG(( DEBUG_VERBOSE, "%a - MmCommunication returne= d %r.\n", __FUNCTION__, Status ));
> +
> +  ReleaseLockOnlyAtBootTime (&mMmCommunicationLock); > +
> +  return (EFI_ERROR( Status )) ? Status : PolicyHeader->= Result;
> +}
> +
> +
> +/**
> +  This API function returns whether or not the policy engin= e is
> +  currently being enforced.
> +
> +  @param[out]   State    &nbs= p;  Pointer to a return value for whether the policy enforcement
> +          &nbs= p;            &= nbsp;    is currently enabled.
> +
> +  @retval     EFI_SUCCESS
> +  @retval     Others   &= nbsp;    An error has prevented this command from completing= .
> +
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +ProtocolIsVariablePolicyEnabled (
> +  OUT BOOLEAN     *State
> +  )
> +{
> +  EFI_STATUS        = ;            &n= bsp;           Status; > +  EFI_MM_COMMUNICATE_HEADER     &n= bsp;           *CommHeade= r;
> +  VAR_CHECK_POLICY_COMM_HEADER     = ;         *PolicyHeader;
> +  VAR_CHECK_POLICY_COMM_IS_ENABLED_PARAMS   *Comm= andParams;
> +  UINTN        &nbs= p;            &= nbsp;           &nbs= p;   BufferSize;
> +
> +  if (State =3D=3D NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  AcquireLockOnlyAtBootTime (&mMmCommunicationLock); > +
> +  // Set up the MM communication.
> +  BufferSize    =3D mMmCommunicationBufferSi= ze;
> +  CommHeader    =3D mMmCommunicationBuffer;<= br> > +  PolicyHeader  =3D (VAR_CHECK_POLICY_COMM_HEADER*)&am= p;CommHeader->Data;
> +  CommandParams =3D (VAR_CHECK_POLICY_COMM_IS_ENABLED_PARAM= S*)(PolicyHeader + 1);
> +  CopyGuid( &CommHeader->HeaderGuid, &gVarCheckP= olicyLibMmiHandlerGuid );
> +  CommHeader->MessageLength =3D BufferSize;
> +  PolicyHeader->Signature   =3D VAR_CHECK_POLI= CY_COMM_SIG;
> +  PolicyHeader->Revision    =3D VAR_CHECK= _POLICY_COMM_REVISION;
> +  PolicyHeader->Command     =3D VAR_= CHECK_POLICY_COMMAND_IS_ENABLED;
> +
> +  Status =3D InternalMmCommunicate (CommHeader, &Buffer= Size);
> +  DEBUG(( DEBUG_VERBOSE, "%a - MmCommunication returne= d %r.\n", __FUNCTION__, Status ));
> +
> +  if (!EFI_ERROR( Status )) {
> +    Status =3D PolicyHeader->Result;
> +    *State =3D CommandParams->State;
> +  }
> +
> +  ReleaseLockOnlyAtBootTime (&mMmCommunicationLock); > +
> +  return Status;
> +}
> +
> +
> +/**
> +  This API function validates and registers a new policy wi= th
> +  the policy enforcement engine.
> +
> +  @param[in]  NewPolicy     Pointe= r to the incoming policy structure.
> +
> +  @retval     EFI_SUCCESS
> +  @retval     EFI_INVALID_PARAMETER&nbs= p;  NewPolicy is NULL or is internally inconsistent.
> +  @retval     EFI_ALREADY_STARTED =     An identical matching policy already exists.
> +  @retval     EFI_WRITE_PROTECTED =     The interface has been locked until the next reboot.
> +  @retval     EFI_UNSUPPORTED &nbs= p;       Policy enforcement has been disabled= . No reason to add more policies.
> +  @retval     EFI_ABORTED  &n= bsp;          A calculation er= ror has prevented this function from completing.
> +  @retval     EFI_OUT_OF_RESOURCES = ;   Cannot grow the table to hold any more policies.
> +
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +ProtocolRegisterVariablePolicy (
> +  IN VARIABLE_POLICY_ENTRY    *NewPolicy
> +  )
> +{
> +  EFI_STATUS        = ;            &n= bsp;           Status; > +  EFI_MM_COMMUNICATE_HEADER     &n= bsp;           *CommHeade= r;
> +  VAR_CHECK_POLICY_COMM_HEADER     = ;         *PolicyHeader;
> +  VOID         = ;            &n= bsp;            = ;    *PolicyBuffer;
> +  UINTN        &nbs= p;            &= nbsp;           &nbs= p;   BufferSize;
> +  UINTN        &nbs= p;            &= nbsp;           &nbs= p;   RequiredSize;
> +
> +  if (NewPolicy =3D=3D NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  // First, make sure that the required size does not excee= d the capabilities
> +  // of the MmCommunication buffer.
> +  RequiredSize =3D OFFSET_OF(EFI_MM_COMMUNICATE_HEADER, Dat= a) + sizeof(VAR_CHECK_POLICY_COMM_HEADER);
> +  Status =3D SafeUintnAdd( RequiredSize, NewPolicy->Size= , &RequiredSize );
> +  if (EFI_ERROR( Status ) || RequiredSize > mMmCommunica= tionBufferSize) {
> +    DEBUG(( DEBUG_ERROR, "%a - Policy too la= rge for buffer! %r, %d > %d \n", __FUNCTION__,
> +          &nbs= p; Status, RequiredSize, mMmCommunicationBufferSize ));
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  AcquireLockOnlyAtBootTime (&mMmCommunicationLock); > +
> +  // Set up the MM communication.
> +  BufferSize    =3D mMmCommunicationBufferSi= ze;
> +  CommHeader    =3D mMmCommunicationBuffer;<= br> > +  PolicyHeader  =3D (VAR_CHECK_POLICY_COMM_HEADER*)&am= p;CommHeader->Data;
> +  PolicyBuffer  =3D (VOID*)(PolicyHeader + 1);
> +  CopyGuid( &CommHeader->HeaderGuid, &gVarCheckP= olicyLibMmiHandlerGuid );
> +  CommHeader->MessageLength =3D BufferSize;
> +  PolicyHeader->Signature   =3D VAR_CHECK_POLI= CY_COMM_SIG;
> +  PolicyHeader->Revision    =3D VAR_CHECK= _POLICY_COMM_REVISION;
> +  PolicyHeader->Command     =3D VAR_= CHECK_POLICY_COMMAND_REGISTER;
> +
> +  // Copy the policy into place. This copy is safe because = we've already tested above.
> +  CopyMem( PolicyBuffer, NewPolicy, NewPolicy->Size ); > +
> +  Status =3D InternalMmCommunicate (CommHeader, &Buffer= Size);
> +  DEBUG(( DEBUG_VERBOSE, "%a - MmCommunication returne= d %r.\n", __FUNCTION__, Status ));
> +
> +  ReleaseLockOnlyAtBootTime (&mMmCommunicationLock); > +
> +  return (EFI_ERROR( Status )) ? Status : PolicyHeader->= Result;
> +}
> +
> +
> +/**
> +  This helper function takes care of the overhead of format= ting, sending, and interpreting
> +  the results for a single DumpVariablePolicy request.
> +
> +  @param[in]      PageRequested&nb= sp;  The page of the paginated results from MM. 0 for metadata.
> +  @param[out]     TotalSize  =      The total size of the entire buffer. Returned as p= art of metadata.
> +  @param[out]     PageSize  &= nbsp;     The size of the current page being returned. = Not valid as part of metadata.
> +  @param[out]     HasMore  &n= bsp;      A flag indicating whether there are more= pages after this one.
> +  @param[out]     Buffer  &nb= sp;       The start of the current page from = MM.
> +
> +  @retval     EFI_SUCCESS  &n= bsp;          Output params ha= ve been updated (either metadata or dump page).
> +  @retval     EFI_INVALID_PARAMETER&nbs= p;  One of the output params is NULL.
> +  @retval     Others   &= nbsp;           &nbs= p;  Response from MM handler.
> +
> +**/
> +STATIC
> +EFI_STATUS
> +DumpVariablePolicyHelper (
> +  IN  UINT32       = PageRequested,
> +  OUT UINT32        *Tot= alSize,
> +  OUT UINT32        *Pag= eSize,
> +  OUT BOOLEAN       *HasMore,=
> +  OUT UINT8        = **Buffer
> +  )
> +{
> +  EFI_STATUS        = ;            &n= bsp;         Status;
> +  EFI_MM_COMMUNICATE_HEADER     &n= bsp;         *CommHeader;
> +  VAR_CHECK_POLICY_COMM_HEADER     = ;       *PolicyHeader;
> +  VAR_CHECK_POLICY_COMM_DUMP_PARAMS    =    *CommandParams;
> +  UINTN        &nbs= p;            &= nbsp;           &nbs= p; BufferSize;
> +
> +  if (TotalSize =3D=3D NULL || PageSize =3D=3D NULL || HasM= ore =3D=3D NULL || Buffer =3D=3D NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  // Set up the MM communication.
> +  BufferSize    =3D mMmCommunicationBufferSi= ze;
> +  CommHeader    =3D mMmCommunicationBuffer;<= br> > +  PolicyHeader  =3D (VAR_CHECK_POLICY_COMM_HEADER*)&am= p;CommHeader->Data;
> +  CommandParams =3D (VAR_CHECK_POLICY_COMM_DUMP_PARAMS*)(Po= licyHeader + 1);
> +  CopyGuid( &CommHeader->HeaderGuid, &gVarCheckP= olicyLibMmiHandlerGuid );
> +  CommHeader->MessageLength =3D BufferSize;
> +  PolicyHeader->Signature   =3D VAR_CHECK_POLI= CY_COMM_SIG;
> +  PolicyHeader->Revision    =3D VAR_CHECK= _POLICY_COMM_REVISION;
> +  PolicyHeader->Command     =3D VAR_= CHECK_POLICY_COMMAND_DUMP;
> +
> +  CommandParams->PageRequested =3D PageRequested;
> +
> +  Status =3D InternalMmCommunicate (CommHeader, &Buffer= Size);
> +  DEBUG(( DEBUG_VERBOSE, "%a - MmCommunication returne= d %r.\n", __FUNCTION__, Status ));
> +
> +  if (!EFI_ERROR( Status )) {
> +    Status =3D PolicyHeader->Result;
> +    *TotalSize =3D CommandParams->TotalSize; > +    *PageSize =3D CommandParams->PageSize;
> +    *HasMore =3D CommandParams->HasMore;
> +    *Buffer =3D (UINT8*)(CommandParams + 1);<= br> > +  }
> +
> +  return Status;
> +}
> +
> +
> +/**
> +  This API function will dump the entire contents of the va= riable policy table.
> +
> +  Similar to GetVariable, the first call can be made with a= 0 size and it will return
> +  the size of the buffer required to hold the entire table.=
> +
> +  @param[out]     Policy  Pointer = to the policy buffer. Can be NULL if Size is 0.
> +  @param[in,out]  Size    On input, the= size of the output buffer. On output, the size
> +          &nbs= p;            &= nbsp;  of the data returned.
> +
> +  @retval     EFI_SUCCESS  &n= bsp;          Policy data is i= n the output buffer and Size has been updated.
> +  @retval     EFI_INVALID_PARAMETER&nbs= p;  Size is NULL, or Size is non-zero and Policy is NULL.
> +  @retval     EFI_BUFFER_TOO_SMALL = ;   Size is insufficient to hold policy. Size updated with requir= ed size.
> +
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +ProtocolDumpVariablePolicy (
> +  OUT UINT8        =      *Policy OPTIONAL,
> +  IN OUT UINT32       &n= bsp; *Size
> +  )
> +{
> +  EFI_STATUS    Status;
> +  UINT8         *So= urce;
> +  UINT8         *De= stination;
> +  UINT32        PolicySi= ze;
> +  UINT32        PageSize= ;
> +  BOOLEAN       HasMore;
> +  UINT32        PageInde= x;
> +
> +  if (Size =3D=3D NULL || (*Size > 0 && Policy = =3D=3D NULL)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  AcquireLockOnlyAtBootTime (&mMmCommunicationLock); > +
> +  // Repeat this whole process until we either have a failu= re case or get the entire buffer.
> +  do {
> +    // First, we must check the zero page to dete= rmine the buffer size and
> +    // reset the internal state.
> +    PolicySize =3D 0;
> +    PageSize =3D 0;
> +    HasMore =3D FALSE;
> +    Status =3D DumpVariablePolicyHelper (0, &= PolicySize, &PageSize, &HasMore, &Source);
> +    if (EFI_ERROR (Status)) {
> +      break;
> +    }
> +
> +    // If we're good, we can at least check the r= equired size now.
> +    if (*Size < PolicySize) {
> +      *Size =3D PolicySize;
> +      Status =3D EFI_BUFFER_TOO_SMALL;<= br> > +      break;
> +    }
> +
> +    // On further thought, let's update the size = either way.
> +    *Size =3D PolicySize;
> +    // And get ready to ROCK.
> +    Destination =3D Policy;
> +
> +    // Keep looping and copying until we're eithe= r done or freak out.
> +    for (PageIndex =3D 1; !EFI_ERROR (Status) &am= p;& HasMore && PageIndex < MAX_UINT32; PageIndex++) = {
> +      Status =3D DumpVariablePolicyHelp= er (PageIndex, &PolicySize, &PageSize, &HasMore, &Source);<= br> > +      if (!EFI_ERROR (Status)) {
> +        CopyMem (Destination,= Source, PageSize);
> +        Destination +=3D = PageSize;
> +      }
> +    }
> +
> +    // Next, we check to see whether
> +  } while (Status =3D=3D EFI_TIMEOUT);
> +
> +  ReleaseLockOnlyAtBootTime (&mMmCommunicationLock); > +
> +  // There's currently no use for this, but it shouldn't be= hard to implement.
> +  return Status;
> +}
> +
> +
> +/**
> +  This API function locks the interface so that no more pol= icy updates
> +  can be performed or changes made to the enforcement until= the next boot.
> +
> +  @retval     EFI_SUCCESS
> +  @retval     Others   &= nbsp;    An error has prevented this command from completing= .
> +
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +ProtocolLockVariablePolicy (
> +  VOID
> +  )
> +{
> +  EFI_STATUS        = ;            Status;=
> +  EFI_MM_COMMUNICATE_HEADER     *CommHe= ader;
> +  VAR_CHECK_POLICY_COMM_HEADER  *PolicyHeader;
> +  UINTN        &nbs= p;            &= nbsp;   BufferSize;
> +
> +  AcquireLockOnlyAtBootTime (&mMmCommunicationLock); > +
> +  // Set up the MM communication.
> +  BufferSize    =3D mMmCommunicationBufferSi= ze;
> +  CommHeader    =3D mMmCommunicationBuffer;<= br> > +  PolicyHeader  =3D (VAR_CHECK_POLICY_COMM_HEADER*)&am= p;CommHeader->Data;
> +  CopyGuid( &CommHeader->HeaderGuid, &gVarCheckP= olicyLibMmiHandlerGuid );
> +  CommHeader->MessageLength =3D BufferSize;
> +  PolicyHeader->Signature   =3D VAR_CHECK_POLI= CY_COMM_SIG;
> +  PolicyHeader->Revision    =3D VAR_CHECK= _POLICY_COMM_REVISION;
> +  PolicyHeader->Command     =3D VAR_= CHECK_POLICY_COMMAND_LOCK;
> +
> +  Status =3D InternalMmCommunicate (CommHeader, &Buffer= Size);
> +  DEBUG(( DEBUG_VERBOSE, "%a - MmCommunication returne= d %r.\n", __FUNCTION__, Status ));
> +
> +  ReleaseLockOnlyAtBootTime (&mMmCommunicationLock); > +
> +  return (EFI_ERROR( Status )) ? Status : PolicyHeader->= Result;
> +}
> +
> +
> +/**
> +  This helper function locates the shared comm buffer and a= ssigns it to input pointers.
> +
> +  @param[in,out]  BufferSize    &n= bsp; On input, the minimum buffer size required INCLUDING the MM communicat= e header.
> +          &nbs= p;            &= nbsp;          On output, the = size of the matching buffer found.
> +  @param[out]     LocatedBuffer &n= bsp; A pointer to the matching buffer.
> +
> +  @retval     EFI_SUCCESS
> +  @retval     EFI_INVALID_PARAMETER&nbs= p;  One of the output pointers was NULL.
> +  @retval     EFI_OUT_OF_RESOURCES = ;   Not enough memory to allocate a comm buffer.
> +
> +**/
> +STATIC
> +EFI_STATUS
> +InitMmCommonCommBuffer (
> +  IN OUT  UINTN       *B= ufferSize,
> +  OUT     VOID    &= nbsp;   **LocatedBuffer
> +  )
> +{
> +  EFI_STATUS        = ;          Status;
> +
> +  Status =3D EFI_SUCCESS;
> +
> +  // Make sure that we're working with good pointers.
> +  if (BufferSize =3D=3D NULL || LocatedBuffer =3D=3D NULL) = {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  // Allocate the runtime memory for the comm buffer.
> +  *LocatedBuffer =3D AllocateRuntimePool (*BufferSize);
> +  if (*LocatedBuffer =3D=3D NULL) {
> +    Status =3D EFI_OUT_OF_RESOURCES;
> +    *BufferSize =3D 0;
> +  }
> +
> +  EfiInitializeLock (&mMmCommunicationLock, TPL_NOTIFY)= ;
> +
> +  return Status;
> +}
> +
> +
> +/**
> +  This helper is responsible for telemetry and any other ac= tions that
> +  need to be taken if the VariablePolicy fails to lock.
> +
> +  NOTE: It's possible that parts of this handling will need= to become
> +        part of a platform po= licy.
> +
> +  @param[in]  FailureStatus   The failure th= at was reported by LockVariablePolicy
> +
> +**/
> +STATIC
> +VOID
> +VariablePolicyHandleFailureToLock (
> +  IN  EFI_STATUS      Failure= Status
> +  )
> +{
> +  // For now, there's no agreed-upon policy for this.
> +  return;
> +}
> +
> +
> +/**
> +  ReadyToBoot Callback
> +  Lock the VariablePolicy interface if it hasn't already be= en locked.
> +
> +  @param[in]  Event     Event whos= e notification function is being invoked
> +  @param[in]  Context   Pointer to the notif= ication function's context
> +
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +LockPolicyInterfaceAtReadyToBoot (
> +  IN      EFI_EVENT  &nb= sp;            =   Event,
> +  IN      VOID   &n= bsp;            = ;      *Context
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  Status =3D ProtocolLockVariablePolicy();
> +
> +  if (EFI_ERROR( Status )) {
> +    VariablePolicyHandleFailureToLock( Status );<= br> > +  }
> +  else {
> +    gBS->CloseEvent( Event );
> +  }
> +
> +}
> +
> +
> +/**
> +  Convert internal pointer addresses to virtual addresses.<= br> > +
> +  @param[in] Event      Event whos= e notification function is being invoked.
> +  @param[in] Context    The pointer to the n= otification function's context, which
> +          &nbs= p;             = is implementation-dependent.
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +VariablePolicyVirtualAddressCallback (
> +  IN  EFI_EVENT   Event,
> +  IN  VOID        *= Context
> +  )
> +{
> +  EfiConvertPointer (0, (VOID **)&mMmCommunication); > +  EfiConvertPointer (0, (VOID **)&mMmCommunicationBuffe= r);
> +}
> +
> +
> +/**
> +  The driver's entry point.
> +
> +  @param[in] ImageHandle  The firmware allocated handl= e for the EFI image.
> +  @param[in] SystemTable  A pointer to the EFI System = Table.
> +
> +  @retval EFI_SUCCESS     The entry poi= nt executed successfully.
> +  @retval other       &n= bsp;   Some error occured when executing this entry point.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +VariablePolicySmmDxeMain (
> +  IN    EFI_HANDLE    &n= bsp;            = ; ImageHandle,
> +  IN    EFI_SYSTEM_TABLE   &n= bsp;        *SystemTable
> +  )
> +{
> +  EFI_STATUS        = ;      Status;
> +  BOOLEAN        &n= bsp;        ProtocolInstalled;
> +  BOOLEAN        &n= bsp;        CallbackRegistered;
> +  BOOLEAN        &n= bsp;        VirtualAddressChangeRegister= ed;
> +  EFI_EVENT        =        ReadyToBootEvent;
> +  EFI_EVENT        =        VirtualAddressChangeEvent;
> +
> +  Status =3D EFI_SUCCESS;
> +  ProtocolInstalled =3D FALSE;
> +  CallbackRegistered =3D FALSE;
> +  VirtualAddressChangeRegistered =3D FALSE;
> +
> +  // Update the minimum buffer size.
> +  mMmCommunicationBufferSize =3D VAR_CHECK_POLICY_MM_COMM_B= UFFER_SIZE;
> +  // Locate the shared comm buffer to use for sending MM co= mmands.
> +  Status =3D InitMmCommonCommBuffer( &mMmCommunicationB= ufferSize, &mMmCommunicationBuffer );
> +  if (EFI_ERROR( Status )) {
> +    DEBUG((DEBUG_ERROR, "%a - Failed to loca= te a viable MM comm buffer! %r\n", __FUNCTION__, Status));
> +    ASSERT_EFI_ERROR( Status );
> +    return Status;
> +  }
> +
> +  // Locate the MmCommunication protocol.
> +  Status =3D gBS->LocateProtocol( &gEfiMmCommunicati= on2ProtocolGuid, NULL, (VOID**)&mMmCommunication );
> +  if (EFI_ERROR( Status )) {
> +    DEBUG((DEBUG_ERROR, "%a - Failed to loca= te MmCommunication protocol! %r\n", __FUNCTION__, Status));
> +    ASSERT_EFI_ERROR( Status );
> +    return Status;
> +  }
> +
> +  // Configure the VariablePolicy protocol structure.
> +  mVariablePolicyProtocol.Revision    &= nbsp;           =3D EDKII= _VARIABLE_POLICY_PROTOCOL_REVISION;
> +  mVariablePolicyProtocol.DisableVariablePolicy  = =3D ProtocolDisableVariablePolicy;
> +  mVariablePolicyProtocol.IsVariablePolicyEnabled =3D Proto= colIsVariablePolicyEnabled;
> +  mVariablePolicyProtocol.RegisterVariablePolicy  =3D = ProtocolRegisterVariablePolicy;

(2) "error: assignment from incompatible pointer type [-Werror]"=

Because, with my local modification for (1), the LHS now takes a
pointer-to-CONST, but the RHS is declared (in the present patch) as

  STATIC
  EFI_STATUS
  EFIAPI
  ProtocolRegisterVariablePolicy (
    IN VARIABLE_POLICY_ENTRY    *NewPolicy     )

So I modified this too, to take a pointer-to-CONST. That allowed the
builds to complete.

Therefore I suggest one of the following:

(a) If CONST is not important in the RegisterVariablePolicy() lib class API, then please remove CONST from there (and the implementation(s) of
that function).

(b) Alternatively, if CONST is preferred in the library, then

(b1) please squash the following hunk:

> diff --git a/MdeModulePkg/Include/Protocol/VariablePolicy.h b/MdeModu= lePkg/Include/Protocol/VariablePolicy.h
> index 30d6c155ae6a..83b6a999df07 100644
> --- a/MdeModulePkg/Include/Protocol/VariablePolicy.h
> +++ b/MdeModulePkg/Include/Protocol/VariablePolicy.h
> @@ -102,7 +102,7 @@ EFI_STATUS
>  typedef
>  EFI_STATUS
>  (EFIAPI *REGISTER_VARIABLE_POLICY)(
> -  IN VARIABLE_POLICY_ENTRY *PolicyEntry
> +  IN CONST VARIABLE_POLICY_ENTRY *PolicyEntry
>    );
>
>  /**

into patch "MdeModulePkg: Define the VariablePolicy protocol interfac= e".

(b2) And please squash the following hunk:

> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolic= ySmmDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe= .c
> index 3d799025983a..e2d4cf4cec1a 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe= .c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable= PolicySmmDxe.c
> @@ -176,7 +176,7 @@ STATIC
>  EFI_STATUS
>  EFIAPI
>  ProtocolRegisterVariablePolicy (
> -  IN VARIABLE_POLICY_ENTRY    *NewPolicy
> +  IN CONST VARIABLE_POLICY_ENTRY    *NewPoli= cy
>    )
>  {
>    EFI_STATUS       = ;            &n= bsp;            Stat= us;

into the present patch.

I've used (b1)+(b2) locally, and those together allow the builds to complete.


Now, I wonder why the GCC / OVMF build(s) in the github.com CI did not
report this problem. According to the C99 standard, "6.7.5.3 Function=
declarators (including prototypes)", paragraph 15:

  For two function types to be compatible, both shall specify compati= ble
  return types. [...]  Moreover, the parameter type lists, if bo= th are
  present, shall agree in the number of parameters and in use of the<= br>   ellipsis terminator; corresponding parameters shall have compatible=
  types. [...]

Furthermore, from "6.7.5.1 Pointer declarators", paragraph 2:
  For two pointer types to be compatible, both shall be identically   qualified and both shall be pointers to compatible types.

Yet further, from "6.5.16.1 Simple assignment", paragraph 1:

  One of the following shall hold:

  [...]

  - both operands are pointers to qualified or unqualified versions o= f
    compatible types, and the type pointed to by the left h= as all the
    qualifiers of the type pointed to by the right;

  [...]

So the thought process is:

- "mVariablePolicyProtocol.RegisterVariablePolicy" has type

    EFI_STATUS (EFIAPI *)(      VA= RIABLE_POLICY_ENTRY *)

  while a pointer to RegisterVariablePolicy(), from the library, has<= br>   type

    EFI_STATUS (EFIAPI *)(const VARIABLE_POLICY_ENTRY *)
  For the assignment, these need to be compatible, per 6.5.16.1p1.
  Are they compatible?

- Per 6.7.5.1p2, that depends on whether the pointed-to function types
  are compatible.

  Are the pointed-to function types compatible?

- Per 6.7.5.3p15, that depends on whether (VARIABLE_POLICY_ENTRY *) and   (const VARIABLE_POLICY_ENTRY *) are compatible.

  Are those pointer types compatible?

- They're not, based on 6.7.5.1p2 -- they are not identically qualified.
So IMO (according to the language standard), the code should have been
flagged by MSVC too; but more importantly, I don't understand why the
GCC5 / Ubuntu builds succeeded in CI.

Thanks!
Laszlo


> +  mVariablePolicyProtocol.DumpVariablePolicy  &nb= sp;   =3D ProtocolDumpVariablePolicy;
> +  mVariablePolicyProtocol.LockVariablePolicy  &nb= sp;   =3D ProtocolLockVariablePolicy;
> +
> +  // Register all the protocols and return the status.
> +  Status =3D gBS->InstallMultipleProtocolInterfaces( &am= p;ImageHandle,
> +          &nbs= p;            &= nbsp;           &nbs= p;            &= nbsp;  &gEdkiiVariablePolicyProtocolGuid, &mVariablePolicyProt= ocol,
> +          &nbs= p;            &= nbsp;           &nbs= p;            &= nbsp;  NULL );
> +  if (EFI_ERROR( Status )) {
> +    DEBUG(( DEBUG_ERROR, "%a - Failed to ins= tall protocol! %r\n", __FUNCTION__, Status ));
> +    goto Exit;
> +  }
> +  else {
> +    ProtocolInstalled =3D TRUE;
> +  }
> +
> +  //
> +  // Register a callback for ReadyToBoot so that the interf= ace is at least locked before
> +  // dispatching any bootloaders or UEFI apps.
> +  Status =3D gBS->CreateEventEx( EVT_NOTIFY_SIGNAL,
> +          &nbs= p;            &= nbsp;       TPL_CALLBACK,
> +          &nbs= p;            &= nbsp;       LockPolicyInterfaceAtReadyToBoot,=
> +          &nbs= p;            &= nbsp;       NULL,
> +          &nbs= p;            &= nbsp;       &gEfiEventReadyToBootGuid, > +          &nbs= p;            &= nbsp;       &ReadyToBootEvent );
> +  if (EFI_ERROR( Status )) {
> +    DEBUG(( DEBUG_ERROR, "%a - Failed to cre= ate ReadyToBoot event! %r\n", __FUNCTION__, Status ));
> +    goto Exit;
> +  }
> +  else {
> +    CallbackRegistered =3D TRUE;
> +  }
> +
> +  //
> +  // Register a VirtualAddressChange callback for the MmCom= m protocol and Comm buffer.
> +  Status =3D gBS->CreateEventEx (EVT_NOTIFY_SIGNAL,
> +          &nbs= p;            &= nbsp;        TPL_NOTIFY,
> +          &nbs= p;            &= nbsp;        VariablePolicyVirtualAddres= sCallback,
> +          &nbs= p;            &= nbsp;        NULL,
> +          &nbs= p;            &= nbsp;        &gEfiEventVirtualAddres= sChangeGuid,
> +          &nbs= p;            &= nbsp;        &VirtualAddressChangeEv= ent);
> +  if (EFI_ERROR( Status )) {
> +    DEBUG(( DEBUG_ERROR, "%a - Failed to cre= ate VirtualAddressChange event! %r\n", __FUNCTION__, Status ));
> +    goto Exit;
> +  }
> +  else {
> +    VirtualAddressChangeRegistered =3D TRUE;
> +  }
> +
> +
> +Exit:
> +  //
> +  // If we're about to return a failed status (and unload t= his driver), we must first undo anything that
> +  // has been successfully done.
> +  if (EFI_ERROR( Status )) {
> +    if (ProtocolInstalled) {
> +      gBS->UninstallProtocolInterfac= e( &ImageHandle, &gEdkiiVariablePolicyProtocolGuid, &mVariableP= olicyProtocol );
> +    }
> +    if (CallbackRegistered) {
> +      gBS->CloseEvent( ReadyToBootEv= ent );
> +    }
> +    if (VirtualAddressChangeRegistered) {
> +      gBS->CloseEvent( VirtualAddres= sChangeEvent );
> +    }
> +  }
> +
> +  return Status;
> +}
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRu= ntimeDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeD= xe.c
> index 663a1aaa128f..c47e614d81f4 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx= e.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable= SmmRuntimeDxe.c
> @@ -65,6 +65,17 @@ EFI_LOCK      &n= bsp;            = ;      mVariableServicesLock;
>  EDKII_VARIABLE_LOCK_PROTOCOL     mVariableL= ock;
>  EDKII_VAR_CHECK_PROTOCOL      &nb= sp;  mVarCheck;
>
> +/**
> +  The logic to initialize the VariablePolicy engine is in i= ts own file.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +VariablePolicySmmDxeMain (
> +  IN    EFI_HANDLE    &n= bsp;            = ; ImageHandle,
> +  IN    EFI_SYSTEM_TABLE   &n= bsp;        *SystemTable
> +  );
> +
>  /**
>    Some Secure Boot Policy Variable may update followi= ng other variable changes(SecureBoot follows PK change, etc).
>    Record their initial State when variable write serv= ice is ready.
> @@ -1796,6 +1807,9 @@ VariableSmmRuntimeInitialize (
>           &mVir= tualAddressChangeEvent
>           );
>
> +  // Initialize the VariablePolicy protocol and engine.
> +  VariablePolicySmmDxeMain (ImageHandle, SystemTable);
> +
>    return EFI_SUCCESS;
>  }
>
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRunti= meDxe.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.i= nf
> index ceea5d1ff9ac..48ac167906f7 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.i= nf
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable= RuntimeDxe.inf
> @@ -10,6 +10,7 @@
>  #  buffer overflow or integer overflow.
>  #
>  # Copyright (c) 2006 - 2019, Intel Corporation. All rights rese= rved.<BR>
> +# Copyright (c) Microsoft Corporation.
>  # SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
>  ##
> @@ -69,6 +70,7 @@ [LibraryClasses]
>    TpmMeasurementLib
>    AuthVariableLib
>    VarCheckLib
> +  VariablePolicyLib
>
>  [Protocols]
>    gEfiFirmwareVolumeBlockProtocolGuid  &nbs= p;        ## CONSUMES
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.i= nf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> index bc3033588d40..bbc8d2080193 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable= Smm.inf
> @@ -19,6 +19,7 @@
>  #  the authentication service provided in this driver will= be broken, and the behavior is undefined.
>  #
>  # Copyright (c) 2010 - 2019, Intel Corporation. All rights rese= rved.<BR>
> +# Copyright (c) Microsoft Corporation.
>  # SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
>  ##
> @@ -78,6 +79,8 @@ [LibraryClasses]
>    AuthVariableLib
>    VarCheckLib
>    UefiBootServicesTableLib
> +  VariablePolicyLib
> +  VariablePolicyHelperLib
>
>  [Protocols]
>    gEfiSmmFirmwareVolumeBlockProtocolGuid  &= nbsp;     ## CONSUMES
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRu= ntimeDxe.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntim= eDxe.inf
> index 01564e4c5068..f217530b2985 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx= e.inf
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable= SmmRuntimeDxe.inf
> @@ -14,6 +14,7 @@
>  #  the authentication service provided in this driver will= be broken, and the behavior is undefined.
>  #
>  # Copyright (c) 2010 - 2019, Intel Corporation. All rights rese= rved.<BR>
> +# Copyright (c) Microsoft Corporation.<BR>
>  # SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
>  ##
> @@ -42,6 +43,7 @@ [Sources]
>    VariableParsing.c
>    VariableParsing.h
>    Variable.h
> +  VariablePolicySmmDxe.c
>
>  [Packages]
>    MdePkg/MdePkg.dec
> @@ -56,6 +58,8 @@ [LibraryClasses]
>    DxeServicesTableLib
>    UefiDriverEntryPoint
>    TpmMeasurementLib
> +  SafeIntLib
> +  PcdLib
>
>  [Protocols]
>    gEfiVariableWriteArchProtocolGuid   =           ## PRODUCES
> @@ -67,11 +71,15 @@ [Protocols]
>    gEfiSmmVariableProtocolGuid
>    gEdkiiVariableLockProtocolGuid   &nb= sp;            ## PR= ODUCES
>    gEdkiiVarCheckProtocolGuid    &= nbsp;           &nbs= p;   ## PRODUCES
> +  gEdkiiVariablePolicyProtocolGuid    &= nbsp;         ## PRODUCES
>
>  [FeaturePcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRun= timeCache           ## CO= NSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdVariableCollectSt= atistics            = ## CONSUMES
>
> +[Pcd]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdAllowVariablePolicyEnfo= rcementDisable     ## CONSUMES
> +
>  [Guids]
>    ## PRODUCES      &nbs= p;      ## GUID # Signature of Variable store head= er
>    ## CONSUMES      &nbs= p;      ## GUID # Signature of Variable store head= er
> @@ -99,6 +107,8 @@ [Guids]
>    ## SOMETIMES_CONSUMES   ## Variable:L&quo= t;dbt"
>    gEfiImageSecurityDatabaseGuid
>
> +  gVarCheckPolicyLibMmiHandlerGuid
> +
>  [Depex]
>    gEfiMmCommunication2ProtocolGuid
>
>


 

--_000_CY4PR21MB07438938D2371F37C6DEE6B1EF8B0CY4PR21MB0743namp_--