Re: [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev

["duntan" <dun.tan@intel.com>]

Hi all,

Could you please help to review this patch set? In this patch set, the IoLib instance BaseIoLibIntrinsic is modified to support AMD SEV feature and the BaseIoLibIntrinsicSev is removed.
Also could you help to do a test on AMD processor to make sure that the SEV feature still works good with this patch set?

Thanks,
Dun

-----Original Message-----
From: Tan, Dun 
Sent: Friday, October 27, 2023 3:35 PM
To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
Subject: RE: [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev

Thanks for the suggestion. 
I'll update the test result once I finished the test. Also the abstract message in this patch has been modified to mention that this patch should not be merged now.

Thanks,
Dun

-----Original Message-----
From: Yao, Jiewen <jiewen.yao@intel.com>
Sent: Friday, October 27, 2023 3:07 PM
To: Tan, Dun <dun.tan@intel.com>; devel@edk2.groups.io
Subject: RE: [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev

Here is my suggestion:

1) Please perform the test to ensure the functional part is correct.

Without that, how can people know you are doing things right?

2) If you do not run any test, before you send out patch, please call out that clearly.
That is important to reminder the maintainer: Don't merge, even if it pass review.

Otherwise, once the review passed, the maintainer may merge it.
I don't think that is the intention.



Thank you
Yao, Jiewen
 

> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Friday, October 27, 2023 2:32 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> Subject: RE: [edk2-devel] [PATCH 0/7] Support Tdx and sev in 
> BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
> 
> Hi Jiewen,
> 
> Currently I'm working on the Tdx test. Since the patch set doesn't 
> change the code logic when Tdx or SEV is enabled, so I want to send 
> out the patch as soon as possible to see if there is any comments from community.
> 
> I will include AMD SEV reviewer in this patch series. Thanks for reminding.
> 
> Thanks,
> Dun
> 
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Friday, October 27, 2023 1:49 PM
> To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>
> Subject: RE: [edk2-devel] [PATCH 0/7] Support Tdx and sev in 
> BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
> 
> HI
> Since this impact TDX and SEV, would you please let me know what kind 
> of test you have done?
> Have you validated TDX and SEV before you submit the patch? Please 
> describe that clearly in your patch description.
> 
> Also please include AMD SEV reviewer in this patch series.
> 
> Thank you
> Yao, Jiewen
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of 
> > duntan
> > Sent: Friday, October 27, 2023 1:43 PM
> > To: devel@edk2.groups.io
> > Subject: [edk2-devel] [PATCH 0/7] Support Tdx and sev in 
> > BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev (Don't merge 
> > because the test hasn't been completed yet.)
> >
> > The goal is to have single BaseIoLibIntrinsic instance that can also 
> > used for sev and Tdx.
> > In this patch set, string I/O instructions are deleted in IoRead/WriteFifo API.
> > Then change the source file of BaseIoLibIntrinsic to also support 
> > Tdx and sev feature. So BaseIoLibIntrinsicSev and related assembly 
> > code can be
> removed.
> >
> > Dun Tan (7):
> >   MdePkg: Create TdxLibNull.inf instance
> >   MdePkg: Add CcProbeLibNull and TdxLibNull implement
> >   MdePkg: simplify IoRead/WriteFifo in IoLibFifo.c
> >   MdePkg:support Tdx and sev in BaseIoLibIntrinsic
> >   OvmfPkg: Add CcProbeLib in PlatformInitLib.inf
> >   OvmfPkg: use BaseIoLibIntrinsic.inf in dsc files
> >   MdePkg:remove BaseIoLibIntrinsicSev related code
> >
> >  MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf    |  14 ++++++++++---
> -
> >  MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf |  61
> > ------------------
> > -------------------------------------------
> >  MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm          | 131 -----------------
> ---
> > --------------------------------------------------------------------
> > --
> > ----------------------------
> > -------------
> >  MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm       | 293 ---------------
> ---
> > --------------------------------------------------------------------
> > --
> > ----------------------------
> > --------------------------------------------------------------------
> > --
> > ----------------------------
> > -------------------------------------------------------------------------------
> >  MdePkg/Library/BaseIoLibIntrinsic/IoLibFifo.c               |  45
> > +++++++++++++++++++++++++++++++++++++--------
> >  MdePkg/Library/BaseIoLibIntrinsic/IoLibSev.h                | 166 ----------------------
> --
> > --------------------------------------------------------------------
> > --
> > ----------------------------
> > --------------------------------------------
> >  MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm           | 120 ------------------
> --
> > --------------------------------------------------------------------
> > --
> > ----------------------------
> > --
> >  MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm        | 282 ----------------
> --
> > --------------------------------------------------------------------
> > --
> > ----------------------------
> > --------------------------------------------------------------------
> > --
> > ----------------------------
> > --------------------------------------------------------------------
> >  MdePkg/Library/TdxLib/TdxLibNull.inf                        |  21
> > +++++++++++++++++++++
> >  MdePkg/MdeLibs.dsc.inc                                      |   4 +++-
> >  MdePkg/MdePkg.dsc                                           |   2 +-
> >  OvmfPkg/AmdSev/AmdSevX64.dsc                                |   2 +-
> >  OvmfPkg/Bhyve/BhyveX64.dsc                                  |   2 +-
> >  OvmfPkg/CloudHv/CloudHvX64.dsc                              |   2 +-
> >  OvmfPkg/IntelTdx/IntelTdxX64.dsc                            |   2 +-
> >  OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf         |   3 ++-
> >  OvmfPkg/Microvm/MicrovmX64.dsc                              |   2 +-
> >  OvmfPkg/OvmfPkgIa32.dsc                                     |   2 +-
> >  OvmfPkg/OvmfPkgIa32X64.dsc                                  |   2 +-
> >  OvmfPkg/OvmfPkgX64.dsc                                      |   2 +-
> >  OvmfPkg/OvmfXen.dsc                                         |   2 +-
> >  21 files changed, 83 insertions(+), 1077 deletions(-)  delete mode
> > 100644 MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
> >  delete mode 100644
> > MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm
> >  delete mode 100644
> > MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm
> >  delete mode 100644 MdePkg/Library/BaseIoLibIntrinsic/IoLibSev.h
> >  delete mode 100644
> > MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm
> >  delete mode 100644
> > MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm
> >  create mode 100644 MdePkg/Library/TdxLib/TdxLibNull.inf
> >
> > --
> > 2.31.1.windows.1
> >
> >
> >
> > 
> >



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110199): https://edk2.groups.io/g/devel/message/110199
Mute This Topic: https://groups.io/mt/102215661/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-