[edk2-devel] [PATCH V3 0/7] Introduce CcProbe in MdePkg

Min Xu posted 7 patches 2 years ago
Failed in applying to current master (apply log)
There is a newer version of this series
.../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
[edk2-devel] [PATCH V3 0/7] Introduce CcProbe in MdePkg
Posted by Min Xu 2 years ago
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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#88969): https://edk2.groups.io/g/devel/message/88969
Mute This Topic: https://groups.io/mt/90516971/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V3 0/7] Introduce CcProbe in MdePkg
Posted by Lendacky, Thomas via groups.io 2 years ago
On 4/16/22 22:01, Min Xu wrote:
> 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>

After working around the PCI library issue (for which Min will be 
submitting a patch), this series boots successfully for SEV, SEV-ES and 
SEV-SNP when built as X64. I documented the issue that SEV has with 
Ia32X64 in patch 5/7 and I'll have to decide what to do there. So...

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.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
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#89021): https://edk2.groups.io/g/devel/message/89021
Mute This Topic: https://groups.io/mt/90516971/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V3 0/7] Introduce CcProbe in MdePkg
Posted by Min Xu 2 years ago
On April 18, 2022 11:27 PM, Lendacky, Thomas wrote:
> After working around the PCI library issue (for which Min will be submitting a
> patch), this series boots successfully for SEV, SEV-ES and SEV-SNP when built
> as X64. I documented the issue that SEV has with
> Ia32X64 in patch 5/7 and I'll have to decide what to do there. So...
>
Thanks Tom. The PCI library issue workaround will be in a separate patch-set. It will be sent out soon.

Min


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#89028): https://edk2.groups.io/g/devel/message/89028
Mute This Topic: https://groups.io/mt/90516971/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH V3 0/7] Introduce CcProbe in MdePkg
Posted by Yao, Jiewen 2 years ago
Series reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>

> -----Original Message-----
> From: Xu, Min M <min.m.xu@intel.com>
> Sent: Sunday, April 17, 2022 11:01 AM
> To: devel@edk2.groups.io
> Cc: Xu, Min M <min.m.xu@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Liu,
> Zhiguang <zhiguang.liu@intel.com>; James Bottomley <jejb@linux.ibm.com>;
> Yao, Jiewen <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>;
> Brijesh Singh <brijesh.singh@amd.com>; Aktas, Erdem
> <erdemaktas@google.com>; Tom Lendacky <thomas.lendacky@amd.com>
> Subject: [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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#88979): https://edk2.groups.io/g/devel/message/88979
Mute This Topic: https://groups.io/mt/90516971/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] 回复: [PATCH V3 0/7] Introduce CcProbe in MdePkg
Posted by gaoliming 2 years ago
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





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#88982): https://edk2.groups.io/g/devel/message/88982
Mute This Topic: https://groups.io/mt/90517312/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-