Differential D59186

xhci: Only reset the data toggle value when the USB stack asks for it
ClosedPublic
Actions

Authored by aokblast on Tue, Aug 25, 7:24 PM.

Details

Reviewers

kevans

Group Reviewers

USB

Commits

rG0f59df83869d: xhci: Only reset the data toggle value when the USB stack asks for it

Summary

The previous patch assumes that we don't want to reset toggle bit in
STOPPED_STEP. However, a device can explicitly call
usbd_clear_data_toggle if necessary. As a result, instead of not
dropping the bit unconditionally, we added a field in xhci to specify
that we want to drop it, so that usbd_clear_data_toggle can handle it
correctly.

Fixes: 28d85db46b48 ("xhci: Do not drop and add bits in xhci")
MFC after: 2 weeks

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 76161
Build 73044: arc lint + arc unit

Event Timeline

aokblast created this revision.Tue, Aug 25, 7:24 PM

Herald added a subscriber: imp. · View Herald TranscriptTue, Aug 25, 7:24 PM

aokblast requested review of this revision.Tue, Aug 25, 7:24 PM

Harbormaster completed remote builds in B76161: Diff 185100.Tue, Aug 25, 7:24 PM

@kevans Could you please check if there is any regression on your yubikey? Thanks!

aokblast added a reviewer: USB.Tue, Aug 25, 7:29 PM

In D59186#1357635, @aokblast wrote:

@kevans Could you please check if there is any regression on your yubikey? Thanks!

Sure thing, I am working on getting this patch to my two test machines now.

oh added a subscriber: oh.Wed, Aug 26, 11:00 AM

The USB key mentioned in D57146 does work with this patch on stable/15 (132e609c2ce5c72e9e04a5d6f3e0e9140e06ebd8)
Thank you.

This LGTM and also tests fine with my Yubikey. My test setup is naive, but just to document it:

kevans@rex:~/.ssh$ while true; do env -i ssh -i id_yubikey_sk 10.9.0.1 date;  done   
Wed Aug 26 08:20:37 CDT 2026
Wed Aug 26 08:20:38 CDT 2026
Wed Aug 26 08:20:39 CDT 2026
Wed Aug 26 08:20:40 CDT 2026
[...]

This reliably locks up after either two or three iterations if we get this wrong. I run this test with both a Yubikey and a Solo key, and on both my frame.work laptop and a Minisforum unit that have demonstrated problems with the test due to two separate issues.

This revision is now accepted and ready to land.Wed, Aug 26, 1:25 PM

CC releng because this should go into 14.5, IMO, which might mean an expedited MFC. I think this is reasonably safe.

In D59186#1357916, @kevans wrote:

CC releng because this should go into 14.5, IMO, which might mean an expedited MFC. I think this is reasonably safe.

Thanks for helping me test this! The previous problematic device is left in Taiwan and I don't have it now.
I set MFC after to 3 days after. Hope everything works well.

Closed by commit rG0f59df83869d: xhci: Only reset the data toggle value when the USB stack asks for it (authored by aokblast). · Explain WhyWed, Aug 26, 8:52 PM

This revision was automatically updated to reflect the committed changes.

aokblast added a commit: rG0f59df83869d: xhci: Only reset the data toggle value when the USB stack asks for it.