Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 29 May 2019 09:02:56 -0500
From:   Bjorn Helgaas <helgaas@kernel.org>
To:     Shawn Anastasio <shawn@anastas.io>
Cc:     linux-pci@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
        benh@kernel.crashing.org, paulus@samba.org, mpe@ellerman.id.au,
        sbobroff@linux.ibm.com, xyjxie@linux.vnet.ibm.com,
        rppt@linux.ibm.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 3/3] powerpc/pseries: Allow user-specified PCI
 resource alignment after init
Message-ID: <20190529140256.GC28250@google.com>
References: <20190528040313.35582-1-shawn@anastas.io>
 <20190528040313.35582-4-shawn@anastas.io>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190528040313.35582-4-shawn@anastas.io>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, May 27, 2019 at 11:03:13PM -0500, Shawn Anastasio wrote:
> On pseries, custom PCI resource alignment specified with the commandline
> argument pci=resource_alignment is disabled due to PCI resources being
> managed by the firmware. However, in the case of PCI hotplug the
> resources are managed by the kernel, so custom alignments should be
> honored in these cases. This is done by only honoring custom
> alignments after initial PCI initialization is done, to ensure that
> all devices managed by the firmware are excluded.
> 
> Without this ability, sub-page BARs sometimes get mapped in between
> page boundaries for hotplugged devices and are therefore unusable
> with the VFIO framework. This change allows users to request
> page alignment for devices they wish to access via VFIO using
> the pci=resource_alignment commandline argument.
> 
> In the future, this could be extended to provide page-aligned
> resources by default for hotplugged devices, similar to what is
> done on powernv by commit 382746376993 ("powerpc/powernv: Override
> pcibios_default_alignment() to force PCI devices to be page aligned")
> 
> Signed-off-by: Shawn Anastasio <shawn@anastas.io>
> ---
>  arch/powerpc/include/asm/machdep.h     |  3 +++
>  arch/powerpc/kernel/pci-common.c       |  9 +++++++++
>  arch/powerpc/platforms/pseries/setup.c | 22 ++++++++++++++++++++++
>  3 files changed, 34 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
> index 2fbfaa9176ed..46eb62c0954e 100644
> --- a/arch/powerpc/include/asm/machdep.h
> +++ b/arch/powerpc/include/asm/machdep.h
> @@ -179,6 +179,9 @@ struct machdep_calls {
>  
>  	resource_size_t (*pcibios_default_alignment)(void);
>  
> +	/* Called when determining PCI resource alignment */
> +	int (*pcibios_ignore_alignment_request)(void);
> +
>  #ifdef CONFIG_PCI_IOV
>  	void (*pcibios_fixup_sriov)(struct pci_dev *pdev);
>  	resource_size_t (*pcibios_iov_resource_alignment)(struct pci_dev *, int resno);
> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index ff4b7539cbdf..8e0d73b4c188 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -238,6 +238,15 @@ resource_size_t pcibios_default_alignment(void)
>  	return 0;
>  }
>  
> +int pcibios_ignore_alignment_request(void)
> +{
> +	if (ppc_md.pcibios_ignore_alignment_request)
> +		return ppc_md.pcibios_ignore_alignment_request();
> +
> +	/* Fall back to default method of checking PCI_PROBE_ONLY */
> +	return pci_has_flag(PCI_PROBE_ONLY);
> +}
> +
>  #ifdef CONFIG_PCI_IOV
>  resource_size_t pcibios_iov_resource_alignment(struct pci_dev *pdev, int resno)
>  {
> diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
> index e4f0dfd4ae33..07f03be02afe 100644
> --- a/arch/powerpc/platforms/pseries/setup.c
> +++ b/arch/powerpc/platforms/pseries/setup.c
> @@ -82,6 +82,8 @@ EXPORT_SYMBOL(CMO_PageSize);
>  
>  int fwnmi_active;  /* TRUE if an FWNMI handler is present */
>  
> +static int initial_pci_init_done; /* TRUE if initial pcibios init has completed */
> +
>  static void pSeries_show_cpuinfo(struct seq_file *m)
>  {
>  	struct device_node *root;
> @@ -749,6 +751,23 @@ static resource_size_t pseries_pci_iov_resource_alignment(struct pci_dev *pdev,
>  }
>  #endif
>  
> +static void pseries_after_init(void)
> +{
> +	initial_pci_init_done = 1;
> +}
> +
> +static int pseries_ignore_alignment_request(void)
> +{
> +	if (initial_pci_init_done)
> +		/*
> +		 * Allow custom alignments after init for things
> +		 * like PCI hotplugging.
> +		 */
> +		return 0;

Hmm, if there's any way to avoid this sort of early/late flag, that
would be nicer.

> +
> +	return pci_has_flag(PCI_PROBE_ONLY);
> +}
> +
>  static void __init pSeries_setup_arch(void)
>  {
>  	set_arch_panic_timeout(10, ARCH_PANIC_TIMEOUT);
> @@ -797,6 +816,9 @@ static void __init pSeries_setup_arch(void)
>  	}
>  
>  	ppc_md.pcibios_root_bridge_prepare = pseries_root_bridge_prepare;
> +	ppc_md.pcibios_after_init = pseries_after_init;
> +	ppc_md.pcibios_ignore_alignment_request =
> +		pseries_ignore_alignment_request;
>  }
>  
>  static void pseries_panic(char *str)
> -- 
> 2.20.1
>