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
