[PATCH v3] efifb: avoid reconfiguration of BAR that covers the framebuffer
From: Ard Biesheuvel <hidden>
Date: 2017-03-27 15:37:57
Also in:
linux-efi, linux-pci
On 23 March 2017 at 15:15, Ard Biesheuvel [off-list ref] wrote:
quoted hunk ↗ jump to hunk
On 23 March 2017 at 14:31, Lorenzo Pieralisi [off-list ref] wrote:quoted
On Thu, Mar 23, 2017 at 12:25:48PM +0000, Ard Biesheuvel wrote:quoted
On 23 March 2017 at 10:57, Lorenzo Pieralisi [off-list ref] wrote:quoted
On Thu, Mar 23, 2017 at 09:04:03AM +0000, Ard Biesheuvel wrote:quoted
On 23 March 2017 at 08:48, Lukas Wunner [off-list ref] wrote:quoted
On Wed, Mar 22, 2017 at 07:32:43PM +0000, Ard Biesheuvel wrote:quoted
On 22 March 2017 at 19:31, Lukas Wunner [off-list ref] wrote:quoted
On Wed, Mar 22, 2017 at 03:30:29PM +0000, Ard Biesheuvel wrote:quoted
On UEFI systems, the PCI subsystem is enumerated by the firmware, and if a graphical framebuffer is exposed by a PCI device, its base address and size are exposed to the OS via the Graphics Output Protocol (GOP). On arm64 PCI systems, the entire PCI hierarchy is reconfigured from scratch at boot. This may result in the GOP framebuffer address to become stale, if the BAR covering the framebuffer is modified. This will cause the framebuffer to become unresponsive, and may in some cases result in unpredictable behavior if the range is reassigned to another device.Hm, commit message seems to indicate the issue is restricted to arm64, yet there's no IS_ENABLED(ARM64) to constrain the added code to that arch?True. I am eager to get some x86 coverage for this, since I would expect this not to do any harm. But I'm fine with making it ARM/arm64 specific in the final version.I see. IIUC, this is only a problem because pci_bus_assign_resources() is called from arch/arm64/kernel/pci.c:pci_acpi_scan_root() (as well as the host drivers) and x86 isn't affected because it doesn't do that.Correct. But on x86 (or rather, on a PC), you can be sure that UEFI (or the legacy PCI bios) performed the resource assignment already. One could argue that this is equally the case when running arm64 in ACPI mode, but in general, you cannot assume the presence of firmware on ARM/arm64 that has already taken care of this, and so the state of the BARs has to be presumed invalid.The story is a bit more convoluted than that owing to x86 (and other arches) legacy. x86 tries to claim all PCI resources (in two passes - first enabled devices, second disabled devices) and that predates ACPI/UEFI. Mind, x86 reassign resources that can't be claimed too, the only difference with ARM64 is that, for the better or the worse, we have decided not to claim the FW PCI set-up on ARM64 even if it is sane, we do not even try, it was a deliberate choice. This patch should be harmless on x86 since if the FB PCI BAR is set up sanely, claiming it again should be a nop (to be checked).I have checked this with OVMF under QEMU. Claiming the resource early like we do this in this patch does not result in any diagnostic output or other symptoms that would suggest that anything unexpected occurs.There is something that I do not understand on how the resource claiming works on x86. IIUC on x86, resource claiming is done in: arch/x86/pci/legacy.c pci_subsys_init() -> pcibios_init() -> pcibios_resource_survey() pci_subsys_init() is run in a subsys_initcall.Yes, the call trace I get for the resource claim for the efifb BAR without this patch is pci_subsys_init+0x3f/0x43 - pcibios_init+0x2c/0x3d -- pcibios_resource_survey+0x38/0x6a--- pci_legacy_init+0x2e/0x2e ---- pcibios_allocate_resources+0x8a/0x240 ----- pci_claim_resource+0xdc/0x140quoted
Now, how do we ensure that resource claiming is carried out _after_ the PCI root busses have been actually scanned ? The ACPI scan handler interface (so the interface to actually scan a PCI root bridge in ACPI) is initialized in acpi_init() (which is a subsys_initcall), how do we guarantee that is run before pci_subsys_init() ?$ nm vmlinux |grep -E '__initcall_(acpi_init|pci_subsys_init)' ffffffff81d540b8 t __initcall_acpi_init4 ffffffff81d54158 t __initcall_pci_subsys_init4 so it appears to depend on link order currently.
Bjorn, what is your take on this? Claiming the BAR resources associated with the efifb region from a DECLARE_PCI_FIXUP_HEADER() handler does not seem to interfere with the way resources are claimed later on. For arm64, we need this to ensure that the BAR doesn't move, but for x86 this does not seem to be an issue. This means we could potentially make the quirk ARM-only, but I am a bit reluctant to do so and create even more divergence. Thanks, Ard.