Re: [Qemu-devel] [PATCH 1/2] q35: implement SMRAM.D_LCK

[Laszlo Ersek]

On 04/16/15 10:12, Paolo Bonzini wrote:
>
>
> On 15/04/2015 15:58, Gerd Hoffmann wrote:
>> The "A Tour Beyond BIOS Implementing S3 Resume with EDKII" white
>> paper lists "Lock SMM. This must be done to maintain SMM integrity."
>> as todo list item for the edk2 resume code path (page 18).

let's mark this with (*) for the end of my email

>> So it seems to me it is the job of the firmware to re-lock smm after
>> S3 (and before handing control back to the OS, obviously).
>
> Right, however "D_LCK can be set to 1 via a normal configuration space
> write but can only be cleared by a Full Reset."  A Full Reset is the
> PLTRST# pin, which is asserted by the south bridge during power-up and
> during CF9h reset.  S3 doesn't seem to be included.
>
> My reading of the EDK2 whitepaper is that this may vary for other
> chipsets.

Page 25, "PEI Instance", option "2)" applies: the S3 resume PEI module
needs to be able to open SMRAM.

>From the OVMF whitepaper
<http://people.redhat.com/~lersek/ovmf-whitepaper-c770f8c.txt>:

>     (b) In PEI, the S3 Resume PEIM (UefiCpuPkg/Universal/Acpi/S3Resume2Pei)
>         retrieves data from the LockBox.
>
>         Presumably, S3Resume2Pei should be considered an "unprivileged PEIM",
>         and the SMRAM access should be layered as seen in DXE. Unfortunately,
>         edk2 does not implement all of the layers in PEI -- the code either
>         doesn't exist, or it is not open source:
>
>   role         | DXE: protocol/module           | PEI: PPI/module
>   
> -------------+--------------------------------+------------------------------
>   unprivileged | any                            | S3Resume2Pei.inf
>   driver       |                                |
>   
> -------------+--------------------------------+------------------------------
>   command      | LIBRARY_CLASS = LockBoxLib     | LIBRARY_CLASS = LockBoxLib
>   formatting   |                                |
>   and response | SmmLockBoxDxeLib.inf           | SmmLockBoxPeiLib.inf
>   parsing      |                                |
>   
> -------------+--------------------------------+------------------------------
>   privilege    | EFI_SMM_COMMUNICATION_PROTOCOL | 
> EFI_PEI_SMM_COMMUNICATION_PPI
>   separation   |                                |
>                | PiSmmCore.inf                  | missing!
>   
> -------------+--------------------------------+------------------------------
>   platform SMM | EFI_SMM_CONTROL2_PROTOCOL      | PEI_SMM_CONTROL_PPI
>   and SMRAM    | EFI_SMM_ACCESS2_PROTOCOL       | PEI_SMM_ACCESS_PPI
>   access       |                                |
>                | to be done in OVMF             | to be done in OVMF
>   
> -------------+--------------------------------+------------------------------
>   command      | LIBRARY_CLASS = LockBoxLib     | LIBRARY_CLASS = LockBoxLib
>   parsing and  |                                |
>   response     | SmmLockBoxSmmLib.inf           | missing!
>   formatting   |                                |
>   
> -------------+--------------------------------+------------------------------
>   privileged   | SmmLockBox.inf                 | missing!
>   LockBox      |                                |
>   driver       |                                |
>
>         Alternatively, in the future OVMF might be able to provide a 
> LockBoxLib
>         instance (an SmmLockBoxPeiLib substitute) for S3Resume2Pei that
>         accesses SMRAM directly, eliminating the need for deeper layers in the
>         stack (that is, EFI_PEI_SMM_COMMUNICATION_PPI and deeper).
>
>         In fact, a "thin" EFI_PEI_SMM_COMMUNICATION_PPI implementation whose
>         sole Communicate() member invariably returns EFI_NOT_STARTED would
>         cause the current SmmLockBoxPeiLib library instance to directly 
> perform
>         full-depth SMRAM access and LockBox search, obviating the "missing"
>         cells. (With reference to A Tour Beyond BIOS: Implementing S3 Resume
>         with EDK2, by Jiewen Yao and Vincent Zimmer, October 2014.)

The code flow in edk2 corresponding to the last paragraph above is:

S3RestoreConfig2()                     
[UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c]
  RestoreLockBox()                     
[MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c]
    SmmCommunicationPpi->Communicate() [1]
    InternalRestoreLockBoxFromSmram()  
[MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c]
      SmmAccess->Open()                [2]

[1] EFI_PEI_SMM_COMMUNICATION_PPI, to be implemented in OVMF; returns
    EFI_NOT_STARTED (see the reasoning above, in both whitepapers)
[2] PEI_SMM_ACCESS_PPI, to be implemented in OVMF

The PEI_SMM_ACCESS_PPI.Open() member, called on the last line above,
runs on S3 resume, and it needs to be able to open up SMRAM. If the lock
is still in place, that won't work.


Additionally, the following language from page 18 in the Intel
whitepaper, marked above as (*):

  o Lock SMM. This must be done to maintain SMM integrity.

is not a TODO list item; it is a responsibility that the S3Resume2Pei
module already covers. See

  S3ResumeExecuteBootScript() 
[UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c]
    SmmAccess->Close()        [2]
    SmmAccess->Lock()         [2]


So, the hardware should clear the lock on S3 resume, then edk2 will open
and close the SMRAM regions as appropriate, and it will also re-lock
SMRAM, before
(a) it executes the S3 boot script (which might contain opcodes from
    UEFI (ie. third party) drivers),
(b) it transfers control to the OS's wakeup vector.

(In our case the S3 boot script has only one entry (an informational
no-op) anyway; the main assurance is that the OS's wakeup vector is
reached with the SMRAM re-locked.)

Thanks!
Laszlo