Re: [edk2] [PATCH v2 3/3] MdeModulePkg: Add NvmExpressLib.
Ok, I am not aware that you will contribute Firmware Management Protocol to EDKII. So this library will be used by EDKII NvmExpress driver. PS: We will check in NvmExpress non-blocking support to the repo soon. Hope it will not bring big merge issue to your patch. Thanks Feng -Original Message- From: El-Haj-Mahmoud, Samer [mailto:samer.el-haj-mahm...@hpe.com] Sent: Wednesday, May 18, 2016 9:51 PM To: Tian, Feng ; Reyes, Darbin ; edk2-devel@lists.01.org Cc: Zeng, Star ; Kinney, Michael D ; Gao, Liming ; El-Haj-Mahmoud, Samer Subject: RE: [PATCH v2 3/3] MdeModulePkg: Add NvmExpressLib. Feng, We plan on submitting updates to NvmExpressDxe to implement Firmware Management Protocol, which will use this library. Also part of this patch is to change existing NvmExpressDxe code (partially) to start using the shared library. We also have proprietary code (in non EDK2 drivers) that need this library to access some of the NVMe disk inventory information. Thanks, --Samer -Original Message- From: Tian, Feng [mailto:feng.t...@intel.com] Sent: Wednesday, May 18, 2016 3:50 AM To: Reyes, Darbin ; edk2-devel@lists.01.org Cc: Zeng, Star ; Kinney, Michael D ; Gao, Liming ; El-Haj-Mahmoud, Samer ; Tian, Feng Subject: RE: [PATCH v2 3/3] MdeModulePkg: Add NvmExpressLib. Hi, Darbin We have discussed this library internally. We would like to know the usage model of this library, that's who will use/consume this lib? Currently there is no EDKII module using it. And the intention of a new library is for code sharing/reuse. Besides that, here are some comments on the library: 1. Please don't use a common name as function name. usually we add a "NvmExpress" prefix prior to such function name. 2. Why you include UefiLib.h at NvmExpressLib.h? it should be redundant. 3. Identify() looks like a little overlap with IdentifyController(). GetLog() is same case with other GetLog variant. Do we need all of them? 4. Suggest you to follow the style of UefiScsiLib, then we can put it to MdePkg. Such as the library instance name is UefiNvmExpressLib rather than DxeNvmExpressLib. Such as you can fill INF like this LIBRARY_CLASS = UefiNvmExpressLib |DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER Thanks Feng -Original Message- From: darbin.emm.re...@hpe.com [mailto:darbin.emm.re...@hpe.com] Sent: Wednesday, May 18, 2016 5:59 AM To: edk2-devel@lists.01.org Cc: Tian, Feng ; Zeng, Star ; Kinney, Michael D ; Gao, Liming ; samer.el-haj-mahm...@hpe.com; Darbin Reyes Subject: [PATCH v2 3/3] MdeModulePkg: Add NvmExpressLib. From: Darbin Reyes Adds a library for functions that facilitate the execution of NVMe commands using the passthru protocol. These functions are intended for use with NVMe controllers that are compliant with version 1.1 of the NVMe spec. The initial implementation of the library contains functions for: 1. Executing a Get Log command. 2. Executing an Identify command. 3. Executing a firmware download and activate command. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Darbin Reyes Reviewed-by: Samer El-Haj-Mahmoud --- MdeModulePkg/Include/Library/NvmExpressLib.h | 279 + .../Library/DxeNvmExpressLib/DxeNvmExpressLib.c| 635 + .../Library/DxeNvmExpressLib/DxeNvmExpressLib.inf | 41 ++ MdeModulePkg/MdeModulePkg.dec | 4 + 4 files changed, 959 insertions(+) create mode 100644 MdeModulePkg/Include/Library/NvmExpressLib.h create mode 100644 MdeModulePkg/Library/DxeNvmExpressLib/DxeNvmExpressLib.c create mode 100644 MdeModulePkg/Library/DxeNvmExpressLib/DxeNvmExpressLib.inf diff --git a/MdeModulePkg/Include/Library/NvmExpressLib.h b/MdeModulePkg/Include/Library/NvmExpressLib.h new file mode 100644 index 000..d1e06a9 --- /dev/null +++ b/MdeModulePkg/Include/Library/NvmExpressLib.h @@ -0,0 +1,279 @@ +/** @file + Functions for sending NVMe commands to an NVMe controller that +supports + the NVMe Specification version 1.1. + + (C) Copyright 2016 Hewlett Packard Enterprise Development LP 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 http://opensource.org/licenses/bsd-license.php + + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. + + @par Specification Reference: + NVMe Specification 1.1 + +**/ + +#ifndef __NVM_E_LIB__ +#define __NVM_E_LIB__ + + + +#include + +#include + +#include + +// +// Time out value for Nvme transaction execution // +#define NVME_GENERIC_TIMEOUT EFI_TIMER_PERIOD_SECONDS (5) + +/** + + Executes a Get Log Page command. + (ref. spec. v1.1 5.10). + + Get Log Page – Command Specific Status Values (ref. spec.
Re: [edk2] [PATCH v2 3/3] MdeModulePkg: Add NvmExpressLib.
Feng, We plan on submitting updates to NvmExpressDxe to implement Firmware Management Protocol, which will use this library. Also part of this patch is to change existing NvmExpressDxe code (partially) to start using the shared library. We also have proprietary code (in non EDK2 drivers) that need this library to access some of the NVMe disk inventory information. Thanks, --Samer -Original Message- From: Tian, Feng [mailto:feng.t...@intel.com] Sent: Wednesday, May 18, 2016 3:50 AM To: Reyes, Darbin ; edk2-devel@lists.01.org Cc: Zeng, Star ; Kinney, Michael D ; Gao, Liming ; El-Haj-Mahmoud, Samer ; Tian, Feng Subject: RE: [PATCH v2 3/3] MdeModulePkg: Add NvmExpressLib. Hi, Darbin We have discussed this library internally. We would like to know the usage model of this library, that's who will use/consume this lib? Currently there is no EDKII module using it. And the intention of a new library is for code sharing/reuse. Besides that, here are some comments on the library: 1. Please don't use a common name as function name. usually we add a "NvmExpress" prefix prior to such function name. 2. Why you include UefiLib.h at NvmExpressLib.h? it should be redundant. 3. Identify() looks like a little overlap with IdentifyController(). GetLog() is same case with other GetLog variant. Do we need all of them? 4. Suggest you to follow the style of UefiScsiLib, then we can put it to MdePkg. Such as the library instance name is UefiNvmExpressLib rather than DxeNvmExpressLib. Such as you can fill INF like this LIBRARY_CLASS = UefiNvmExpressLib |DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER Thanks Feng -Original Message- From: darbin.emm.re...@hpe.com [mailto:darbin.emm.re...@hpe.com] Sent: Wednesday, May 18, 2016 5:59 AM To: edk2-devel@lists.01.org Cc: Tian, Feng ; Zeng, Star ; Kinney, Michael D ; Gao, Liming ; samer.el-haj-mahm...@hpe.com; Darbin Reyes Subject: [PATCH v2 3/3] MdeModulePkg: Add NvmExpressLib. From: Darbin Reyes Adds a library for functions that facilitate the execution of NVMe commands using the passthru protocol. These functions are intended for use with NVMe controllers that are compliant with version 1.1 of the NVMe spec. The initial implementation of the library contains functions for: 1. Executing a Get Log command. 2. Executing an Identify command. 3. Executing a firmware download and activate command. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Darbin Reyes Reviewed-by: Samer El-Haj-Mahmoud --- MdeModulePkg/Include/Library/NvmExpressLib.h | 279 + .../Library/DxeNvmExpressLib/DxeNvmExpressLib.c| 635 + .../Library/DxeNvmExpressLib/DxeNvmExpressLib.inf | 41 ++ MdeModulePkg/MdeModulePkg.dec | 4 + 4 files changed, 959 insertions(+) create mode 100644 MdeModulePkg/Include/Library/NvmExpressLib.h create mode 100644 MdeModulePkg/Library/DxeNvmExpressLib/DxeNvmExpressLib.c create mode 100644 MdeModulePkg/Library/DxeNvmExpressLib/DxeNvmExpressLib.inf diff --git a/MdeModulePkg/Include/Library/NvmExpressLib.h b/MdeModulePkg/Include/Library/NvmExpressLib.h new file mode 100644 index 000..d1e06a9 --- /dev/null +++ b/MdeModulePkg/Include/Library/NvmExpressLib.h @@ -0,0 +1,279 @@ +/** @file + Functions for sending NVMe commands to an NVMe controller that +supports + the NVMe Specification version 1.1. + + (C) Copyright 2016 Hewlett Packard Enterprise Development LP 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 http://opensource.org/licenses/bsd-license.php + + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. + + @par Specification Reference: + NVMe Specification 1.1 + +**/ + +#ifndef __NVM_E_LIB__ +#define __NVM_E_LIB__ + + + +#include + +#include + +#include + +// +// Time out value for Nvme transaction execution // +#define NVME_GENERIC_TIMEOUT EFI_TIMER_PERIOD_SECONDS (5) + +/** + + Executes a Get Log Page command. + (ref. spec. v1.1 5.10). + + Get Log Page – Command Specific Status Values (ref. spec. v1.1 + figure 79). + + @param[in] NVMePassThruProtocol Pointer to an instance of the NVMe pass thru protocol. + @param[in] LogId Log Page Identifier. + @param[in] LogSize Size in bytes of LogBuffer. + @param[out] LogBuffer Pointer to buffer in which to return the log. + @param[out] NvmeCompletionPointer to an NVMe completion struct. + + @retval EFI_SUCCESSThe NVM Express Command Packet was sent by the host. + @retval EFI_BAD_BUFFER_SIZEThe NVM Express Command Packet was not executed. + @retval EFI
Re: [edk2] [PATCH v2 3/3] MdeModulePkg: Add NvmExpressLib.
Hi, Darbin We have discussed this library internally. We would like to know the usage model of this library, that's who will use/consume this lib? Currently there is no EDKII module using it. And the intention of a new library is for code sharing/reuse. Besides that, here are some comments on the library: 1. Please don't use a common name as function name. usually we add a "NvmExpress" prefix prior to such function name. 2. Why you include UefiLib.h at NvmExpressLib.h? it should be redundant. 3. Identify() looks like a little overlap with IdentifyController(). GetLog() is same case with other GetLog variant. Do we need all of them? 4. Suggest you to follow the style of UefiScsiLib, then we can put it to MdePkg. Such as the library instance name is UefiNvmExpressLib rather than DxeNvmExpressLib. Such as you can fill INF like this LIBRARY_CLASS = UefiNvmExpressLib |DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER Thanks Feng -Original Message- From: darbin.emm.re...@hpe.com [mailto:darbin.emm.re...@hpe.com] Sent: Wednesday, May 18, 2016 5:59 AM To: edk2-devel@lists.01.org Cc: Tian, Feng ; Zeng, Star ; Kinney, Michael D ; Gao, Liming ; samer.el-haj-mahm...@hpe.com; Darbin Reyes Subject: [PATCH v2 3/3] MdeModulePkg: Add NvmExpressLib. From: Darbin Reyes Adds a library for functions that facilitate the execution of NVMe commands using the passthru protocol. These functions are intended for use with NVMe controllers that are compliant with version 1.1 of the NVMe spec. The initial implementation of the library contains functions for: 1. Executing a Get Log command. 2. Executing an Identify command. 3. Executing a firmware download and activate command. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Darbin Reyes Reviewed-by: Samer El-Haj-Mahmoud --- MdeModulePkg/Include/Library/NvmExpressLib.h | 279 + .../Library/DxeNvmExpressLib/DxeNvmExpressLib.c| 635 + .../Library/DxeNvmExpressLib/DxeNvmExpressLib.inf | 41 ++ MdeModulePkg/MdeModulePkg.dec | 4 + 4 files changed, 959 insertions(+) create mode 100644 MdeModulePkg/Include/Library/NvmExpressLib.h create mode 100644 MdeModulePkg/Library/DxeNvmExpressLib/DxeNvmExpressLib.c create mode 100644 MdeModulePkg/Library/DxeNvmExpressLib/DxeNvmExpressLib.inf diff --git a/MdeModulePkg/Include/Library/NvmExpressLib.h b/MdeModulePkg/Include/Library/NvmExpressLib.h new file mode 100644 index 000..d1e06a9 --- /dev/null +++ b/MdeModulePkg/Include/Library/NvmExpressLib.h @@ -0,0 +1,279 @@ +/** @file + Functions for sending NVMe commands to an NVMe controller that +supports + the NVMe Specification version 1.1. + + (C) Copyright 2016 Hewlett Packard Enterprise Development LP + 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 http://opensource.org/licenses/bsd-license.php + + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. + + @par Specification Reference: + NVMe Specification 1.1 + +**/ + +#ifndef __NVM_E_LIB__ +#define __NVM_E_LIB__ + + + +#include + +#include + +#include + +// +// Time out value for Nvme transaction execution // +#define NVME_GENERIC_TIMEOUT EFI_TIMER_PERIOD_SECONDS (5) + +/** + + Executes a Get Log Page command. + (ref. spec. v1.1 5.10). + + Get Log Page – Command Specific Status Values (ref. spec. v1.1 + figure 79). + + @param[in] NVMePassThruProtocol Pointer to an instance of the NVMe pass thru protocol. + @param[in] LogId Log Page Identifier. + @param[in] LogSize Size in bytes of LogBuffer. + @param[out] LogBuffer Pointer to buffer in which to return the log. + @param[out] NvmeCompletionPointer to an NVMe completion struct. + + @retval EFI_SUCCESSThe NVM Express Command Packet was sent by the host. + @retval EFI_BAD_BUFFER_SIZEThe NVM Express Command Packet was not executed. + @retval EFI_NOT_READY The NVM Express Command Packet could not be sent because the controller is not ready. The caller + may retry again later. + @retval EFI_DEVICE_ERROR A device error occurred while attempting to send the NVM Express Command Packet. + @retval EFI_INVALID_PARAMETER The contents of the command are invalid. The NVM Express Command Packet was not sent, + so no additional status information is available. NVMePassThruProtocol, LogBuffer, or NvmeCompletion + arguments are NULL. + @retval EFI_UNSUPPORTEDThe command described by the NVM E