Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40968 )
Change subject: soc/intel/tigerlake: Clear TBT PCIe root ports BME at exit boot service ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40968/6/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.c:
https://review.coreboot.org/c/coreboot/+/40968/6/src/soc/intel/tigerlake/chi... PS6, Line 142: if ((pci_read_config32(dev, CAPID0_A) & VTD_DISABLE)) I don't understand this logic. I would think the policy should not be dependent on VTD disable. I also am confused why we think it's necessary to not disable bme when VTD is disabled.
https://review.coreboot.org/c/coreboot/+/40968/6/src/soc/intel/tigerlake/chi... PS6, Line 157: clear_tbt_pcie_rp_bme(dev, PCI_DEVICE_ID_INTEL_TGL_TBT_RP3);
We only need to disable root ports BME before handing off to OS. […]
Why is it ever appropriate to have BME set on these ports in the firmware? A pci driver is straight forward for this: The device operations can use all the defaults except for .enable_resources() where that would turn into the following sequence.
pci_dev_disable_bus_master(dev); dev->command &= ~PCI_COMMAND_MASTER; pci_dev_set_resources(dev);