Re: [edk2-devel] [PATCH] MdeModulePkg/NvmExpressPei: Fix Opal S3 unlock issue

["Wu, Hao A" <hao.a.wu@intel.com>]

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Wu,
> Hao A
> Sent: Monday, October 28, 2019 1:43 PM
> To: Chu, Maggie; devel@edk2.groups.io
> Cc: Wang, Jian J; Ni, Ray; Zeng, Star; Dong, Eric
> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/NvmExpressPei: Fix Opal S3
> unlock issue
> 
> > -----Original Message-----
> > From: Chu, Maggie
> > Sent: Monday, October 28, 2019 1:08 PM
> > To: devel@edk2.groups.io
> > Cc: Wu, Hao A; Wang, Jian J; Ni, Ray; Zeng, Star; Dong, Eric
> > Subject: [PATCH] MdeModulePkg/NvmExpressPei: Fix Opal S3 unlock issue
> >
> > https://bugzilla.tianocore.org/show_bug.cgi?id=2312
> >
> > This patch is for fixing unexpected system hang during S3 unlock process.
> > FatPei driver maintained and updated internal BlockIo devices list
> > when there is new BlockIo PPI has installed, and it relied on BlockIo PPI
> service
> > to get data from devices. Because BlockIo Ppi leverage NvmExpressPei Ppi to
> > transit
> > Nvm command to device, we should make sure NvmePassThruPpi installed
> > before BlockIo PPI.
> 
> 
> The change is good to me,
> Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
> 
> If no other comment, I will adjust the length of some commit log message lines
> when pushing this patch, since the PatchCheck.py script is complaining:
> 
> The commit message format is not valid:
>  * Line 7 of commit message is too long.
>  * Line 8 of commit message is too long.
>  * Line 9 of commit message is too long.


Patch has been pushed via commit dc254af6a4.

Best Regards,
Hao Wu


> 
> Best Regards,
> Hao Wu
> 
> 
> >
> > Signed-off-by: Maggie Chu <maggie.chu@intel.com>
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Eric Dong <eric.dong@intel.com>
> > ---
> >  MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.c | 43
> ++++++++++++-
> > ---------
> >  1 file changed, 23 insertions(+), 20 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.c
> > b/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.c
> > index 987eed420e..a8cb7f3a67 100644
> > --- a/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.c
> > +++ b/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.c
> > @@ -376,6 +376,29 @@ NvmExpressPeimEntry (
> >        continue;
> >
> >      }
> >
> >
> >
> > +    //
> >
> > +    // Nvm Express Pass Thru PPI
> >
> > +    //
> >
> > +    Private->PassThruMode.Attributes            =
> > EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_PHYSICAL |
> >
> > +
> > EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_LOGICAL |
> >
> > +
> > EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_CMD_SET_NVM;
> >
> > +    Private->PassThruMode.IoAlign               = sizeof (UINTN);
> >
> > +    Private->PassThruMode.NvmeVersion           =
> > EDKII_PEI_NVM_EXPRESS_PASS_THRU_PPI_REVISION;
> >
> > +    Private->NvmePassThruPpi.Mode               = &Private->PassThruMode;
> >
> > +    Private->NvmePassThruPpi.GetDevicePath      =
> > NvmePassThruGetDevicePath;
> >
> > +    Private->NvmePassThruPpi.GetNextNameSpace   =
> > NvmePassThruGetNextNameSpace;
> >
> > +    Private->NvmePassThruPpi.PassThru           = NvmePassThru;
> >
> > +    CopyMem (
> >
> > +      &Private->NvmePassThruPpiList,
> >
> > +      &mNvmePassThruPpiListTemplate,
> >
> > +      sizeof (EFI_PEI_PPI_DESCRIPTOR)
> >
> > +      );
> >
> > +    Private->NvmePassThruPpiList.Ppi            = &Private->NvmePassThruPpi;
> >
> > +    PeiServicesInstallPpi (&Private->NvmePassThruPpiList);
> >
> > +
> >
> > +    //
> >
> > +    // Block Io PPI
> >
> > +    //
> >
> >      Private->BlkIoPpi.GetNumberOfBlockDevices  =
> > NvmeBlockIoPeimGetDeviceNo;
> >
> >      Private->BlkIoPpi.GetBlockDeviceMediaInfo  =
> > NvmeBlockIoPeimGetMediaInfo;
> >
> >      Private->BlkIoPpi.ReadBlocks               = NvmeBlockIoPeimReadBlocks;
> >
> > @@ -398,26 +421,6 @@ NvmExpressPeimEntry (
> >      Private->BlkIo2PpiList.Ppi                 = &Private->BlkIo2Ppi;
> >
> >      PeiServicesInstallPpi (&Private->BlkIoPpiList);
> >
> >
> >
> > -    //
> >
> > -    // Nvm Express Pass Thru PPI
> >
> > -    //
> >
> > -    Private->PassThruMode.Attributes            =
> > EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_PHYSICAL |
> >
> > -
> > EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_LOGICAL |
> >
> > -
> > EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_CMD_SET_NVM;
> >
> > -    Private->PassThruMode.IoAlign               = sizeof (UINTN);
> >
> > -    Private->PassThruMode.NvmeVersion           =
> > EDKII_PEI_NVM_EXPRESS_PASS_THRU_PPI_REVISION;
> >
> > -    Private->NvmePassThruPpi.Mode               = &Private->PassThruMode;
> >
> > -    Private->NvmePassThruPpi.GetDevicePath      =
> > NvmePassThruGetDevicePath;
> >
> > -    Private->NvmePassThruPpi.GetNextNameSpace   =
> > NvmePassThruGetNextNameSpace;
> >
> > -    Private->NvmePassThruPpi.PassThru           = NvmePassThru;
> >
> > -    CopyMem (
> >
> > -      &Private->NvmePassThruPpiList,
> >
> > -      &mNvmePassThruPpiListTemplate,
> >
> > -      sizeof (EFI_PEI_PPI_DESCRIPTOR)
> >
> > -      );
> >
> > -    Private->NvmePassThruPpiList.Ppi            = &Private->NvmePassThruPpi;
> >
> > -    PeiServicesInstallPpi (&Private->NvmePassThruPpiList);
> >
> > -
> >
> >      //
> >
> >      // Check if the NVME controller supports the Security Receive/Send
> > commands
> >
> >      //
> >
> > --
> > 2.16.2.windows.1
> 
> 
>