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

[Matt Fleming]

(Pulling in efifb maintainer, Peter)

On Fri, 29 Apr, at 06:31:19PM, Bjorn Helgaas wrote:
> 
> 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.
 
This discussion came up recently here,

  https://lkml.kernel.org/r/20160216151859.gb11...@redhat.com

There's nothing PCI-specific about the EFI framebuffer per se, but in
practice it's always a PCI device.

> 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).
 
Matthew Garrett wrote the x86 code for guessing where the console is.
We look for the ConOut protocol with a fallback to first GOP device if
we can't find it. I think the heuristic was based on reading the
implementation in EDK2. See commit 38cb5ef4473c ("X86: Improve GOP
detection in the EFI boot stub").

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

[Matt Fleming]

(Pulling in efifb maintainer, Peter)

On Fri, 29 Apr, at 06:31:19PM, Bjorn Helgaas wrote:
> 
> 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.
 
This discussion came up recently here,

  https://lkml.kernel.org/r/20160216151859.gb11...@redhat.com

There's nothing PCI-specific about the EFI framebuffer per se, but in
practice it's always a PCI device.

> 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).
 
Matthew Garrett wrote the x86 code for guessing where the console is.
We look for the ConOut protocol with a fallback to first GOP device if
we can't find it. I think the heuristic was based on reading the
implementation in EDK2. See commit 38cb5ef4473c ("X86: Improve GOP
detection in the EFI boot stub").

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
   

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
   

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

[Alexander Graf]

On 29.04.16 23:52, Alexander Graf wrote:
> 
> 
> On 29.04.16 23:37, Bjorn Helgaas wrote:
>
> 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.

I should've probably read Lorenzos email more closely before - it
contains the pointer.


Alex

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

[Alexander Graf]

On 29.04.16 23:52, Alexander Graf wrote:
> 
> 
> On 29.04.16 23:37, Bjorn Helgaas wrote:
>
> 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.

I should've probably read Lorenzos email more closely before - it
contains the pointer.


Alex

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

[Alexander Graf]

On 29.04.16 23:37, Bjorn Helgaas wrote:
> On Fri, Apr 29, 2016 at 10:51:34PM +0200, Alexander Graf wrote:
>>
>>
>> On 29.04.16 22:25, Ard Biesheuvel wrote:
>>>
 On 29 apr. 2016, at 22:06, Bjorn Helgaas  wrote:

> On Fri, Apr 29, 2016 at 03:51:49PM +0200, Ard Biesheuvel wrote:
>> On 29 April 2016 at 15:41, Bjorn Helgaas  wrote:
>>> On Thu, Apr 28, 2016 at 11:39:35PM +0200, Alexander Graf wrote:
>>> On 28.04.16 20:06, Bjorn Helgaas wrote:

 If firmware is giving us a bare address of something, that seems like
 a clue that it might depend on that address staying the same.
>>>
>>> Well, I'd look at it from the other side. It gives us a correct address
>>> on entry with the system configured at exactly the state it's in on
>>> entry. If Linux changes the system, some guarantees obviously don't work
>>> anymore.
>>
>> Can you point me to the part of the EFI spec that communicates this?
>> I'm curious what the intent is and whether there's any indication that
>> EFI expects the OS to preserve some configuration.  I don't think it's
>> reasonable for the OS to preserve this sort of configuration because
>> it limits how well we can support hotplug.
>>
>> I wonder if we're using this frame buffer address as more than what
>> EFI intended.  For example, maybe it was intended for use by an early
>> console driver, but there's some other mechanism we should be using
>> after that.
>
> The UEFI spec describes this as follows (UEFIv2.6 section 11.9)
>
> """
> Graphics output may also be required as part of the startup of an
> operating system. There are
> potentially times in modern operating systems prior to the loading of
> a high performance OS
> graphics driver where access to graphics output device is required.
> The Graphics Output Protocol
> supports this capability by providing the EFI OS loader access to a
> hardware frame buffer and
> enough information to allow the OS to draw directly to the graphics
> output device.
> """
>
> So the intent is to provide minimal framebuffer services until the
> 'real' driver takes over.

 Makes sense.  A 'real' driver for a PCI device would use
 pci_register_driver() and use pci_resource_start() or similar to
 locate the framebuffer, which would avoid the problem because the PCI
 core doesn't change BARs while a driver owns the device.

> The GOP protocol only describes the base and size of the framebuffer,
> and the pixel format. At boot time, the early UEFI code in the kernel
> could potentially figure out which PCI device it is related to, if
> necessary, but i am not sure if this would solve the x86 case as well.

 Does drivers/video/fbdev/efifb.c support only a single framebuffer
 device?  If a system has several, how does it decide which to use?  I
 assume UEFI would provide GOP for all the framebuffers?

>>>
>>> Yes. The early efi code iterates over all gop instances to figure out which 
>>> one is connected to the efi console.
> 
> How does it match the EFI console to the GOP instance?  Sorry, I'm
> pretty EFI-illiterate, and "find . -name \*efi\*.c | xargs grep -i gop"
> shows nothing :)
> 
 If we could fix this by making efifb claim a PCI device, I think that
 would be cleaner.  I don't know how to figure out the correct device,
 but that would solve the "BAR changed" problem, and it would have
 cleaner ownership semantics, too.
>>>
>>> If the bus layout is static, the efi init code can record the 
>>> segment/bus/device and match it with the kernel's view. is that guaranteed 
>>> to be sufficient for all implementations of pci?
>>
>> 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 :).

Can you point me to the code that does "read[ing] the BARs"? So far my
impression was 

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

[Alexander Graf]

On 29.04.16 23:37, Bjorn Helgaas wrote:
> On Fri, Apr 29, 2016 at 10:51:34PM +0200, Alexander Graf wrote:
>>
>>
>> On 29.04.16 22:25, Ard Biesheuvel wrote:
>>>
 On 29 apr. 2016, at 22:06, Bjorn Helgaas  wrote:

> On Fri, Apr 29, 2016 at 03:51:49PM +0200, Ard Biesheuvel wrote:
>> On 29 April 2016 at 15:41, Bjorn Helgaas  wrote:
>>> On Thu, Apr 28, 2016 at 11:39:35PM +0200, Alexander Graf wrote:
>>> On 28.04.16 20:06, Bjorn Helgaas wrote:

 If firmware is giving us a bare address of something, that seems like
 a clue that it might depend on that address staying the same.
>>>
>>> Well, I'd look at it from the other side. It gives us a correct address
>>> on entry with the system configured at exactly the state it's in on
>>> entry. If Linux changes the system, some guarantees obviously don't work
>>> anymore.
>>
>> Can you point me to the part of the EFI spec that communicates this?
>> I'm curious what the intent is and whether there's any indication that
>> EFI expects the OS to preserve some configuration.  I don't think it's
>> reasonable for the OS to preserve this sort of configuration because
>> it limits how well we can support hotplug.
>>
>> I wonder if we're using this frame buffer address as more than what
>> EFI intended.  For example, maybe it was intended for use by an early
>> console driver, but there's some other mechanism we should be using
>> after that.
>
> The UEFI spec describes this as follows (UEFIv2.6 section 11.9)
>
> """
> Graphics output may also be required as part of the startup of an
> operating system. There are
> potentially times in modern operating systems prior to the loading of
> a high performance OS
> graphics driver where access to graphics output device is required.
> The Graphics Output Protocol
> supports this capability by providing the EFI OS loader access to a
> hardware frame buffer and
> enough information to allow the OS to draw directly to the graphics
> output device.
> """
>
> So the intent is to provide minimal framebuffer services until the
> 'real' driver takes over.

 Makes sense.  A 'real' driver for a PCI device would use
 pci_register_driver() and use pci_resource_start() or similar to
 locate the framebuffer, which would avoid the problem because the PCI
 core doesn't change BARs while a driver owns the device.

> The GOP protocol only describes the base and size of the framebuffer,
> and the pixel format. At boot time, the early UEFI code in the kernel
> could potentially figure out which PCI device it is related to, if
> necessary, but i am not sure if this would solve the x86 case as well.

 Does drivers/video/fbdev/efifb.c support only a single framebuffer
 device?  If a system has several, how does it decide which to use?  I
 assume UEFI would provide GOP for all the framebuffers?

>>>
>>> Yes. The early efi code iterates over all gop instances to figure out which 
>>> one is connected to the efi console.
> 
> How does it match the EFI console to the GOP instance?  Sorry, I'm
> pretty EFI-illiterate, and "find . -name \*efi\*.c | xargs grep -i gop"
> shows nothing :)
> 
 If we could fix this by making efifb claim a PCI device, I think that
 would be cleaner.  I don't know how to figure out the correct device,
 but that would solve the "BAR changed" problem, and it would have
 cleaner ownership semantics, too.
>>>
>>> If the bus layout is static, the efi init code can record the 
>>> segment/bus/device and match it with the kernel's view. is that guaranteed 
>>> to be sufficient for all implementations of pci?
>>
>> 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 :).

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 

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

[Bjorn Helgaas]

On Fri, Apr 29, 2016 at 10:51:34PM +0200, Alexander Graf wrote:
> 
> 
> On 29.04.16 22:25, Ard Biesheuvel wrote:
> > 
> >> On 29 apr. 2016, at 22:06, Bjorn Helgaas  wrote:
> >>
> >>> On Fri, Apr 29, 2016 at 03:51:49PM +0200, Ard Biesheuvel wrote:
>  On 29 April 2016 at 15:41, Bjorn Helgaas  wrote:
> > On Thu, Apr 28, 2016 at 11:39:35PM +0200, Alexander Graf wrote:
> > On 28.04.16 20:06, Bjorn Helgaas wrote:
> >>
> >> If firmware is giving us a bare address of something, that seems like
> >> a clue that it might depend on that address staying the same.
> >
> > Well, I'd look at it from the other side. It gives us a correct address
> > on entry with the system configured at exactly the state it's in on
> > entry. If Linux changes the system, some guarantees obviously don't work
> > anymore.
> 
>  Can you point me to the part of the EFI spec that communicates this?
>  I'm curious what the intent is and whether there's any indication that
>  EFI expects the OS to preserve some configuration.  I don't think it's
>  reasonable for the OS to preserve this sort of configuration because
>  it limits how well we can support hotplug.
> 
>  I wonder if we're using this frame buffer address as more than what
>  EFI intended.  For example, maybe it was intended for use by an early
>  console driver, but there's some other mechanism we should be using
>  after that.
> >>>
> >>> The UEFI spec describes this as follows (UEFIv2.6 section 11.9)
> >>>
> >>> """
> >>> Graphics output may also be required as part of the startup of an
> >>> operating system. There are
> >>> potentially times in modern operating systems prior to the loading of
> >>> a high performance OS
> >>> graphics driver where access to graphics output device is required.
> >>> The Graphics Output Protocol
> >>> supports this capability by providing the EFI OS loader access to a
> >>> hardware frame buffer and
> >>> enough information to allow the OS to draw directly to the graphics
> >>> output device.
> >>> """
> >>>
> >>> So the intent is to provide minimal framebuffer services until the
> >>> 'real' driver takes over.
> >>
> >> Makes sense.  A 'real' driver for a PCI device would use
> >> pci_register_driver() and use pci_resource_start() or similar to
> >> locate the framebuffer, which would avoid the problem because the PCI
> >> core doesn't change BARs while a driver owns the device.
> >>
> >>> The GOP protocol only describes the base and size of the framebuffer,
> >>> and the pixel format. At boot time, the early UEFI code in the kernel
> >>> could potentially figure out which PCI device it is related to, if
> >>> necessary, but i am not sure if this would solve the x86 case as well.
> >>
> >> Does drivers/video/fbdev/efifb.c support only a single framebuffer
> >> device?  If a system has several, how does it decide which to use?  I
> >> assume UEFI would provide GOP for all the framebuffers?
> >>
> > 
> > Yes. The early efi code iterates over all gop instances to figure out which 
> > one is connected to the efi console.

How does it match the EFI console to the GOP instance?  Sorry, I'm
pretty EFI-illiterate, and "find . -name \*efi\*.c | xargs grep -i gop"
shows nothing :)

> >> If we could fix this by making efifb claim a PCI device, I think that
> >> would be cleaner.  I don't know how to figure out the correct device,
> >> but that would solve the "BAR changed" problem, and it would have
> >> cleaner ownership semantics, too.
> > 
> > If the bus layout is static, the efi init code can record the 
> > segment/bus/device and match it with the kernel's view. is that guaranteed 
> > to be sufficient for all implementations of pci?
> 
> 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.

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?

Bjorn

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

[Bjorn Helgaas]

On Fri, Apr 29, 2016 at 10:51:34PM +0200, Alexander Graf wrote:
> 
> 
> On 29.04.16 22:25, Ard Biesheuvel wrote:
> > 
> >> On 29 apr. 2016, at 22:06, Bjorn Helgaas  wrote:
> >>
> >>> On Fri, Apr 29, 2016 at 03:51:49PM +0200, Ard Biesheuvel wrote:
>  On 29 April 2016 at 15:41, Bjorn Helgaas  wrote:
> > On Thu, Apr 28, 2016 at 11:39:35PM +0200, Alexander Graf wrote:
> > On 28.04.16 20:06, Bjorn Helgaas wrote:
> >>
> >> If firmware is giving us a bare address of something, that seems like
> >> a clue that it might depend on that address staying the same.
> >
> > Well, I'd look at it from the other side. It gives us a correct address
> > on entry with the system configured at exactly the state it's in on
> > entry. If Linux changes the system, some guarantees obviously don't work
> > anymore.
> 
>  Can you point me to the part of the EFI spec that communicates this?
>  I'm curious what the intent is and whether there's any indication that
>  EFI expects the OS to preserve some configuration.  I don't think it's
>  reasonable for the OS to preserve this sort of configuration because
>  it limits how well we can support hotplug.
> 
>  I wonder if we're using this frame buffer address as more than what
>  EFI intended.  For example, maybe it was intended for use by an early
>  console driver, but there's some other mechanism we should be using
>  after that.
> >>>
> >>> The UEFI spec describes this as follows (UEFIv2.6 section 11.9)
> >>>
> >>> """
> >>> Graphics output may also be required as part of the startup of an
> >>> operating system. There are
> >>> potentially times in modern operating systems prior to the loading of
> >>> a high performance OS
> >>> graphics driver where access to graphics output device is required.
> >>> The Graphics Output Protocol
> >>> supports this capability by providing the EFI OS loader access to a
> >>> hardware frame buffer and
> >>> enough information to allow the OS to draw directly to the graphics
> >>> output device.
> >>> """
> >>>
> >>> So the intent is to provide minimal framebuffer services until the
> >>> 'real' driver takes over.
> >>
> >> Makes sense.  A 'real' driver for a PCI device would use
> >> pci_register_driver() and use pci_resource_start() or similar to
> >> locate the framebuffer, which would avoid the problem because the PCI
> >> core doesn't change BARs while a driver owns the device.
> >>
> >>> The GOP protocol only describes the base and size of the framebuffer,
> >>> and the pixel format. At boot time, the early UEFI code in the kernel
> >>> could potentially figure out which PCI device it is related to, if
> >>> necessary, but i am not sure if this would solve the x86 case as well.
> >>
> >> Does drivers/video/fbdev/efifb.c support only a single framebuffer
> >> device?  If a system has several, how does it decide which to use?  I
> >> assume UEFI would provide GOP for all the framebuffers?
> >>
> > 
> > Yes. The early efi code iterates over all gop instances to figure out which 
> > one is connected to the efi console.

How does it match the EFI console to the GOP instance?  Sorry, I'm
pretty EFI-illiterate, and "find . -name \*efi\*.c | xargs grep -i gop"
shows nothing :)

> >> If we could fix this by making efifb claim a PCI device, I think that
> >> would be cleaner.  I don't know how to figure out the correct device,
> >> but that would solve the "BAR changed" problem, and it would have
> >> cleaner ownership semantics, too.
> > 
> > If the bus layout is static, the efi init code can record the 
> > segment/bus/device and match it with the kernel's view. is that guaranteed 
> > to be sufficient for all implementations of pci?
> 
> 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.

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?

Bjorn

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

[Bjorn Helgaas]

On Fri, Apr 29, 2016 at 10:25:13PM +0200, Ard Biesheuvel wrote:
> 
> > On 29 apr. 2016, at 22:06, Bjorn Helgaas  wrote:
> > 
> >> On Fri, Apr 29, 2016 at 03:51:49PM +0200, Ard Biesheuvel wrote:
> >>> On 29 April 2016 at 15:41, Bjorn Helgaas  wrote:
>  On Thu, Apr 28, 2016 at 11:39:35PM +0200, Alexander Graf wrote:
>  On 28.04.16 20:06, Bjorn Helgaas wrote:
> > 
> > If firmware is giving us a bare address of something, that seems like
> > a clue that it might depend on that address staying the same.
>  
>  Well, I'd look at it from the other side. It gives us a correct address
>  on entry with the system configured at exactly the state it's in on
>  entry. If Linux changes the system, some guarantees obviously don't work
>  anymore.
> >>> 
> >>> Can you point me to the part of the EFI spec that communicates this?
> >>> I'm curious what the intent is and whether there's any indication that
> >>> EFI expects the OS to preserve some configuration.  I don't think it's
> >>> reasonable for the OS to preserve this sort of configuration because
> >>> it limits how well we can support hotplug.
> >>> 
> >>> I wonder if we're using this frame buffer address as more than what
> >>> EFI intended.  For example, maybe it was intended for use by an early
> >>> console driver, but there's some other mechanism we should be using
> >>> after that.
> >> 
> >> The UEFI spec describes this as follows (UEFIv2.6 section 11.9)
> >> 
> >> """
> >> Graphics output may also be required as part of the startup of an
> >> operating system. There are
> >> potentially times in modern operating systems prior to the loading of
> >> a high performance OS
> >> graphics driver where access to graphics output device is required.
> >> The Graphics Output Protocol
> >> supports this capability by providing the EFI OS loader access to a
> >> hardware frame buffer and
> >> enough information to allow the OS to draw directly to the graphics
> >> output device.
> >> """
> >> 
> >> So the intent is to provide minimal framebuffer services until the
> >> 'real' driver takes over.
> > 
> > Makes sense.  A 'real' driver for a PCI device would use
> > pci_register_driver() and use pci_resource_start() or similar to
> > locate the framebuffer, which would avoid the problem because the PCI
> > core doesn't change BARs while a driver owns the device.
> > 
> >> The GOP protocol only describes the base and size of the framebuffer,
> >> and the pixel format. At boot time, the early UEFI code in the kernel
> >> could potentially figure out which PCI device it is related to, if
> >> necessary, but i am not sure if this would solve the x86 case as well.
> > 
> > Does drivers/video/fbdev/efifb.c support only a single framebuffer
> > device?  If a system has several, how does it decide which to use?  I
> > assume UEFI would provide GOP for all the framebuffers?
> > 
> 
> Yes. The early efi code iterates over all gop instances to figure out which 
> one is connected to the efi console.
> 
> > If we could fix this by making efifb claim a PCI device, I think that
> > would be cleaner.  I don't know how to figure out the correct device,
> > but that would solve the "BAR changed" problem, and it would have
> > cleaner ownership semantics, too.
> 
> If the bus layout is static, the efi init code can record the 
> segment/bus/device and match it with the kernel's view. is that guaranteed to 
> be sufficient for all implementations of pci?

In theory, no.  The kernel can reassign bus numbers to make space for
hotplug devices.  In practice, they're like BARs: if the firmware sets
them up, we don't normally change them unless they're obviously
broken.

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

[Bjorn Helgaas]

On Fri, Apr 29, 2016 at 10:25:13PM +0200, Ard Biesheuvel wrote:
> 
> > On 29 apr. 2016, at 22:06, Bjorn Helgaas  wrote:
> > 
> >> On Fri, Apr 29, 2016 at 03:51:49PM +0200, Ard Biesheuvel wrote:
> >>> On 29 April 2016 at 15:41, Bjorn Helgaas  wrote:
>  On Thu, Apr 28, 2016 at 11:39:35PM +0200, Alexander Graf wrote:
>  On 28.04.16 20:06, Bjorn Helgaas wrote:
> > 
> > If firmware is giving us a bare address of something, that seems like
> > a clue that it might depend on that address staying the same.
>  
>  Well, I'd look at it from the other side. It gives us a correct address
>  on entry with the system configured at exactly the state it's in on
>  entry. If Linux changes the system, some guarantees obviously don't work
>  anymore.
> >>> 
> >>> Can you point me to the part of the EFI spec that communicates this?
> >>> I'm curious what the intent is and whether there's any indication that
> >>> EFI expects the OS to preserve some configuration.  I don't think it's
> >>> reasonable for the OS to preserve this sort of configuration because
> >>> it limits how well we can support hotplug.
> >>> 
> >>> I wonder if we're using this frame buffer address as more than what
> >>> EFI intended.  For example, maybe it was intended for use by an early
> >>> console driver, but there's some other mechanism we should be using
> >>> after that.
> >> 
> >> The UEFI spec describes this as follows (UEFIv2.6 section 11.9)
> >> 
> >> """
> >> Graphics output may also be required as part of the startup of an
> >> operating system. There are
> >> potentially times in modern operating systems prior to the loading of
> >> a high performance OS
> >> graphics driver where access to graphics output device is required.
> >> The Graphics Output Protocol
> >> supports this capability by providing the EFI OS loader access to a
> >> hardware frame buffer and
> >> enough information to allow the OS to draw directly to the graphics
> >> output device.
> >> """
> >> 
> >> So the intent is to provide minimal framebuffer services until the
> >> 'real' driver takes over.
> > 
> > Makes sense.  A 'real' driver for a PCI device would use
> > pci_register_driver() and use pci_resource_start() or similar to
> > locate the framebuffer, which would avoid the problem because the PCI
> > core doesn't change BARs while a driver owns the device.
> > 
> >> The GOP protocol only describes the base and size of the framebuffer,
> >> and the pixel format. At boot time, the early UEFI code in the kernel
> >> could potentially figure out which PCI device it is related to, if
> >> necessary, but i am not sure if this would solve the x86 case as well.
> > 
> > Does drivers/video/fbdev/efifb.c support only a single framebuffer
> > device?  If a system has several, how does it decide which to use?  I
> > assume UEFI would provide GOP for all the framebuffers?
> > 
> 
> Yes. The early efi code iterates over all gop instances to figure out which 
> one is connected to the efi console.
> 
> > If we could fix this by making efifb claim a PCI device, I think that
> > would be cleaner.  I don't know how to figure out the correct device,
> > but that would solve the "BAR changed" problem, and it would have
> > cleaner ownership semantics, too.
> 
> If the bus layout is static, the efi init code can record the 
> segment/bus/device and match it with the kernel's view. is that guaranteed to 
> be sufficient for all implementations of pci?

In theory, no.  The kernel can reassign bus numbers to make space for
hotplug devices.  In practice, they're like BARs: if the firmware sets
them up, we don't normally change them unless they're obviously
broken.

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

[Alexander Graf]

On 29.04.16 22:25, Ard Biesheuvel wrote:
> 
>> On 29 apr. 2016, at 22:06, Bjorn Helgaas  wrote:
>>
>>> On Fri, Apr 29, 2016 at 03:51:49PM +0200, Ard Biesheuvel wrote:
 On 29 April 2016 at 15:41, Bjorn Helgaas  wrote:
> On Thu, Apr 28, 2016 at 11:39:35PM +0200, Alexander Graf wrote:
> On 28.04.16 20:06, Bjorn Helgaas wrote:
>>
>> If firmware is giving us a bare address of something, that seems like
>> a clue that it might depend on that address staying the same.
>
> Well, I'd look at it from the other side. It gives us a correct address
> on entry with the system configured at exactly the state it's in on
> entry. If Linux changes the system, some guarantees obviously don't work
> anymore.

 Can you point me to the part of the EFI spec that communicates this?
 I'm curious what the intent is and whether there's any indication that
 EFI expects the OS to preserve some configuration.  I don't think it's
 reasonable for the OS to preserve this sort of configuration because
 it limits how well we can support hotplug.

 I wonder if we're using this frame buffer address as more than what
 EFI intended.  For example, maybe it was intended for use by an early
 console driver, but there's some other mechanism we should be using
 after that.
>>>
>>> The UEFI spec describes this as follows (UEFIv2.6 section 11.9)
>>>
>>> """
>>> Graphics output may also be required as part of the startup of an
>>> operating system. There are
>>> potentially times in modern operating systems prior to the loading of
>>> a high performance OS
>>> graphics driver where access to graphics output device is required.
>>> The Graphics Output Protocol
>>> supports this capability by providing the EFI OS loader access to a
>>> hardware frame buffer and
>>> enough information to allow the OS to draw directly to the graphics
>>> output device.
>>> """
>>>
>>> So the intent is to provide minimal framebuffer services until the
>>> 'real' driver takes over.
>>
>> Makes sense.  A 'real' driver for a PCI device would use
>> pci_register_driver() and use pci_resource_start() or similar to
>> locate the framebuffer, which would avoid the problem because the PCI
>> core doesn't change BARs while a driver owns the device.
>>
>>> The GOP protocol only describes the base and size of the framebuffer,
>>> and the pixel format. At boot time, the early UEFI code in the kernel
>>> could potentially figure out which PCI device it is related to, if
>>> necessary, but i am not sure if this would solve the x86 case as well.
>>
>> Does drivers/video/fbdev/efifb.c support only a single framebuffer
>> device?  If a system has several, how does it decide which to use?  I
>> assume UEFI would provide GOP for all the framebuffers?
>>
> 
> Yes. The early efi code iterates over all gop instances to figure out which 
> one is connected to the efi console.
> 
>> If we could fix this by making efifb claim a PCI device, I think that
>> would be cleaner.  I don't know how to figure out the correct device,
>> but that would solve the "BAR changed" problem, and it would have
>> cleaner ownership semantics, too.
> 
> If the bus layout is static, the efi init code can record the 
> segment/bus/device and match it with the kernel's view. is that guaranteed to 
> be sufficient for all implementations of pci?

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?


Alex

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

[Alexander Graf]

On 29.04.16 22:25, Ard Biesheuvel wrote:
> 
>> On 29 apr. 2016, at 22:06, Bjorn Helgaas  wrote:
>>
>>> On Fri, Apr 29, 2016 at 03:51:49PM +0200, Ard Biesheuvel wrote:
 On 29 April 2016 at 15:41, Bjorn Helgaas  wrote:
> On Thu, Apr 28, 2016 at 11:39:35PM +0200, Alexander Graf wrote:
> On 28.04.16 20:06, Bjorn Helgaas wrote:
>>
>> If firmware is giving us a bare address of something, that seems like
>> a clue that it might depend on that address staying the same.
>
> Well, I'd look at it from the other side. It gives us a correct address
> on entry with the system configured at exactly the state it's in on
> entry. If Linux changes the system, some guarantees obviously don't work
> anymore.

 Can you point me to the part of the EFI spec that communicates this?
 I'm curious what the intent is and whether there's any indication that
 EFI expects the OS to preserve some configuration.  I don't think it's
 reasonable for the OS to preserve this sort of configuration because
 it limits how well we can support hotplug.

 I wonder if we're using this frame buffer address as more than what
 EFI intended.  For example, maybe it was intended for use by an early
 console driver, but there's some other mechanism we should be using
 after that.
>>>
>>> The UEFI spec describes this as follows (UEFIv2.6 section 11.9)
>>>
>>> """
>>> Graphics output may also be required as part of the startup of an
>>> operating system. There are
>>> potentially times in modern operating systems prior to the loading of
>>> a high performance OS
>>> graphics driver where access to graphics output device is required.
>>> The Graphics Output Protocol
>>> supports this capability by providing the EFI OS loader access to a
>>> hardware frame buffer and
>>> enough information to allow the OS to draw directly to the graphics
>>> output device.
>>> """
>>>
>>> So the intent is to provide minimal framebuffer services until the
>>> 'real' driver takes over.
>>
>> Makes sense.  A 'real' driver for a PCI device would use
>> pci_register_driver() and use pci_resource_start() or similar to
>> locate the framebuffer, which would avoid the problem because the PCI
>> core doesn't change BARs while a driver owns the device.
>>
>>> The GOP protocol only describes the base and size of the framebuffer,
>>> and the pixel format. At boot time, the early UEFI code in the kernel
>>> could potentially figure out which PCI device it is related to, if
>>> necessary, but i am not sure if this would solve the x86 case as well.
>>
>> Does drivers/video/fbdev/efifb.c support only a single framebuffer
>> device?  If a system has several, how does it decide which to use?  I
>> assume UEFI would provide GOP for all the framebuffers?
>>
> 
> Yes. The early efi code iterates over all gop instances to figure out which 
> one is connected to the efi console.
> 
>> If we could fix this by making efifb claim a PCI device, I think that
>> would be cleaner.  I don't know how to figure out the correct device,
>> but that would solve the "BAR changed" problem, and it would have
>> cleaner ownership semantics, too.
> 
> If the bus layout is static, the efi init code can record the 
> segment/bus/device and match it with the kernel's view. is that guaranteed to 
> be sufficient for all implementations of pci?

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?


Alex

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

[Ard Biesheuvel]

> On 29 apr. 2016, at 22:06, Bjorn Helgaas  wrote:
> 
>> On Fri, Apr 29, 2016 at 03:51:49PM +0200, Ard Biesheuvel wrote:
>>> On 29 April 2016 at 15:41, Bjorn Helgaas  wrote:
 On Thu, Apr 28, 2016 at 11:39:35PM +0200, Alexander Graf wrote:
 On 28.04.16 20:06, Bjorn Helgaas wrote:
> 
> If firmware is giving us a bare address of something, that seems like
> a clue that it might depend on that address staying the same.
 
 Well, I'd look at it from the other side. It gives us a correct address
 on entry with the system configured at exactly the state it's in on
 entry. If Linux changes the system, some guarantees obviously don't work
 anymore.
>>> 
>>> Can you point me to the part of the EFI spec that communicates this?
>>> I'm curious what the intent is and whether there's any indication that
>>> EFI expects the OS to preserve some configuration.  I don't think it's
>>> reasonable for the OS to preserve this sort of configuration because
>>> it limits how well we can support hotplug.
>>> 
>>> I wonder if we're using this frame buffer address as more than what
>>> EFI intended.  For example, maybe it was intended for use by an early
>>> console driver, but there's some other mechanism we should be using
>>> after that.
>> 
>> The UEFI spec describes this as follows (UEFIv2.6 section 11.9)
>> 
>> """
>> Graphics output may also be required as part of the startup of an
>> operating system. There are
>> potentially times in modern operating systems prior to the loading of
>> a high performance OS
>> graphics driver where access to graphics output device is required.
>> The Graphics Output Protocol
>> supports this capability by providing the EFI OS loader access to a
>> hardware frame buffer and
>> enough information to allow the OS to draw directly to the graphics
>> output device.
>> """
>> 
>> So the intent is to provide minimal framebuffer services until the
>> 'real' driver takes over.
> 
> Makes sense.  A 'real' driver for a PCI device would use
> pci_register_driver() and use pci_resource_start() or similar to
> locate the framebuffer, which would avoid the problem because the PCI
> core doesn't change BARs while a driver owns the device.
> 
>> The GOP protocol only describes the base and size of the framebuffer,
>> and the pixel format. At boot time, the early UEFI code in the kernel
>> could potentially figure out which PCI device it is related to, if
>> necessary, but i am not sure if this would solve the x86 case as well.
> 
> Does drivers/video/fbdev/efifb.c support only a single framebuffer
> device?  If a system has several, how does it decide which to use?  I
> assume UEFI would provide GOP for all the framebuffers?
> 

Yes. The early efi code iterates over all gop instances to figure out which one 
is connected to the efi console.

> If we could fix this by making efifb claim a PCI device, I think that
> would be cleaner.  I don't know how to figure out the correct device,
> but that would solve the "BAR changed" problem, and it would have
> cleaner ownership semantics, too.

If the bus layout is static, the efi init code can record the 
segment/bus/device and match it with the kernel's view. is that guaranteed to 
be sufficient for all implementations of pci?

> It looks like the current situation is that a device-specific driver
> (radeon, i915, etc.) could claim the device via the usual
> pci_register_driver() path, and the efifb driver could think it owns
> the device at the same time.  This seems like too obvious a problem,
> so maybe there's some ad hoc mechanism that resolves this?

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

[Ard Biesheuvel]

> On 29 apr. 2016, at 22:06, Bjorn Helgaas  wrote:
> 
>> On Fri, Apr 29, 2016 at 03:51:49PM +0200, Ard Biesheuvel wrote:
>>> On 29 April 2016 at 15:41, Bjorn Helgaas  wrote:
 On Thu, Apr 28, 2016 at 11:39:35PM +0200, Alexander Graf wrote:
 On 28.04.16 20:06, Bjorn Helgaas wrote:
> 
> If firmware is giving us a bare address of something, that seems like
> a clue that it might depend on that address staying the same.
 
 Well, I'd look at it from the other side. It gives us a correct address
 on entry with the system configured at exactly the state it's in on
 entry. If Linux changes the system, some guarantees obviously don't work
 anymore.
>>> 
>>> Can you point me to the part of the EFI spec that communicates this?
>>> I'm curious what the intent is and whether there's any indication that
>>> EFI expects the OS to preserve some configuration.  I don't think it's
>>> reasonable for the OS to preserve this sort of configuration because
>>> it limits how well we can support hotplug.
>>> 
>>> I wonder if we're using this frame buffer address as more than what
>>> EFI intended.  For example, maybe it was intended for use by an early
>>> console driver, but there's some other mechanism we should be using
>>> after that.
>> 
>> The UEFI spec describes this as follows (UEFIv2.6 section 11.9)
>> 
>> """
>> Graphics output may also be required as part of the startup of an
>> operating system. There are
>> potentially times in modern operating systems prior to the loading of
>> a high performance OS
>> graphics driver where access to graphics output device is required.
>> The Graphics Output Protocol
>> supports this capability by providing the EFI OS loader access to a
>> hardware frame buffer and
>> enough information to allow the OS to draw directly to the graphics
>> output device.
>> """
>> 
>> So the intent is to provide minimal framebuffer services until the
>> 'real' driver takes over.
> 
> Makes sense.  A 'real' driver for a PCI device would use
> pci_register_driver() and use pci_resource_start() or similar to
> locate the framebuffer, which would avoid the problem because the PCI
> core doesn't change BARs while a driver owns the device.
> 
>> The GOP protocol only describes the base and size of the framebuffer,
>> and the pixel format. At boot time, the early UEFI code in the kernel
>> could potentially figure out which PCI device it is related to, if
>> necessary, but i am not sure if this would solve the x86 case as well.
> 
> Does drivers/video/fbdev/efifb.c support only a single framebuffer
> device?  If a system has several, how does it decide which to use?  I
> assume UEFI would provide GOP for all the framebuffers?
> 

Yes. The early efi code iterates over all gop instances to figure out which one 
is connected to the efi console.

> If we could fix this by making efifb claim a PCI device, I think that
> would be cleaner.  I don't know how to figure out the correct device,
> but that would solve the "BAR changed" problem, and it would have
> cleaner ownership semantics, too.

If the bus layout is static, the efi init code can record the 
segment/bus/device and match it with the kernel's view. is that guaranteed to 
be sufficient for all implementations of pci?

> It looks like the current situation is that a device-specific driver
> (radeon, i915, etc.) could claim the device via the usual
> pci_register_driver() path, and the efifb driver could think it owns
> the device at the same time.  This seems like too obvious a problem,
> so maybe there's some ad hoc mechanism that resolves this?

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

[Bjorn Helgaas]

On Fri, Apr 29, 2016 at 03:51:49PM +0200, Ard Biesheuvel wrote:
> On 29 April 2016 at 15:41, Bjorn Helgaas  wrote:
> > On Thu, Apr 28, 2016 at 11:39:35PM +0200, Alexander Graf wrote:
> >> On 28.04.16 20:06, Bjorn Helgaas wrote:

> >> > If firmware is giving us a bare address of something, that seems like
> >> > a clue that it might depend on that address staying the same.
> >>
> >> Well, I'd look at it from the other side. It gives us a correct address
> >> on entry with the system configured at exactly the state it's in on
> >> entry. If Linux changes the system, some guarantees obviously don't work
> >> anymore.
> >
> > Can you point me to the part of the EFI spec that communicates this?
> > I'm curious what the intent is and whether there's any indication that
> > EFI expects the OS to preserve some configuration.  I don't think it's
> > reasonable for the OS to preserve this sort of configuration because
> > it limits how well we can support hotplug.
> >
> > I wonder if we're using this frame buffer address as more than what
> > EFI intended.  For example, maybe it was intended for use by an early
> > console driver, but there's some other mechanism we should be using
> > after that.
> >
> 
> The UEFI spec describes this as follows (UEFIv2.6 section 11.9)
> 
> """
> Graphics output may also be required as part of the startup of an
> operating system. There are
> potentially times in modern operating systems prior to the loading of
> a high performance OS
> graphics driver where access to graphics output device is required.
> The Graphics Output Protocol
> supports this capability by providing the EFI OS loader access to a
> hardware frame buffer and
> enough information to allow the OS to draw directly to the graphics
> output device.
> """
> 
> So the intent is to provide minimal framebuffer services until the
> 'real' driver takes over.

Makes sense.  A 'real' driver for a PCI device would use
pci_register_driver() and use pci_resource_start() or similar to
locate the framebuffer, which would avoid the problem because the PCI
core doesn't change BARs while a driver owns the device.

> The GOP protocol only describes the base and size of the framebuffer,
> and the pixel format. At boot time, the early UEFI code in the kernel
> could potentially figure out which PCI device it is related to, if
> necessary, but i am not sure if this would solve the x86 case as well.

Does drivers/video/fbdev/efifb.c support only a single framebuffer
device?  If a system has several, how does it decide which to use?  I
assume UEFI would provide GOP for all the framebuffers?

If we could fix this by making efifb claim a PCI device, I think that
would be cleaner.  I don't know how to figure out the correct device,
but that would solve the "BAR changed" problem, and it would have
cleaner ownership semantics, too.

It looks like the current situation is that a device-specific driver
(radeon, i915, etc.) could claim the device via the usual
pci_register_driver() path, and the efifb driver could think it owns
the device at the same time.  This seems like too obvious a problem,
so maybe there's some ad hoc mechanism that resolves this?

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

[Bjorn Helgaas]

On Fri, Apr 29, 2016 at 03:51:49PM +0200, Ard Biesheuvel wrote:
> On 29 April 2016 at 15:41, Bjorn Helgaas  wrote:
> > On Thu, Apr 28, 2016 at 11:39:35PM +0200, Alexander Graf wrote:
> >> On 28.04.16 20:06, Bjorn Helgaas wrote:

> >> > If firmware is giving us a bare address of something, that seems like
> >> > a clue that it might depend on that address staying the same.
> >>
> >> Well, I'd look at it from the other side. It gives us a correct address
> >> on entry with the system configured at exactly the state it's in on
> >> entry. If Linux changes the system, some guarantees obviously don't work
> >> anymore.
> >
> > Can you point me to the part of the EFI spec that communicates this?
> > I'm curious what the intent is and whether there's any indication that
> > EFI expects the OS to preserve some configuration.  I don't think it's
> > reasonable for the OS to preserve this sort of configuration because
> > it limits how well we can support hotplug.
> >
> > I wonder if we're using this frame buffer address as more than what
> > EFI intended.  For example, maybe it was intended for use by an early
> > console driver, but there's some other mechanism we should be using
> > after that.
> >
> 
> The UEFI spec describes this as follows (UEFIv2.6 section 11.9)
> 
> """
> Graphics output may also be required as part of the startup of an
> operating system. There are
> potentially times in modern operating systems prior to the loading of
> a high performance OS
> graphics driver where access to graphics output device is required.
> The Graphics Output Protocol
> supports this capability by providing the EFI OS loader access to a
> hardware frame buffer and
> enough information to allow the OS to draw directly to the graphics
> output device.
> """
> 
> So the intent is to provide minimal framebuffer services until the
> 'real' driver takes over.

Makes sense.  A 'real' driver for a PCI device would use
pci_register_driver() and use pci_resource_start() or similar to
locate the framebuffer, which would avoid the problem because the PCI
core doesn't change BARs while a driver owns the device.

> The GOP protocol only describes the base and size of the framebuffer,
> and the pixel format. At boot time, the early UEFI code in the kernel
> could potentially figure out which PCI device it is related to, if
> necessary, but i am not sure if this would solve the x86 case as well.

Does drivers/video/fbdev/efifb.c support only a single framebuffer
device?  If a system has several, how does it decide which to use?  I
assume UEFI would provide GOP for all the framebuffers?

If we could fix this by making efifb claim a PCI device, I think that
would be cleaner.  I don't know how to figure out the correct device,
but that would solve the "BAR changed" problem, and it would have
cleaner ownership semantics, too.

It looks like the current situation is that a device-specific driver
(radeon, i915, etc.) could claim the device via the usual
pci_register_driver() path, and the efifb driver could think it owns
the device at the same time.  This seems like too obvious a problem,
so maybe there's some ad hoc mechanism that resolves this?

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

[Ard Biesheuvel]

On 29 April 2016 at 15:41, Bjorn Helgaas  wrote:
> On Thu, Apr 28, 2016 at 11:39:35PM +0200, Alexander Graf wrote:
>> On 28.04.16 20:06, Bjorn Helgaas wrote:
>> > On Thu, Apr 28, 2016 at 06:41:42PM +0200, Alexander Graf wrote:
>> >> On 04/28/2016 06:20 PM, Bjorn Helgaas wrote:
>> >>> On Thu, Apr 28, 2016 at 12:22:24AM +0200, Alexander Graf wrote:
>>  When booting with efifb, we get a frame buffer address passed into the 
>>  system.
>>  This address can be backed by any device, including PCI devices.
>> >>> I guess we get the frame buffer address via EFI, but it doesn't tell
>> >>> us what PCI device it's connected to?
>> >>
>> >> Pretty much, yes. We can get the frame buffer address from a
>> >> multitude of sources using various boot protocols, but the case
>> >> where I ran into this was with efi on arm64.
>> >>
>> >>> This same thing could happen on any EFI arch, I guess.  Maybe even on
>> >>
>> >> Yes and no :). I would've put it into whatever code "owns"
>> >> screen_info, but I couldn't find any. So instead I figured I'd make
>> >> the approach as generic as I could and implemented the calculation
>> >> for the case where I saw it break.
>> >>
>> >> The reason we don't see this on x86 (if I understand all pieces of
>> >> the puzzle correctly) is that we get the BAR allocation from
>> >> firmware using _CRS attributes in ACPI, so firmware tells the OS
>> >> where to put the BARs.
>> >
>> > I think the real reason is that on x86, firmware typically assigns all
>> > the BARs and Linux typically doesn't change them.  PCI host bridges
>>
>> Can you point me to the code that "doesn't change them"? I couldn't find
>> it, but I haven't see Linux reallocate BARs on x86.
>
> Lorenzo already answered this, I think.  I'll just reiterate that all
> we can really do is check whether a BAR's current value is inside the
> upstream bridge aperture.  If it is, we assume the BAR has been
> assigned and we try to use that assignment unchanged.  Zero is a valid
> BAR value, so we can't just check for something non-zero.
>
>> >> In the device tree case (which is what I'm
>> >> running on arm64) we however allocate BARs dynamically.
>
> Side note, from a PCI core point of view, this is not a DT vs. ACPI
> issue.  It's just a question of whether the BARs have been assigned
> already, which might appear to correlate with DT or ACPI, but AFAIK
> it's outside the scope of those specs.
>

That is true, but the distinction is made because ACPI implies UEFI on
arm64, so there we know there is firmware that executes before the
kernel.

A DT kernel could theoretically boot almost straight out of reset
(i.e., on a system which does not implement the security extensions),
which is most likely the historic explanation of why PCI on ARM
assumes that the PCI subsystem needs to be configured from scratch.

>> >>> ... if there's a way to discover the frame buffer address


>> >>> as a bare address rather than a "offset X into BAR Y of PCI device Z"
>> >>> sort of thing.
>> >>
>> >> It'd be perfectly doable today - we do get a cpu physical address
>> >> and use that in the notifier. All we would need to do is move the
>> >> code that I added in arm64/efi.c to something more generic that
>> >> "owns" the frame buffer address. Then any boot protocol that passes
>> >> a screen_info in would get the frame buffer relocated on BAR remap.
>> >
>> > We could consider a quirk that would mark any BAR that happened to
>> > contain the frame buffer address as IORESOURCE_PCI_FIXED.  That would
>> > (in theory, anyway) keep the PCI core from moving it.
>>
>> That's what I thought I should do at first. Then I realized that we
>> could have a PCIe GPU in the system that provides a really big BAR which
>> we would need to map into an mmio64 region to make full use of it.
>> Firmware however - because of limitations - only maps it into the mmio32
>> space though.
>>
>> That means we now break a case that would work without efifb, right?
>
> I'm not sure I understand.  Are you saying you might have, say, a 2GB
> BAR, and firmware might put it in an mmio32 1GB host bridge aperture?
> I guess you *could* program the BAR that way, but obviously a driver
> would only be able to see the first 1GB of the BAR.
>
> Linux would consider that invalid because the BAR doesn't fit in the
> aperture and would reassign it.  But I don't think I understand the
> whole picture.
>
>> > If firmware is giving us a bare address of something, that seems like
>> > a clue that it might depend on that address staying the same.
>>
>> Well, I'd look at it from the other side. It gives us a correct address
>> on entry with the system configured at exactly the state it's in on
>> entry. If Linux changes the system, some guarantees obviously don't work
>> anymore.
>
> Can you point me to the part of the EFI spec that communicates this?
> I'm curious what the intent is and whether there's any indication that
> EFI expects the OS to preserve some 

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

[Ard Biesheuvel]

On 29 April 2016 at 15:41, Bjorn Helgaas  wrote:
> On Thu, Apr 28, 2016 at 11:39:35PM +0200, Alexander Graf wrote:
>> On 28.04.16 20:06, Bjorn Helgaas wrote:
>> > On Thu, Apr 28, 2016 at 06:41:42PM +0200, Alexander Graf wrote:
>> >> On 04/28/2016 06:20 PM, Bjorn Helgaas wrote:
>> >>> On Thu, Apr 28, 2016 at 12:22:24AM +0200, Alexander Graf wrote:
>>  When booting with efifb, we get a frame buffer address passed into the 
>>  system.
>>  This address can be backed by any device, including PCI devices.
>> >>> I guess we get the frame buffer address via EFI, but it doesn't tell
>> >>> us what PCI device it's connected to?
>> >>
>> >> Pretty much, yes. We can get the frame buffer address from a
>> >> multitude of sources using various boot protocols, but the case
>> >> where I ran into this was with efi on arm64.
>> >>
>> >>> This same thing could happen on any EFI arch, I guess.  Maybe even on
>> >>
>> >> Yes and no :). I would've put it into whatever code "owns"
>> >> screen_info, but I couldn't find any. So instead I figured I'd make
>> >> the approach as generic as I could and implemented the calculation
>> >> for the case where I saw it break.
>> >>
>> >> The reason we don't see this on x86 (if I understand all pieces of
>> >> the puzzle correctly) is that we get the BAR allocation from
>> >> firmware using _CRS attributes in ACPI, so firmware tells the OS
>> >> where to put the BARs.
>> >
>> > I think the real reason is that on x86, firmware typically assigns all
>> > the BARs and Linux typically doesn't change them.  PCI host bridges
>>
>> Can you point me to the code that "doesn't change them"? I couldn't find
>> it, but I haven't see Linux reallocate BARs on x86.
>
> Lorenzo already answered this, I think.  I'll just reiterate that all
> we can really do is check whether a BAR's current value is inside the
> upstream bridge aperture.  If it is, we assume the BAR has been
> assigned and we try to use that assignment unchanged.  Zero is a valid
> BAR value, so we can't just check for something non-zero.
>
>> >> In the device tree case (which is what I'm
>> >> running on arm64) we however allocate BARs dynamically.
>
> Side note, from a PCI core point of view, this is not a DT vs. ACPI
> issue.  It's just a question of whether the BARs have been assigned
> already, which might appear to correlate with DT or ACPI, but AFAIK
> it's outside the scope of those specs.
>

That is true, but the distinction is made because ACPI implies UEFI on
arm64, so there we know there is firmware that executes before the
kernel.

A DT kernel could theoretically boot almost straight out of reset
(i.e., on a system which does not implement the security extensions),
which is most likely the historic explanation of why PCI on ARM
assumes that the PCI subsystem needs to be configured from scratch.

>> >>> ... if there's a way to discover the frame buffer address


>> >>> as a bare address rather than a "offset X into BAR Y of PCI device Z"
>> >>> sort of thing.
>> >>
>> >> It'd be perfectly doable today - we do get a cpu physical address
>> >> and use that in the notifier. All we would need to do is move the
>> >> code that I added in arm64/efi.c to something more generic that
>> >> "owns" the frame buffer address. Then any boot protocol that passes
>> >> a screen_info in would get the frame buffer relocated on BAR remap.
>> >
>> > We could consider a quirk that would mark any BAR that happened to
>> > contain the frame buffer address as IORESOURCE_PCI_FIXED.  That would
>> > (in theory, anyway) keep the PCI core from moving it.
>>
>> That's what I thought I should do at first. Then I realized that we
>> could have a PCIe GPU in the system that provides a really big BAR which
>> we would need to map into an mmio64 region to make full use of it.
>> Firmware however - because of limitations - only maps it into the mmio32
>> space though.
>>
>> That means we now break a case that would work without efifb, right?
>
> I'm not sure I understand.  Are you saying you might have, say, a 2GB
> BAR, and firmware might put it in an mmio32 1GB host bridge aperture?
> I guess you *could* program the BAR that way, but obviously a driver
> would only be able to see the first 1GB of the BAR.
>
> Linux would consider that invalid because the BAR doesn't fit in the
> aperture and would reassign it.  But I don't think I understand the
> whole picture.
>
>> > If firmware is giving us a bare address of something, that seems like
>> > a clue that it might depend on that address staying the same.
>>
>> Well, I'd look at it from the other side. It gives us a correct address
>> on entry with the system configured at exactly the state it's in on
>> entry. If Linux changes the system, some guarantees obviously don't work
>> anymore.
>
> Can you point me to the part of the EFI spec that communicates this?
> I'm curious what the intent is and whether there's any indication that
> EFI expects the OS to preserve some configuration.  I don't think 

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

[Bjorn Helgaas]

On Thu, Apr 28, 2016 at 11:39:35PM +0200, Alexander Graf wrote:
> On 28.04.16 20:06, Bjorn Helgaas wrote:
> > On Thu, Apr 28, 2016 at 06:41:42PM +0200, Alexander Graf wrote:
> >> On 04/28/2016 06:20 PM, Bjorn Helgaas wrote:
> >>> On Thu, Apr 28, 2016 at 12:22:24AM +0200, Alexander Graf wrote:
>  When booting with efifb, we get a frame buffer address passed into the 
>  system.
>  This address can be backed by any device, including PCI devices.
> >>> I guess we get the frame buffer address via EFI, but it doesn't tell
> >>> us what PCI device it's connected to?
> >>
> >> Pretty much, yes. We can get the frame buffer address from a
> >> multitude of sources using various boot protocols, but the case
> >> where I ran into this was with efi on arm64.
> >>
> >>> This same thing could happen on any EFI arch, I guess.  Maybe even on
> >>
> >> Yes and no :). I would've put it into whatever code "owns"
> >> screen_info, but I couldn't find any. So instead I figured I'd make
> >> the approach as generic as I could and implemented the calculation
> >> for the case where I saw it break.
> >>
> >> The reason we don't see this on x86 (if I understand all pieces of
> >> the puzzle correctly) is that we get the BAR allocation from
> >> firmware using _CRS attributes in ACPI, so firmware tells the OS
> >> where to put the BARs. 
> > 
> > I think the real reason is that on x86, firmware typically assigns all
> > the BARs and Linux typically doesn't change them.  PCI host bridges
> 
> Can you point me to the code that "doesn't change them"? I couldn't find
> it, but I haven't see Linux reallocate BARs on x86.

Lorenzo already answered this, I think.  I'll just reiterate that all
we can really do is check whether a BAR's current value is inside the
upstream bridge aperture.  If it is, we assume the BAR has been
assigned and we try to use that assignment unchanged.  Zero is a valid
BAR value, so we can't just check for something non-zero.

> >> In the device tree case (which is what I'm
> >> running on arm64) we however allocate BARs dynamically.

Side note, from a PCI core point of view, this is not a DT vs. ACPI
issue.  It's just a question of whether the BARs have been assigned
already, which might appear to correlate with DT or ACPI, but AFAIK
it's outside the scope of those specs.

> >>> ... if there's a way to discover the frame buffer address
> >>> as a bare address rather than a "offset X into BAR Y of PCI device Z"
> >>> sort of thing.
> >>
> >> It'd be perfectly doable today - we do get a cpu physical address
> >> and use that in the notifier. All we would need to do is move the
> >> code that I added in arm64/efi.c to something more generic that
> >> "owns" the frame buffer address. Then any boot protocol that passes
> >> a screen_info in would get the frame buffer relocated on BAR remap.
> > 
> > We could consider a quirk that would mark any BAR that happened to
> > contain the frame buffer address as IORESOURCE_PCI_FIXED.  That would
> > (in theory, anyway) keep the PCI core from moving it.
> 
> That's what I thought I should do at first. Then I realized that we
> could have a PCIe GPU in the system that provides a really big BAR which
> we would need to map into an mmio64 region to make full use of it.
> Firmware however - because of limitations - only maps it into the mmio32
> space though.
> 
> That means we now break a case that would work without efifb, right?

I'm not sure I understand.  Are you saying you might have, say, a 2GB
BAR, and firmware might put it in an mmio32 1GB host bridge aperture?
I guess you *could* program the BAR that way, but obviously a driver
would only be able to see the first 1GB of the BAR.

Linux would consider that invalid because the BAR doesn't fit in the
aperture and would reassign it.  But I don't think I understand the
whole picture.

> > If firmware is giving us a bare address of something, that seems like
> > a clue that it might depend on that address staying the same.
> 
> Well, I'd look at it from the other side. It gives us a correct address
> on entry with the system configured at exactly the state it's in on
> entry. If Linux changes the system, some guarantees obviously don't work
> anymore.

Can you point me to the part of the EFI spec that communicates this?
I'm curious what the intent is and whether there's any indication that
EFI expects the OS to preserve some configuration.  I don't think it's
reasonable for the OS to preserve this sort of configuration because
it limits how well we can support hotplug.

I wonder if we're using this frame buffer address as more than what
EFI intended.  For example, maybe it was intended for use by an early
console driver, but there's some other mechanism we should be using
after that.

Bjorn

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

[Bjorn Helgaas]

On Thu, Apr 28, 2016 at 11:39:35PM +0200, Alexander Graf wrote:
> On 28.04.16 20:06, Bjorn Helgaas wrote:
> > On Thu, Apr 28, 2016 at 06:41:42PM +0200, Alexander Graf wrote:
> >> On 04/28/2016 06:20 PM, Bjorn Helgaas wrote:
> >>> On Thu, Apr 28, 2016 at 12:22:24AM +0200, Alexander Graf wrote:
>  When booting with efifb, we get a frame buffer address passed into the 
>  system.
>  This address can be backed by any device, including PCI devices.
> >>> I guess we get the frame buffer address via EFI, but it doesn't tell
> >>> us what PCI device it's connected to?
> >>
> >> Pretty much, yes. We can get the frame buffer address from a
> >> multitude of sources using various boot protocols, but the case
> >> where I ran into this was with efi on arm64.
> >>
> >>> This same thing could happen on any EFI arch, I guess.  Maybe even on
> >>
> >> Yes and no :). I would've put it into whatever code "owns"
> >> screen_info, but I couldn't find any. So instead I figured I'd make
> >> the approach as generic as I could and implemented the calculation
> >> for the case where I saw it break.
> >>
> >> The reason we don't see this on x86 (if I understand all pieces of
> >> the puzzle correctly) is that we get the BAR allocation from
> >> firmware using _CRS attributes in ACPI, so firmware tells the OS
> >> where to put the BARs. 
> > 
> > I think the real reason is that on x86, firmware typically assigns all
> > the BARs and Linux typically doesn't change them.  PCI host bridges
> 
> Can you point me to the code that "doesn't change them"? I couldn't find
> it, but I haven't see Linux reallocate BARs on x86.

Lorenzo already answered this, I think.  I'll just reiterate that all
we can really do is check whether a BAR's current value is inside the
upstream bridge aperture.  If it is, we assume the BAR has been
assigned and we try to use that assignment unchanged.  Zero is a valid
BAR value, so we can't just check for something non-zero.

> >> In the device tree case (which is what I'm
> >> running on arm64) we however allocate BARs dynamically.

Side note, from a PCI core point of view, this is not a DT vs. ACPI
issue.  It's just a question of whether the BARs have been assigned
already, which might appear to correlate with DT or ACPI, but AFAIK
it's outside the scope of those specs.

> >>> ... if there's a way to discover the frame buffer address
> >>> as a bare address rather than a "offset X into BAR Y of PCI device Z"
> >>> sort of thing.
> >>
> >> It'd be perfectly doable today - we do get a cpu physical address
> >> and use that in the notifier. All we would need to do is move the
> >> code that I added in arm64/efi.c to something more generic that
> >> "owns" the frame buffer address. Then any boot protocol that passes
> >> a screen_info in would get the frame buffer relocated on BAR remap.
> > 
> > We could consider a quirk that would mark any BAR that happened to
> > contain the frame buffer address as IORESOURCE_PCI_FIXED.  That would
> > (in theory, anyway) keep the PCI core from moving it.
> 
> That's what I thought I should do at first. Then I realized that we
> could have a PCIe GPU in the system that provides a really big BAR which
> we would need to map into an mmio64 region to make full use of it.
> Firmware however - because of limitations - only maps it into the mmio32
> space though.
> 
> That means we now break a case that would work without efifb, right?

I'm not sure I understand.  Are you saying you might have, say, a 2GB
BAR, and firmware might put it in an mmio32 1GB host bridge aperture?
I guess you *could* program the BAR that way, but obviously a driver
would only be able to see the first 1GB of the BAR.

Linux would consider that invalid because the BAR doesn't fit in the
aperture and would reassign it.  But I don't think I understand the
whole picture.

> > If firmware is giving us a bare address of something, that seems like
> > a clue that it might depend on that address staying the same.
> 
> Well, I'd look at it from the other side. It gives us a correct address
> on entry with the system configured at exactly the state it's in on
> entry. If Linux changes the system, some guarantees obviously don't work
> anymore.

Can you point me to the part of the EFI spec that communicates this?
I'm curious what the intent is and whether there's any indication that
EFI expects the OS to preserve some configuration.  I don't think it's
reasonable for the OS to preserve this sort of configuration because
it limits how well we can support hotplug.

I wonder if we're using this frame buffer address as more than what
EFI intended.  For example, maybe it was intended for use by an early
console driver, but there's some other mechanism we should be using
after that.

Bjorn

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

[Lorenzo Pieralisi]

On Thu, Apr 28, 2016 at 11:39:35PM +0200, Alexander Graf wrote:
> 
> 
> On 28.04.16 20:06, Bjorn Helgaas wrote:
> > On Thu, Apr 28, 2016 at 06:41:42PM +0200, Alexander Graf wrote:
> >> On 04/28/2016 06:20 PM, Bjorn Helgaas wrote:
> >>> On Thu, Apr 28, 2016 at 12:22:24AM +0200, Alexander Graf wrote:
>  When booting with efifb, we get a frame buffer address passed into the 
>  system.
>  This address can be backed by any device, including PCI devices.
> >>> I guess we get the frame buffer address via EFI, but it doesn't tell
> >>> us what PCI device it's connected to?
> >>
> >> Pretty much, yes. We can get the frame buffer address from a
> >> multitude of sources using various boot protocols, but the case
> >> where I ran into this was with efi on arm64.
> >>
> >>> This same thing could happen on any EFI arch, I guess.  Maybe even on
> >>
> >> Yes and no :). I would've put it into whatever code "owns"
> >> screen_info, but I couldn't find any. So instead I figured I'd make
> >> the approach as generic as I could and implemented the calculation
> >> for the case where I saw it break.
> >>
> >> The reason we don't see this on x86 (if I understand all pieces of
> >> the puzzle correctly) is that we get the BAR allocation from
> >> firmware using _CRS attributes in ACPI, so firmware tells the OS
> >> where to put the BARs. 
> > 
> > I think the real reason is that on x86, firmware typically assigns all
> > the BARs and Linux typically doesn't change them.  PCI host bridges
> 
> Can you point me to the code that "doesn't change them"? I couldn't find
> it, but I haven't see Linux reallocate BARs on x86.
> 
> The thing is that if a BAR is already allocated, we could as well not
> remap it on arm as well - but how do we know?

We don't. Long story short: if I understand X86 code correctly,
on X86 PCI resources are claimed at boot:

arch/x86/pci/i386.c (pcibios_resource_survey())

which means that if the BARs are set-up in a way that passes the resource
claiming validation tests (ie the resource fits into the resource tree),
the BAR resources are inserted into the resource tree and are not touched
by the code that reassigns the PCI resources.

Ergo, FW set-up is kept intact, that's my understanding of X86 code.

The other way of preventing a PCI resource to be moved is by marking it
IORESOURCE_PCI_FIXED, I am not sure that's what X86 does in your specific
case though.

> > have _CRS, which tells us where the host bridge windows are.  PCI
> > devices themselves don't normally have _CRS; we just make sure their
> > BARs are inside the ranges of an upstream _CRS.  If/when we get x86
> > boxes where firmware doesn't assign all the BARs, we should see the
> > same problem there.
> 
> So the check is whether all BARs get assigned by firmware?

Eheh, the problem, and I am glad that you raised the point, is how
do we know that FW assigned the BARs ? The only thing we can do is
we try to claim the BAR resource, if it fits into the resource tree
we successfully claim the resource and the kernel won't reassign it.

On ARM PCI resources are never claimed, they are always reassigned,
and that's the reason why you are experiencing these failures.

> >> In the device tree case (which is what I'm
> >> running on arm64) we however allocate BARs dynamically.
> >>
> >>> non-EFI arches, if there's a way to discover the frame buffer address
> >>> as a bare address rather than a "offset X into BAR Y of PCI device Z"
> >>> sort of thing.
> >>
> >> It'd be perfectly doable today - we do get a cpu physical address
> >> and use that in the notifier. All we would need to do is move the
> >> code that I added in arm64/efi.c to something more generic that
> >> "owns" the frame buffer address. Then any boot protocol that passes
> >> a screen_info in would get the frame buffer relocated on BAR remap.
> > 
> > We could consider a quirk that would mark any BAR that happened to
> > contain the frame buffer address as IORESOURCE_PCI_FIXED.  That would
> > (in theory, anyway) keep the PCI core from moving it.
> 
> That's what I thought I should do at first. Then I realized that we
> could have a PCIe GPU in the system that provides a really big BAR which
> we would need to map into an mmio64 region to make full use of it.
> Firmware however - because of limitations - only maps it into the mmio32
> space though.
> 
> That means we now break a case that would work without efifb, right?
> 
> > Is there any run-time EFI (or other firmware) dependency on the frame
> > buffer address?  If there is, things will break when we move it, even
> > if we have your notifier to tell efifb about it.
> 
> Simple answer is no :).
> 
> >> Drivers like vesafb might benefit from this as well - though
> >> apparently x86 fixed this using ACPI.
> > 
> > Where is this x86 vesafb ACPI fix?  I don't see anything ACPI-related
> > in drivers/video/fbdev/vesafb.c.  I'm just curious what this fix looks
> > like.
> 
> I don't know of any - I haven't found the code that 

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

[Lorenzo Pieralisi]

On Thu, Apr 28, 2016 at 11:39:35PM +0200, Alexander Graf wrote:
> 
> 
> On 28.04.16 20:06, Bjorn Helgaas wrote:
> > On Thu, Apr 28, 2016 at 06:41:42PM +0200, Alexander Graf wrote:
> >> On 04/28/2016 06:20 PM, Bjorn Helgaas wrote:
> >>> On Thu, Apr 28, 2016 at 12:22:24AM +0200, Alexander Graf wrote:
>  When booting with efifb, we get a frame buffer address passed into the 
>  system.
>  This address can be backed by any device, including PCI devices.
> >>> I guess we get the frame buffer address via EFI, but it doesn't tell
> >>> us what PCI device it's connected to?
> >>
> >> Pretty much, yes. We can get the frame buffer address from a
> >> multitude of sources using various boot protocols, but the case
> >> where I ran into this was with efi on arm64.
> >>
> >>> This same thing could happen on any EFI arch, I guess.  Maybe even on
> >>
> >> Yes and no :). I would've put it into whatever code "owns"
> >> screen_info, but I couldn't find any. So instead I figured I'd make
> >> the approach as generic as I could and implemented the calculation
> >> for the case where I saw it break.
> >>
> >> The reason we don't see this on x86 (if I understand all pieces of
> >> the puzzle correctly) is that we get the BAR allocation from
> >> firmware using _CRS attributes in ACPI, so firmware tells the OS
> >> where to put the BARs. 
> > 
> > I think the real reason is that on x86, firmware typically assigns all
> > the BARs and Linux typically doesn't change them.  PCI host bridges
> 
> Can you point me to the code that "doesn't change them"? I couldn't find
> it, but I haven't see Linux reallocate BARs on x86.
> 
> The thing is that if a BAR is already allocated, we could as well not
> remap it on arm as well - but how do we know?

We don't. Long story short: if I understand X86 code correctly,
on X86 PCI resources are claimed at boot:

arch/x86/pci/i386.c (pcibios_resource_survey())

which means that if the BARs are set-up in a way that passes the resource
claiming validation tests (ie the resource fits into the resource tree),
the BAR resources are inserted into the resource tree and are not touched
by the code that reassigns the PCI resources.

Ergo, FW set-up is kept intact, that's my understanding of X86 code.

The other way of preventing a PCI resource to be moved is by marking it
IORESOURCE_PCI_FIXED, I am not sure that's what X86 does in your specific
case though.

> > have _CRS, which tells us where the host bridge windows are.  PCI
> > devices themselves don't normally have _CRS; we just make sure their
> > BARs are inside the ranges of an upstream _CRS.  If/when we get x86
> > boxes where firmware doesn't assign all the BARs, we should see the
> > same problem there.
> 
> So the check is whether all BARs get assigned by firmware?

Eheh, the problem, and I am glad that you raised the point, is how
do we know that FW assigned the BARs ? The only thing we can do is
we try to claim the BAR resource, if it fits into the resource tree
we successfully claim the resource and the kernel won't reassign it.

On ARM PCI resources are never claimed, they are always reassigned,
and that's the reason why you are experiencing these failures.

> >> In the device tree case (which is what I'm
> >> running on arm64) we however allocate BARs dynamically.
> >>
> >>> non-EFI arches, if there's a way to discover the frame buffer address
> >>> as a bare address rather than a "offset X into BAR Y of PCI device Z"
> >>> sort of thing.
> >>
> >> It'd be perfectly doable today - we do get a cpu physical address
> >> and use that in the notifier. All we would need to do is move the
> >> code that I added in arm64/efi.c to something more generic that
> >> "owns" the frame buffer address. Then any boot protocol that passes
> >> a screen_info in would get the frame buffer relocated on BAR remap.
> > 
> > We could consider a quirk that would mark any BAR that happened to
> > contain the frame buffer address as IORESOURCE_PCI_FIXED.  That would
> > (in theory, anyway) keep the PCI core from moving it.
> 
> That's what I thought I should do at first. Then I realized that we
> could have a PCIe GPU in the system that provides a really big BAR which
> we would need to map into an mmio64 region to make full use of it.
> Firmware however - because of limitations - only maps it into the mmio32
> space though.
> 
> That means we now break a case that would work without efifb, right?
> 
> > Is there any run-time EFI (or other firmware) dependency on the frame
> > buffer address?  If there is, things will break when we move it, even
> > if we have your notifier to tell efifb about it.
> 
> Simple answer is no :).
> 
> >> Drivers like vesafb might benefit from this as well - though
> >> apparently x86 fixed this using ACPI.
> > 
> > Where is this x86 vesafb ACPI fix?  I don't see anything ACPI-related
> > in drivers/video/fbdev/vesafb.c.  I'm just curious what this fix looks
> > like.
> 
> I don't know of any - I haven't found the code that 

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

[Alexander Graf]

On 28.04.16 20:06, Bjorn Helgaas wrote:
> On Thu, Apr 28, 2016 at 06:41:42PM +0200, Alexander Graf wrote:
>> On 04/28/2016 06:20 PM, Bjorn Helgaas wrote:
>>> On Thu, Apr 28, 2016 at 12:22:24AM +0200, Alexander Graf wrote:
 When booting with efifb, we get a frame buffer address passed into the 
 system.
 This address can be backed by any device, including PCI devices.
>>> I guess we get the frame buffer address via EFI, but it doesn't tell
>>> us what PCI device it's connected to?
>>
>> Pretty much, yes. We can get the frame buffer address from a
>> multitude of sources using various boot protocols, but the case
>> where I ran into this was with efi on arm64.
>>
>>> This same thing could happen on any EFI arch, I guess.  Maybe even on
>>
>> Yes and no :). I would've put it into whatever code "owns"
>> screen_info, but I couldn't find any. So instead I figured I'd make
>> the approach as generic as I could and implemented the calculation
>> for the case where I saw it break.
>>
>> The reason we don't see this on x86 (if I understand all pieces of
>> the puzzle correctly) is that we get the BAR allocation from
>> firmware using _CRS attributes in ACPI, so firmware tells the OS
>> where to put the BARs. 
> 
> I think the real reason is that on x86, firmware typically assigns all
> the BARs and Linux typically doesn't change them.  PCI host bridges

Can you point me to the code that "doesn't change them"? I couldn't find
it, but I haven't see Linux reallocate BARs on x86.

The thing is that if a BAR is already allocated, we could as well not
remap it on arm as well - but how do we know?

> have _CRS, which tells us where the host bridge windows are.  PCI
> devices themselves don't normally have _CRS; we just make sure their
> BARs are inside the ranges of an upstream _CRS.  If/when we get x86
> boxes where firmware doesn't assign all the BARs, we should see the
> same problem there.

So the check is whether all BARs get assigned by firmware?

> 
>> In the device tree case (which is what I'm
>> running on arm64) we however allocate BARs dynamically.
>>
>>> non-EFI arches, if there's a way to discover the frame buffer address
>>> as a bare address rather than a "offset X into BAR Y of PCI device Z"
>>> sort of thing.
>>
>> It'd be perfectly doable today - we do get a cpu physical address
>> and use that in the notifier. All we would need to do is move the
>> code that I added in arm64/efi.c to something more generic that
>> "owns" the frame buffer address. Then any boot protocol that passes
>> a screen_info in would get the frame buffer relocated on BAR remap.
> 
> We could consider a quirk that would mark any BAR that happened to
> contain the frame buffer address as IORESOURCE_PCI_FIXED.  That would
> (in theory, anyway) keep the PCI core from moving it.

That's what I thought I should do at first. Then I realized that we
could have a PCIe GPU in the system that provides a really big BAR which
we would need to map into an mmio64 region to make full use of it.
Firmware however - because of limitations - only maps it into the mmio32
space though.

That means we now break a case that would work without efifb, right?

> Is there any run-time EFI (or other firmware) dependency on the frame
> buffer address?  If there is, things will break when we move it, even
> if we have your notifier to tell efifb about it.

Simple answer is no :).

>> Drivers like vesafb might benefit from this as well - though
>> apparently x86 fixed this using ACPI.
> 
> Where is this x86 vesafb ACPI fix?  I don't see anything ACPI-related
> in drivers/video/fbdev/vesafb.c.  I'm just curious what this fix looks
> like.

I don't know of any - I haven't found the code that would actually
prevent the same thing from happening on x86. Ard pointed to ACPI as the
reason it works there. I couldn't really identify why though.

>> I'm not sure if offb could potentially also break. At the end of the
>> day, it might, depending on how it's backed. For that we would then
>> need another callback, since it doesn't use screen_info.
> 
> If firmware is giving us a bare address of something, that seems like
> a clue that it might depend on that address staying the same.

Well, I'd look at it from the other side. It gives us a correct address
on entry with the system configured at exactly the state it's in on
entry. If Linux changes the system, some guarantees obviously don't work
anymore.

Whether something is still valid for runtime services is a different
question and should get handled differently IMHO. In the EFI case, we
know which memory regions are marked as runtime required. We could reuse
the notifier there too to change the virtual runtime efi page tables to
point to the new BAR address if an RTS MMIO region falls inside a BAR.


Alex

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

[Alexander Graf]

On 28.04.16 20:06, Bjorn Helgaas wrote:
> On Thu, Apr 28, 2016 at 06:41:42PM +0200, Alexander Graf wrote:
>> On 04/28/2016 06:20 PM, Bjorn Helgaas wrote:
>>> On Thu, Apr 28, 2016 at 12:22:24AM +0200, Alexander Graf wrote:
 When booting with efifb, we get a frame buffer address passed into the 
 system.
 This address can be backed by any device, including PCI devices.
>>> I guess we get the frame buffer address via EFI, but it doesn't tell
>>> us what PCI device it's connected to?
>>
>> Pretty much, yes. We can get the frame buffer address from a
>> multitude of sources using various boot protocols, but the case
>> where I ran into this was with efi on arm64.
>>
>>> This same thing could happen on any EFI arch, I guess.  Maybe even on
>>
>> Yes and no :). I would've put it into whatever code "owns"
>> screen_info, but I couldn't find any. So instead I figured I'd make
>> the approach as generic as I could and implemented the calculation
>> for the case where I saw it break.
>>
>> The reason we don't see this on x86 (if I understand all pieces of
>> the puzzle correctly) is that we get the BAR allocation from
>> firmware using _CRS attributes in ACPI, so firmware tells the OS
>> where to put the BARs. 
> 
> I think the real reason is that on x86, firmware typically assigns all
> the BARs and Linux typically doesn't change them.  PCI host bridges

Can you point me to the code that "doesn't change them"? I couldn't find
it, but I haven't see Linux reallocate BARs on x86.

The thing is that if a BAR is already allocated, we could as well not
remap it on arm as well - but how do we know?

> have _CRS, which tells us where the host bridge windows are.  PCI
> devices themselves don't normally have _CRS; we just make sure their
> BARs are inside the ranges of an upstream _CRS.  If/when we get x86
> boxes where firmware doesn't assign all the BARs, we should see the
> same problem there.

So the check is whether all BARs get assigned by firmware?

> 
>> In the device tree case (which is what I'm
>> running on arm64) we however allocate BARs dynamically.
>>
>>> non-EFI arches, if there's a way to discover the frame buffer address
>>> as a bare address rather than a "offset X into BAR Y of PCI device Z"
>>> sort of thing.
>>
>> It'd be perfectly doable today - we do get a cpu physical address
>> and use that in the notifier. All we would need to do is move the
>> code that I added in arm64/efi.c to something more generic that
>> "owns" the frame buffer address. Then any boot protocol that passes
>> a screen_info in would get the frame buffer relocated on BAR remap.
> 
> We could consider a quirk that would mark any BAR that happened to
> contain the frame buffer address as IORESOURCE_PCI_FIXED.  That would
> (in theory, anyway) keep the PCI core from moving it.

That's what I thought I should do at first. Then I realized that we
could have a PCIe GPU in the system that provides a really big BAR which
we would need to map into an mmio64 region to make full use of it.
Firmware however - because of limitations - only maps it into the mmio32
space though.

That means we now break a case that would work without efifb, right?

> Is there any run-time EFI (or other firmware) dependency on the frame
> buffer address?  If there is, things will break when we move it, even
> if we have your notifier to tell efifb about it.

Simple answer is no :).

>> Drivers like vesafb might benefit from this as well - though
>> apparently x86 fixed this using ACPI.
> 
> Where is this x86 vesafb ACPI fix?  I don't see anything ACPI-related
> in drivers/video/fbdev/vesafb.c.  I'm just curious what this fix looks
> like.

I don't know of any - I haven't found the code that would actually
prevent the same thing from happening on x86. Ard pointed to ACPI as the
reason it works there. I couldn't really identify why though.

>> I'm not sure if offb could potentially also break. At the end of the
>> day, it might, depending on how it's backed. For that we would then
>> need another callback, since it doesn't use screen_info.
> 
> If firmware is giving us a bare address of something, that seems like
> a clue that it might depend on that address staying the same.

Well, I'd look at it from the other side. It gives us a correct address
on entry with the system configured at exactly the state it's in on
entry. If Linux changes the system, some guarantees obviously don't work
anymore.

Whether something is still valid for runtime services is a different
question and should get handled differently IMHO. In the EFI case, we
know which memory regions are marked as runtime required. We could reuse
the notifier there too to change the virtual runtime efi page tables to
point to the new BAR address if an RTS MMIO region falls inside a BAR.


Alex

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

[Bjorn Helgaas]

On Thu, Apr 28, 2016 at 06:41:42PM +0200, Alexander Graf wrote:
> On 04/28/2016 06:20 PM, Bjorn Helgaas wrote:
> >On Thu, Apr 28, 2016 at 12:22:24AM +0200, Alexander Graf wrote:
> >>When booting with efifb, we get a frame buffer address passed into the 
> >>system.
> >>This address can be backed by any device, including PCI devices.
> >I guess we get the frame buffer address via EFI, but it doesn't tell
> >us what PCI device it's connected to?
> 
> Pretty much, yes. We can get the frame buffer address from a
> multitude of sources using various boot protocols, but the case
> where I ran into this was with efi on arm64.
> 
> >This same thing could happen on any EFI arch, I guess.  Maybe even on
> 
> Yes and no :). I would've put it into whatever code "owns"
> screen_info, but I couldn't find any. So instead I figured I'd make
> the approach as generic as I could and implemented the calculation
> for the case where I saw it break.
> 
> The reason we don't see this on x86 (if I understand all pieces of
> the puzzle correctly) is that we get the BAR allocation from
> firmware using _CRS attributes in ACPI, so firmware tells the OS
> where to put the BARs. 

I think the real reason is that on x86, firmware typically assigns all
the BARs and Linux typically doesn't change them.  PCI host bridges
have _CRS, which tells us where the host bridge windows are.  PCI
devices themselves don't normally have _CRS; we just make sure their
BARs are inside the ranges of an upstream _CRS.  If/when we get x86
boxes where firmware doesn't assign all the BARs, we should see the
same problem there.

> In the device tree case (which is what I'm
> running on arm64) we however allocate BARs dynamically.
> 
> >non-EFI arches, if there's a way to discover the frame buffer address
> >as a bare address rather than a "offset X into BAR Y of PCI device Z"
> >sort of thing.
> 
> It'd be perfectly doable today - we do get a cpu physical address
> and use that in the notifier. All we would need to do is move the
> code that I added in arm64/efi.c to something more generic that
> "owns" the frame buffer address. Then any boot protocol that passes
> a screen_info in would get the frame buffer relocated on BAR remap.

We could consider a quirk that would mark any BAR that happened to
contain the frame buffer address as IORESOURCE_PCI_FIXED.  That would
(in theory, anyway) keep the PCI core from moving it.

Is there any run-time EFI (or other firmware) dependency on the frame
buffer address?  If there is, things will break when we move it, even
if we have your notifier to tell efifb about it.

> Drivers like vesafb might benefit from this as well - though
> apparently x86 fixed this using ACPI.

Where is this x86 vesafb ACPI fix?  I don't see anything ACPI-related
in drivers/video/fbdev/vesafb.c.  I'm just curious what this fix looks
like.

> I'm not sure if offb could potentially also break. At the end of the
> day, it might, depending on how it's backed. For that we would then
> need another callback, since it doesn't use screen_info.

If firmware is giving us a bare address of something, that seems like
a clue that it might depend on that address staying the same.

Bjorn

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

[Bjorn Helgaas]

On Thu, Apr 28, 2016 at 06:41:42PM +0200, Alexander Graf wrote:
> On 04/28/2016 06:20 PM, Bjorn Helgaas wrote:
> >On Thu, Apr 28, 2016 at 12:22:24AM +0200, Alexander Graf wrote:
> >>When booting with efifb, we get a frame buffer address passed into the 
> >>system.
> >>This address can be backed by any device, including PCI devices.
> >I guess we get the frame buffer address via EFI, but it doesn't tell
> >us what PCI device it's connected to?
> 
> Pretty much, yes. We can get the frame buffer address from a
> multitude of sources using various boot protocols, but the case
> where I ran into this was with efi on arm64.
> 
> >This same thing could happen on any EFI arch, I guess.  Maybe even on
> 
> Yes and no :). I would've put it into whatever code "owns"
> screen_info, but I couldn't find any. So instead I figured I'd make
> the approach as generic as I could and implemented the calculation
> for the case where I saw it break.
> 
> The reason we don't see this on x86 (if I understand all pieces of
> the puzzle correctly) is that we get the BAR allocation from
> firmware using _CRS attributes in ACPI, so firmware tells the OS
> where to put the BARs. 

I think the real reason is that on x86, firmware typically assigns all
the BARs and Linux typically doesn't change them.  PCI host bridges
have _CRS, which tells us where the host bridge windows are.  PCI
devices themselves don't normally have _CRS; we just make sure their
BARs are inside the ranges of an upstream _CRS.  If/when we get x86
boxes where firmware doesn't assign all the BARs, we should see the
same problem there.

> In the device tree case (which is what I'm
> running on arm64) we however allocate BARs dynamically.
> 
> >non-EFI arches, if there's a way to discover the frame buffer address
> >as a bare address rather than a "offset X into BAR Y of PCI device Z"
> >sort of thing.
> 
> It'd be perfectly doable today - we do get a cpu physical address
> and use that in the notifier. All we would need to do is move the
> code that I added in arm64/efi.c to something more generic that
> "owns" the frame buffer address. Then any boot protocol that passes
> a screen_info in would get the frame buffer relocated on BAR remap.

We could consider a quirk that would mark any BAR that happened to
contain the frame buffer address as IORESOURCE_PCI_FIXED.  That would
(in theory, anyway) keep the PCI core from moving it.

Is there any run-time EFI (or other firmware) dependency on the frame
buffer address?  If there is, things will break when we move it, even
if we have your notifier to tell efifb about it.

> Drivers like vesafb might benefit from this as well - though
> apparently x86 fixed this using ACPI.

Where is this x86 vesafb ACPI fix?  I don't see anything ACPI-related
in drivers/video/fbdev/vesafb.c.  I'm just curious what this fix looks
like.

> I'm not sure if offb could potentially also break. At the end of the
> day, it might, depending on how it's backed. For that we would then
> need another callback, since it doesn't use screen_info.

If firmware is giving us a bare address of something, that seems like
a clue that it might depend on that address staying the same.

Bjorn

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

[Alexander Graf]

On 04/28/2016 06:20 PM, Bjorn Helgaas wrote:

On Thu, Apr 28, 2016 at 12:22:24AM +0200, Alexander Graf wrote:

When booting with efifb, we get a frame buffer address passed into the system.
This address can be backed by any device, including PCI devices.

I guess we get the frame buffer address via EFI, but it doesn't tell
us what PCI device it's connected to?


Pretty much, yes. We can get the frame buffer address from a multitude 
of sources using various boot protocols, but the case where I ran into 
this was with efi on arm64.



This same thing could happen on any EFI arch, I guess.  Maybe even on


Yes and no :). I would've put it into whatever code "owns" screen_info, 
but I couldn't find any. So instead I figured I'd make the approach as 
generic as I could and implemented the calculation for the case where I 
saw it break.


The reason we don't see this on x86 (if I understand all pieces of the 
puzzle correctly) is that we get the BAR allocation from firmware using 
_CRS attributes in ACPI, so firmware tells the OS where to put the BARs. 
In the device tree case (which is what I'm running on arm64) we however 
allocate BARs dynamically.



non-EFI arches, if there's a way to discover the frame buffer address
as a bare address rather than a "offset X into BAR Y of PCI device Z"
sort of thing.


It'd be perfectly doable today - we do get a cpu physical address and 
use that in the notifier. All we would need to do is move the code that 
I added in arm64/efi.c to something more generic that "owns" the frame 
buffer address. Then any boot protocol that passes a screen_info in 
would get the frame buffer relocated on BAR remap. Drivers like vesafb 
might benefit from this as well - though apparently x86 fixed this using 
ACPI.


I'm not sure if offb could potentially also break. At the end of the 
day, it might, depending on how it's backed. For that we would then need 
another callback, since it doesn't use screen_info.


I don't feel incredibly comfortable preemptively fixing issues people 
haven't seen yet. There's just a good chance we end up breaking more 
than we fix :). But if you like and can point me to a better place for 
the screen_info modification, I'm happy to move it there.



Alex

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

[Alexander Graf]

On 04/28/2016 06:20 PM, Bjorn Helgaas wrote:

On Thu, Apr 28, 2016 at 12:22:24AM +0200, Alexander Graf wrote:

When booting with efifb, we get a frame buffer address passed into the system.
This address can be backed by any device, including PCI devices.

I guess we get the frame buffer address via EFI, but it doesn't tell
us what PCI device it's connected to?


Pretty much, yes. We can get the frame buffer address from a multitude 
of sources using various boot protocols, but the case where I ran into 
this was with efi on arm64.



This same thing could happen on any EFI arch, I guess.  Maybe even on


Yes and no :). I would've put it into whatever code "owns" screen_info, 
but I couldn't find any. So instead I figured I'd make the approach as 
generic as I could and implemented the calculation for the case where I 
saw it break.


The reason we don't see this on x86 (if I understand all pieces of the 
puzzle correctly) is that we get the BAR allocation from firmware using 
_CRS attributes in ACPI, so firmware tells the OS where to put the BARs. 
In the device tree case (which is what I'm running on arm64) we however 
allocate BARs dynamically.



non-EFI arches, if there's a way to discover the frame buffer address
as a bare address rather than a "offset X into BAR Y of PCI device Z"
sort of thing.


It'd be perfectly doable today - we do get a cpu physical address and 
use that in the notifier. All we would need to do is move the code that 
I added in arm64/efi.c to something more generic that "owns" the frame 
buffer address. Then any boot protocol that passes a screen_info in 
would get the frame buffer relocated on BAR remap. Drivers like vesafb 
might benefit from this as well - though apparently x86 fixed this using 
ACPI.


I'm not sure if offb could potentially also break. At the end of the 
day, it might, depending on how it's backed. For that we would then need 
another callback, since it doesn't use screen_info.


I don't feel incredibly comfortable preemptively fixing issues people 
haven't seen yet. There's just a good chance we end up breaking more 
than we fix :). But if you like and can point me to a better place for 
the screen_info modification, I'm happy to move it there.



Alex

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

[Bjorn Helgaas]

On Thu, Apr 28, 2016 at 12:22:24AM +0200, Alexander Graf wrote:
> When booting with efifb, we get a frame buffer address passed into the system.
> This address can be backed by any device, including PCI devices.

I guess we get the frame buffer address via EFI, but it doesn't tell
us what PCI device it's connected to?

This same thing could happen on any EFI arch, I guess.  Maybe even on
non-EFI arches, if there's a way to discover the frame buffer address
as a bare address rather than a "offset X into BAR Y of PCI device Z"
sort of thing.

> PCI devices can have their BARs mapped to various places inside the PCI window
> though. Linux makes use of that on early boot and usually maps PCI BARs 
> wherever
> it thinks makes sense.
> 
> If we now load the efifb driver after that BAR map has happened, the frame
> buffer address we received may be invalid, because it was in a BAR map before
> Linux modified it.
> 
> To work around that issue, this patch introduces a BAR mapping callback that
> gets called every time Linux (re)allocates a BAR. That way our arm64 efi code
> can check whether the frame buffer is inside the old map and adjust it to
> the new one.
> 
> With this and the efifb patches applied, I can successfully see efifb output
> even after Linux remapped BARs.
> 
> Signed-off-by: Alexander Graf 
> ---
>  arch/arm64/kernel/efi.c | 40 +++-
>  drivers/pci/setup-res.c | 29 +
>  include/linux/pci.h |  8 
>  3 files changed, 76 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index 56a76b6..3612110 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -27,6 +27,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -213,6 +214,41 @@ static __init void reserve_regions(void)
>   set_bit(EFI_MEMMAP, );
>  }
>  
> +#ifdef CONFIG_PCI
> +static bool efi_pci_overlaps_efifb(struct pci_bar_update_info *update_info)
> +{
> + /* is the screen_info frame buffer inside the pci BAR? */
> + if (screen_info.lfb_base >= update_info->old_start &&
> + (screen_info.lfb_base + screen_info.lfb_size) <=
> +  (update_info->old_start + update_info->size))
> + return true;
> +
> + return false;
> +}
> +
> +static int efi_pci_notifier(struct notifier_block *self,
> + unsigned long cmd, void *v)
> +{
> + struct pci_bar_update_info *update_info = v;
> +
> + /*
> +  * When we reallocate a BAR that contains our frame buffer, set the
> +  * screen_info base to where it belongs
> +  */
> + if (efi_pci_overlaps_efifb(update_info)) {
> + u64 diff = (update_info->new_start - update_info->old_start);
> + screen_info.lfb_base += diff;
> + }
> +
> + return NOTIFY_OK;
> +}
> +static struct notifier_block efi_pci_notifier_block = {
> + .notifier_call = efi_pci_notifier,
> +};
> +#else
> +#define pci_notify_on_update_resource(a)
> +#endif
> +
>  void __init efi_init_fdt(void *fdt)
>  {
>   struct efi_fdt_params params;
> @@ -246,8 +282,10 @@ void __init efi_init_fdt(void *fdt)
>   reserve_regions();
>   early_memunmap(memmap.map, params.mmap_size);
>  
> - if (screen_info.orig_video_isVGA == VIDEO_TYPE_EFI)
> + if (screen_info.orig_video_isVGA == VIDEO_TYPE_EFI) {
> + pci_notify_on_update_resource(_pci_notifier_block);
>   memblock_reserve(screen_info.lfb_base, screen_info.lfb_size);
> + }
>  }
>  
>  static int __init register_gop_device(void)
> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index 604011e..d5c24fc 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
> @@ -23,8 +23,10 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include "pci.h"
>  
> +static RAW_NOTIFIER_HEAD(bar_update_chain);
>  
>  void pci_update_resource(struct pci_dev *dev, int resno)
>  {
> @@ -35,6 +37,9 @@ void pci_update_resource(struct pci_dev *dev, int resno)
>   int reg;
>   enum pci_bar_type type;
>   struct resource *res = dev->resource + resno;
> + struct pci_bar_update_info update_info;
> + struct pci_bus_region update_reg;
> + struct resource update_res;
>  
>   if (dev->is_virtfn) {
>   dev_warn(>dev, "can't update VF BAR%d\n", resno);
> @@ -77,6 +82,22 @@ void pci_update_resource(struct pci_dev *dev, int resno)
>   }
>  
>   /*
> +  * Fetch the old BAR location from the device, so we can notify
> +  * users of that BAR that its location is changing.
> +  */
> + pci_read_config_dword(dev, reg, );
> + update_reg.start = check & PCI_BASE_ADDRESS_MEM_MASK;
> + if (check & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> + pci_read_config_dword(dev, reg, );
> + update_reg.start |= ((u64)check) << 32;
> + }
> + update_info.size = region.end - region.start;
> 

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

[Bjorn Helgaas]

On Thu, Apr 28, 2016 at 12:22:24AM +0200, Alexander Graf wrote:
> When booting with efifb, we get a frame buffer address passed into the system.
> This address can be backed by any device, including PCI devices.

I guess we get the frame buffer address via EFI, but it doesn't tell
us what PCI device it's connected to?

This same thing could happen on any EFI arch, I guess.  Maybe even on
non-EFI arches, if there's a way to discover the frame buffer address
as a bare address rather than a "offset X into BAR Y of PCI device Z"
sort of thing.

> PCI devices can have their BARs mapped to various places inside the PCI window
> though. Linux makes use of that on early boot and usually maps PCI BARs 
> wherever
> it thinks makes sense.
> 
> If we now load the efifb driver after that BAR map has happened, the frame
> buffer address we received may be invalid, because it was in a BAR map before
> Linux modified it.
> 
> To work around that issue, this patch introduces a BAR mapping callback that
> gets called every time Linux (re)allocates a BAR. That way our arm64 efi code
> can check whether the frame buffer is inside the old map and adjust it to
> the new one.
> 
> With this and the efifb patches applied, I can successfully see efifb output
> even after Linux remapped BARs.
> 
> Signed-off-by: Alexander Graf 
> ---
>  arch/arm64/kernel/efi.c | 40 +++-
>  drivers/pci/setup-res.c | 29 +
>  include/linux/pci.h |  8 
>  3 files changed, 76 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index 56a76b6..3612110 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -27,6 +27,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -213,6 +214,41 @@ static __init void reserve_regions(void)
>   set_bit(EFI_MEMMAP, );
>  }
>  
> +#ifdef CONFIG_PCI
> +static bool efi_pci_overlaps_efifb(struct pci_bar_update_info *update_info)
> +{
> + /* is the screen_info frame buffer inside the pci BAR? */
> + if (screen_info.lfb_base >= update_info->old_start &&
> + (screen_info.lfb_base + screen_info.lfb_size) <=
> +  (update_info->old_start + update_info->size))
> + return true;
> +
> + return false;
> +}
> +
> +static int efi_pci_notifier(struct notifier_block *self,
> + unsigned long cmd, void *v)
> +{
> + struct pci_bar_update_info *update_info = v;
> +
> + /*
> +  * When we reallocate a BAR that contains our frame buffer, set the
> +  * screen_info base to where it belongs
> +  */
> + if (efi_pci_overlaps_efifb(update_info)) {
> + u64 diff = (update_info->new_start - update_info->old_start);
> + screen_info.lfb_base += diff;
> + }
> +
> + return NOTIFY_OK;
> +}
> +static struct notifier_block efi_pci_notifier_block = {
> + .notifier_call = efi_pci_notifier,
> +};
> +#else
> +#define pci_notify_on_update_resource(a)
> +#endif
> +
>  void __init efi_init_fdt(void *fdt)
>  {
>   struct efi_fdt_params params;
> @@ -246,8 +282,10 @@ void __init efi_init_fdt(void *fdt)
>   reserve_regions();
>   early_memunmap(memmap.map, params.mmap_size);
>  
> - if (screen_info.orig_video_isVGA == VIDEO_TYPE_EFI)
> + if (screen_info.orig_video_isVGA == VIDEO_TYPE_EFI) {
> + pci_notify_on_update_resource(_pci_notifier_block);
>   memblock_reserve(screen_info.lfb_base, screen_info.lfb_size);
> + }
>  }
>  
>  static int __init register_gop_device(void)
> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index 604011e..d5c24fc 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
> @@ -23,8 +23,10 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include "pci.h"
>  
> +static RAW_NOTIFIER_HEAD(bar_update_chain);
>  
>  void pci_update_resource(struct pci_dev *dev, int resno)
>  {
> @@ -35,6 +37,9 @@ void pci_update_resource(struct pci_dev *dev, int resno)
>   int reg;
>   enum pci_bar_type type;
>   struct resource *res = dev->resource + resno;
> + struct pci_bar_update_info update_info;
> + struct pci_bus_region update_reg;
> + struct resource update_res;
>  
>   if (dev->is_virtfn) {
>   dev_warn(>dev, "can't update VF BAR%d\n", resno);
> @@ -77,6 +82,22 @@ void pci_update_resource(struct pci_dev *dev, int resno)
>   }
>  
>   /*
> +  * Fetch the old BAR location from the device, so we can notify
> +  * users of that BAR that its location is changing.
> +  */
> + pci_read_config_dword(dev, reg, );
> + update_reg.start = check & PCI_BASE_ADDRESS_MEM_MASK;
> + if (check & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> + pci_read_config_dword(dev, reg, );
> + update_reg.start |= ((u64)check) << 32;
> + }
> + update_info.size = region.end - region.start;
> +