Re: [PATCH] MdeModulePkg/PciBusDxe: make BAR degradation dependent on OPROM presence

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 13 September 2016 at 06:46, Ni, Ruiyu <ruiyu.ni@intel.com> wrote:
>
>
> Regards,
> Ray
>
>>-----Original Message-----
>>From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek
>>Sent: Monday, September 12, 2016 9:49 PM
>>To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Yao, Jiewen <jiewen.yao@intel.com>
>>Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org <edk2-devel@ml01.01.org>;
>>leif.lindholm@linaro.org; Gao, Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
>>Subject: Re: [edk2] [PATCH] MdeModulePkg/PciBusDxe: make BAR degradation dependent on OPROM presence
>>
>>On 09/12/16 15:16, Ard Biesheuvel wrote:
>>> On 12 September 2016 at 14:15, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>>>> HI Ard
>>>> We should not let MdeModulePkg depend on IntelFrameworkPkg.
>>>> You patch violates the dependency rule.
>>>> I suggest we figure out other solution to resolve the problem.
>>>>
>>>
>>> Yes, please. And please keep us informed about any solutions you come up with.
>>
>>* One idea is to parse the PCI expansion ROM in order to see what image
>>types are contained within. If there is no (Code type == 0x00) image in
>>the oprom, then the oprom is useless for legacy boot anyway, so it
>>shouldn't trigger degradation.
>>
>>Unfortunately, this wouldn't help a lot in practice, since it's surely
>>going to be years before hw vendors migrate to pure UEFI oproms on their
>>graphics and network cards. :(
> Yes the first idea doesn't work because it cannot solve all the problems: some
> cards may still contain legacy option ROM. The resource degrade happens
> before PciBus knows which option rom to run by platform. And we even met case
> the card only contains legacy option rom but platform just don't want to dispatch
> the legacy rom.
>
>>* Another idea is to check a dynamic PCD that the platform can set. New
>>PCDs are frowned upon in MdeModulePkg however, so I don't expect this to
>>be a popular fix.
>
> I agree with you.
> The requirement of dynamic PCD actually indicates we have a missing interface
> between platform and PciBus core driver.
> I personally don't like dynamic PCD very much. It's equivalent to protocol. Then
> why should we use PCD instead of protocol? To avoid changing spec??
>
> The usage of PciIncompatibleDevice protocol is the solution we came up.
> It's also the currently recommended way to solve this type of issue.
> ECR 1529 introduced in PI 1.4A was initiated by this issue.
>

We are all aware that the PciIncompatibleDevice protocol can be used
to work around this. But this means that, while this issue only exists
on X64, all architectures need to install additional protocols and
rely on runtime processing to disable a feature they did not want in
the first place. Is the practice of degrading 64-bit MMIO BARs for
option ROMs even in the specs?

So how about a PCD feature flag? Or even simply a '#ifdef MDE_CPU_X64'
around the block that degrades the 64-bit MMIO BARs if an option ROM
is detected? Ideally, we should implement PciIncompatibleDevice
protocol the other way around, and add a default implementation to
IntelFrameworkModulePkg to performs the BAR degradation on platforms
with a legacy BIOS and option ROMs

-- 
Ard.