On Mon, 2 Dec 2024 13:36:25 -0600 Mitchell Augustin <mitchell.augustin@xxxxxxxxxxxxx> wrote: > Thanks! > > This approach makes sense to me - the only concern I have is that I > see this restriction in a comment in __pci_read_base(): > > `/* No printks while decoding is disabled! */` > > At the end of __pci_read_base(), we do have several pci_info() and > pci_err() calls - so I think we would need to also print that info one > level up after the new decode enable if we do decide to move decode > disable/enable one level up. Let me know if you agree, or if there is > a more straightforward alternative that I am missing. Nope, I agree. The console might be the device we've disabled for sizing or might be downstream of that device. The logging would need to be deferred until the device is enabled. Thanks, Alex > On Wed, Nov 27, 2024 at 11:22 AM Alex Williamson > <alex.williamson@xxxxxxxxxx> wrote: > > > > On Tue, 26 Nov 2024 19:12:35 -0600 > > Mitchell Augustin <mitchell.augustin@xxxxxxxxxxxxx> wrote: > > > > > Thanks for the breakdown! > > > > > > > That alone calls __pci_read_base() three separate times, each time > > > > disabling and re-enabling decode on the bridge. [...] So we're > > > > really being bitten that we toggle decode-enable/memory enable > > > > around reading each BAR size > > > > > > That makes sense to me. Is this something that could theoretically be > > > done in a less redundant way, or is there some functional limitation > > > that would prevent that or make it inadvisable? (I'm still new to pci > > > subsystem debugging, so apologies if that's a bit vague.) > > > > The only requirement is that decode should be disabled while sizing > > BARs, the fact that we repeat it around each BAR is, I think, just the > > way the code is structured. It doesn't take into account that toggling > > the command register bit is not a trivial operation in a virtualized > > environment. IMO we should push the command register manipulation up a > > layer so that we only toggle it once per device rather than once per > > BAR. Thanks, > > > > Alex > > > >
