public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] MdeModulePkg/NvmExpressPei: Fix Opal S3 unlock issue
@ 2019-10-28  5:08 Maggie Chu
  2019-10-28  5:42 ` Wu, Hao A
  0 siblings, 1 reply; 3+ messages in thread
From: Maggie Chu @ 2019-10-28  5:08 UTC (permalink / raw)
  To: devel; +Cc: Hao A Wu, Jian J Wang, Ray Ni, Star Zeng, Eric Dong

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.

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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] MdeModulePkg/NvmExpressPei: Fix Opal S3 unlock issue
  2019-10-28  5:08 [PATCH] MdeModulePkg/NvmExpressPei: Fix Opal S3 unlock issue Maggie Chu
@ 2019-10-28  5:42 ` Wu, Hao A
  2019-10-30  0:22   ` [edk2-devel] " Wu, Hao A
  0 siblings, 1 reply; 3+ messages in thread
From: Wu, Hao A @ 2019-10-28  5:42 UTC (permalink / raw)
  To: Chu, Maggie, devel@edk2.groups.io
  Cc: Wang, Jian J, Ni, Ray, Zeng, Star, Dong, Eric

> -----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.

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg/NvmExpressPei: Fix Opal S3 unlock issue
  2019-10-28  5:42 ` Wu, Hao A
@ 2019-10-30  0:22   ` Wu, Hao A
  0 siblings, 0 replies; 3+ messages in thread
From: Wu, Hao A @ 2019-10-30  0:22 UTC (permalink / raw)
  To: devel@edk2.groups.io, Wu, Hao A, Chu, Maggie
  Cc: Wang, Jian J, Ni, Ray, Zeng, Star, Dong, Eric

> -----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
> 
> 
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-10-30  0:22 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-10-28  5:08 [PATCH] MdeModulePkg/NvmExpressPei: Fix Opal S3 unlock issue Maggie Chu
2019-10-28  5:42 ` Wu, Hao A
2019-10-30  0:22   ` [edk2-devel] " Wu, Hao A

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox