From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.groups.io with SMTP id smtpd.web11.545.1571079031675697204 for ; Mon, 14 Oct 2019 11:50:31 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 31E531DCC; Mon, 14 Oct 2019 18:50:31 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-117-254.ams2.redhat.com [10.36.117.254]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6E3125D9C9; Mon, 14 Oct 2019 18:50:17 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH V3 9/9] OvmfPkg: Disable variable runtime cache To: devel@edk2.groups.io, michael.a.kubacki@intel.com Cc: Ard Biesheuvel , Liming Gao , Jordan Justen , Hao A Wu References: <20191014180846.31660-1-michael.a.kubacki@intel.com> <20191014180846.31660-10-michael.a.kubacki@intel.com> From: "Laszlo Ersek" Message-ID: Date: Mon, 14 Oct 2019 20:50:13 +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: <20191014180846.31660-10-michael.a.kubacki@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.6.2 (mx1.redhat.com [10.5.110.71]); Mon, 14 Oct 2019 18:50:31 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 10/14/19 20:08, Kubacki, Michael A wrote: > Sets gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache > to FALSE in OvmfPkgIa32.dsc, OvmfPkgIa32X64.dsc, and OvmfPkgX64.dsc > so that when SMM_REQUIRE is TRUE, the SMM variable driver will not > use the runtime variable cache. > > This is done for OvmfPkg because it currently depends upon a SMM > variable GetVariable ()implementation as a simple method to exercise > the SMM driver stack. This allows the following commands to be used > for variables such as Boot####, BootOrder, and BootNext to test SMM > timing and stability differences on the BSP (e.g. CPU#0) vs an > AP (e.g. CPU#1). > # taskset -c 0 efibootmgr > # taskset -c 1 efibootmgr > > Cc: Ard Biesheuvel > Cc: Laszlo Ersek > Cc: Liming Gao > Cc: Jordan Justen > Cc: Hao A Wu > Signed-off-by: Michael Kubacki > --- > OvmfPkg/OvmfPkgIa32.dsc | 1 + > OvmfPkg/OvmfPkgIa32X64.dsc | 1 + > OvmfPkg/OvmfPkgX64.dsc | 1 + > 3 files changed, 3 insertions(+) > > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc > index 66e944436a..4861e2342b 100644 > --- a/OvmfPkg/OvmfPkgIa32.dsc > +++ b/OvmfPkg/OvmfPkgIa32.dsc > @@ -428,6 +428,7 @@ > !if $(SMM_REQUIRE) == TRUE > gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE > + gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache|FALSE > !endif > > [PcdsFixedAtBuild] > diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc > index 51c2bfb44f..f316d97d6a 100644 > --- a/OvmfPkg/OvmfPkgIa32X64.dsc > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc > @@ -433,6 +433,7 @@ > !if $(SMM_REQUIRE) == TRUE > gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE > + gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache|FALSE > !endif > > [PcdsFixedAtBuild] > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc > index ba7a758844..546649f0a2 100644 > --- a/OvmfPkg/OvmfPkgX64.dsc > +++ b/OvmfPkg/OvmfPkgX64.dsc > @@ -433,6 +433,7 @@ > !if $(SMM_REQUIRE) == TRUE > gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE > + gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache|FALSE > !endif > > [PcdsFixedAtBuild] > For this patch: Reviewed-by: Laszlo Ersek And many thanks for accommodating this use case! However, I'd prefer the following order of changes: - patch 7: introduce the PCD with default FALSE value - patch 9: set the PCD explicitly to FALSE in OvmfPkg - patch 10 (new patch): flip the PCD default value to TRUE Because with v3, if someone were to bisect an issue, using OVMF, and the bisection landed them at patch 7 or at patch 8, they'd see a (transitory) change in variable caching behavior. Thanks, Laszlo