Project

General

Profile

Actions

Bug #207

closed

QEMU i440FX platform broken in master / PCI: dev is NULL!

Added by Lukasz Siudut almost 5 years ago. Updated almost 5 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
-
Category:
-
Target version:
-
Start date:
05/20/2019
Due date:
% Done:

0%

Estimated time:
Affected versions:
Needs backport to:
Affected hardware:
Affected OS:

Description

It seems that the breakage was introduced in e079e5ccc2e707e5b6bd3b011e04c9138f159808 [1] and reworked multiple times in the meantime. Now the failure occurs in PCI_BDF macro in src/include/device/pci_type.h [2] file.

I don't entirely understand how much is it a problem and how much intended behavior. My understanding is that this is mostly to catch up clowny pci_read_config calls when pointer to the device is NULL. I think so because changing #if 1 to #if 0 in ./src/include/device/pci_type.h [2] effectively fixes the issue (this changes calls from pcidev_assert to pcidev_bdf directly).

Why is it breaking for i440fx? Because lots of code assumes that LPC controller (00:1f.0) exists in the system but this is not true for this (device (0,0) also seems to be returning NULL, so is this call even valid for i440?). Plenty of examples can be found in ./src/southbridge/intel/i82801ix/smi.c [3]. pcidev_on_root(0x1f, 0) returns NULL and it's passed to pci_read_config16 directly.

It's even worse for read_pmbase32 functions which are used all around and they all instantly break.

I tried to track all of those occurrences but there's plenty of them and I definitely don't have enough knowledge to fix it [properly]. The quick fix that comes to my mind is to make pcidev_assert [2] call dependable on architecture and stop calling it for QEMU builds. Someone may tell though that this should be fixed "properly", whatever that means. I don't even know if this is valid solution giving that we will end up with reading pci configs from NULL in multiple places (it boots though!).

That said, it's broken now :-).

[1] https://github.com/coreboot/coreboot/commit/e079e5ccc2e707e5b6bd3b011e04c9138f159808
[2] https://github.com/coreboot/coreboot/blob/master/src/include/device/pci_type.h#L38
[3] https://github.com/coreboot/coreboot/blob/master/src/southbridge/intel/i82801ix/smi.c#L54

Actions #1

Updated by Lukasz Siudut almost 5 years ago

Oh my. Right after I posted it I realized (aka rubberducked) that there is no issue at all. I simply built Coreboot for pc35 architecture and tried to run it on i440fx.

The change in behavior simply surprised me. It used to work just fine!

I guess it's safe to close this one and let me hide in my box of shame :).

Actions #2

Updated by Patrick Rudolph almost 5 years ago

  • Status changed from New to In Progress
Actions #3

Updated by Patrick Rudolph almost 5 years ago

  • Status changed from In Progress to Resolved
Actions

Also available in: Atom PDF