On 18.9.2020 17.20, Andrzej Pietrasiewicz wrote: > Hi Mathias, > > W dniu 18.09.2020 o 12:50, Mathias Nyman pisze: >> On 17.9.2020 18.30, Andrzej Pietrasiewicz wrote: >>> Dear All, >>> >>> I have encountered a problem in xhci which leads to general protection fault. >>> >>> The problem is triggered by running this program: >>> >>> https://gitlab.collabora.com/andrzej.p/bulk-cancel.git >>> >>> $ sudo ./bulk-cancel -D /dev/bus/usb/002/006 -i 1 -b 1 >>> >>> where /dev/bus/usb/002/006 is a Gadget Zero: >>> >>> It takes less than a minute until the crash happens. >>> The DMAR (iommu) errors don't happen always, i.e. there are crashes >>> when they are not reported in the system log. In either case the >>> >>> "WARN Cannot submit Set TR Deq Ptr" >>> "A Set TR Deq Ptr command is pending." >>> "WARN Set TR Deq Ptr cmd failed due to incorrect slot or ep state." >>> >>> messages do appear. >>> >> >> Nice testcase and report, thanks. >> >> I started looking at issues in this area some time ago, and wrote a couple patches but >> it was left hanging. The two patches (now rebased on 5.9-rc3) can be found in my tree in the >> fix_invalid_context_at_stop_endpoint branch >> >> git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git fix_invalid_context_at_stop_endpoint >> >> https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/log/?h=fix_invalid_context_at_stop_endpoint >> >> If you could give those a try and see if they help I'd be grateful. > > No, it doesn't help, albeit the errors are slightly different: > > xhci_hcd 0000:00:14.0: WARN Cannot submit Set TR Deq Ptr > xhci_hcd 0000:00:14.0: A Set TR Deq Ptr command is pending. > dmar_fault: 44 callbacks suppressed > DRHD: handling fault status reg 3> DMAR: [DMA Write] Request device [00:14.0] PASID ffffffff fault addr ffcda000 [fault reason 05] PTE Write access is not set > DMAR: DRHD: handling fault status reg 3 Ok, thanks, the DMA problems make sense to me now. If a transfer ring stops on a transfer request (TRB) that should be canceled (manual cancel, or error) it's not enough to just turn the TRB to a no-op. HW has most likely cached the TRB, and we need to move the transfer ring dequeue pointer past this TRB. Moving deq also clears controller cache. We do all this, but if we fail to queue the Set TR Deq command the TRB (with DMA pointers) will stay on the ring, and controller will access the TRB DMA address once it continues running. At this point xhci driver has already given back the canceled/erroneous TRB, and is probably unmapped already. Hence the DMAR entries. Looks like this part of the code needs a more extensive rewrite, on top of this we are not handling races between endpoints halted due errors, and endpoints stopped by driver to cancel URBs. -Mathias
