Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46617 )
Change subject: device/pci_device: Map big PCI bars above 4 GiB ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46617/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46617/1//COMMIT_MSG@10 PS1, Line 10: FIXME: This should probably go into the resource allocator.
The prefmem PCI bridge window covers both, prefmem BARs below 4GiB and prefmem BARs above 4 GiB, which is wrong.
So the bridge resource contains a single window but the child device resource allocation happens outside that window? Would you be able to share the logs with this change to show how the resource allocation happens?
So this definitely needs support in the resource allocator that must place all prefmem BARs behind a bridge in one continuous MMIO region.
Yes, if that is not happening, it is a bug. If it is not in a continuous MMIO region, then it means we are allocating outside the window on the bridge. Based on what you said, I am guessing that the downstream device has resources which are varying in sizes -- some which are greater than 4GiB and others which are smaller. That is the reason why the resource allocator ends up allocating non-contiguous MMIO space. This is the reason why I had mentioned that it would be best to have a specific PCI driver for the device which knows that it needs all resources above or below 4GiB boundary and can make appropriate request.
In this case it's an external GPU and mainboard code cannot make any assuptions about the devices plugged in.
But, the PCI driver for the external GPU device can set the request for all its resources to ensure that they get allocated above 4GiB boundary. I agree that we should have checks in resource allocator to ensure that we never do a split allocation. That is just wrong irrespective of what the request is.
Instead of having an error prone lookup table of devices that needs a BAR below 4GiB it's time to move to x86_64 in coreboot and deprecate 32bit OS as well.
I don't disagree, but requires larger discussion and scoping :).