From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.81]) by mx.groups.io with SMTP id smtpd.web12.22797.1590183699626782404 for ; Fri, 22 May 2020 14:41:40 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=Hgdw1Ef3; spf=pass (domain: redhat.com, ip: 207.211.31.81, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1590183698; 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=GkzJ82D1uAZB9KXM7Ieg/txTPQEClGrH4eChISWa3Ww=; b=Hgdw1Ef3S3xjXGQxdG9U02wn8kC5jS2PWsjuF711OlxVPh0uSHNk2exlNRkOhdDuyWvFHq I8KfXszdKkj4AXu7XVGgKjasWPSRzr3YCk/yFdN5t2xqxvw67LvB4lM3/+B7qYtJ70rkN2 yezBD2Lfwgp9VbkWtN40rSHAIYN5vzM= 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-143-8Au2jq9DPuCRixy5FWAIRA-1; Fri, 22 May 2020 17:41:12 -0400 X-MC-Unique: 8Au2jq9DPuCRixy5FWAIRA-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 22A46107ACCA; Fri, 22 May 2020 21:41:11 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-40.ams2.redhat.com [10.36.112.40]) by smtp.corp.redhat.com (Postfix) with ESMTP id 900C45C1D0; Fri, 22 May 2020 21:41:09 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v3 05/14] OvmfPkg: Add VariablePolicy engine to OvmfPkg platform To: devel@edk2.groups.io, michael.kubacki@outlook.com Cc: Jordan Justen , Ard Biesheuvel , Bret Barkelew References: <20200521224331.15616-1-michael.kubacki@outlook.com> From: "Laszlo Ersek" Message-ID: <2e4a7b3c-a732-48bd-27b2-d3aef4b1690a@redhat.com> Date: Fri, 22 May 2020 23:41:08 +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=utf-8 Content-Transfer-Encoding: 7bit Hello Michael / Bret, I don't understand the (lack of) updates in this patch: On 05/22/20 00:43, Michael Kubacki wrote: > From: Bret Barkelew > > https://bugzilla.tianocore.org/show_bug.cgi?id=2522 > > Cc: Jordan Justen > Cc: Laszlo Ersek > Cc: Ard Biesheuvel > Cc: Bret Barkelew > Signed-off-by: Michael Kubacki > --- > OvmfPkg/OvmfPkgIa32.dsc | 8 ++++++++ > OvmfPkg/OvmfPkgIa32X64.dsc | 8 ++++++++ > OvmfPkg/OvmfPkgX64.dsc | 8 ++++++++ > OvmfPkg/OvmfXen.dsc | 7 +++++++ My request (1) under the corresponding v2 patch was to include the OvmfXen.dsc platform in the modifications. That request has been addressed. Please find said v2 feedback from me here: http://mid.mail-archive.com/a0e0e3d4-6712-078a-4d95-29408109b0b0@redhat.com (Alternative link: .) However: > 4 files changed, 31 insertions(+) > > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc > index cbc5f0e583bc..2c64591f88a3 100644 > --- a/OvmfPkg/OvmfPkgIa32.dsc > +++ b/OvmfPkg/OvmfPkgIa32.dsc > @@ -3,6 +3,7 @@ > # > # Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP
> +# Copyright (c) Microsoft Corporation.
> # > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > @@ -194,6 +195,8 @@ [LibraryClasses] > AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf > !endif > VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf > + VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf > + VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf > > > # > @@ -327,6 +330,7 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER] > BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf > PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf > QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf > + VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf > > [LibraryClasses.common.UEFI_DRIVER] > PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf > @@ -480,6 +484,9 @@ [PcdsFixedAtBuild] > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize|0x40000 > !endif > > + # Optional: Omit if VariablePolicy should be always-on. > + gEfiMdeModulePkgTokenSpaceGuid.PcdAllowVariablePolicyEnforcementDisable|TRUE > + My request (2) in the same message was to drop this PCD setting. That request has not been addressed. So I'm now confused -- based on addressing (1), it seems like my v2 review has been processed. But then, why was my request (2) silently ignored? Did you miss it somehow? ... Maybe you entirely missed my message that I posted in response to version 2 of this specific patch (i.e. you may have fully missed the message I link at the top). That could be the case because I mentioned "OvmfXen.dsc" under the v2 blurb as well. So perhaps you only read my feedback to the blurb. In v4, please remove the "PcdAllowVariablePolicyEnforcementDisable" setting. The reason why I'm requesting that is captured in my v2 feedback (see link near the top). Thanks, Laszlo > gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 > > gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 > @@ -921,6 +928,7 @@ [Components] > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf { > > NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf > + NULL|MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf > } > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf > > diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc > index 6d69cc6cb56f..99527e03b9d0 100644 > --- a/OvmfPkg/OvmfPkgIa32X64.dsc > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc > @@ -3,6 +3,7 @@ > # > # Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP
> +# Copyright (c) Microsoft Corporation.
> # > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > @@ -198,6 +199,8 @@ [LibraryClasses] > AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf > !endif > VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf > + VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf > + VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf > > > # > @@ -331,6 +334,7 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER] > BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf > PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf > QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf > + VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf > > [LibraryClasses.common.UEFI_DRIVER] > PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf > @@ -484,6 +488,9 @@ [PcdsFixedAtBuild] > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize|0x40000 > !endif > > + # Optional: Omit if VariablePolicy should be always-on. > + gEfiMdeModulePkgTokenSpaceGuid.PcdAllowVariablePolicyEnforcementDisable|TRUE > + > gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 > > gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 > @@ -934,6 +941,7 @@ [Components.X64] > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf { > > NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf > + NULL|MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf > } > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf > > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc > index 5ad4f461ce52..4a6b18d7899d 100644 > --- a/OvmfPkg/OvmfPkgX64.dsc > +++ b/OvmfPkg/OvmfPkgX64.dsc > @@ -3,6 +3,7 @@ > # > # Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP
> +# Copyright (c) Microsoft Corporation.
> # > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > @@ -198,6 +199,8 @@ [LibraryClasses] > AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf > !endif > VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf > + VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf > + VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf > > > # > @@ -331,6 +334,7 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER] > BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf > PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf > QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf > + VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf > > [LibraryClasses.common.UEFI_DRIVER] > PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf > @@ -484,6 +488,9 @@ [PcdsFixedAtBuild] > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize|0x40000 > !endif > > + # Optional: Omit if VariablePolicy should be always-on. > + gEfiMdeModulePkgTokenSpaceGuid.PcdAllowVariablePolicyEnforcementDisable|TRUE > + > gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 > > gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 > @@ -932,6 +939,7 @@ [Components] > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf { > > NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf > + NULL|MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf > } > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf > > diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc > index 47ee8db8b884..c2d476133b9d 100644 > --- a/OvmfPkg/OvmfXen.dsc > +++ b/OvmfPkg/OvmfXen.dsc > @@ -3,6 +3,7 @@ > # > # Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP
> +# Copyright (c) Microsoft Corporation.
> # Copyright (c) 2019, Citrix Systems, Inc. > # > # SPDX-License-Identifier: BSD-2-Clause-Patent > @@ -182,6 +183,8 @@ [LibraryClasses] > > AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf > VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf > + VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf > + VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf > > > # > @@ -301,6 +304,7 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER] > BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf > PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf > QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf > + VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf > > [LibraryClasses.common.UEFI_DRIVER] > PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf > @@ -394,6 +398,9 @@ [PcdsFixedAtBuild] > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize|0x40000 > !endif > > + # Optional: Omit if VariablePolicy should be always-on. > + gEfiMdeModulePkgTokenSpaceGuid.PcdAllowVariablePolicyEnforcementDisable|TRUE > + > gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 > > gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 >