Aaron Durbin 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
Seemed to reduce code to (base | bus:dev. […]
So it loaded the base | bus:dev.fn one time and then OR'd in the register offset vs reloading everything?
You might be able to achieve the same thing by using a local variable in the inline function. Not sure if that would pick up the constant folding optimization or not. Worth a try, I suppose.
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;
It's in global scope, if not declared const compiler could not optimise it. […]
I get that one could work around things by casting to a mutable varible to accommodate a change. However, if we actually applied proper protection to .text and read only segments this would hack would be impossible.
-
Aaron Durbin (Code Review)