Re: [edk2-devel] [PATCH v4 08/40] UefiCpuPkg/CpuExceptionHandler: Add base support for the #VC exception

["Lendacky, Thomas" <thomas.lendacky@amd.com>]

On 2/10/20 8:09 AM, Laszlo Ersek wrote:
> On 02/07/20 16:07, Tom Lendacky wrote:
>> On 2/6/20 2:03 AM, Laszlo Ersek wrote:
>>> Hi Tom,
>>>
>>> On 02/05/20 00:01, Lendacky, Thomas wrote:
>>>> BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2198&data=02%7C01%7Cthomas.lendacky%40amd.com%7Cbc9827468c994becadec08d7ae32f2be%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637169406157311599&sdata=l71UCfDDrjvXt6WV3OfZHrwKiQVcAmUa1VVn8Ff5Ijs%3D&reserved=0
>>>>
>>>> Add base support to handle #VC exceptions.  This includes a stub routine
>>>> to invoke when a #VC exception occurs and special checks in the common
>>>> exception handlers to invoke the #VC exception handler routine.
>>>>
>>>> Cc: Eric Dong <eric.dong@intel.com>
>>>> Cc: Ray Ni <ray.ni@intel.com>
>>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>> Cc: Maurice Ma <maurice.ma@intel.com>
>>>> Cc: Guo Dong <guo.dong@intel.com>
>>>> Cc: Benjamin You <benjamin.you@intel.com>
>>>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>>>> ---
>>>>  OvmfPkg/OvmfPkgIa32.dsc                       |  5 ++
>>>>  OvmfPkg/OvmfPkgIa32X64.dsc                    |  5 ++
>>>>  OvmfPkg/OvmfPkgX64.dsc                        |  5 ++
>>>
>>>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
>>>> index 4568b78cadf1..41ec761e3f17 100644
>>>> --- a/OvmfPkg/OvmfPkgIa32.dsc
>>>> +++ b/OvmfPkg/OvmfPkgIa32.dsc
>>>> @@ -236,6 +236,7 @@ [LibraryClasses.common.SEC]
>>>>    PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf
>>>>    MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf
>>>>    CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
>>>> +  VmgExitLib|UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf
>>>>
>>>>  [LibraryClasses.common.PEI_CORE]
>>>>    HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
>>>> @@ -274,6 +275,7 @@ [LibraryClasses.common.PEIM]
>>>>    DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgentLib.inf
>>>>  !endif
>>>>    CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf
>>>> +  VmgExitLib|UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf
>>>>    MpInitLib|UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
>>>>    QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf
>>>>    PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
>>>> @@ -299,6 +301,7 @@ [LibraryClasses.common.DXE_CORE]
>>>>    DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgentLib.inf
>>>>  !endif
>>>>    CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf
>>>> +  VmgExitLib|UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf
>>>>    PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
>>>>
>>>>  [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>>>> @@ -349,6 +352,7 @@ [LibraryClasses.common.DXE_DRIVER]
>>>>    PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
>>>>    QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
>>>>    CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf
>>>> +  VmgExitLib|UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf
>>>>  !if $(SMM_REQUIRE) == TRUE
>>>>    LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.inf
>>>>  !else
>>>> @@ -392,6 +396,7 @@ [LibraryClasses.common.DXE_SMM_DRIVER]
>>>>    DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
>>>>  !endif
>>>>    CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf
>>>> +  VmgExitLib|UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf
>>>>  !if $(SOURCE_DEBUG_ENABLE) == TRUE
>>>>    DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/SmmDebugAgentLib.inf
>>>>  !endif
>>>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
>>>> index 152b5d067116..41cc3eec3757 100644
>>>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
>>>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
>>>> @@ -241,6 +241,7 @@ [LibraryClasses.common.SEC]
>>>>    PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf
>>>>    MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf
>>>>    CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
>>>> +  VmgExitLib|UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf
>>>>
>>>>  [LibraryClasses.common.PEI_CORE]
>>>>    HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
>>>> @@ -279,6 +280,7 @@ [LibraryClasses.common.PEIM]
>>>>    DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgentLib.inf
>>>>  !endif
>>>>    CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf
>>>> +  VmgExitLib|UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf
>>>>    MpInitLib|UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
>>>>    QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf
>>>>    PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
>>>> @@ -304,6 +306,7 @@ [LibraryClasses.common.DXE_CORE]
>>>>    DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgentLib.inf
>>>>  !endif
>>>>    CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf
>>>> +  VmgExitLib|UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf
>>>>    PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
>>>>
>>>>  [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>>>> @@ -354,6 +357,7 @@ [LibraryClasses.common.DXE_DRIVER]
>>>>    PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
>>>>    QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
>>>>    CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf
>>>> +  VmgExitLib|UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf
>>>>  !if $(SMM_REQUIRE) == TRUE
>>>>    LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.inf
>>>>  !else
>>>> @@ -397,6 +401,7 @@ [LibraryClasses.common.DXE_SMM_DRIVER]
>>>>    DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
>>>>  !endif
>>>>    CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf
>>>> +  VmgExitLib|UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf
>>>>  !if $(SOURCE_DEBUG_ENABLE) == TRUE
>>>>    DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/SmmDebugAgentLib.inf
>>>>  !endif
>>>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
>>>> index 4bfad441bd9f..46a679a0073e 100644
>>>> --- a/OvmfPkg/OvmfPkgX64.dsc
>>>> +++ b/OvmfPkg/OvmfPkgX64.dsc
>>>> @@ -241,6 +241,7 @@ [LibraryClasses.common.SEC]
>>>>    PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf
>>>>    MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf
>>>>    CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
>>>> +  VmgExitLib|UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf
>>>>
>>>>  [LibraryClasses.common.PEI_CORE]
>>>>    HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
>>>> @@ -279,6 +280,7 @@ [LibraryClasses.common.PEIM]
>>>>    DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgentLib.inf
>>>>  !endif
>>>>    CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf
>>>> +  VmgExitLib|UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf
>>>>    MpInitLib|UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
>>>>    QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/PeiQemuFwCfgS3LibFwCfg.inf
>>>>    PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
>>>> @@ -304,6 +306,7 @@ [LibraryClasses.common.DXE_CORE]
>>>>    DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgentLib.inf
>>>>  !endif
>>>>    CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf
>>>> +  VmgExitLib|UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf
>>>>    PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
>>>>
>>>>  [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>>>> @@ -354,6 +357,7 @@ [LibraryClasses.common.DXE_DRIVER]
>>>>    PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
>>>>    QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
>>>>    CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf
>>>> +  VmgExitLib|UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf
>>>>  !if $(SMM_REQUIRE) == TRUE
>>>>    LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.inf
>>>>  !else
>>>> @@ -397,6 +401,7 @@ [LibraryClasses.common.DXE_SMM_DRIVER]
>>>>    DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
>>>>  !endif
>>>>    CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf
>>>> +  VmgExitLib|UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf
>>>>  !if $(SOURCE_DEBUG_ENABLE) == TRUE
>>>>    DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/SmmDebugAgentLib.inf
>>>>  !endif
>>>
>>> The three OvmfPkg DSC files should be updated in a different patch; most
>>> likely in the first OvmfPkg patch that relies on #VC exception handling.
>>
>> I did that because the build will fail at this stage without the Ovmf
>> changes:
>>
>> build.py...
>> /root/kernels/ovmf-build-X64/OvmfPkg/OvmfPkgX64.dsc(...): error 4000: Instance of library class [VmgExitLib] is not found
>>         in [/root/kernels/ovmf-build-X64/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf] [X64]
>>         consumed by module [/root/kernels/ovmf-build-X64/UefiCpuPkg/CpuMpPei/CpuMpPei.inf]
>>
>> - Failed -
>>
>> Is there a way that we can avoid updating the Ovmf DSC files entirely when
>> a DSC file specifies one of the exception libraries? Otherwise, I guess I
>> would have to update the Ovmf DSC files before this patch? Or I could make
>> the changes to use the VmgExitLib all at once, which would be a single
>> patch that adds the VmgExitLib and corresponding AMD exception handling
>> source files at once across UefiCpuPkg and Ovmf?
>>
>> (Which triggers that I need to check other DSC files that include these
>> exception libraries to be sure I don't break anything).
> 
> Between the following two patches:
> 
>   [edk2-devel] [PATCH v4 07/40]
>   UefiCpuPkg: Implement library support for VMGEXIT
> 
>   [edk2-devel] [PATCH v4 08/40]
>   UefiCpuPkg/CpuExceptionHandler: Add base support for the #VC exception
> 
> please locate all the platform DSC files that rely on any of the
> exception handler library instances that you extend with a VmgExitLib
> dependency in v4 08/40. And then please insert patches (one patch per
> Pkg) for resolving VmgExitLib in those DSC files.

Ok, that works for me, will do.

Thanks,
Tom

> 
> $ git grep -l -E \
>   'UefiCpuPkg/Library/CpuExceptionHandlerLib/(SecPei|Pei|Dxe|Smm)CpuExceptionHandlerLib\.inf' \
>    -- '*dsc*'
> 
> OvmfPkg/OvmfPkgIa32.dsc
> OvmfPkg/OvmfPkgIa32X64.dsc
> OvmfPkg/OvmfPkgX64.dsc
> OvmfPkg/OvmfXen.dsc
> UefiCpuPkg/UefiCpuPkg.dsc
> UefiPayloadPkg/UefiPayloadPkgIa32.dsc
> UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc
> 
> Thanks!
> Laszlo
> 
> 
>>
>>>
>>> Furthermore, seeing how the same lib instance is used for resolving the
>>> VmgExitLib class for all (necessary) module types, I think you could add
>>> a single common resolution per DSC file, in the [LibraryClasses] section.
>>
>> Yup, makes sense, will do.
>>
>> Thanks,
>> Tom
>>
>>>
>>> Thanks
>>> Laszlo
>>>
>>
>