From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 03316203564BC for ; Tue, 28 Nov 2017 04:51:19 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 323C86A7C2; Tue, 28 Nov 2017 12:55:42 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-99.rdu2.redhat.com [10.10.120.99]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3A1CF5D753; Tue, 28 Nov 2017 12:55:39 +0000 (UTC) To: "Ni, Ruiyu" Cc: "edk2-devel@lists.01.org" , Paolo Bonzini , "Yao, Jiewen" , "Zeng, Star" References: <20171127011948.279312-1-ruiyu.ni@intel.com> <2625ccb9-e16a-b1cb-e775-580bbeba3f4d@redhat.com> <734D49CCEBEEF84792F5B80ED585239D5BAD41A6@SHSMSX104.ccr.corp.intel.com> From: Laszlo Ersek Message-ID: Date: Tue, 28 Nov 2017 13:55:39 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5BAD41A6@SHSMSX104.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Tue, 28 Nov 2017 12:55:42 +0000 (UTC) Subject: Re: [PATCH 0/2] MdeModulePkg/AtaAtapiPassThru: Revert patch to disable PCI attributes 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, 28 Nov 2017 12:51:20 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit On 11/28/17 08:39, Ni, Ruiyu wrote: > Laszlo, > The Windows resume issue is urgent to fix. > I'd like to check-in the patches first. > If I didn't know the change may cause combability issue, I'd be very fine > to change it to AhciReset() then submit the patch, then after got a R-b, > I'd be very happy to check in that patch. > But since now I know there might be some combability issue, I don't > want to switch to AhciReset() solution without thoroughly testing. > And frankly I don't know what a thoroughly testing would be like. > There are many OSes and many PCI storage cards! ☹ Fair enough; thank you for the explanation. Reviewed-by: Laszlo Ersek Laszlo >> -----Original Message----- >> From: Laszlo Ersek [mailto:lersek@redhat.com] >> Sent: Monday, November 27, 2017 10:10 PM >> To: Ni, Ruiyu >> Cc: edk2-devel@lists.01.org; Paolo Bonzini ; Yao, >> Jiewen ; Zeng, Star >> Subject: Re: [edk2] [PATCH 0/2] MdeModulePkg/AtaAtapiPassThru: Revert >> patch to disable PCI attributes >> >> Hello Ray, >> >> On 11/27/17 02:19, Ruiyu Ni wrote: >>> The patches caused Windows 10 S4 resume failure. >>> Considering the similar changes are reverted from PciBus driver, >>> revert the patches from AtaAtapiPassThru as well. >>> >>> Ruiyu Ni (2): >>> MdeModulePkg/AtaAtapiPassThru: Revert patch to disable Bus Master >>> MdeModulePkg/AtaAtapiPassThru: Revert patch to disable PCI >>> attributes >>> >>> .../Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 58 +-------------------- >> - >>> .../Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h | 5 -- >>> 2 files changed, 1 insertion(+), 62 deletions(-) >>> >> >> it looks like these patches have not been committed yet, which is a good >> thing, because apparently there's a better solution than a full revert. >> Namely, in the other sub-thread at >> > 7fd1ad742c36@redhat.com> >> (alternative link: >> ), >> Jiewen and Star seem to accept AhciReset() as a better way to abort pending >> DMA. >> >> This means that we need not revert the EBS-handler altogether, only change >> what it does. Could you give that a try please? >> >> (If the Windows regression is very urgent to fix, then I don't mind if the Bus >> Master disabling is removed separately, before AhciReset() is added; but in >> that case, a full revert looks unjustified, since the EBS handler will have to be >> re-added for AhciReset() anyway.) >> >> Thanks, >> Laszlo