On Wed, Sep 16, 2020 at 09:19:37PM +0000, Mikolaj Kucharski wrote:
> So time of the crash varies and I guess probably there is no pattern
> there. Here is new panic report, with some kernel printf()s added.
>
> Problem seems to be related that dataphyspage is greater than
> dataphyslastpage: lastpage=0xa0e0000 page=0xa0e1000 phys=0xa0e1000.
> The same is observable in all those previous panics.
>
Here ia nother analysis, without my patches, I think they may be
confusing. I will show flow of ehci_alloc_sqtd_chain() with values
of variables which I collected via my debugging effort.
I think I have understanding what is the flow, but not sure what
code should do in those circumstances.
Values are based on panic:
lastpage=0xa0ec000 ehci_page=0xa0ed000 phys=0xa0ed000
ehci_alloc_sqtd_chain(ffff8000000db000,1000,fffffd812e8d6990,ffff8000225adfc0,ffff8000225adfc8)
at ehci_alloc_sqtd_chain+0x766
but there are also panics with alen=0x2000
ehci_alloc_sqtd_chain(ffff8000000db000,2000,fffffd812e8d6aa0,ffff8000225c60e0,ffff8000225c60e8)
at ehci_alloc_sqtd_chain+0x729
1 usbd_status
2 ehci_alloc_sqtd_chain(struct ehci_softc *sc, u_int alen, struct
usbd_xfer *xfer,
3 struct ehci_soft_qtd **sp, struct ehci_soft_qtd **ep)
4 {
5 struct ehci_soft_qtd *next, *cur;
6 ehci_physaddr_t dataphys, dataphyspage, dataphyslastpage,
nextphys;
7 u_int32_t qtdstatus;
8 u_int len, curlen;
9 int mps, i, iscontrol, forceshort;
10 int rd = usbd_xfer_isread(xfer);
11 struct usb_dma *dma = &xfer->dmabuf;
12
13 DPRINTFN(alen<4*4096,("ehci_alloc_sqtd_chain: start len=%d\n",
alen));
14
15 len = alen;
alen=4096
16 iscontrol =
UE_GET_XFERTYPE(xfer->pipe->endpoint->edesc->bmAttributes) ==
17 UE_CONTROL;
iscontrol=0
18
19 dataphys = DMAADDR(dma, 0);
20 dataphyslastpage = EHCI_PAGE(dataphys + len - 1);
dataphys=0xa0ec000
dataphyslastpage=0xa0ec000
21 qtdstatus = EHCI_QTD_ACTIVE |
22 EHCI_QTD_SET_PID(rd ? EHCI_QTD_PID_IN : EHCI_QTD_PID_OUT) |
23 EHCI_QTD_SET_CERR(3); /* IOC and BYTES set below */
24 mps = UGETW(xfer->pipe->endpoint->edesc->wMaxPacketSize);
mps=512
25 forceshort = ((xfer->flags & USBD_FORCE_SHORT_XFER) || len ==
0) &&
26 len % mps == 0;
xfer->flags=0x9
USBD_FORCE_SHORT_XFER=0x8
(xfer->flags & USBD_FORCE_SHORT_XFER)=0x8
forceshort=1
27 /*
28 * The control transfer data stage always starts with a toggle
of 1.
29 * For other transfers we let the hardware track the toggle
state.
30 */
31 if (iscontrol)
32 qtdstatus |= EHCI_QTD_SET_TOGGLE(1);
33
34 cur = ehci_alloc_sqtd(sc);
35 *sp = cur;
36 if (cur == NULL)
37 goto nomem;
38
39 usb_syncmem(dma, 0, alen,
40 rd ? BUS_DMASYNC_PREREAD : BUS_DMASYNC_PREWRITE);
41 for (;;) {
42 dataphyspage = EHCI_PAGE(dataphys);
dataphyspage=0xa0ec000
43 /* The EHCI hardware can handle at most 5 pages. */
44 if (dataphyslastpage - dataphyspage <
45 EHCI_QTD_NBUFFERS * EHCI_PAGE_SIZE) {
EHCI_QTD_NBUFFERS=5
EHCI_PAGE_SIZE=4096
0 < 5*4096 (20480) -> true
46 /* we can handle it in this QTD */
47 curlen = len;
curlen=4096
48 } else {
else branch is not executed
49 /* must use multiple TDs, fill as much as
possible. */
50 curlen = EHCI_QTD_NBUFFERS * EHCI_PAGE_SIZE -
51 EHCI_PAGE_OFFSET(dataphys);
52 #ifdef DIAGNOSTIC
53 if (curlen > len) {
54 printf("ehci_alloc_sqtd_chain:
curlen=%u "
55 "len=%u offs=0x%x\n", curlen, len,
56 EHCI_PAGE_OFFSET(dataphys));
57 printf("lastpage=0x%x page=0x%x
phys=0x%x\n",
58 dataphyslastpage, dataphyspage,
dataphys);
59 curlen = len;
60 }
61 #endif
62 /* the length must be a multiple of the max
size */
63 curlen -= curlen % mps;
64 DPRINTFN(1,("ehci_alloc_sqtd_chain: multiple
QTDs, "
65 "curlen=%u\n", curlen));
66 #ifdef DIAGNOSTIC
67 if (curlen == 0)
68 panic("ehci_alloc_std: curlen == 0");
69 #endif
70 }
71
72 DPRINTFN(4,("ehci_alloc_sqtd_chain: dataphys=0x%08x "
73 "dataphyslastpage=0x%08x len=%u curlen=%u\n",
74 dataphys, dataphyslastpage, len, curlen));
75 len -= curlen;
len=0
76
77 /*
78 * Allocate another transfer if there's more data left,
79 * or if force last short transfer flag is set and we're
80 * allocating a multiple of the max packet size.
81 */
82 if (len != 0 || forceshort) {
len=0
forceshort=1 -> true
83 next = ehci_alloc_sqtd(sc);
84 if (next == NULL)
85 goto nomem;
86 nextphys = htole32(next->physaddr);
87 } else {
else branch is not executed
88 next = NULL;
89 nextphys = htole32(EHCI_LINK_TERMINATE);
90 }
91
92 for (i = 0; i * EHCI_PAGE_SIZE <
93 curlen + EHCI_PAGE_OFFSET(dataphys); i++) {
i=0 * EHCI_PAGE_SIZE=4096 < curlen=4096 +
EHCI_PAGE_OFFSET(dataphys)=0
0 < 4096 -> true
94 ehci_physaddr_t a = dataphys + i *
EHCI_PAGE_SIZE;
a=dataphys=0xa0ec000
i=0
95 if (i != 0) /* use offset only in first buffer
*/
96 a = EHCI_PAGE(a);
97 #ifdef DIAGNOSTIC
98 if (i >= EHCI_QTD_NBUFFERS) {
99 printf("ehci_alloc_sqtd_chain: i=%d\n",
i);
100 goto nomem;
101 }
102 #endif
103 cur->qtd.qtd_buffer[i] = htole32(a);
104 cur->qtd.qtd_buffer_hi[i] = 0;
105 }
above loop iterates only once with alen=4096
some panics have alen=8192 and then loop iterates twice
106 cur->nextqtd = next;
107 cur->qtd.qtd_next = cur->qtd.qtd_altnext = nextphys;
108 cur->qtd.qtd_status = htole32(qtdstatus |
109 EHCI_QTD_SET_BYTES(curlen));
110 cur->len = curlen;
111 DPRINTFN(10,("ehci_alloc_sqtd_chain: cbp=0x%08x
end=0x%08x\n",
112 dataphys, dataphys + curlen));
113 DPRINTFN(10,("ehci_alloc_sqtd_chain: curlen=%u\n",
curlen));
114 if (iscontrol) {
115 /*
116 * adjust the toggle based on the number of
packets
117 * in this qtd
118 */
119 if ((((curlen + mps - 1) / mps) & 1) || curlen
== 0)
120 qtdstatus ^= EHCI_QTD_TOGGLE_MASK;
121 }
122 if (len == 0) {
len=0 -> true
123 if (! forceshort)
forceshort=1 -> false
124 break;
125 forceshort = 0;
above break is not executed, forceshort is set
to zero
126 }
127 usb_syncmem(&cur->dma, cur->offs, sizeof(cur->qtd),
128 BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD);
129 DPRINTFN(10,("ehci_alloc_sqtd_chain: extend chain\n"));
130 dataphys += curlen;
dataphys=0xa0ec000
curlen=4096 -> dataphys=0xa0ed000
This is panic condition as EHCI_PAGE(dataphys) >
dataphyslastpage
What should happen here or maybe a bit earlier?
What would be the correct flow?
In my code I have panic here, as there is no
point going further, code will hit panic()
defined earlier in next iteration of this loop,
in line 68 in this code lising.
if (EHCI_PAGE(dataphys) > dataphyslastpage)
panic("%s: EHCI_PAGE(dataphys) >
dataphyslastpage", __func__);
131 cur = next;
132 }
133 cur->qtd.qtd_status |= htole32(EHCI_QTD_IOC);
134 usb_syncmem(&cur->dma, cur->offs, sizeof(cur->qtd),
135 BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD);
136 *ep = cur;
137
138 DPRINTFN(10,("ehci_alloc_sqtd_chain: return sqtd=%p
sqtdend=%p\n",
139 *sp, *ep));
140
141 return (USBD_NORMAL_COMPLETION);
142
143 nomem:
144 /* XXX free chain */
145 DPRINTFN(-1,("ehci_alloc_sqtd_chain: no memory\n"));
146 return (USBD_NOMEM);
147 }
--
Regards,
Mikolaj
