Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42459 )
Change subject: [RFC] device: Add method for configuring bus master based on config option ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42459/1/src/device/Kconfig File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/42459/1/src/device/Kconfig@519 PS1, Line 519: config PCI_ALLOW_BUS_MASTER
Why, though? Security? To prevent PCI devices from initiating transfers? […]
FSP might set it temporarily (didn't check the code), but in our tests it left it disabled when returning to coreboot.
The basic rule is that enabling bus mastering is wrong unless you want a device to do DMA (or take over your system). There is a slight chance that some integrated devices are buggy enough to require it enabled during their init sequence, but that is no excuse to leave it enabled. In some cases, coreboot enables it _after_ the init sequence which makes no sense at all.
https://review.coreboot.org/c/coreboot/+/42459/4/src/include/device/pci.h File src/include/device/pci.h:
https://review.coreboot.org/c/coreboot/+/42459/4/src/include/device/pci.h@13... PS4, Line 131: void pci_configure_bus_master(struct device *dev);
void pci_dev_enable_bus_master(const struct device *dev);
Ack to the `const` but we didn't name it `enable` on purpose, because the point is to not enable it unconditionally.