Page MenuHomeFreeBSD
Differential D51735

bhyve: introduce cancel interface for usb device
Needs ReviewPublic
Actions

Authored by kevans on Aug 5 2025, 9:37 AM.
Tags
None
Referenced Files
F174257335: D51735.id159748.diff
Thu, Oct 1, 7:14 PM
F174252897: D51735.id161045.diff
Thu, Oct 1, 6:19 PM
F174252421: D51735.id161036.diff
Thu, Oct 1, 6:14 PM
F174249068: D51735.id161059.diff
Thu, Oct 1, 5:27 PM
F174247556: D51735.id186899.diff
Thu, Oct 1, 5:09 PM
F174244085: D51735.id161097.diff
Thu, Oct 1, 4:26 PM
F174237315: D51735.id179608.diff
Thu, Oct 1, 3:02 PM
F174237179: D51735.id161115.diff
Thu, Oct 1, 3:00 PM
View All Files
Subscribers
bcran
imp
kevans
rgrimes

Details

Reviewers
markj
lwhsu
khng
bz
aokblast
Group Reviewers
bhyve
Summary

When disabling a port, xHCI should allow the backend a chance to cancel
all active transfers before deallocating memory. To support this, we
introduce an interface in UE that lets the backend register a
cancellation callback.

Additionally, this commit allows the backend to populate the tr_softc
field when setting up a transfer in ue_data, enabling context-specific
handling for transfer cancellation and cleanup.

Sponsored By: The FreeBSD Foundation

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 76988
Build 73871: arc lint + arc unit

Event Timeline

aokblast created this revision.Aug 5 2025, 9:37 AM
Herald added a reviewer: bhyve. · View Herald TranscriptAug 5 2025, 9:37 AM
Herald added subscribers: bcran, rgrimes, imp. · View Herald Transcript
aokblast requested review of this revision.Aug 5 2025, 9:37 AM
Harbormaster completed remote builds in B65965: Diff 159748.Aug 5 2025, 9:37 AM
aokblast added reviewers: markj, lwhsu, khng.Aug 5 2025, 9:38 AM
aokblast added a child revision: D52166: bhyve: implement single USB device passthrough support.Aug 26 2025, 4:04 PM
aokblast updated this revision to Diff 161036.Aug 27 2025, 1:32 PM

Minor fixes

Harbormaster completed remote builds in B66559: Diff 161036.Aug 27 2025, 1:32 PM
bz accepted this revision.Aug 27 2025, 1:35 PM
This revision is now accepted and ready to land.Aug 27 2025, 1:35 PM
markj added a comment.Aug 27 2025, 1:40 PM

You don't need to call ue_cancel in pci_xhci_cmd_reset_ep()?

aokblast updated this revision to Diff 161040.Aug 27 2025, 3:31 PM

Add missing ue_cancel

This revision now requires review to proceed.Aug 27 2025, 3:31 PM
Harbormaster completed remote builds in B66561: Diff 161040.Aug 27 2025, 3:31 PM
aokblast added inline comments.Aug 27 2025, 3:34 PM
usr.sbin/bhyve/pci_xhci.c
1216–1217

The rest ep only stop the xfer. However, the parameter of ue_reset pass a whole device instead of a xfer.

1216–1217

Sorry, only clear the stall bit instead of stop whole transfer.

markj accepted this revision as: markj.Aug 27 2025, 3:41 PM
markj added inline comments.
usr.sbin/bhyve/pci_xhci.c
1220

We definitely don't need to be able to return an error from ue_cancel?

This revision is now accepted and ready to land.Aug 27 2025, 3:41 PM
aokblast updated this revision to Diff 161044.Aug 27 2025, 4:01 PM

Use int

This revision now requires review to proceed.Aug 27 2025, 4:01 PM
Harbormaster completed remote builds in B66564: Diff 161044.Aug 27 2025, 4:01 PM
aokblast updated this revision to Diff 161045.Aug 27 2025, 4:03 PM

Formatting

Harbormaster completed remote builds in B66565: Diff 161045.Aug 27 2025, 4:03 PM
markj added inline comments.Aug 27 2025, 4:05 PM
usr.sbin/bhyve/pci_xhci.c
757

Or print a warning if it fails.

1218
aokblast updated this revision to Diff 161059.Aug 27 2025, 6:25 PM

Use (void)

Harbormaster completed remote builds in B66576: Diff 161059.Aug 27 2025, 6:25 PM
aokblast updated this revision to Diff 161097.Aug 28 2025, 6:30 AM

Destroy mutex

Harbormaster completed remote builds in B66589: Diff 161097.Aug 28 2025, 6:30 AM
aokblast updated this revision to Diff 161115.Aug 28 2025, 2:51 PM

No clear in reset

Harbormaster completed remote builds in B66599: Diff 161115.Aug 28 2025, 2:51 PM
markj added inline comments.Sep 2 2025, 1:10 PM
usr.sbin/bhyve/pci_xhci.c
704

If you move the mutex_destroy to pci_xhci_disable_ep() (which also handles freeing the xfer), you can use this subroutine in pci_xhci_reset_slot() as well.

1221
aokblast updated this revision to Diff 179608.Jun 11 2026, 3:03 PM

Reflect markj's comment

Harbormaster completed remote builds in B73825: Diff 179608.Jun 11 2026, 3:03 PM
markj added inline comments.Jul 13 2026, 4:00 PM
usr.sbin/bhyve/pci_xhci.c
1196

Why did we drop handling of XHCI_TRB_TYPE_RESET_EP?

kevans added a subscriber: kevans.Sun, Sep 13, 3:51 PM
kevans added inline comments.
usr.sbin/bhyve/pci_xhci.c
1196

I thought this was actually a bugfix, but section 4.6.8 of the xhci spec kind of buries the key detail: "[...] any subsequent packets received for the endpoint will be silently dropped." We only have usb_mouse.c as an example, but its implementation clears sc->newdata which would seem to silently drop future packets. I will restore it here.

kevans updated this revision to Diff 186897.Wed, Sep 16, 5:43 PM

Restore handling of XHCI_TRB_TYPE_RESET_EP to be sure that we don't retain data
that should be dropped according to the spec.

Harbormaster completed remote builds in B76986: Diff 186897.Wed, Sep 16, 5:43 PM
kevans commandeered this revision.Wed, Sep 16, 5:44 PM
kevans added a reviewer: aokblast.
kevans updated this revision to Diff 186899.Wed, Sep 16, 5:47 PM
kevans marked 5 inline comments as done.

Fixed one indentation issue

Harbormaster completed remote builds in B76988: Diff 186899.Wed, Sep 16, 5:47 PM

Revision Contents
Changeset List

PathSize
usr.sbin/
bhyve/
55 lines
1 line

Diff 186899

View Options

usr.sbin/bhyve/pci_xhci.c

Loading...
View Options

usr.sbin/bhyve/usb_emul.h

Loading...