Patch Set 3: Code-Review-1

In the SMI handler, this might work because of the static PCI_DEV()
value (it would access the correct PCI config space and likely run
into the 0xffffffff check later).

In ramstage however, where the BUG is printed, it is an actual BUG!
There are null-pointer dereferences, or worse, reads from the wrong
devices (see comment in pci_dev_path_on_root_debug()). One can't
simply check for NULL, because of the mess with these device macros
(they are only pointers in the !__SIMPLE_DEVICE__ case). People
started to write code common to simple and non-simple device cases,
before that was supported by the coreboot infrastructure, now every-
thing is chaos...

Generally, all soc code that does a #if defined(__SIMPLE_DEVICE__)
can be considered broken. If you want the BUG (messages) to go away,
please fix the code. IIRC, Kyösti just abandoned some efforts in
that area (because it is too hard to fix while others add new copies
of this broken code pattern). If you can help him out, maybe we can
fix this topic for good.

I had a quick look at the pci_dev_path_on_root_debug(). From that I am not sure if I correctly understand the issue. From this I don't get the impression that pci_dev_path_on_root returns false non NULL results. Is that a correct assumption? Or is the entire device tree structure broken cause random errors?

What I am doing here is check if a possible root port is on the devicetree. If it is a non NULL value is returned is it isn't a NULL value is returned and the PCI device shouldn't be accessed. Previously this returned the bug message which is fine for standard accesses but shouldn't be done if you just want to check if a device is on the tree.

View Change

To view, visit change 38750. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6bc04e48e97b0a29aef8aa050d3aad116cff1a14
Gerrit-Change-Number: 38750
Gerrit-PatchSet: 3
Gerrit-Owner: Wim Vervoorn <wvervoorn@eltan.com>
Gerrit-Reviewer: Frans Hendriks <fhendriks@eltan.com>
Gerrit-Reviewer: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org>
Gerrit-Reviewer: Wim Vervoorn <wvervoorn@eltan.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-Comment-Date: Tue, 11 Feb 2020 08:04:55 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment