From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 845A42095D225 for ; Tue, 27 Jun 2017 02:43:24 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3D2D1C04B938; Tue, 27 Jun 2017 09:44:54 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 3D2D1C04B938 Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=lersek@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 3D2D1C04B938 Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-38.phx2.redhat.com [10.3.116.38]) by smtp.corp.redhat.com (Postfix) with ESMTP id 78B9D5C544; Tue, 27 Jun 2017 09:44:52 +0000 (UTC) To: "Zeng, Star" , Amit Kumar , "edk2-devel@lists.01.org" Cc: "Tian, Feng" , "Gao, Liming" , "Kinney, Michael D" , "Gabriel L. Somlo (GMail)" , "Fan, Jeff" References: <0C09AFA07DD0434D9E2A0C6AEB0483103B8ED8A7@shsmsx102.ccr.corp.intel.com> <0C09AFA07DD0434D9E2A0C6AEB0483103B8ED94C@shsmsx102.ccr.corp.intel.com> From: Laszlo Ersek Message-ID: Date: Tue, 27 Jun 2017 11:44:51 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <0C09AFA07DD0434D9E2A0C6AEB0483103B8ED94C@shsmsx102.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Tue, 27 Jun 2017 09:44:54 +0000 (UTC) Subject: Re: [PATCH V4] MdeModulePkg/DxeCore: Fixed Interface returned by CoreOpenProtocol 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, 27 Jun 2017 09:43:24 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Hi Star, On 06/27/17 03:37, Zeng, Star wrote: > I have reverted this patch at fd220166c435479a81dfe8519c92abec9acf7d82 first. Thanks for that. More comments below: > > > Thanks, > Star > -----Original Message----- > From: Zeng, Star > Sent: Tuesday, June 27, 2017 8:53 AM > To: Laszlo Ersek ; Amit Kumar ; edk2-devel@lists.01.org > Cc: Tian, Feng ; Gao, Liming ; Kinney, Michael D ; Gabriel L. Somlo (GMail) ; Fan, Jeff ; Zeng, Star > Subject: RE: [edk2] [PATCH V4] MdeModulePkg/DxeCore: Fixed Interface returned by CoreOpenProtocol > > Laszlo, > > I agree to revert this patch first since it breaks something. > Could you help do that with detailed revert log? > > Anyway, have you found any bug in this patch itself? No, as I said, I think the patch has the right goal (it improves compliance with the UEFI specification). And, while I didn't personally review the patch, I trust your and Liming's review on it. The problem is not with the patch per se. The problem is that the patch triggers an existing bug in many other drivers. So the actual bugs are in those drivers that incorrectly expect OpenProtocol() to set Interface even when OpenProtocol() returns EFI_ALREADY_STARTED. The end result is that all these drivers should be fixed first (with separate patches), before fixing OpenProtocol() itself. I'll try to work a bit more on those drivers and discover how many there are of them. Obviously I can't audit *all* drivers in the edk2 tree, just those that break the OVMF boot process. In fact, the newest idea I have is the following: I think I'll introduce an OpenProtocol() wrapper function to UefiLib. This function should work identically to OpenProtocol(), except when OpenProtocol() returns EFI_ALREADY_STARTED. In that case, the wrapper function will repeat the exact same OpenProtocol() call just with GET_PROTOCOL attribute, and set Interface on output. This will restore and legitimize the OpenProtocol() behavior expected by all those problematic drivers, without having to do intrusive error handling in them. Thanks, Laszlo