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 <agraf@xxxxxxx>
> ---
>  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 <linux/slab.h>
>  #include <linux/spinlock.h>
>  #include <linux/platform_device.h>
> +#include <linux/pci.h>
>  
>  #include <asm/cacheflush.h>
>  #include <asm/efi.h>
> @@ -213,6 +214,41 @@ static __init void reserve_regions(void)
>  	set_bit(EFI_MEMMAP, &efi.flags);
>  }
>  
> +#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(&efi_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 <linux/ioport.h>
>  #include <linux/cache.h>
>  #include <linux/slab.h>
> +#include <linux/notifier.h>
>  #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->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, &check);
> +	update_reg.start = check & PCI_BASE_ADDRESS_MEM_MASK;
> +	if (check & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> +		pci_read_config_dword(dev, reg, &check);
> +		update_reg.start |= ((u64)check) << 32;
> +	}
> +	update_info.size = region.end - region.start;
> +	update_reg.end = update_reg.start + update_info.size;
> +	pcibios_bus_to_resource(dev->bus, &update_res, &update_reg);
> +	update_info.old_start = update_res.start;
> +	update_info.new_start = res->start;
> +
> +	/*
>  	 * We can't update a 64-bit BAR atomically, so when possible,
>  	 * disable decoding so that a half-updated BAR won't conflict
>  	 * with another device.
> @@ -108,6 +129,14 @@ void pci_update_resource(struct pci_dev *dev, int resno)
>  
>  	if (disable)
>  		pci_write_config_word(dev, PCI_COMMAND, cmd);
> +
> +	/* Tell interested parties that the BAR mapping changed */
> +	raw_notifier_call_chain(&bar_update_chain, 0, &update_info);
> +}
> +
> +int pci_notify_on_update_resource(struct notifier_block *nb)
> +{
> +        return raw_notifier_chain_register(&bar_update_chain, nb);
>  }
>  
>  int pci_claim_resource(struct pci_dev *dev, int resource)
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index c061250..04a430e 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -30,6 +30,7 @@
>  #include <linux/device.h>
>  #include <linux/io.h>
>  #include <linux/resource_ext.h>
> +#include <linux/notifier.h>
>  #include <uapi/linux/pci.h>
>  
>  #include <linux/pci_ids.h>
> @@ -1037,6 +1038,13 @@ int pci_select_bars(struct pci_dev *dev, unsigned long flags);
>  bool pci_device_is_present(struct pci_dev *pdev);
>  void pci_ignore_hotplug(struct pci_dev *dev);
>  
> +struct pci_bar_update_info {
> +	u64 old_start;
> +	u64 new_start;
> +	u64 size;
> +};
> +int pci_notify_on_update_resource(struct notifier_block *nb);
> +
>  /* ROM control related routines */
>  int pci_enable_rom(struct pci_dev *pdev);
>  void pci_disable_rom(struct pci_dev *pdev);
> -- 
> 1.8.5.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html