From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Thu, 01 Aug 2019 18:42:01 -0700 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5E39F50143; Fri, 2 Aug 2019 01:42:01 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-67.ams2.redhat.com [10.36.116.67]) by smtp.corp.redhat.com (Postfix) with ESMTP id D77B060BE2; Fri, 2 Aug 2019 01:41:59 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v2 2/2] UefiCpuPkg/PiSmmCpu: PcdCpuSmmAccessOut controls SMM access-out policy To: "Ni, Ray" , "Yao, Jiewen" , "devel@edk2.groups.io" Cc: "Dong, Eric" , "Wang, Jian J" References: <20190731163852.191708-1-ray.ni@intel.com> <20190731163852.191708-3-ray.ni@intel.com> <50b073a1-bf04-4b8a-e267-2af933d34c5c@redhat.com> <734D49CCEBEEF84792F5B80ED585239D5C266BC4@SHSMSX104.ccr.corp.intel.com> <74D8A39837DF1E4DA445A8C0B3885C503F738DCC@shsmsx102.ccr.corp.intel.com> <734D49CCEBEEF84792F5B80ED585239D5C266CC5@SHSMSX104.ccr.corp.intel.com> <74D8A39837DF1E4DA445A8C0B3885C503F739126@shsmsx102.ccr.corp.intel.com> <734D49CCEBEEF84792F5B80ED585239D5C26720C@SHSMSX104.ccr.corp.intel.com> From: "Laszlo Ersek" Message-ID: <452af2e4-8800-23dc-91fc-dfc092bdf496@redhat.com> Date: Fri, 2 Aug 2019 03:41:58 +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: <734D49CCEBEEF84792F5B80ED585239D5C26720C@SHSMSX104.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Fri, 02 Aug 2019 01:42:01 +0000 (UTC) Content-Type: text/plain; charset=UTF-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 08/01/19 08:25, Ni, Ray wrote: > Jiewen, Laszlo, > Firstly let's assume we are aligned to use single/one PCD to control > both static page table creation and SMM access out. > > With this assumption, we need more discussion about what proper PCD > name to choose. > > For now there are two candidates: PcdCpuSmmStaticPageTable and > PcdCpuSmmAccessOut. > Laszlo doesn't like PcdCpuSmmStaticPageTable (Jiewen proposed. I > don't like this name either). > Jiewen doesn't like PcdCpuSmmAccessOut (I proposed. Not sure if > Laszlo likes it or not). > > When using PcdCpuSmmStaticPageTable, the straightforward meaning is > whether the page table to cover all memory that needs access/forbidden > is created in advance. When page table is created in advance, it's a > bit hard to deduce that access out is forbidden. When page table is > dynamically created in PF handler, it's also a bit hard to deduce that > access out is allowed. > > When using PcdCpuSmmAccessOut, the straightforward meaning is whether > to allow SMM code to access non-SMRAM memory. Exception is ACPI NVS/ > RSVD, Runtime and MMIO can always be accessible to SMM code. When > PcdCpuSmmAccessOut is TRUE, whether to use static page table is > PiSmmCpu's implementation choice, but since dynamic page table > creation in PF handler saves the SMRAM, the PiSmmCpu can choose to use > the optimal solution which means to create page table in PF handler > (Identical to Jiewen's case #2). When PcdCpuSmmAccessOut is FALSE, > it's more secure to have a static page table (Identical to Jiewen's > case #3). What I want to say here is SmmAccessOut is the policy that > PiSmmCpu driver wants to get from platform side, whether to use static > page table is its own implementation choice to meet platform > SmmAccessOut requirement. > > So I think PcdCpuSmmAccessOut is better than PcdCpuSmmStaticPageTable. > > I agree with Jiewen's concern this name is not very precise to > describe the behavior. > > Laszlo, Jiewen, > Do you have some proposals? This is not how I remember Jiewen's original explanation of PcdCpuSmmStaticPageTable. Whenever I've been in doubt about PcdCpuSmmStaticPageTable in the last few years, the following has been the answer that I've gone back to, repeatedly: http://mid.mail-archive.com/74D8A39837DF1E4DA445A8C0B3885C50386BD98A@shsmsx102.ccr.corp.intel.com Alternative link: https://lists.01.org/pipermail/edk2-devel/2016-November/004106.html Key part being > If we use dynamic paging, we can still provide *partial* protection. > And hope page table is not modified by other component. For me that decided the question; I'd *always* want static paging enabled. It's not an implementation choice for PiSmmCpuDxeSmm, but something I'd like to dictate from the platform side. I also strongly dislike the idea of dynamically allocating SMRAM at OS runtime. (For whatever purpose -- for example, for page tables.) If you run out of SMRAM before SMM-ready-to-lock, that's one thing; you don't lose OS data. If you run out of SMRAM at OS runtime, you lose OS data in the crash. I don't want PiSmmCpuDxeSmm to save SMRAM on page tables; instead I want the SMRAM footprint to be predictable and allocated as early as possible during boot. --*-- In one of my earlier emails, I did consider the case when a single boolean PCD sufficed. I suggested that we pick that approach only if the two aspects (access-out and static page table building) are inherently related: basically, equivalent. By equivalence I don't mean that our "table of variations" decays from 2*2=4 to just 2 elements, after we figure out what we want to support -- because "what we like to support" is not "inherent relationship". In my opinion, the non-equivalence of both features is shown by our struggle to come up with *one* name for the PCD such that it describe and control both features. The non-equivalence is also shown by IA32 supporting access-out but not supporting dynamic tables, but X64 supporting both. I also considered the case (earlier) that not all 2*2=4 combinations might be sensible or desirable. For that, I suggested that we add ASSERT()s to the entry point of PiSmmCpuDxeSmm, to catch invalid combinations of both PCDs early. That slims down the testing matrix, and it still keeps the concepts separate. Ultimately, what I care about is OVMF: - keep static paging enabled, - disable access-out, - build for both IA32 and X64, - behave identically (wrt. static paging, and access-out), on IA32 and X64. Thanks Laszlo