acpi_pcib: Trust decoded bus range from _CRS over _BBN

[jhb]

Summary

Currently if _BBN doesn't match the first bus in the decoded bus range
from _CRS for a Host to PCI bridge, the driver fails to attach as a
defensive measure.

There is now firmware in the field where these do not match, and the
_BBN values are clearly wrong, so rather than failing attach, trust
the range from _CRS over _BBN.

Reported by: gibbs

[imp]

Functionally: lgtm. One quibble over an old ERROR message that's now just a warning message and how it might be adjusted.

sys/dev/acpica/acpi_pcib_acpi.c

520–531

does this message need to be adjusted to indicate that we're overriding the found decoded range?

[gibbs]

sys/dev/acpica/acpi_pcib_acpi.c

520–531

I changed this when integrating John's patch to this:

if (decode_bus_range(sc, &start, &end) == 0) {
    if (sc->ap_bus != start) {
        device_printf(dev,
            "WARNING: BIOS configured bus number (%d) is not within "
            "decoded bus number range (%ju - %ju). "
            "Using range start (%ju) as bus number.\n",
            sc->ap_bus, (uintmax_t)start, (uintmax_t)end,
            (uintmax_t)start);
        sc->ap_bus = start;
    }
}

[jhb]

sys/dev/acpica/acpi_pcib_acpi.c

520–531

I'll go with Justin's message. To be clear, we aren't overriding the range, we are just overriding the bus number of the bridge.

[gibbs]

sys/dev/acpica/acpi_pcib_acpi.c

520–531

Small nit. "decoded_bus_range" kinda implies the return value will be the range. That's why I called the function "decode_bus_range" instead.

[jhb]

sys/dev/acpica/acpi_pcib_acpi.c

520–531

I hadn't actually seen that part of your diff at first. I had used decoded originally (even when the return value was an errno) as the idea I was trying to communicate was "fetch the first bus number decoded by the bus" and now "fetch the entire range of bus numbers (start and end) decoded by the bus". If this were C++ the signature would be something like: std::optional<std::pair<rman_res_t, rman_res_t>> acpi_pcib_acpi::decoded_bus_range(). But C is not really conducive to that sort of return value, hence returning the actual range via out parameters instead (but logically still "returning" start and end) with the return value just being a success/failure indicator (the return value should probably be a bool rather than an errno value though).

[jhb]

sys/dev/acpica/acpi_pcib_acpi.c

520–531

Mmm, but perhaps the implicit verb there is my problem and get_decoded_bus_range or fetch_decoded_bus_range would be clearer? (In particular, we aren't doing any real decoding (e.g. we aren't parsing the result of _CRS) but are instead returning a cached result of a previous decoding of _CRS.)

[gibbs]

sys/dev/acpica/acpi_pcib_acpi.c

520–531

I think a "get_decoded_bus_range()" that returns a bool makes the most sense.