On 04/08/2022 13.43, Thomas Huth wrote:
On 04/08/2022 12.17, Peter Maydell wrote:
On Thu, 4 Aug 2022 at 11:07, Thomas Huth <th...@redhat.com> wrote:
On 04/08/2022 10.56, Peter Maydell wrote:
But the point of TRB_LINK_LIMIT is that regardless of what the
contents of the TRBs are, the loop is not supposed to
be able to continue for more than TRB_LINK_LIMIT iterations,
ie 32 times. In this example case, do we stop after 32 TRBs
(case 2) or not (case 1)?
Oh, wait, I think we were maybe looking at different spots. The problem
likely does not occur in the xhci_ring_fetch() function
(which you were likely looking at), but only in the xhci_ring_chain_length()
function (which I was looking at)!
xhci_ring_chain_length() can certainly continue more than 32 times. In
xhci_ring_chain_length() the TRB_LINK_LIMIT only applies if "type ==
TR_LINK", but the TRBs we're talking about here are *not* of type TR_LINK.
That sounds like we do still have an unbounded-loop problem,
then: there's no limit on the number of consecutive TRBs
we try to read in that function. Maybe we're missing an
error check of some kind (does the spec limit how many
consecutive TRBs there can be somehow?) or else we need
another artificial limit.
I'm not an XHCI expert at all, but while at least having a quick glance at
the spec, I did not see any limit there. So I assume that we should enforce
an artificial limit? What would be a good value for this? Gerd, do you maybe
have any opinion?
Ok, after looking at the spec a little bit longer, I discovered chapter "6
Data Structures", where they say that Transfer Ring segments should be
limited to 64kB each. That sounds like a reasonable limit. I'll update my
patch accordingly.
Thomas
