Re: [edk2-devel] [PATCH v2 2/2] UefiCpuPkg/PiSmmCpu: PcdCpuSmmAccessOut controls SMM access-out policy

["Laszlo Ersek" <lersek@redhat.com>]

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