Re: [PATCH V4] MdeModulePkg/DxeCore: Fixed Interface returned by CoreOpenProtocol

["Zeng, Star" <star.zeng@intel.com>]

Just added note in https://mantis.uefi.org/mantis/view.php?id=1815, and I will post the patch soon.

Thanks,
Star
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com] 
Sent: Wednesday, June 28, 2017 8:39 PM
To: Zeng, Star <star.zeng@intel.com>; Amit Kumar <amit.ak@samsung.com>; edk2-devel@lists.01.org
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Tian, Feng <feng.tian@intel.com>; Gabriel L. Somlo (GMail) <gsomlo@gmail.com>; Gao, Liming <liming.gao@intel.com>; Fan, Jeff <jeff.fan@intel.com>
Subject: Re: [edk2] [PATCH V4] MdeModulePkg/DxeCore: Fixed Interface returned by CoreOpenProtocol

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
<https://mantis.uefi.org/mantis/view.php?id=1815>, 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