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

[Leif Lindholm <leif.lindholm@linaro.org>]

Adding Andrew, Mike,

On Tue, Sep 13, 2016 at 08:43:39AM +0100, Ard Biesheuvel wrote:
> On 13 September 2016 at 06:46, Ni, Ruiyu <ruiyu.ni@intel.com> wrote:
> >>* 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

Yes, the situation of requiring an implementation of
PciIncompatibleDevice to support an actually compatible device, in
order to not require it for actually incompatible platforms seems at
the very least somewhat semantically dubious to me.

By order of my preference, I would say we could:
- Invert the use of PciIncompatibleDevice with a default
  implementation for IntelFrameworkModulePkg
- #ifdef MDE_CPU_X64
- Add a dynamic Pcd
- Retain the semantically incorrect use of PciIncompatibleDevice, but
  provide a single implementation reusable across OvmfPkg and all !X64
  platforms.

Regards,

Leif