Bug #207

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

Added by Lukasz Siudut 27 days ago. Updated 27 days ago.

Status:NewStart date:05/20/2019
Priority:NormalDue date:
Assignee:-% Done:

0%

Category:-
Target version:-

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

History

#1 Updated by Lukasz Siudut 27 days 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 :).

Also available in: Atom PDF