ehci: Improve detection of invalid entries in the async schedule
PATCH 1/1 ehci: Improve detection of invalid entries in the async schedule
Hi Jan:
Another patch for the ehci branch - emptying out my patch queue as I need to set this aside for a while.
I am testing qemu and qemu-kvm with 3 different USB keys -- all Kingstons of various sizes 1 GB, 4GB and 8GB. I am definitely seeing different behaviors with the 4GB and 8GB working really well. With the 4GB key I am seeing data rates of 5MB/sec with qemu and close to 8MB/sec with kvm. The 1 GB key hits hiccups from time to time.
If you see the stall enable debugging in usb-linux:
usb-linux.c +1 -1
@@ -243,7 +243,7 @@ static void async_complete(void *opaque) p = aurb->packet; -DPRINTF ("husb: async completed. aurb %p status %d alen %d\n" , +printf ("husb: async completed. aurb %p status %d alen %d\n" , aurb, aurb->urb.status, aurb->urb.actual_length);if (p) {
David
PATCH 1/1 Improve detection of invalid entries in the async schedule
--- hw/usb-ehci.c | 41 +++++++++++++++++++++++++++++------------ 1 files changed, 29 insertions(+), 12 deletions(-)
hw/usb-ehci.c +29 -12
@@ -144,7 +144,7 @@#define NB_MAXINTRATE 8 // Max rate at which controller issues ints#define NB_PORTS 4 // Number of downstream ports#define BUFF_SIZE 5*4096 // Max bytes to transfer per transaction -#define MAX_ITERATIONS 50 // Max number of states before we abort +#define MAX_ITERATIONS 20 // Max number of QH before we break the loop#define MAX_QH 100 // Max allowable queue heads in a chain/* Internal periodic / asynchronous schedule state machine states
@@ -1319,6 +1319,11 @@ static int ehci_state_fetchentry(EHCIState *ehci, int async, int *state) } }#endif +if (entry <0x1000 ) { +DPRINTF ("fetchentry: entry invalid (0x%08x)\n" , entry); + *state =EST_ACTIVE ; + goto out; + }/* section 4.8, only QH in async schedule */ if (async && (NLPTR_TYPE_GET (entry) !=NLPTR_TYPE_QH )) {
@@ -1348,6 +1353,7 @@ static int ehci_state_fetchentry(EHCIState *ehci, int async, int *state)return -1 ; } +out:return again; }
@@ -1394,7 +1400,7 @@ static int ehci_state_fetchqh(EHCIState *ehci, int async, int *state) *state =EST_HORIZONTALQH ; again =1 ; - }else if (qh->token &QTD_TOKEN_ACTIVE ) { + }else if ((qh->token &QTD_TOKEN_ACTIVE ) && (qh->current_qtd >0x1000 )) {DPRINTF_ST ("FETCHQH: Active, !Halt, execute - fetch qTD\n" ); ehci->qtdaddr = qh->current_qtd; *state =EST_FETCHQTD ;
@@ -1499,10 +1505,17 @@ static int ehci_state_fetchqtd(EHCIState *ehci, int async, int *state)static int ehci_state_horizqh (EHCIState *ehci,int async,int *state) { - ehci->fetch_addr = ehci->qh.next; - *state =EST_FETCHENTRY ; +int again =0 ; -return 1 ; +if (ehci->fetch_addr != ehci->qh.next) { + ehci->fetch_addr = ehci->qh.next; + *state =EST_FETCHENTRY ; + again =1 ; + }else { + *state =EST_ACTIVE ; + } + +return again; }static int ehci_state_execute (EHCIState *ehci,int async,int *state)
@@ -1682,11 +1695,17 @@ static int ehci_advance_state(EHCIState *ehci,int iter =0 ; do { - iter++; - if (iter > MAX_ITERATIONS) { - DPRINTF("\n*** advance_state: bailing on MAX ITERATIONS***\n\n"); - state = EST_ACTIVE; - break; + if (state == EST_FETCHQH) { + iter++; + /* if we are roaming a lot of QH without executing a qTD + * something is wrong with the linked list. TO-DO: why is + * this hack needed? + */ + if (iter > MAX_ITERATIONS) { + DPRINTF("\n*** advance_state: bailing on MAX ITERATIONS***\n"); + state = EST_ACTIVE; + break; + } } switch(state) { case EST_WAITLISTHEAD:
@@ -1723,12 +1742,10 @@ static int ehci_advance_state(EHCIState *ehci,break ;case EST_EXECUTING : - iter =0 ; again =ehci_state_executing (ehci, async, &state);break ;case EST_WRITEBACK : - iter =0 ; again =ehci_state_writeback (ehci, async, &state);break ;
Signature · 1 line
1.6.6.1
[Qemu-devel] Re: [PATCH] Improve detection of invalid entries in the async schedule
Quoted 1 line · David Ahern
Signed-off-by David Ahern <[email protected]>
Thanks, merged (with a tiny cleanup).
Jan
Quoted 103 lines
--- hw/usb-ehci.c | 41 +++++++++++++++++++++++++++++------------ 1 files changed, 29 insertions(+), 12 deletions(-)hw/usb-ehci.c +29
-12@@ -144,7 +144,7 @@#define NB_MAXINTRATE 8 // Max rate at which controller issues ints#define NB_PORTS 4 // Number of downstream ports#define BUFF_SIZE 5*4096 // Max bytes to transfer per transaction -#define MAX_ITERATIONS 50 // Max number of states before we abort +#define MAX_ITERATIONS 20 // Max number of QH before we break the loop#define MAX_QH 100 // Max allowable queue heads in a chain/* Internal periodic / asynchronous schedule state machine states @@ -1319,6 +1319,11 @@ static int ehci_state_fetchentry(EHCIState *ehci, int async, int *state) } }#endif +if (entry <0x1000 ) { +DPRINTF ("fetchentry: entry invalid (0x%08x)\n" , entry); + *state =EST_ACTIVE ; + goto out; + }/* section 4.8, only QH in async schedule */ if (async && (NLPTR_TYPE_GET (entry) !=NLPTR_TYPE_QH )) {@@ -1348,6 +1353,7 @@ static int ehci_state_fetchentry(EHCIState *ehci, int async, int *state)return -1 ; } +out:return again; }@@ -1394,7 +1400,7 @@ static int ehci_state_fetchqh(EHCIState *ehci, int async, int *state) *state =EST_HORIZONTALQH ; again =1 ; - }else if (qh->token &QTD_TOKEN_ACTIVE ) { + }else if ((qh->token &QTD_TOKEN_ACTIVE ) && (qh->current_qtd >0x1000 )) {DPRINTF_ST ("FETCHQH: Active, !Halt, execute - fetch qTD\n" ); ehci->qtdaddr = qh->current_qtd; *state =EST_FETCHQTD ;@@ -1499,10 +1505,17 @@ static int ehci_state_fetchqtd(EHCIState *ehci, int async, int *state)static int ehci_state_horizqh (EHCIState *ehci,int async,int *state) { - ehci->fetch_addr = ehci->qh.next; - *state =EST_FETCHENTRY ; +int again =0 ; -return 1 ; +if (ehci->fetch_addr != ehci->qh.next) { + ehci->fetch_addr = ehci->qh.next; + *state =EST_FETCHENTRY ; + again =1 ; + }else { + *state =EST_ACTIVE ; + } + +return again; }static int ehci_state_execute (EHCIState *ehci,int async,int *state)@@ -1682,11 +1695,17 @@ static int ehci_advance_state(EHCIState *ehci,int iter =0 ; do { - iter++; - if (iter > MAX_ITERATIONS) { - DPRINTF("\n*** advance_state: bailing on MAX ITERATIONS***\n\n"); - state = EST_ACTIVE; - break; + if (state == EST_FETCHQH) { + iter++; + /* if we are roaming a lot of QH without executing a qTD + * something is wrong with the linked list. TO-DO: why is + * this hack needed? + */ + if (iter > MAX_ITERATIONS) { + DPRINTF("\n*** advance_state: bailing on MAX ITERATIONS***\n"); + state = EST_ACTIVE; + break; + } } switch(state) { case EST_WAITLISTHEAD:@@ -1723,12 +1742,10 @@ static int ehci_advance_state(EHCIState *ehci,break ;case EST_EXECUTING : - iter =0 ; again =ehci_state_executing (ehci, async, &state);break ;case EST_WRITEBACK : - iter =0 ; again =ehci_state_writeback (ehci, async, &state);break ;