On Mon, Apr 13, 2020 at 09:36:04PM +0200, Mark Kettenis wrote:
> > From: "Theo de Raadt" <dera...@openbsd.org>
> > cc: Patrick Wildt <patr...@blueri.se>
> > Comments: In-reply-to Mikolaj Kucharski <miko...@kucharski.name>
> > message dated "Mon, 13 Apr 2020 18:55:37 -0000."
> > Content-Type: text/plain; charset="us-ascii"
> > Content-ID: <61132.158680562...@cvs.openbsd.org>
> > Date: Mon, 13 Apr 2020 13:20:27 -0600
> >
> > Mikolaj Kucharski <miko...@kucharski.name> wrote:
> >
> > > On Mon, Apr 13, 2020 at 12:44:54PM -0600, Theo de Raadt wrote:
> > > > Then you need to look at why this crazy value is composed here in this
> > > > loop:
> > > >
> > > > actlen = 0;
> > > > for (sqtd = ex->sqtdstart; sqtd != NULL; sqtd = sqtd->nextqtd) {
> > > > usb_syncmem(&sqtd->dma, sqtd->offs, sizeof(sqtd->qtd),
> > > > BUS_DMASYNC_POSTWRITE | BUS_DMASYNC_POSTREAD);
> > > > nstatus = letoh32(sqtd->qtd.qtd_status);
> > > > if (nstatus & EHCI_QTD_ACTIVE)
> > > > break;
> > > >
> > > > status = nstatus;
> > > > /* halt is ok if descriptor is last, and complete */
> > > > if (sqtd->qtd.qtd_next == htole32(EHCI_LINK_TERMINATE)
> > > > &&
> > > > EHCI_QTD_GET_BYTES(status) == 0)
> > > > status &= ~EHCI_QTD_HALTED;
> > > > if (EHCI_QTD_GET_PID(status) != EHCI_QTD_PID_SETUP)
> > > > actlen += sqtd->len -
> > > > EHCI_QTD_GET_BYTES(status);
> > > > }
> > >
> > > But I just described that in one of my previous emails few minutes ago.
> > >
> > > In my diff I'm using second loop of the same code, but I guess I can
> > > provide more surgurucal diff to be more explicit. What I wrote in my
> > > previous email can be written also as follows:
> > >
> > > actlen += sqtd->len - EHCI_QTD_GET_BYTES(status);
> > >
> > > In above equation, on first for loop iteration:
> > >
> > > actlen is 0
> > > EHCI_QTD_GET_BYTES(status) is 16000
> > > sqtd->len is 8
> > >
> > > actlen += 8 - 1600;
> > >
> > > Which results of actlen being 4294951304, from that moment we are
> > > heading to kernel panic.
> >
> > Then next step is to figure out why the sqtd is incoherent.
>
> I actually suspect that the 16000 here is bogus.
Exactly. I looked at it yesterday, without his additional info, and
realized that in this case the calculated actlen for that control
transfer must be broken.
xfer->length was 8, xfer->actlen was 15996, so something was doing
(0 - 4), or maybe (4 - 1600), which actually makes much more sense,
since otherwise sqtd->len would have been 0, which would be very
weird.
So now, why does EHCI_QTD_GET_BYTES return 16000...
> Can you print the status (full 32 bits) of that particular QTD?
Yeah, I wish I could see the status field for each sqtd in that
chain.
On the bright side: This is not a bug in my DMA diff! But it
exposed a bug in the actlen calculation.
Patrick
