From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.120]) by mx.groups.io with SMTP id smtpd.web11.2911.1589364630955914501 for ; Wed, 13 May 2020 03:10:32 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=dd6K0ky0; spf=pass (domain: redhat.com, ip: 205.139.110.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1589364629; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=sC0ZINrhq8N7ONcrDOlRAqcoAHRVNWoy6ZvGvlPd3UU=; b=dd6K0ky0Rr0lGadBvc1K6AuUhsixE5Q49SQ6Wfp2YToNRd14gjYhOa6Ua/ssRq3UIU8IG4 LAmxPuFpbZQqa+ZiuBNXMGRb6QpibnfpSX5yMNv+xQlMaeyqcPJCV40nE+W60MiExQEWd7 eai7Ug86uWcDi/VseSkb7OyDIZClIwo= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-128-3E3P55_YPdK-NBthuUr9iA-1; Wed, 13 May 2020 06:10:23 -0400 X-MC-Unique: 3E3P55_YPdK-NBthuUr9iA-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id BAE1B460; Wed, 13 May 2020 10:10:21 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-115-95.ams2.redhat.com [10.36.115.95]) by smtp.corp.redhat.com (Postfix) with ESMTP id 0D6A65C1C3; Wed, 13 May 2020 10:10:19 +0000 (UTC) Subject: Re: [EXTERNAL] Re: [edk2-devel] [PATCH v2 01/12] MdeModulePkg: Define the VariablePolicy protocol interface To: Bret Barkelew , "devel@edk2.groups.io" , "michael.kubacki@outlook.com" Cc: Jian J Wang , Hao A Wu , "liming.gao" References: <20200512064635.14640-1-michael.kubacki@outlook.com> From: "Laszlo Ersek" Message-ID: <79e3ca51-26ee-1f51-1bc9-e8cbecdccedd@redhat.com> Date: Wed, 13 May 2020 12:10:19 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: 8bit On 05/13/20 06:51, Bret Barkelew wrote: > I don’t entirely disagree with the name suggestion, but it’s pretty late in the process. If it’s not a hard-stop, I’d rather not. I'll let the MdeModulePkg maintainers determine the severity. > Other change has been made. Thanks Laszlo > From: Laszlo Ersek via groups.io > Sent: Tuesday, May 12, 2020 5:19 AM > To: devel@edk2.groups.io; michael.kubacki@outlook.com > Cc: Jian J Wang; Hao A Wu; liming.gao > Subject: [EXTERNAL] Re: [edk2-devel] [PATCH v2 01/12] MdeModulePkg: Define the VariablePolicy protocol interface > > On 05/12/20 08:46, Michael Kubacki wrote: >> From: Bret Barkelew >> >> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2522&data=02%7C01%7CBret.Barkelew%40microsoft.com%7Cf9d61ace6d7d42a2b9c008d7f66eb4b3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637248827657827126&sdata=s80j2lvjZROfSb9GR6g0NO0FwGN2c18v9Im8pmRRenE%3D&reserved=0 >> >> VariablePolicy is an updated interface to >> replace VarLock and VarCheckProtocol. >> >> Add the VariablePolicy protocol interface >> header and add to the MdeModulePkg.dec file. >> >> Cc: Jian J Wang >> Cc: Hao A Wu >> Cc: Liming Gao >> Signed-off-by: Michael Kubacki >> --- >> MdeModulePkg/Include/Protocol/VariablePolicy.h | 157 ++++++++++++++++++++ >> MdeModulePkg/MdeModulePkg.dec | 14 +- >> 2 files changed, 170 insertions(+), 1 deletion(-) >> >> diff --git a/MdeModulePkg/Include/Protocol/VariablePolicy.h b/MdeModulePkg/Include/Protocol/VariablePolicy.h >> new file mode 100644 >> index 000000000000..2cd025860554 >> --- /dev/null >> +++ b/MdeModulePkg/Include/Protocol/VariablePolicy.h >> @@ -0,0 +1,157 @@ >> +/** @file -- VariablePolicy.h >> + >> +This protocol allows communication with Variable Policy Engine. >> + >> +Copyright (c) Microsoft Corporation. >> +SPDX-License-Identifier: BSD-2-Clause-Patent >> +**/ >> + >> +#ifndef __VARIABLE_POLICY_PROTOCOL__ >> +#define __VARIABLE_POLICY_PROTOCOL__ >> + >> +#define VARIABLE_POLICY_PROTOCOL_REVISION 0x0000000000010000 >> + >> +#define VARIABLE_POLICY_PROTOCOL_GUID \ >> + { \ >> + 0x81D1675C, 0x86F6, 0x48DF, { 0xBD, 0x95, 0x9A, 0x6E, 0x4F, 0x09, 0x25, 0xC3 } \ >> + } >> + >> +#define VARIABLE_POLICY_ENTRY_REVISION 0x00010000 >> + >> +#pragma pack(push, 1) >> +typedef struct { >> + UINT32 Version; >> + UINT16 Size; >> + UINT16 OffsetToName; >> + EFI_GUID Namespace; >> + UINT32 MinSize; >> + UINT32 MaxSize; >> + UINT32 AttributesMustHave; >> + UINT32 AttributesCantHave; >> + UINT8 LockPolicyType; >> + UINT8 Padding[3]; >> + // UINT8 LockPolicy[]; // Variable Length Field >> + // CHAR16 Name[] // Variable Length Field >> +} VARIABLE_POLICY_ENTRY; >> + >> +#define VARIABLE_POLICY_NO_MIN_SIZE 0 >> +#define VARIABLE_POLICY_NO_MAX_SIZE MAX_UINT32 >> +#define VARIABLE_POLICY_NO_MUST_ATTR 0 >> +#define VARIABLE_POLICY_NO_CANT_ATTR 0 >> + >> +#define VARIABLE_POLICY_TYPE_NO_LOCK 0 >> +#define VARIABLE_POLICY_TYPE_LOCK_NOW 1 >> +#define VARIABLE_POLICY_TYPE_LOCK_ON_CREATE 2 >> +#define VARIABLE_POLICY_TYPE_LOCK_ON_VAR_STATE 3 >> + >> +typedef struct { >> + EFI_GUID Namespace; >> + UINT8 Value; >> + UINT8 Padding; >> + // CHAR16 Name[]; // Variable Length Field >> +} VARIABLE_LOCK_ON_VAR_STATE_POLICY; >> +#pragma pack(pop) >> + >> +/** >> + 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 boot. >> + @retval EFI_WRITE_PROTECTED Interface has been locked until reboot. >> + @retval EFI_WRITE_PROTECTED Interface option is disabled by platform PCD. >> + >> +**/ >> +typedef >> +EFI_STATUS >> +(EFIAPI *DISABLE_VARIABLE_POLICY)( >> + VOID >> + ); >> + >> +/** >> + 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 policy enforcement >> + is currently enabled. >> + >> + @retval EFI_SUCCESS >> + @retval Others An error has prevented this command from completing. >> + >> +**/ >> +typedef >> +EFI_STATUS >> +(EFIAPI *IS_VARIABLE_POLICY_ENABLED)( >> + OUT BOOLEAN *State >> + ); >> + >> +/** >> + 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 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_ABORTED A calculation error has prevented this function from completing. >> + @retval EFI_OUT_OF_RESOURCES Cannot grow the table to hold any more policies. >> + >> +**/ >> +typedef >> +EFI_STATUS >> +(EFIAPI *REGISTER_VARIABLE_POLICY)( >> + IN VARIABLE_POLICY_ENTRY *PolicyEntry >> + ); >> + >> +/** >> + This API function will dump the entire contents of the variable 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 >> + of the data returned. >> + >> + @retval EFI_SUCCESS Policy data is in the output buffer 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 policy. Size updated with required size. >> + >> +**/ >> +typedef >> +EFI_STATUS >> +(EFIAPI *DUMP_VARIABLE_POLICY)( >> + IN OUT UINT8 *Policy, >> + IN OUT UINT32 *Size >> + ); >> + >> +/** >> + This API function locks the interface so that no more policy updates >> + can be performed or changes made to the enforcement until the next boot. >> + >> + @retval EFI_SUCCESS >> + @retval Others An error has prevented this command from completing. >> + >> +**/ >> +typedef >> +EFI_STATUS >> +(EFIAPI *LOCK_VARIABLE_POLICY)( >> + VOID >> + ); >> + >> +typedef struct { >> + UINT64 Revision; >> + DISABLE_VARIABLE_POLICY DisableVariablePolicy; >> + IS_VARIABLE_POLICY_ENABLED IsVariablePolicyEnabled; >> + REGISTER_VARIABLE_POLICY RegisterVariablePolicy; >> + DUMP_VARIABLE_POLICY DumpVariablePolicy; >> + LOCK_VARIABLE_POLICY LockVariablePolicy; >> +} _VARIABLE_POLICY_PROTOCOL; >> + >> +typedef _VARIABLE_POLICY_PROTOCOL VARIABLE_POLICY_PROTOCOL; >> + >> +extern EFI_GUID gVariablePolicyProtocolGuid; >> + >> +#endif >> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec >> index 4f44af694862..f74fea00b6e7 100644 >> --- a/MdeModulePkg/MdeModulePkg.dec >> +++ b/MdeModulePkg/MdeModulePkg.dec >> @@ -8,7 +8,7 @@ >> # Copyright (c) 2016, Linaro Ltd. All rights reserved.
>> # (C) Copyright 2016 - 2019 Hewlett Packard Enterprise Development LP
>> # Copyright (c) 2017, AMD Incorporated. All rights reserved.
>> -# Copyright (c) 2016, Microsoft Corporation
>> +# Copyright (c) Microsoft Corporation.
>> # SPDX-License-Identifier: BSD-2-Clause-Patent >> # >> ## >> @@ -624,6 +624,9 @@ >> # 0x80000006 | Incorrect error code provided. >> # >> >> + ## Include/Protocol/VariablePolicy.h >> + gVariablePolicyProtocolGuid = { 0x81D1675C, 0x86F6, 0x48DF, { 0xBD, 0x95, 0x9A, 0x6E, 0x4F, 0x09, 0x25, 0xC3 } } >> + > > (1) Should be called gEdkiiVariablePolicyProtocolGuid, IMO. > > Similarly, all VARIABLE_POLICY_PROTOCOL substrings should be > EDKII_VARIABLE_POLICY_PROTOCOL, in the protocol header file, I believe. > >> [PcdsFeatureFlag] >> ## Indicates if the platform can support update capsule across a system reset.

>> # TRUE - Supports update capsule across a system reset.
>> @@ -1129,6 +1132,15 @@ >> # @Prompt Variable storage size. >> gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0x10000|UINT32|0x30000005 >> >> + ## Toggle for whether the VariablePolicy engine should allow disabling. >> + # The engine is enabled at power-on, but the interface allows the platform to >> + # disable enforcement for servicing flexibility. If this PCD is disabled, it will block the ability to >> + # disable the enforcement and VariablePolicy enforcement will always be ON. >> + # TRUE - VariablePolicy can be disabled by request through the interface (until interface is locked) >> + # FALSE - VariablePolicy interface will not accept requests to disable and is ALWAYS ON >> + # @Prompt Allow VariablePolicy enforcement to be disabled. >> + gEfiMdeModulePkgTokenSpaceGuid.PcdAllowVariablePolicyEnforcementDisable|FALSE|BOOLEAN|0x30000020 >> + >> ## FFS filename to find the ACPI tables. >> # @Prompt FFS name of ACPI tables storage. >> gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiTableStorageFile|{ 0x25, 0x4e, 0x37, 0x7e, 0x01, 0x8e, 0xee, 0x4f, 0x87, 0xf2, 0x39, 0xc, 0x23, 0xc6, 0x6, 0xcd }|VOID*|0x30000016 >> > > (2) This patch should update "MdeModulePkg.uni" in tandem with > "MdeModulePkg.dec", I think. > > Thanks > Laszlo > > > > >