Hard and silent lock up since linux 3.14 with PCIe pass through (vfio)

Hello!

Since long time now, I'm using w/o any problem PCIe pass through with a Gigabyte GA-990XA-UD3/GA-990XA-UD3 mainboard (AMD 990X chipset) and enabled IOMMU with vfio-pci.

The last kernel working w/o any problem is kernel 3.13.7 (I didn't use .8 and .9, but I do not think they would have been problematic).

Since 3.14.19 (I didn't test any 3.14 kernel before) I'm encountering a hard and silent lock up of the complete machine when starting the VM with the PCIe card passed through.

That's the relevant PCIe card, which locks up the machine (here running w/ 3.12.28) when passed to the VM:

03:00.0 Network controller: Qualcomm Atheros AR93xx Wireless Network Adapter (rev 01)
Subsystem: Qualcomm Atheros Device 3112
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx-
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
Latency: 0, Cache Line Size: 64 bytes
Interrupt: pin A routed to IRQ 17
Region 0: Memory at fdbc0000 (64-bit, non-prefetchable) [size=128K]
Expansion ROM at fda00000 [size=64K]
Capabilities: [40] Power Management version 3
Flags: PMEClk- DSI- D1+ D2- AuxCurrent=375mA PME(D0+,D1+,D2-,D3hot+,D3cold-)
Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
Capabilities: [50] MSI: Enable- Count=1/4 Maskable+ 64bit+
Address: 0000000000000000 Data: 0000
Masking: 00000000 Pending: 00000000
Capabilities: [70] Express (v2) Endpoint, MSI 00
DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s <1us, L1 <8us
ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset-
DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop-
MaxPayload 128 bytes, MaxReadReq 512 bytes
DevSta: CorrErr+ UncorrErr- FatalErr- UnsuppReq+ AuxPwr- TransPend-
LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Latency L0 <2us, L1 <64us
ClockPM- Surprise- LLActRep- BwNot-
LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- Retrain- CommClk+
ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt-
DevCap2: Completion Timeout: Not Supported, TimeoutDis+, LTR-, OBFF Not Supported
DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR-, OBFF Disabled
LnkCtl2: Target Link Speed: 2.5GT/s, EnterCompliance- SpeedDis-
Transmit Margin: Normal Operating Range, EnterModifiedCompliance- ComplianceSOS- Compliance De-emphasis: -6dB
LnkSta2: Current De-emphasis Level: -6dB, EqualizationComplete-, EqualizationPhase1-
EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest-
Capabilities: [100 v1] Advanced Error Reporting
UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
UESvrt: DLP+ SDES+ TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol-
CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout+ NonFatalErr+
CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+
AERCap: First Error Pointer: 00, GenCap- CGenEn- ChkCap- ChkEn-
Capabilities: [140 v1] Virtual Channel
Caps: LPEVC=0 RefClk=100ns PATEntryBits=1
Arb: Fixed- WRR32- WRR64- WRR128-
Ctrl: ArbSelect=Fixed
Status: InProgress-
VC0: Caps: PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-
Arb: Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-
Ctrl: Enable+ ID=0 ArbSelect=Fixed TC/VC=ff
Status: NegoPending- InProgress-
Capabilities: [300 v1] Device Serial Number 00-00-00-00-00-00-00-00
Kernel driver in use: vfio-pci
Kernel modules: ath9k

Unbinding it works w/o any problem. The lock up encounters about 4 s after the start of the VM.

On 3.12.x, I can see the following message on the error terminal when starting the VM:
vfio-pci: 03:00.0: invalid ROM contents.

I compared AMD-Vi debug output between 3.12 and 3.14, but couldn't see any difference. I compared /proc/interrupts between 3.12 and 3.14 and couldn't see any difference too so far.

qemu version I'm using is 1.7.0.

It is strange(?), that a second VM using PCI (legacy) pass through works w/o any problem. I tried to start the problematic VM even w/o running this VM - same result: machine is locked up hard.

Do you have any idea, what could be going on there? Or how to debug it to see what happened?

Quoted 90 lines · Andreas Hartmann

Hello!

Since long time now, I'm using w/o any problem PCIe pass through with a Gigabyte GA-990XA-UD3/GA-990XA-UD3 mainboard (AMD 990X chipset) and enabled IOMMU with vfio-pci.

The last kernel working w/o any problem is kernel 3.13.7 (I didn't use .8 and .9, but I do not think they would have been problematic).

Since 3.14.19 (I didn't test any 3.14 kernel before) I'm encountering a hard and silent lock up of the complete machine when starting the VM with the PCIe card passed through.

That's the relevant PCIe card, which locks up the machine (here running w/ 3.12.28) when passed to the VM:

03:00.0 Network controller: Qualcomm Atheros AR93xx Wireless Network Adapter (rev 01)
Subsystem: Qualcomm Atheros Device 3112
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx-
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
Latency: 0, Cache Line Size: 64 bytes
Interrupt: pin A routed to IRQ 17
Region 0: Memory at fdbc0000 (64-bit, non-prefetchable) [size=128K]
Expansion ROM at fda00000 [size=64K]
Capabilities: [40] Power Management version 3
Flags: PMEClk- DSI- D1+ D2- AuxCurrent=375mA PME(D0+,D1+,D2-,D3hot+,D3cold-)
Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
Capabilities: [50] MSI: Enable- Count=1/4 Maskable+ 64bit+
Address: 0000000000000000 Data: 0000
Masking: 00000000 Pending: 00000000
Capabilities: [70] Express (v2) Endpoint, MSI 00
DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s <1us, L1 <8us
ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset-
DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop-
MaxPayload 128 bytes, MaxReadReq 512 bytes
DevSta: CorrErr+ UncorrErr- FatalErr- UnsuppReq+ AuxPwr- TransPend-
LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Latency L0 <2us, L1 <64us
ClockPM- Surprise- LLActRep- BwNot-
LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- Retrain- CommClk+
ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt-
DevCap2: Completion Timeout: Not Supported, TimeoutDis+, LTR-, OBFF Not Supported
DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR-, OBFF Disabled
LnkCtl2: Target Link Speed: 2.5GT/s, EnterCompliance- SpeedDis-
Transmit Margin: Normal Operating Range, EnterModifiedCompliance- ComplianceSOS- Compliance De-emphasis: -6dB
LnkSta2: Current De-emphasis Level: -6dB, EqualizationComplete-, EqualizationPhase1-
EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest-
Capabilities: [100 v1] Advanced Error Reporting
UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
UESvrt: DLP+ SDES+ TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol-
CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout+ NonFatalErr+
CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+
AERCap: First Error Pointer: 00, GenCap- CGenEn- ChkCap- ChkEn-
Capabilities: [140 v1] Virtual Channel
Caps: LPEVC=0 RefClk=100ns PATEntryBits=1
Arb: Fixed- WRR32- WRR64- WRR128-
Ctrl: ArbSelect=Fixed
Status: InProgress-
VC0: Caps: PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-
Arb: Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-
Ctrl: Enable+ ID=0 ArbSelect=Fixed TC/VC=ff
Status: NegoPending- InProgress-
Capabilities: [300 v1] Device Serial Number 00-00-00-00-00-00-00-00
Kernel driver in use: vfio-pci
Kernel modules: ath9k

Unbinding it works w/o any problem. The lock up encounters about 4 s after the start of the VM.

On 3.12.x, I can see the following message on the error terminal when starting the VM:
vfio-pci: 03:00.0: invalid ROM contents.

I compared AMD-Vi debug output between 3.12 and 3.14, but couldn't see any difference. I compared /proc/interrupts between 3.12 and 3.14 and couldn't see any difference too so far.

qemu version I'm using is 1.7.0.

It is strange(?), that a second VM using PCI (legacy) pass through works w/o any problem. I tried to start the problematic VM even w/o running this VM - same result: machine is locked up hard.

Quoted 2 lines

Do you have any idea, what could be going on there? Or how to debug it to see what happened?

Are you able to setup a serial console on this system? Enabling sysrq and getting a dump of task states (t) via serial is often the best way to determine the problem.

There weren't many vfio changes between 3.13 and 3.14.

Have you tested whether the problem still occurs on 3.16 +

newer QEMU?

Maybe also remove the ROM from the equation with the
rombar=0 option for the vfio-pci device in QEMU.

Alex Williamson wrote:

Quoted 3 lines

Are you able to setup a serial console on this system? Enabling sysrq and getting a dump of task states (t) via serial is often the best way to determine the problem.

I'll try it.

Quoted 90 lines · Andreas Hartmann

Hello!

Since long time now, I'm using w/o any problem PCIe pass through with a Gigabyte GA-990XA-UD3/GA-990XA-UD3 mainboard (AMD 990X chipset) and enabled IOMMU with vfio-pci.

The last kernel working w/o any problem is kernel 3.13.7 (I didn't use .8 and .9, but I do not think they would have been problematic).

Since 3.14.19 (I didn't test any 3.14 kernel before) I'm encountering a hard and silent lock up of the complete machine when starting the VM with the PCIe card passed through.

That's the relevant PCIe card, which locks up the machine (here running w/ 3.12.28) when passed to the VM:

03:00.0 Network controller: Qualcomm Atheros AR93xx Wireless Network Adapter (rev 01)
Subsystem: Qualcomm Atheros Device 3112
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx-
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
Latency: 0, Cache Line Size: 64 bytes
Interrupt: pin A routed to IRQ 17
Region 0: Memory at fdbc0000 (64-bit, non-prefetchable) [size=128K]
Expansion ROM at fda00000 [size=64K]
Capabilities: [40] Power Management version 3
Flags: PMEClk- DSI- D1+ D2- AuxCurrent=375mA PME(D0+,D1+,D2-,D3hot+,D3cold-)
Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
Capabilities: [50] MSI: Enable- Count=1/4 Maskable+ 64bit+
Address: 0000000000000000 Data: 0000
Masking: 00000000 Pending: 00000000
Capabilities: [70] Express (v2) Endpoint, MSI 00
DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s <1us, L1 <8us
ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset-
DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop-
MaxPayload 128 bytes, MaxReadReq 512 bytes
DevSta: CorrErr+ UncorrErr- FatalErr- UnsuppReq+ AuxPwr- TransPend-
LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Latency L0 <2us, L1 <64us
ClockPM- Surprise- LLActRep- BwNot-
LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- Retrain- CommClk+
ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt-
DevCap2: Completion Timeout: Not Supported, TimeoutDis+, LTR-, OBFF Not Supported
DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR-, OBFF Disabled
LnkCtl2: Target Link Speed: 2.5GT/s, EnterCompliance- SpeedDis-
Transmit Margin: Normal Operating Range, EnterModifiedCompliance- ComplianceSOS- Compliance De-emphasis: -6dB
LnkSta2: Current De-emphasis Level: -6dB, EqualizationComplete-, EqualizationPhase1-
EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest-
Capabilities: [100 v1] Advanced Error Reporting
UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
UESvrt: DLP+ SDES+ TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol-
CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout+ NonFatalErr+
CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+
AERCap: First Error Pointer: 00, GenCap- CGenEn- ChkCap- ChkEn-
Capabilities: [140 v1] Virtual Channel
Caps: LPEVC=0 RefClk=100ns PATEntryBits=1
Arb: Fixed- WRR32- WRR64- WRR128-
Ctrl: ArbSelect=Fixed
Status: InProgress-
VC0: Caps: PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-
Arb: Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-
Ctrl: Enable+ ID=0 ArbSelect=Fixed TC/VC=ff
Status: NegoPending- InProgress-
Capabilities: [300 v1] Device Serial Number 00-00-00-00-00-00-00-00
Kernel driver in use: vfio-pci
Kernel modules: ath9k

Unbinding it works w/o any problem. The lock up encounters about 4 s after the start of the VM.

On 3.12.x, I can see the following message on the error terminal when starting the VM:
vfio-pci: 03:00.0: invalid ROM contents.

I compared AMD-Vi debug output between 3.12 and 3.14, but couldn't see any difference. I compared /proc/interrupts between 3.12 and 3.14 and couldn't see any difference too so far.

qemu version I'm using is 1.7.0.

It is strange(?), that a second VM using PCI (legacy) pass through works w/o any problem. I tried to start the problematic VM even w/o running this VM - same result: machine is locked up hard.

Quoted 2 lines

Do you have any idea, what could be going on there? Or how to debug it to see what happened?

There weren't many vfio changes between 3.13 and 3.14.

Have you tested whether the problem still occurs on 3.16 +

newer QEMU?

Maybe also remove the ROM from the equation with the
rombar=0 option for the vfio-pci device in QEMU.

shortly: I retested w/ qemu 2.1.0 and Linux 3.17.0 - no change in behaviour.

Quoted 13 lines · Alex Williamson

Quoted 12 lines · Andreas Hartmann

Hello!

Since long time now, I'm using w/o any problem PCIe pass through with a Gigabyte GA-990XA-UD3/GA-990XA-UD3 mainboard (AMD 990X chipset) and enabled IOMMU with vfio-pci.

The last kernel working w/o any problem is kernel 3.13.7 (I didn't use .8 and .9, but I do not think they would have been problematic).

Since 3.14.19 (I didn't test any 3.14 kernel before) I'm encountering a hard and silent lock up of the complete machine when starting the VM with the PCIe card passed through.

Quoted 16 lines · Andreas Hartmann

shortly: I retested w/ qemu 2.1.0 and Linux 3.17.0 - no change in behaviour.

Quoted 13 lines · Alex Williamson

Quoted 12 lines · Andreas Hartmann

Hello!

Since long time now, I'm using w/o any problem PCIe pass through with a Gigabyte GA-990XA-UD3/GA-990XA-UD3 mainboard (AMD 990X chipset) and enabled IOMMU with vfio-pci.

The last kernel working w/o any problem is kernel 3.13.7 (I didn't use .8 and .9, but I do not think they would have been problematic).

Since 3.14.19 (I didn't test any 3.14 kernel before) I'm encountering a hard and silent lock up of the complete machine when starting the VM with the PCIe card passed through.

Since we're not really making any progress on this yet, would it be possible to bisect it? We already know that 3.13.7 works and 3.14.19 fails, and "git bisect start v3.14 v3.13" says it's about 13 steps. I know that's still quite a bit of work, but at least it sounds like the problem is easy to reproduce.

Quoted 19 lines · Bjorn Helgaas

Quoted 16 lines · Andreas Hartmann

shortly: I retested w/ qemu 2.1.0 and Linux 3.17.0 - no change in behaviour.

Quoted 13 lines · Alex Williamson

Quoted 12 lines · Andreas Hartmann

Hello!

Since long time now, I'm using w/o any problem PCIe pass through with a Gigabyte GA-990XA-UD3/GA-990XA-UD3 mainboard (AMD 990X chipset) and enabled IOMMU with vfio-pci.

The last kernel working w/o any problem is kernel 3.13.7 (I didn't use .8 and .9, but I do not think they would have been problematic).

Since 3.14.19 (I didn't test any 3.14 kernel before) I'm encountering a hard and silent lock up of the complete machine when starting the VM with the PCIe card passed through.

Quoted 5 lines

Since we're not really making any progress on this yet, would it be possible to bisect it? We already know that 3.13.7 works and 3.14.19 fails, and "git bisect start v3.14 v3.13" says it's about 13 steps. I know that's still quite a bit of work, but at least it sounds like the problem is easy to reproduce.

Which git repository should I use best?

Quoted 26 lines · Andreas Hartmann

Quoted 24 lines · Bjorn Helgaas

Quoted 16 lines · Andreas Hartmann

shortly: I retested w/ qemu 2.1.0 and Linux 3.17.0 - no change in behaviour.

Quoted 13 lines · Alex Williamson

Quoted 12 lines · Andreas Hartmann

Hello!

Since long time now, I'm using w/o any problem PCIe pass through with a Gigabyte GA-990XA-UD3/GA-990XA-UD3 mainboard (AMD 990X chipset) and enabled IOMMU with vfio-pci.

The last kernel working w/o any problem is kernel 3.13.7 (I didn't use .8 and .9, but I do not think they would have been problematic).

Since 3.14.19 (I didn't test any 3.14 kernel before) I'm encountering a hard and silent lock up of the complete machine when starting the VM with the PCIe card passed through.

Since we're not really making any progress on this yet, would it be possible to bisect it? We already know that 3.13.7 works and 3.14.19 fails, and "git bisect start v3.14 v3.13" says it's about 13 steps. I know that's still quite a bit of work, but at least it sounds like the problem is easy to reproduce.

Quoted 1 line

Which git repository should I use best?

The linux-stable repository [1] contains both the v3.13.x and the v3.14.x branches, but apparently you can't bisect directly between v3.13.7 and v3.14.19:

Quoted 30 lines · Bjorn Helgaas

Quoted 27 lines · Andreas Hartmann

Quoted 19 lines · Bjorn Helgaas

Quoted 16 lines · Andreas Hartmann

shortly: I retested w/ qemu 2.1.0 and Linux 3.17.0 - no change in behaviour.

Quoted 13 lines · Alex Williamson

Quoted 12 lines · Andreas Hartmann

Hello!

Since long time now, I'm using w/o any problem PCIe pass through with a Gigabyte GA-990XA-UD3/GA-990XA-UD3 mainboard (AMD 990X chipset) and enabled IOMMU with vfio-pci.

The last kernel working w/o any problem is kernel 3.13.7 (I didn't use .8 and .9, but I do not think they would have been problematic).

Since 3.14.19 (I didn't test any 3.14 kernel before) I'm encountering a hard and silent lock up of the complete machine when starting the VM with the PCIe card passed through.

Quoted 5 lines

Since we're not really making any progress on this yet, would it be possible to bisect it? We already know that 3.13.7 works and 3.14.19 fails, and "git bisect start v3.14 v3.13" says it's about 13 steps. I know that's still quite a bit of work, but at least it sounds like the problem is easy to reproduce.

Which git repository should I use best?

Quoted 3 lines

The linux-stable repository [1] contains both the v3.13.x and the v3.14.x branches, but apparently you can't bisect directly between v3.13.7 and v3.14.19:

I know that the first version after 3.13.0 (patch-v3.13-next-20140121) is already broken. Therefore, it must be between 3.13.7 and patch-v3.13-next-20140121.

Quoted 31 lines · Andreas Hartmann

Quoted 30 lines · Bjorn Helgaas

Quoted 27 lines · Andreas Hartmann

Quoted 19 lines · Bjorn Helgaas

Quoted 16 lines · Andreas Hartmann

shortly: I retested w/ qemu 2.1.0 and Linux 3.17.0 - no change in behaviour.

Quoted 13 lines · Alex Williamson

Quoted 12 lines · Andreas Hartmann

Hello!

Since long time now, I'm using w/o any problem PCIe pass through with a Gigabyte GA-990XA-UD3/GA-990XA-UD3 mainboard (AMD 990X chipset) and enabled IOMMU with vfio-pci.

The last kernel working w/o any problem is kernel 3.13.7 (I didn't use .8 and .9, but I do not think they would have been problematic).

Since 3.14.19 (I didn't test any 3.14 kernel before) I'm encountering a hard and silent lock up of the complete machine when starting the VM with the PCIe card passed through.

Quoted 5 lines

Since we're not really making any progress on this yet, would it be possible to bisect it? We already know that 3.13.7 works and 3.14.19 fails, and "git bisect start v3.14 v3.13" says it's about 13 steps. I know that's still quite a bit of work, but at least it sounds like the problem is easy to reproduce.

Which git repository should I use best?

Quoted 7 lines

Quoted 3 lines

The linux-stable repository [1] contains both the v3.13.x and the v3.14.x branches, but apparently you can't bisect directly between v3.13.7 and v3.14.19:

I know that the first version after 3.13.0 (patch-v3.13-next-20140121) is already broken. Therefore, it must be between 3.13.7 and patch-v3.13-next-20140121.

Quoted 33 lines · Bjorn Helgaas

Quoted 31 lines · Andreas Hartmann

Quoted 30 lines · Bjorn Helgaas

Quoted 27 lines · Andreas Hartmann

Quoted 19 lines · Bjorn Helgaas

Quoted 16 lines · Andreas Hartmann

shortly: I retested w/ qemu 2.1.0 and Linux 3.17.0 - no change in behaviour.

Quoted 13 lines · Alex Williamson

Quoted 12 lines · Andreas Hartmann

Hello!

Since long time now, I'm using w/o any problem PCIe pass through with a Gigabyte GA-990XA-UD3/GA-990XA-UD3 mainboard (AMD 990X chipset) and enabled IOMMU with vfio-pci.

The last kernel working w/o any problem is kernel 3.13.7 (I didn't use .8 and .9, but I do not think they would have been problematic).

Since 3.14.19 (I didn't test any 3.14 kernel before) I'm encountering a hard and silent lock up of the complete machine when starting the VM with the PCIe card passed through.

Quoted 5 lines

Since we're not really making any progress on this yet, would it be possible to bisect it? We already know that 3.13.7 works and 3.14.19 fails, and "git bisect start v3.14 v3.13" says it's about 13 steps. I know that's still quite a bit of work, but at least it sounds like the problem is easy to reproduce.

Which git repository should I use best?

Quoted 7 lines

Quoted 7 lines

Quoted 3 lines

The linux-stable repository [1] contains both the v3.13.x and the v3.14.x branches, but apparently you can't bisect directly between v3.13.7 and v3.14.19:

I know that the first version after 3.13.0 (patch-v3.13-next-20140121) is already broken. Therefore, it must be between 3.13.7 and patch-v3.13-next-20140121.

Ok, this is the result of git bisect:

425c1b223dac456d00a61fd6b451b6d1cf00d065 is the first bad commit commit 425c1b223dac456d00a61fd6b451b6d1cf00d065

Author: Alex Williamson <[email protected]>
Date:   Tue Dec 17 16:43:51 2013 -0700

PCI: Add Virtual Channel to save/restore support

While we don't really have any infrastructure for making use of VC support, the system BIOS can configure the topology to non-default VC values prior to boot. This may be due to silicon bugs, desire to reserve traffic classes, or perhaps just BIOS bugs. When we reset devices, the VC configuration may return to default values, which can be incompatible with devices upstream. For instance, Nvidia GRID cards provide a PCIe switch and some number of GPUs, all supporting VC. The power-on default for VC is to support TC0-7 across VC0, however some platforms will only enable TC0/VC0 mapping across the topology. When we do a secondary bus reset on the downstream switch port, the GPU is reset to a TC0-7/VC0 mapping while the opposite end of the link only enables TC0/VC0. If the GPU attempts to use TC1-7, it fails.

This patch attempts to provide complete support for VC save/restore, even beyond the minimally required use case above. This includes save/restore and reload of the arbitration table, save/restore and reload of the port arbitration tables, and re-enabling of the channels for VC, VC9, and MFVC capabilities.

Signed-off-by Alex Williamson <[email protected]>

Signed-off-by Bjorn Helgaas <[email protected]>

Quoted 72 lines · Andreas Hartmann

Quoted 33 lines · Bjorn Helgaas

Quoted 31 lines · Andreas Hartmann

Quoted 30 lines · Bjorn Helgaas

Quoted 27 lines · Andreas Hartmann

Quoted 19 lines · Bjorn Helgaas

Quoted 16 lines · Andreas Hartmann

shortly: I retested w/ qemu 2.1.0 and Linux 3.17.0 - no change in behaviour.

Quoted 13 lines · Alex Williamson

Quoted 12 lines · Andreas Hartmann

Hello!

Since long time now, I'm using w/o any problem PCIe pass through with a Gigabyte GA-990XA-UD3/GA-990XA-UD3 mainboard (AMD 990X chipset) and enabled IOMMU with vfio-pci.

The last kernel working w/o any problem is kernel 3.13.7 (I didn't use .8 and .9, but I do not think they would have been problematic).

Since 3.14.19 (I didn't test any 3.14 kernel before) I'm encountering a hard and silent lock up of the complete machine when starting the VM with the PCIe card passed through.

Quoted 5 lines

Since we're not really making any progress on this yet, would it be possible to bisect it? We already know that 3.13.7 works and 3.14.19 fails, and "git bisect start v3.14 v3.13" says it's about 13 steps. I know that's still quite a bit of work, but at least it sounds like the problem is easy to reproduce.

Which git repository should I use best?

Quoted 7 lines

Quoted 7 lines

Quoted 3 lines

The linux-stable repository [1] contains both the v3.13.x and the v3.14.x branches, but apparently you can't bisect directly between v3.13.7 and v3.14.19:

I know that the first version after 3.13.0 (patch-v3.13-next-20140121) is already broken. Therefore, it must be between 3.13.7 and patch-v3.13-next-20140121.

Ok, this is the result of git bisect:

425c1b223dac456d00a61fd6b451b6d1cf00d065 is the first bad commit commit 425c1b223dac456d00a61fd6b451b6d1cf00d065

Author: Alex Williamson <[email protected]>
Date:   Tue Dec 17 16:43:51 2013 -0700

PCI: Add Virtual Channel to save/restore support

While we don't really have any infrastructure for making use of VC support, the system BIOS can configure the topology to non-default VC values prior to boot. This may be due to silicon bugs, desire to reserve traffic classes, or perhaps just BIOS bugs. When we reset devices, the VC configuration may return to default values, which can be incompatible with devices upstream. For instance, Nvidia GRID cards provide a PCIe switch and some number of GPUs, all supporting VC. The power-on default for VC is to support TC0-7 across VC0, however some platforms will only enable TC0/VC0 mapping across the topology. When we do a secondary bus reset on the downstream switch port, the GPU is reset to a TC0-7/VC0 mapping while the opposite end of the link only enables TC0/VC0. If the GPU attempts to use TC1-7, it fails.

This patch attempts to provide complete support for VC save/restore, even beyond the minimally required use case above. This includes save/restore and reload of the arbitration table, save/restore and reload of the port arbitration tables, and re-enabling of the channels for VC, VC9, and MFVC capabilities.

Quoted 2 lines

Signed-off-by Alex Williamson <[email protected]>

Signed-off-by Bjorn Helgaas <[email protected]>

Wow, I'm amazed that you could get that done so fast... you must have spent your whole day working on this!

To double-check this, can you try applying the patch below? It should be enough to make things work if 425c1b223dac is really what's causing the trouble.

This patch is based on v3.17, but 425c1b223dac appeared in v3.14, so you should be able to apply it to v3.14 or any later kernel.

Bjorn

drivers/pci/pci.c +0 -3

index 2c9ac70254e2..8ef8bc56a584 100644
@@ -1007,8 +1007,6 @@ int pci_save_state(struct pci_dev *dev)
 		return i;
 	if ((i = pci_save_pcix_state(dev)) != 0)
 		return i;
-	if ((i = pci_save_vc_state(dev)) != 0)
-		return i;
 	return 0;
 }
 EXPORT_SYMBOL(pci_save_state);

@@ -1072,7 +1070,6 @@ void pci_restore_state(struct pci_dev *dev)
 	/* PCI Express register must be restored first */
 	pci_restore_pcie_state(dev);
 	pci_restore_ats_state(dev);
-	pci_restore_vc_state(dev);
 
 	pci_restore_config_space(dev);
 

Quoted 73 lines · Bjorn Helgaas

Quoted 72 lines · Andreas Hartmann

Quoted 33 lines · Bjorn Helgaas

Quoted 31 lines · Andreas Hartmann

Quoted 30 lines · Bjorn Helgaas

Quoted 27 lines · Andreas Hartmann

Quoted 19 lines · Bjorn Helgaas

Quoted 16 lines · Andreas Hartmann

shortly: I retested w/ qemu 2.1.0 and Linux 3.17.0 - no change in behaviour.

Quoted 13 lines · Alex Williamson

Quoted 12 lines · Andreas Hartmann

Hello!

Since long time now, I'm using w/o any problem PCIe pass through with a Gigabyte GA-990XA-UD3/GA-990XA-UD3 mainboard (AMD 990X chipset) and enabled IOMMU with vfio-pci.

The last kernel working w/o any problem is kernel 3.13.7 (I didn't use .8 and .9, but I do not think they would have been problematic).

Since 3.14.19 (I didn't test any 3.14 kernel before) I'm encountering a hard and silent lock up of the complete machine when starting the VM with the PCIe card passed through.

Quoted 5 lines

Since we're not really making any progress on this yet, would it be possible to bisect it? We already know that 3.13.7 works and 3.14.19 fails, and "git bisect start v3.14 v3.13" says it's about 13 steps. I know that's still quite a bit of work, but at least it sounds like the problem is easy to reproduce.

Which git repository should I use best?

Quoted 7 lines

Quoted 7 lines

Quoted 3 lines

The linux-stable repository [1] contains both the v3.13.x and the v3.14.x branches, but apparently you can't bisect directly between v3.13.7 and v3.14.19:

I know that the first version after 3.13.0 (patch-v3.13-next-20140121) is already broken. Therefore, it must be between 3.13.7 and patch-v3.13-next-20140121.

Ok, this is the result of git bisect:

425c1b223dac456d00a61fd6b451b6d1cf00d065 is the first bad commit commit 425c1b223dac456d00a61fd6b451b6d1cf00d065

Author: Alex Williamson <[email protected]>
Date:   Tue Dec 17 16:43:51 2013 -0700

PCI: Add Virtual Channel to save/restore support

While we don't really have any infrastructure for making use of VC support, the system BIOS can configure the topology to non-default VC values prior to boot. This may be due to silicon bugs, desire to reserve traffic classes, or perhaps just BIOS bugs. When we reset devices, the VC configuration may return to default values, which can be incompatible with devices upstream. For instance, Nvidia GRID cards provide a PCIe switch and some number of GPUs, all supporting VC. The power-on default for VC is to support TC0-7 across VC0, however some platforms will only enable TC0/VC0 mapping across the topology. When we do a secondary bus reset on the downstream switch port, the GPU is reset to a TC0-7/VC0 mapping while the opposite end of the link only enables TC0/VC0. If the GPU attempts to use TC1-7, it fails.

This patch attempts to provide complete support for VC save/restore, even beyond the minimally required use case above. This includes save/restore and reload of the arbitration table, save/restore and reload of the port arbitration tables, and re-enabling of the channels for VC, VC9, and MFVC capabilities.

Quoted 5 lines

Quoted 2 lines

Signed-off-by Alex Williamson <[email protected]>

Signed-off-by Bjorn Helgaas <[email protected]>

Wow, I'm amazed that you could get that done so fast... you must have spent your whole day working on this!

If I would have been more familiar with the versioning of the kernels and if I would have a faster internet connection and if there wouldn't be another bug in systemd, which has bitten me on booting with broken fs (but I found a cool workaround now :-)), I would have been much faster:
my 8 core machine and 8 GB of RAM, where I've been compiling the kernel in and my special kernel config (which Im using since 3.10) only containing my requests, with parts of the process automated makes it possible to have a turn around of ~ 7 minutes :-).
I too had no problem with reproducibility, because the problem always comes up at the start of the vm after 1 or 2 secs.

Quoted 33 lines

To double-check this, can you try applying the patch below? It should be enough to make things work if 425c1b223dac is really what's causing the trouble.

This patch is based on v3.17, but 425c1b223dac appeared in v3.14, so you should be able to apply it to v3.14 or any later kernel.

Bjorn

drivers/pci/pci.c +0 -3

index 2c9ac70254e2..8ef8bc56a584 100644
@@ -1007,8 +1007,6 @@ int pci_save_state(struct pci_dev *dev)
 		return i;
 	if ((i = pci_save_pcix_state(dev)) != 0)
 		return i;
-	if ((i = pci_save_vc_state(dev)) != 0)
-		return i;
 	return 0;
 }
 EXPORT_SYMBOL(pci_save_state);

@@ -1072,7 +1070,6 @@ void pci_restore_state(struct pci_dev *dev)
 	/* PCI Express register must be restored first */
 	pci_restore_pcie_state(dev);
 	pci_restore_ats_state(dev);
-	pci_restore_vc_state(dev);
 
 	pci_restore_config_space(dev);
 

drivers/pci/pci.c void pci_allocate_cap_save_buffers(struct pci_dev *dev) · line 2172

Quoted 8 lines

drivers/pci/pci.c +0 -2

@@ -2170,8 +2167,6 @@ void pci_allocate_cap_save_buffers(struct pci_dev *dev)
 	if (error)
 		dev_err(&dev->dev,
 			"unable to preallocate PCI-X save buffer\n");
-
-	pci_allocate_vc_save_buffers(dev);
 }
 
 void pci_free_cap_save_buffers(struct pci_dev *dev)

This patch proofed the git bisect result. I applied it to patch-v3.13-next-20140122 and the machine worked pretty fine :-).

Thanks,
Regards,
Andreas

Hi Andreas,

Sorry for the breakage. Is it possible to run lspci on the device in a loop from the host and capture whether we're failing to restore some of the VC bits to their previous state?

Does the problem also occur if you
unbind from host driver,

echo 1 > reset in pci-sysfs,

and re-bind to the

host? I'll also try to reproduce on my 990fx system, but I won't be able to do that until next week due to travel. Thanks,

Hello Alex,

Quoted 1 line · Alex Williamson

Hi Andreas,

[...]

Quoted 6 lines

Sorry for the breakage. Is it possible to run lspci on the device in a loop from the host and capture whether we're failing to restore some of the VC bits to their previous state?

Does the problem also occur if you
unbind from host driver,

The machine is booted w/ blacklisted ath9k. Then, the device is bound to vfio:

echo "168c 0030" > /sys/bus/pci/drivers/vfio-pci/new_id echo 0000:03:00.0 > /sys/bus/pci/devices/0000:03:00.0/driver/unbind echo 0000:03:00.0 > /sys/bus/pci/drivers/vfio-pci/bind

afterwards the VM is started -> hang.

W/o starting th VM, I can bind it to vfio and unbind it from vfio w/o any problem.

Quoted 1 line

echo 1 > reset in pci-sysfs,

echo 1 > /sys/bus/pci/devices/0000:03:00.0 works w/o any problem while bound to vfio. Even after unbinding from vfio and rebinding to vfio again ... .

Quoted 1 line

and re-bind to the

Do you mean loading ath9k in host system after unbinding from vfio? If
yes: Works w/o any problem. It's even possible to reset it or do a ifconfig wlan0 up, ifconfig wlan0 down, rmmod ath9k, bind it to vfio again and reset it, ....

Looks like the hang only is triggered by qemu-system_x86_64 on startup the VM.

Quoted 2 lines

host? I'll also try to reproduce on my 990fx system, but I won't be able to do that until next week due to travel. Thanks,

Hi Andreas,

Quoted 39 lines · Andreas Hartmann

Hello Alex,

Quoted 1 line · Alex Williamson

Hi Andreas,

[...]

Quoted 6 lines

Sorry for the breakage. Is it possible to run lspci on the device in a loop from the host and capture whether we're failing to restore some of the VC bits to their previous state?

Does the problem also occur if you
unbind from host driver,

The machine is booted w/ blacklisted ath9k. Then, the device is bound to vfio:

echo "168c 0030" > /sys/bus/pci/drivers/vfio-pci/new_id echo 0000:03:00.0 > /sys/bus/pci/devices/0000:03:00.0/driver/unbind echo 0000:03:00.0 > /sys/bus/pci/drivers/vfio-pci/bind

afterwards the VM is started -> hang.

W/o starting th VM, I can bind it to vfio and unbind it from vfio w/o any problem.

Quoted 1 line

echo 1 > reset in pci-sysfs,

echo 1 > /sys/bus/pci/devices/0000:03:00.0 works w/o any problem while bound to vfio. Even after unbinding from vfio and rebinding to vfio again ... .

Quoted 1 line

and re-bind to the

Do you mean loading ath9k in host system after unbinding from vfio? If
yes: Works w/o any problem. It's even possible to reset it or do a ifconfig wlan0 up, ifconfig wlan0 down, rmmod ath9k, bind it to vfio again and reset it, ....

Looks like the hang only is triggered by qemu-system_x86_64 on startup the VM.

Quoted 2 lines

Quoted 2 lines

host? I'll also try to reproduce on my 990fx system, but I won't be able to do that until next week due to travel. Thanks,

Could you send me the lspci -vvvxxxx for the device and parent root port? Thanks,

Alex

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Hi Andreas,

Quoted 37 lines · Andreas Hartmann

Hello Alex,

Quoted 1 line · Alex Williamson

Hi Andreas,

[...]

Quoted 6 lines

Sorry for the breakage. Is it possible to run lspci on the device in a loop from the host and capture whether we're failing to restore some of the VC bits to their previous state?

Does the problem also occur if you
unbind from host driver,

The machine is booted w/ blacklisted ath9k. Then, the device is bound to vfio:

echo "168c 0030" > /sys/bus/pci/drivers/vfio-pci/new_id echo 0000:03:00.0 > /sys/bus/pci/devices/0000:03:00.0/driver/unbind echo 0000:03:00.0 > /sys/bus/pci/drivers/vfio-pci/bind

afterwards the VM is started -> hang.

W/o starting th VM, I can bind it to vfio and unbind it from vfio w/o any problem.

Quoted 1 line

echo 1 > reset in pci-sysfs,

echo 1 > /sys/bus/pci/devices/0000:03:00.0 works w/o any problem while bound to vfio. Even after unbinding from vfio and rebinding to vfio again ... .

Quoted 1 line

and re-bind to the

Do you mean loading ath9k in host system after unbinding from vfio? If
yes: Works w/o any problem. It's even possible to reset it or do a ifconfig wlan0 up, ifconfig wlan0 down, rmmod ath9k, bind it to vfio again and reset it, ....

Quoted 5 lines

Looks like the hang only is triggered by qemu-system_x86_64 on startup the VM.

Quoted 2 lines

host? I'll also try to reproduce on my 990fx system, but I won't be able to do that until next week due to travel. Thanks,

Could you send me the lspci -vvvxxxx for the device and parent root port? Thanks,

Quoted 40 lines · Alex Williamson

Hi Andreas,

Quoted 37 lines · Andreas Hartmann

Hello Alex,

Quoted 1 line · Alex Williamson

Hi Andreas,

[...]

Quoted 6 lines

Sorry for the breakage. Is it possible to run lspci on the device in a loop from the host and capture whether we're failing to restore some of the VC bits to their previous state?

Does the problem also occur if you
unbind from host driver,

The machine is booted w/ blacklisted ath9k. Then, the device is bound to vfio:

echo "168c 0030" > /sys/bus/pci/drivers/vfio-pci/new_id echo 0000:03:00.0 > /sys/bus/pci/devices/0000:03:00.0/driver/unbind echo 0000:03:00.0 > /sys/bus/pci/drivers/vfio-pci/bind

afterwards the VM is started -> hang.

W/o starting th VM, I can bind it to vfio and unbind it from vfio w/o any problem.

Quoted 1 line

echo 1 > reset in pci-sysfs,

echo 1 > /sys/bus/pci/devices/0000:03:00.0 works w/o any problem while bound to vfio. Even after unbinding from vfio and rebinding to vfio again ... .

Quoted 1 line

and re-bind to the

Do you mean loading ath9k in host system after unbinding from vfio? If
yes: Works w/o any problem. It's even possible to reset it or do a ifconfig wlan0 up, ifconfig wlan0 down, rmmod ath9k, bind it to vfio again and reset it, ....

Quoted 8 lines

Quoted 5 lines

Looks like the hang only is triggered by qemu-system_x86_64 on startup the VM.

Quoted 2 lines

host? I'll also try to reproduce on my 990fx system, but I won't be able to do that until next week due to travel. Thanks,

Could you send me the lspci -vvvxxxx for the device and parent root port? Thanks,

Done with kernel 3.12.28 in host while the device was used in VM:

# lspci -vt
-[0000:00]-+-00.0  Advanced Micro Devices, Inc. [AMD/ATI] RD890 PCI to PCI bridge (external gfx0 port B)
           +-00.2  Advanced Micro Devices, Inc. [AMD/ATI] RD990 I/O Memory Management Unit (IOMMU)
           +-02.0-[01]--+-00.0  Advanced Micro Devices, Inc. [AMD/ATI] Turks PRO [Radeon HD 6570/7570]
           |            \-00.1  Advanced Micro Devices, Inc. [AMD/ATI] Turks/Whistler HDMI Audio [Radeon HD 6000 Series]
           +-04.0-[02]----00.0  Etron Technology, Inc. EJ168 USB 3.0 Host Controller
           +-05.0-[03]----00.0  Qualcomm Atheros AR93xx Wireless Network Adapter
           +-09.0-[04]----00.0  Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller
           +-0a.0-[05]----00.0  Etron Technology, Inc. EJ168 USB 3.0 Host Controller
           +-11.0  Advanced Micro Devices, Inc. [AMD/ATI] SB7x0/SB8x0/SB9x0 SATA Controller [AHCI mode]
           +-12.0  Advanced Micro Devices, Inc. [AMD/ATI] SB7x0/SB8x0/SB9x0 USB OHCI0 Controller
           +-12.2  Advanced Micro Devices, Inc. [AMD/ATI] SB7x0/SB8x0/SB9x0 USB EHCI Controller
           +-13.0  Advanced Micro Devices, Inc. [AMD/ATI] SB7x0/SB8x0/SB9x0 USB OHCI0 Controller
           +-13.2  Advanced Micro Devices, Inc. [AMD/ATI] SB7x0/SB8x0/SB9x0 USB EHCI Controller
           +-14.0  Advanced Micro Devices, Inc. [AMD/ATI] SBx00 SMBus Controller
           +-14.2  Advanced Micro Devices, Inc. [AMD/ATI] SBx00 Azalia (Intel HDA)
           +-14.3  Advanced Micro Devices, Inc. [AMD/ATI] SB7x0/SB8x0/SB9x0 LPC host controller
           +-14.4-[06]--+-06.0  Intel Corporation 82557/8/9/0/1 Ethernet Pro 100
           |            \-0e.0  VIA Technologies, Inc. VT6306/7/8 [Fire II(M)] IEEE 1394 OHCI Controller
           +-14.5  Advanced Micro Devices, Inc. [AMD/ATI] SB7x0/SB8x0/SB9x0 USB OHCI2 Controller
           +-15.0-[07]--
           +-16.0  Advanced Micro Devices, Inc. [AMD/ATI] SB7x0/SB8x0/SB9x0 USB OHCI0 Controller
           +-16.2  Advanced Micro Devices, Inc. [AMD/ATI] SB7x0/SB8x0/SB9x0 USB EHCI Controller
           +-18.0  Advanced Micro Devices, Inc. [AMD] Family 15h Processor Function 0
           +-18.1  Advanced Micro Devices, Inc. [AMD] Family 15h Processor Function 1
           +-18.2  Advanced Micro Devices, Inc. [AMD] Family 15h Processor Function 2
           +-18.3  Advanced Micro Devices, Inc. [AMD] Family 15h Processor Function 3
           +-18.4  Advanced Micro Devices, Inc. [AMD] Family 15h Processor Function 4
           \-18.5  Advanced Micro Devices, Inc. [AMD] Family 15h Processor Function 5

# lspci -s 03:00 -vvvxxxx
03:00.0 Network controller: Qualcomm Atheros AR93xx Wireless Network Adapter (rev 01)

I'm not sure what you mean with "parent root port". Could it be this:

Quoted 84 lines

Quoted 40 lines · Alex Williamson

Hi Andreas,

Quoted 37 lines · Andreas Hartmann

Hello Alex,

Quoted 1 line · Alex Williamson

Hi Andreas,

[...]

Quoted 6 lines

Sorry for the breakage. Is it possible to run lspci on the device in a loop from the host and capture whether we're failing to restore some of the VC bits to their previous state?

Does the problem also occur if you
unbind from host driver,

The machine is booted w/ blacklisted ath9k. Then, the device is bound to vfio:

echo "168c 0030" > /sys/bus/pci/drivers/vfio-pci/new_id echo 0000:03:00.0 > /sys/bus/pci/devices/0000:03:00.0/driver/unbind echo 0000:03:00.0 > /sys/bus/pci/drivers/vfio-pci/bind

afterwards the VM is started -> hang.

W/o starting th VM, I can bind it to vfio and unbind it from vfio w/o any problem.

Quoted 1 line

echo 1 > reset in pci-sysfs,

echo 1 > /sys/bus/pci/devices/0000:03:00.0 works w/o any problem while bound to vfio. Even after unbinding from vfio and rebinding to vfio again ... .

Quoted 1 line

and re-bind to the

Do you mean loading ath9k in host system after unbinding from vfio? If
yes: Works w/o any problem. It's even possible to reset it or do a ifconfig wlan0 up, ifconfig wlan0 down, rmmod ath9k, bind it to vfio again and reset it, ....

Quoted 8 lines

Quoted 5 lines

Looks like the hang only is triggered by qemu-system_x86_64 on startup the VM.

Quoted 2 lines

host? I'll also try to reproduce on my 990fx system, but I won't be able to do that until next week due to travel. Thanks,

Could you send me the lspci -vvvxxxx for the device and parent root port? Thanks,

Done with kernel 3.12.28 in host while the device was used in VM:

# lspci -vt
-[0000:00]-+-00.0  Advanced Micro Devices, Inc. [AMD/ATI] RD890 PCI to PCI bridge (external gfx0 port B)
           +-00.2  Advanced Micro Devices, Inc. [AMD/ATI] RD990 I/O Memory Management Unit (IOMMU)
           +-02.0-[01]--+-00.0  Advanced Micro Devices, Inc. [AMD/ATI] Turks PRO [Radeon HD 6570/7570]
           |            \-00.1  Advanced Micro Devices, Inc. [AMD/ATI] Turks/Whistler HDMI Audio [Radeon HD 6000 Series]
           +-04.0-[02]----00.0  Etron Technology, Inc. EJ168 USB 3.0 Host Controller
           +-05.0-[03]----00.0  Qualcomm Atheros AR93xx Wireless Network Adapter
           +-09.0-[04]----00.0  Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller
           +-0a.0-[05]----00.0  Etron Technology, Inc. EJ168 USB 3.0 Host Controller
           +-11.0  Advanced Micro Devices, Inc. [AMD/ATI] SB7x0/SB8x0/SB9x0 SATA Controller [AHCI mode]
           +-12.0  Advanced Micro Devices, Inc. [AMD/ATI] SB7x0/SB8x0/SB9x0 USB OHCI0 Controller
           +-12.2  Advanced Micro Devices, Inc. [AMD/ATI] SB7x0/SB8x0/SB9x0 USB EHCI Controller
           +-13.0  Advanced Micro Devices, Inc. [AMD/ATI] SB7x0/SB8x0/SB9x0 USB OHCI0 Controller
           +-13.2  Advanced Micro Devices, Inc. [AMD/ATI] SB7x0/SB8x0/SB9x0 USB EHCI Controller
           +-14.0  Advanced Micro Devices, Inc. [AMD/ATI] SBx00 SMBus Controller
           +-14.2  Advanced Micro Devices, Inc. [AMD/ATI] SBx00 Azalia (Intel HDA)
           +-14.3  Advanced Micro Devices, Inc. [AMD/ATI] SB7x0/SB8x0/SB9x0 LPC host controller
           +-14.4-[06]--+-06.0  Intel Corporation 82557/8/9/0/1 Ethernet Pro 100
           |            \-0e.0  VIA Technologies, Inc. VT6306/7/8 [Fire II(M)] IEEE 1394 OHCI Controller
           +-14.5  Advanced Micro Devices, Inc. [AMD/ATI] SB7x0/SB8x0/SB9x0 USB OHCI2 Controller
           +-15.0-[07]--
           +-16.0  Advanced Micro Devices, Inc. [AMD/ATI] SB7x0/SB8x0/SB9x0 USB OHCI0 Controller
           +-16.2  Advanced Micro Devices, Inc. [AMD/ATI] SB7x0/SB8x0/SB9x0 USB EHCI Controller
           +-18.0  Advanced Micro Devices, Inc. [AMD] Family 15h Processor Function 0
           +-18.1  Advanced Micro Devices, Inc. [AMD] Family 15h Processor Function 1
           +-18.2  Advanced Micro Devices, Inc. [AMD] Family 15h Processor Function 2
           +-18.3  Advanced Micro Devices, Inc. [AMD] Family 15h Processor Function 3
           +-18.4  Advanced Micro Devices, Inc. [AMD] Family 15h Processor Function 4
           \-18.5  Advanced Micro Devices, Inc. [AMD] Family 15h Processor Function 5

Quoted 2 lines

# lspci -s 03:00 -vvvxxxx
03:00.0 Network controller: Qualcomm Atheros AR93xx Wireless Network Adapter (rev 01)

[snip]

Quoted 3 lines

I'm not sure what you mean with "parent root port". Could it be this:

No, it's 00:05.0

Quoted 42 lines · Alex Williamson

Hi Andreas,

Quoted 39 lines · Andreas Hartmann

Hello Alex,

Quoted 1 line · Alex Williamson

Hi Andreas,

[...]

Quoted 6 lines

Sorry for the breakage. Is it possible to run lspci on the device in a loop from the host and capture whether we're failing to restore some of the VC bits to their previous state?

Does the problem also occur if you
unbind from host driver,

The machine is booted w/ blacklisted ath9k. Then, the device is bound to vfio:

echo "168c 0030" > /sys/bus/pci/drivers/vfio-pci/new_id echo 0000:03:00.0 > /sys/bus/pci/devices/0000:03:00.0/driver/unbind echo 0000:03:00.0 > /sys/bus/pci/drivers/vfio-pci/bind

afterwards the VM is started -> hang.

W/o starting th VM, I can bind it to vfio and unbind it from vfio w/o any problem.

Quoted 1 line

echo 1 > reset in pci-sysfs,

echo 1 > /sys/bus/pci/devices/0000:03:00.0 works w/o any problem while bound to vfio. Even after unbinding from vfio and rebinding to vfio again ... .

Quoted 1 line

and re-bind to the

Do you mean loading ath9k in host system after unbinding from vfio? If
yes: Works w/o any problem. It's even possible to reset it or do a ifconfig wlan0 up, ifconfig wlan0 down, rmmod ath9k, bind it to vfio again and reset it, ....

Looks like the hang only is triggered by qemu-system_x86_64 on startup the VM.

Also, this might be because QEMU since 1.7 will favor doing a bus reset for a device over PM reset while the sysfs reset interface will only do a bus reset if there are no other methods available and there are no other devices on the bus. Can you reproduce the hang using the sysfs reset interface without QEMU if you modify the kernel like this:

drivers/pci/pci.c +1 -1

@@ -3308,15 +3308,15 @@ static int __pci_dev_reset(struct pci_dev *dev, int prob
        if (rc != -ENOTTY)
                goto done;
 
-       rc = pci_pm_reset(dev, probe);
+       rc = pci_dev_reset_slot_function(dev, probe);
        if (rc != -ENOTTY)
                goto done;
 

Quoted 16 lines · Alex Williamson

drivers/pci/pci.c +2 -2

@@ -3308,15 +3308,15 @@ static int __pci_dev_reset(struct pci_dev *dev, int prob
        if (rc != -ENOTTY)
                goto done;
 
-       rc = pci_pm_reset(dev, probe);
+       rc = pci_dev_reset_slot_function(dev, probe);
        if (rc != -ENOTTY)
                goto done;
 
-       rc = pci_dev_reset_slot_function(dev, probe);
+       rc = pci_parent_bus_reset(dev, probe);
        if (rc != -ENOTTY)
                goto done;
 

drivers/pci/pci.c static int __pci_dev_reset(struct pci_dev *dev, int prob · line 3322

Quoted 5 lines

drivers/pci/pci.c +1 -1

-       rc = pci_parent_bus_reset(dev, probe);
+       rc = pci_pm_reset(dev, probe);
 done:
        return rc;
 }

This way it's crashing with echo 1 > reset, too.

Quoted 17 lines · Andreas Hartmann

Quoted 16 lines · Alex Williamson

drivers/pci/pci.c +2 -2

@@ -3308,15 +3308,15 @@ static int __pci_dev_reset(struct pci_dev *dev, int prob
        if (rc != -ENOTTY)
                goto done;
 
-       rc = pci_pm_reset(dev, probe);
+       rc = pci_dev_reset_slot_function(dev, probe);
        if (rc != -ENOTTY)
                goto done;
 
-       rc = pci_dev_reset_slot_function(dev, probe);
+       rc = pci_parent_bus_reset(dev, probe);
        if (rc != -ENOTTY)
                goto done;
 

Quoted 7 lines

drivers/pci/pci.c static int __pci_dev_reset(struct pci_dev *dev, int prob · line 3322

Quoted 5 lines

drivers/pci/pci.c +1 -1

-       rc = pci_parent_bus_reset(dev, probe);
+       rc = pci_pm_reset(dev, probe);
 done:
        return rc;
 }

This way it's crashing with echo 1 > reset, too.

Ok, so it's somehow related to doing a bus reset with virtual channel save/restore while PM reset with VC save/restore works ok as apparently does bus reset without VC save/restore. Let's try to do a manual bus reset so we can look at the post reset state of the device before the kernel tries to restore it.

First bind the target device 03:00.0 to pci-stub or vfio-pci so that we know it's not being used.

Next capture lspci -xxxx -s 3:00.0 so we have the starting state.

Then we'll do a bus reset using setpci:

# setpci -s 00:05.0 3e.w=40:40
<if you script this, wait at least 2ms here>
# setpci -s 00:05.0 3e.w=00:40
<wait 1 second here>

Now re-capture lspci -xxxx -s 3:00.0

Quoted 37 lines · Alex Williamson

Quoted 17 lines · Andreas Hartmann

Quoted 16 lines · Alex Williamson

drivers/pci/pci.c +2 -2

@@ -3308,15 +3308,15 @@ static int __pci_dev_reset(struct pci_dev *dev, int prob
        if (rc != -ENOTTY)
                goto done;
 
-       rc = pci_pm_reset(dev, probe);
+       rc = pci_dev_reset_slot_function(dev, probe);
        if (rc != -ENOTTY)
                goto done;
 
-       rc = pci_dev_reset_slot_function(dev, probe);
+       rc = pci_parent_bus_reset(dev, probe);
        if (rc != -ENOTTY)
                goto done;
 

Quoted 7 lines

drivers/pci/pci.c static int __pci_dev_reset(struct pci_dev *dev, int prob · line 3322

Quoted 5 lines

drivers/pci/pci.c +1 -1

-       rc = pci_parent_bus_reset(dev, probe);
+       rc = pci_pm_reset(dev, probe);
 done:
        return rc;
 }

This way it's crashing with echo 1 > reset, too.

Ok, so it's somehow related to doing a bus reset with virtual channel save/restore while PM reset with VC save/restore works ok as apparently does bus reset without VC save/restore. Let's try to do a manual bus reset so we can look at the post reset state of the device before the kernel tries to restore it.

First bind the target device 03:00.0 to pci-stub or vfio-pci so that we know it's not being used.

Next capture lspci -xxxx -s 3:00.0 so we have the starting state.

Quoted 7 lines

Then we'll do a bus reset using setpci:

# setpci -s 00:05.0 3e.w=40:40
<if you script this, wait at least 2ms here>
# setpci -s 00:05.0 3e.w=00:40
<wait 1 second here>

Now re-capture lspci -xxxx -s 3:00.0

The machine is booted w/ vfio bound to 3:00.0 as usual (now for testing linux 3.14)

lspci -xxxx -s 3:00.0
setpci -s 00:05.0 3e.w=40:40
usleep 10
setpci -s 00:05.0 3e.w=00:40
sleep 1
lspci -xxxx -s 3:00.0

I didn't get the second lspci because the machine already was hanging. The first output is attached completely.

Quoted 56 lines · Andreas Hartmann

Quoted 37 lines · Alex Williamson

Quoted 17 lines · Andreas Hartmann

Quoted 16 lines · Alex Williamson

drivers/pci/pci.c +2 -2

@@ -3308,15 +3308,15 @@ static int __pci_dev_reset(struct pci_dev *dev, int prob
        if (rc != -ENOTTY)
                goto done;
 
-       rc = pci_pm_reset(dev, probe);
+       rc = pci_dev_reset_slot_function(dev, probe);
        if (rc != -ENOTTY)
                goto done;
 
-       rc = pci_dev_reset_slot_function(dev, probe);
+       rc = pci_parent_bus_reset(dev, probe);
        if (rc != -ENOTTY)
                goto done;
 

Quoted 7 lines

drivers/pci/pci.c static int __pci_dev_reset(struct pci_dev *dev, int prob · line 3322

Quoted 5 lines

drivers/pci/pci.c +1 -1

-       rc = pci_parent_bus_reset(dev, probe);
+       rc = pci_pm_reset(dev, probe);
 done:
        return rc;
 }

This way it's crashing with echo 1 > reset, too.

Ok, so it's somehow related to doing a bus reset with virtual channel save/restore while PM reset with VC save/restore works ok as apparently does bus reset without VC save/restore. Let's try to do a manual bus reset so we can look at the post reset state of the device before the kernel tries to restore it.

First bind the target device 03:00.0 to pci-stub or vfio-pci so that we know it's not being used.

Next capture lspci -xxxx -s 3:00.0 so we have the starting state.

Quoted 7 lines

Then we'll do a bus reset using setpci:

# setpci -s 00:05.0 3e.w=40:40
<if you script this, wait at least 2ms here>
# setpci -s 00:05.0 3e.w=00:40
<wait 1 second here>

Now re-capture lspci -xxxx -s 3:00.0

The machine is booted w/ vfio bound to 3:00.0 as usual (now for testing linux 3.14)

lspci -xxxx -s 3:00.0
setpci -s 00:05.0 3e.w=40:40
usleep 10
setpci -s 00:05.0 3e.w=00:40
sleep 1
lspci -xxxx -s 3:00.0

Quoted 2 lines

I didn't get the second lspci because the machine already was hanging. The first output is attached completely.

Hmm, that doesn't make much sense. You had found that if you disabled the VC save/restore then QEMU works. That should have still been using secondary bus reset as we're trying to do here, so I don't understand why we can't do a manual secondary bus reset now.

If you use Bjorn's previous patch to disable VC save/restore and my patch to reorder the reset mechanisms, does echo 1 > reset for the sysfs entry for the device also still cause a hang?

Can you provide a link to the specific model for this card? Thanks,

If you use Bjorn's previous patch to disable VC save/restore and my patch to reorder the reset mechanisms, does echo 1 > reset for the sysfs entry for the device also still cause a hang?

Alex Williamson wrote:
[...]

Quoted 3 lines

If you use Bjorn's previous patch to disable VC save/restore and my patch to reorder the reset mechanisms, does echo 1 > reset for the sysfs entry for the device also still cause a hang?

Yes - it's hanging too (w/ vfio bound to the device - didn't test other possibilities).

Quoted 8 lines · Andreas Hartmann

Alex Williamson wrote:
[...]

Quoted 3 lines

If you use Bjorn's previous patch to disable VC save/restore and my patch to reorder the reset mechanisms, does echo 1 > reset for the sysfs entry for the device also still cause a hang?

Yes - it's hanging too (w/ vfio bound to the device - didn't test other possibilities).

Does it happen regardless of the slot the card is plugged into? Thanks,

Quoted 8 lines · Andreas Hartmann

Alex Williamson wrote:
[...]

Quoted 3 lines

If you use Bjorn's previous patch to disable VC save/restore and my patch to reorder the reset mechanisms, does echo 1 > reset for the sysfs entry for the device also still cause a hang?

Yes - it's hanging too (w/ vfio bound to the device - didn't test other possibilities).

Does it happen regardless of the slot the card is plugged into? Thanks,

Out of interest:
Bjorn's patch disables vc save/restore support - and the machine works fine again. Why is it needed at all if it seems to work perfectly w/o it? What's the additional benefit? Or in other words: What am I missing until today :-) ? What would be better? What could I do more?

Quoted 6 lines · Andreas Hartmann

Out of interest:
Bjorn's patch disables vc save/restore support - and the machine works fine again. Why is it needed at all if it seems to work perfectly w/o it? What's the additional benefit? Or in other words: What am I missing until today :-) ? What would be better? What could I do more?

You're right, in the configuration you have the endpoint device has a Virtual Channel capability but the upstream root port does not. The spec is not at all clear about defining the endpoints for enabling Virtual Channel in each type of configuration, but I think that if we have an upstream port that does not support Virtual Channel, we can skip the save/restore. Please test the patch below.

I'm also still completely confused about whether this is a VC save/restore issue or a bus reset issue. You originally bisected this back to the VC save/restore patch, but you also found that a manual, setpci-based bus reset triggered a system hang.

I believe that
re-ordering the kernel reset mechanisms also triggered this. Since recent versions of QEMU are going to favor a bus reset over PM reset, I don't have a lot of confidence that we're actually solving the problem for you. Please make sure to test with a recent QEMU to be sure we'll do a bus reset.

drivers/pci/vc.c +19 -0

index 7e1304d..6d13d34 100644
@@ -339,6 +339,25 @@ static int pci_vc_do_save_buffer(struct pci_dev *dev, int pos,
 	return buf ? 0 : len;
 }
 
+/**
+ * pci_vc_needs_save - Determine whether a VC capability needs to be saved
+ * @dev: device
+ * @id: VC capability ID (VC/VC9/MFVC)
+ *
+ * In configurations where we have a VC or MFVC capability, but the upstream
+ * device does not, we assume that VC save (and therefore restore) is not
+ * necessary.  The intention is to only do VC save/restore in configuration
+ * where it's necessary and hopefully avoid reset issues.
+ */
+static bool pci_vc_needs_save(struct pci_dev *dev, u16 id)
+{
+	if (id == PCI_EXT_CAP_ID_VC9 || pci_is_root_bus(dev->bus) ||
+	    pci_find_ext_capability(dev->bus->self, PCI_EXT_CAP_ID_VC))
+		return true;
+
+	return false;
+}
+
 static struct {
 	u16 id;
 	const char *name;

Quoted 16 lines · Alex Williamson

Quoted 6 lines · Andreas Hartmann

Out of interest:
Bjorn's patch disables vc save/restore support - and the machine works fine again. Why is it needed at all if it seems to work perfectly w/o it? What's the additional benefit? Or in other words: What am I missing until today :-) ? What would be better? What could I do more?

You're right, in the configuration you have the endpoint device has a Virtual Channel capability but the upstream root port does not. The spec is not at all clear about defining the endpoints for enabling Virtual Channel in each type of configuration, but I think that if we have an upstream port that does not support Virtual Channel, we can skip the save/restore. Please test the patch below.

Quoted 4 lines

I'm also still completely confused about whether this is a VC save/restore issue or a bus reset issue. You originally bisected this back to the VC save/restore patch, but you also found that a manual, setpci-based bus reset triggered a system hang.

With your additional patch posted here:
http://article.gmane.org/gmane.linux.kernel.pci/36162

Quoted 7 lines

I believe that
re-ordering the kernel reset mechanisms also triggered this. Since recent versions of QEMU are going to favor a bus reset over PM reset, I don't have a lot of confidence that we're actually solving the problem for you. Please make sure to test with a recent QEMU to be sure we'll do a bus reset.

I'm running qemu 2.1.0 (newest is 2.1.2 - but this shouldn't be a problem) and tested w/ linux 3.17.

Quoted 33 lines

drivers/pci/vc.c +19 -0

index 7e1304d..6d13d34 100644
@@ -339,6 +339,25 @@ static int pci_vc_do_save_buffer(struct pci_dev *dev, int pos,
 	return buf ? 0 : len;
 }
 
+/**
+ * pci_vc_needs_save - Determine whether a VC capability needs to be saved
+ * @dev: device
+ * @id: VC capability ID (VC/VC9/MFVC)
+ *
+ * In configurations where we have a VC or MFVC capability, but the upstream
+ * device does not, we assume that VC save (and therefore restore) is not
+ * necessary.  The intention is to only do VC save/restore in configuration
+ * where it's necessary and hopefully avoid reset issues.
+ */
+static bool pci_vc_needs_save(struct pci_dev *dev, u16 id)
+{
+	if (id == PCI_EXT_CAP_ID_VC9 || pci_is_root_bus(dev->bus) ||
+	    pci_find_ext_capability(dev->bus->self, PCI_EXT_CAP_ID_VC))
+		return true;
+
+	return false;
+}
+
 static struct {
 	u16 id;
 	const char *name;

@@ -362,7 +381,7 @@ int pci_save_vc_state(struct pci_dev *dev)
 		struct pci_cap_saved_state *save_state;
 

drivers/pci/vc.c int pci_save_vc_state(struct pci_dev *dev) · line 384

Quoted 3 lines

drivers/pci/vc.c +1 -1

 		pos = pci_find_ext_capability(dev, vc_caps[i].id);
-		if (!pos)
+		if (!posi || !pci_vc_needs_save(dev, vc_caps[i].id))

                        ^

This should be most probably !pos (and not !posi - because !posi does through a compile error).

Quoted 11 lines

drivers/pci/vc.c +1 -1

 			continue;
 
 		save_state = pci_find_saved_ext_cap(dev, vc_caps[i].id);

@@ -422,7 +441,7 @@ void pci_allocate_vc_save_buffers(struct pci_dev *dev)
 	for (i = 0; i < ARRAY_SIZE(vc_caps); i++) {
 		int len, pos = pci_find_ext_capability(dev, vc_caps[i].id);
 
-		if (!pos)
+		if (!pos || !pci_vc_needs_save(dev, vc_caps[i].id))
 			continue;
 

drivers/pci/vc.c void pci_allocate_vc_save_buffers(struct pci_dev *dev) · line 447

Quoted 1 line

drivers/pci/vc.c +0 -0

 		len = pci_vc_do_save_buffer(dev, pos, NULL, false);

W/ the above patch, the machine hangs again (w/ qemu and setpci), but w/ Bjorn's patch (and nothing more applied) which disables vc save/restore, the machine just works fine ... . I especially retested this case to be really sure. I'm so sorry. But that's how it behaves here :-(

Quoted 17 lines · Andreas Hartmann

Quoted 16 lines · Alex Williamson

Quoted 6 lines · Andreas Hartmann

Out of interest:
Bjorn's patch disables vc save/restore support - and the machine works fine again. Why is it needed at all if it seems to work perfectly w/o it? What's the additional benefit? Or in other words: What am I missing until today :-) ? What would be better? What could I do more?

You're right, in the configuration you have the endpoint device has a Virtual Channel capability but the upstream root port does not. The spec is not at all clear about defining the endpoints for enabling Virtual Channel in each type of configuration, but I think that if we have an upstream port that does not support Virtual Channel, we can skip the save/restore. Please test the patch below.

Quoted 7 lines

Quoted 4 lines

I'm also still completely confused about whether this is a VC save/restore issue or a bus reset issue. You originally bisected this back to the VC save/restore patch, but you also found that a manual, setpci-based bus reset triggered a system hang.

With your additional patch posted here:
http://article.gmane.org/gmane.linux.kernel.pci/36162

Right, a reset via sysfs also triggered it with that patch, but the reset via setpci is independent of any VC save/restore and still hung your box.

Quoted 10 lines

Quoted 7 lines

I believe that
re-ordering the kernel reset mechanisms also triggered this. Since recent versions of QEMU are going to favor a bus reset over PM reset, I don't have a lot of confidence that we're actually solving the problem for you. Please make sure to test with a recent QEMU to be sure we'll do a bus reset.

I'm running qemu 2.1.0 (newest is 2.1.2 - but this shouldn't be a problem) and tested w/ linux 3.17.

Yep, just want to make sure it's QEMU new enough to do a bus reset and kernel with matching support.

Quoted 33 lines

Quoted 33 lines

drivers/pci/vc.c +19 -0

index 7e1304d..6d13d34 100644
@@ -339,6 +339,25 @@ static int pci_vc_do_save_buffer(struct pci_dev *dev, int pos,
 	return buf ? 0 : len;
 }
 
+/**
+ * pci_vc_needs_save - Determine whether a VC capability needs to be saved
+ * @dev: device
+ * @id: VC capability ID (VC/VC9/MFVC)
+ *
+ * In configurations where we have a VC or MFVC capability, but the upstream
+ * device does not, we assume that VC save (and therefore restore) is not
+ * necessary.  The intention is to only do VC save/restore in configuration
+ * where it's necessary and hopefully avoid reset issues.
+ */
+static bool pci_vc_needs_save(struct pci_dev *dev, u16 id)
+{
+	if (id == PCI_EXT_CAP_ID_VC9 || pci_is_root_bus(dev->bus) ||
+	    pci_find_ext_capability(dev->bus->self, PCI_EXT_CAP_ID_VC))
+		return true;
+
+	return false;
+}
+
 static struct {
 	u16 id;
 	const char *name;

@@ -362,7 +381,7 @@ int pci_save_vc_state(struct pci_dev *dev)
 		struct pci_cap_saved_state *save_state;
 

Quoted 6 lines

drivers/pci/vc.c int pci_save_vc_state(struct pci_dev *dev) · line 384

Quoted 3 lines

drivers/pci/vc.c +1 -1

 		pos = pci_find_ext_capability(dev, vc_caps[i].id);
-		if (!pos)
+		if (!posi || !pci_vc_needs_save(dev, vc_caps[i].id))

                        ^

This should be most probably !pos (and not !posi - because !posi does through a compile error).

Oops, sorry.

Quoted 11 lines

Quoted 11 lines

drivers/pci/vc.c +1 -1

 			continue;
 
 		save_state = pci_find_saved_ext_cap(dev, vc_caps[i].id);

@@ -422,7 +441,7 @@ void pci_allocate_vc_save_buffers(struct pci_dev *dev)
 	for (i = 0; i < ARRAY_SIZE(vc_caps); i++) {
 		int len, pos = pci_find_ext_capability(dev, vc_caps[i].id);
 
-		if (!pos)
+		if (!pos || !pci_vc_needs_save(dev, vc_caps[i].id))
 			continue;
 

Quoted 6 lines

drivers/pci/vc.c void pci_allocate_vc_save_buffers(struct pci_dev *dev) · line 447

Quoted 1 line

drivers/pci/vc.c +0 -0

 		len = pci_vc_do_save_buffer(dev, pos, NULL, false);

W/ the above patch, the machine hangs again (w/ qemu and setpci), but w/ Bjorn's patch (and nothing more applied) which disables vc save/restore, the machine just works fine ... . I especially retested this case to be really sure. I'm so sorry. But that's how it behaves here :-(

Hmm, the intention was that this should effectively do the same thing as Bjorn's patch. The Atheros device (03:00.0) reports a VC capability but the root port above it (00:05.0) does not.

Quoted 104 lines · Alex Williamson

Quoted 17 lines · Andreas Hartmann

Quoted 16 lines · Alex Williamson

Quoted 6 lines · Andreas Hartmann

Out of interest:
Bjorn's patch disables vc save/restore support - and the machine works fine again. Why is it needed at all if it seems to work perfectly w/o it? What's the additional benefit? Or in other words: What am I missing until today :-) ? What would be better? What could I do more?

You're right, in the configuration you have the endpoint device has a Virtual Channel capability but the upstream root port does not. The spec is not at all clear about defining the endpoints for enabling Virtual Channel in each type of configuration, but I think that if we have an upstream port that does not support Virtual Channel, we can skip the save/restore. Please test the patch below.

Quoted 7 lines

Quoted 4 lines

I'm also still completely confused about whether this is a VC save/restore issue or a bus reset issue. You originally bisected this back to the VC save/restore patch, but you also found that a manual, setpci-based bus reset triggered a system hang.

With your additional patch posted here:
http://article.gmane.org/gmane.linux.kernel.pci/36162

Right, a reset via sysfs also triggered it with that patch, but the reset via setpci is independent of any VC save/restore and still hung your box.

Quoted 10 lines

Quoted 7 lines

I believe that
re-ordering the kernel reset mechanisms also triggered this. Since recent versions of QEMU are going to favor a bus reset over PM reset, I don't have a lot of confidence that we're actually solving the problem for you. Please make sure to test with a recent QEMU to be sure we'll do a bus reset.

I'm running qemu 2.1.0 (newest is 2.1.2 - but this shouldn't be a problem) and tested w/ linux 3.17.

Yep, just want to make sure it's QEMU new enough to do a bus reset and kernel with matching support.

Quoted 33 lines

Quoted 33 lines

drivers/pci/vc.c +19 -0

index 7e1304d..6d13d34 100644
@@ -339,6 +339,25 @@ static int pci_vc_do_save_buffer(struct pci_dev *dev, int pos,
 	return buf ? 0 : len;
 }
 
+/**
+ * pci_vc_needs_save - Determine whether a VC capability needs to be saved
+ * @dev: device
+ * @id: VC capability ID (VC/VC9/MFVC)
+ *
+ * In configurations where we have a VC or MFVC capability, but the upstream
+ * device does not, we assume that VC save (and therefore restore) is not
+ * necessary.  The intention is to only do VC save/restore in configuration
+ * where it's necessary and hopefully avoid reset issues.
+ */
+static bool pci_vc_needs_save(struct pci_dev *dev, u16 id)
+{
+	if (id == PCI_EXT_CAP_ID_VC9 || pci_is_root_bus(dev->bus) ||
+	    pci_find_ext_capability(dev->bus->self, PCI_EXT_CAP_ID_VC))
+		return true;
+
+	return false;
+}
+
 static struct {
 	u16 id;
 	const char *name;

@@ -362,7 +381,7 @@ int pci_save_vc_state(struct pci_dev *dev)
 		struct pci_cap_saved_state *save_state;
 

Quoted 6 lines

drivers/pci/vc.c int pci_save_vc_state(struct pci_dev *dev) · line 384

Quoted 3 lines

drivers/pci/vc.c +1 -1

 		pos = pci_find_ext_capability(dev, vc_caps[i].id);
-		if (!pos)
+		if (!posi || !pci_vc_needs_save(dev, vc_caps[i].id))

                        ^

This should be most probably !pos (and not !posi - because !posi does through a compile error).

Oops, sorry.

Quoted 11 lines

Quoted 11 lines

drivers/pci/vc.c +1 -1

 			continue;
 
 		save_state = pci_find_saved_ext_cap(dev, vc_caps[i].id);

@@ -422,7 +441,7 @@ void pci_allocate_vc_save_buffers(struct pci_dev *dev)
 	for (i = 0; i < ARRAY_SIZE(vc_caps); i++) {
 		int len, pos = pci_find_ext_capability(dev, vc_caps[i].id);
 
-		if (!pos)
+		if (!pos || !pci_vc_needs_save(dev, vc_caps[i].id))
 			continue;
 

Quoted 6 lines

drivers/pci/vc.c void pci_allocate_vc_save_buffers(struct pci_dev *dev) · line 447

Quoted 1 line

drivers/pci/vc.c +0 -0

 		len = pci_vc_do_save_buffer(dev, pos, NULL, false);

W/ the above patch, the machine hangs again (w/ qemu and setpci), but w/ Bjorn's patch (and nothing more applied) which disables vc save/restore, the machine just works fine ... . I especially retested this case to be really sure. I'm so sorry. But that's how it behaves here :-(

Quoted 3 lines

Hmm, the intention was that this should effectively do the same thing as Bjorn's patch. The Atheros device (03:00.0) reports a VC capability but the root port above it (00:05.0) does not.

Are you sure, that this patch really works (-> here!) as expected? Would it be possible to add some debug output printing to the actual console (not to log file) to be sure it really works as expected? Maybe some more output to get an idea what's actually going on? Or is it just a timing issue?

Quoted 109 lines · Andreas Hartmann

Quoted 104 lines · Alex Williamson

Quoted 17 lines · Andreas Hartmann

Quoted 16 lines · Alex Williamson

Quoted 6 lines · Andreas Hartmann

Out of interest:
Bjorn's patch disables vc save/restore support - and the machine works fine again. Why is it needed at all if it seems to work perfectly w/o it? What's the additional benefit? Or in other words: What am I missing until today :-) ? What would be better? What could I do more?

You're right, in the configuration you have the endpoint device has a Virtual Channel capability but the upstream root port does not. The spec is not at all clear about defining the endpoints for enabling Virtual Channel in each type of configuration, but I think that if we have an upstream port that does not support Virtual Channel, we can skip the save/restore. Please test the patch below.

Quoted 7 lines

Quoted 4 lines

I'm also still completely confused about whether this is a VC save/restore issue or a bus reset issue. You originally bisected this back to the VC save/restore patch, but you also found that a manual, setpci-based bus reset triggered a system hang.

With your additional patch posted here:
http://article.gmane.org/gmane.linux.kernel.pci/36162

Right, a reset via sysfs also triggered it with that patch, but the reset via setpci is independent of any VC save/restore and still hung your box.

Quoted 10 lines

Quoted 7 lines

I believe that
re-ordering the kernel reset mechanisms also triggered this. Since recent versions of QEMU are going to favor a bus reset over PM reset, I don't have a lot of confidence that we're actually solving the problem for you. Please make sure to test with a recent QEMU to be sure we'll do a bus reset.

I'm running qemu 2.1.0 (newest is 2.1.2 - but this shouldn't be a problem) and tested w/ linux 3.17.

Yep, just want to make sure it's QEMU new enough to do a bus reset and kernel with matching support.

Quoted 33 lines

Quoted 33 lines

drivers/pci/vc.c +19 -0

index 7e1304d..6d13d34 100644
@@ -339,6 +339,25 @@ static int pci_vc_do_save_buffer(struct pci_dev *dev, int pos,
 	return buf ? 0 : len;
 }
 
+/**
+ * pci_vc_needs_save - Determine whether a VC capability needs to be saved
+ * @dev: device
+ * @id: VC capability ID (VC/VC9/MFVC)
+ *
+ * In configurations where we have a VC or MFVC capability, but the upstream
+ * device does not, we assume that VC save (and therefore restore) is not
+ * necessary.  The intention is to only do VC save/restore in configuration
+ * where it's necessary and hopefully avoid reset issues.
+ */
+static bool pci_vc_needs_save(struct pci_dev *dev, u16 id)
+{
+	if (id == PCI_EXT_CAP_ID_VC9 || pci_is_root_bus(dev->bus) ||
+	    pci_find_ext_capability(dev->bus->self, PCI_EXT_CAP_ID_VC))
+		return true;
+
+	return false;
+}
+
 static struct {
 	u16 id;
 	const char *name;

@@ -362,7 +381,7 @@ int pci_save_vc_state(struct pci_dev *dev)
 		struct pci_cap_saved_state *save_state;
 

Quoted 6 lines

drivers/pci/vc.c int pci_save_vc_state(struct pci_dev *dev)

Quoted 3 lines

drivers/pci/vc.c +1 -1

 		pos = pci_find_ext_capability(dev, vc_caps[i].id);
-		if (!pos)
+		if (!posi || !pci_vc_needs_save(dev, vc_caps[i].id))

                        ^

This should be most probably !pos (and not !posi - because !posi does through a compile error).

Oops, sorry.

Quoted 11 lines

Quoted 11 lines

drivers/pci/vc.c +1 -1

 			continue;
 
 		save_state = pci_find_saved_ext_cap(dev, vc_caps[i].id);

@@ -422,7 +441,7 @@ void pci_allocate_vc_save_buffers(struct pci_dev *dev)
 	for (i = 0; i < ARRAY_SIZE(vc_caps); i++) {
 		int len, pos = pci_find_ext_capability(dev, vc_caps[i].id);
 
-		if (!pos)
+		if (!pos || !pci_vc_needs_save(dev, vc_caps[i].id))
 			continue;
 

Quoted 6 lines

drivers/pci/vc.c void pci_allocate_vc_save_buffers(struct pci_dev *dev) · line 450

Quoted 1 line

drivers/pci/vc.c +0 -0

 		len = pci_vc_do_save_buffer(dev, pos, NULL, false);

W/ the above patch, the machine hangs again (w/ qemu and setpci), but w/ Bjorn's patch (and nothing more applied) which disables vc save/restore, the machine just works fine ... . I especially retested this case to be really sure. I'm so sorry. But that's how it behaves here :-(

Quoted 3 lines

Hmm, the intention was that this should effectively do the same thing as Bjorn's patch. The Atheros device (03:00.0) reports a VC capability but the root port above it (00:05.0) does not.

Quoted 5 lines

Are you sure, that this patch really works (-> here!) as expected? Would it be possible to add some debug output printing to the actual console (not to log file) to be sure it really works as expected? Maybe some more output to get an idea what's actually going on? Or is it just a timing issue?

Sure, here's some added printks (and fixed posi). You should be able to run 'dmesg | grep pci_vc_needs_save' after boot and see device 0000:03:00.0. Hopefully you won't see the pci_save_vc_state() printk as you assign the device.

Quoted 116 lines · Alex Williamson

Quoted 109 lines · Andreas Hartmann

Quoted 104 lines · Alex Williamson

Quoted 17 lines · Andreas Hartmann

Quoted 16 lines · Alex Williamson

Quoted 6 lines · Andreas Hartmann

Out of interest:
Bjorn's patch disables vc save/restore support - and the machine works fine again. Why is it needed at all if it seems to work perfectly w/o it? What's the additional benefit? Or in other words: What am I missing until today :-) ? What would be better? What could I do more?

You're right, in the configuration you have the endpoint device has a Virtual Channel capability but the upstream root port does not. The spec is not at all clear about defining the endpoints for enabling Virtual Channel in each type of configuration, but I think that if we have an upstream port that does not support Virtual Channel, we can skip the save/restore. Please test the patch below.

Quoted 7 lines

Quoted 4 lines

I'm also still completely confused about whether this is a VC save/restore issue or a bus reset issue. You originally bisected this back to the VC save/restore patch, but you also found that a manual, setpci-based bus reset triggered a system hang.

With your additional patch posted here:
http://article.gmane.org/gmane.linux.kernel.pci/36162

Right, a reset via sysfs also triggered it with that patch, but the reset via setpci is independent of any VC save/restore and still hung your box.

Quoted 10 lines

Quoted 7 lines

I believe that
re-ordering the kernel reset mechanisms also triggered this. Since recent versions of QEMU are going to favor a bus reset over PM reset, I don't have a lot of confidence that we're actually solving the problem for you. Please make sure to test with a recent QEMU to be sure we'll do a bus reset.

I'm running qemu 2.1.0 (newest is 2.1.2 - but this shouldn't be a problem) and tested w/ linux 3.17.

Yep, just want to make sure it's QEMU new enough to do a bus reset and kernel with matching support.

Quoted 33 lines

Quoted 33 lines

drivers/pci/vc.c +19 -0

index 7e1304d..6d13d34 100644
@@ -339,6 +339,25 @@ static int pci_vc_do_save_buffer(struct pci_dev *dev, int pos,
 	return buf ? 0 : len;
 }
 
+/**
+ * pci_vc_needs_save - Determine whether a VC capability needs to be saved
+ * @dev: device
+ * @id: VC capability ID (VC/VC9/MFVC)
+ *
+ * In configurations where we have a VC or MFVC capability, but the upstream
+ * device does not, we assume that VC save (and therefore restore) is not
+ * necessary.  The intention is to only do VC save/restore in configuration
+ * where it's necessary and hopefully avoid reset issues.
+ */
+static bool pci_vc_needs_save(struct pci_dev *dev, u16 id)
+{
+	if (id == PCI_EXT_CAP_ID_VC9 || pci_is_root_bus(dev->bus) ||
+	    pci_find_ext_capability(dev->bus->self, PCI_EXT_CAP_ID_VC))
+		return true;
+
+	return false;
+}
+
 static struct {
 	u16 id;
 	const char *name;

@@ -362,7 +381,7 @@ int pci_save_vc_state(struct pci_dev *dev)
 		struct pci_cap_saved_state *save_state;
 

Quoted 6 lines

drivers/pci/vc.c int pci_save_vc_state(struct pci_dev *dev)

Quoted 3 lines

drivers/pci/vc.c +1 -1

 		pos = pci_find_ext_capability(dev, vc_caps[i].id);
-		if (!pos)
+		if (!posi || !pci_vc_needs_save(dev, vc_caps[i].id))

                        ^

This should be most probably !pos (and not !posi - because !posi does through a compile error).

Oops, sorry.

Quoted 11 lines

Quoted 11 lines

drivers/pci/vc.c +1 -1

 			continue;
 
 		save_state = pci_find_saved_ext_cap(dev, vc_caps[i].id);

@@ -422,7 +441,7 @@ void pci_allocate_vc_save_buffers(struct pci_dev *dev)
 	for (i = 0; i < ARRAY_SIZE(vc_caps); i++) {
 		int len, pos = pci_find_ext_capability(dev, vc_caps[i].id);
 
-		if (!pos)
+		if (!pos || !pci_vc_needs_save(dev, vc_caps[i].id))
 			continue;
 

Quoted 6 lines

drivers/pci/vc.c void pci_allocate_vc_save_buffers(struct pci_dev *dev) · line 450

Quoted 1 line

drivers/pci/vc.c +0 -0

 		len = pci_vc_do_save_buffer(dev, pos, NULL, false);

W/ the above patch, the machine hangs again (w/ qemu and setpci), but w/ Bjorn's patch (and nothing more applied) which disables vc save/restore, the machine just works fine ... . I especially retested this case to be really sure. I'm so sorry. But that's how it behaves here :-(

Quoted 3 lines

Hmm, the intention was that this should effectively do the same thing as Bjorn's patch. The Atheros device (03:00.0) reports a VC capability but the root port above it (00:05.0) does not.

Quoted 5 lines

Are you sure, that this patch really works (-> here!) as expected? Would it be possible to add some debug output printing to the actual console (not to log file) to be sure it really works as expected? Maybe some more output to get an idea what's actually going on? Or is it just a timing issue?

Quoted 4 lines

Sure, here's some added printks (and fixed posi). You should be able to run 'dmesg | grep pci_vc_needs_save' after boot and see device 0000:03:00.0. Hopefully you won't see the pci_save_vc_state() printk as you assign the device.

[...]

I'm getting the expected output:

[    1.156857] pci_vc_needs_save(0000:03:00.0, 2) returning false
[    1.158866] pci_vc_needs_save(0000:04:00.0, 2) returning false

This is most probably triggered by pci_allocate_vc_save_buffers, true?

Therefore, I never should need pci_save_vc_state and pci_restore_vc_state. Thus, it should be ok to add "return" at the beginning of each of these function, true? Then it should work.

I tested it. It worked.

But if I'm removing only one of these returns either in pci_save_vc_state or pci_restore_vc_state, the machine hangs again.

Therefore, there must be something odd going on in the for loops. Isn't it possible to add some useful debug code to these loops to see what's really going on? But the output *must* go to the actual console, otherwise I can't see it!

int pci_save_vc_state(struct pci_dev *dev)

{
        return 0; // must be set
        int i;

        for (i = 0; i < ARRAY_SIZE(vc_caps); i++) {
                int pos, ret;
                struct pci_cap_saved_state *save_state;

                pos = pci_find_ext_capability(dev, vc_caps[i].id);
                if (!pos || !pci_vc_needs_save(dev, vc_caps[i].id))
                        continue;

                save_state = pci_find_saved_ext_cap(dev, vc_caps[i].id);

                if (!save_state) {
                        dev_err(&dev->dev, "%s buffer not found in %s\n",
                                vc_caps[i].name, __func__);
                        return -ENOMEM;
                }

                printk("%s doing %s save on %s\n", __func__, vc_caps[i].name, pci_name(dev));
                ret = pci_vc_do_save_buffer(dev, pos, save_state, true);
                if (ret) {
                        dev_err(&dev->dev, "%s save unsuccessful %s\n",
                                vc_caps[i].name, __func__);
                        return ret;
                }
        }

        return 0;
}

void pci_restore_vc_state(struct pci_dev *dev)

{
        return; // must be set
        int i;

        for (i = 0; i < ARRAY_SIZE(vc_caps); i++) {
                int pos;
                struct pci_cap_saved_state *save_state;

                pos = pci_find_ext_capability(dev, vc_caps[i].id);
                save_state = pci_find_saved_ext_cap(dev, vc_caps[i].id);

                if (!save_state || !pos)
                        continue;

                printk("%s doing %s restore on %s\n", __func__, vc_caps[i].name, pci_name(dev));
                pci_vc_do_save_buffer(dev, pos, save_state, false);
        }
}

In the "working" case with Bjorn's patch, are you actually trying to use the device or just testing to see if the system survives reset?

On Wed, 2014-10-29 at 20:43 +0100, Andreas Hartmann wrote:

Quoted 15 lines

Therefore, I never should need pci_save_vc_state and pci_restore_vc_state. Thus, it should be ok to add "return" at the beginning of each of these function, true? Then it should work.

I tested it. It worked.

But if I'm removing only one of these returns either in pci_save_vc_state or pci_restore_vc_state, the machine hangs again.

Therefore, there must be something odd going on in the for loops. Isn't it possible to add some useful debug code to these loops to see what's really going on? But the output *must* go to the actual console, otherwise I can't see it!

Quoted 6 lines

int pci_save_vc_state(struct pci_dev *dev)

{
        return 0; // must be set
        int i;

        for (i = 0; i < ARRAY_SIZE(vc_caps); i++) {

Quoted 2 lines

                int pos, ret;
                struct pci_cap_saved_state *save_state;

Quoted 4 lines

                pos = pci_find_ext_capability(dev, vc_caps[i].id);
                if (!pos || !pci_vc_needs_save(dev, vc_caps[i].id))
                        continue;

Take the next logical step, comment out the if here and we'll statically take the continue. Does it still fail? If so, move the continue above the call to pci_find_ext_capability(), if not...

Quoted 2 lines

                save_state = pci_find_saved_ext_cap(dev, vc_caps[i].id);

If not, add a continue; here. Unless my pci_vc_needs_save() function is broken, we shouldn't be getting here anyway.

Quoted 20 lines

                if (!save_state) {
                        dev_err(&dev->dev, "%s buffer not found in %s\n",
                                vc_caps[i].name, __func__);
                        return -ENOMEM;
                }

                printk("%s doing %s save on %s\n", __func__, vc_caps[i].name, pci_name(dev));
                ret = pci_vc_do_save_buffer(dev, pos, save_state, true);
                if (ret) {
                        dev_err(&dev->dev, "%s save unsuccessful %s\n",
                                vc_caps[i].name, __func__);
                        return ret;
                }
        }

        return 0;
}

void pci_restore_vc_state(struct pci_dev *dev)

Quoted 10 lines

{
        return; // must be set
        int i;

        for (i = 0; i < ARRAY_SIZE(vc_caps); i++) {
                int pos;
                struct pci_cap_saved_state *save_state;

                pos = pci_find_ext_capability(dev, vc_caps[i].id);
                save_state = pci_find_saved_ext_cap(dev, vc_caps[i].id);

This should never find a save_state with the pci_vc_needs_save() patch, so we should always take the branch below. Comment out the if (... and leave the continue, does the behavior change? If so, add a continue; line above pci_find_saved_ext_cap(), does it work? If not, add another continue above pci_find_ext_capability().

Quoted 7 lines

                if (!save_state || !pos)
                        continue;

                printk("%s doing %s restore on %s\n", __func__, vc_caps[i].name, pci_name(dev));
                pci_vc_do_save_buffer(dev, pos, save_state, false);
        }
}

In the "working" case with Bjorn's patch, are you actually trying to use the device or just testing to see if the system survives reset? You might at least want to run lspci -xxxx on it after reset to make sure it's really there. Thanks,

Alex

Quoted 1 line · Alex Williamson

On Wed, 2014-10-29 at 20:43 +0100, Andreas Hartmann wrote:

[...]

Quoted 15 lines

Quoted 15 lines

Therefore, I never should need pci_save_vc_state and pci_restore_vc_state. Thus, it should be ok to add "return" at the beginning of each of these function, true? Then it should work.

I tested it. It worked.

But if I'm removing only one of these returns either in pci_save_vc_state or pci_restore_vc_state, the machine hangs again.

Therefore, there must be something odd going on in the for loops. Isn't it possible to add some useful debug code to these loops to see what's really going on? But the output *must* go to the actual console, otherwise I can't see it!

Quoted 6 lines

Quoted 6 lines

int pci_save_vc_state(struct pci_dev *dev)

{
        return 0; // must be set
        int i;

        for (i = 0; i < ARRAY_SIZE(vc_caps); i++) {

		   // continue; -> works

Quoted 2 lines

Quoted 2 lines

                int pos, ret;
                struct pci_cap_saved_state *save_state;

		   // continue does not work!

--> Most probably the

struct pci_cap_saved_state *save_state;

makes the system hang!

ARRAY_SIZE(vc_caps) is 3 and the whole
function is called 3 times when starting the vm.

Quoted 60 lines

Quoted 4 lines

                pos = pci_find_ext_capability(dev, vc_caps[i].id);
                if (!pos || !pci_vc_needs_save(dev, vc_caps[i].id))
                        continue;

Take the next logical step, comment out the if here and we'll statically take the continue. Does it still fail? If so, move the continue above the call to pci_find_ext_capability(), if not...

Quoted 2 lines

                save_state = pci_find_saved_ext_cap(dev, vc_caps[i].id);

If not, add a continue; here. Unless my pci_vc_needs_save() function is broken, we shouldn't be getting here anyway.

Quoted 20 lines

                if (!save_state) {
                        dev_err(&dev->dev, "%s buffer not found in %s\n",
                                vc_caps[i].name, __func__);
                        return -ENOMEM;
                }

                printk("%s doing %s save on %s\n", __func__, vc_caps[i].name, pci_name(dev));
                ret = pci_vc_do_save_buffer(dev, pos, save_state, true);
                if (ret) {
                        dev_err(&dev->dev, "%s save unsuccessful %s\n",
                                vc_caps[i].name, __func__);
                        return ret;
                }
        }

        return 0;
}

void pci_restore_vc_state(struct pci_dev *dev)

Quoted 10 lines

{
        return; // must be set
        int i;

        for (i = 0; i < ARRAY_SIZE(vc_caps); i++) {
                int pos;
                struct pci_cap_saved_state *save_state;

                pos = pci_find_ext_capability(dev, vc_caps[i].id);
                save_state = pci_find_saved_ext_cap(dev, vc_caps[i].id);

This should never find a save_state with the pci_vc_needs_save() patch, so we should always take the branch below. Comment out the if (... and leave the continue, does the behavior change? If so, add a continue; line above pci_find_saved_ext_cap(), does it work? If not, add another continue above pci_find_ext_capability().

Quoted 7 lines

                if (!save_state || !pos)
                        continue;

                printk("%s doing %s restore on %s\n", __func__, vc_caps[i].name, pci_name(dev));
                pci_vc_do_save_buffer(dev, pos, save_state, false);
        }
}

Quoted 7 lines

In the "working" case with Bjorn's patch, are you actually trying to use the device or just testing to see if the system survives reset? You might at least want to run lspci -xxxx on it after reset to make sure it's really there. Thanks,

Alex

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 31 lines · Andreas Hartmann

Quoted 1 line · Alex Williamson

On Wed, 2014-10-29 at 20:43 +0100, Andreas Hartmann wrote:

[...]

Quoted 15 lines

Quoted 15 lines

Therefore, I never should need pci_save_vc_state and pci_restore_vc_state. Thus, it should be ok to add "return" at the beginning of each of these function, true? Then it should work.

I tested it. It worked.

But if I'm removing only one of these returns either in pci_save_vc_state or pci_restore_vc_state, the machine hangs again.

Therefore, there must be something odd going on in the for loops. Isn't it possible to add some useful debug code to these loops to see what's really going on? But the output *must* go to the actual console, otherwise I can't see it!

Quoted 6 lines

Quoted 6 lines

int pci_save_vc_state(struct pci_dev *dev)

{
        return 0; // must be set
        int i;

        for (i = 0; i < ARRAY_SIZE(vc_caps); i++) {

		   // continue; -> works

Quoted 2 lines

Quoted 2 lines

                int pos, ret;
                struct pci_cap_saved_state *save_state;

		   // continue does not work!

--> Most probably the

Quoted 3 lines

struct pci_cap_saved_state *save_state;

makes the system hang!

We've done nothing more than declare variables there, there's no actual code. What happens if you increase the delay after bus reset, edit drivers/pci/pci.c, find the call to ssleep(1) and change the 1 to a 2, doubling the delay after reset.

It seems like VC save/restore is just a
scapegoat for the platform already being broken by the bus reset. Also, if you have any other card to test in this slot, it would be useful comparison data to know if we're dealing with an endpoint issue or a bus issue.

Quoted 32 lines · Alex Williamson

Quoted 31 lines · Andreas Hartmann

Quoted 1 line · Alex Williamson

On Wed, 2014-10-29 at 20:43 +0100, Andreas Hartmann wrote:

[...]

Quoted 15 lines

Quoted 15 lines

Therefore, I never should need pci_save_vc_state and pci_restore_vc_state. Thus, it should be ok to add "return" at the beginning of each of these function, true? Then it should work.

I tested it. It worked.

But if I'm removing only one of these returns either in pci_save_vc_state or pci_restore_vc_state, the machine hangs again.

Therefore, there must be something odd going on in the for loops. Isn't it possible to add some useful debug code to these loops to see what's really going on? But the output *must* go to the actual console, otherwise I can't see it!

Quoted 6 lines

Quoted 6 lines

int pci_save_vc_state(struct pci_dev *dev)

{
        return 0; // must be set
        int i;

        for (i = 0; i < ARRAY_SIZE(vc_caps); i++) {

		   // continue; -> works

Quoted 2 lines

Quoted 2 lines

                int pos, ret;
                struct pci_cap_saved_state *save_state;

		   // continue does not work!

--> Most probably the

Quoted 8 lines

Quoted 3 lines

struct pci_cap_saved_state *save_state;

makes the system hang!

We've done nothing more than declare variables there, there's no actual code. What happens if you increase the delay after bus reset, edit drivers/pci/pci.c, find the call to ssleep(1) and change the 1 to a 2, doubling the delay after reset.

Same behaviour.

Quoted 5 lines

It seems like VC save/restore is just a
scapegoat for the platform already being broken by the bus reset. Also, if you have any other card to test in this slot, it would be useful comparison data to know if we're dealing with an endpoint issue or a bus issue.

I organized an Intel pcie card:

03:00.0 Ethernet controller: Intel Corporation 82574L Gigabit Network Connection
Subsystem: Intel Corporation Gigabit CT Desktop Adapter
Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
Interrupt: pin A routed to IRQ 17
Region 0: Memory at fdbc0000 (32-bit, non-prefetchable) [disabled] [size=128K]
Region 1: Memory at fdb00000 (32-bit, non-prefetchable) [disabled] [size=512K]
Region 2: I/O ports at cf00 [disabled] [size=32]
Region 3: Memory at fdbfc000 (32-bit, non-prefetchable) [disabled] [size=16K]
[virtual] Expansion ROM at fdb80000 [disabled] [size=256K]
Capabilities: [c8] Power Management version 2
Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold+)
Status: D0 NoSoftRst- PME-Enable+ DSel=0 DScale=1 PME-
Capabilities: [d0] MSI: Enable- Count=1/1 Maskable- 64bit+
Address: 0000000000000000 Data: 0000
Capabilities: [e0] Express (v1) Endpoint, MSI 00
DevCap: MaxPayload 256 bytes, PhantFunc 0, Latency L0s <512ns, L1 <64us
ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset-
DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
MaxPayload 128 bytes, MaxReadReq 512 bytes
DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr+ TransPend-
LnkCap: Port #1, Speed 2.5GT/s, Width x1, ASPM L0s L1, Latency L0 <128ns, L1 <64us
ClockPM- Surprise- LLActRep- BwNot-
LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- Retrain- CommClk+
ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt-
Capabilities: [a0] MSI-X: Enable- Count=5 Masked-
Vector table: BAR=3 offset=00000000
PBA: BAR=3 offset=00002000
Capabilities: [100 v1] Advanced Error Reporting
UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
UESvrt: DLP+ SDES- TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol-
CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+
CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+
AERCap: First Error Pointer: 00, GenCap- CGenEn- ChkCap- ChkEn-
Capabilities: [140 v1] Device Serial Number 00-1b-21-ff-ff-cf-8f-57
Kernel driver in use: vfio-pci

and tested with the same kernel, which hangs w/ atheros card. It just worked. Not just once, but each of the tests I did. I retested w/ atheros -> hang. Tested again with intel-card -> works. Back to atheros -> hang.

Seems to be really a problem w/ the atheros card, which is triggered by new vc save/restore.

Well, but what to do now? I know how to "fix" it. But this means I have to compile my kernels again on my own if it is >= 3.14.

On Thu, 2014-10-30 at 20:09 +0100, Andreas Hartmann wrote:

Quoted 2 lines

Well, but what to do now? I know how to "fix" it. But this means I have to compile my kernels again on my own if it is >= 3.14.

Let's not give up hope just yet, I'd like to try another bus reset mechanism with setpci. Install the Atheros card and bind it to pci-stub, then do:

setpci -s 00:05.0 68.w=0010:0010
sleep 0.1
setpci -s 00:05.0 68.w=0000:0010
sleep 1
lspci -xxx -s 3:00.0