Re: [edk2-devel] [PATCH 3/4] OvmfPkg: Use toolchain appropriate CpuExceptionHandlerLib

["Laszlo Ersek" <lersek@redhat.com>]

On 05/01/20 22:17, Lendacky, Thomas wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2340
> 
> Use the XCODE5 CpuExceptionHandlerLib library in place of the standard
> library when building with the XCODE5 toolchain. The XCODE5 version of
> the library performs binary patching and should only be used when building
> with the XCODE5 toolchain.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Julien Grall <julien@xen.org>
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  OvmfPkg/OvmfPkgIa32.dsc    | 20 ++++++++++++++++++++
>  OvmfPkg/OvmfPkgIa32X64.dsc | 20 ++++++++++++++++++++
>  OvmfPkg/OvmfPkgX64.dsc     | 20 ++++++++++++++++++++
>  OvmfPkg/OvmfXen.dsc        | 16 ++++++++++++++++
>  4 files changed, 76 insertions(+)
> 
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index fcd9779b5ba2..f27fcd7e1087 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -245,7 +245,11 @@ [LibraryClasses.common.SEC]
>    PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf
>    PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf
>    MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf
> +!if $(TOOL_CHAIN_TAG) != "XCODE5"
>    CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
> +!else
> +  CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf
> +!endif

(1) In accordance with my point (1) under patch#1, please keep the SEC
phase hunks only, for all four DSC files, and drop the rest of the hunks.

Only SecPeiCpuExceptionHandlerLib needs to be conditionalized.

(2) Style request: given that we have an "!else" branch too, please use
the "==" operator with the "!if".

I think that the double-negation arising from testing "!=" first, and
then having an "!else", is hard to read.

Thanks!
Laszlo