Re: [PATCH] powerpc/pci: Enable PCI domains in /proc when PCI bus numbers are not unique
On Thu, 2022-09-01 at 13:53 +1000, Michael Ellerman wrote: > > > > I sent two patches which do another steps to achieve it: > > https://lore.kernel.org/linuxppc-dev/20220817163927.24453-1-p...@kernel.org/t/#u > > > > Main blocker is pci-OF-bus-map which is in direct conflict with > > CONFIG_PPC_PCI_BUS_NUM_DOMAIN_DEPENDENT and which used on chrp and pmac. > > And I have no idea if pci-OF-bus-map is still needed or not. > > Yeah thanks, I saw those patches. > > I can't find any code that refers to pci-OF-bus-map, so I'm inclined to > remove it entirely. > > But I'll do some more searching to see if I can find any references to > it in old code. Trying to remember ... :-) So this is what I recall at this point: - Ancient X11 didn't understand domains in /proc and thus would barf, which was the primary reason for not enabling them always iirc... - There might be something else with early PowerMacs (Grand Central chipset) where we have effectively two domains (gc and chaos) but overlapping bus numbers. There might still be pre-historical code in there that assumes it's that way though I can't see anything obvious. Paul might still have one of these :-) (PowerMac 7200/7500/8500/9500 afaik). - pci-OF-bus-map predates the PCI layer keeping track of the PCI/OF relationship. I don't believe it's still used anywhere in the kernel, though it's possible (unlikely ?) that some garbage remains in userspace that does. At this point, I wouldn't object to tearing this all out and just having domains always (and see what the fallout is). Cheers, Ben.
Re: [PATCH] powerpc/pci: Enable PCI domains in /proc when PCI bus numbers are not unique
On Thursday 01 September 2022 13:53:56 Michael Ellerman wrote: > Pali Rohár writes: > > On Thursday 25 August 2022 17:49:28 Michael Ellerman wrote: > >> Pali Rohár writes: > >> > On 32-bit powerpc systems with more PCIe controllers and more PCI > >> > domains, > >> > where on more PCI domains are same PCI numbers, when kernel is compiled > >> > with CONFIG_PROC_FS=y and CONFIG_PPC_PCI_BUS_NUM_DOMAIN_DEPENDENT=y > >> > options, kernel prints "proc_dir_entry 'pci/01' already registered" error > >> > message. > >> > >> Thanks, I'll pick this up. > >> > >> > This regression started appearing after commit 566356813082 > >> > ("powerpc/pci: > >> > Add config option for using all 256 PCI buses") in case in each mPCIe > >> > slot > >> > is connected PCIe card and therefore PCI bus 1 is populated in for every > >> > PCIe controller / PCI domain. > >> > > >> > The reason is that PCI procfs code expects that when PCI bus numbers are > >> > not unique across all PCI domains, function pci_proc_domain() returns > >> > true > >> > for domain dependent buses. > >> > > >> > Fix this issue by setting PCI_ENABLE_PROC_DOMAINS and PCI_COMPAT_DOMAIN_0 > >> > flags for 32-bit powerpc code when > >> > CONFIG_PPC_PCI_BUS_NUM_DOMAIN_DEPENDENT > >> > is enabled. Same approach is already implemented for 64-bit powerpc code > >> > (where PCI bus numbers are always domain dependent). > >> > >> We also have the same in ppc4xx_pci_find_bridges(). > >> > >> And if we can eventually make CONFIG_PPC_PCI_BUS_NUM_DOMAIN_DEPENDENT > >> the standard behaviour on 32-bit then everything would behave the same > >> and we could simplify pci_proc_domain() to match what other arches do. > > > > I sent two patches which do another steps to achieve it: > > https://lore.kernel.org/linuxppc-dev/20220817163927.24453-1-p...@kernel.org/t/#u > > > > Main blocker is pci-OF-bus-map which is in direct conflict with > > CONFIG_PPC_PCI_BUS_NUM_DOMAIN_DEPENDENT and which used on chrp and pmac. > > And I have no idea if pci-OF-bus-map is still needed or not. > > Yeah thanks, I saw those patches. > > I can't find any code that refers to pci-OF-bus-map, so I'm inclined to > remove it entirely. > > But I'll do some more searching to see if I can find any references to > it in old code. > > cheers >From the code itself I have feeling that some external programs or maybe some firmware can access or use this property. But I have really no idea.
Re: [PATCH] powerpc/pci: Enable PCI domains in /proc when PCI bus numbers are not unique
Pali Rohár writes: > On Thursday 25 August 2022 17:49:28 Michael Ellerman wrote: >> Pali Rohár writes: >> > On 32-bit powerpc systems with more PCIe controllers and more PCI domains, >> > where on more PCI domains are same PCI numbers, when kernel is compiled >> > with CONFIG_PROC_FS=y and CONFIG_PPC_PCI_BUS_NUM_DOMAIN_DEPENDENT=y >> > options, kernel prints "proc_dir_entry 'pci/01' already registered" error >> > message. >> >> Thanks, I'll pick this up. >> >> > This regression started appearing after commit 566356813082 ("powerpc/pci: >> > Add config option for using all 256 PCI buses") in case in each mPCIe slot >> > is connected PCIe card and therefore PCI bus 1 is populated in for every >> > PCIe controller / PCI domain. >> > >> > The reason is that PCI procfs code expects that when PCI bus numbers are >> > not unique across all PCI domains, function pci_proc_domain() returns true >> > for domain dependent buses. >> > >> > Fix this issue by setting PCI_ENABLE_PROC_DOMAINS and PCI_COMPAT_DOMAIN_0 >> > flags for 32-bit powerpc code when CONFIG_PPC_PCI_BUS_NUM_DOMAIN_DEPENDENT >> > is enabled. Same approach is already implemented for 64-bit powerpc code >> > (where PCI bus numbers are always domain dependent). >> >> We also have the same in ppc4xx_pci_find_bridges(). >> >> And if we can eventually make CONFIG_PPC_PCI_BUS_NUM_DOMAIN_DEPENDENT >> the standard behaviour on 32-bit then everything would behave the same >> and we could simplify pci_proc_domain() to match what other arches do. > > I sent two patches which do another steps to achieve it: > https://lore.kernel.org/linuxppc-dev/20220817163927.24453-1-p...@kernel.org/t/#u > > Main blocker is pci-OF-bus-map which is in direct conflict with > CONFIG_PPC_PCI_BUS_NUM_DOMAIN_DEPENDENT and which used on chrp and pmac. > And I have no idea if pci-OF-bus-map is still needed or not. Yeah thanks, I saw those patches. I can't find any code that refers to pci-OF-bus-map, so I'm inclined to remove it entirely. But I'll do some more searching to see if I can find any references to it in old code. cheers
Re: [PATCH] powerpc/pci: Enable PCI domains in /proc when PCI bus numbers are not unique
On Sat, 20 Aug 2022 13:51:13 +0200, Pali Rohár wrote: > On 32-bit powerpc systems with more PCIe controllers and more PCI domains, > where on more PCI domains are same PCI numbers, when kernel is compiled > with CONFIG_PROC_FS=y and CONFIG_PPC_PCI_BUS_NUM_DOMAIN_DEPENDENT=y > options, kernel prints "proc_dir_entry 'pci/01' already registered" error > message. > > [1.708861] [ cut here ] > [1.713429] proc_dir_entry 'pci/01' already registered > [1.718595] WARNING: CPU: 0 PID: 1 at fs/proc/generic.c:377 > proc_register+0x1a8/0x1ac > [1.726361] Modules linked in: > [1.729404] CPU: 0 PID: 1 Comm: swapper/0 Tainted: GW > 5.19.0-rc5-0caacb197b677410bdac81bc34f05235+ #109 > [1.740183] NIP: c02846e8 LR: c02846e8 CTR: c0015154 > [1.745225] REGS: c146fc90 TRAP: 0700 Tainted: GW > (5.19.0-rc5-0caacb197b677410bdac81bc34f05235+) > [1.755657] MSR: 00029000 CR: 28000822 XER: > [1.761829] > [1.761829] GPR00: c02846e8 c146fd80 c14a8000 002a 3fffefff c146fc40 > c146fc38 > [1.761829] GPR08: 3fffefff c10ac04c 24000824 > c0004548 > [1.761829] GPR16: > 0007 > [1.761829] GPR24: c1d0 c167da54 c167da00 c112 c167dd6c c10b4abc > c167dc58 c167dd00 > [1.796707] NIP [c02846e8] proc_register+0x1a8/0x1ac > [1.801663] LR [c02846e8] proc_register+0x1a8/0x1ac > [1.806532] Call Trace: > [1.808966] [c146fd80] [c02846e8] proc_register+0x1a8/0x1ac (unreliable) > [1.815659] [c146fdb0] [c028481c] _proc_mkdir+0x78/0xa4 > [1.820875] [c146fdd0] [c05a92e4] pci_proc_attach_device+0x11c/0x168 > [1.827221] [c146fe10] [c101f7a4] pci_proc_init+0x80/0x98 > [1.832611] [c146fe30] [c0004150] do_one_initcall+0x80/0x284 > [1.838262] [c146fea0] [c10011a8] kernel_init_freeable+0x1f4/0x2a0 > [1.844434] [c146fee0] [c000456c] kernel_init+0x24/0x150 > [1.849737] [c146ff00] [c001326c] ret_from_kernel_thread+0x5c/0x64 > [1.855910] Instruction dump: > [1.858866] 83810020 83a10024 83c10028 83e1002c 38210030 4e800020 > 809a0064 3c60c0a8 > [1.866602] 7f85e378 3863af28 4cc63182 4bdb8155 <0fe0> 9421ffe0 > 3920 7c0802a6 > [1.874513] ---[ end trace ]--- > > [...] Applied to powerpc/fixes. [1/1] powerpc/pci: Enable PCI domains in /proc when PCI bus numbers are not unique https://git.kernel.org/powerpc/c/0382a35bef70ecc074db67192ff8d37737d02b21 cheers
Re: [PATCH] powerpc/pci: Enable PCI domains in /proc when PCI bus numbers are not unique
On Thursday 25 August 2022 17:49:28 Michael Ellerman wrote: > Pali Rohár writes: > > On 32-bit powerpc systems with more PCIe controllers and more PCI domains, > > where on more PCI domains are same PCI numbers, when kernel is compiled > > with CONFIG_PROC_FS=y and CONFIG_PPC_PCI_BUS_NUM_DOMAIN_DEPENDENT=y > > options, kernel prints "proc_dir_entry 'pci/01' already registered" error > > message. > > Thanks, I'll pick this up. > > > This regression started appearing after commit 566356813082 ("powerpc/pci: > > Add config option for using all 256 PCI buses") in case in each mPCIe slot > > is connected PCIe card and therefore PCI bus 1 is populated in for every > > PCIe controller / PCI domain. > > > > The reason is that PCI procfs code expects that when PCI bus numbers are > > not unique across all PCI domains, function pci_proc_domain() returns true > > for domain dependent buses. > > > > Fix this issue by setting PCI_ENABLE_PROC_DOMAINS and PCI_COMPAT_DOMAIN_0 > > flags for 32-bit powerpc code when CONFIG_PPC_PCI_BUS_NUM_DOMAIN_DEPENDENT > > is enabled. Same approach is already implemented for 64-bit powerpc code > > (where PCI bus numbers are always domain dependent). > > We also have the same in ppc4xx_pci_find_bridges(). > > And if we can eventually make CONFIG_PPC_PCI_BUS_NUM_DOMAIN_DEPENDENT > the standard behaviour on 32-bit then everything would behave the same > and we could simplify pci_proc_domain() to match what other arches do. I sent two patches which do another steps to achieve it: https://lore.kernel.org/linuxppc-dev/20220817163927.24453-1-p...@kernel.org/t/#u Main blocker is pci-OF-bus-map which is in direct conflict with CONFIG_PPC_PCI_BUS_NUM_DOMAIN_DEPENDENT and which used on chrp and pmac. And I have no idea if pci-OF-bus-map is still needed or not. > cheers > > > > Fixes: 566356813082 ("powerpc/pci: Add config option for using all 256 PCI > > buses") > > Signed-off-by: Pali Rohár > > --- > > arch/powerpc/kernel/pci_32.c | 9 + > > 1 file changed, 9 insertions(+) > > > > diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c > > index ffc4e1928c80..8acbc9592ebb 100644 > > --- a/arch/powerpc/kernel/pci_32.c > > +++ b/arch/powerpc/kernel/pci_32.c > > @@ -249,6 +249,15 @@ static int __init pcibios_init(void) > > > > printk(KERN_INFO "PCI: Probing PCI hardware\n"); > > > > +#ifdef CONFIG_PPC_PCI_BUS_NUM_DOMAIN_DEPENDENT > > + /* > > +* Enable PCI domains in /proc when PCI bus numbers are not unique > > +* across all PCI domains to prevent conflicts. And keep PCI domain 0 > > +* backward compatible in /proc for video cards. > > +*/ > > + pci_add_flags(PCI_ENABLE_PROC_DOMAINS | PCI_COMPAT_DOMAIN_0); > > +#endif > > + > > if (pci_has_flag(PCI_REASSIGN_ALL_BUS)) > > pci_assign_all_buses = 1; > > > > -- > > 2.20.1
Re: [PATCH] powerpc/pci: Enable PCI domains in /proc when PCI bus numbers are not unique
Pali Rohár writes: > On 32-bit powerpc systems with more PCIe controllers and more PCI domains, > where on more PCI domains are same PCI numbers, when kernel is compiled > with CONFIG_PROC_FS=y and CONFIG_PPC_PCI_BUS_NUM_DOMAIN_DEPENDENT=y > options, kernel prints "proc_dir_entry 'pci/01' already registered" error > message. Thanks, I'll pick this up. > This regression started appearing after commit 566356813082 ("powerpc/pci: > Add config option for using all 256 PCI buses") in case in each mPCIe slot > is connected PCIe card and therefore PCI bus 1 is populated in for every > PCIe controller / PCI domain. > > The reason is that PCI procfs code expects that when PCI bus numbers are > not unique across all PCI domains, function pci_proc_domain() returns true > for domain dependent buses. > > Fix this issue by setting PCI_ENABLE_PROC_DOMAINS and PCI_COMPAT_DOMAIN_0 > flags for 32-bit powerpc code when CONFIG_PPC_PCI_BUS_NUM_DOMAIN_DEPENDENT > is enabled. Same approach is already implemented for 64-bit powerpc code > (where PCI bus numbers are always domain dependent). We also have the same in ppc4xx_pci_find_bridges(). And if we can eventually make CONFIG_PPC_PCI_BUS_NUM_DOMAIN_DEPENDENT the standard behaviour on 32-bit then everything would behave the same and we could simplify pci_proc_domain() to match what other arches do. cheers > Fixes: 566356813082 ("powerpc/pci: Add config option for using all 256 PCI > buses") > Signed-off-by: Pali Rohár > --- > arch/powerpc/kernel/pci_32.c | 9 + > 1 file changed, 9 insertions(+) > > diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c > index ffc4e1928c80..8acbc9592ebb 100644 > --- a/arch/powerpc/kernel/pci_32.c > +++ b/arch/powerpc/kernel/pci_32.c > @@ -249,6 +249,15 @@ static int __init pcibios_init(void) > > printk(KERN_INFO "PCI: Probing PCI hardware\n"); > > +#ifdef CONFIG_PPC_PCI_BUS_NUM_DOMAIN_DEPENDENT > + /* > + * Enable PCI domains in /proc when PCI bus numbers are not unique > + * across all PCI domains to prevent conflicts. And keep PCI domain 0 > + * backward compatible in /proc for video cards. > + */ > + pci_add_flags(PCI_ENABLE_PROC_DOMAINS | PCI_COMPAT_DOMAIN_0); > +#endif > + > if (pci_has_flag(PCI_REASSIGN_ALL_BUS)) > pci_assign_all_buses = 1; > > -- > 2.20.1
[PATCH] powerpc/pci: Enable PCI domains in /proc when PCI bus numbers are not unique
On 32-bit powerpc systems with more PCIe controllers and more PCI domains, where on more PCI domains are same PCI numbers, when kernel is compiled with CONFIG_PROC_FS=y and CONFIG_PPC_PCI_BUS_NUM_DOMAIN_DEPENDENT=y options, kernel prints "proc_dir_entry 'pci/01' already registered" error message. [1.708861] [ cut here ] [1.713429] proc_dir_entry 'pci/01' already registered [1.718595] WARNING: CPU: 0 PID: 1 at fs/proc/generic.c:377 proc_register+0x1a8/0x1ac [1.726361] Modules linked in: [1.729404] CPU: 0 PID: 1 Comm: swapper/0 Tainted: GW 5.19.0-rc5-0caacb197b677410bdac81bc34f05235+ #109 [1.740183] NIP: c02846e8 LR: c02846e8 CTR: c0015154 [1.745225] REGS: c146fc90 TRAP: 0700 Tainted: GW (5.19.0-rc5-0caacb197b677410bdac81bc34f05235+) [1.755657] MSR: 00029000 CR: 28000822 XER: [1.761829] [1.761829] GPR00: c02846e8 c146fd80 c14a8000 002a 3fffefff c146fc40 c146fc38 [1.761829] GPR08: 3fffefff c10ac04c 24000824 c0004548 [1.761829] GPR16: 0007 [1.761829] GPR24: c1d0 c167da54 c167da00 c112 c167dd6c c10b4abc c167dc58 c167dd00 [1.796707] NIP [c02846e8] proc_register+0x1a8/0x1ac [1.801663] LR [c02846e8] proc_register+0x1a8/0x1ac [1.806532] Call Trace: [1.808966] [c146fd80] [c02846e8] proc_register+0x1a8/0x1ac (unreliable) [1.815659] [c146fdb0] [c028481c] _proc_mkdir+0x78/0xa4 [1.820875] [c146fdd0] [c05a92e4] pci_proc_attach_device+0x11c/0x168 [1.827221] [c146fe10] [c101f7a4] pci_proc_init+0x80/0x98 [1.832611] [c146fe30] [c0004150] do_one_initcall+0x80/0x284 [1.838262] [c146fea0] [c10011a8] kernel_init_freeable+0x1f4/0x2a0 [1.844434] [c146fee0] [c000456c] kernel_init+0x24/0x150 [1.849737] [c146ff00] [c001326c] ret_from_kernel_thread+0x5c/0x64 [1.855910] Instruction dump: [1.858866] 83810020 83a10024 83c10028 83e1002c 38210030 4e800020 809a0064 3c60c0a8 [1.866602] 7f85e378 3863af28 4cc63182 4bdb8155 <0fe0> 9421ffe0 3920 7c0802a6 [1.874513] ---[ end trace ]--- This regression started appearing after commit 566356813082 ("powerpc/pci: Add config option for using all 256 PCI buses") in case in each mPCIe slot is connected PCIe card and therefore PCI bus 1 is populated in for every PCIe controller / PCI domain. The reason is that PCI procfs code expects that when PCI bus numbers are not unique across all PCI domains, function pci_proc_domain() returns true for domain dependent buses. Fix this issue by setting PCI_ENABLE_PROC_DOMAINS and PCI_COMPAT_DOMAIN_0 flags for 32-bit powerpc code when CONFIG_PPC_PCI_BUS_NUM_DOMAIN_DEPENDENT is enabled. Same approach is already implemented for 64-bit powerpc code (where PCI bus numbers are always domain dependent). Fixes: 566356813082 ("powerpc/pci: Add config option for using all 256 PCI buses") Signed-off-by: Pali Rohár --- arch/powerpc/kernel/pci_32.c | 9 + 1 file changed, 9 insertions(+) diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c index ffc4e1928c80..8acbc9592ebb 100644 --- a/arch/powerpc/kernel/pci_32.c +++ b/arch/powerpc/kernel/pci_32.c @@ -249,6 +249,15 @@ static int __init pcibios_init(void) printk(KERN_INFO "PCI: Probing PCI hardware\n"); +#ifdef CONFIG_PPC_PCI_BUS_NUM_DOMAIN_DEPENDENT + /* +* Enable PCI domains in /proc when PCI bus numbers are not unique +* across all PCI domains to prevent conflicts. And keep PCI domain 0 +* backward compatible in /proc for video cards. +*/ + pci_add_flags(PCI_ENABLE_PROC_DOMAINS | PCI_COMPAT_DOMAIN_0); +#endif + if (pci_has_flag(PCI_REASSIGN_ALL_BUS)) pci_assign_all_buses = 1; -- 2.20.1