[PATCH v2 1/2] powerpc/pci: Add config option for using OF 'reg' for PCI domain

[PATCH v2 1/2] powerpc/pci: Add config option for using OF 'reg' for PCI domain

[Pali Rohár]

Since commit 63a72284b159 ("powerpc/pci: Assign fixed PHB number based on
device-tree properties"), powerpc kernel always fallback to PCI domain
assignment from OF / Device Tree 'reg' property of the PCI controller.

In most cases 'reg' property is not zero and therefore there it cause that
PCI domain zero is not present in system anymore.

PCI code for other Linux architectures use increasing assignment of the PCI
domain for individual controllers (assign the first free number), like it
was also for powerpc prior mentioned commit. Also it starts numbering
domains from zero.

Upgrading powerpc kernels from LTS 4.4 version (which does not contain
mentioned commit) to new LTS versions brings a change in domain assignment.

It can be problematic for embedded machines with single PCIe controller
where it is expected that PCIe card is connected on the domain zero.
Also it can be problematic as that commit changes PCIe domains in
multi-controller setup with fixed number of controller (without hotplug
support).

Originally that change was intended for powernv and pservers and specially
server machines with more PCI domains or hot plug support.

Fix this issue and introduce a new option CONFIG_PPC_PCI_DOMAIN_FROM_OF_REG.
When this option is disabled then powerpc kernel would assign PCI domains
in the similar way like it is doing kernel for other architectures,
starting from zero and also how it was done prior that commit.

This option is by default enabled for powernv and pseries platform for which
was that commit originally intended.

With this change upgrading kernels from LTS 4.4 version does not change PCI
domain on smaller embedded platforms with fixed number of PCIe controllers.
And also ensure that PCI domain zero is present as before that commit.

Fixes: 63a72284b159 ("powerpc/pci: Assign fixed PHB number based on device-tree properties")
Signed-off-by: Pali Rohár <pali@kernel.org>
---
Changes in v2:
* Enable CONFIG_PPC_PCI_DOMAIN_FROM_OF_REG by default on powernv and pseries
---
 arch/powerpc/Kconfig             | 11 +++++++++++
 arch/powerpc/kernel/pci-common.c |  4 ++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index f66084bc1dfe..053a88e84049 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -386,6 +386,17 @@ config PPC_OF_PLATFORM_PCI
 	depends on PCI
 	depends on PPC64 # not supported on 32 bits yet
 
+config PPC_PCI_DOMAIN_FROM_OF_REG
+	bool "Use OF reg property for PCI domain"
+	depends on PCI
+	default y if PPC_PSERIES || PPC_POWERNV
+	help
+	  By default PCI domain for host bridge during its registration is
+	  chosen as the lowest unused PCI domain number.
+
+	  When this option is enabled then PCI domain can be determined
+	  also from lower bits of the OF / Device Tree 'reg' property.
+
 config ARCH_SUPPORTS_UPROBES
 	def_bool y
 
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 068410cd54a3..7f959df34833 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -74,7 +74,6 @@ void __init set_pci_dma_ops(const struct dma_map_ops *dma_ops)
 static int get_phb_number(struct device_node *dn)
 {
 	int ret, phb_id = -1;
-	u32 prop_32;
 	u64 prop;
 
 	/*
@@ -83,7 +82,8 @@ static int get_phb_number(struct device_node *dn)
 	 * reading "ibm,opal-phbid", only present in OPAL environment.
 	 */
 	ret = of_property_read_u64(dn, "ibm,opal-phbid", &prop);
-	if (ret) {
+	if (ret && IS_ENABLED(CONFIG_PPC_PCI_DOMAIN_FROM_OF_REG)) {
+		u32 prop_32;
 		ret = of_property_read_u32_index(dn, "reg", 1, &prop_32);
 		prop = prop_32;
 	}
-- 
2.20.1

[PATCH v2 2/2] powerpc/pci: Prefer PCI domain assignment via DT 'linux,pci-domain' and alias

[Pali Rohár]

Other Linux architectures use DT property 'linux,pci-domain' for specifying
fixed PCI domain of PCI controller specified in Device-Tree.

And lot of Freescale powerpc boards have defined numbered pci alias in
Device-Tree for every PCIe controller which number specify preferred PCI
domain.

So prefer usage of DT property 'linux,pci-domain' (via function
of_get_pci_domain_nr()) and DT pci alias (via function of_alias_get_id())
on powerpc architecture for assigning PCI domain to PCI controller.

Fixes: 63a72284b159 ("powerpc/pci: Assign fixed PHB number based on device-tree properties")
Signed-off-by: Pali Rohár <pali@kernel.org>
---
Changes in v2:
* New patch
---
 arch/powerpc/kernel/pci-common.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 7f959df34833..0715d74855b3 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -78,10 +78,25 @@ static int get_phb_number(struct device_node *dn)
 
 	/*
 	 * Try fixed PHB numbering first, by checking archs and reading
-	 * the respective device-tree properties. Firstly, try powernv by
-	 * reading "ibm,opal-phbid", only present in OPAL environment.
+	 * the respective device-tree properties. Firstly, try reading
+	 * standard "linux,pci-domain", then try reading "ibm,opal-phbid"
+	 * (only present in powernv OPAL environment), then try device-tree
+	 * alias and as the last try to use lower bits of "reg" property
+	 * (only if CONFIG_PPC_PCI_DOMAIN_FROM_OF_REG is enabled).
 	 */
-	ret = of_property_read_u64(dn, "ibm,opal-phbid", &prop);
+	ret = of_get_pci_domain_nr(dn);
+	if (ret >= 0) {
+		prop = ret;
+		ret = 0;
+	}
+	if (ret)
+		ret = of_property_read_u64(dn, "ibm,opal-phbid", &prop);
+	if (ret)
+		ret = of_alias_get_id(dn, "pci");
+	if (ret >= 0) {
+		prop = ret;
+		ret = 0;
+	}
 	if (ret && IS_ENABLED(CONFIG_PPC_PCI_DOMAIN_FROM_OF_REG)) {
 		u32 prop_32;
 		ret = of_property_read_u32_index(dn, "reg", 1, &prop_32);
@@ -95,10 +110,7 @@ static int get_phb_number(struct device_node *dn)
 	if ((phb_id >= 0) && !test_and_set_bit(phb_id, phb_bitmap))
 		return phb_id;
 
-	/*
-	 * If not pseries nor powernv, or if fixed PHB numbering tried to add
-	 * the same PHB number twice, then fallback to dynamic PHB numbering.
-	 */
+	/* If everything fails then fallback to dynamic PHB numbering. */
 	phb_id = find_first_zero_bit(phb_bitmap, MAX_PHBS);
 	BUG_ON(phb_id >= MAX_PHBS);
 	set_bit(phb_id, phb_bitmap);
-- 
2.20.1

Re: [PATCH v2 1/2] powerpc/pci: Add config option for using OF 'reg' for PCI domain

[Guilherme G. Piccoli]

On 06/07/2022 07:21, Pali Rohár wrote:
> [...] 
> Fix this issue and introduce a new option CONFIG_PPC_PCI_DOMAIN_FROM_OF_REG.
> When this option is disabled then powerpc kernel would assign PCI domains
> in the similar way like it is doing kernel for other architectures,
> starting from zero and also how it was done prior that commit.

I found this sentence a bit weird, "in the similar way like it is doing
kernel for other architectures", but other than that:

Reviewed-by: Guilherme G. Piccoli <gpiccoli@igalia.com>

Thanks for the improvement!
Cheers,


Guilherme


> 
> This option is by default enabled for powernv and pseries platform for which
> was that commit originally intended.
> 
> With this change upgrading kernels from LTS 4.4 version does not change PCI
> domain on smaller embedded platforms with fixed number of PCIe controllers.
> And also ensure that PCI domain zero is present as before that commit.
> 
> Fixes: 63a72284b159 ("powerpc/pci: Assign fixed PHB number based on device-tree properties")
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
> Changes in v2:
> * Enable CONFIG_PPC_PCI_DOMAIN_FROM_OF_REG by default on powernv and pseries
> ---
>  arch/powerpc/Kconfig             | 11 +++++++++++
>  arch/powerpc/kernel/pci-common.c |  4 ++--
>  2 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index f66084bc1dfe..053a88e84049 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -386,6 +386,17 @@ config PPC_OF_PLATFORM_PCI
>  	depends on PCI
>  	depends on PPC64 # not supported on 32 bits yet
>  
> +config PPC_PCI_DOMAIN_FROM_OF_REG
> +	bool "Use OF reg property for PCI domain"
> +	depends on PCI
> +	default y if PPC_PSERIES || PPC_POWERNV
> +	help
> +	  By default PCI domain for host bridge during its registration is
> +	  chosen as the lowest unused PCI domain number.
> +
> +	  When this option is enabled then PCI domain can be determined
> +	  also from lower bits of the OF / Device Tree 'reg' property.
> +
>  config ARCH_SUPPORTS_UPROBES
>  	def_bool y
>  
> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index 068410cd54a3..7f959df34833 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -74,7 +74,6 @@ void __init set_pci_dma_ops(const struct dma_map_ops *dma_ops)
>  static int get_phb_number(struct device_node *dn)
>  {
>  	int ret, phb_id = -1;
> -	u32 prop_32;
>  	u64 prop;
>  
>  	/*
> @@ -83,7 +82,8 @@ static int get_phb_number(struct device_node *dn)
>  	 * reading "ibm,opal-phbid", only present in OPAL environment.
>  	 */
>  	ret = of_property_read_u64(dn, "ibm,opal-phbid", &prop);
> -	if (ret) {
> +	if (ret && IS_ENABLED(CONFIG_PPC_PCI_DOMAIN_FROM_OF_REG)) {
> +		u32 prop_32;
>  		ret = of_property_read_u32_index(dn, "reg", 1, &prop_32);
>  		prop = prop_32;
>  	}

Re: [PATCH v2 1/2] powerpc/pci: Add config option for using OF 'reg' for PCI domain

[Pali Rohár]

On Friday 15 July 2022 11:55:04 Guilherme G. Piccoli wrote:
> On 06/07/2022 07:21, Pali Rohár wrote:
> > [...] 
> > Fix this issue and introduce a new option CONFIG_PPC_PCI_DOMAIN_FROM_OF_REG.
> > When this option is disabled then powerpc kernel would assign PCI domains
> > in the similar way like it is doing kernel for other architectures,
> > starting from zero and also how it was done prior that commit.
> 
> I found this sentence a bit weird, "in the similar way like it is doing
> kernel for other architectures", but other than that:

If you have some idea how to improve commit description, let me know and
I can change it.

> Reviewed-by: Guilherme G. Piccoli <gpiccoli@igalia.com>
> 
> Thanks for the improvement!
> Cheers,
> 
> 
> Guilherme
> 
> 
> > 
> > This option is by default enabled for powernv and pseries platform for which
> > was that commit originally intended.
> > 
> > With this change upgrading kernels from LTS 4.4 version does not change PCI
> > domain on smaller embedded platforms with fixed number of PCIe controllers.
> > And also ensure that PCI domain zero is present as before that commit.
> > 
> > Fixes: 63a72284b159 ("powerpc/pci: Assign fixed PHB number based on device-tree properties")
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> > Changes in v2:
> > * Enable CONFIG_PPC_PCI_DOMAIN_FROM_OF_REG by default on powernv and pseries
> > ---
> >  arch/powerpc/Kconfig             | 11 +++++++++++
> >  arch/powerpc/kernel/pci-common.c |  4 ++--
> >  2 files changed, 13 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > index f66084bc1dfe..053a88e84049 100644
> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -386,6 +386,17 @@ config PPC_OF_PLATFORM_PCI
> >  	depends on PCI
> >  	depends on PPC64 # not supported on 32 bits yet
> >  
> > +config PPC_PCI_DOMAIN_FROM_OF_REG
> > +	bool "Use OF reg property for PCI domain"
> > +	depends on PCI
> > +	default y if PPC_PSERIES || PPC_POWERNV
> > +	help
> > +	  By default PCI domain for host bridge during its registration is
> > +	  chosen as the lowest unused PCI domain number.
> > +
> > +	  When this option is enabled then PCI domain can be determined
> > +	  also from lower bits of the OF / Device Tree 'reg' property.
> > +
> >  config ARCH_SUPPORTS_UPROBES
> >  	def_bool y
> >  
> > diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> > index 068410cd54a3..7f959df34833 100644
> > --- a/arch/powerpc/kernel/pci-common.c
> > +++ b/arch/powerpc/kernel/pci-common.c
> > @@ -74,7 +74,6 @@ void __init set_pci_dma_ops(const struct dma_map_ops *dma_ops)
> >  static int get_phb_number(struct device_node *dn)
> >  {
> >  	int ret, phb_id = -1;
> > -	u32 prop_32;
> >  	u64 prop;
> >  
> >  	/*
> > @@ -83,7 +82,8 @@ static int get_phb_number(struct device_node *dn)
> >  	 * reading "ibm,opal-phbid", only present in OPAL environment.
> >  	 */
> >  	ret = of_property_read_u64(dn, "ibm,opal-phbid", &prop);
> > -	if (ret) {
> > +	if (ret && IS_ENABLED(CONFIG_PPC_PCI_DOMAIN_FROM_OF_REG)) {
> > +		u32 prop_32;
> >  		ret = of_property_read_u32_index(dn, "reg", 1, &prop_32);
> >  		prop = prop_32;
> >  	}

Re: [PATCH v2 1/2] powerpc/pci: Add config option for using OF 'reg' for PCI domain

[Guilherme G. Piccoli]

On 15/07/2022 14:11, Pali Rohár wrote:
> [...]
>>
>> I found this sentence a bit weird, "in the similar way like it is doing
>> kernel for other architectures", but other than that:
> 
> If you have some idea how to improve commit description, let me know and
> I can change it.
> 

Oh, for example: "in similar way the kernel is doing for other
architectures". The sentence idea is perfectly fine, it's just that it's
a bit oddly constructed IMHO heh

Cheers!

Re: [PATCH v2 1/2] powerpc/pci: Add config option for using OF 'reg' for PCI domain

[Pali Rohár]

On Friday 15 July 2022 15:32:56 Guilherme G. Piccoli wrote:
> On 15/07/2022 14:11, Pali Rohár wrote:
> > [...]
> >>
> >> I found this sentence a bit weird, "in the similar way like it is doing
> >> kernel for other architectures", but other than that:
> > 
> > If you have some idea how to improve commit description, let me know and
> > I can change it.
> > 
> 
> Oh, for example: "in similar way the kernel is doing for other
> architectures". The sentence idea is perfectly fine, it's just that it's
> a bit oddly constructed IMHO heh
> 
> Cheers!

Ou, sorry. English is not my first nor second language.

Michael, should I resend this patch series with Guilherme's suggestion
for changing working in commit description? Or will you adjust it
manually?

Re: [PATCH v2 1/2] powerpc/pci: Add config option for using OF 'reg' for PCI domain

[Michael Ellerman]

Pali Rohár <pali@kernel.org> writes:
> Since commit 63a72284b159 ("powerpc/pci: Assign fixed PHB number based on
> device-tree properties"), powerpc kernel always fallback to PCI domain
> assignment from OF / Device Tree 'reg' property of the PCI controller.
>
> In most cases 'reg' property is not zero and therefore there it cause that
> PCI domain zero is not present in system anymore.
>
> PCI code for other Linux architectures use increasing assignment of the PCI
> domain for individual controllers (assign the first free number), like it
> was also for powerpc prior mentioned commit. Also it starts numbering
> domains from zero.
>
> Upgrading powerpc kernels from LTS 4.4 version (which does not contain
> mentioned commit) to new LTS versions brings a change in domain assignment.
>
> It can be problematic for embedded machines with single PCIe controller
> where it is expected that PCIe card is connected on the domain zero.
> Also it can be problematic as that commit changes PCIe domains in
> multi-controller setup with fixed number of controller (without hotplug
> support).
>
> Originally that change was intended for powernv and pservers and specially
> server machines with more PCI domains or hot plug support.
>
> Fix this issue and introduce a new option CONFIG_PPC_PCI_DOMAIN_FROM_OF_REG.

As I said in my previous reply, I don't want a config option for this.

Adding an option now would revert the behaviour back to the way it was,
which has the potential to break things, as you described.

Maybe we shouldn't have changed the numbering to begin with, but it's
been 6 years, so it's too late to change it back.

cheers

Re: [PATCH v2 2/2] powerpc/pci: Prefer PCI domain assignment via DT 'linux,pci-domain' and alias

[Guenter Roeck]

On Wed, Jul 06, 2022 at 12:21:48PM +0200, Pali Rohár wrote:
> Other Linux architectures use DT property 'linux,pci-domain' for specifying
> fixed PCI domain of PCI controller specified in Device-Tree.
> 
> And lot of Freescale powerpc boards have defined numbered pci alias in
> Device-Tree for every PCIe controller which number specify preferred PCI
> domain.
> 
> So prefer usage of DT property 'linux,pci-domain' (via function
> of_get_pci_domain_nr()) and DT pci alias (via function of_alias_get_id())
> on powerpc architecture for assigning PCI domain to PCI controller.
> 
> Fixes: 63a72284b159 ("powerpc/pci: Assign fixed PHB number based on device-tree properties")
> Signed-off-by: Pali Rohár <pali@kernel.org>

This patch results in a number of boot warnings with various qemu
boot tests.

Sample log and bisect results are attached.

Guenter

---
BUG: sleeping function called from invalid context at kernel/locking/mutex.c:580
in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 1, name: swapper
preempt_count: 1, expected: 0
RCU nest depth: 0, expected: 0
1 lock held by swapper/1:
 #0: c157efb0 (hose_spinlock){+.+.}-{2:2}, at: pcibios_alloc_controller+0x64/0x220
Preemption disabled at:
[<00000000>] 0x0
CPU: 0 PID: 1 Comm: swapper Not tainted 5.19.0-yocto-standard+ #1
Call Trace:
[d101dc90] [c073b264] dump_stack_lvl+0x50/0x8c (unreliable)
[d101dcb0] [c0093b70] __might_resched+0x258/0x2a8
[d101dcd0] [c0d3e634] __mutex_lock+0x6c/0x6ec
[d101dd50] [c0a84174] of_alias_get_id+0x50/0xf4
[d101dd80] [c002ec78] pcibios_alloc_controller+0x1b8/0x220
[d101ddd0] [c140c9dc] pmac_pci_init+0x198/0x784
[d101de50] [c140852c] discover_phbs+0x30/0x4c
[d101de60] [c0007fd4] do_one_initcall+0x94/0x344
[d101ded0] [c1403b40] kernel_init_freeable+0x1a8/0x22c
[d101df10] [c00086e0] kernel_init+0x34/0x160
[d101df30] [c001b334] ret_from_kernel_thread+0x5c/0x64

=============================
[ BUG: Invalid wait context ]
5.19.0-yocto-standard+ #1 Tainted: G        W
-----------------------------
swapper/1 is trying to lock:
c16a9dd8 (of_mutex){+.+.}-{3:3}, at: of_alias_get_id+0x50/0xf4
other info that might help us debug this:
context-{4:4}
1 lock held by swapper/1:
 #0: c157efb0 (hose_spinlock){+.+.}-{2:2}, at: pcibios_alloc_controller+0x64/0x220
stack backtrace:
CPU: 0 PID: 1 Comm: swapper Tainted: G        W          5.19.0-yocto-standard+ #1
Call Trace:
[d101dbc0] [c073b264] dump_stack_lvl+0x50/0x8c (unreliable)
[d101dbe0] [c00bb8e8] __lock_acquire+0x8c4/0x2278
[d101dc60] [c00ba4b8] lock_acquire+0x148/0x3b4
[d101dcd0] [c0d3e688] __mutex_lock+0xc0/0x6ec
[d101dd50] [c0a84174] of_alias_get_id+0x50/0xf4
[d101dd80] [c002ec78] pcibios_alloc_controller+0x1b8/0x220
[d101ddd0] [c140c9dc] pmac_pci_init+0x198/0x784
[d101de50] [c140852c] discover_phbs+0x30/0x4c
[d101de60] [c0007fd4] do_one_initcall+0x94/0x344
[d101ded0] [c1403b40] kernel_init_freeable+0x1a8/0x22c
[d101df10] [c00086e0] kernel_init+0x34/0x160
[d101df30] [c001b334] ret_from_kernel_thread+0x5c/0x64
Found UniNorth PCI host bridge at 0x00000000f2000000. Firmware bus number: 0->0
PCI host bridge /pci@f2000000 (primary) ranges:
  IO 0x00000000f2000000..0x00000000f27fffff -> 0x0000000000000000
 MEM 0x0000000080000000..0x000000008fffffff -> 0x0000000080000000

---
# bad: [69dac8e431af26173ca0a1ebc87054e01c585bcc] Merge tag 'riscv-for-linus-5.20-mw2' of git://git.kernel.org/pub/scm/linux/kernel/git/riscv/linux
# good: [6614a3c3164a5df2b54abb0b3559f51041cf705b] Merge tag 'mm-stable-2022-08-03' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
git bisect start 'HEAD' '6614a3c3164a'
# bad: [24cb958695724ffb4488ef4f65892c0767bcd2f2] Merge tag 's390-5.20-1' of git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux
git bisect bad 24cb958695724ffb4488ef4f65892c0767bcd2f2
# good: [a3b5d4715fd5a839857f8b7be78dff258a8d5a47] Merge tag 'asoc-v5.20-2' of https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound into for-linus
git bisect good a3b5d4715fd5a839857f8b7be78dff258a8d5a47
# good: [1d239c1eb873c7d6c6cbc80d68330c939fd86136] Merge tag 'iommu-updates-v5.20-or-v6.0' of git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu
git bisect good 1d239c1eb873c7d6c6cbc80d68330c939fd86136
# bad: [4cfa6ff24a9744ba484521c38bea613134fbfcb3] powerpc/64e: Fix kexec build error
git bisect bad 4cfa6ff24a9744ba484521c38bea613134fbfcb3
# good: [78988b273d592ce74c8aecdd5d748906c38a9e9d] powerpc/perf: Give generic PMU a nice name
git bisect good 78988b273d592ce74c8aecdd5d748906c38a9e9d
# good: [de40303b54bc458d7df0d4b4ee1d296df7fe98c7] powerpc/ppc-opcode: Define and use PPC_RAW_SETB()
git bisect good de40303b54bc458d7df0d4b4ee1d296df7fe98c7
# bad: [738f9dca0df3bb630e6f06a19573ab4e31bd443a] powerpc/sysdev: Fix comment typo
git bisect bad 738f9dca0df3bb630e6f06a19573ab4e31bd443a
# good: [4d5c5bad51935482437528f7fa4dffdcb3330d8b] powerpc: Remove asm/prom.h from asm/mpc52xx.h and asm/pci.h
git bisect good 4d5c5bad51935482437528f7fa4dffdcb3330d8b
# good: [d80f6de9d601c30b53c17f00cb7cfe3169f2ddad] powerpc/iommu: Fix iommu_table_in_use for a small default DMA window case
git bisect good d80f6de9d601c30b53c17f00cb7cfe3169f2ddad
# bad: [0fe1e96fef0a5c53b4c0d1500d356f3906000f81] powerpc/pci: Prefer PCI domain assignment via DT 'linux,pci-domain' and alias
git bisect bad 0fe1e96fef0a5c53b4c0d1500d356f3906000f81
# good: [d20c96deb3e2c1cedc47d2be9fc110ffed81b1af] powerpc/85xx: Fix description of MPC85xx and P1/P2 boards options
git bisect good d20c96deb3e2c1cedc47d2be9fc110ffed81b1af
# first bad commit: [0fe1e96fef0a5c53b4c0d1500d356f3906000f81] powerpc/pci: Prefer PCI domain assignment via DT 'linux,pci-domain' and alias

Re: [PATCH v2 2/2] powerpc/pci: Prefer PCI domain assignment via DT 'linux,pci-domain' and alias

[Michael Ellerman]

Guenter Roeck <linux@roeck-us.net> writes:
> On Wed, Jul 06, 2022 at 12:21:48PM +0200, Pali Rohár wrote:
>> Other Linux architectures use DT property 'linux,pci-domain' for specifying
>> fixed PCI domain of PCI controller specified in Device-Tree.
>> 
>> And lot of Freescale powerpc boards have defined numbered pci alias in
>> Device-Tree for every PCIe controller which number specify preferred PCI
>> domain.
>> 
>> So prefer usage of DT property 'linux,pci-domain' (via function
>> of_get_pci_domain_nr()) and DT pci alias (via function of_alias_get_id())
>> on powerpc architecture for assigning PCI domain to PCI controller.
>> 
>> Fixes: 63a72284b159 ("powerpc/pci: Assign fixed PHB number based on device-tree properties")
>> Signed-off-by: Pali Rohár <pali@kernel.org>
>
> This patch results in a number of boot warnings with various qemu
> boot tests.

Thanks for the report.

I have automated qemu boot tests to catch things like this, they even
have DEBUG_ATOMIC_SLEEP enabled ... but I inadvertantly broke my script
that checks for "BUG:" in the console log. Sometimes you just can't
win.

cheers

Re: [PATCH v2 2/2] powerpc/pci: Prefer PCI domain assignment via DT 'linux,pci-domain' and alias

[Benjamin Herrenschmidt]

On Mon, 2022-08-15 at 15:46 +1000, Michael Ellerman wrote:
> Guenter Roeck <linux@roeck-us.net> writes:
> > On Wed, Jul 06, 2022 at 12:21:48PM +0200, Pali Rohár wrote:
> > > Other Linux architectures use DT property 'linux,pci-domain' for specifying
> > > fixed PCI domain of PCI controller specified in Device-Tree.
> > > 
> > > And lot of Freescale powerpc boards have defined numbered pci alias in
> > > Device-Tree for every PCIe controller which number specify preferred PCI
> > > domain.
> > > 
> > > So prefer usage of DT property 'linux,pci-domain' (via function
> > > of_get_pci_domain_nr()) and DT pci alias (via function of_alias_get_id())
> > > on powerpc architecture for assigning PCI domain to PCI controller.
> > > 
> > > Fixes: 63a72284b159 ("powerpc/pci: Assign fixed PHB number based on device-tree properties")
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > 
> > This patch results in a number of boot warnings with various qemu
> > boot tests.
> 
> Thanks for the report.
> 
> I have automated qemu boot tests to catch things like this, they even
> have DEBUG_ATOMIC_SLEEP enabled ... but I inadvertantly broke my script
> that checks for "BUG:" in the console log. Sometimes you just can't
> win.

So the problem is

 	spin_lock(&hose_spinlock);

get_phb_number() relies on it for the phb_bitmap allocation. You can
move it out of the lock but you'll have to either:

 - Take the lock inside it to protect the allocation

 - Turn find_first_zero_bit/set_bit into a loop of
find_first_zero_bit+test_and_set_bit() which wouldn't require a lock.

Note about the other "reg" numbering conversation ... I'm pretty sure
that breaks some old PowerMac crap which shows nobody really uses these
things considering how long the patch has been there :-)

I'm pretty sure something somewhere assumes the primary bus is 0. Some
old userspace definitely does though that might no longer be relevant.
The whole business with "domain 0" being special and avoiding domain
numbers in /proc relies on this too...

Cheers,
Ben.