Page MenuHomeFreeBSD

ioat(4): pci_save/restore_state to persist MSI-X registers over BDXDE reset
ClosedPublic

Authored by cem on Sep 2 2015, 7:38 PM.

Details

Summary

As suggested by jhb@.

Test Plan

I will give it a reboot and see if interrupts work :).

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

cem retitled this revision from to ioat(4): pci_save/restore_state to persist MSI-X registers over BDXDE reset.
cem updated this object.
cem edited the test plan for this revision. (Show Details)
cem added reviewers: markj, jimharris, jhb.

First make sure it works. :) If it does, you might just want to do the save/restore unconditionally rather than only for these parts as it wouldn't surprise me if other devices might reset other PCI registers on reset? I'm happy to defer to more well-informed comments on how isolated this is to the specific model (BDXDE) however.

In D3552#73088, @jhb wrote:

First make sure it works. :)

I've tested it now. It does appear to work.

If it does, you might just want to do the save/restore unconditionally rather than only for these parts as it wouldn't surprise me if other devices might reset other PCI registers on reset?

The word from Intel is that only these PCIIDs do it, and they intend to fix the behavior in future hardware revisions.

I'm happy to defer to more well-informed comments on how isolated this is to the specific model (BDXDE) however.

From Jim Harris,

BDX-DE CB-DMA is based on a different underlying DMA unit than the ones in the mainline Xeon processors. This DMA unit effectively does a PCI function level reset when you reset the DMA unit. This means you have to reset the IRQs after the reset operation. It is not ideal, but the Linux driver has a workaround routine that checks for this type of CB-DMA and does the reinitialization if necessary.

From Dave Jiang at Intel (Linux ioat maintainer),

Yep this sounds like what it's doing. The Linux driver re-init the MSIX interrupts after a chan reset to workaround the issue. You can key off the PCI dev id to only do that for those devices on BDX-DE. We are hoping that will be fixed next gen. This is not an issue for the CBDMA v3.2 devices that are on Xeon EP CPUs. So far all affected devices are CBDMA v3.3 based.

ngie added inline comments.
sys/dev/ioat/ioat.c
989–990 ↗(On Diff #8431)

Would this case need to be handled too?

jhb edited edge metadata.

Seems good to me then, thanks!

Regarding @ngie's question, I'm not sure what the right thing to do is if the reset fails. In theory it doesn't hurt to do a restore as it should be a no-op if nothing happened, but it seems dubious to be messing with the hardware at all if it fails to reset.

This revision is now accepted and ready to land.Sep 2 2015, 9:20 PM
markj edited edge metadata.

Technically the BWD (stands for Briarwood - an Atom-based SOC) dev IDs should be treated the same as BDX (Broadwell) DE (aka Xeon D). I have no Briarwood hardware to test on though. But should not hurt to just add those device IDs anyways.

Technically the BWD (stands for Briarwood - an Atom-based SOC) dev IDs should be treated the same as BDX (Broadwell) DE (aka Xeon D). I have no Briarwood hardware to test on though. But should not hurt to just add those device IDs anyways.

I'll throw those in, thanks.

This revision was automatically updated to reflect the committed changes.