On 06/23/17 12:09, Amit Kumar wrote:
> Change since v3:
> 1) Fixed issue when Attributes = EFI_OPEN_PROTOCOL_TEST_PROTOCOL
> and Inteface = NULL case. [Reported by:star.zeng at intel.com]
> 
> Change Since v2:
> 1) Modified to use EFI_ERROR to get status code
> 
> Change since v1:
> 1) Fixed typo protocal to protocol
> 2) Fixed coding style
> 
> Modified source code to update Interface as per spec.
> 1) In case of Protocol is un-supported, interface should be returned NULL.
> 2) In case of any error, interface should not be modified.
> 3) In case of Test Protocol, interface is optional.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Amit Kumar <amit.ak@samsung.com>
> ---
>  MdeModulePkg/Core/Dxe/Hand/Handle.c | 26 ++++++++++++++------------
>  1 file changed, 14 insertions(+), 12 deletions(-)

This patch breaks *at least* the following drivers:
- IntelFrameworkModulePkg/.../IsaBusDxe
- IntelFrameworkModulePkg/.../IsaSerialDxe
- MdeModulePkg/.../TerminalDxe
- MdeModulePkg/.../ScsiBusDxe

I have patches for the first three. I'm too tired to complete the patch
for the fourth module (and any modules that might still be affected, but
I'd only see those in the OVMF boot process if I got past the ScsiBusDxe
problem.)

The issue was reported by Gabriel here:

https://lists.01.org/pipermail/edk2-devel/2017-June/011886.html

although he didn't get past of the first driver, of course.

Now, what this patch does is valid, as far as the UEFI spec is
concerned. And the above (now broken) drivers are all incorrect to
assume that EFI_ALREADY_STARTED guarantees that OpenProtocol() has
filled in Interface on output.

However, such an intrusive fix must be accompanied by extensive testing;
preferably using one of the in-tree emulation platforms, such as OvmfPkg
running on QEMU. And, when testing uncovers the failures in those
drivers, patches should be submitted to fix those drivers *first*, and
then the patch for OpenProtocol() should be presented *last*.

I'm attaching the first three patches I have now. As I said I'm too
tired to work on ScsiBusDxe (the SCSIBusDriverBindingStart() function
has the same bug around EFI_ALREADY_STARTED). Which is why I'm not
posting the first three patches normally, with git-send-email; the work
is incomplete.

Honestly, I'm thinking that commit 45cfcd8dccf8 should be reverted now,
then all the drivers used by OvmfPkg should be fixed up (not by me,
obviously!), and once OVMF boots again, we can re-commit (= cherry-pick)
commit 45cfcd8dccf8.

Thanks
Laszlo