答复: 答复: [PATCH] xhci: sideband: check vdev liveness before removing endpoints on unregister

[胡连勤 <hulianqin@vivo.com>]

Hi Mathias,

> >> hub_event()
> >>     port_event()
> >>       usb_reset_device(udev)
> >>         if (config) //and for each interface in this config: , for each interface)
> >>           if (cintf->dev.driver) {
> >>             drv = to_usb_driver(cintf->dev.driver);
> >>             if (drv->pre_reset && drv->post_reset)
> >>             unbind = (drv->pre_reset)(cintf);
> >>
> >>
> >> Any chance you could trace the whole call path in more details and see exactly which path
> >> is staken before the crash.
> >>
> >> port_event() should only call usb_reset_device() for usb3 devices with link stuck in
> >> ss.inactive for longer than ~100ms.  Is this really a usb3 audio device?
> >>
> > No, it is not. The device is a full-speed Apple EarPods
> > (USB 1.1):
> >
> >    usb 1-1: new full-speed USB device number 2 using xhci-hcd
> >    usb 1-1: Product: EarPods
> >    usb 1-1: New USB device found, idVendor=05ac, idProduct=110b
> >
> > So the port_event() → usb_reset_device() warm-reset path for
> > SS.Inactive does not apply here.
> > The actual crash path is through usb_disconnect(), not
> > usb_reset_device(). The full sequence from dmesg + crash trace:
> 
> Thanks for clarifying, the crash part is now clear.
> 
> What is still unclear is the parts before this.
> How do we end up in a situation where we are resetting and addressing
> a device with sideband still registered.
> 
> What codepath ends up calling the futile
> 
> hub_port_init()
>    ->hub_address_device()
> 
> that ends up freeing and reallocating the xhci vdev?
> 
The trigger is reset-resume, not warm reset. From dmesg:

  [158077.907] usb 1-1: usb_autoresume_device: cnt 1 -> 0
  [158078.017] xhci-hcd: WARN Set TR Deq Ptr cmd failed due to incorrect slot or ep state.
  [158078.017] xhci-hcd: WARN urb submitted to disabled ep
  [158078.017] usb 1-1: reset full-speed USB device number 2 using xhci-hcd

The device was resuming from autosuspend but the port was disabled.
In hub_port_status_check() (hub.c:3329):

  else if (!(portstatus & USB_PORT_STAT_ENABLE) && !udev->reset_resume) {
      if (udev->persist_enabled)
          udev->reset_resume = 1;

This sets reset_resume = 1, then finish_port_resume() (hub.c:3678)
calls usb_reset_and_verify_device() → hub_port_init() →
hub_set_address() → xhci_setup_device().

> Is there a way (like drv->pre_reset call) among that path to
> inform audio driver to unregister sideband?
> 
No. The reset-resume path goes through finish_port_resume() →
usb_reset_and_verify_device() (hub.c:3678), which calls
usb_reset_and_verify_device() directly, bypassing
usb_reset_device(). So drv->pre_reset() is never called in
this path, and the interface remains bound throughout the reset
attempt.

This is why the crash trace shows usb_unbind_interface() being
called from usb_disconnect() — the interface was never unbound
before the reset.

The snd-usb-audio driver does have a .reset_resume callback
(usb_audio_resume), but it handles stream restart, not sideband
unregistration. There is currently no hook in the reset-resume
path for the audio driver to unregister sideband before
usb_reset_and_verify_device() begins.

Regards,
Lianqin