On 11/06/18 03:59, Ruiyu Ni wrote:
> From: Jiewen Yao <[email protected]>
> 
> Today's implementation blocks SMM read-out no matter static paging
> is enabled or not. But certain platform may need to read non-SMM
> content from SMM code. These platforms don't have a way to disable
> the read-out blocking.
> 
> The patch updates the policy to only block SMM read-out when static
> paging is enabled. So that the static paging can be disabled for
> those platforms that want SMM read-out.
> 
> Setting PcdCpuSmmStaticPageTable to FALSE can disable the static
> paging.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jiewen Yao <[email protected]>
> Signed-off-by: Ruiyu Ni <[email protected]>
> Cc: Eric Dong <[email protected]>
> Cc: Jiewen Yao <[email protected]>
> Cc: Laszlo Ersek <[email protected]>
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c 
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> index 5bb7d57238..117502dafa 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> @@ -1,7 +1,7 @@
>  /** @file
>  Page Fault (#PF) handler for X64 processors
>  
> -Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
>  Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
>  
>  This program and the accompanying materials
> @@ -890,7 +890,7 @@ SmiPFHandler (
>        CpuDeadLoop ();
>      }
>  
> -    if (IsSmmCommBufferForbiddenAddress (PFAddress)) {
> +    if (mCpuSmmStaticPageTable && IsSmmCommBufferForbiddenAddress 
> (PFAddress)) {
>        DumpCpuContext (InterruptType, SystemContext);
>        DEBUG ((DEBUG_ERROR, "Access SMM communication forbidden address 
> (0x%lx)!\n", PFAddress));
>        DEBUG_CODE (
> 

OVMF inherits the default TRUE value for PcdCpuSmmStaticPageTable, from
"UefiCpuPkg.dec", and that's intentional. Therefore this patch should be
a no-op from OVMF's perspective.

Acked-by: Laszlo Ersek <[email protected]>

More generally, is the use of PcdCpuSmmStaticPageTable for controlling
this kind of read-out just a convenience / simplification (in which case
I don't think it's great!), or are these topics inherently connected
somehow?

I remember that Jiewen said earlier that with "static paging" enabled
(i.e., building the page tables used in SMM all in advance), we provide
more page protection.

Also, I see that PcdCpuSmmProfileEnable can only be enabled with
PcdCpuSmmStaticPageTable set to FALSE.

So it seems that with PcdCpuSmmStaticPageTable set to TRUE, our page
fault handling in SMM is generally strict(er). This patch looks
consistent with that, but it would be nice if the commit message spelled
out why *exactly* it makes sense to use PcdCpuSmmStaticPageTable for
this new purpose as well.

(I hope my question makes sense. :) )

Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to