linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2] powerpc/eeh: Fix kernel crash when passing through VF
@ 2014-09-11  3:42 Wei Yang
  2014-09-12  3:55 ` Michael Ellerman
  2014-09-15  8:08 ` [PATCH] " Wei Yang
  0 siblings, 2 replies; 11+ messages in thread
From: Wei Yang @ 2014-09-11  3:42 UTC (permalink / raw)
  To: gwshan, linuxppc-dev; +Cc: Wei Yang

When doing vfio passthrough a VF, the kernel will crash with following
message:

[  442.656459] Unable to handle kernel paging request for data at address 0x00000060
[  442.656593] Faulting instruction address: 0xc000000000038b88
[  442.656706] Oops: Kernel access of bad area, sig: 11 [#1]
[  442.656798] SMP NR_CPUS=1024 NUMA PowerNV
[  442.656890] Modules linked in: vfio_pci mlx4_core nf_conntrack_netbios_ns nf_conntrack_broadcast ipt_MASQUERADE ip6t_REJECT xt_conntrack bnep bluetooth rfkill ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle iptable_security iptable_raw tg3 nfsd be2net nfs_acl ses lockd ptp enclosure pps_core kvm_hv kvm_pr shpchp binfmt_misc kvm sunrpc uinput lpfc scsi_transport_fc ipr scsi_tgt [last unloaded: mlx4_core]
[  442.658152] CPU: 40 PID: 14948 Comm: qemu-system-ppc Not tainted 3.10.42yw-pkvm+ #37
[  442.658219] task: c000000f7e2a9a00 ti: c000000f6dc3c000 task.ti: c000000f6dc3c000
[  442.658287] NIP: c000000000038b88 LR: c0000000004435a8 CTR: c000000000455bc0
[  442.658352] REGS: c000000f6dc3f580 TRAP: 0300   Not tainted  (3.10.42yw-pkvm+)
[  442.658419] MSR: 9000000000009032 <SF,HV,EE,ME,IR,DR,RI>  CR: 28004882  XER: 20000000
[  442.658577] CFAR: c00000000000908c DAR: 0000000000000060 DSISR: 40000000 SOFTE: 1
GPR00: c0000000004435a8 c000000f6dc3f800 c0000000012b1c10 c00000000da24000
GPR04: 0000000000000003 0000000000001004 00000000000015b3 000000000000ffff
GPR08: c00000000127f5d8 0000000000000000 000000000000ffff 0000000000000000
GPR12: c000000000068078 c00000000fdd6800 000001003c320c80 000001003c3607f0
GPR16: 0000000000000001 00000000105480c8 000000001055aaa8 000001003c31ab18
GPR20: 000001003c10fb40 000001003c360ae8 000000001063bcf0 000000001063bdb0
GPR24: 000001003c15ed70 0000000010548f40 c000001fe5514c88 c000001fe5514cb0
GPR28: c00000000da24000 0000000000000000 c00000000da24000 0000000000000003
[  442.659471] NIP [c000000000038b88] .pcibios_set_pcie_reset_state+0x28/0x130
[  442.659530] LR [c0000000004435a8] .pci_set_pcie_reset_state+0x28/0x40
[  442.659585] Call Trace:
[  442.659610] [c000000f6dc3f800] [00000000000719e0] 0x719e0 (unreliable)
[  442.659677] [c000000f6dc3f880] [c0000000004435a8] .pci_set_pcie_reset_state+0x28/0x40
[  442.659757] [c000000f6dc3f900] [c000000000455bf8] .reset_fundamental+0x38/0x80
[  442.659835] [c000000f6dc3f980] [c0000000004562a8] .pci_dev_specific_reset+0xa8/0xf0
[  442.659913] [c000000f6dc3fa00] [c0000000004448c4] .__pci_dev_reset+0x44/0x430
[  442.659980] [c000000f6dc3fab0] [c000000000444d5c] .pci_reset_function+0x7c/0xc0
[  442.660059] [c000000f6dc3fb30] [d00000001c141ab8] .vfio_pci_open+0xe8/0x2b0 [vfio_pci]
[  442.660139] [c000000f6dc3fbd0] [c000000000586c30] .vfio_group_fops_unl_ioctl+0x3a0/0x630
[  442.660219] [c000000f6dc3fc90] [c000000000255fbc] .do_vfs_ioctl+0x4ec/0x7c0
[  442.660286] [c000000f6dc3fd80] [c000000000256364] .SyS_ioctl+0xd4/0xf0
[  442.660354] [c000000f6dc3fe30] [c000000000009e54] syscall_exit+0x0/0x98
[  442.660420] Instruction dump:
[  442.660454] 4bfffce9 4bfffee4 7c0802a6 fbc1fff0 fbe1fff8 f8010010 f821ff81 7c7e1b78
[  442.660566] 7c9f2378 60000000 60000000 e93e02c8 <e8690060> 2fa30000 41de00c4 2b9f0002
[  442.660679] ---[ end trace a64ac9546bcf0328 ]---
[  442.660724]

The reason is current VF is not EEH enabled.

This patch is a quick fix for this problem.

Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
Acked-by: Gavin Shan <gwshan@linux.vnet.ibm.com>

V1 -> V2: 
   1. code style and patch subject adjustment

---
 arch/powerpc/kernel/eeh.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 4a45ba8..403445e 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -625,7 +625,7 @@ int eeh_pci_enable(struct eeh_pe *pe, int function)
 int pcibios_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state)
 {
 	struct eeh_dev *edev = pci_dev_to_eeh_dev(dev);
-	struct eeh_pe *pe = edev->pe;
+	struct eeh_pe *pe = edev ? edev->pe : NULL;
 
 	if (!pe) {
 		pr_err("%s: No PE found on PCI device %s\n",
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH V2] powerpc/eeh: Fix kernel crash when passing through VF
  2014-09-11  3:42 [PATCH V2] powerpc/eeh: Fix kernel crash when passing through VF Wei Yang
@ 2014-09-12  3:55 ` Michael Ellerman
  2014-09-12  5:05   ` Gavin Shan
  2014-09-15  8:08 ` [PATCH] " Wei Yang
  1 sibling, 1 reply; 11+ messages in thread
From: Michael Ellerman @ 2014-09-12  3:55 UTC (permalink / raw)
  To: Wei Yang; +Cc: linuxppc-dev, gwshan

On Thu, 2014-09-11 at 11:42 +0800, Wei Yang wrote:
> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> index 4a45ba8..403445e 100644
> --- a/arch/powerpc/kernel/eeh.c
> +++ b/arch/powerpc/kernel/eeh.c
> @@ -625,7 +625,7 @@ int eeh_pci_enable(struct eeh_pe *pe, int function)
>  int pcibios_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state)
>  {
>  	struct eeh_dev *edev = pci_dev_to_eeh_dev(dev);
> -	struct eeh_pe *pe = edev->pe;
> +	struct eeh_pe *pe = edev ? edev->pe : NULL;
>  
>  	if (!pe) {
>  		pr_err("%s: No PE found on PCI device %s\n",


We seem to do this or something similar in a few places. Is it worth having a
pci_dev_to_eeh_pe() inline?

cheers

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH V2] powerpc/eeh: Fix kernel crash when passing through VF
  2014-09-12  3:55 ` Michael Ellerman
@ 2014-09-12  5:05   ` Gavin Shan
  2014-09-12  9:35     ` Wei Yang
  2014-09-15  2:49     ` Michael Ellerman
  0 siblings, 2 replies; 11+ messages in thread
From: Gavin Shan @ 2014-09-12  5:05 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: Wei Yang, linuxppc-dev, gwshan

On Fri, Sep 12, 2014 at 01:55:23PM +1000, Michael Ellerman wrote:
>On Thu, 2014-09-11 at 11:42 +0800, Wei Yang wrote:
>> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
>> index 4a45ba8..403445e 100644
>> --- a/arch/powerpc/kernel/eeh.c
>> +++ b/arch/powerpc/kernel/eeh.c
>> @@ -625,7 +625,7 @@ int eeh_pci_enable(struct eeh_pe *pe, int function)
>>  int pcibios_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state)
>>  {
>>  	struct eeh_dev *edev = pci_dev_to_eeh_dev(dev);
>> -	struct eeh_pe *pe = edev->pe;
>> +	struct eeh_pe *pe = edev ? edev->pe : NULL;
>>  
>>  	if (!pe) {
>>  		pr_err("%s: No PE found on PCI device %s\n",
>
>
>We seem to do this or something similar in a few places. Is it worth having a
>pci_dev_to_eeh_pe() inline?
>

Yes, maybe we just need a eeh_dev_to_pe() because converting
pci_dev to eeh_dev is already coverred by pci_dev_to_eeh_dev().

With eeh_dev_to_pe(), it looks like this:

struct pci_dev *pdev;
struct eeh_dev *edev = pci_dev_to_eeh_dev(pdev);
struct eeh_pe *pe = eeh_dev_to_pe(edev);

Or another case:

struct device_node *dn;
struct eeh_dev *edev = of_node_to_eeh_dev(dn);
struct eeh_pe *pe = eeh_dev_to_pe(edev);

Thanks,
Gavin

>cheers
>
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH V2] powerpc/eeh: Fix kernel crash when passing through VF
  2014-09-12  5:05   ` Gavin Shan
@ 2014-09-12  9:35     ` Wei Yang
  2014-09-14  1:19       ` Gavin Shan
  2014-09-15  2:49     ` Michael Ellerman
  1 sibling, 1 reply; 11+ messages in thread
From: Wei Yang @ 2014-09-12  9:35 UTC (permalink / raw)
  To: Gavin Shan; +Cc: Wei Yang, linuxppc-dev

On Fri, Sep 12, 2014 at 03:05:18PM +1000, Gavin Shan wrote:
>On Fri, Sep 12, 2014 at 01:55:23PM +1000, Michael Ellerman wrote:
>>On Thu, 2014-09-11 at 11:42 +0800, Wei Yang wrote:
>>> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
>>> index 4a45ba8..403445e 100644
>>> --- a/arch/powerpc/kernel/eeh.c
>>> +++ b/arch/powerpc/kernel/eeh.c
>>> @@ -625,7 +625,7 @@ int eeh_pci_enable(struct eeh_pe *pe, int function)
>>>  int pcibios_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state)
>>>  {
>>>  	struct eeh_dev *edev = pci_dev_to_eeh_dev(dev);
>>> -	struct eeh_pe *pe = edev->pe;
>>> +	struct eeh_pe *pe = edev ? edev->pe : NULL;
>>>  
>>>  	if (!pe) {
>>>  		pr_err("%s: No PE found on PCI device %s\n",
>>
>>
>>We seem to do this or something similar in a few places. Is it worth having a
>>pci_dev_to_eeh_pe() inline?
>>
>
>Yes, maybe we just need a eeh_dev_to_pe() because converting
>pci_dev to eeh_dev is already coverred by pci_dev_to_eeh_dev().
>
>With eeh_dev_to_pe(), it looks like this:
>
>struct pci_dev *pdev;
>struct eeh_dev *edev = pci_dev_to_eeh_dev(pdev);
>struct eeh_pe *pe = eeh_dev_to_pe(edev);
>
>Or another case:
>
>struct device_node *dn;
>struct eeh_dev *edev = of_node_to_eeh_dev(dn);
>struct eeh_pe *pe = eeh_dev_to_pe(edev);

With these helper, it would be more consolidate to jump between those data.

Gavin,

You would add these helpers? Or would like me to add them?

>
>Thanks,
>Gavin
>
>>cheers
>>
>>

-- 
Richard Yang
Help you, Help me

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH V2] powerpc/eeh: Fix kernel crash when passing through VF
  2014-09-12  9:35     ` Wei Yang
@ 2014-09-14  1:19       ` Gavin Shan
  0 siblings, 0 replies; 11+ messages in thread
From: Gavin Shan @ 2014-09-14  1:19 UTC (permalink / raw)
  To: Wei Yang; +Cc: linuxppc-dev, Gavin Shan

On Fri, Sep 12, 2014 at 05:35:25PM +0800, Wei Yang wrote:
>On Fri, Sep 12, 2014 at 03:05:18PM +1000, Gavin Shan wrote:
>>On Fri, Sep 12, 2014 at 01:55:23PM +1000, Michael Ellerman wrote:
>>>On Thu, 2014-09-11 at 11:42 +0800, Wei Yang wrote:
>>>> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
>>>> index 4a45ba8..403445e 100644
>>>> --- a/arch/powerpc/kernel/eeh.c
>>>> +++ b/arch/powerpc/kernel/eeh.c
>>>> @@ -625,7 +625,7 @@ int eeh_pci_enable(struct eeh_pe *pe, int function)
>>>>  int pcibios_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state)
>>>>  {
>>>>  	struct eeh_dev *edev = pci_dev_to_eeh_dev(dev);
>>>> -	struct eeh_pe *pe = edev->pe;
>>>> +	struct eeh_pe *pe = edev ? edev->pe : NULL;
>>>>  
>>>>  	if (!pe) {
>>>>  		pr_err("%s: No PE found on PCI device %s\n",
>>>
>>>
>>>We seem to do this or something similar in a few places. Is it worth having a
>>>pci_dev_to_eeh_pe() inline?
>>>
>>
>>Yes, maybe we just need a eeh_dev_to_pe() because converting
>>pci_dev to eeh_dev is already coverred by pci_dev_to_eeh_dev().
>>
>>With eeh_dev_to_pe(), it looks like this:
>>
>>struct pci_dev *pdev;
>>struct eeh_dev *edev = pci_dev_to_eeh_dev(pdev);
>>struct eeh_pe *pe = eeh_dev_to_pe(edev);
>>
>>Or another case:
>>
>>struct device_node *dn;
>>struct eeh_dev *edev = of_node_to_eeh_dev(dn);
>>struct eeh_pe *pe = eeh_dev_to_pe(edev);
>
>With these helper, it would be more consolidate to jump between those data.
>
>Gavin,
>
>You would add these helpers? Or would like me to add them?
>

It would be Richard to help on this :-)

Thanks,
Gavin

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH V2] powerpc/eeh: Fix kernel crash when passing through VF
  2014-09-12  5:05   ` Gavin Shan
  2014-09-12  9:35     ` Wei Yang
@ 2014-09-15  2:49     ` Michael Ellerman
  1 sibling, 0 replies; 11+ messages in thread
From: Michael Ellerman @ 2014-09-15  2:49 UTC (permalink / raw)
  To: Gavin Shan; +Cc: Wei Yang, linuxppc-dev

On Fri, 2014-09-12 at 15:05 +1000, Gavin Shan wrote:
> On Fri, Sep 12, 2014 at 01:55:23PM +1000, Michael Ellerman wrote:
> >On Thu, 2014-09-11 at 11:42 +0800, Wei Yang wrote:
> >> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> >> index 4a45ba8..403445e 100644
> >> --- a/arch/powerpc/kernel/eeh.c
> >> +++ b/arch/powerpc/kernel/eeh.c
> >> @@ -625,7 +625,7 @@ int eeh_pci_enable(struct eeh_pe *pe, int function)
> >>  int pcibios_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state)
> >>  {
> >>  	struct eeh_dev *edev = pci_dev_to_eeh_dev(dev);
> >> -	struct eeh_pe *pe = edev->pe;
> >> +	struct eeh_pe *pe = edev ? edev->pe : NULL;
> >>  
> >>  	if (!pe) {
> >>  		pr_err("%s: No PE found on PCI device %s\n",
> >
> >
> >We seem to do this or something similar in a few places. Is it worth having a
> >pci_dev_to_eeh_pe() inline?
> >
> 
> Yes, maybe we just need a eeh_dev_to_pe() because converting
> pci_dev to eeh_dev is already coverred by pci_dev_to_eeh_dev().
> 
> With eeh_dev_to_pe(), it looks like this:
> 
> struct pci_dev *pdev;
> struct eeh_dev *edev = pci_dev_to_eeh_dev(pdev);
> struct eeh_pe *pe = eeh_dev_to_pe(edev);
> 
> Or another case:
> 
> struct device_node *dn;
> struct eeh_dev *edev = of_node_to_eeh_dev(dn);
> struct eeh_pe *pe = eeh_dev_to_pe(edev);

Yeah I guess.

I saw a few places where we go from pci_dev to eeh_pe via a eeh_dev but then
don't use the eeh_dev at all. So for those it would make sense to have one
macro that does the full conversion from pci_dev to eeh_pe.

But if you think that's not very common then yeah a macro to do each stage is
fine.

cheers

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] powerpc/eeh: Fix kernel crash when passing through VF
  2014-09-11  3:42 [PATCH V2] powerpc/eeh: Fix kernel crash when passing through VF Wei Yang
  2014-09-12  3:55 ` Michael Ellerman
@ 2014-09-15  8:08 ` Wei Yang
  2014-09-16  4:03   ` Michael Ellerman
  2014-09-17  2:48   ` [PATCH V4] " Wei Yang
  1 sibling, 2 replies; 11+ messages in thread
From: Wei Yang @ 2014-09-15  8:08 UTC (permalink / raw)
  To: gwshan, linuxppc-dev, mpe; +Cc: Wei Yang

When doing vfio passthrough a VF, the kernel will crash with following
message:

[  442.656459] Unable to handle kernel paging request for data at address 0x00000060
[  442.656593] Faulting instruction address: 0xc000000000038b88
[  442.656706] Oops: Kernel access of bad area, sig: 11 [#1]
[  442.656798] SMP NR_CPUS=1024 NUMA PowerNV
[  442.656890] Modules linked in: vfio_pci mlx4_core nf_conntrack_netbios_ns nf_conntrack_broadcast ipt_MASQUERADE ip6t_REJECT xt_conntrack bnep bluetooth rfkill ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle iptable_security iptable_raw tg3 nfsd be2net nfs_acl ses lockd ptp enclosure pps_core kvm_hv kvm_pr shpchp binfmt_misc kvm sunrpc uinput lpfc scsi_transport_fc ipr scsi_tgt [last unloaded: mlx4_core]
[  442.658152] CPU: 40 PID: 14948 Comm: qemu-system-ppc Not tainted 3.10.42yw-pkvm+ #37
[  442.658219] task: c000000f7e2a9a00 ti: c000000f6dc3c000 task.ti: c000000f6dc3c000
[  442.658287] NIP: c000000000038b88 LR: c0000000004435a8 CTR: c000000000455bc0
[  442.658352] REGS: c000000f6dc3f580 TRAP: 0300   Not tainted  (3.10.42yw-pkvm+)
[  442.658419] MSR: 9000000000009032 <SF,HV,EE,ME,IR,DR,RI>  CR: 28004882  XER: 20000000
[  442.658577] CFAR: c00000000000908c DAR: 0000000000000060 DSISR: 40000000 SOFTE: 1
GPR00: c0000000004435a8 c000000f6dc3f800 c0000000012b1c10 c00000000da24000
GPR04: 0000000000000003 0000000000001004 00000000000015b3 000000000000ffff
GPR08: c00000000127f5d8 0000000000000000 000000000000ffff 0000000000000000
GPR12: c000000000068078 c00000000fdd6800 000001003c320c80 000001003c3607f0
GPR16: 0000000000000001 00000000105480c8 000000001055aaa8 000001003c31ab18
GPR20: 000001003c10fb40 000001003c360ae8 000000001063bcf0 000000001063bdb0
GPR24: 000001003c15ed70 0000000010548f40 c000001fe5514c88 c000001fe5514cb0
GPR28: c00000000da24000 0000000000000000 c00000000da24000 0000000000000003
[  442.659471] NIP [c000000000038b88] .pcibios_set_pcie_reset_state+0x28/0x130
[  442.659530] LR [c0000000004435a8] .pci_set_pcie_reset_state+0x28/0x40
[  442.659585] Call Trace:
[  442.659610] [c000000f6dc3f800] [00000000000719e0] 0x719e0 (unreliable)
[  442.659677] [c000000f6dc3f880] [c0000000004435a8] .pci_set_pcie_reset_state+0x28/0x40
[  442.659757] [c000000f6dc3f900] [c000000000455bf8] .reset_fundamental+0x38/0x80
[  442.659835] [c000000f6dc3f980] [c0000000004562a8] .pci_dev_specific_reset+0xa8/0xf0
[  442.659913] [c000000f6dc3fa00] [c0000000004448c4] .__pci_dev_reset+0x44/0x430
[  442.659980] [c000000f6dc3fab0] [c000000000444d5c] .pci_reset_function+0x7c/0xc0
[  442.660059] [c000000f6dc3fb30] [d00000001c141ab8] .vfio_pci_open+0xe8/0x2b0 [vfio_pci]
[  442.660139] [c000000f6dc3fbd0] [c000000000586c30] .vfio_group_fops_unl_ioctl+0x3a0/0x630
[  442.660219] [c000000f6dc3fc90] [c000000000255fbc] .do_vfs_ioctl+0x4ec/0x7c0
[  442.660286] [c000000f6dc3fd80] [c000000000256364] .SyS_ioctl+0xd4/0xf0
[  442.660354] [c000000f6dc3fe30] [c000000000009e54] syscall_exit+0x0/0x98
[  442.660420] Instruction dump:
[  442.660454] 4bfffce9 4bfffee4 7c0802a6 fbc1fff0 fbe1fff8 f8010010 f821ff81 7c7e1b78
[  442.660566] 7c9f2378 60000000 60000000 e93e02c8 <e8690060> 2fa30000 41de00c4 2b9f0002
[  442.660679] ---[ end trace a64ac9546bcf0328 ]---
[  442.660724]

The reason is current VF is not EEH enabled.

This patch introduces a marco to convert eeh_dev to eeh_pe. By doing so, it
will prevent converting with NULL pointer.

Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
Acked-by: Gavin Shan <gwshan@linux.vnet.ibm.com>

V2 -> V3:
   1. rebased on 3.17-rc4
   2. introduce a marco
   3. use this marco in several other places

V1 -> V2:
   1. code style and patch subject adjustment
---
 arch/powerpc/kernel/eeh.c    |    4 ++--
 arch/powerpc/kernel/eeh_pe.c |    2 +-
 include/linux/pci.h          |    5 +++++
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 59a64f8..0f1b637 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -410,7 +410,7 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
 	}
 	dn = eeh_dev_to_of_node(edev);
 	dev = eeh_dev_to_pci_dev(edev);
-	pe = edev->pe;
+	pe = eeh_dev_to_pe(edev);
 
 	/* Access to IO BARs might get this far and still not want checking. */
 	if (!pe) {
@@ -634,7 +634,7 @@ int eeh_pci_enable(struct eeh_pe *pe, int function)
 int pcibios_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state)
 {
 	struct eeh_dev *edev = pci_dev_to_eeh_dev(dev);
-	struct eeh_pe *pe = edev->pe;
+	struct eeh_pe *pe = eeh_dev_to_pe(edev);
 
 	if (!pe) {
 		pr_err("%s: No PE found on PCI device %s\n",
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index 00e3844..5864017 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -428,7 +428,7 @@ int eeh_rmv_from_parent_pe(struct eeh_dev *edev)
 	}
 
 	/* Remove the EEH device */
-	pe = edev->pe;
+	pe = eeh_dev_to_pe(edev);
 	edev->pe = NULL;
 	list_del(&edev->list);
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index fd03e819..9656f92 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1824,6 +1824,11 @@ static inline struct eeh_dev *pci_dev_to_eeh_dev(struct pci_dev *pdev)
 {
 	return pdev->dev.archdata.edev;
 }
+
+static inline struct eeh_pe *eeh_dev_to_pe(struct eeh_dev* edev)
+{
+	return edev ? edev->pe : NULL;
+}
 #endif
 
 int pci_for_each_dma_alias(struct pci_dev *pdev,
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] powerpc/eeh: Fix kernel crash when passing through VF
  2014-09-15  8:08 ` [PATCH] " Wei Yang
@ 2014-09-16  4:03   ` Michael Ellerman
  2014-09-16  6:02     ` Wei Yang
  2014-09-17  2:48   ` [PATCH V4] " Wei Yang
  1 sibling, 1 reply; 11+ messages in thread
From: Michael Ellerman @ 2014-09-16  4:03 UTC (permalink / raw)
  To: Wei Yang; +Cc: linuxppc-dev, gwshan

On Mon, 2014-09-15 at 16:08 +0800, Wei Yang wrote:
> This patch introduces a marco to convert eeh_dev to eeh_pe. By doing so, it
> will prevent converting with NULL pointer.
> 
> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
> Acked-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> 
> V2 -> V3:
>    1. rebased on 3.17-rc4
>    2. introduce a marco
>    3. use this marco in several other places

Macro :)

> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> index 59a64f8..0f1b637 100644
> --- a/arch/powerpc/kernel/eeh.c
> +++ b/arch/powerpc/kernel/eeh.c
> @@ -410,7 +410,7 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
>  	}
>  	dn = eeh_dev_to_of_node(edev);
>  	dev = eeh_dev_to_pci_dev(edev);
> -	pe = edev->pe;
> +	pe = eeh_dev_to_pe(edev);

This looks good, but ..

> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index fd03e819..9656f92 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1824,6 +1824,11 @@ static inline struct eeh_dev *pci_dev_to_eeh_dev(struct pci_dev *pdev)
>  {
>  	return pdev->dev.archdata.edev;
>  }
> +
> +static inline struct eeh_pe *eeh_dev_to_pe(struct eeh_dev* edev)
> +{
> +	return edev ? edev->pe : NULL;
> +}
>  #endif

Why is it in linux/pci.h ? It's powerpc specific, it should be in arch/powerpc/include/asm/pci.h

cheers

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] powerpc/eeh: Fix kernel crash when passing through VF
  2014-09-16  4:03   ` Michael Ellerman
@ 2014-09-16  6:02     ` Wei Yang
  2014-09-16  7:14       ` Gavin Shan
  0 siblings, 1 reply; 11+ messages in thread
From: Wei Yang @ 2014-09-16  6:02 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: Wei Yang, linuxppc-dev, gwshan

On Tue, Sep 16, 2014 at 02:03:56PM +1000, Michael Ellerman wrote:
>On Mon, 2014-09-15 at 16:08 +0800, Wei Yang wrote:
>> This patch introduces a marco to convert eeh_dev to eeh_pe. By doing so, it
>> will prevent converting with NULL pointer.
>> 
>> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>> Acked-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>> 
>> V2 -> V3:
>>    1. rebased on 3.17-rc4
>>    2. introduce a marco
>>    3. use this marco in several other places
>
>Macro :)

:-)

>
>> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
>> index 59a64f8..0f1b637 100644
>> --- a/arch/powerpc/kernel/eeh.c
>> +++ b/arch/powerpc/kernel/eeh.c
>> @@ -410,7 +410,7 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
>>  	}
>>  	dn = eeh_dev_to_of_node(edev);
>>  	dev = eeh_dev_to_pci_dev(edev);
>> -	pe = edev->pe;
>> +	pe = eeh_dev_to_pe(edev);
>
>This looks good, but ..
>
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index fd03e819..9656f92 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -1824,6 +1824,11 @@ static inline struct eeh_dev *pci_dev_to_eeh_dev(struct pci_dev *pdev)
>>  {
>>  	return pdev->dev.archdata.edev;
>>  }
>> +
>> +static inline struct eeh_pe *eeh_dev_to_pe(struct eeh_dev* edev)
>> +{
>> +	return edev ? edev->pe : NULL;
>> +}
>>  #endif
>
>Why is it in linux/pci.h ? It's powerpc specific, it should be in arch/powerpc/include/asm/pci.h
>

Good question, let me move to powerpc directory and have a try.

>cheers
>

-- 
Richard Yang
Help you, Help me

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] powerpc/eeh: Fix kernel crash when passing through VF
  2014-09-16  6:02     ` Wei Yang
@ 2014-09-16  7:14       ` Gavin Shan
  0 siblings, 0 replies; 11+ messages in thread
From: Gavin Shan @ 2014-09-16  7:14 UTC (permalink / raw)
  To: Wei Yang; +Cc: linuxppc-dev, gwshan

On Tue, Sep 16, 2014 at 02:02:18PM +0800, Wei Yang wrote:
>On Tue, Sep 16, 2014 at 02:03:56PM +1000, Michael Ellerman wrote:
>>On Mon, 2014-09-15 at 16:08 +0800, Wei Yang wrote:
>>> This patch introduces a marco to convert eeh_dev to eeh_pe. By doing so, it
>>> will prevent converting with NULL pointer.
>>> 
>>> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>>> Acked-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>>> 
>>> V2 -> V3:
>>>    1. rebased on 3.17-rc4
>>>    2. introduce a marco
>>>    3. use this marco in several other places
>>
>>Macro :)
>
>:-)
>
>>
>>> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
>>> index 59a64f8..0f1b637 100644
>>> --- a/arch/powerpc/kernel/eeh.c
>>> +++ b/arch/powerpc/kernel/eeh.c
>>> @@ -410,7 +410,7 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
>>>  	}
>>>  	dn = eeh_dev_to_of_node(edev);
>>>  	dev = eeh_dev_to_pci_dev(edev);
>>> -	pe = edev->pe;
>>> +	pe = eeh_dev_to_pe(edev);
>>
>>This looks good, but ..
>>
>>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>>> index fd03e819..9656f92 100644
>>> --- a/include/linux/pci.h
>>> +++ b/include/linux/pci.h
>>> @@ -1824,6 +1824,11 @@ static inline struct eeh_dev *pci_dev_to_eeh_dev(struct pci_dev *pdev)
>>>  {
>>>  	return pdev->dev.archdata.edev;
>>>  }
>>> +
>>> +static inline struct eeh_pe *eeh_dev_to_pe(struct eeh_dev* edev)
>>> +{
>>> +	return edev ? edev->pe : NULL;
>>> +}
>>>  #endif
>>
>>Why is it in linux/pci.h ? It's powerpc specific, it should be in arch/powerpc/include/asm/pci.h
>>
>
>Good question, let me move to powerpc directory and have a try.
>

It would be part of arch/powerpc/include/asm/eeh.h :-)

Thanks,
Gavin

>>cheers
>>
>
>-- 
>Richard Yang
>Help you, Help me

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH V4] powerpc/eeh: Fix kernel crash when passing through VF
  2014-09-15  8:08 ` [PATCH] " Wei Yang
  2014-09-16  4:03   ` Michael Ellerman
@ 2014-09-17  2:48   ` Wei Yang
  1 sibling, 0 replies; 11+ messages in thread
From: Wei Yang @ 2014-09-17  2:48 UTC (permalink / raw)
  To: linuxppc-dev, gwshan; +Cc: Wei Yang

When doing vfio passthrough a VF, the kernel will crash with following
message:

[  442.656459] Unable to handle kernel paging request for data at address 0x00000060
[  442.656593] Faulting instruction address: 0xc000000000038b88
[  442.656706] Oops: Kernel access of bad area, sig: 11 [#1]
[  442.656798] SMP NR_CPUS=1024 NUMA PowerNV
[  442.656890] Modules linked in: vfio_pci mlx4_core nf_conntrack_netbios_ns nf_conntrack_broadcast ipt_MASQUERADE ip6t_REJECT xt_conntrack bnep bluetooth rfkill ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle iptable_security iptable_raw tg3 nfsd be2net nfs_acl ses lockd ptp enclosure pps_core kvm_hv kvm_pr shpchp binfmt_misc kvm sunrpc uinput lpfc scsi_transport_fc ipr scsi_tgt [last unloaded: mlx4_core]
[  442.658152] CPU: 40 PID: 14948 Comm: qemu-system-ppc Not tainted 3.10.42yw-pkvm+ #37
[  442.658219] task: c000000f7e2a9a00 ti: c000000f6dc3c000 task.ti: c000000f6dc3c000
[  442.658287] NIP: c000000000038b88 LR: c0000000004435a8 CTR: c000000000455bc0
[  442.658352] REGS: c000000f6dc3f580 TRAP: 0300   Not tainted  (3.10.42yw-pkvm+)
[  442.658419] MSR: 9000000000009032 <SF,HV,EE,ME,IR,DR,RI>  CR: 28004882  XER: 20000000
[  442.658577] CFAR: c00000000000908c DAR: 0000000000000060 DSISR: 40000000 SOFTE: 1
GPR00: c0000000004435a8 c000000f6dc3f800 c0000000012b1c10 c00000000da24000
GPR04: 0000000000000003 0000000000001004 00000000000015b3 000000000000ffff
GPR08: c00000000127f5d8 0000000000000000 000000000000ffff 0000000000000000
GPR12: c000000000068078 c00000000fdd6800 000001003c320c80 000001003c3607f0
GPR16: 0000000000000001 00000000105480c8 000000001055aaa8 000001003c31ab18
GPR20: 000001003c10fb40 000001003c360ae8 000000001063bcf0 000000001063bdb0
GPR24: 000001003c15ed70 0000000010548f40 c000001fe5514c88 c000001fe5514cb0
GPR28: c00000000da24000 0000000000000000 c00000000da24000 0000000000000003
[  442.659471] NIP [c000000000038b88] .pcibios_set_pcie_reset_state+0x28/0x130
[  442.659530] LR [c0000000004435a8] .pci_set_pcie_reset_state+0x28/0x40
[  442.659585] Call Trace:
[  442.659610] [c000000f6dc3f800] [00000000000719e0] 0x719e0 (unreliable)
[  442.659677] [c000000f6dc3f880] [c0000000004435a8] .pci_set_pcie_reset_state+0x28/0x40
[  442.659757] [c000000f6dc3f900] [c000000000455bf8] .reset_fundamental+0x38/0x80
[  442.659835] [c000000f6dc3f980] [c0000000004562a8] .pci_dev_specific_reset+0xa8/0xf0
[  442.659913] [c000000f6dc3fa00] [c0000000004448c4] .__pci_dev_reset+0x44/0x430
[  442.659980] [c000000f6dc3fab0] [c000000000444d5c] .pci_reset_function+0x7c/0xc0
[  442.660059] [c000000f6dc3fb30] [d00000001c141ab8] .vfio_pci_open+0xe8/0x2b0 [vfio_pci]
[  442.660139] [c000000f6dc3fbd0] [c000000000586c30] .vfio_group_fops_unl_ioctl+0x3a0/0x630
[  442.660219] [c000000f6dc3fc90] [c000000000255fbc] .do_vfs_ioctl+0x4ec/0x7c0
[  442.660286] [c000000f6dc3fd80] [c000000000256364] .SyS_ioctl+0xd4/0xf0
[  442.660354] [c000000f6dc3fe30] [c000000000009e54] syscall_exit+0x0/0x98
[  442.660420] Instruction dump:
[  442.660454] 4bfffce9 4bfffee4 7c0802a6 fbc1fff0 fbe1fff8 f8010010 f821ff81 7c7e1b78
[  442.660566] 7c9f2378 60000000 60000000 e93e02c8 <e8690060> 2fa30000 41de00c4 2b9f0002
[  442.660679] ---[ end trace a64ac9546bcf0328 ]---
[  442.660724]

The reason is current VF is not EEH enabled.

This patch introduces a macro to convert eeh_dev to eeh_pe. By doing so, it
will prevent converting with NULL pointer.

Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
Acked-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
CC: Michael Ellerman <mpe@ellerman.id.au>

V3 -> V4:
   1. move the macro definition from include/linux/pci.h to
      arch/powerpc/include/asm/eeh.h

V2 -> V3:
   1. rebased on 3.17-rc4
   2. introduce a macro
   3. use this macro in several other places

V1 -> V2:
   1. code style and patch subject adjustment
---
 arch/powerpc/include/asm/eeh.h |    5 +++++
 arch/powerpc/kernel/eeh.c      |    4 ++--
 arch/powerpc/kernel/eeh_pe.c   |    2 +-
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 9983c3d..757014f 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -146,6 +146,11 @@ static inline struct pci_dev *eeh_dev_to_pci_dev(struct eeh_dev *edev)
 	return edev ? edev->pdev : NULL;
 }
 
+static inline struct eeh_pe *eeh_dev_to_pe(struct eeh_dev* edev)
+{
+	return edev ? edev->pe : NULL;
+}
+
 /* Return values from eeh_ops::next_error */
 enum {
 	EEH_NEXT_ERR_NONE = 0,
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 59a64f8..0f1b637 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -410,7 +410,7 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
 	}
 	dn = eeh_dev_to_of_node(edev);
 	dev = eeh_dev_to_pci_dev(edev);
-	pe = edev->pe;
+	pe = eeh_dev_to_pe(edev);
 
 	/* Access to IO BARs might get this far and still not want checking. */
 	if (!pe) {
@@ -634,7 +634,7 @@ int eeh_pci_enable(struct eeh_pe *pe, int function)
 int pcibios_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state)
 {
 	struct eeh_dev *edev = pci_dev_to_eeh_dev(dev);
-	struct eeh_pe *pe = edev->pe;
+	struct eeh_pe *pe = eeh_dev_to_pe(edev);
 
 	if (!pe) {
 		pr_err("%s: No PE found on PCI device %s\n",
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index 00e3844..5864017 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -428,7 +428,7 @@ int eeh_rmv_from_parent_pe(struct eeh_dev *edev)
 	}
 
 	/* Remove the EEH device */
-	pe = edev->pe;
+	pe = eeh_dev_to_pe(edev);
 	edev->pe = NULL;
 	list_del(&edev->list);
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2014-09-17  2:48 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-11  3:42 [PATCH V2] powerpc/eeh: Fix kernel crash when passing through VF Wei Yang
2014-09-12  3:55 ` Michael Ellerman
2014-09-12  5:05   ` Gavin Shan
2014-09-12  9:35     ` Wei Yang
2014-09-14  1:19       ` Gavin Shan
2014-09-15  2:49     ` Michael Ellerman
2014-09-15  8:08 ` [PATCH] " Wei Yang
2014-09-16  4:03   ` Michael Ellerman
2014-09-16  6:02     ` Wei Yang
2014-09-16  7:14       ` Gavin Shan
2014-09-17  2:48   ` [PATCH V4] " Wei Yang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).