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 9508B2095D21E for ; Wed, 28 Jun 2017 05:37:39 -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 7D9867CE0A; Wed, 28 Jun 2017 12:39:10 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 7D9867CE0A Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=lersek@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 7D9867CE0A Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-57.phx2.redhat.com [10.3.116.57]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5C52471D7B; Wed, 28 Jun 2017 12:39:08 +0000 (UTC) To: "Zeng, Star" , Amit Kumar , "edk2-devel@lists.01.org" Cc: "Kinney, Michael D" , "Tian, Feng" , "Gabriel L. Somlo (GMail)" , "Gao, Liming" , "Fan, Jeff" References: <0C09AFA07DD0434D9E2A0C6AEB0483103B8ED8A7@shsmsx102.ccr.corp.intel.com> <0C09AFA07DD0434D9E2A0C6AEB0483103B8ED94C@shsmsx102.ccr.corp.intel.com> <0C09AFA07DD0434D9E2A0C6AEB0483103B8EEED6@shsmsx102.ccr.corp.intel.com> <0C09AFA07DD0434D9E2A0C6AEB0483103B8EEF75@shsmsx102.ccr.corp.intel.com> <0C09AFA07DD0434D9E2A0C6AEB0483103B8EF64C@shsmsx102.ccr.corp.intel.com> From: Laszlo Ersek Message-ID: <04a41a83-905e-ba8d-d476-183b7a70e572@redhat.com> Date: Wed, 28 Jun 2017 14:39:07 +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: <0C09AFA07DD0434D9E2A0C6AEB0483103B8EF64C@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.28]); Wed, 28 Jun 2017 12:39:10 +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: Wed, 28 Jun 2017 12:37:39 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 06/28/17 11:34, Zeng, Star wrote: > Laszlo, > > Summary the return status codes for OpenProtocol() as below. > EFI_SUCCESS > Return the protocol interface in *Interface if Attributes is not EFI_OPEN_PROTOCOL_TEST_PROTOCOL. > EFI_INVALID_PARAMETER > Do not update *Interface because one of the parameters has a value that does not allow this API to function correctly. > EFI_UNSUPPORTED > Return NULL in *Interface because UserHandle does not support Protocol if Attributes is not EFI_OPEN_PROTOCOL_TEST_PROTOCOL. > EFI_ACCESS_DENIED > Do not update *Interface because the protocol is already being used exclusively be someone else. Returning the *Interface just makes it easy for the caller to use an interface they are not allowed to use. > EFI_ALREADY_STARTED > Return the protocol interface in *Interface. This allows bus drivers to use the interface they are already managing. > > How about we clarify UEFI spec to only return the corresponding protocol interface in *Interface if the return status is EFI_SUCCESS or EFI_ALREADY_STARTED. > Then it preserves the exiting behavior for EFI_ALREADY_STARTED, but also updates OpenProtocol() to not modify Interface output parameter for all other error conditions except EFI_UNSUPPORTED (NULL will be returned in *Interface if Attributes is not EFI_OPEN_PROTOCOL_TEST_PROTOCOL). > > UEFI Spec (Current language) > ========================== > There are a number of reasons that this function call can return an error. If an error is returned, then AgentHandle, ControllerHandle, and Attributes are not added to the list of agents consuming the protocol interface specified by Handle and Protocol, and Interface is returned unmodified. The following is the list of conditions that must be checked before this function can return EFI_SUCCESS. > > UEFI Spec (Proposed new language) > ========================== > There are a number of reasons that this function call can return an error. If an error is returned, then AgentHandle, ControllerHandle, and Attributes are not added to the list of agents consuming the protocol interface specified by Handle and Protocol. Interface is returned unmodified for all error conditions except EFI_UNSUPPORTED and EFI_ALREADY_STARTED, NULL will be returned in *Interface when EFI_UNSUPPORTED and Attributes is not EFI_OPEN_PROTOCOL_TEST_PROTOCOL, the protocol interface will be returned in *Interface when EFI_ALREADY_STARTED. The following is the list of conditions that must be checked before this function can return EFI_SUCCESS. This looks great to me! (EFI_ALREADY_STARTED is only possible for BY_DRIVER and BY_DRIVER|EXCLUSIVE, so TEST_PROTOCOL need not be considered for EFI_ALREADY_STARTED. Good!) Can you please add this language to , in a new note? > Of course, the code still needs to be updated and the revised patch based on this V4 patch is attached. Please post the patch to the list and let the respective owners / maintainers review it :) If it matches the above specification, then it should be good. Thanks! Laszlo