public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Wu, Hao A" <hao.a.wu@intel.com>
To: "Wu, Hao A" <hao.a.wu@intel.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Dong, Eric" <eric.dong@intel.com>,
	"Yao, Jiewen" <jiewen.yao@intel.com>,
	 "Zeng, Star" <star.zeng@intel.com>,
	Laszlo Ersek <lersek@redhat.com>,
	"Zhang, Chao B" <chao.b.zhang@intel.com>
Subject: Re: [PATCH v6 00/13] Split the S3 PEI phase HW init codes from Opal driver
Date: Fri, 22 Feb 2019 01:54:11 +0000	[thread overview]
Message-ID: <B80AF82E9BFB8E4FBD8C89DA810C6A093C89A1B2@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <20190221002440.2272-1-hao.a.wu@intel.com>

Thanks all,

Series pushed via commits:
94e0dd1afe53d23286968130d836ff0755c17c9b..a3efbc29c45183fe69bcb311c2d974ddc4e7c00a

Best Regards,
Hao Wu


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Hao Wu
> Sent: Thursday, February 21, 2019 8:24 AM
> To: edk2-devel@lists.01.org
> Cc: Dong, Eric; Wu, Hao A; Yao, Jiewen; Zeng, Star; Laszlo Ersek; Zhang, Chao
> B
> Subject: [edk2] [PATCH v6 00/13] Split the S3 PEI phase HW init codes from
> Opal driver
> 
> The series is also available at:
> https://github.com/hwu25/edk2/tree/opal_remodel_v6
> 
> V6 changes:
> For patch 6, rename driver internal function NvmeCheckHcDevicePath() to
> NvmeIsHcDevicePathValid().
> 
> For patch 7, add more comments for purpose of selective NVM Express
> controller initialization during S3 resume.
> 
> For patch 8:
> * Rename driver internal function AhciCheckHcDevicePath() to
>   AhciIsHcDevicePathValid();
> * Add more comments for purpose of selective port enumeration/
>   initialization on a ATA AHCI controller during S3 resume.
> 
> For patch 6~8, since no functional impact, I still keep the 'Reviewed-by'
> from Eric.
> 
> For patch 13, as suggested by Ray, simplifies the logic within PPI
> notification callback to directly use the PPI instance provided by the
> input parameter of callback function.
> 
> 
> V5 history:
> For patch 11, as suggested by Star:
> * Refine the UpdateLockBox() API description comment to be more precise;
> * Ensure the parameter for macro 'EFI_SIZE_TO_PAGES' is of type 'UINTN';
> 
> Since the changes are minor, I still keep the origin 'Reviewed-by' from
> Ray and the 'Acked-by' tag from Laszlo.
> 
> For patch 12, sync the UpdateLockBox() API description comment with the
> change made in patch 11.
> 
> Since the changes are minor, I still keep the origin 'Reviewed-by' from
> Laszlo an Ray.
> 
> For patch 13, additional handling logic to prevent NULL pointer
> de-reference, just as what has been done in commit
> d72d8561fbe03a64e01c2ad57a93777de4b9ae2f.
> 
> 
> V4 history:
> 
> As suggested by Laszlo, add a new patch to sync the API description
> comment for UpdateLockBox() in file
> "OvmfPkg/Library/LockBoxLib/LockBoxLib.c" with its counterparts in
> MdeModulePkg.
> 
> For patch 13 (compared with patch 12 in V3), minor type refinement in
> structure definition 'OPAL_DEVICE_LOCKBOX_DATA'.
> 
> V3 history:
> 
> For patch 2, reuse the definitions within AtaPassThru protocol header file
> to reduce code duplication.
> 
> For patch 4, add detailed comments to illustrate the content that will be
> stored in the newly introduced LockBox.
> 
> For patch 6, device path validity check refinement within function
> NvmeCheckHcDevicePath().
> 
> For patch 7:
> * Remove a redundant check within function NvmeS3SkipThisController();
> * Replace internal implementation of GetNextDevicePathInstance() with
>   GetDevicePathInstanceSize(), which avoids unnecessary memory allocation.
> 
> For patch 8:
> * Device path validity check refinement within function
>   AhciCheckHcDevicePath();
> * Remove a redundant check within function AhciS3GetEumeratePorts();
> * Replace internal implementation of GetNextDevicePathInstance() with
>   GetDevicePathInstanceSize(), which avoids unnecessary memory allocation.
> 
> For patch 11:
> * Remove the ASSERT() for memory allocation failure. Since error handling
>   codes already exist, no new code is added for this;
> * Refine the codes to only allocate new SMM buffer when the required
>   LockBox size is greater than the size of origin pages allocated;
> * Add additional parameter check for 'Offset' & 'Length' to prevent
>   potential numeric overflow.
> 
> 
> V2 history:
> 
> For patch 8, the new series removes the codes to produce the Block IO PPIs
> from the AhciPei driver.
> 
> The task to produce the Block IO services is out of the scope of BZ-1409
> (actually covered by BZ-1483). And we will later propose seperate patch(s)
> to address this.
> 
> V1 history:
> 
> For the below 2 types of storage device:
> 
> 1. NVM Express devices;
> 2. ATA hard disk devices working under AHCI mode.
> 
> the OpalPassword driver supports auto-unlocking those devices during S3
> resume.
> 
> Current implementation of the OpalPassword driver is handling the device
> initialization (using boot script to restore the host controller PCI
> configuration space also counts) in the PEI phase during S3 resume by
> itself.
> 
> Meanwhile, the NvmExpressPei driver in MdeModulePkg also handles the
> NVME
> device initialization during the PEI phase in order to produce the Block
> IO PPI.
> 
> Moreover, there is a Bugzilla request (BZ-1483) for adding the Block IO
> PPI support for ATA device as well. So there is likely to be an PEI driver
> for ATA device that will handle the ATA device initialization.
> 
> In order to remove code duplication, the series will split the S3 phase
> device initialization related codes out from the OpalPassword driver. And
> let the existing NvmExpressPei driver and the new AhciPei driver to handle
> the task.
> 
> After this remodel, NvmExpressPei and AhciPei drivers will produce a PPI
> called Storage Security Command PPI. And the OpalPassword driver will
> consume this PPI to perform the device auto-unlock in S3 resume.
> 
> 
> Patch   1~4: Add the definitions of PPIs and GUIDs;
> Patch     5: Refinement for the NvmExpressPei driver;
> Patch   6~7: Update the NvmExpressPei driver to produce the PPI needed by
>              OpalPassword;
> Patch     8: Add the Ahci mode ATA device support in the PEI phase, it
>              will produce the PPI needed by OpalPassword;
> Patch  9~10: Refinements for the SmmLockBoxLib;
> Patch    11: Support LockBox enlarge for LockBoxLib API UpdateLockBox();
> Patch    12: Remove the hardware initialization codes from the
>              OpalPassword driver. And consume the SSC PPI to unlock device
> 	     in S3 resume.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> 
> Hao Wu (12):
>   MdeModulePkg: Add definitions for ATA AHCI host controller PPI
>   MdeModulePkg: Add definitions for EDKII PEI ATA PassThru PPI
>   MdeModulePkg: Add definitions for Storage Security Command PPI
>   MdeModulePkg: Add GUID for LockBox to save storage dev to init in S3
>   MdeModulePkg/NvmExpressPei: Avoid updating the module-level variable
>   MdeModulePkg/NvmExpressPei: Add logic to produce SSC PPI
>   MdeModulePkg/NvmExpressPei: Consume S3StorageDeviceInitList LockBox
>   MdeModulePkg/AhciPei: Add AHCI mode ATA device support in PEI
>   MdeModulePkg/SmmLockBoxLib: Use 'DEBUG_' prefix instead of 'EFI_D_'
>   MdeModulePkg/SmmLockBox(PEI): Remove an ASSERT in RestoreLockBox()
>   MdeModulePkg/SmmLockBoxLib: Support LockBox enlarge in
> UpdateLockBox()
>   OvmfPkg/LockBoxLib: Update the comments for API UpdateLockBox()
>   SecurityPkg/OpalPassword: Remove HW init codes and consume SSC PPI
> 
>  MdeModulePkg/MdeModulePkg.dec                                     |   12 +
>  MdeModulePkg/MdeModulePkg.dsc                                     |    1 +
>  MdeModulePkg/Bus/Ata/AhciPei/AhciPei.inf                          |   74 +
>  MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.inf              |   18 +-
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordDxe.inf             |    6 +-
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordPei.inf             |   12 +-
>  MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h                            |  708 +++++++
>  MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.h                    |  184 ++
>  MdeModulePkg/Bus/Ata/AhciPei/AhciPeiStorageSecurity.h             |  247 +++
>  MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.h                |  106 +-
>  MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPeiHci.h             |   20 +-
>  MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPeiStorageSecurity.h
> |  247 +++
>  MdeModulePkg/Include/Guid/S3StorageDeviceInitList.h               |   64 +
>  MdeModulePkg/Include/Library/LockBoxLib.h                         |    7 +-
>  MdeModulePkg/Include/Ppi/AtaAhciController.h                      |   89 +
>  MdeModulePkg/Include/Ppi/AtaPassThru.h                            |  219 +++
>  MdeModulePkg/Include/Ppi/StorageSecurityCommand.h                 |  283 +++
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalAhciMode.h                  |  412 ----
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h                    |    4 +-
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalNvmeMode.h                  |  327 ----
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalNvmeReg.h                   |  815 --------
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordCommon.h            |   45 +-
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordPei.h               |   97 +-
>  MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c                           | 2015
> ++++++++++++++++++++
>  MdeModulePkg/Bus/Ata/AhciPei/AhciPei.c                            |  310 +++
>  MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c                    |  521 +++++
>  MdeModulePkg/Bus/Ata/AhciPei/AhciPeiS3.c                          |  139 ++
>  MdeModulePkg/Bus/Ata/AhciPei/AhciPeiStorageSecurity.c             |  391
> ++++
>  MdeModulePkg/Bus/Ata/AhciPei/DevicePath.c                         |  284 +++
>  MdeModulePkg/Bus/Ata/AhciPei/DmaMem.c                             |  270 +++
>  MdeModulePkg/Bus/Pci/NvmExpressPei/DevicePath.c                   |  284 +++
>  MdeModulePkg/Bus/Pci/NvmExpressPei/DmaMem.c                       |  153 +-
>  MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.c                |  172 +-
>  MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPeiHci.c             |   32 +-
>  MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPeiS3.c              |  114
> ++
>  MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPeiStorageSecurity.c
> |  423 ++++
>  MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.c              |    7 +-
>  MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c             |   27
> +-
>  MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c             |   32
> +-
>  MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c             |
> 148 +-
>  OvmfPkg/Library/LockBoxLib/LockBoxLib.c                           |   11 +-
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalAhciMode.c                  | 1282 --------
> -----
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c                    |  414 ++--
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalNvmeMode.c                  | 1823 ------
> ------------
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordPei.c               |  702 ++----
> -
>  MdeModulePkg/Bus/Ata/AhciPei/AhciPei.uni                          |   21 +
>  MdeModulePkg/Bus/Ata/AhciPei/AhciPeiExtra.uni                     |   19 +
>  47 files changed, 7680 insertions(+), 5911 deletions(-)
>  create mode 100644 MdeModulePkg/Bus/Ata/AhciPei/AhciPei.inf
>  create mode 100644 MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
>  create mode 100644 MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.h
>  create mode 100644
> MdeModulePkg/Bus/Ata/AhciPei/AhciPeiStorageSecurity.h
>  create mode 100644
> MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPeiStorageSecurity.h
>  create mode 100644
> MdeModulePkg/Include/Guid/S3StorageDeviceInitList.h
>  create mode 100644 MdeModulePkg/Include/Ppi/AtaAhciController.h
>  create mode 100644 MdeModulePkg/Include/Ppi/AtaPassThru.h
>  create mode 100644
> MdeModulePkg/Include/Ppi/StorageSecurityCommand.h
>  delete mode 100644 SecurityPkg/Tcg/Opal/OpalPassword/OpalAhciMode.h
>  delete mode 100644
> SecurityPkg/Tcg/Opal/OpalPassword/OpalNvmeMode.h
>  delete mode 100644 SecurityPkg/Tcg/Opal/OpalPassword/OpalNvmeReg.h
>  create mode 100644 MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
>  create mode 100644 MdeModulePkg/Bus/Ata/AhciPei/AhciPei.c
>  create mode 100644 MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
>  create mode 100644 MdeModulePkg/Bus/Ata/AhciPei/AhciPeiS3.c
>  create mode 100644
> MdeModulePkg/Bus/Ata/AhciPei/AhciPeiStorageSecurity.c
>  create mode 100644 MdeModulePkg/Bus/Ata/AhciPei/DevicePath.c
>  create mode 100644 MdeModulePkg/Bus/Ata/AhciPei/DmaMem.c
>  create mode 100644 MdeModulePkg/Bus/Pci/NvmExpressPei/DevicePath.c
>  create mode 100644
> MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPeiS3.c
>  create mode 100644
> MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPeiStorageSecurity.c
>  delete mode 100644 SecurityPkg/Tcg/Opal/OpalPassword/OpalAhciMode.c
>  delete mode 100644
> SecurityPkg/Tcg/Opal/OpalPassword/OpalNvmeMode.c
>  create mode 100644 MdeModulePkg/Bus/Ata/AhciPei/AhciPei.uni
>  create mode 100644 MdeModulePkg/Bus/Ata/AhciPei/AhciPeiExtra.uni
> 
> --
> 2.12.0.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


      parent reply	other threads:[~2019-02-22  1:54 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-21  0:24 [PATCH v6 00/13] Split the S3 PEI phase HW init codes from Opal driver Hao Wu
2019-02-21  0:24 ` [PATCH v6 01/13] MdeModulePkg: Add definitions for ATA AHCI host controller PPI Hao Wu
2019-02-21  0:24 ` [PATCH v6 02/13] MdeModulePkg: Add definitions for EDKII PEI ATA PassThru PPI Hao Wu
2019-02-21  0:24 ` [PATCH v6 03/13] MdeModulePkg: Add definitions for Storage Security Command PPI Hao Wu
2019-02-21  0:24 ` [PATCH v6 04/13] MdeModulePkg: Add GUID for LockBox to save storage dev to init in S3 Hao Wu
2019-02-21  0:24 ` [PATCH v6 05/13] MdeModulePkg/NvmExpressPei: Avoid updating the module-level variable Hao Wu
2019-02-21  0:24 ` [PATCH v6 06/13] MdeModulePkg/NvmExpressPei: Add logic to produce SSC PPI Hao Wu
2019-02-21  0:24 ` [PATCH v6 07/13] MdeModulePkg/NvmExpressPei: Consume S3StorageDeviceInitList LockBox Hao Wu
2019-02-21  0:24 ` [PATCH v6 08/13] MdeModulePkg/AhciPei: Add AHCI mode ATA device support in PEI Hao Wu
2019-02-21  0:24 ` [PATCH v6 09/13] MdeModulePkg/SmmLockBoxLib: Use 'DEBUG_' prefix instead of 'EFI_D_' Hao Wu
2019-02-21  0:24 ` [PATCH v6 10/13] MdeModulePkg/SmmLockBox(PEI): Remove an ASSERT in RestoreLockBox() Hao Wu
2019-02-21  0:24 ` [PATCH v6 11/13] MdeModulePkg/SmmLockBoxLib: Support LockBox enlarge in UpdateLockBox() Hao Wu
2019-02-21  0:24 ` [PATCH v6 12/13] OvmfPkg/LockBoxLib: Update the comments for API UpdateLockBox() Hao Wu
2019-02-21  0:24 ` [PATCH v6 13/13] SecurityPkg/OpalPassword: Remove HW init codes and consume SSC PPI Hao Wu
2019-02-21  4:50   ` Ni, Ray
2019-02-21  5:48   ` Dong, Eric
2019-02-22  1:54 ` Wu, Hao A [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=B80AF82E9BFB8E4FBD8C89DA810C6A093C89A1B2@SHSMSX104.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox