From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:400e:c05::22d; helo=mail-pg0-x22d.google.com; envelope-from=heyi.guo@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-pg0-x22d.google.com (mail-pg0-x22d.google.com [IPv6:2607:f8b0:400e:c05::22d]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 40ABF20356242 for ; Mon, 4 Dec 2017 17:09:06 -0800 (PST) Received: by mail-pg0-x22d.google.com with SMTP id b18so9511310pgv.10 for ; Mon, 04 Dec 2017 17:13:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding:content-language; bh=+xRocRJrp9BYI95WB09FYsZefZdGEJCpbyu+WqFxtXk=; b=TTMpZ3l0FiSqEaDPs9zJqamp6+vWvnP/jCk+M8SDRfT1T4qDnvcNZ9ELdbGCJzBU2V gTFVRp6Bsera4DQumsD0jsfJQs1atnVcoFM78S8YnGA3KXM4rP8rrhyzjnZlFoBNnSag CVE351pyq+0t/aw+xehYyrXdVsZhIbI6gGyf8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding :content-language; bh=+xRocRJrp9BYI95WB09FYsZefZdGEJCpbyu+WqFxtXk=; b=c0XKierDLk/xjj3a48As4wvIOevTltPC5MwDQj4bPeahQoXFYR2+1YpFrNfEClP3YZ ZU0k70RbttrVIwXQkK6LK7rjG+OP6gCSC6JOhzbL9LPbAxSj7JhYrjpaFmzQNSuM8RcN yRZp5x9p0c+uLvs/4PIZ+GvuDgnwRq0cXW8AVDXxWF684bgTdNMjRvNJaOQdWXPFFkbm xooegQ38lqTregKG1g6AZDR+I69KxuU7aBojJ5eVBef2tYXVCOQUjGWY5Uf85lSobNv8 j/oyjwJupGhw9nUjF4hTtFV7Jyc361IL7Fv6MCErYT6pNI7fqsasuCYqCxrAuseo/Hbn Sdcg== X-Gm-Message-State: AJaThX7cNpYMB9nWn+d8ygIFYh4zQRyrB7wRwDM7/VIWlXzWN+t1N/cx C32fP7+nO7nDZfuHiK3nE/n9aQ== X-Google-Smtp-Source: AGs4zMal7XS+a6AsqcdqtkqDBvk7ddicICSsVaHciBodFzHWdsXjnHooeowlnFzL0gNBhRb4dgJP3g== X-Received: by 10.159.252.11 with SMTP id n11mr16658924pls.196.1512436416678; Mon, 04 Dec 2017 17:13:36 -0800 (PST) Received: from [10.191.212.206] ([104.237.91.144]) by smtp.gmail.com with ESMTPSA id e8sm26755544pfk.6.2017.12.04.17.13.34 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 04 Dec 2017 17:13:35 -0800 (PST) To: "Wu, Hao A" , "Zeng, Star" , "linaro-uefi@lists.linaro.org" , "edk2-devel@lists.01.org" Cc: "Dong, Eric" , "Ni, Ruiyu" References: <1512354474-38200-1-git-send-email-heyi.guo@linaro.org> <0C09AFA07DD0434D9E2A0C6AEB0483103B9BF262@shsmsx102.ccr.corp.intel.com> From: Heyi Guo Message-ID: <01a7ad62-29e8-62c2-2394-aeedfedf0669@linaro.org> Date: Tue, 5 Dec 2017 09:13:09 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: Subject: Re: [PATCH v2] MdeModulePkg/NvmExpressDxe: fix error status override X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Dec 2017 01:09:07 -0000 Content-Type: text/plain; charset=gbk; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US Thanks Hao :) ÔÚ 12/5/2017 8:29 AM, Wu, Hao A дµÀ: > Pushed at 9a77210b43ef34af52ea7285fadc0ce5779306fe. > > Best Regards, > Hao Wu > > >> -----Original Message----- >> From: Zeng, Star >> Sent: Monday, December 04, 2017 1:54 PM >> To: Wu, Hao A; Heyi Guo; linaro-uefi@lists.linaro.org; edk2-devel@lists.01.org >> Cc: Dong, Eric; Ni, Ruiyu; Zeng, Star >> Subject: RE: [PATCH v2] MdeModulePkg/NvmExpressDxe: fix error status >> override >> >> Reviewed-by: Star Zeng >> >> Hao, please help push the patch if no other comments received. >> >> >> Thanks, >> Star >> -----Original Message----- >> From: Wu, Hao A >> Sent: Monday, December 4, 2017 10:47 AM >> To: Heyi Guo ; linaro-uefi@lists.linaro.org; edk2- >> devel@lists.01.org >> Cc: Zeng, Star ; Dong, Eric ; Ni, >> Ruiyu >> Subject: RE: [PATCH v2] MdeModulePkg/NvmExpressDxe: fix error status >> override >> >> Reviewed-by: Hao Wu >> >> Best Regards, >> Hao Wu >> >> >>> -----Original Message----- >>> From: Heyi Guo [mailto:heyi.guo@linaro.org] >>> Sent: Monday, December 04, 2017 10:28 AM >>> To: linaro-uefi@lists.linaro.org; edk2-devel@lists.01.org >>> Cc: Heyi Guo; Zeng, Star; Dong, Eric; Wu, Hao A; Ni, Ruiyu >>> Subject: [PATCH v2] MdeModulePkg/NvmExpressDxe: fix error status >>> override >>> >>> Commit f6b139b added return status handling to PciIo->Mem.Write. >>> However, the second status handling will override EFI_DEVICE_ERROR >>> returned in this branch: >>> >>> // >>> // Check the NVMe cmd execution result >>> // >>> if (Status != EFI_TIMEOUT) { >>> if ((Cq->Sct == 0) && (Cq->Sc == 0)) { >>> Status = EFI_SUCCESS; >>> } else { >>> Status = EFI_DEVICE_ERROR; >>> ^^^^^^^^^^^^^^^^ >>> >>> Since PciIo->Mem.Write will probably return SUCCESS, it causes >>> NvmExpressPassThru to return SUCCESS even when DEVICE_ERROR occurs. >>> Callers of NvmExpressPassThru will then continue executing which may >>> cause further unexpected results, e.g. DiscoverAllNamespaces couldn't >>> break out the loop. >>> >>> So we save previous status before calling PciIo->Mem.Write and restore >>> the previous one if it already contains error. >>> >>> Contributed-under: TianoCore Contribution Agreement 1.1 >>> Signed-off-by: Heyi Guo >>> Cc: Star Zeng >>> Cc: Eric Dong >>> Cc: Hao Wu >>> Cc: Ruiyu Ni >>> --- >>> MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c >>> b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c >>> index c33038f..7356c1d 100644 >>> --- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c >>> +++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c >>> @@ -453,6 +453,7 @@ NvmExpressPassThru ( { >>> NVME_CONTROLLER_PRIVATE_DATA *Private; >>> EFI_STATUS Status; >>> + EFI_STATUS PreviousStatus; >>> EFI_PCI_IO_PROTOCOL *PciIo; >>> NVME_SQ *Sq; >>> NVME_CQ *Cq; >>> @@ -831,6 +832,7 @@ NvmExpressPassThru ( >>> } >>> >>> Data = ReadUnaligned32 ((UINT32*)&Private->CqHdbl[QueueId]); >>> + PreviousStatus = Status; >>> Status = PciIo->Mem.Write ( >>> PciIo, >>> EfiPciIoWidthUint32, >>> @@ -839,6 +841,9 @@ NvmExpressPassThru ( >>> 1, >>> &Data >>> ); >>> + // The return status of PciIo->Mem.Write should not override // >>> + previous status if previous status contains error. >>> + Status = EFI_ERROR (PreviousStatus) ? PreviousStatus : Status; >>> >>> // >>> // For now, the code does not support the non-blocking feature for >>> admin queue. >>> -- >>> 2.7.2.windows.1