| Message ID | 1403667127-1622-1-git-send-email-qiudayu@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
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
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
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
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 > >
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);
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(+)