Patch Set 9:

coreboot's current resource allocation does not work at all for modern
hardware. This is not just advanced server platforms, but also mobile platforms
will be affected soon.

And it is exactly those platforms where I would expect that power-management related hot-plug events will require firmware to fill/assign properties to PCIe rootports or bridges in ACPI tables. Can you give me a definitive answer that is not the case or that you have already considered that and have some solution in mind?

I don't remember Ron committing significant work to PCI subsystem or devicetree for quite some time, so I don't know if is aware how devicetree interconnects with ACPI since 2014 or so.

coreboot's allocator could be worked on to provide 64-bit allocation of these
resources, which would be one way of addressing the problem. However, the
difficulty of doing so compared to the benefit is questionable. Right now, the
Linux and Tianocore payloads are fully capable of doing their own PCIe
resource allocation. However, with devices being initialized by coreboot, there
is no easy way to allow the devices to be re-initialized by the payload.

Benefit: Both linux-as-a-payload and TianoCore users are in the minority of installations nowadays, while everyone would want those large MMIO resources pushed above 4 GiB allocation solved. 

> What this patch does is provide a quick and easy to understand way for an
> advanced platform to do PCIe resource allocation in the payload. No massive
> rewrite of the coreboot allocator has to take place. The payloads that do not
> support this can continue using the simple allocator coreboot has. coreboot
> pushes advanced initialization to the payload where it probably should be.

Please comment on and review my previous arguments. One can complete PCI enumeration without assigning the resources, that would avoid the ACPI namespace pitfalls.

Also development guidelines have certain and somewhat strong suggestion against squashing patches together, This one is borderline when the commit message already starts with this does a) and b) and c). Also we nowadays require unresolved comments to be addressed before submit is allowed, and in general rubber-stamp reviews have reduced a lot since 2014.

View Change

To view, visit change 36221. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2073d9f8e9297c2b02530821ebb634ea2a5c758e
Gerrit-Change-Number: 36221
Gerrit-PatchSet: 9
Gerrit-Owner: ron minnich <rminnich@gmail.com>
Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan@google.com>
Gerrit-Reviewer: Jeremy Soller <jeremy@system76.com>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki@gmail.com>
Gerrit-Reviewer: Lean Sheng Tan <lean.sheng.tan@intel.com>
Gerrit-Reviewer: Martin Roth <martinroth@google.com>
Gerrit-Reviewer: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki@gmail.com>
Gerrit-Reviewer: Shelley Chen <shchen@google.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-Reviewer: ron minnich <rminnich@gmail.com>
Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-Comment-Date: Fri, 25 Oct 2019 16:31:34 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment