[coreboot-gerrit] Change in coreboot[master]: soc/intel: Fix regression with hidden PCI devices

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34285 )

Change subject: soc/intel: Fix regression with hidden PCI devices
......................................................................

Patch Set 4:
> Patch Set 3:
>
> (1 comment)
> > Patch Set 3:
> >
> > Also we have pci_s_read_config() that takes BDF as argument, no need for device node pointer. I can use that on code like p2sb_hide() and p2sb_unhide():
>
> Why not do it for all devices that are currently accessed using PCH_DEV_* i.e. just use _s_ variant to pass in PCH_DEVFN_* and get rid of all PCH_DEV_* completely.

IMO what p2sb_hide/unhide does is a specs violation, sending configuration cycles to a PCI device that does not respond to the PCI ID register query. Currently I am assuming the register with hide/unide bit is the only one hardware always decodes.

I would rather make that one an explicit quirk, complain in the logs (maybe BIOS_DEBUG, not BIOS_ERR) of having to do so, and maybe even make it require a "select PCI_QUIRK_SPECS_VIOLATION" in the platform configs. I did not check PCI space of that device, I bet there would be bunch of interesting bits for firmware, maybe even OS. Like SERR/PERR perhaps.