Re: [PATCH] arm64: Relocate screen_info.lfb_base on PCI BAR allocation

[Bjorn Helgaas]

[+cc Matt, because I'm out of my depth in this UEFI stuff]

On Fri, Apr 29, 2016 at 11:52:47PM +0200, Alexander Graf wrote:
> On 29.04.16 23:37, Bjorn Helgaas wrote:
> > On Fri, Apr 29, 2016 at 10:51:34PM +0200, Alexander Graf wrote:

> >> I think the only thing we can really take as granted is the physical
> >> address we receive. Any efi device topology may be as implementation
> >> dependent as your firmware vendor wants to it to be.
> >>
> >> So couldn't we just try to see whether the efifb is within a pci mmio
> >> window? We could then go through all devices on it, search for
> >> overlapping BAR allocations and know the device. From there it would
> >> just be a matter of officially reserving the region - or setting the
> >> resource to FIXED, right?
> > 
> > You could do something like a quirk that ran on every device after
> > reading the BARs.  Then you could see if any of the device's resources
> > contain the framebuffer address, and if so, set the FIXED bit for that
> > resource.  Or you could save the pci_dev and the BAR number and make
> > the framebuffer driver actually claim that device.  That would have
> > the advantage of cleaner ownership and allowing the core to reassign
> > it if necessary.
> 
> Unfortunately the framebuffer driver gets initialized after the BARs got
> reassigned, so that wouldn't work :).

Quirks (fixups) can run at several different times, including before
we read the BARs, after we read them but before any reassignment, etc.
I'll attach a call graph of the enumeration process (somewhat out of
date now, but enough to show the fixup points).  If we went this
route, a header quirk (DECLARE_PCI_FIXUP_HEADER) would do what we
need.

> Can you point me to the code that does "read[ing] the BARs"? So far my
> impression was that we don't even try to read out the current BAR values
> from pci config space, but I can't claim I fully grasp the Linux pci
> code quite yet.

The efifb.c driver doesn't do anything at all with PCI (it includes
linux/pci.h, but probably doesn't need it).  That's part of what I'm
suggesting -- if it *did* register as a PCI device driver, then it
would look at pci_dev->resource[n], which is populated by the PCI
core based on the BAR values.

> > It would be a lot nicer if UEFI told us which device was the console.
> > We can't figure this out from the ConOut variable or the HCDP or PCDP
> > table or something?
> 
> I guess you could try to look at the device path for the GOP handle, but
> I'm not sure any layout of the device paths is mandated by the spec. For
> all I know a valid implementation could just claim the GOP interface
> fell from the skies and it'd be legitimate. But I'm happy to stand
> corrected if someone points me to the respective part of the spec.

Is ConOut what you're after?  I.e., is the whole point of this
exercise to get a framebuffer driver attached to the device that was
the firmware console?  I would think the ConOut path should be
decodable -- it has to tell you how to navigate the interconnect from
the CPU to the device.  But I don't know how to do it.

It looks like on x86, at least, setup_gop32()/setup_gop64() might be
extracting the framebuffer address from the ConOut device and stuffing
it into screen_info, which is what efifb.c later looks at (maybe this
is what Ard was referring to).

> I suppose HCDP and PCDP are ACPI tables? We can't rely on those :).
> AArch64 systems can (and some do) boot perfectly fine without any ACPI
> tables exposed via uEFI. And the less we have to marry ACPI with uEFI
> the happier I am - uEFI seems like a pretty nice path towards
> standardized booting while ACPI was quite a nightmare every time I
> looked at it so far ;).

HCDP/PCDP was from DIG64, originally for ia64, but I can't find a copy
of the spec anymore.  The http://www.dig64.org/specifications/ pointer
in pcdp.h appears broken.  So forget I brought this up; this looks dead.

Bjorn


PCI enumeration call graph:

  pci_scan_root_bus
    pci_create_root_bus
      pci_alloc_host_bridge
      device_register(pci_host_bridge)
      device_register(pci_bus)
      pcibios_add_bus(pci_bus)                  # pcibios

    pci_scan_child_bus
      pci_scan_slot
        pci_scan_single_device
          pci_scan_device
            pci_alloc_dev
            pci_setup_device
              printk("[%04x:%04x] type ...")
              pci_fixup_device(pci_fixup_early) # fixup
              PCI_HEADER_TYPE_NORMAL:
                pci_read_irq
                pci_read_bases                  # <-- read BARs
              PCI_HEADER_TYPE_BRIDGE:
                pci_read_irq
                pci_read_bases
              PCI_HEADER_TYPE_CARDBUS:
                pci_read_irq
                pci_read_bases
          pci_device_add
            pci_fixup_device(pci_fixup_header)  # fixup
            pci_reassigndev_resource_alignment
            pci_init_capabilities
            pcibios_add_device                  # pcibios
            device_add
        pcie_aspm_init_link_state

      for (pass = 0; pass < 2; pass++)
        list_for_each_entry
          pci_scan_bridge
            pci_add_new_bus
              pci_alloc_child_bus
                pci_alloc_bus
                dev_set_name(pci_bus)
                device_register(pci_bus)
                pcibios_add_bus                 # pcibios
                pci_create_legacy_files
            pci_scan_child_bus                  # recursive

    pci_bus_add_devices
      list_for_each_entry(dev, &bus->devices, ...)
        pci_bus_add_device
          pci_fixup_device(pci_fixup_final)             # fixup
          pci_create_sysfs_dev_files
          pci_proc_attach_device
      list_for_each_entry(dev, &bus->devices, ...)
        pci_bus_add_devices(dev->subordinate)           # recursive

  pci_assign_unassigned_resources                       # from arch code
    list_for_each_entry(..., &pci_root_buses, ...)
      pci_assign_unassigned_root_bus_resources          # <-- assignment
        __pci_bus_size_bridges
          pci_bridge_check_ranges
            pci_read_config_word(..., PCI_IO_BASE, ...)
            pci_read_config_word(..., PCI_PREF_MEMORY_BASE, ...)

  <driver>
    pci_enable_device
      pci_enable_device_flags(MEM | IO)
        pci_enable_bridge(pci_upstream_bridge)
          pci_enable_bridge(pci_upstream_bridge)          # recursive
          pci_enable_device
          pci_set_master
        do_pci_enable_device
          pci_set_power_state(D0)
          pcibios_enable_device                           # pcibios
          pci_fixup_device(pci_fixup_enable)              # fixup