On 09/19/19 21:52, Lendacky, Thomas wrote: > From: Tom Lendacky <thomas.lenda...@amd.com> > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198 > > The SEC phase of OVMF will need access to the MemEncryptSevLib library, > so make the library available during SEC. > > Cc: Jordan Justen <jordan.l.jus...@intel.com> > Cc: Laszlo Ersek <ler...@redhat.com> > Cc: Ard Biesheuvel <ard.biesheu...@linaro.org> > Signed-off-by: Tom Lendacky <thomas.lenda...@amd.com> > --- > OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf > b/OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf > index 7c44d0952815..755d49cc22dc 100644 > --- a/OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf > @@ -14,7 +14,7 @@ [Defines] > FILE_GUID = c1594631-3888-4be4-949f-9c630dbc842b > MODULE_TYPE = BASE > VERSION_STRING = 1.0 > - LIBRARY_CLASS = MemEncryptSevLib|PEIM DXE_DRIVER > DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_DRIVER > + LIBRARY_CLASS = MemEncryptSevLib|SEC PEIM DXE_DRIVER > DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_DRIVER > > # > # The following information is for reference only and not required by the > build >
This is not a good idea, at least in this form. This library instance uses multiple variables with static storage duration, such as: - mSevStatus - mSevEsStatus - mSevStatusChecked - mAddressEncMaskChecked [X64] - mAddressEncMask [X64] - mPageTablePool [X64] SEC runs from pflash, so such variables are read-only (writes to them won't trap, but will have no effect). What are the functions from "OvmfPkg/Include/Library/MemEncryptSevLib.h" that you need in the SEC phase? Because, maybe we should split the library class in two classes. One lib class would declare (for example): - MemEncryptSevEsIsEnabled - MemEncryptSevIsEnabled The other lib class would declare: - MemEncryptSevClearPageEncMask - MemEncryptSevSetPageEncMask - MemEncryptSevLocateInitialSmramSaveStateMapPages The first lib class could have two instances, one for SEC (using no global variables for caching / speeding up the checks), and another instance for PEI and later (with the global variables used for caching the detection results). The second lib class would have only one instance (the current one), and it would not be usable in the SEC phase. (The main issue is that, for manipulating PTEs, MemEncryptSevSetPageEncMask and MemEncryptSevClearPageEncMask sometimes need to split page tables, and for that, they need to allocate pages dynamically. We can't do that in SEC.) The [LibraryClasses] sections of some INF files that currently list "MemEncryptSevLib" may have to be updated, corresponding to the split. Again, all of this depends on the exact subset of APIs you need in SEC. If it's just one API, e.g. MemEncryptSevEsIsEnabled(), needed in just one place in SEC, then it might not necessarily be tragic to simply open-code (duplicate) the CPUID logic in SEC. Thanks, Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#48369): https://edk2.groups.io/g/devel/message/48369 Mute This Topic: https://groups.io/mt/34203552/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
