[RFC PATCH] PCI: Add "pci=reassign_all_bus" boot parameter

[RFC PATCH] PCI: Add "pci=reassign_all_bus" boot parameter

[Yao Hongbo]

PCI bridges may be misconfigured by te system BIOS, and then the OS
scan the bridges that need to be reconfigured.
However, the PCI bus topology configured by the bios may be wrong:

[   19.376273] pci 0000:40:00.0: bridge configuration invalid ([bus
00-00]), reconfiguring
[   19.384443] pci_bus 0000:47: busn_res: can not insert [bus 47-46]
under [bus 40-46] (conflicts with (null) [bus 40-46])

The primary bus number and subordinate bus number written by the bios
were wrong, and the OS continues to add bridges on the wrong bus
topology.

In order to avoid such problems, a kernel cmdline needs to be
added to support the os to fully configure the pci bus.

Signed-off-by: Yao Hongbo <yaohongbo@linux.alibaba.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 1 +
 drivers/acpi/pci_root.c                         | 3 +++
 drivers/pci/pci.c                               | 5 +++++
 include/linux/pci.h                             | 2 ++
 4 files changed, 11 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 2fba824..c83a2e5 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4084,6 +4084,7 @@
 		nomio		[S390] Do not use MIO instructions.
 		norid		[S390] ignore the RID field and force use of
 				one PCI domain per PCI function
+		reassign_all_bus	The OS fully configure the PCI bus.
 
 	pcie_aspm=	[PCIE] Forcibly enable or disable PCIe Active State Power
 			Management.
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index ab2f7df..e21ac25 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -592,6 +592,9 @@ static int acpi_pci_root_add(struct acpi_device *device,
 	is_pcie = strcmp(acpi_device_hid(device), "PNP0A08") == 0;
 	negotiate_os_control(root, &no_aspm, is_pcie);
 
+	if (pci_reassign_all_bus)
+		pci_add_flags(PCI_REASSIGN_ALL_BUS);
+
 	/*
 	 * TBD: Need PCI interface for enumeration/configuration of roots.
 	 */
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 3d2fb39..5746e88 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -142,6 +142,9 @@ bool pci_reset_supported(struct pci_dev *dev)
 /* If set, the PCI config space of each device is printed during boot. */
 bool pci_early_dump;
 
+/* If set, the pci will reassign resources*/
+bool pci_reassign_all_bus;
+
 bool pci_ats_disabled(void)
 {
 	return pcie_ats_disabled;
@@ -6846,6 +6849,8 @@ static int __init pci_setup(char *str)
 				pci_add_flags(PCI_SCAN_ALL_PCIE_DEVS);
 			} else if (!strncmp(str, "disable_acs_redir=", 18)) {
 				disable_acs_redir_param = str + 18;
+			} else if (!strncmp(str, "reassign_all_bus", 16)) {
+				pci_reassign_all_bus = true;
 			} else {
 				pr_err("PCI: Unknown option `%s'\n", str);
 			}
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 18a75c8e..ad0e3e9 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2119,6 +2119,8 @@ int pcim_iomap_regions_request_all(struct pci_dev *pdev, int mask,
 extern u8 pci_dfl_cache_line_size;
 extern u8 pci_cache_line_size;
 
+extern bool pci_reassign_all_bus;
+
 /* Architecture-specific versions may override these (weak) */
 void pcibios_disable_device(struct pci_dev *dev);
 void pcibios_set_master(struct pci_dev *dev);
-- 
1.8.3.1

Re: [RFC PATCH] PCI: Add "pci=reassign_all_bus" boot parameter

[Randy Dunlap]

Hi--

On 12/30/21 01:30, Yao Hongbo wrote:
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 2fba824..c83a2e5 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -4084,6 +4084,7 @@
>  		nomio		[S390] Do not use MIO instructions.
>  		norid		[S390] ignore the RID field and force use of
>  				one PCI domain per PCI function
> +		reassign_all_bus	The OS fully configure the PCI bus.

		                                     configures
>  
>  	pcie_aspm=	[PCIE] Forcibly enable or disable PCIe Active State Power
>  			Management.

thanks.
-- 
~Randy

Re: [RFC PATCH] PCI: Add "pci=reassign_all_bus" boot parameter

[Yao Hongbo]

Hi, Bjorn,
Gentel ping! Any comments on this patch?

I think this is useful to find whether there is a problem with
the PCI enumeration of the bios or the OS.

在 2021/12/30 下午5:30, Yao Hongbo 写道:
> PCI bridges may be misconfigured by te system BIOS, and then the OS
> scan the bridges that need to be reconfigured.
> However, the PCI bus topology configured by the bios may be wrong:
> 
> [   19.376273] pci 0000:40:00.0: bridge configuration invalid ([bus
> 00-00]), reconfiguring
> [   19.384443] pci_bus 0000:47: busn_res: can not insert [bus 47-46]
> under [bus 40-46] (conflicts with (null) [bus 40-46])
> 
> The primary bus number and subordinate bus number written by the bios
> were wrong, and the OS continues to add bridges on the wrong bus
> topology.
> 
> In order to avoid such problems, a kernel cmdline needs to be
> added to support the os to fully configure the pci bus.
> 
> Signed-off-by: Yao Hongbo <yaohongbo@linux.alibaba.com>
> ---
>  Documentation/admin-guide/kernel-parameters.txt | 1 +
>  drivers/acpi/pci_root.c                         | 3 +++
>  drivers/pci/pci.c                               | 5 +++++
>  include/linux/pci.h                             | 2 ++
>  4 files changed, 11 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 2fba824..c83a2e5 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -4084,6 +4084,7 @@
>  		nomio		[S390] Do not use MIO instructions.
>  		norid		[S390] ignore the RID field and force use of
>  				one PCI domain per PCI function
> +		reassign_all_bus	The OS fully configure the PCI bus.
>  
>  	pcie_aspm=	[PCIE] Forcibly enable or disable PCIe Active State Power
>  			Management.
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index ab2f7df..e21ac25 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -592,6 +592,9 @@ static int acpi_pci_root_add(struct acpi_device *device,
>  	is_pcie = strcmp(acpi_device_hid(device), "PNP0A08") == 0;
>  	negotiate_os_control(root, &no_aspm, is_pcie);
>  
> +	if (pci_reassign_all_bus)
> +		pci_add_flags(PCI_REASSIGN_ALL_BUS);
> +
>  	/*
>  	 * TBD: Need PCI interface for enumeration/configuration of roots.
>  	 */
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 3d2fb39..5746e88 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -142,6 +142,9 @@ bool pci_reset_supported(struct pci_dev *dev)
>  /* If set, the PCI config space of each device is printed during boot. */
>  bool pci_early_dump;
>  
> +/* If set, the pci will reassign resources*/
> +bool pci_reassign_all_bus;
> +
>  bool pci_ats_disabled(void)
>  {
>  	return pcie_ats_disabled;
> @@ -6846,6 +6849,8 @@ static int __init pci_setup(char *str)
>  				pci_add_flags(PCI_SCAN_ALL_PCIE_DEVS);
>  			} else if (!strncmp(str, "disable_acs_redir=", 18)) {
>  				disable_acs_redir_param = str + 18;
> +			} else if (!strncmp(str, "reassign_all_bus", 16)) {
> +				pci_reassign_all_bus = true;
>  			} else {
>  				pr_err("PCI: Unknown option `%s'\n", str);
>  			}
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 18a75c8e..ad0e3e9 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -2119,6 +2119,8 @@ int pcim_iomap_regions_request_all(struct pci_dev *pdev, int mask,
>  extern u8 pci_dfl_cache_line_size;
>  extern u8 pci_cache_line_size;
>  
> +extern bool pci_reassign_all_bus;
> +
>  /* Architecture-specific versions may override these (weak) */
>  void pcibios_disable_device(struct pci_dev *dev);
>  void pcibios_set_master(struct pci_dev *dev);

Re: [RFC PATCH] PCI: Add "pci=reassign_all_bus" boot parameter

[Bjorn Helgaas]

On Thu, Dec 30, 2021 at 05:30:13PM +0800, Yao Hongbo wrote:
> PCI bridges may be misconfigured by te system BIOS, and then the OS
> scan the bridges that need to be reconfigured.
> However, the PCI bus topology configured by the bios may be wrong:
> 
> [   19.376273] pci 0000:40:00.0: bridge configuration invalid ([bus
> 00-00]), reconfiguring
> [   19.384443] pci_bus 0000:47: busn_res: can not insert [bus 47-46]
> under [bus 40-46] (conflicts with (null) [bus 40-46])
> 
> The primary bus number and subordinate bus number written by the bios
> were wrong, and the OS continues to add bridges on the wrong bus
> topology.
> 
> In order to avoid such problems, a kernel cmdline needs to be
> added to support the os to fully configure the pci bus.

Why can't we make Linux smart enough to fix this by itself, without
forcing the user to boot with "pci=reassign_all_bus"?

> Signed-off-by: Yao Hongbo <yaohongbo@linux.alibaba.com>
> ---
>  Documentation/admin-guide/kernel-parameters.txt | 1 +
>  drivers/acpi/pci_root.c                         | 3 +++
>  drivers/pci/pci.c                               | 5 +++++
>  include/linux/pci.h                             | 2 ++
>  4 files changed, 11 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 2fba824..c83a2e5 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -4084,6 +4084,7 @@
>  		nomio		[S390] Do not use MIO instructions.
>  		norid		[S390] ignore the RID field and force use of
>  				one PCI domain per PCI function
> +		reassign_all_bus	The OS fully configure the PCI bus.
>  
>  	pcie_aspm=	[PCIE] Forcibly enable or disable PCIe Active State Power
>  			Management.
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index ab2f7df..e21ac25 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -592,6 +592,9 @@ static int acpi_pci_root_add(struct acpi_device *device,
>  	is_pcie = strcmp(acpi_device_hid(device), "PNP0A08") == 0;
>  	negotiate_os_control(root, &no_aspm, is_pcie);
>  
> +	if (pci_reassign_all_bus)
> +		pci_add_flags(PCI_REASSIGN_ALL_BUS);
> +
>  	/*
>  	 * TBD: Need PCI interface for enumeration/configuration of roots.
>  	 */
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 3d2fb39..5746e88 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -142,6 +142,9 @@ bool pci_reset_supported(struct pci_dev *dev)
>  /* If set, the PCI config space of each device is printed during boot. */
>  bool pci_early_dump;
>  
> +/* If set, the pci will reassign resources*/
> +bool pci_reassign_all_bus;
> +
>  bool pci_ats_disabled(void)
>  {
>  	return pcie_ats_disabled;
> @@ -6846,6 +6849,8 @@ static int __init pci_setup(char *str)
>  				pci_add_flags(PCI_SCAN_ALL_PCIE_DEVS);
>  			} else if (!strncmp(str, "disable_acs_redir=", 18)) {
>  				disable_acs_redir_param = str + 18;
> +			} else if (!strncmp(str, "reassign_all_bus", 16)) {
> +				pci_reassign_all_bus = true;
>  			} else {
>  				pr_err("PCI: Unknown option `%s'\n", str);
>  			}
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 18a75c8e..ad0e3e9 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -2119,6 +2119,8 @@ int pcim_iomap_regions_request_all(struct pci_dev *pdev, int mask,
>  extern u8 pci_dfl_cache_line_size;
>  extern u8 pci_cache_line_size;
>  
> +extern bool pci_reassign_all_bus;
> +
>  /* Architecture-specific versions may override these (weak) */
>  void pcibios_disable_device(struct pci_dev *dev);
>  void pcibios_set_master(struct pci_dev *dev);
> -- 
> 1.8.3.1
> 

Re: [RFC PATCH] PCI: Add "pci=reassign_all_bus" boot parameter

[Yao Hongbo]

在 2022/1/12 下午11:43, Bjorn Helgaas 写道:
> On Thu, Dec 30, 2021 at 05:30:13PM +0800, Yao Hongbo wrote:
>> PCI bridges may be misconfigured by te system BIOS, and then the OS
>> scan the bridges that need to be reconfigured.
>> However, the PCI bus topology configured by the bios may be wrong:
>>
>> [   19.376273] pci 0000:40:00.0: bridge configuration invalid ([bus
>> 00-00]), reconfiguring
>> [   19.384443] pci_bus 0000:47: busn_res: can not insert [bus 47-46]
>> under [bus 40-46] (conflicts with (null) [bus 40-46])
>>
>> The primary bus number and subordinate bus number written by the bios
>> were wrong, and the OS continues to add bridges on the wrong bus
>> topology.
>>
>> In order to avoid such problems, a kernel cmdline needs to be
>> added to support the os to fully configure the pci bus.
> 
> Why can't we make Linux smart enough to fix this by itself, without
> forcing the user to boot with "pci=reassign_all_bus"?
> 

   Hi, Bjorn.

   You're right, it's better for us to make pci enumeration more smart.

   But i think it's better to provide an additional option for users on ACPI systems,

which can quickly distingguish the issues between the BIOS and the OS.
>> Signed-off-by: Yao Hongbo <yaohongbo@linux.alibaba.com>
>> ---
>>  Documentation/admin-guide/kernel-parameters.txt | 1 +
>>  drivers/acpi/pci_root.c                         | 3 +++
>>  drivers/pci/pci.c                               | 5 +++++
>>  include/linux/pci.h                             | 2 ++
>>  4 files changed, 11 insertions(+)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 2fba824..c83a2e5 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -4084,6 +4084,7 @@
>>  		nomio		[S390] Do not use MIO instructions.
>>  		norid		[S390] ignore the RID field and force use of
>>  				one PCI domain per PCI function
>> +		reassign_all_bus	The OS fully configure the PCI bus.
>>  
>>  	pcie_aspm=	[PCIE] Forcibly enable or disable PCIe Active State Power
>>  			Management.
>> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
>> index ab2f7df..e21ac25 100644
>> --- a/drivers/acpi/pci_root.c
>> +++ b/drivers/acpi/pci_root.c
>> @@ -592,6 +592,9 @@ static int acpi_pci_root_add(struct acpi_device *device,
>>  	is_pcie = strcmp(acpi_device_hid(device), "PNP0A08") == 0;
>>  	negotiate_os_control(root, &no_aspm, is_pcie);
>>  
>> +	if (pci_reassign_all_bus)
>> +		pci_add_flags(PCI_REASSIGN_ALL_BUS);
>> +
>>  	/*
>>  	 * TBD: Need PCI interface for enumeration/configuration of roots.
>>  	 */
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index 3d2fb39..5746e88 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -142,6 +142,9 @@ bool pci_reset_supported(struct pci_dev *dev)
>>  /* If set, the PCI config space of each device is printed during boot. */
>>  bool pci_early_dump;
>>  
>> +/* If set, the pci will reassign resources*/
>> +bool pci_reassign_all_bus;
>> +
>>  bool pci_ats_disabled(void)
>>  {
>>  	return pcie_ats_disabled;
>> @@ -6846,6 +6849,8 @@ static int __init pci_setup(char *str)
>>  				pci_add_flags(PCI_SCAN_ALL_PCIE_DEVS);
>>  			} else if (!strncmp(str, "disable_acs_redir=", 18)) {
>>  				disable_acs_redir_param = str + 18;
>> +			} else if (!strncmp(str, "reassign_all_bus", 16)) {
>> +				pci_reassign_all_bus = true;
>>  			} else {
>>  				pr_err("PCI: Unknown option `%s'\n", str);
>>  			}
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index 18a75c8e..ad0e3e9 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -2119,6 +2119,8 @@ int pcim_iomap_regions_request_all(struct pci_dev *pdev, int mask,
>>  extern u8 pci_dfl_cache_line_size;
>>  extern u8 pci_cache_line_size;
>>  
>> +extern bool pci_reassign_all_bus;
>> +
>>  /* Architecture-specific versions may override these (weak) */
>>  void pcibios_disable_device(struct pci_dev *dev);
>>  void pcibios_set_master(struct pci_dev *dev);
>> -- 
>> 1.8.3.1
>>

Re: [RFC PATCH] PCI: Add "pci=reassign_all_bus" boot parameter

[Maciej W. Rozycki]

On Mon, 17 Jan 2022, Yao Hongbo wrote:

> > Why can't we make Linux smart enough to fix this by itself, without
> > forcing the user to boot with "pci=reassign_all_bus"?
> 
>    You're right, it's better for us to make pci enumeration more smart.
> 
>    But i think it's better to provide an additional option for users on ACPI systems,
> 
> which can quickly distingguish the issues between the BIOS and the OS.

 Even if so, x86 has had `pci=assign-busses' for decades already (i.e. 
from Linux 2.4.6), so why not reuse it for whatever your ACPI platform is?

  Maciej