回复: [PATCH V3 0/7] Introduce CcProbe in MdePkg

["gaoliming" <gaoliming@byosoft.com.cn>]

Min:
  The change in MdePkg is good to me. Reviewed-by: Liming Gao
<gaoliming@byosoft.com.cn>

Thanks
Liming
> -----邮件原件-----
> 发件人: Min Xu <min.m.xu@intel.com>
> 发送时间: 2022年4月17日 11:01
> 收件人: devel@edk2.groups.io
> 抄送: Min Xu <min.m.xu@intel.com>; Michael D Kinney
> <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>;
> Zhiguang Liu <zhiguang.liu@intel.com>; James Bottomley
> <jejb@linux.ibm.com>; Jiewen Yao <jiewen.yao@intel.com>; Gerd Hoffmann
> <kraxel@redhat.com>; Brijesh Singh <brijesh.singh@amd.com>; Erdem Aktas
> <erdemaktas@google.com>; Tom Lendacky <thomas.lendacky@amd.com>
> 主题: [PATCH V3 0/7] Introduce CcProbe in MdePkg
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3902
> 
> Bad IO performance in SEC phase is observed after TDX features was
> introduced. (after commit b6b2de884864 - "MdePkg: Support mmio for
> Tdx guest in BaseIoLibIntrinsic").
> 
> This is because IsTdxGuest() will be called in each MMIO operation.
> It is trying to cache the result of the probe in the efi data segment.
> However, that doesn't work in SEC, because the data segment is read only
> (so the write seems to succeed but a read will always return the
> original value), leading to us calling TdIsEnabled() check for every
> mmio we do, which is causing the slowdown because it's very expensive.
> 
> CcProbe is introduced in this patch-set. It is called in
> BaseIoLibIntrinsicSev instead of IsTdxGuest. There are 2 versions of
> the CcProbeLib. Null instance of CcProbe always returns
> CCGuestTypeNonEncrypted. Its OvmfPkg version checks the Ovmf work area
> and returns the CC guest type.
> 
> In this patch-set another issue is fixed with CcProbe as well. If the
> working guest is SEV and in the beginning of SecMain.c TdIsEnabled()
> was called. At this point, exception handling is not established and
> a CPUID instruction will generate a #VC and cause the booting SEV guest
> to crash. Patch #7 is to fix this broken.
> 
> Code is at: https://github.com/mxu9/edk2/tree/cc_probe.v3
> 
> v3 changes:
>  - Fix the broken issue in SEV guest at SecMain.c. Please refer to
>    Patch #7.
> 
> v2 changes:
>  - Rename TdProbe to CcProbe to make the lib work for Confidential
>    Computing guests.
>  - Rename the GUEST_TYPE to CC_GUEST_TYPE and move it from
>    WorkArea.h@OvmfPkg to ConfidentialComputingGuestAttr.h@MdePkg.
>    This is because CcProbeLib is designed to return the CC Guest
>    type and the lib is located at MdePkg.
>  - Rename the CC_GUEST_TYPE's fields name to Camel style. See the
>    commit message in patch #1.
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> 
> Min Xu (7):
>   MdePkg: Add CC_GUEST_TYPE in ConfidentialComputingGuestAttr.h
>   OvmfPkg: Replace GUEST_TYPE with CC_GUEST_TYPE
>   MdePkg: Add CcProbeLib
>   OvmfPkg: Add CcProbeLib
>   OvmfPkg: Add CcProbeLib in *.dsc
>   MdePkg: Probe Cc guest in BaseIoLibIntrinsicSev
>   OvmfPkg: Call CcProbe in SecMain.c instead of TsIsEnabled
> 
>  .../Include/ConfidentialComputingGuestAttr.h  | 11 ++++++-
>  MdePkg/Include/Library/CcProbeLib.h           | 26
> ++++++++++++++++
>  .../BaseIoLibIntrinsicSev.inf                 |  1 +
>  .../BaseIoLibIntrinsic/IoLibInternalTdx.c     | 13 ++------
>  .../Library/CcProbeLibNull/CcProbeLibNull.c   | 26 ++++++++++++++++
>  .../Library/CcProbeLibNull/CcProbeLibNull.inf | 21 +++++++++++++
>  MdePkg/MdePkg.dec                             |  5 +++
>  MdePkg/MdePkg.dsc                             |  1 +
>  OvmfPkg/AmdSev/AmdSevX64.dsc                  |  1 +
>  OvmfPkg/Bhyve/BhyveX64.dsc                    |  1 +
>  OvmfPkg/CloudHv/CloudHvX64.dsc                |  1 +
>  OvmfPkg/Include/WorkArea.h                    |  9 +-----
>  OvmfPkg/IntelTdx/IntelTdxX64.dsc              |  1 +
>  OvmfPkg/IntelTdx/Sec/SecMain.c                |  6 ++--
>  OvmfPkg/IntelTdx/Sec/SecMain.inf              |  1 +
>  .../PeiMemEncryptSevLibInternal.c             |  2 +-
>  .../SecMemEncryptSevLibInternal.c             |  2 +-
>  OvmfPkg/Library/CcProbeLib/CcProbeLib.c       | 31
> +++++++++++++++++++
>  OvmfPkg/Library/CcProbeLib/CcProbeLib.inf     | 25 +++++++++++++++
>  OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c   |  2 +-
>  OvmfPkg/Microvm/MicrovmX64.dsc                |  1 +
>  OvmfPkg/OvmfPkgIa32.dsc                       |  1 +
>  OvmfPkg/OvmfPkgIa32X64.dsc                    |  1 +
>  OvmfPkg/OvmfPkgX64.dsc                        |  1 +
>  OvmfPkg/OvmfXen.dsc                           |  1 +
>  OvmfPkg/Sec/AmdSev.c                          |  2 +-
>  OvmfPkg/Sec/SecMain.c                         |  5 +--
>  OvmfPkg/Sec/SecMain.inf                       |  1 +
>  28 files changed, 170 insertions(+), 29 deletions(-)
>  create mode 100644 MdePkg/Include/Library/CcProbeLib.h
>  create mode 100644 MdePkg/Library/CcProbeLibNull/CcProbeLibNull.c
>  create mode 100644 MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
>  create mode 100644 OvmfPkg/Library/CcProbeLib/CcProbeLib.c
>  create mode 100644 OvmfPkg/Library/CcProbeLib/CcProbeLib.inf
> 
> --
> 2.29.2.windows.2