Re: [PATCH v1 2/3] MdeModulePkg/NvmExpressDxe: Always copy CQ entry to PassThru packet

["Ni, Ruiyu" <ruiyu.ni@Intel.com>]

On 10/18/2018 2:41 PM, Hao Wu wrote:
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1259
> 
> According to the the NVM Express spec Revision 1.1, for some commands,
> command-related information will be stored in the Dword 0 of the
> completion queue entry.
> 
> One case is for the Get Features Command (Section 5.9.2 of the spec),
> Dword 0 of the completion queue entry may contain feature information.
> 
> Hence, this commit will always copy the content of completion queue entry
> to the PassThru packet regardless of the execution result of the command.
> 
> Cc: Liangcheng Tang <liangcheng.tang@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
>   MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 9 ++++-----
>   1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> index bfcd349794..c52e960771 100644
> --- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> +++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> @@ -781,17 +781,16 @@ NvmExpressPassThru (
>       } else {
>         Status = EFI_DEVICE_ERROR;
>         //
> -      // Copy the Respose Queue entry for this command to the callers response buffer
> -      //
> -      CopyMem(Packet->NvmeCompletion, Cq, sizeof(EFI_NVM_EXPRESS_COMPLETION));
> -
> -      //
>         // Dump every completion entry status for debugging.
>         //
>         DEBUG_CODE_BEGIN();
>           NvmeDumpStatus(Cq);
>         DEBUG_CODE_END();
>       }
> +    //
> +    // Copy the Respose Queue entry for this command to the callers response buffer
> +    //
> +    CopyMem(Packet->NvmeCompletion, Cq, sizeof(EFI_NVM_EXPRESS_COMPLETION));
>     } else {
>       //
>       // Timeout occurs for an NVMe command. Reset the controller to abort the
> 
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>

-- 
Thanks,
Ray