Re: [RFT PATCH 2/2] xhci: handle port status events for removed USB3 hcd

[Jack Pham <jackp@codeaurora.org>]

Hi Mathias,

On Fri, Sep 28, 2018 at 03:35:10AM +0000, Peter Chen wrote:
>  
> > > Cc: <stable@vger.kernel.org>
> > > Reported-by: Peter Chen <peter.chen@nxp.com>
> > > Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> > > ---
> > >   drivers/usb/host/xhci-ring.c | 7 +++++++
> > >   1 file changed, 7 insertions(+)
> > >
> > > diff --git a/drivers/usb/host/xhci-ring.c
> > > b/drivers/usb/host/xhci-ring.c index f0a99aa..3d314b8 100644
> > > --- a/drivers/usb/host/xhci-ring.c
> > > +++ b/drivers/usb/host/xhci-ring.c
> > > @@ -1552,6 +1552,13 @@ static void handle_port_status(struct xhci_hcd *xhci,
> > >   		goto cleanup;
> > >   	}
> > >
> > > +	/* We might get interrupts after shared_hcd is removed */
> > > +	if (port->rhub == &xhci->usb3_rhub && xhci->shared_hcd == NULL) {
> > > +		xhci_dbg(xhci, "ignore port event for removed USB3 hcd\n");
> > > +		bogus_port_status = true;
> > > +		goto cleanup;
> > > +	}
> > > +
> > >   	hcd = port->rhub->hcd;
> > >   	bus_state = &xhci->bus_state[hcd_index(hcd)];
> > >   	hcd_portnum = port->hcd_portnum;
> > >
> > 
> > This probably only applies from 4.18 onwards, to test on older kernel try something
> > like this instead:
> > 
> > diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index
> > 6996235..7925da9 100644
> > --- a/drivers/usb/host/xhci-ring.c
> > +++ b/drivers/usb/host/xhci-ring.c
> > @@ -1606,7 +1606,11 @@ static void handle_port_status(struct xhci_hcd *xhci,
> >          hcd = xhci_to_hcd(xhci);
> >          if ((major_revision == 0x03) != (hcd->speed >= HCD_USB3))
> >                  hcd = xhci->shared_hcd;
> > -
> > +       if (!hcd) {

For testing on 4.14 I also added the debug print "ignore port event"
here as well. Maybe it should be there in the final -stable patch as
well.

> > +               bogus_port_status = true;
> > +               goto cleanup;
> > +       }
> >          if (major_revision == 0) {
> >                  xhci_warn(xhci, "Event for port %u not in "
> >                                  "Extended Capabilities, ignoring.\n",
> > 
> > 
> > Jack, Peter, do these patches solve the remove issues you are seeing?
> 
> At my two USB3 platforms, only apply the 1st patch can fix my problem.  Maybe
> my USB3 port change interrupt occurs always before removing USB2 HCD.
> 
> Peter

Ditto. I think the xhci_irq() is getting triggered by something during
usb_remove_hcd() (usb_disconnect on the root hub?) but is able to
complete before it returns. That is, the NULL pointer dereference is
resolved yet I don't see that "ignore port event for removed USB3 hcd"
message at all.

Regardless, it's good to have here just in case, so
Tested-by: Jack Pham <jackp@codeaurora.org>

Will you be sending this as separate patches for -rc vs -stable?

Thanks,
Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project