Re: [RFC v4 04/13] OvmfPkg/BaseMemcryptSevLib: Add SEV helper library

[Brijesh Singh <brijesh.singh@amd.com>]

On 05/11/2017 09:04 AM, Laszlo Ersek wrote:
>
> I have the following suggestions for this patch:
>
> (1) The InternalMemEncryptSevIsEnabled() function is identical between
> "Ia32/MemEncryptSevLib.c" and "X64/MemEncryptSevLib.c". In addition,
> that function is the only one exported by "MemEncryptSevLibInternal.h".
>
> I suggest to eliminate "MemEncryptSevLibInternal.h", and to move the
> common implementation of InternalMemEncryptSevIsEnabled() into
> "MemEncryptSevLibInternal.c". The moved function should be made STATIC.
>
> This decreases code duplication and removes an internal header file.
>
> (2) Please prefix the names of the extern functions SetMemoryDecrypted()
> and SetMemoryEncrypted() with "InternalMemEncryptSev".
>
> (3) Since this is a BASE library, please don't use EFI_STATUS,
> EFI_INVALID_PARAMETER, EFI_NO_MAPPING, EFI_SUCCESS; use RETURN_xxx instead.
>
> (4) Since this library is going to be linked into multiple modules
> (presumably), please consider modifying all the debug messages as
> follows: the format strings should start with "%a: %a: ", and the
> arguments to pass in should be gEfiCallerBaseName and __FUNCTION__.
>
> The former arg will make the library print the BASE_NAME of the
> containing driver module (from its INF). And __FUNCTION__ helps jumping
> to the location more quickly.

I was not aware of built-in gEfiCallerBaseName, it will be really handy.
I agree with all your comments and will update the patch to remove the
code duplication.

-Brijesh