Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31752 )
Change subject: device/pci: Place MMCONF_BASE_ADDRESS in const variable ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/#/c/31752/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31752/2//COMMIT_MSG@10 PS2, Line 10: optimisation, specially when multiple registers on
In what way? Such that register is loaded once? Isn't the constant folding already done? This feels […]
Seemed to reduce code to (base | bus:dev.fn) loaded just once, instead of loading immediate (base | register) for every single register access. For many cases, PCI register constant is 8bit immediate.
I'll paste some examples once I have x86_64 for comparison.
https://review.coreboot.org/#/c/31752/2/src/include/device/pci_mmio_cfg.h File src/include/device/pci_mmio_cfg.h:
https://review.coreboot.org/#/c/31752/2/src/include/device/pci_mmio_cfg.h@24 PS2, Line 24: extern const uintptr_t var_mmconf_base;
What's the point of this? If it's const it can't be changed, right?
It's in global scope, if not declared const compiler could not optimise it. Maybe there is better way to achieve the same?
Even if declared const here, with casting ramstage could still change this (and the BAR !) to effectively move PCIe config region.
-
Kyösti Mälkki (Code Review)