Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

e1000: fix I219 hang on reset #540

Closed
wants to merge 1 commit into from
Closed

Conversation

kev009
Copy link
Member

@kev009 kev009 commented Sep 23, 2021

Clear the rings before reset to avoid a HW hang.

Inspired by em-7.7.8 and DPDK (1fc9701238edcf0541289b9ae15565b6d9d7ab30)

@ricera

tctl = E1000_READ_REG(hw, E1000_TCTL);
E1000_WRITE_REG(hw, E1000_TCTL, tctl | E1000_TCTL_EN);

txd = &txr->tx_base[txr->tx_cidx_processed];
Copy link
Member Author

@kev009 kev009 Sep 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure this index is correct, although it seems like we just need a scratch descriptor prior to the head.

txd->upper.data = 0;

/* flush descriptors to memory before notifying the HW */
wmb();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These membars are from the OOT driver and are macro'd out to nothing in e1000_osdep.h,

@kev009 kev009 requested a review from bsdimp September 23, 2021 18:31
@@ -37,7 +37,8 @@
/*********************************************************************
* Driver version:
*********************************************************************/
char em_driver_version[] = "7.6.1-k";
char em_driver_version[] = "7.7.8-fbsd";
char igb_driver_version[] = "2.5.19-fbsd";
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incrementing these to avoid confusion in users; we are roughly on par with these OOT versions.

Clear the rings before reset to avoid a HW hang.

Inspired by em-7.7.8 and DPDK (1fc9701238edcf0541289b9ae15565b6d9d7ab30)
@bsdimp
Copy link
Member

bsdimp commented Feb 5, 2023

what's the status of this pull request? IT looks like it's stalled and hasn't landed in main yet.

@kev009
Copy link
Member Author

kev009 commented Feb 8, 2023

@bsdimp it needs review, hopefully from intel

Copy link
Contributor

@ricera ricera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it's alright? It looks like the workaround code has been in the OOT driver since 2015, but somehow didn't get upstreamed, or got removed?

@@ -588,7 +590,7 @@ static struct if_shared_ctx igb_sctx_init = {
.isc_ntxqs = 1,
.isc_admin_intrcnt = 1,
.isc_vendor_info = igb_vendor_info_array,
.isc_driver_version = em_driver_version,
.isc_driver_version = igb_driver_version,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's a good catch

@@ -1907,6 +1909,10 @@ em_if_stop(if_ctx_t ctx)

INIT_DEBUGOUT("em_if_stop: begin");

/* I219 needs special flushing to avoid hangs */
if (sc->hw.mac.type >= e1000_pch_spt && sc->hw.mac.type < igb_mac_min)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking here: https://git.dpdk.org/dpdk/tree/drivers/net/e1000/em_ethdev.c#n728, this check was updated to apply this workaround to cnp in addition to spt, but I see that the device ID list only goes up to (7) for DPDK, and not up to the current (23) that we're up to, so maybe this check doesn't need to change and we should apply it to everything spt and newer.

@kev009
Copy link
Member Author

kev009 commented Feb 8, 2023

Merged as ae1dca7

@kev009 kev009 closed this Feb 8, 2023
@kev009 kev009 deleted the i219-flush-desc branch February 8, 2023 19:31
freebsd-git pushed a commit that referenced this pull request Feb 8, 2023
Clear the rings before reset to avoid a HW hang.

Inspired by em-7.7.8 and DPDK (1fc9701238edcf0541289b9ae15565b6d9d7ab30)

Reviewed by:	erj
MFC after:	2 weeks
Sponsored by:	BBOX.io
Pull Request:	#540
freebsd-git pushed a commit that referenced this pull request Feb 8, 2023
Incrementing these to avoid confusion in users; we are on par with these
out of tree versions.

Reviewed by:	erj
MFC after:	2 weeks
Sponsored by:	BBOX.io
Pull Request:	#540
@bsdimp
Copy link
Member

bsdimp commented Feb 8, 2023

thanks @kev009 for closing these

hardenedbsd-services pushed a commit to HardenedBSD/hardenedBSD that referenced this pull request Feb 10, 2023
Pull a part the locking from the static range. This enables
locking to be added to a range directly, and does not require
it to be made static.  This is useful in cases where the source
of memory is shared between threads, but not static.  I.e. there
are multiple instances of the same type.
freebsd-git pushed a commit that referenced this pull request Feb 22, 2023
Clear the rings before reset to avoid a HW hang.

Inspired by em-7.7.8 and DPDK (1fc9701238edcf0541289b9ae15565b6d9d7ab30)

Reviewed by:	erj
Sponsored by:	BBOX.io
Pull Request:	#540

(cherry picked from commit ae1dca7)
freebsd-git pushed a commit that referenced this pull request Feb 22, 2023
Clear the rings before reset to avoid a HW hang.

Inspired by em-7.7.8 and DPDK (1fc9701238edcf0541289b9ae15565b6d9d7ab30)

Reviewed by:	erj
Sponsored by:	BBOX.io
Pull Request:	#540

(cherry picked from commit ae1dca7)
freebsd-git pushed a commit that referenced this pull request Feb 22, 2023
Incrementing these to avoid confusion in users; we are on par with these
out of tree versions.

Reviewed by:	erj
Sponsored by:	BBOX.io
Pull Request:	#540

(cherry picked from commit 647f2d2)
freebsd-git pushed a commit that referenced this pull request Feb 22, 2023
Incrementing these to avoid confusion in users; we are on par with these
out of tree versions.

Reviewed by:	erj
Sponsored by:	BBOX.io
Pull Request:	#540

(cherry picked from commit 647f2d2)
bsdjhb pushed a commit to bsdjhb/cheribsd that referenced this pull request Mar 22, 2023
Clear the rings before reset to avoid a HW hang.

Inspired by em-7.7.8 and DPDK (1fc9701238edcf0541289b9ae15565b6d9d7ab30)

Reviewed by:	erj
MFC after:	2 weeks
Sponsored by:	BBOX.io
Pull Request:	freebsd/freebsd-src#540
bsdjhb pushed a commit to bsdjhb/cheribsd that referenced this pull request Mar 22, 2023
Incrementing these to avoid confusion in users; we are on par with these
out of tree versions.

Reviewed by:	erj
MFC after:	2 weeks
Sponsored by:	BBOX.io
Pull Request:	freebsd/freebsd-src#540
fichtner pushed a commit to opnsense/src that referenced this pull request May 9, 2023
Clear the rings before reset to avoid a HW hang.

Inspired by em-7.7.8 and DPDK (1fc9701238edcf0541289b9ae15565b6d9d7ab30)

Reviewed by:	erj
Sponsored by:	BBOX.io
Pull Request:	freebsd/freebsd-src#540

(cherry picked from commit ae1dca7)
fichtner pushed a commit to opnsense/src that referenced this pull request May 9, 2023
Incrementing these to avoid confusion in users; we are on par with these
out of tree versions.

Reviewed by:	erj
Sponsored by:	BBOX.io
Pull Request:	freebsd/freebsd-src#540

(cherry picked from commit 647f2d2)
@emaste emaste added the merged label Jun 12, 2023
laffer1 pushed a commit to MidnightBSD/src that referenced this pull request Jun 27, 2023
Clear the rings before reset to avoid a HW hang.

Inspired by em-7.7.8 and DPDK (1fc9701238edcf0541289b9ae15565b6d9d7ab30)

Reviewed by:	erj
Sponsored by:	BBOX.io
Pull Request:	freebsd/freebsd-src#540

(cherry picked from commit ae1dca798e0f826de46f4ec11914ba4c91928d7a)
laffer1 pushed a commit to MidnightBSD/src that referenced this pull request Jun 27, 2023
Incrementing these to avoid confusion in users; we are on par with these
out of tree versions.

Reviewed by:	erj
Sponsored by:	BBOX.io
Pull Request:	freebsd/freebsd-src#540

(cherry picked from commit 647f2d2bc0cb9357ac083bf2aae4b669167dd66b)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants