On Mon, May 05, 2025 at 03:05:34PM +0200, Thomas Zimmermann wrote:
Am 22.04.25 um 23:47 schrieb Bjorn Helgaas:
On Tue, Apr 22, 2025 at 09:49:57AM +0200, Thomas Zimmermann wrote:
Apply bridge window offsets to screen_info framebuffers during relocation. Fixes invalid access to I/O memory.
Resources behind a PCI bridge can be located at a certain offset in the kernel's I/O range. The framebuffer memory range stored in screen_info refers to the offset as seen during boot (essentialy 0). During boot up, the kernel may assign a different memory offset to the bridge device and thereby relocating the framebuffer address of the PCI graphics device as seen by the kernel. The information in screen_info must be updated as well.
I can't see the bug report below, so I'm not sure what's happening here. Apparently the platform is one where PCI bus addresses are not identical to CPU physical addresses. On such platforms, the PCI host bridge applies an offset between CPU and PCI addresses. There are several systems like that, but I'm not aware of any that change that CPU->PCI bus address offset at runtime.
So I suspect the problem is not that the kernel has assigned a different offset. I think it's more likely that the hardware or firmware has determined the offset before the kernel starts, and this code just doesn't account for that.
Right, that's what I'm trying to say. I guess my explanation simply isn't clear.
Yeah, the part about the "kernel assigning a different offset" is a bit misleading because the kernel doesn't actually assign or *change* that offset; it only *discovers* the offset, typically from an ACPI _TRA method or from device tree.
This bug isn't public. Can it be made public? Or even better, a report at https://bugzilla.kernel.org?
Try again, please. I've updated the settings of this bug report.
Works now, thanks!
@@ -69,10 +69,21 @@ static void screen_info_fixup_lfb(struct pci_dev *pdev) for (i = 0; i < numres; ++i) { struct resource *r = &res[i];
struct pci_bus_region bus_region = {.start = r->start,.end = r->end,};screen_info_resources() above fills in "struct resource res[]", but that's not quite right. A struct resource contains CPU addresses, and screen_info_resources() fills in PCI bus addresses (0xa0000, etc).
struct pci_bus_region is meant to hold PCI bus addresses, so this assignment gets them back where they should be.
const struct resource *pr; if (!(r->flags & IORESOURCE_MEM)) continue;
/** Translate the address to resource if the framebuffer* is behind a PCI bridge.*/pcibios_bus_to_resource(pdev->bus, r, &bus_region);And this converts the PCI bus addresses to CPU addresses, so this makes sense.
The comment might be a little misleading, though. When PCI bus addresses are different from CPU addresses, it's because the PCI host bridge has applied an offset. This only happens at the host bridge, never at a PCI-PCI bridge (which is what "PCI bridge" usually means).
The commit log and comments could maybe be clarified, but this all looks to me like it's doing the right thing in spite of abusing the use of struct resource.
Thanks for reviewing. I'll try to clarify on the commit message. Not sure how to change the issue with struct pci_bus_region though.
Yeah, I don't know either. screen_info_resources() takes a struct resource pointer, but puts bus addresses into it. That's misleading at best, but it would be quite a bit more work to fix that.
I'm not sure we have a generic struct for bus addresses. We have struct pci_bus_region, but I'm not sure if screen_info is necessarily specific to PCI.
Bjorn