Re: xhci(4) isoc: fix bogus handling of chained TRBs
hi,
It works on AMD Bolton xHCI (78141022), Intel PCH (1e318086),
and ASM1042 (10421b21).
I simply play with ffplay -f v4l2 /dev/video0 to test.
At this moment it does not work on VL805, but I have no idea.
I'll investigate furthermore...
BTW I think xhci_ring_alloc should free ring dma buffer
if trb_processed[] allocation fails.
On 2020/07/18 14:47, Marcus Glocker wrote:
> Hey,
>
> On Sat, 18 Jul 2020 00:14:32 +
> sc.dy...@gmail.com wrote:
>
>> hi,
>>
>> On 2020/07/15 21:28, sc.dy...@gmail.com wrote:
>>> hi,
>>>
>>> The patch works well on Intel PCH xhci, but not on AMD Bolton xHCI.
>>> I'll investigate this problem.
>>
>> Bolton xHCI sometimes raises following events.
>> (I added printf Completion Code.)
>>
>> #246 cc 13 remain 0 type 5 origlen 2048 frlengths[6] 2048
>> #247 cc 1 remain 0 type 1 origlen 1024 frlengths[6] 3072
>>
>> It seems this behavior violates the spec. 4.10.1.1.
>> When the remain of 1. TRB is 0, CC should be SUCCESS, not SHORT_PKT,
>> and HC should not raise interrupt.
>> The problem is how many length 2. TRB transferred.
>> If real CC of 1. TRB = SUCCESS, we should add 2. TRB length to
>> frlengths. If SHORT_PKT, we should keep frlengths as is.
>> Actually with the latter assumption I can see video smoothly w/o
>> errors.
>>
>> I changed your patch to skip adding to frlengths and actlen
>> if previous TRB is processed AND remain == 0.
>
> Interesting! Thanks for tracing this down.
> So in general we can say that for a chained TD, if the first TRB is
> SHORT, we can simply skip the second TRB.
>
> To make the code a bit more understandable, and use the new
> trb_processed flag to control this, I made another diff which keeps
> track whether the TRB was not processed, processed, or processed short.
>
> It also includes the malloc NULL check and replaces malloc by
> mallocarray as suggested by tb@.
>
> Does this diff also work for you on your AMD xhci?
>
>
> Index: xhci.c
> ===
> RCS file: /cvs/src/sys/dev/usb/xhci.c,v
> retrieving revision 1.116
> diff -u -p -u -p -r1.116 xhci.c
> --- xhci.c30 Jun 2020 10:21:59 - 1.116
> +++ xhci.c18 Jul 2020 14:20:28 -
> @@ -82,7 +82,7 @@ voidxhci_event_xfer(struct xhci_softc *
> int xhci_event_xfer_generic(struct xhci_softc *, struct usbd_xfer *,
> struct xhci_pipe *, uint32_t, int, uint8_t, uint8_t, uint8_t);
> int xhci_event_xfer_isoc(struct usbd_xfer *, struct xhci_pipe *,
> - uint32_t, int);
> + uint32_t, int, uint8_t);
> void xhci_event_command(struct xhci_softc *, uint64_t);
> void xhci_event_port_change(struct xhci_softc *, uint64_t, uint32_t);
> int xhci_pipe_init(struct xhci_softc *, struct usbd_pipe *);
> @@ -800,7 +800,7 @@ xhci_event_xfer(struct xhci_softc *sc, u
> return;
> break;
> case UE_ISOCHRONOUS:
> - if (xhci_event_xfer_isoc(xfer, xp, remain, trb_idx))
> + if (xhci_event_xfer_isoc(xfer, xp, remain, trb_idx, code))
> return;
> break;
> default:
> @@ -927,13 +927,23 @@ xhci_event_xfer_generic(struct xhci_soft
>
> int
> xhci_event_xfer_isoc(struct usbd_xfer *xfer, struct xhci_pipe *xp,
> -uint32_t remain, int trb_idx)
> +uint32_t remain, int trb_idx, uint8_t code)
> {
> struct usbd_xfer *skipxfer;
> struct xhci_xfer *xx = (struct xhci_xfer *)xfer;
> - int trb0_idx, frame_idx = 0;
> + int trb0_idx, frame_idx = 0, skip_trb = 0;
>
> KASSERT(xx->index >= 0);
> +
> + switch (code) {
> + case XHCI_CODE_SHORT_XFER:
> + xp->ring.trb_processed[trb_idx] = TRB_PROCESSED_SHORT;
> + break;
> + default:
> + xp->ring.trb_processed[trb_idx] = TRB_PROCESSED_YES;
> + break;
> + }
> +
> trb0_idx =
> ((xx->index + xp->ring.ntrb) - xx->ntrb) % (xp->ring.ntrb - 1);
>
> @@ -958,15 +968,21 @@ xhci_event_xfer_isoc(struct usbd_xfer *x
> trb0_idx = xp->ring.ntrb - 2;
> else
> trb0_idx = trb_idx - 1;
> - if (xfer->frlengths[frame_idx] == 0) {
> + if (xp->ring.trb_processed[trb0_idx] == TRB_PROCESSED_NO) {
> xfer->frlengths[frame_idx] = XHCI_TRB_LEN(letoh32(
> xp->ring.trbs[trb0_idx].trb_status));
> + } else if (xp->ring.trb_processed[trb0_idx] ==
> + TRB_PROCESSED_SHORT) {
> + skip_trb = 1;
> }
> }
>
> - xfer->frlengths[frame_idx] +=
> - XHCI_TRB_LEN(letoh32(xp->ring.trbs[trb_idx].trb_status)) - remain;
> - xfer->actlen += xfer->frlengths[frame_idx];
> + if (!skip_trb) {
> + xfer->frlengths[frame_idx] +=
> + XHCI_TRB_LEN(letoh32(xp->ring.trbs[trb_idx].trb_status)) -
> + remain;
> + xfer->actlen += xfer->frlengths[frame
ftpd: ignore -l in LIST path
Hello,
Below is a patch that repairs ftpd LIST operation for at least Chrome browser.
Discard the patch if the intention is to make clients change behavior. A bug
report has already been filed with the browser. Edge browser works fine.
Other browsers untested.
Symptom in raw output in Chrome 83:
ftpd: -l: No such file or directory
Reproduce using Chrome 83:
ftp:///?raw
Cheers,
Brian
Index: popen.c
===
RCS file: /home/brian/cvs/src/libexec/ftpd/popen.c,v
retrieving revision 1.29
diff -u -r1.29 popen.c
--- popen.c 15 Jan 2020 22:06:59 - 1.29
+++ popen.c 18 Jul 2020 23:52:49 -
@@ -76,7 +76,7 @@
argv[argc++] = "--";
/* glob that path */
- if (path != NULL) {
+ if (path != NULL && strcmp(path, "-l") != 0) {
glob_t gl;
memset(&gl, 0, sizeof(gl));
Update minimal i386 configuration
Update minimal i386 configuration based on current requirements OK? Index: distrib/notes/i386/hardware === RCS file: /cvs/src/distrib/notes/i386/hardware,v retrieving revision 1.292 diff -u -r1.292 hardware --- distrib/notes/i386/hardware 2 Sep 2016 12:17:32 - 1.292 +++ distrib/notes/i386/hardware 18 Jul 2020 18:58:23 - @@ -10,9 +10,9 @@ systems. To support SMP operation, a separate SMP kernel (bsd.mp) is included with the installation file sets. -The minimal configuration to install the system is 32MB of RAM and -at least 250MB of disk space to accommodate the `base' set. -To install the entire system, at least 600MB of disk are required, +The minimal configuration to install the system is 256MB of RAM and +at least 512MB of disk space to accommodate the `base' set. +To install the entire system, at least 1GB of disk are required, and to run X or compile the system, more RAM is recommended. Please refer to the website for a full list of supported hardware:
events.html -- add year links and corresponding id parameters as anchor targets
Hello tech@, The following adds a line of clickable year numbers near the top of events.html. This is to aid navigation of this now very long page, so visitors can jump to any desired year without being condemned to the smartphone user one-button coma of having to scroll, scroll, scroll. Ian Index: www/events.html === RCS file: /cvs/www/events.html,v retrieving revision 1.1197 diff -u -r1.1197 events.html --- www/events.html 17 Jul 2020 17:19:25 - 1.1197 +++ www/events.html 18 Jul 2020 18:37:00 - @@ -48,9 +48,36 @@ +2020 +2019 +2018 +2017 +2016 +2015 +2014 +2013 +2012 +2011 +2010 +2009 +2008 +2007 +2006 +2005 +2004 +2003 +2002 +2001 +2000 +1999 +1998 +1997 + + + Past events: -2020 +2020 @@ -68,7 +95,7 @@ -2019 +2019 @@ -182,7 +209,7 @@ -2018 +2018 @@ -336,7 +363,7 @@ -2017 +2017 @@ -450,7 +477,7 @@ -2016 +2016 @@ -552,7 +579,7 @@ -2015 +2015 @@ -727,7 +754,7 @@ -2014 +2014 @@ -885,7 +912,7 @@ -2013 +2013 @@ -995,7 +1022,7 @@ -2012 +2012 @@ -1059,7 +1086,7 @@ -2011 +2011 T-Dose 2011, @@ -1136,7 +1163,7 @@ -2010 +2010 @@ -1219,7 +1246,7 @@ -2009 +2009 @@ -1351,7 +1378,7 @@ -2008 +2008 @@ -1573,7 +1600,7 @@ -2007 +2007 @@ -1943,7 +1970,7 @@ -2006 +2006 @@ -2382,7 +2409,7 @@ -2005 +2005 @@ -2844,7 +2871,7 @@ -2004 +2004 @@ -3092,7 +3119,7 @@ -2003 +2003 @@ -3288,7 +3315,7 @@ -2002 +2002 @@ -3382,7 +3409,7 @@ -2001 +2001 @@ -3479,7 +3506,7 @@ -2000 +2000 @@ -3707,7 +3734,7 @@ -1999 +1999 The Bazaar, @@ -3887,7 +3914,7 @@ -1998 +1998 @@ -4008,7 +4035,7 @@ -1997 +1997
atheros pcidevs
Looking to add a few Atheros devices to pcidevs. ok? Index: pcidevs === RCS file: /cvs/src/sys/dev/pci/pcidevs,v retrieving revision 1.1928 diff -u -p -u -r1.1928 pcidevs --- pcidevs 30 Jun 2020 04:37:24 - 1.1928 +++ pcidevs 18 Jul 2020 16:53:55 - @@ -1081,6 +1081,7 @@ product ATHEROS AR5212_7 0x0019 AR5212 product ATHEROS AR2413 0x001a AR2413 product ATHEROS AR5413 0x001b AR5413 product ATHEROS AR5424 0x001c AR5424 +product ATHEROS AR2417 0x001d AR2417 product ATHEROS AR5416 0x0023 AR5416 product ATHEROS AR5418 0x0024 AR5418 product ATHEROS AR9160 0x0027 AR9160 @@ -1095,6 +1096,9 @@ product ATHEROS AR94850x0032 AR9485 product ATHEROS AR9462 0x0034 AR9462 product ATHEROS AR9565 0x0036 AR9565 product ATHEROS QCA988X0x003c QCA986x/988x +product ATHEROS QCA61740x003e QCA6174 +product ATHEROS QCA61640x0041 QCA6164 +product ATHEROS QCA93770x0042 QCA9377 product ATHEROS AR5210_AP 0x0207 AR5210 product ATHEROS AR5212_IBM 0x1014 AR5212 product ATHEROS AR5210_DEFAULT 0x1107 AR5210
Re: xhci(4) isoc: fix bogus handling of chained TRBs
Hey,
On Sat, 18 Jul 2020 00:14:32 +
sc.dy...@gmail.com wrote:
> hi,
>
> On 2020/07/15 21:28, sc.dy...@gmail.com wrote:
> > hi,
> >
> > The patch works well on Intel PCH xhci, but not on AMD Bolton xHCI.
> > I'll investigate this problem.
>
> Bolton xHCI sometimes raises following events.
> (I added printf Completion Code.)
>
> #246 cc 13 remain 0 type 5 origlen 2048 frlengths[6] 2048
> #247 cc 1 remain 0 type 1 origlen 1024 frlengths[6] 3072
>
> It seems this behavior violates the spec. 4.10.1.1.
> When the remain of 1. TRB is 0, CC should be SUCCESS, not SHORT_PKT,
> and HC should not raise interrupt.
> The problem is how many length 2. TRB transferred.
> If real CC of 1. TRB = SUCCESS, we should add 2. TRB length to
> frlengths. If SHORT_PKT, we should keep frlengths as is.
> Actually with the latter assumption I can see video smoothly w/o
> errors.
>
> I changed your patch to skip adding to frlengths and actlen
> if previous TRB is processed AND remain == 0.
Interesting! Thanks for tracing this down.
So in general we can say that for a chained TD, if the first TRB is
SHORT, we can simply skip the second TRB.
To make the code a bit more understandable, and use the new
trb_processed flag to control this, I made another diff which keeps
track whether the TRB was not processed, processed, or processed short.
It also includes the malloc NULL check and replaces malloc by
mallocarray as suggested by tb@.
Does this diff also work for you on your AMD xhci?
Index: xhci.c
===
RCS file: /cvs/src/sys/dev/usb/xhci.c,v
retrieving revision 1.116
diff -u -p -u -p -r1.116 xhci.c
--- xhci.c 30 Jun 2020 10:21:59 - 1.116
+++ xhci.c 18 Jul 2020 14:20:28 -
@@ -82,7 +82,7 @@ void xhci_event_xfer(struct xhci_softc *
intxhci_event_xfer_generic(struct xhci_softc *, struct usbd_xfer *,
struct xhci_pipe *, uint32_t, int, uint8_t, uint8_t, uint8_t);
intxhci_event_xfer_isoc(struct usbd_xfer *, struct xhci_pipe *,
- uint32_t, int);
+ uint32_t, int, uint8_t);
void xhci_event_command(struct xhci_softc *, uint64_t);
void xhci_event_port_change(struct xhci_softc *, uint64_t, uint32_t);
intxhci_pipe_init(struct xhci_softc *, struct usbd_pipe *);
@@ -800,7 +800,7 @@ xhci_event_xfer(struct xhci_softc *sc, u
return;
break;
case UE_ISOCHRONOUS:
- if (xhci_event_xfer_isoc(xfer, xp, remain, trb_idx))
+ if (xhci_event_xfer_isoc(xfer, xp, remain, trb_idx, code))
return;
break;
default:
@@ -927,13 +927,23 @@ xhci_event_xfer_generic(struct xhci_soft
int
xhci_event_xfer_isoc(struct usbd_xfer *xfer, struct xhci_pipe *xp,
-uint32_t remain, int trb_idx)
+uint32_t remain, int trb_idx, uint8_t code)
{
struct usbd_xfer *skipxfer;
struct xhci_xfer *xx = (struct xhci_xfer *)xfer;
- int trb0_idx, frame_idx = 0;
+ int trb0_idx, frame_idx = 0, skip_trb = 0;
KASSERT(xx->index >= 0);
+
+ switch (code) {
+ case XHCI_CODE_SHORT_XFER:
+ xp->ring.trb_processed[trb_idx] = TRB_PROCESSED_SHORT;
+ break;
+ default:
+ xp->ring.trb_processed[trb_idx] = TRB_PROCESSED_YES;
+ break;
+ }
+
trb0_idx =
((xx->index + xp->ring.ntrb) - xx->ntrb) % (xp->ring.ntrb - 1);
@@ -958,15 +968,21 @@ xhci_event_xfer_isoc(struct usbd_xfer *x
trb0_idx = xp->ring.ntrb - 2;
else
trb0_idx = trb_idx - 1;
- if (xfer->frlengths[frame_idx] == 0) {
+ if (xp->ring.trb_processed[trb0_idx] == TRB_PROCESSED_NO) {
xfer->frlengths[frame_idx] = XHCI_TRB_LEN(letoh32(
xp->ring.trbs[trb0_idx].trb_status));
+ } else if (xp->ring.trb_processed[trb0_idx] ==
+ TRB_PROCESSED_SHORT) {
+ skip_trb = 1;
}
}
- xfer->frlengths[frame_idx] +=
- XHCI_TRB_LEN(letoh32(xp->ring.trbs[trb_idx].trb_status)) - remain;
- xfer->actlen += xfer->frlengths[frame_idx];
+ if (!skip_trb) {
+ xfer->frlengths[frame_idx] +=
+ XHCI_TRB_LEN(letoh32(xp->ring.trbs[trb_idx].trb_status)) -
+ remain;
+ xfer->actlen += xfer->frlengths[frame_idx];
+ }
if (xx->index != trb_idx)
return (1);
@@ -1702,6 +1718,15 @@ xhci_ring_alloc(struct xhci_softc *sc, s
ring->ntrb = ntrb;
+ ring->trb_processed =
+ mallocarray(ring->ntrb, sizeof(*ring->trb_processed), M_DEVBUF,
+ M_NOWAIT);
+ if (ring->trb_processed == NULL) {
+ printf("%s: unable to allocate ring trb_processed array\n",
+ DEVNAME(sc));
+ return
