From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM02-CY1-obe.outbound.protection.outlook.com (NAM02-CY1-obe.outbound.protection.outlook.com [40.107.76.117]) by mx.groups.io with SMTP id smtpd.web10.108.1590186931622794278 for ; Fri, 22 May 2020 15:35:31 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@microsoft.com header.s=selector2 header.b=iQvjWqEl; spf=pass (domain: microsoft.com, ip: 40.107.76.117, mailfrom: bret.barkelew@microsoft.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=AZv8RzO4JQV9U0kJE9p96a8sOSajSMQxkKKkDNSwZ7rezMeaaJ55aBMiUjqRHgo9kUTkjg5i64CII4cCQsyMbqQ5Eg+plLB9Vlz0PN6/vt5CzqbHE+l0bH5+1r31VJIZa+eQbyOcYrgDFkp4hNWuwpbH5hWlATe3ESICismKeIYKCTHSwBn8U+HxGvPpS70aFZ8cXzZjXKQF3tAkHF/YT7fq4qijOIZEtBHjLzMc8a8hOuYishglnb0ISZbLMVu1tjsb4LKUq23dNb3shmFkkl7D+r7/JWXrLZslzHyeynd/A9ZM0mu4HHOkT54b7pEuIWRFJr5w5nVwhhgDnS4VvA== 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=ncIUr+g/QytipxXNTGrVf85Hz1uQFUFExU8gspDzBXk=; b=J6jtgLdGu30nOfCU6cHLQuqCJLqkHb3IGKPatkIHdw4/iQabWeqKaIemTkSuGt8vLd+zAf9LVb/xUvsC4/p/Z6/GXV3QijwJNOUgOQcWQ0jZYn5aexJJRpVtgYJdsca5CLibz7RPCf3zdqq30xff0fu2mQw+yPUGxfkLtBihl0Rp/g0C4GMOM5GR5ioN7zo4AZX+iAAWXpesuN4hQOVj8lcyr2w2C0G2wZeXjRNzndOJI8He8mHbwkqPNWBVJNp8zSWZSrDxmIFw+xB8t8UrZSeUh4ly3s7ncC5WFsGStYAUiywwVzOFE1N5/HKMFEH7Hw/pVUae/f5k3tP40eE0YA== 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=ncIUr+g/QytipxXNTGrVf85Hz1uQFUFExU8gspDzBXk=; b=iQvjWqElwx7WQ1x04wmg+HS/zOZUdfhtQ9HdnFluBAjWsX9BGum/hBXTUHdfLsh8KmLBVONehHN5fuUlFn5FV2dh/pK/MLijn2adfDbP+jIvY/gp6UAIffjfxapRHQjKxnuMdritHYkuRm7nJMuLSZkqhxrTyh2qw/vE2qq8bng= Received: from CY4PR21MB0743.namprd21.prod.outlook.com (2603:10b6:903:b2::9) by CY4PR21MB1586.namprd21.prod.outlook.com (2603:10b6:910:90::10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3045.8; Fri, 22 May 2020 22:35:30 +0000 Received: from CY4PR21MB0743.namprd21.prod.outlook.com ([fe80::f112:82fb:d4fd:f7dd]) by CY4PR21MB0743.namprd21.prod.outlook.com ([fe80::f112:82fb:d4fd:f7dd%12]) with mapi id 15.20.3045.005; Fri, 22 May 2020 22:35:30 +0000 From: "Bret Barkelew" To: Laszlo Ersek , "devel@edk2.groups.io" , "michael.kubacki@outlook.com" CC: Jordan Justen , Ard Biesheuvel Subject: Re: [EXTERNAL] Re: [edk2-devel] [PATCH v3 05/14] OvmfPkg: Add VariablePolicy engine to OvmfPkg platform Thread-Topic: [EXTERNAL] Re: [edk2-devel] [PATCH v3 05/14] OvmfPkg: Add VariablePolicy engine to OvmfPkg platform Thread-Index: AQHWMIHBfYgTMIRUa0e1nV0eT2pimqi0sRE8 Date: Fri, 22 May 2020 22:35:29 +0000 Message-ID: References: <20200521224331.15616-1-michael.kubacki@outlook.com> ,<2e4a7b3c-a732-48bd-27b2-d3aef4b1690a@redhat.com> In-Reply-To: <2e4a7b3c-a732-48bd-27b2-d3aef4b1690a@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-05-22T22:33:39.5374490Z;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_ContentBits=0;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Method=Privileged authentication-results: redhat.com; dkim=none (message not signed) header.d=none;redhat.com; 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: 3eb02af6-1724-4a56-29fe-08d7fea06ea6 x-ms-traffictypediagnostic: CY4PR21MB1586: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-forefront-prvs: 04111BAC64 x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: 1Jp5rddYuVN2eQoiHiaG+XxO1GAhtU3pAfE616gUeA6VmltdAcGwtTGKBP5+AX5x6Pj7Bhc5oBWdygk3+Kk+yfp24XfXQ/njrHkzCcSLSbVgZ+8J6K6xbX/VmnNwrgVyBOZEDHrtmOOnobryzwRz+Zl+RMrzoOoKQSKjfxNQyXKKSPVy9ljsaIg+pJQ8iiQF7EikVLpDDkRoONVvOJ8gK0jF1fG+vrGF1ZBoL5GC1qNxt7JSxdZxi0rUnENNmFeyJL4MIBQg5E5SwOCygrTcp1f6BWS0u1C0y8TFGLP2utmJn39kx0pDLKaU0BhWlO6EekIgIbJDQEUDQLC2pTha1/nmmyc7NaE3AHOeQKMrE8FBBz3wPqdVo+0MDwfdCjb8wsTmcHqjtcjaAJ+PYKt0ng== 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)(39860400002)(396003)(366004)(346002)(376002)(110136005)(82950400001)(316002)(54906003)(9686003)(10290500003)(55016002)(33656002)(86362001)(8676002)(8936002)(52536014)(2906002)(71200400001)(82960400001)(26005)(19627235002)(166002)(966005)(478600001)(30864003)(7696005)(4326008)(5660300002)(66446008)(66946007)(66476007)(64756008)(66556008)(91956017)(76116006)(8990500004)(6506007)(186003)(53546011);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata: xsu7M2vatlBdNbZjmkiQH/224GD2uVrdO008QI+z0L9ygQR3RZibZdui8IV5u9j1JuJWoqWlBvT9hpz9jGnT+9AxlpZ28aNpMi31QUlRHRWGba+MRyxAZKWMGcMJVqD5qXOBzF+NOWiM1cEz26eGQIddlgslghSZ6MiO7lArZJ+4XHwgbUY7YXOlZogWQXlu0AyjusZWuBqPvGdrkHgBXsaUatrx3jzLtM4yG6BhU6LAsWxgnAU54jXGNd788J7KzlAY1pUW/h6MG4ixYUZgKPnqSl5t0QAfDPCN89Wjw5QNVRLs9dxfWSUO0xB8RgpfuI8X48HdU8/mlK5LyYbPNyyhHUG077HkVa+BBeSHeYjywimK2G0zPWC03dKwM9qwFElX1JRpFv3GMOV/X3jeH/rggtaDKFUxe92t9s660of7Y2dyvPUj1LIfUKWk52Z2FGd4iV2O6VUDgRu3vssAhVGEJPVtK3OxAEGWCwlgQy0= x-ms-exchange-transport-forked: True MIME-Version: 1.0 X-OriginatorOrg: microsoft.com X-MS-Exchange-CrossTenant-Network-Message-Id: 3eb02af6-1724-4a56-29fe-08d7fea06ea6 X-MS-Exchange-CrossTenant-originalarrivaltime: 22 May 2020 22:35:29.9990 (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: AW6zelm96C+XbP4zyGV8d+KIgH/zKGQbdUtl7ylrGazeRQA+ZePbNDzY4Am/SMkXEzK1lm8FTUcL1MUlztYVhw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR21MB1586 Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_CY4PR21MB074307E6FED6B5AA778DCBD8EFB40CY4PR21MB0743namp_" --_000_CY4PR21MB074307E6FED6B5AA778DCBD8EFB40CY4PR21MB0743namp_ Content-Type: text/plain; charset="Windows-1252" Content-Transfer-Encoding: quoted-printable =93Maybe 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.=94 Yup. That=92s the one. Saw this request, but not the patch feedback. Will a= ddress 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 somethin= g I would like to light up in future revisions, so I would need to hear the= argument against it. - Bret From: Laszlo Ersek Sent: Friday, May 22, 2020 2:41 PM To: devel@edk2.groups.io; michael.kubacki@outl= ook.com Cc: Jordan Justen; Ard Biesheuvel; Bret Barkelew Subject: [EXTERNAL] Re: [edk2-devel] [PATCH v3 05/14] OvmfPkg: Add Variable= Policy 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 > > https://nam06.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fbugzi= lla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2522&data=3D02%7C01%7CBret.Bark= elew%40microsoft.com%7C10ad08aa2f8b41619cdc08d7fe98e2f3%7C72f988bf86f141af9= 1ab2d7cd011db47%7C1%7C0%7C637257804909549643&sdata=3Df2vsskZHgV2wgxtcFq= xigKPVpSyz6hBE1Y74qEsAWTs%3D&reserved=3D0 > > 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: https://nam06.safelinks.protection.outlook.com/?url=3Dhttp%3A%2F%2Fmid.mail= -archive.com%2Fa0e0e3d4-6712-078a-4d95-29408109b0b0%40redhat.com&data= =3D02%7C01%7CBret.Barkelew%40microsoft.com%7C10ad08aa2f8b41619cdc08d7fe98e2= f3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637257804909549643&sdat= a=3D8ajjcCIXFpvLAylSu9SBamajUhApYVtsJ6hUZQsG%2BXQ%3D&reserved=3D0 (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/AuthVariableL= ibNull.inf > !endif > VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf > + VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolic= yLib.inf > + VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/V= ariablePolicyHelperLib.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.i= nf > + VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolic= yLibRuntimeDxe.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.PcdAllowVariablePolicyEnforcementDisabl= e|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/AuthVariableL= ibNull.inf > !endif > VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf > + VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolic= yLib.inf > + VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/V= ariablePolicyHelperLib.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.i= nf > + VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolic= yLibRuntimeDxe.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.PcdAllowVariablePolicyEnforcementDisabl= e|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/AuthVariableL= ibNull.inf > !endif > VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf > + VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolic= yLib.inf > + VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/V= ariablePolicyHelperLib.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.i= nf > + VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolic= yLibRuntimeDxe.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.PcdAllowVariablePolicyEnforcementDisabl= e|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/AuthVariableL= ibNull.inf > VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf > + VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolic= yLib.inf > + VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/V= ariablePolicyHelperLib.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.i= nf > + VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolic= yLibRuntimeDxe.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.PcdAllowVariablePolicyEnforcementDisabl= e|TRUE > + > gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0 > > gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 > --_000_CY4PR21MB074307E6FED6B5AA778DCBD8EFB40CY4PR21MB0743namp_ Content-Type: text/html; charset="Windows-1252" Content-Transfer-Encoding: quoted-printable

=93Maybe you entirely missed my message that I poste= d 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.=94

 

Yup. That=92s 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 somethin= g I would like to light up in future revisions, so I would need to hear the= argument against it.

 

- Bret

 

From: Laszlo Ersek
Sent: Friday, May 22, 2020 2:41 PM
To: devel@edk2.groups.io= ; michael.kubacki@outlook.com
Cc: Jordan Justen; = Ard Biesheuvel; Bret Bar= kelew
Subject: [EXTERNAL] Re: [edk2-devel] [PATCH v3 05/14] OvmfPkg: Add V= ariablePolicy 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@microsoft.com>
>
> https://nam06.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fbugzill= a.tianocore.org%2Fshow_bug.cgi%3Fid%3D2522&amp;data=3D02%7C01%7CBret.Ba= rkelew%40microsoft.com%7C10ad08aa2f8b41619cdc08d7fe98e2f3%7C72f988bf86f141a= f91ab2d7cd011db47%7C1%7C0%7C637257804909549643&amp;sdata=3Df2vsskZHgV2w= gxtcFqxigKPVpSyz6hBE1Y74qEsAWTs%3D&amp;reserved=3D0
>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Bret Barkelew <brbarkel@microsoft.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com> > ---
>  OvmfPkg/OvmfPkgIa32.dsc    | 8 +++= 3;++++
>  OvmfPkg/OvmfPkgIa32X64.dsc | 8 ++++++= 3;+
>  OvmfPkg/OvmfPkgX64.dsc     | 8 ++= 3;+++++
>  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=3Dhttp= %3A%2F%2Fmid.mail-archive.com%2Fa0e0e3d4-6712-078a-4d95-29408109b0b0%40redh= at.com&amp;data=3D02%7C01%7CBret.Barkelew%40microsoft.com%7C10ad08aa2f8= b41619cdc08d7fe98e2f3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63725780= 4909549643&amp;sdata=3D8ajjcCIXFpvLAylSu9SBamajUhApYVtsJ6hUZQsG%2BXQ%3D= &amp;reserved=3D0

(Alternative link: <https://nam06.safelinks.protection.outlook.com/?url=3Dhttp= s%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F59271&amp;data=3D02%7C= 01%7CBret.Barkelew%40microsoft.com%7C10ad08aa2f8b41619cdc08d7fe98e2f3%7C72f= 988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637257804909549643&amp;sdata=3D= R4S5YPpNwBcUSy03I8g3Jqs6dk%2Ba6lZTFaL6iajwLFM%3D&amp;reserved=3D0>.<= /a>)

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 Developmen= t LP<BR>
> +#  Copyright (c) Microsoft Corporation.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -194,6 +195,8 @@ [LibraryClasses]
>    AuthVariableLib|MdeModulePkg/Library/AuthVariableLib= Null/AuthVariableLibNull.inf
>  !endif
>    VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarChec= kLib.inf
> +  VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/V= ariablePolicyLib.inf
> +  VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolic= yHelperLib/VariablePolicyHelperLib.inf


>    #
> @@ -327,6 +330,7 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>    BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeC= ryptLib.inf
>    PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI= 440FxQ35.inf
>    QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQem= uFwCfgS3LibFwCfg.inf
> +  VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/V= ariablePolicyLibRuntimeDxe.inf

>  [LibraryClasses.common.UEFI_DRIVER]
>    PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> @@ -480,6 +484,9 @@ [PcdsFixedAtBuild]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariabl= eSize|0x40000
>  !endif

> +  # Optional: Omit if VariablePolicy should be always-on. > +  gEfiMdeModulePkgTokenSpaceGuid.PcdAllowVariablePolicyEnfor= cementDisable|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 rea= d my
feedback to the blurb.

In v4, please remove the "PcdAllowVariablePolicyEnforcementDisable&quo= t;
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.PcdReportStatusCodeProperty= Mask|0x07
> @@ -921,6 +928,7 @@ [Components]
>    MdeModulePkg/Universal/Variable/RuntimeDxe/VariableS= mm.inf {
>      <LibraryClasses>
>        NULL|MdeModulePkg/Library/Va= rCheckUefiLib/VarCheckUefiLib.inf
> +      NULL|MdeModulePkg/Library/VarCheck= PolicyLib/VarCheckPolicyLib.inf
>    }
>    MdeModulePkg/Universal/Variable/RuntimeDxe/VariableS= mmRuntimeDxe.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 Developmen= t LP<BR>
> +#  Copyright (c) Microsoft Corporation.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -198,6 +199,8 @@ [LibraryClasses]
>    AuthVariableLib|MdeModulePkg/Library/AuthVariableLib= Null/AuthVariableLibNull.inf
>  !endif
>    VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarChec= kLib.inf
> +  VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/V= ariablePolicyLib.inf
> +  VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolic= yHelperLib/VariablePolicyHelperLib.inf


>    #
> @@ -331,6 +334,7 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>    BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeC= ryptLib.inf
>    PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI= 440FxQ35.inf
>    QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQem= uFwCfgS3LibFwCfg.inf
> +  VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/V= ariablePolicyLibRuntimeDxe.inf

>  [LibraryClasses.common.UEFI_DRIVER]
>    PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> @@ -484,6 +488,9 @@ [PcdsFixedAtBuild]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariabl= eSize|0x40000
>  !endif

> +  # Optional: Omit if VariablePolicy should be always-on. > +  gEfiMdeModulePkgTokenSpaceGuid.PcdAllowVariablePolicyEnfor= cementDisable|TRUE
> +
>    gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0=

>    gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodeProperty= Mask|0x07
> @@ -934,6 +941,7 @@ [Components.X64]
>    MdeModulePkg/Universal/Variable/RuntimeDxe/VariableS= mm.inf {
>      <LibraryClasses>
>        NULL|MdeModulePkg/Library/Va= rCheckUefiLib/VarCheckUefiLib.inf
> +      NULL|MdeModulePkg/Library/VarCheck= PolicyLib/VarCheckPolicyLib.inf
>    }
>    MdeModulePkg/Universal/Variable/RuntimeDxe/VariableS= mmRuntimeDxe.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 Developmen= t LP<BR>
> +#  Copyright (c) Microsoft Corporation.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -198,6 +199,8 @@ [LibraryClasses]
>    AuthVariableLib|MdeModulePkg/Library/AuthVariableLib= Null/AuthVariableLibNull.inf
>  !endif
>    VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarChec= kLib.inf
> +  VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/V= ariablePolicyLib.inf
> +  VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolic= yHelperLib/VariablePolicyHelperLib.inf


>    #
> @@ -331,6 +334,7 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>    BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeC= ryptLib.inf
>    PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI= 440FxQ35.inf
>    QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQem= uFwCfgS3LibFwCfg.inf
> +  VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/V= ariablePolicyLibRuntimeDxe.inf

>  [LibraryClasses.common.UEFI_DRIVER]
>    PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> @@ -484,6 +488,9 @@ [PcdsFixedAtBuild]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariabl= eSize|0x40000
>  !endif

> +  # Optional: Omit if VariablePolicy should be always-on. > +  gEfiMdeModulePkgTokenSpaceGuid.PcdAllowVariablePolicyEnfor= cementDisable|TRUE
> +
>    gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0=

>    gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodeProperty= Mask|0x07
> @@ -932,6 +939,7 @@ [Components]
>    MdeModulePkg/Universal/Variable/RuntimeDxe/VariableS= mm.inf {
>      <LibraryClasses>
>        NULL|MdeModulePkg/Library/Va= rCheckUefiLib/VarCheckUefiLib.inf
> +      NULL|MdeModulePkg/Library/VarCheck= PolicyLib/VarCheckPolicyLib.inf
>    }
>    MdeModulePkg/Universal/Variable/RuntimeDxe/VariableS= mmRuntimeDxe.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 Developmen= t 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/AuthVariableLib= Null/AuthVariableLibNull.inf
>    VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarChec= kLib.inf
> +  VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/V= ariablePolicyLib.inf
> +  VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolic= yHelperLib/VariablePolicyHelperLib.inf


>    #
> @@ -301,6 +304,7 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>    BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeC= ryptLib.inf
>    PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI= 440FxQ35.inf
>    QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQem= uFwCfgS3LibFwCfg.inf
> +  VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/V= ariablePolicyLibRuntimeDxe.inf

>  [LibraryClasses.common.UEFI_DRIVER]
>    PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> @@ -394,6 +398,9 @@ [PcdsFixedAtBuild]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariabl= eSize|0x40000
>  !endif

> +  # Optional: Omit if VariablePolicy should be always-on. > +  gEfiMdeModulePkgTokenSpaceGuid.PcdAllowVariablePolicyEnfor= cementDisable|TRUE
> +
>    gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0=

>    gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodeProperty= Mask|0x07
>

 

--_000_CY4PR21MB074307E6FED6B5AA778DCBD8EFB40CY4PR21MB0743namp_--