* [PATCH 0/2] Notify NVME HW when system reset happens
@ 2017-08-07 3:31 Ruiyu Ni
2017-08-07 3:31 ` [PATCH 1/2] MdePkg/Nvme: Add NVME shutdown notification related macros Ruiyu Ni
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Ruiyu Ni @ 2017-08-07 3:31 UTC (permalink / raw)
To: edk2-devel
Per NVM Express Spec, software should notify NVME HW when shutdown
occurs.
The host should set the Shutdown Notification (CC.SHN) field to 01b
to indicate a normal shutdown operation. The controller indicates
when shutdown processing is completed by updating the Shutdown Status
(CSTS.SHST) field to 10b.
Ruiyu Ni (2):
MdePkg/Nvme: Add NVME shutdown notification related macros
MdeModulePkg/NvmExpressDxe: Notify NVME HW when system reset happens
MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c | 7 +-
MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.h | 23 ++-
.../Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf | 3 +-
MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c | 181 ++++++++++++++++++++-
MdePkg/Include/IndustryStandard/Nvme.h | 6 +-
5 files changed, 215 insertions(+), 5 deletions(-)
--
2.12.2.windows.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] MdePkg/Nvme: Add NVME shutdown notification related macros
2017-08-07 3:31 [PATCH 0/2] Notify NVME HW when system reset happens Ruiyu Ni
@ 2017-08-07 3:31 ` Ruiyu Ni
2017-08-07 3:31 ` [PATCH 2/2] MdeModulePkg/NvmExpressDxe: Notify NVME HW when system reset happens Ruiyu Ni
2017-08-07 7:14 ` [PATCH 0/2] " Wu, Hao A
2 siblings, 0 replies; 6+ messages in thread
From: Ruiyu Ni @ 2017-08-07 3:31 UTC (permalink / raw)
To: edk2-devel; +Cc: Hao A Wu
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
---
MdePkg/Include/IndustryStandard/Nvme.h | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/MdePkg/Include/IndustryStandard/Nvme.h b/MdePkg/Include/IndustryStandard/Nvme.h
index 85649f0d3f..d0bd354139 100644
--- a/MdePkg/Include/IndustryStandard/Nvme.h
+++ b/MdePkg/Include/IndustryStandard/Nvme.h
@@ -2,6 +2,7 @@
Definitions based on NVMe spec. version 1.1.
(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
+ Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -86,6 +87,8 @@ typedef struct {
UINT8 Iocqes:4; // I/O Completion Queue Entry Size
UINT8 Rsvd2;
} NVME_CC;
+#define NVME_CC_SHN_NORMAL_SHUTDOWN 1
+#define NVME_CC_SHN_ABRUPT_SHUTDOWN 2
//
// 3.1.6 Offset 1Ch: CSTS - Controller Status
@@ -97,7 +100,8 @@ typedef struct {
UINT32 Nssro:1; // NVM Subsystem Reset Occurred
UINT32 Rsvd1:27;
} NVME_CSTS;
-
+#define NVME_CSTS_SHST_SHUTDOWN_OCCURRING 1
+#define NVME_CSTS_SHST_SHUTDOWN_COMPLETED 2
//
// 3.1.8 Offset 24h: AQA - Admin Queue Attributes
//
--
2.12.2.windows.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] MdeModulePkg/NvmExpressDxe: Notify NVME HW when system reset happens
2017-08-07 3:31 [PATCH 0/2] Notify NVME HW when system reset happens Ruiyu Ni
2017-08-07 3:31 ` [PATCH 1/2] MdePkg/Nvme: Add NVME shutdown notification related macros Ruiyu Ni
@ 2017-08-07 3:31 ` Ruiyu Ni
2017-08-07 7:14 ` [PATCH 0/2] " Wu, Hao A
2 siblings, 0 replies; 6+ messages in thread
From: Ruiyu Ni @ 2017-08-07 3:31 UTC (permalink / raw)
To: edk2-devel; +Cc: Hao A Wu
Per NVM Express Spec, software should notify NVME HW when shutdown
occurs.
The host should set the Shutdown Notification (CC.SHN) field to 01b
to indicate a normal shutdown operation. The controller indicates
when shutdown processing is completed by updating the Shutdown Status
(CSTS.SHST) field to 10b.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
---
MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c | 7 +-
MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.h | 23 ++-
.../Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf | 3 +-
MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c | 181 ++++++++++++++++++++-
4 files changed, 210 insertions(+), 4 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
index de5c2a05ea..571c2e1b78 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
@@ -2,7 +2,7 @@
NvmExpressDxe driver is used to manage non-volatile memory subsystem which follows
NVM Express specification.
- Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -1046,6 +1046,8 @@ NvmExpressDriverBindingStart (
if (EFI_ERROR (Status)) {
goto Exit;
}
+
+ NvmeRegisterShutdownNotification ();
} else {
Status = gBS->OpenProtocol (
Controller,
@@ -1239,6 +1241,9 @@ NvmExpressDriverBindingStop (
This->DriverBindingHandle,
Controller
);
+
+ NvmeUnregisterShutdownNotification ();
+
return EFI_SUCCESS;
}
diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.h b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.h
index fa4a34ab53..ad0d9b8966 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.h
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.h
@@ -3,7 +3,7 @@
NVM Express specification.
(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
- Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -34,6 +34,7 @@
#include <Protocol/DiskInfo.h>
#include <Protocol/DriverSupportedEfiVersion.h>
#include <Protocol/StorageSecurityCommand.h>
+#include <Protocol/ResetNotification.h>
#include <Library/BaseLib.h>
#include <Library/BaseMemoryLib.h>
@@ -721,4 +722,24 @@ NvmeDumpStatus (
IN NVME_CQ *Cq
);
+/**
+ Register the shutdown notification through the ResetNotification protocol.
+
+ Register the shutdown notification when mNvmeControllerNumber increased from 0 to 1.
+**/
+VOID
+NvmeRegisterShutdownNotification (
+ VOID
+ );
+
+/**
+ Unregister the shutdown notification through the ResetNotification protocol.
+
+ Unregister the shutdown notification when mNvmeControllerNumber decreased from 1 to 0.
+**/
+VOID
+NvmeUnregisterShutdownNotification (
+ VOID
+ );
+
#endif
diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf
index 3270042e02..4918696104 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf
@@ -4,7 +4,7 @@
# NvmExpressDxe driver is used to manage non-volatile memory subsystem which follows
# NVM Express specification.
#
-# Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved.<BR>
#
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
@@ -71,6 +71,7 @@ [Protocols]
gEfiDiskInfoProtocolGuid ## BY_START
gEfiStorageSecurityCommandProtocolGuid ## BY_START
gEfiDriverSupportedEfiVersionProtocolGuid ## PRODUCES
+ gEfiResetNotificationProtocolGuid ## CONSUMES
# [Event]
# EVENT_TYPE_RELATIVE_TIMER ## SOMETIMES_CONSUMES
diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c
index ad6cdb15a5..ea48e78827 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c
@@ -2,7 +2,7 @@
NvmExpressDxe driver is used to manage non-volatile memory subsystem which follows
NVM Express specification.
- Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -15,6 +15,14 @@
#include "NvmExpress.h"
+#define NVME_SHUTDOWN_PROCESS_TIMEOUT 45
+
+//
+// The number of NVME controllers managed by this driver, used by
+// NvmeRegisterShutdownNotification() and NvmeUnregisterShutdownNotification().
+//
+UINTN mNvmeControllerNumber = 0;
+
/**
Read Nvm Express controller capability register.
@@ -1050,3 +1058,174 @@ NvmeControllerInit (
return Status;
}
+/**
+ This routine is called to properly shutdown the Nvm Express controller per NVMe spec.
+
+ @param[in] ResetType The type of reset to perform.
+ @param[in] ResetStatus The status code for the reset.
+ @param[in] DataSize The size, in bytes, of ResetData.
+ @param[in] ResetData For a ResetType of EfiResetCold, EfiResetWarm, or
+ EfiResetShutdown the data buffer starts with a Null-terminated
+ string, optionally followed by additional binary data.
+ The string is a description that the caller may use to further
+ indicate the reason for the system reset. ResetData is only
+ valid if ResetStatus is something other than EFI_SUCCESS
+ unless the ResetType is EfiResetPlatformSpecific
+ where a minimum amount of ResetData is always required.
+ For a ResetType of EfiResetPlatformSpecific the data buffer
+ also starts with a Null-terminated string that is followed
+ by an EFI_GUID that describes the specific type of reset to perform.
+**/
+VOID
+EFIAPI
+NvmeShutdownAllControllers (
+ IN EFI_RESET_TYPE ResetType,
+ IN EFI_STATUS ResetStatus,
+ IN UINTN DataSize,
+ IN VOID *ResetData OPTIONAL
+ )
+{
+ EFI_STATUS Status;
+ EFI_HANDLE *Handles;
+ UINTN HandleCount;
+ UINTN HandleIndex;
+ EFI_OPEN_PROTOCOL_INFORMATION_ENTRY *OpenInfos;
+ UINTN OpenInfoCount;
+ UINTN OpenInfoIndex;
+ EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL *NvmePassThru;
+ NVME_CC Cc;
+ NVME_CSTS Csts;
+ UINTN Index;
+ NVME_CONTROLLER_PRIVATE_DATA *Private;
+
+ Status = gBS->LocateHandleBuffer (
+ ByProtocol,
+ &gEfiPciIoProtocolGuid,
+ NULL,
+ &HandleCount,
+ &Handles
+ );
+ if (EFI_ERROR (Status)) {
+ HandleCount = 0;
+ }
+
+ for (HandleIndex = 0; HandleIndex < HandleCount; HandleIndex++) {
+ Status = gBS->OpenProtocolInformation (
+ Handles[HandleIndex],
+ &gEfiPciIoProtocolGuid,
+ &OpenInfos,
+ &OpenInfoCount
+ );
+ if (EFI_ERROR (Status)) {
+ continue;
+ }
+
+ for (OpenInfoIndex = 0; OpenInfoIndex < OpenInfoCount; OpenInfoIndex++) {
+ //
+ // Find all the NVME controller managed by this driver.
+ // gImageHandle equals to DriverBinding handle for this driver.
+ //
+ if (((OpenInfos[OpenInfoIndex].Attributes & EFI_OPEN_PROTOCOL_BY_DRIVER) != 0) &&
+ (OpenInfos[OpenInfoIndex].AgentHandle == gImageHandle)) {
+ Status = gBS->OpenProtocol (
+ OpenInfos[OpenInfoIndex].ControllerHandle,
+ &gEfiNvmExpressPassThruProtocolGuid,
+ (VOID **) &NvmePassThru,
+ NULL,
+ NULL,
+ EFI_OPEN_PROTOCOL_GET_PROTOCOL
+ );
+ if (EFI_ERROR (Status)) {
+ continue;
+ }
+ Private = NVME_CONTROLLER_PRIVATE_DATA_FROM_PASS_THRU (NvmePassThru);
+
+ //
+ // Read Controller Configuration Register.
+ //
+ Status = ReadNvmeControllerConfiguration (Private, &Cc);
+ if (EFI_ERROR(Status)) {
+ continue;
+ }
+ //
+ // The host should set the Shutdown Notification (CC.SHN) field to 01b
+ // to indicate a normal shutdown operation.
+ //
+ Cc.Shn = NVME_CC_SHN_NORMAL_SHUTDOWN;
+ Status = WriteNvmeControllerConfiguration (Private, &Cc);
+ if (EFI_ERROR(Status)) {
+ continue;
+ }
+
+ //
+ // The controller indicates when shutdown processing is completed by updating the
+ // Shutdown Status (CSTS.SHST) field to 10b.
+ // Wait up to 45 seconds (break down to 4500 x 10ms) for the shutdown to complete.
+ //
+ for (Index = 0; Index < NVME_SHUTDOWN_PROCESS_TIMEOUT * 100; Index++) {
+ Status = ReadNvmeControllerStatus (Private, &Csts);
+ if (!EFI_ERROR(Status) && (Csts.Shst == NVME_CSTS_SHST_SHUTDOWN_COMPLETED)) {
+ DEBUG((DEBUG_INFO, "NvmeShutdownController: shutdown processing is completed after %dms.\n", Index * 10));
+ break;
+ }
+ //
+ // Stall for 10ms
+ //
+ gBS->Stall (10 * 1000);
+ }
+
+ if (Index == NVME_SHUTDOWN_PROCESS_TIMEOUT * 100) {
+ DEBUG((DEBUG_ERROR, "NvmeShutdownController: shutdown processing is timed out\n"));
+ }
+ }
+ }
+ }
+}
+
+/**
+ Register the shutdown notification through the ResetNotification protocol.
+
+ Register the shutdown notification when mNvmeControllerNumber increased from 0 to 1.
+**/
+VOID
+NvmeRegisterShutdownNotification (
+ VOID
+ )
+{
+ EFI_STATUS Status;
+ EFI_RESET_NOTIFICATION_PROTOCOL *ResetNotify;
+
+ mNvmeControllerNumber++;
+ if (mNvmeControllerNumber == 1) {
+ Status = gBS->LocateProtocol (&gEfiResetNotificationProtocolGuid, NULL, (VOID **) &ResetNotify);
+ if (!EFI_ERROR (Status)) {
+ Status = ResetNotify->RegisterResetNotify (ResetNotify, NvmeShutdownAllControllers);
+ ASSERT_EFI_ERROR (Status);
+ } else {
+ DEBUG ((DEBUG_WARN, "NVME: ResetNotification absent! Shutdown notification cannot be performed!\n"));
+ }
+ }
+}
+
+/**
+ Unregister the shutdown notification through the ResetNotification protocol.
+
+ Unregister the shutdown notification when mNvmeControllerNumber decreased from 1 to 0.
+**/
+VOID
+NvmeUnregisterShutdownNotification (
+ VOID
+ )
+{
+ EFI_STATUS Status;
+ EFI_RESET_NOTIFICATION_PROTOCOL *ResetNotify;
+
+ mNvmeControllerNumber--;
+ if (mNvmeControllerNumber == 0) {
+ Status = gBS->LocateProtocol (&gEfiResetNotificationProtocolGuid, NULL, (VOID **) &ResetNotify);
+ if (!EFI_ERROR (Status)) {
+ Status = ResetNotify->UnregisterResetNotify (ResetNotify, NvmeShutdownAllControllers);
+ ASSERT_EFI_ERROR (Status);
+ }
+ }
+}
--
2.12.2.windows.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] Notify NVME HW when system reset happens
2017-08-07 3:31 [PATCH 0/2] Notify NVME HW when system reset happens Ruiyu Ni
2017-08-07 3:31 ` [PATCH 1/2] MdePkg/Nvme: Add NVME shutdown notification related macros Ruiyu Ni
2017-08-07 3:31 ` [PATCH 2/2] MdeModulePkg/NvmExpressDxe: Notify NVME HW when system reset happens Ruiyu Ni
@ 2017-08-07 7:14 ` Wu, Hao A
2017-08-09 7:56 ` Ni, Ruiyu
2 siblings, 1 reply; 6+ messages in thread
From: Wu, Hao A @ 2017-08-07 7:14 UTC (permalink / raw)
To: Ni, Ruiyu, edk2-devel@lists.01.org
Hi Ray,
Just for curiosity, I checked the NVME spec for the purpose of a shutdown
notification to the controller when there is a power-off for the device. And
found the following:
1. Will increase the field 'Unsafe Shutdowns' in the SMART/Health Information
data (Section 5.10.1.2).
2. Subsequent reads for locations written to the volatile write cache that were
not written to non-volatile storage may return older data. (Section 6.4.2.1)
For 2, the implementation of the NVME driver always sets the Force Unit
Access (FUA) bit for write operations. Hence, the read operation will reflect
the latest data on device.
So missing the shutdown notification will increase the number of 'Unsafe
Shutdowns' tracked by the controller. Is there any other issue being introduced
by missing the notification?
Besides, the change is good for me.
Best Regards,
Hao Wu
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ruiyu
> Ni
> Sent: Monday, August 07, 2017 11:32 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH 0/2] Notify NVME HW when system reset happens
>
> Per NVM Express Spec, software should notify NVME HW when shutdown
> occurs.
>
> The host should set the Shutdown Notification (CC.SHN) field to 01b
> to indicate a normal shutdown operation. The controller indicates
> when shutdown processing is completed by updating the Shutdown Status
> (CSTS.SHST) field to 10b.
>
> Ruiyu Ni (2):
> MdePkg/Nvme: Add NVME shutdown notification related macros
> MdeModulePkg/NvmExpressDxe: Notify NVME HW when system reset
> happens
>
> MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c | 7 +-
> MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.h | 23 ++-
> .../Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf | 3 +-
> MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c | 181
> ++++++++++++++++++++-
> MdePkg/Include/IndustryStandard/Nvme.h | 6 +-
> 5 files changed, 215 insertions(+), 5 deletions(-)
>
> --
> 2.12.2.windows.2
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] Notify NVME HW when system reset happens
2017-08-07 7:14 ` [PATCH 0/2] " Wu, Hao A
@ 2017-08-09 7:56 ` Ni, Ruiyu
2017-08-09 8:01 ` Wu, Hao A
0 siblings, 1 reply; 6+ messages in thread
From: Ni, Ruiyu @ 2017-08-09 7:56 UTC (permalink / raw)
To: Wu, Hao A, edk2-devel@lists.01.org
The NVME controller requires notification for shutdown as part of its management of internal structures. Even with FUA, failing to notify the NVME controller to shutdown power off causes the NVME controller to take quite some time to organize its tables on the next power on. This time exceeds the normal timeout, so we would fail to boot the NVME disk.
I will put above explanation into the commit message as well.
Thanks/Ray
> -----Original Message-----
> From: Wu, Hao A
> Sent: Monday, August 7, 2017 3:14 PM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
> Subject: RE: [edk2] [PATCH 0/2] Notify NVME HW when system reset
> happens
>
> Hi Ray,
>
> Just for curiosity, I checked the NVME spec for the purpose of a shutdown
> notification to the controller when there is a power-off for the device. And
> found the following:
>
> 1. Will increase the field 'Unsafe Shutdowns' in the SMART/Health
> Information data (Section 5.10.1.2).
>
> 2. Subsequent reads for locations written to the volatile write cache that
> were not written to non-volatile storage may return older data. (Section
> 6.4.2.1)
>
> For 2, the implementation of the NVME driver always sets the Force Unit
> Access (FUA) bit for write operations. Hence, the read operation will reflect
> the latest data on device.
>
> So missing the shutdown notification will increase the number of 'Unsafe
> Shutdowns' tracked by the controller. Is there any other issue being
> introduced by missing the notification?
>
> Besides, the change is good for me.
>
>
> Best Regards,
> Hao Wu
>
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Ruiyu Ni
> > Sent: Monday, August 07, 2017 11:32 AM
> > To: edk2-devel@lists.01.org
> > Subject: [edk2] [PATCH 0/2] Notify NVME HW when system reset happens
> >
> > Per NVM Express Spec, software should notify NVME HW when shutdown
> > occurs.
> >
> > The host should set the Shutdown Notification (CC.SHN) field to 01b to
> > indicate a normal shutdown operation. The controller indicates when
> > shutdown processing is completed by updating the Shutdown Status
> > (CSTS.SHST) field to 10b.
> >
> > Ruiyu Ni (2):
> > MdePkg/Nvme: Add NVME shutdown notification related macros
> > MdeModulePkg/NvmExpressDxe: Notify NVME HW when system reset
> happens
> >
> > MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c | 7 +-
> > MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.h | 23 ++-
> > .../Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf | 3 +-
> > MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c | 181
> > ++++++++++++++++++++-
> > MdePkg/Include/IndustryStandard/Nvme.h | 6 +-
> > 5 files changed, 215 insertions(+), 5 deletions(-)
> >
> > --
> > 2.12.2.windows.2
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] Notify NVME HW when system reset happens
2017-08-09 7:56 ` Ni, Ruiyu
@ 2017-08-09 8:01 ` Wu, Hao A
0 siblings, 0 replies; 6+ messages in thread
From: Wu, Hao A @ 2017-08-09 8:01 UTC (permalink / raw)
To: Ni, Ruiyu, edk2-devel@lists.01.org
> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Wednesday, August 09, 2017 3:57 PM
> To: Wu, Hao A; edk2-devel@lists.01.org
> Subject: RE: [edk2] [PATCH 0/2] Notify NVME HW when system reset happens
>
> The NVME controller requires notification for shutdown as part of its
> management of internal structures. Even with FUA, failing to notify the NVME
> controller to shutdown power off causes the NVME controller to take quite
> some time to organize its tables on the next power on. This time exceeds the
> normal timeout, so we would fail to boot the NVME disk.
Got it, thanks for the clarification.
>
> I will put above explanation into the commit message as well.
The series is good to me.
Reviewed-by: Hao Wu <hao.a.wu@intel.com>
Best Regards,
Hao Wu
>
> Thanks/Ray
>
> > -----Original Message-----
> > From: Wu, Hao A
> > Sent: Monday, August 7, 2017 3:14 PM
> > To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
> > Subject: RE: [edk2] [PATCH 0/2] Notify NVME HW when system reset
> > happens
> >
> > Hi Ray,
> >
> > Just for curiosity, I checked the NVME spec for the purpose of a shutdown
> > notification to the controller when there is a power-off for the device. And
> > found the following:
> >
> > 1. Will increase the field 'Unsafe Shutdowns' in the SMART/Health
> > Information data (Section 5.10.1.2).
> >
> > 2. Subsequent reads for locations written to the volatile write cache that
> > were not written to non-volatile storage may return older data. (Section
> > 6.4.2.1)
> >
> > For 2, the implementation of the NVME driver always sets the Force Unit
> > Access (FUA) bit for write operations. Hence, the read operation will reflect
> > the latest data on device.
> >
> > So missing the shutdown notification will increase the number of 'Unsafe
> > Shutdowns' tracked by the controller. Is there any other issue being
> > introduced by missing the notification?
> >
> > Besides, the change is good for me.
> >
> >
> > Best Regards,
> > Hao Wu
> >
> > > -----Original Message-----
> > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > > Ruiyu Ni
> > > Sent: Monday, August 07, 2017 11:32 AM
> > > To: edk2-devel@lists.01.org
> > > Subject: [edk2] [PATCH 0/2] Notify NVME HW when system reset happens
> > >
> > > Per NVM Express Spec, software should notify NVME HW when shutdown
> > > occurs.
> > >
> > > The host should set the Shutdown Notification (CC.SHN) field to 01b to
> > > indicate a normal shutdown operation. The controller indicates when
> > > shutdown processing is completed by updating the Shutdown Status
> > > (CSTS.SHST) field to 10b.
> > >
> > > Ruiyu Ni (2):
> > > MdePkg/Nvme: Add NVME shutdown notification related macros
> > > MdeModulePkg/NvmExpressDxe: Notify NVME HW when system reset
> > happens
> > >
> > > MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c | 7 +-
> > > MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.h | 23 ++-
> > > .../Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf | 3 +-
> > > MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c | 181
> > > ++++++++++++++++++++-
> > > MdePkg/Include/IndustryStandard/Nvme.h | 6 +-
> > > 5 files changed, 215 insertions(+), 5 deletions(-)
> > >
> > > --
> > > 2.12.2.windows.2
> > >
> > > _______________________________________________
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-08-09 7:59 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-07 3:31 [PATCH 0/2] Notify NVME HW when system reset happens Ruiyu Ni
2017-08-07 3:31 ` [PATCH 1/2] MdePkg/Nvme: Add NVME shutdown notification related macros Ruiyu Ni
2017-08-07 3:31 ` [PATCH 2/2] MdeModulePkg/NvmExpressDxe: Notify NVME HW when system reset happens Ruiyu Ni
2017-08-07 7:14 ` [PATCH 0/2] " Wu, Hao A
2017-08-09 7:56 ` Ni, Ruiyu
2017-08-09 8:01 ` 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