Re: [edk2] [PATCH] OvmfPkg: prevent 64-bit MMIO BAR degradation if there is no CSM

["Laszlo Ersek" <lersek@redhat.com>]

(updating Ray's email)

On 06/20/19 00:19, David Woodhouse wrote:
> 
>> the driver is thoroughly commented. See especially the
>> DriverInitialize() function. Can you determine which one(s) of those
>> statements doesn't / don't hold any longer?
>>
>> Or maybe IncompatiblePciDeviceSupportDxe works as before, but commit
>> 065ae7d717f9 ("MdeModulePkg/PciBusDxe: make OPROM BAR degradation
>> configurable", 2016-09-26) made a difference? (Adding Ard.)
>>
>> I'm just guessing of course; a bisection could prove more effective.
> 
> I think I worked it out. The problem is that the nvme controller doesn't
> have a ROM so it wasn't triggering the downgrade to 32-bit in the first
> place.
> 
> By hacking IncompatiblePciDeviceSupportDxe to always return configuration
> with 32+bit "granularity" I can boot. That does it for *all* devices, of
> course... but I don't get the PCI class; only device/vendor IDs.

Doesn't this imply that we have a more generic problem in PciBusDxe?

AIUI, the current aperture degradation (for BAR allocation purposes) is
meant to allow the device's own (specific) legacy option ROM to access
the BARs. If the device has no option ROM, this particular goal falls
away. (And OVMF's driver basically implements an override, in case the
oprom does exist, but "legacy" is whole-sale unsupported by the platform.)

If the generic (device independent) CSM code -- *any* generic CSM in
fact -- is expected to access BARs, then the logic in PciBusDxe is both
too lax and too restrictive at the same time. It's too lax due to
factors discussed previously (see the paragraph above, and commit
065ae7d717f9), and too restrictive because it misses the "CSM per se
needs BAR access" case.

For working this around in OVMF, we'd have to change the OVMF driver's
behavior from "force 64-bit, or keep silent", to "force 64-bit, or force
32-bit". This looks like a kludge that entirely supplants the PciBusDxe
logic. So I'm not a fan of it. That said, if you can patch
IncompatiblePciDeviceSupportDxe such that the change only affect the
CSM_ENABLE build of OVMF observably, I guess I can live with it.

Thanks
Laszlo