[EXTERNAL] Re: [edk2-devel] [PATCH v3 05/14] OvmfPkg: Add VariablePolicy engine to OvmfPkg platform
Bret Barkelew via groups.io
bret.barkelew=microsoft.com at groups.io
Fri May 22 22:35:29 UTC 2020
“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.”
Yup. That’s the one. Saw this request, but not the patch feedback. Will address in the next version.
You want the PCD dropped just for Xen?
I would posit that dropping it for all of OVMF would negate the ability to use the unittest test to confirm the functionality in CI, which is something I would like to light up in future revisions, so I would need to hear the argument against it.
- Bret
From: Laszlo Ersek<mailto:lersek at redhat.com>
Sent: Friday, May 22, 2020 2:41 PM
To: devel at edk2.groups.io<mailto:devel at edk2.groups.io>; michael.kubacki at outlook.com<mailto:michael.kubacki at outlook.com>
Cc: Jordan Justen<mailto:jordan.l.justen at intel.com>; Ard Biesheuvel<mailto:ard.biesheuvel at arm.com>; Bret Barkelew<mailto:Bret.Barkelew at microsoft.com>
Subject: [EXTERNAL] Re: [edk2-devel] [PATCH v3 05/14] OvmfPkg: Add VariablePolicy engine to OvmfPkg platform
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 <brbarkel at microsoft.com>
>
> 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%7C10ad08aa2f8b41619cdc08d7fe98e2f3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637257804909549643&sdata=f2vsskZHgV2wgxtcFqxigKPVpSyz6hBE1Y74qEsAWTs%3D&reserved=0
>
> Cc: Jordan Justen <jordan.l.justen at intel.com>
> Cc: Laszlo Ersek <lersek at redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel at arm.com>
> Cc: Bret Barkelew <brbarkel at microsoft.com>
> Signed-off-by: Michael Kubacki <michael.kubacki at microsoft.com>
> ---
> 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:
https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmid.mail-archive.com%2Fa0e0e3d4-6712-078a-4d95-29408109b0b0%40redhat.com&data=02%7C01%7CBret.Barkelew%40microsoft.com%7C10ad08aa2f8b41619cdc08d7fe98e2f3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637257804909549643&sdata=8ajjcCIXFpvLAylSu9SBamajUhApYVtsJ6hUZQsG%2BXQ%3D&reserved=0
(Alternative link: <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F59271&data=02%7C01%7CBret.Barkelew%40microsoft.com%7C10ad08aa2f8b41619cdc08d7fe98e2f3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637257804909549643&sdata=R4S5YPpNwBcUSy03I8g3Jqs6dk%2Ba6lZTFaL6iajwLFM%3D&reserved=0>.<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F59271&data=02%7C01%7CBret.Barkelew%40microsoft.com%7C10ad08aa2f8b41619cdc08d7fe98e2f3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637257804909549643&sdata=R4S5YPpNwBcUSy03I8g3Jqs6dk%2Ba6lZTFaL6iajwLFM%3D&reserved=0%3e.>)
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.<BR>
> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> +# Copyright (c) Microsoft Corporation.<BR>
> #
> # 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 {
> <LibraryClasses>
> 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.<BR>
> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> +# Copyright (c) Microsoft Corporation.<BR>
> #
> # 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 {
> <LibraryClasses>
> 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.<BR>
> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> +# Copyright (c) Microsoft Corporation.<BR>
> #
> # 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 {
> <LibraryClasses>
> 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.<BR>
> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> +# Copyright (c) Microsoft Corporation.<BR>
> # 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
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#60178): https://edk2.groups.io/g/devel/message/60178
Mute This Topic: https://groups.io/mt/74410208/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/edk2-devel-archive/attachments/20200522/97b31f0b/attachment.htm>
More information about the edk2-devel-archive
mailing list