Re: [RFC PATCH v2 04/10] OvmfPkg/BaseMemcryptSevLib: Add SEV helper library

[Laszlo Ersek <lersek@redhat.com>]

On 03/27/17 20:44, Brijesh Singh wrote:
> On Mon, Mar 27, 2017 at 5:07 AM, Laszlo Ersek <lersek@redhat.com> wrote:
> 
>> On 03/27/17 11:19, Laszlo Ersek wrote:
>>> On 03/21/17 22:13, Brijesh Singh wrote:
>>
>>>> +  Returns a boolean to indicate whether SEV is enabled
>>>> +
>>>> +  @retval TRUE           When SEV is active
>>>> +  @retval FALSE          When SEV is not enabled
>>>> +  **/
>>>> +BOOLEAN
>>>> +EFIAPI
>>>> +MemEncryptSevIsEnabled (
>>>> +  VOID
>>>> +  );
>>>
>>> Would it make sense to call this library function in PlatformPei, rather
>>> than add a separate SevIsEnabled() function to it (in patch #3)? The
>>> implementations look nearly identical.
>>
>> I realize that earlier I seemingly suggested the opposite:
>>
>> http://mid.mail-archive.com/dd9436dc-415c-9fab-081c-
>> 39dd2cd71fd5@redhat.com
>>
>> http://mid.mail-archive.com/9193d837-6a78-b1c4-42c0-
>> 427fbc1f2364@redhat.com
>>
>> However, at that time, my understanding was that this library would only
>> be used in PlatformPei (hence the single user wouldn't justify the new
>> library instance). Now it seems that there are going to be several
>> client modules that check on SEV enablement. Is that right?
>>
>>
> 
> Yes, I do expect several client module link against this library to check
> whether the SEV is enabled.
> Are you okay if we link MemEncryptSevLib in PlatformPei and make use of
> MemEncryptSevIsEnabled()
> routine instead of having a local copy ? I was not sure which way to go
> hence I still have PlatformPei
> and QemuFwCfgPei using the local implementation of the same functions. My
> personal perference would
> be to link with MemEncryptSevLib instead of having local function. But as
> always I am open to suggestions.

I think the library function should be used (caching the CPUID detection
results) whenever we have writeable memory (PEI and onwards).

Thanks
Laszlo