Re: [edk2-devel] [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe: Support PCIe Resizable BAR Capability

["Ni, Ray" <ray.ni@intel.com>]

> > It seems like the max BAR size is selected first, but if there's a
> > "resource conflict" (running out of a particular resource type
> > aperture), then the minimum BAR size is selected. I don't know what
> > set of devices and/or resizable BARs this logic applies to, if there
> > are multiple of them. 

> > Per the PCIe specification (revision 5.0, version 0.9) 7.8.6:
> >
> >   Software determines, through a proprietary mechanism, what the
> >   optimal size is for the resource, and programs that size via the BAR
> >   Size field of the Resizable BAR Control register.
> >
> > Furthermore, Table 7-114 defines the Bar Size field of the control
> > register stating:
> >
> >   The default value of this field is equal to the default size of the
> >   address space that the BAR resource is requesting via the BAR's
> >   read-only bits.
> >
> > Therefore the maximum size is not necessarily optimal, nor should the
> > minimum size be considered the default.  In fact, [we] tested various
> > handoff BAR sizes for [a particular] GPU and found that Windows didn't
> > like the maximum BAR size.
> >
> > Elsewhere in the discussion [1] the AMD author of the kernel support
> > for resizeable BARs indicates that FPGA devices might implement the
> > REBAR capability as part of their standard PCI wrapper ([our]
> > interpretation), but the BAR usage would be determined by the actual
> > bitstream written to the device, therefore there might be a full
> > bitmask for the BAR sizes supported by the device.
> >
> > [1] https://lists.freedesktop.org/archives/dri-devel/2021-January/thread.html
> >
> > It would certainly make sense for the firmware to take REBAR
> > capabilities into account when sizing bridge apertures, but to
> > generically enable extended BAR sizes would make lots of assumptions
> > about the device usage and compatibility.
> >
> > [...] At least for GPUs the expectation would be a default, smaller
> > compatibility size expanding to some representation that allows direct
> > DMA to the entire memory of the card.
> 
> So this patch should either be reverted; or minimally, the default value
> of "PcdPcieResizableBarSupport" should be set to FALSE, as the policy
> for BAR sizing doesn't look robust or portable.
> 
> 
> General request for the future: if you implement some kind of policy in
> core edk2, please at least *document* the policy somewhere. It's
> unacceptable to have to decipher the source code for such a possibly
> impactful change in the core. There is no need for a wiki page or an
> RFC, but a sane bugzilla ticket and a sane commit message are required.
> 
> (The documentation of the PCD in the "MdeModulePkg.dec" file is
> unsatisfactory too, and the UNI file has not been updated at all.)
> 



Your understanding is correct. Original idea is to let platform supply the policy about
what the optimal BAR size is for each resizable BAR.
The current implementation is a try to avoid asking platform code for such policy because
we thought it's a burden for platform to supply the policy data.

I agree that we set the PCD default value as disabled and after a period of study, we will
understand whether a platform policy is really needed.

Thanks,
Ray