[PATCH] Bugfix: powerpc/eeh: Create eeh sysfs entry in post_init()

[PATCH] Bugfix: powerpc/eeh: Create eeh sysfs entry in post_init()

[Mike Qiu]

Eeh sysfs entry created must be after EEH_ENABLED been set
in eeh_subsystem_flags.

In PowerNV platform, it try to create sysfs entry before
EEH_ENABLED been set, when boot up. So nothing will be
created for eeh in sysfs.

Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/eeh-ioda.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
index 8ad0c5b..5f95581 100644
--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
@@ -136,6 +136,9 @@ static int ioda_eeh_post_init(struct pci_controller *hose)
 	struct pnv_phb *phb = hose->private_data;
 	int ret;
 
+	/* Creat sysfs after EEH_ENABLED been set */
+	eeh_add_sysfs_files(hose->bus);
+
 	/* Register OPAL event notifier */
 	if (!ioda_eeh_nb_init) {
 		ret = opal_notifier_register(&ioda_eeh_nb);
-- 
1.8.1.4

[PATCH] Bugfix: powerpc/eeh: Create eeh sysfs entry in post_init()

[Mike Qiu]

Eeh sysfs entry created must be after EEH_ENABLED been set
in eeh_subsystem_flags.

In PowerNV platform, it try to create sysfs entry before
EEH_ENABLED been set, when boot up. So nothing will be
created for eeh in sysfs.

Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/eeh-ioda.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
index 8ad0c5b..5f95581 100644
--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
@@ -136,6 +136,9 @@ static int ioda_eeh_post_init(struct pci_controller *hose)
 	struct pnv_phb *phb = hose->private_data;
 	int ret;
 
+	/* Creat sysfs after EEH_ENABLED been set */
+	eeh_add_sysfs_files(hose->bus);
+
 	/* Register OPAL event notifier */
 	if (!ioda_eeh_nb_init) {
 		ret = opal_notifier_register(&ioda_eeh_nb);
-- 
1.8.1.4

Re: [PATCH] Bugfix: powerpc/eeh: Create eeh sysfs entry in post_init()

[Gavin Shan]

On Tue, Jun 24, 2014 at 11:32:07PM -0400, Mike Qiu wrote:

[ cc Richard ]

>Eeh sysfs entry created must be after EEH_ENABLED been set
>in eeh_subsystem_flags.
>
>In PowerNV platform, it try to create sysfs entry before
>EEH_ENABLED been set, when boot up. So nothing will be
>created for eeh in sysfs.
>

Could you please make the commit log more clear? :-)

I guess the issue is introduced by commit 2213fb1 ("
powerpc/eeh: Skip eeh sysfs when eeh is disabled"). The
commit checks EEH is enabled while creating PCI device
EEH sysfs files. If not, the sysfs files won't be created.
That's to avoid warning reported during PCI hotplug.

The problem you're reporting (if I understand completely):
You don't see the sysfs files after the system boots up.
If it's the case, you probably need following changes in
arch/powerpc/platforms/powernv/pci.c::pnv_pci_ioda_fixup().
Could you have a try with it?

#ifdef CONFIG_EEH
	eeh_probe_mode_set(EEH_PROBE_MODE_DEV);
-	eeh_addr_cache_build();
	eeh_init();
+	eeh_addr_cache_build();
#endif

Eventually PowerNV/pSeries have same function call sequence:

- Set EEH probe mode
- Doing probe (with device node or PCI device)
- Build address cache.
 
>Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
>---
> arch/powerpc/platforms/powernv/eeh-ioda.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
>index 8ad0c5b..5f95581 100644
>--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
>+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
>@@ -136,6 +136,9 @@ static int ioda_eeh_post_init(struct pci_controller *hose)
> 	struct pnv_phb *phb = hose->private_data;
> 	int ret;
>
>+	/* Creat sysfs after EEH_ENABLED been set */
>+	eeh_add_sysfs_files(hose->bus);
>+
> 	/* Register OPAL event notifier */
> 	if (!ioda_eeh_nb_init) {
> 		ret = opal_notifier_register(&ioda_eeh_nb);

Thanks,
Gavin

Re: [PATCH] Bugfix: powerpc/eeh: Create eeh sysfs entry in post_init()

[Gavin Shan]

On Tue, Jun 24, 2014 at 11:32:07PM -0400, Mike Qiu wrote:

[ cc Richard ]

>Eeh sysfs entry created must be after EEH_ENABLED been set
>in eeh_subsystem_flags.
>
>In PowerNV platform, it try to create sysfs entry before
>EEH_ENABLED been set, when boot up. So nothing will be
>created for eeh in sysfs.
>

Could you please make the commit log more clear? :-)

I guess the issue is introduced by commit 2213fb1 ("
powerpc/eeh: Skip eeh sysfs when eeh is disabled"). The
commit checks EEH is enabled while creating PCI device
EEH sysfs files. If not, the sysfs files won't be created.
That's to avoid warning reported during PCI hotplug.

The problem you're reporting (if I understand completely):
You don't see the sysfs files after the system boots up.
If it's the case, you probably need following changes in
arch/powerpc/platforms/powernv/pci.c::pnv_pci_ioda_fixup().
Could you have a try with it?

#ifdef CONFIG_EEH
	eeh_probe_mode_set(EEH_PROBE_MODE_DEV);
-	eeh_addr_cache_build();
	eeh_init();
+	eeh_addr_cache_build();
#endif

Eventually PowerNV/pSeries have same function call sequence:

- Set EEH probe mode
- Doing probe (with device node or PCI device)
- Build address cache.
 
>Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
>---
> arch/powerpc/platforms/powernv/eeh-ioda.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
>index 8ad0c5b..5f95581 100644
>--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
>+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
>@@ -136,6 +136,9 @@ static int ioda_eeh_post_init(struct pci_controller *hose)
> 	struct pnv_phb *phb = hose->private_data;
> 	int ret;
>
>+	/* Creat sysfs after EEH_ENABLED been set */
>+	eeh_add_sysfs_files(hose->bus);
>+
> 	/* Register OPAL event notifier */
> 	if (!ioda_eeh_nb_init) {
> 		ret = opal_notifier_register(&ioda_eeh_nb);

Thanks,
Gavin

Re: [PATCH] Bugfix: powerpc/eeh: Create eeh sysfs entry in post_init()

[Wei Yang]

On Wed, Jun 25, 2014 at 03:33:12PM +1000, Gavin Shan wrote:
>On Tue, Jun 24, 2014 at 11:32:07PM -0400, Mike Qiu wrote:
>
>[ cc Richard ]
>
>>Eeh sysfs entry created must be after EEH_ENABLED been set
>>in eeh_subsystem_flags.
>>
>>In PowerNV platform, it try to create sysfs entry before
>>EEH_ENABLED been set, when boot up. So nothing will be
>>created for eeh in sysfs.
>>
>
>Could you please make the commit log more clear? :-)
>
>I guess the issue is introduced by commit 2213fb1 ("
>powerpc/eeh: Skip eeh sysfs when eeh is disabled"). The
>commit checks EEH is enabled while creating PCI device
>EEH sysfs files. If not, the sysfs files won't be created.
>That's to avoid warning reported during PCI hotplug.
>
>The problem you're reporting (if I understand completely):
>You don't see the sysfs files after the system boots up.
>If it's the case, you probably need following changes in
>arch/powerpc/platforms/powernv/pci.c::pnv_pci_ioda_fixup().
>Could you have a try with it?
>
>#ifdef CONFIG_EEH
>	eeh_probe_mode_set(EEH_PROBE_MODE_DEV);
>-	eeh_addr_cache_build();
>	eeh_init();
>+	eeh_addr_cache_build();
>#endif
>

I think this is a more proper fix.

BTW, I have one confusion in this mode set.

eeh_init()
  -> eeh_ops->dev_probe()
     -> powernv_eeh_dev_probe()
        -> eeh_set_enable(true)   <- here the eeh is marked enabled

We can see this flag would be set for each pci_dev. So is it possible to make
this "set" only once?

>Eventually PowerNV/pSeries have same function call sequence:
>
>- Set EEH probe mode
>- Doing probe (with device node or PCI device)
>- Build address cache.
>
>>Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
>>---
>> arch/powerpc/platforms/powernv/eeh-ioda.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>>diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
>>index 8ad0c5b..5f95581 100644
>>--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
>>+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
>>@@ -136,6 +136,9 @@ static int ioda_eeh_post_init(struct pci_controller *hose)
>> 	struct pnv_phb *phb = hose->private_data;
>> 	int ret;
>>
>>+	/* Creat sysfs after EEH_ENABLED been set */
>>+	eeh_add_sysfs_files(hose->bus);
>>+
>> 	/* Register OPAL event notifier */
>> 	if (!ioda_eeh_nb_init) {
>> 		ret = opal_notifier_register(&ioda_eeh_nb);
>
>Thanks,
>Gavin

-- 
Richard Yang
Help you, Help me

Re: [PATCH] Bugfix: powerpc/eeh: Create eeh sysfs entry in post_init()

[Wei Yang]

On Wed, Jun 25, 2014 at 03:33:12PM +1000, Gavin Shan wrote:
>On Tue, Jun 24, 2014 at 11:32:07PM -0400, Mike Qiu wrote:
>
>[ cc Richard ]
>
>>Eeh sysfs entry created must be after EEH_ENABLED been set
>>in eeh_subsystem_flags.
>>
>>In PowerNV platform, it try to create sysfs entry before
>>EEH_ENABLED been set, when boot up. So nothing will be
>>created for eeh in sysfs.
>>
>
>Could you please make the commit log more clear? :-)
>
>I guess the issue is introduced by commit 2213fb1 ("
>powerpc/eeh: Skip eeh sysfs when eeh is disabled"). The
>commit checks EEH is enabled while creating PCI device
>EEH sysfs files. If not, the sysfs files won't be created.
>That's to avoid warning reported during PCI hotplug.
>
>The problem you're reporting (if I understand completely):
>You don't see the sysfs files after the system boots up.
>If it's the case, you probably need following changes in
>arch/powerpc/platforms/powernv/pci.c::pnv_pci_ioda_fixup().
>Could you have a try with it?
>
>#ifdef CONFIG_EEH
>	eeh_probe_mode_set(EEH_PROBE_MODE_DEV);
>-	eeh_addr_cache_build();
>	eeh_init();
>+	eeh_addr_cache_build();
>#endif
>

I think this is a more proper fix.

BTW, I have one confusion in this mode set.

eeh_init()
  -> eeh_ops->dev_probe()
     -> powernv_eeh_dev_probe()
        -> eeh_set_enable(true)   <- here the eeh is marked enabled

We can see this flag would be set for each pci_dev. So is it possible to make
this "set" only once?

>Eventually PowerNV/pSeries have same function call sequence:
>
>- Set EEH probe mode
>- Doing probe (with device node or PCI device)
>- Build address cache.
>
>>Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
>>---
>> arch/powerpc/platforms/powernv/eeh-ioda.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>>diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
>>index 8ad0c5b..5f95581 100644
>>--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
>>+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
>>@@ -136,6 +136,9 @@ static int ioda_eeh_post_init(struct pci_controller *hose)
>> 	struct pnv_phb *phb = hose->private_data;
>> 	int ret;
>>
>>+	/* Creat sysfs after EEH_ENABLED been set */
>>+	eeh_add_sysfs_files(hose->bus);
>>+
>> 	/* Register OPAL event notifier */
>> 	if (!ioda_eeh_nb_init) {
>> 		ret = opal_notifier_register(&ioda_eeh_nb);
>
>Thanks,
>Gavin

-- 
Richard Yang
Help you, Help me

Re: [PATCH] Bugfix: powerpc/eeh: Create eeh sysfs entry in post_init()

[Gavin Shan]

On Wed, Jun 25, 2014 at 02:23:53PM +0800, Wei Yang wrote:
>On Wed, Jun 25, 2014 at 03:33:12PM +1000, Gavin Shan wrote:
>>On Tue, Jun 24, 2014 at 11:32:07PM -0400, Mike Qiu wrote:
>>
>>[ cc Richard ]
>>
>>>Eeh sysfs entry created must be after EEH_ENABLED been set
>>>in eeh_subsystem_flags.
>>>
>>>In PowerNV platform, it try to create sysfs entry before
>>>EEH_ENABLED been set, when boot up. So nothing will be
>>>created for eeh in sysfs.
>>>
>>
>>Could you please make the commit log more clear? :-)
>>
>>I guess the issue is introduced by commit 2213fb1 ("
>>powerpc/eeh: Skip eeh sysfs when eeh is disabled"). The
>>commit checks EEH is enabled while creating PCI device
>>EEH sysfs files. If not, the sysfs files won't be created.
>>That's to avoid warning reported during PCI hotplug.
>>
>>The problem you're reporting (if I understand completely):
>>You don't see the sysfs files after the system boots up.
>>If it's the case, you probably need following changes in
>>arch/powerpc/platforms/powernv/pci.c::pnv_pci_ioda_fixup().
>>Could you have a try with it?
>>
>>#ifdef CONFIG_EEH
>>	eeh_probe_mode_set(EEH_PROBE_MODE_DEV);
>>-	eeh_addr_cache_build();
>>	eeh_init();
>>+	eeh_addr_cache_build();
>>#endif
>>
>
>I think this is a more proper fix.
>
>BTW, I have one confusion in this mode set.
>
>eeh_init()
>  -> eeh_ops->dev_probe()
>     -> powernv_eeh_dev_probe()
>        -> eeh_set_enable(true)   <- here the eeh is marked enabled
>
>We can see this flag would be set for each pci_dev. So is it possible to make
>this "set" only once?
>

It shouldn't be a problem because there might not have PCI devices
supporting EEH in the guest. All PCI devices are emulated.

>>Eventually PowerNV/pSeries have same function call sequence:
>>
>>- Set EEH probe mode
>>- Doing probe (with device node or PCI device)
>>- Build address cache.
>>
>>>Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
>>>---
>>> arch/powerpc/platforms/powernv/eeh-ioda.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>>diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
>>>index 8ad0c5b..5f95581 100644
>>>--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
>>>+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
>>>@@ -136,6 +136,9 @@ static int ioda_eeh_post_init(struct pci_controller *hose)
>>> 	struct pnv_phb *phb = hose->private_data;
>>> 	int ret;
>>>
>>>+	/* Creat sysfs after EEH_ENABLED been set */
>>>+	eeh_add_sysfs_files(hose->bus);
>>>+
>>> 	/* Register OPAL event notifier */
>>> 	if (!ioda_eeh_nb_init) {
>>> 		ret = opal_notifier_register(&ioda_eeh_nb);

Thanks,
Gavin

Re: [PATCH] Bugfix: powerpc/eeh: Create eeh sysfs entry in post_init()

[Gavin Shan]

On Wed, Jun 25, 2014 at 02:23:53PM +0800, Wei Yang wrote:
>On Wed, Jun 25, 2014 at 03:33:12PM +1000, Gavin Shan wrote:
>>On Tue, Jun 24, 2014 at 11:32:07PM -0400, Mike Qiu wrote:
>>
>>[ cc Richard ]
>>
>>>Eeh sysfs entry created must be after EEH_ENABLED been set
>>>in eeh_subsystem_flags.
>>>
>>>In PowerNV platform, it try to create sysfs entry before
>>>EEH_ENABLED been set, when boot up. So nothing will be
>>>created for eeh in sysfs.
>>>
>>
>>Could you please make the commit log more clear? :-)
>>
>>I guess the issue is introduced by commit 2213fb1 ("
>>powerpc/eeh: Skip eeh sysfs when eeh is disabled"). The
>>commit checks EEH is enabled while creating PCI device
>>EEH sysfs files. If not, the sysfs files won't be created.
>>That's to avoid warning reported during PCI hotplug.
>>
>>The problem you're reporting (if I understand completely):
>>You don't see the sysfs files after the system boots up.
>>If it's the case, you probably need following changes in
>>arch/powerpc/platforms/powernv/pci.c::pnv_pci_ioda_fixup().
>>Could you have a try with it?
>>
>>#ifdef CONFIG_EEH
>>	eeh_probe_mode_set(EEH_PROBE_MODE_DEV);
>>-	eeh_addr_cache_build();
>>	eeh_init();
>>+	eeh_addr_cache_build();
>>#endif
>>
>
>I think this is a more proper fix.
>
>BTW, I have one confusion in this mode set.
>
>eeh_init()
>  -> eeh_ops->dev_probe()
>     -> powernv_eeh_dev_probe()
>        -> eeh_set_enable(true)   <- here the eeh is marked enabled
>
>We can see this flag would be set for each pci_dev. So is it possible to make
>this "set" only once?
>

It shouldn't be a problem because there might not have PCI devices
supporting EEH in the guest. All PCI devices are emulated.

>>Eventually PowerNV/pSeries have same function call sequence:
>>
>>- Set EEH probe mode
>>- Doing probe (with device node or PCI device)
>>- Build address cache.
>>
>>>Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
>>>---
>>> arch/powerpc/platforms/powernv/eeh-ioda.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>>diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
>>>index 8ad0c5b..5f95581 100644
>>>--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
>>>+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
>>>@@ -136,6 +136,9 @@ static int ioda_eeh_post_init(struct pci_controller *hose)
>>> 	struct pnv_phb *phb = hose->private_data;
>>> 	int ret;
>>>
>>>+	/* Creat sysfs after EEH_ENABLED been set */
>>>+	eeh_add_sysfs_files(hose->bus);
>>>+
>>> 	/* Register OPAL event notifier */
>>> 	if (!ioda_eeh_nb_init) {
>>> 		ret = opal_notifier_register(&ioda_eeh_nb);

Thanks,
Gavin

Re: [PATCH] Bugfix: powerpc/eeh: Create eeh sysfs entry in post_init()

[Mike Qiu]

On 06/25/2014 01:33 PM, Gavin Shan wrote:
> On Tue, Jun 24, 2014 at 11:32:07PM -0400, Mike Qiu wrote:
>
> [ cc Richard ]
>
>> Eeh sysfs entry created must be after EEH_ENABLED been set
>> in eeh_subsystem_flags.
>>
>> In PowerNV platform, it try to create sysfs entry before
>> EEH_ENABLED been set, when boot up. So nothing will be
>> created for eeh in sysfs.
>>
> Could you please make the commit log more clear? :-)
>
> I guess the issue is introduced by commit 2213fb1 ("
> powerpc/eeh: Skip eeh sysfs when eeh is disabled"). The
> commit checks EEH is enabled while creating PCI device
> EEH sysfs files. If not, the sysfs files won't be created.
> That's to avoid warning reported during PCI hotplug.
>
> The problem you're reporting (if I understand completely):
> You don't see the sysfs files after the system boots up.
> If it's the case, you probably need following changes in
> arch/powerpc/platforms/powernv/pci.c::pnv_pci_ioda_fixup().
> Could you have a try with it?
>
> #ifdef CONFIG_EEH
> 	eeh_probe_mode_set(EEH_PROBE_MODE_DEV);
> -	eeh_addr_cache_build();
> 	eeh_init();
> +	eeh_addr_cache_build();
> #endif

But this was not work, as I test, see boot log below:

[    0.233993] Unable to handle kernel paging request for data at 
address 0x00000010
[    0.234086] Faulting instruction address: 0xc000000000036c84
[    0.234144] Oops: Kernel access of bad area, sig: 11 [#1]
[    0.234188] SMP NR_CPUS=1024 NUMA PowerNV
[    0.234235] Modules linked in:
[    0.234282] CPU: 4 PID: 1 Comm: swapper/0 Not tainted 3.16.0-rc1+ #61
[    0.234339] task: c0000003bfcc0000 ti: c0000003bfd00000 task.ti: 
c0000003bfd00000
[    0.234405] NIP: c000000000036c84 LR: c000000000036c4c CTR: 
0000000000000000
[    0.234472] REGS: c0000003bfd03430 TRAP: 0300   Not tainted (3.16.0-rc1+)
[    0.234528] MSR: 9000000000009032 <SF,HV,EE,ME,IR,DR,RI> CR: 
44008088  XER: 00000000
[    0.234686] CFAR: c000000000009358 DAR: 0000000000000010 DSISR: 
40000000 SOFTE: 1
GPR00: c000000000036c4c c0000003bfd036b0 c000000001448d58 c0000003bce30080
GPR04: 0000000000000000 0000000000000000 0000000000000001 c0000003bce300c8
GPR08: c0000003bce300e8 0000000000000000 0000000000000000 000000003030f000
GPR12: 0000000022008042 c00000000fee1200 c000000000b0e1f0 0000000000000000
GPR16: f000000000019600 0000000000000008 000000000000003f c000000003022280
GPR20: c000000000b0e058 0000000000400000 0000000000000008 0000000000000007
GPR24: c000000003120f80 c000000000b0e2d0 c0000000013bc6f0 c0000003bca18400
GPR28: 0000000000000000 c000000003010000 c0000003bce30080 c0000003bb2c3b40
[    0.235582] NIP [c000000000036c84] .eeh_add_to_parent_pe+0x164/0x340
[    0.235639] LR [c000000000036c4c] .eeh_add_to_parent_pe+0x12c/0x340
[    0.235695] Call Trace:
[    0.235719] [c0000003bfd036b0] [c000000000036c4c] 
.eeh_add_to_parent_pe+0x12c/0x340 (unreliable)
[    0.235810] [c0000003bfd03730] [c000000000070ee8] 
.powernv_eeh_dev_probe+0x158/0x1d0
[    0.235890] [c0000003bfd037c0] [c00000000048768c] 
.pci_walk_bus+0x8c/0x120
[    0.235957] [c0000003bfd03860] [c0000000000341c4] .eeh_init+0xf4/0x310
[    0.236025] [c0000003bfd03900] [c00000000006e7a8] 
.pnv_pci_ioda_fixup+0x688/0xb30
[    0.236105] [c0000003bfd03a60] [c000000000c2ee90] 
.pcibios_resource_survey+0x334/0x3f4
[    0.236183] [c0000003bfd03b50] [c000000000c2e65c] .pcibios_init+0xa0/0xd4
[    0.236251] [c0000003bfd03be0] [c00000000000bc94] 
.do_one_initcall+0x124/0x280
[    0.236329] [c0000003bfd03cd0] [c000000000c24acc] 
.kernel_init_freeable+0x250/0x348
[    0.236408] [c0000003bfd03db0] [c00000000000c4c4] .kernel_init+0x24/0x140
[    0.236475] [c0000003bfd03e30] [c00000000000a45c] 
.ret_from_kernel_thread+0x58/0x7c
[    0.236553] Instruction dump:
[    0.236586] 815f000c 60000000 e9228890 915e000c 81290000 7926f7e3 
813f0008 913e0008
[    0.236698] 41820018 2fbf0000 419e0154 e93f0088 <e9290010> f93e0018 
e93f0080 48000034
[    0.236819] ---[ end trace e78b31e354e84859 ]---
[    0.236864]
[    2.236933] Kernel panic - not syncing: Attempted to kill init! 
exitcode=0x0000000b

This may because  edev->pdev is set in eeh_addr_cache_build(), while 
eeh_init() use that entry.

After changed the code, the call patch:

eeh_init() ---->
                         pci_walk_bus()---->
powernv_eeh_dev_probe() ----->
                                        eeh_add_to_parent_pe()
eeh_addr_cache_build()

We can see in
eeh_add_to_parent_pe() {
......
pe->bus = eeh_dev_to_pci_dev(edev)->bus;
......
}

That is sure eeh_dev_to_pci_dev(edev) will be *NULL*, because this is 
set in  eeh_addr_cache_build()


Thanks
Mike
> Eventually PowerNV/pSeries have same function call sequence:
>
> - Set EEH probe mode
> - Doing probe (with device node or PCI device)
> - Build address cache.
>
>> Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/platforms/powernv/eeh-ioda.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
>> index 8ad0c5b..5f95581 100644
>> --- a/arch/powerpc/platforms/powernv/eeh-ioda.c
>> +++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
>> @@ -136,6 +136,9 @@ static int ioda_eeh_post_init(struct pci_controller *hose)
>> 	struct pnv_phb *phb = hose->private_data;
>> 	int ret;
>>
>> +	/* Creat sysfs after EEH_ENABLED been set */
>> +	eeh_add_sysfs_files(hose->bus);
>> +
>> 	/* Register OPAL event notifier */
>> 	if (!ioda_eeh_nb_init) {
>> 		ret = opal_notifier_register(&ioda_eeh_nb);
> Thanks,
> Gavin
>
>

Re: [PATCH] Bugfix: powerpc/eeh: Create eeh sysfs entry in post_init()

[Gavin Shan]

[-- Attachment #1: Type: text/plain, Size: 1519 bytes --]

On Wed, Jun 25, 2014 at 03:27:55PM +0800, Mike Qiu wrote:
>On 06/25/2014 01:33 PM, Gavin Shan wrote:
>>On Tue, Jun 24, 2014 at 11:32:07PM -0400, Mike Qiu wrote:
>>
>>[ cc Richard ]
>>
>>>Eeh sysfs entry created must be after EEH_ENABLED been set
>>>in eeh_subsystem_flags.
>>>
>>>In PowerNV platform, it try to create sysfs entry before
>>>EEH_ENABLED been set, when boot up. So nothing will be
>>>created for eeh in sysfs.
>>>
>>Could you please make the commit log more clear? :-)
>>
>>I guess the issue is introduced by commit 2213fb1 ("
>>powerpc/eeh: Skip eeh sysfs when eeh is disabled"). The
>>commit checks EEH is enabled while creating PCI device
>>EEH sysfs files. If not, the sysfs files won't be created.
>>That's to avoid warning reported during PCI hotplug.
>>
>>The problem you're reporting (if I understand completely):
>>You don't see the sysfs files after the system boots up.
>>If it's the case, you probably need following changes in
>>arch/powerpc/platforms/powernv/pci.c::pnv_pci_ioda_fixup().
>>Could you have a try with it?
>>
>>#ifdef CONFIG_EEH
>>	eeh_probe_mode_set(EEH_PROBE_MODE_DEV);
>>-	eeh_addr_cache_build();
>>	eeh_init();
>>+	eeh_addr_cache_build();
>>#endif
>
>But this was not work, as I test, see boot log below:
>

Yeah, we can't convert eeh_dev to pci_dev that time. The
association is populated by eeh_addr_cache_build(). The
attached patch should fix your issue. I tried on P7 machine
and sysfs entries created. Could you help having a test
on your machine? :-)

Thanks,
Gavin

[-- Attachment #2: 0001-powerpc-eeh-sysfs-entries-lost.patch --]
[-- Type: text/x-diff, Size: 3740 bytes --]

>From 70b8ac1d64192954e04bc4a4f2736349d7df6a8a Mon Sep 17 00:00:00 2001
From: Gavin Shan <gwshan@linux.vnet.ibm.com>
Date: Thu, 26 Jun 2014 09:50:25 +1000
Subject: [PATCH] powerpc/eeh: sysfs entries lost

The sysfs entries are lost because of commit 2213fb1 ("powerpc/eeh:
Skip eeh sysfs when eeh is disabled"). That commit added condition
to create sysfs entries with EEH_ENABLED, which isn't populated
when trying to create sysfs entries on PowerNV platform during system
boot time. The patch fixes the issue by:

   * Reoder EEH initialization functions so that they're same on
     PowerNV/pSeries.
   * Cache PE's primary bus by PowerNV platform instead of EEH core
     to avoid kernel crash caused by the function reorder. Another
     benefit with this is to avoid one eeh_probe_mode_dev() in EEH
     core.

Reported-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/eeh_pe.c                 | 11 -----------
 arch/powerpc/platforms/powernv/eeh-powernv.c | 17 ++++++++++++++++-
 arch/powerpc/platforms/powernv/pci-ioda.c    |  2 +-
 3 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index fbd01eb..1dce071a 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -351,17 +351,6 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
 	pe->config_addr	= edev->config_addr;
 
 	/*
-	 * While doing PE reset, we probably hot-reset the
-	 * upstream bridge. However, the PCI devices including
-	 * the associated EEH devices might be removed when EEH
-	 * core is doing recovery. So that won't safe to retrieve
-	 * the bridge through downstream EEH device. We have to
-	 * trace the parent PCI bus, then the upstream bridge.
-	 */
-	if (eeh_probe_mode_dev())
-		pe->bus = eeh_dev_to_pci_dev(edev)->bus;
-
-	/*
 	 * Put the new EEH PE into hierarchy tree. If the parent
 	 * can't be found, the newly created PE will be attached
 	 * to PHB directly. Otherwise, we have to associate the
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 56a206f..48eb223 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -107,6 +107,7 @@ static int powernv_eeh_dev_probe(struct pci_dev *dev, void *flag)
 	struct pnv_phb *phb = hose->private_data;
 	struct device_node *dn = pci_device_to_OF_node(dev);
 	struct eeh_dev *edev = of_node_to_eeh_dev(dn);
+	int ret;
 
 	/*
 	 * When probing the root bridge, which doesn't have any
@@ -143,7 +144,21 @@ static int powernv_eeh_dev_probe(struct pci_dev *dev, void *flag)
 	edev->pe_config_addr	= phb->bdfn_to_pe(phb, dev->bus, dev->devfn & 0xff);
 
 	/* Create PE */
-	eeh_add_to_parent_pe(edev);
+	ret = eeh_add_to_parent_pe(edev);
+	if (ret) {
+		pr_warn("%s: Can't add PCI dev %s to parent PE (%d)\n",
+			__func__, pci_name(dev), ret);
+		return ret;
+	}
+
+	/*
+	 * Cache the PE primary bus, which can't be fetched when
+	 * full hotplug is in progress. In that case, all child
+	 * PCI devices of the PE are expected to be removed prior
+	 * to PE reset.
+	 */
+	if (!edev->pe->bus)
+		edev->pe->bus = dev->bus;
 
 	/*
 	 * Enable EEH explicitly so that we will do EEH check
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index de19ede..81f2d3a 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1142,8 +1142,8 @@ static void pnv_pci_ioda_fixup(void)
 
 #ifdef CONFIG_EEH
 	eeh_probe_mode_set(EEH_PROBE_MODE_DEV);
-	eeh_addr_cache_build();
 	eeh_init();
+	eeh_addr_cache_build();
 #endif
 }
 
-- 
1.8.3.2

Re: [PATCH] Bugfix: powerpc/eeh: Create eeh sysfs entry in post_init()

[Mike Qiu]

On 06/26/2014 08:12 AM, Gavin Shan wrote:
> On Wed, Jun 25, 2014 at 03:27:55PM +0800, Mike Qiu wrote:
>> On 06/25/2014 01:33 PM, Gavin Shan wrote:
>>> On Tue, Jun 24, 2014 at 11:32:07PM -0400, Mike Qiu wrote:
>>>
>>> [ cc Richard ]
>>>
>>>> Eeh sysfs entry created must be after EEH_ENABLED been set
>>>> in eeh_subsystem_flags.
>>>>
>>>> In PowerNV platform, it try to create sysfs entry before
>>>> EEH_ENABLED been set, when boot up. So nothing will be
>>>> created for eeh in sysfs.
>>>>
>>> Could you please make the commit log more clear? :-)
>>>
>>> I guess the issue is introduced by commit 2213fb1 ("
>>> powerpc/eeh: Skip eeh sysfs when eeh is disabled"). The
>>> commit checks EEH is enabled while creating PCI device
>>> EEH sysfs files. If not, the sysfs files won't be created.
>>> That's to avoid warning reported during PCI hotplug.
>>>
>>> The problem you're reporting (if I understand completely):
>>> You don't see the sysfs files after the system boots up.
>>> If it's the case, you probably need following changes in
>>> arch/powerpc/platforms/powernv/pci.c::pnv_pci_ioda_fixup().
>>> Could you have a try with it?
>>>
>>> #ifdef CONFIG_EEH
>>> 	eeh_probe_mode_set(EEH_PROBE_MODE_DEV);
>>> -	eeh_addr_cache_build();
>>> 	eeh_init();
>>> +	eeh_addr_cache_build();
>>> #endif
>> But this was not work, as I test, see boot log below:
>>
> Yeah, we can't convert eeh_dev to pci_dev that time. The
> association is populated by eeh_addr_cache_build(). The
> attached patch should fix your issue. I tried on P7 machine
> and sysfs entries created. Could you help having a test
> on your machine? :-)

I have tested, works good.

Thanks
Mike
>
> Thanks,
> Gavin