Message ID | 20190321085212.10796-1-lvivier@redhat.com |
---|---|
State | New |
Headers | show |
Series | ohci: don't die on ED_LINK_LIMIT overflow | expand |
On Thu, 21 Mar 2019, Laurent Vivier wrote: > Stop processing the descriptor list instead. The next frame timer tick will > resume the work Shoud this log something when the limit is reached to let the user know why USB stopped working instead of silently ignoring frames? Regards, BALATON Zoltan > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1686705 > Suggested-by: Gerd Hoffmann <kraxel@redhat.com> > Signed-off-by: Laurent Vivier <lvivier@redhat.com> > --- > hw/usb/hcd-ohci.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c > index 196a9f72002d..81cf5ab7a5a7 100644 > --- a/hw/usb/hcd-ohci.c > +++ b/hw/usb/hcd-ohci.c > @@ -1200,7 +1200,7 @@ static int ohci_service_ed_list(OHCIState *ohci, uint32_t head, int completion) > if (head == 0) > return 0; > > - for (cur = head; cur; cur = next_ed) { > + for (cur = head; cur && link_cnt++ < ED_LINK_LIMIT; cur = next_ed) { > if (ohci_read_ed(ohci, cur, &ed)) { > trace_usb_ohci_ed_read_error(cur); > ohci_die(ohci); > @@ -1209,11 +1209,6 @@ static int ohci_service_ed_list(OHCIState *ohci, uint32_t head, int completion) > > next_ed = ed.next & OHCI_DPTR_MASK; > > - if (++link_cnt > ED_LINK_LIMIT) { > - ohci_die(ohci); > - return 0; > - } > - > if ((ed.head & OHCI_ED_H) || (ed.flags & OHCI_ED_K)) { > uint32_t addr; > /* Cancel pending packets for ED that have been paused. */ >
On 21/03/2019 11:15, BALATON Zoltan wrote: > On Thu, 21 Mar 2019, Laurent Vivier wrote: >> Stop processing the descriptor list instead. The next frame timer tick >> will >> resume the work > > Shoud this log something when the limit is reached to let the user know > why USB stopped working instead of silently ignoring frames? I don't think frames are ignored, they are only delayed. The aim of ED_LINK_LIMIT is to avoid an infinite loop inside QEMU. Thanks, Laurent > Regards, > BALATON Zoltan > >> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1686705 >> Suggested-by: Gerd Hoffmann <kraxel@redhat.com> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com> >> --- >> hw/usb/hcd-ohci.c | 7 +------ >> 1 file changed, 1 insertion(+), 6 deletions(-) >> >> diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c >> index 196a9f72002d..81cf5ab7a5a7 100644 >> --- a/hw/usb/hcd-ohci.c >> +++ b/hw/usb/hcd-ohci.c >> @@ -1200,7 +1200,7 @@ static int ohci_service_ed_list(OHCIState *ohci, >> uint32_t head, int completion) >> if (head == 0) >> return 0; >> >> - for (cur = head; cur; cur = next_ed) { >> + for (cur = head; cur && link_cnt++ < ED_LINK_LIMIT; cur = next_ed) { >> if (ohci_read_ed(ohci, cur, &ed)) { >> trace_usb_ohci_ed_read_error(cur); >> ohci_die(ohci); >> @@ -1209,11 +1209,6 @@ static int ohci_service_ed_list(OHCIState >> *ohci, uint32_t head, int completion) >> >> next_ed = ed.next & OHCI_DPTR_MASK; >> >> - if (++link_cnt > ED_LINK_LIMIT) { >> - ohci_die(ohci); >> - return 0; >> - } >> - >> if ((ed.head & OHCI_ED_H) || (ed.flags & OHCI_ED_K)) { >> uint32_t addr; >> /* Cancel pending packets for ED that have been paused. */ >>
On Thu, 21 Mar 2019, Laurent Vivier wrote: > On 21/03/2019 11:15, BALATON Zoltan wrote: >> On Thu, 21 Mar 2019, Laurent Vivier wrote: >>> Stop processing the descriptor list instead. The next frame timer tick >>> will >>> resume the work >> >> Shoud this log something when the limit is reached to let the user know >> why USB stopped working instead of silently ignoring frames? > > I don't think frames are ignored, they are only delayed. The aim of > ED_LINK_LIMIT is to avoid an infinite loop inside QEMU. > > Thanks, > Laurent I don't know, just asked. It wasn't clear from commit message to me. >> Regards, >> BALATON Zoltan >> >>> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1686705 >>> Suggested-by: Gerd Hoffmann <kraxel@redhat.com> >>> Signed-off-by: Laurent Vivier <lvivier@redhat.com> >>> --- >>> hw/usb/hcd-ohci.c | 7 +------ >>> 1 file changed, 1 insertion(+), 6 deletions(-) >>> >>> diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c >>> index 196a9f72002d..81cf5ab7a5a7 100644 >>> --- a/hw/usb/hcd-ohci.c >>> +++ b/hw/usb/hcd-ohci.c >>> @@ -1200,7 +1200,7 @@ static int ohci_service_ed_list(OHCIState *ohci, >>> uint32_t head, int completion) >>> ??? if (head == 0) >>> ??????? return 0; >>> >>> -??? for (cur = head; cur; cur = next_ed) { >>> +??? for (cur = head; cur && link_cnt++ < ED_LINK_LIMIT; cur = next_ed) { >>> ??????? if (ohci_read_ed(ohci, cur, &ed)) { >>> ??????????? trace_usb_ohci_ed_read_error(cur); >>> ??????????? ohci_die(ohci); >>> @@ -1209,11 +1209,6 @@ static int ohci_service_ed_list(OHCIState >>> *ohci, uint32_t head, int completion) >>> >>> ??????? next_ed = ed.next & OHCI_DPTR_MASK; >>> >>> -??????? if (++link_cnt > ED_LINK_LIMIT) { >>> -??????????? ohci_die(ohci); >>> -??????????? return 0; >>> -??????? } >>> - >>> ??????? if ((ed.head & OHCI_ED_H) || (ed.flags & OHCI_ED_K)) { >>> ??????????? uint32_t addr; >>> ??????????? /* Cancel pending packets for ED that have been paused.? */ >>> > >
On Thu, Mar 21, 2019 at 09:52:12AM +0100, Laurent Vivier wrote: > Stop processing the descriptor list instead. The next frame timer tick will > resume the work > > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1686705 > Suggested-by: Gerd Hoffmann <kraxel@redhat.com> > Signed-off-by: Laurent Vivier <lvivier@redhat.com> Added to patch queue. thanks, Gerd
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c index 196a9f72002d..81cf5ab7a5a7 100644 --- a/hw/usb/hcd-ohci.c +++ b/hw/usb/hcd-ohci.c @@ -1200,7 +1200,7 @@ static int ohci_service_ed_list(OHCIState *ohci, uint32_t head, int completion) if (head == 0) return 0; - for (cur = head; cur; cur = next_ed) { + for (cur = head; cur && link_cnt++ < ED_LINK_LIMIT; cur = next_ed) { if (ohci_read_ed(ohci, cur, &ed)) { trace_usb_ohci_ed_read_error(cur); ohci_die(ohci); @@ -1209,11 +1209,6 @@ static int ohci_service_ed_list(OHCIState *ohci, uint32_t head, int completion) next_ed = ed.next & OHCI_DPTR_MASK; - if (++link_cnt > ED_LINK_LIMIT) { - ohci_die(ohci); - return 0; - } - if ((ed.head & OHCI_ED_H) || (ed.flags & OHCI_ED_K)) { uint32_t addr; /* Cancel pending packets for ED that have been paused. */
Stop processing the descriptor list instead. The next frame timer tick will resume the work Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1686705 Suggested-by: Gerd Hoffmann <kraxel@redhat.com> Signed-off-by: Laurent Vivier <lvivier@redhat.com> --- hw/usb/hcd-ohci.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)