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

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

I also prefer to document it in UEFI spec personally.
And we are also having more discussion about it internally, nice to share more after that. :)


Thanks,
Star
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com] 
Sent: Tuesday, June 27, 2017 9:23 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/27/17 13:15, Laszlo Ersek wrote:
> On 06/27/17 11:59, Zeng, Star wrote:
>> Laszlo,
>>
>> I got the point and I like the idea of wrapper function personally.
>> Although we can clean up the UEFI drivers in EDK2 tree, but it is really hard to evaluate the UEFI drivers in OPROM on add-on card.
>> The patch did not just break OVMF, but also broke all real platforms we have in hand (it's my fault that I did not catch the failure when reviewing patch), the patch is so risky.
> 
> Can we perhaps add a FeaturePCD (default value: FALSE) and rework 
> Amit's patch to consider the PCD? I really like the idea of being true 
> to the UEFI spec.
> 
> In the edk2 tree we could rebase the affected drivers to the wrapper 
> function. And in OVMF (and in other in-tree emulation platforms), we 
> could set the FeaturePCD to TRUE.
> 
> It is possible to use OVMF with assigned physical PCI devices, but 
> people can easily rebuild OVMF themselves, with the FeaturePCD re-set 
> to FALSE. Users can also quickly report the exact cards / oproms that 
> break with the FeaturePCD set to TRUE.
> 
> 
> If you think it's simply not worth the effort, I'm OK with that, but 
> then we should specify this behavior in the UEFI spec -- we'll need a 
> Mantis bug for that. IMO it's not great that so many UEFI_DRIVERs in 
> the wild depend on behavior that is expressly forbidden by the UEFI 
> spec at the moment. If we can't modify all those drivers, then we 
> should adapt the spec, so that it reflects reality.
> 
> I'm not going to suggest specific language for the UEFI spec right now.
> But the wording could be based on the documentation of the wrapper 
> function that I'm working on now. I think I might post the patches 
> just for illustration.

OK, I'm giving up. It is too hard to track down every single crash -- the register dump written by UefiCpuPkg's exception handler to the serial port is not much help, even though it names a module to look at.

So I guess we have to accept that the dependency on EFI_ALREADY_STARTED setting Interface is just too wide-spread. :/ As I wrote above, I think this should be documented in the UEFI spec.

I filed the following mantis ticket:

  Permit OpenProtocol() to output the supported protocol interface even
  on error
  https://mantis.uefi.org/mantis/view.php?id=1815

Thanks
Laszlo