Patchwork [v2,1/2] PCI: Fix issue with "pci=disable_acs_redir" parameter being ignored

login
register
mail settings
Submitter Logan Gunthorpe
Date April 10, 2019, 9:05 p.m.
Message ID <20190410210532.4538-2-logang@deltatee.com>
Download mbox | patch
Permalink /patch/770181/
State New
Headers show

Comments

Logan Gunthorpe - April 10, 2019, 9:05 p.m.
In most cases, kmalloc will not be available early in boot when
pci_setup() is called. Thus, the kstrdup call that was added to fix the
__initdata bug with the disable_acs_redir parameter usually returns
NULL. Thus the parameter is discarded and it does not take into effect.

To fix this, we store the string that's in initdata until a initcall
function can allocate the memory appropriately. This way we
don't need any additional static memory.

Fixes: d2fd6e81912a ("PCI: Fix __initdata issue with "pci=disable_acs_redir" parameter")
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/pci.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)
Bjorn Helgaas - April 12, 2019, 8:32 p.m.
On Wed, Apr 10, 2019 at 03:05:31PM -0600, Logan Gunthorpe wrote:
> In most cases, kmalloc will not be available early in boot when
> pci_setup() is called. Thus, the kstrdup call that was added to fix the
> __initdata bug with the disable_acs_redir parameter usually returns
> NULL. Thus the parameter is discarded and it does not take into effect.
> 
> To fix this, we store the string that's in initdata until a initcall
> function can allocate the memory appropriately. This way we
> don't need any additional static memory.
> 
> Fixes: d2fd6e81912a ("PCI: Fix __initdata issue with "pci=disable_acs_redir" parameter")
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>

Applied to for-linus for v5.1, thanks, Logan!

> ---
>  drivers/pci/pci.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 7c1b362f599a..766f5779db92 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -6262,8 +6262,7 @@ static int __init pci_setup(char *str)
>  			} else if (!strncmp(str, "pcie_scan_all", 13)) {
>  				pci_add_flags(PCI_SCAN_ALL_PCIE_DEVS);
>  			} else if (!strncmp(str, "disable_acs_redir=", 18)) {
> -				disable_acs_redir_param =
> -					kstrdup(str + 18, GFP_KERNEL);
> +				disable_acs_redir_param = str + 18;
>  			} else {
>  				printk(KERN_ERR "PCI: Unknown option `%s'\n",
>  						str);
> @@ -6274,3 +6273,19 @@ static int __init pci_setup(char *str)
>  	return 0;
>  }
>  early_param("pci", pci_setup);
> +
> +/*
> + * 'disable_acs_redir_param' is initialized in pci_setup(), above, to point
> + * to data in the __initdata section which will be freed after the init
> + * sequence is complete. We can't allocate memory in pci_setup() because some
> + * architectures do not have any memory allocation service available during
> + * an early_param() call. So we allocate memory and copy the variable here
> + * before the init section is freed.
> + */
> +static int __init pci_realloc_setup_params(void)
> +{
> +	disable_acs_redir_param = kstrdup(disable_acs_redir_param, GFP_KERNEL);
> +
> +	return 0;
> +}
> +pure_initcall(pci_realloc_setup_params);
> -- 
> 2.20.1
>

Patch

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 7c1b362f599a..766f5779db92 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -6262,8 +6262,7 @@  static int __init pci_setup(char *str)
 			} else if (!strncmp(str, "pcie_scan_all", 13)) {
 				pci_add_flags(PCI_SCAN_ALL_PCIE_DEVS);
 			} else if (!strncmp(str, "disable_acs_redir=", 18)) {
-				disable_acs_redir_param =
-					kstrdup(str + 18, GFP_KERNEL);
+				disable_acs_redir_param = str + 18;
 			} else {
 				printk(KERN_ERR "PCI: Unknown option `%s'\n",
 						str);
@@ -6274,3 +6273,19 @@  static int __init pci_setup(char *str)
 	return 0;
 }
 early_param("pci", pci_setup);
+
+/*
+ * 'disable_acs_redir_param' is initialized in pci_setup(), above, to point
+ * to data in the __initdata section which will be freed after the init
+ * sequence is complete. We can't allocate memory in pci_setup() because some
+ * architectures do not have any memory allocation service available during
+ * an early_param() call. So we allocate memory and copy the variable here
+ * before the init section is freed.
+ */
+static int __init pci_realloc_setup_params(void)
+{
+	disable_acs_redir_param = kstrdup(disable_acs_redir_param, GFP_KERNEL);
+
+	return 0;
+}
+pure_initcall(pci_realloc_setup_params);