1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH] ohci: don't die on ED_LINK_LIMIT overflow

Laurent Vivier posted 1 patch 6 years, 10 months ago
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test asan passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190321085212.10796-1-lvivier@redhat.com
Maintainers: Gerd Hoffmann <kraxel@redhat.com>
hw/usb/hcd-ohci.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
[Qemu-devel] [PATCH] ohci: don't die on ED_LINK_LIMIT overflow
Posted by Laurent Vivier 6 years, 10 months ago 
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(-)

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.  */
-- 
2.20.1
Re: [Qemu-devel] [PATCH] ohci: don't die on ED_LINK_LIMIT overflow
Posted by BALATON Zoltan 6 years, 10 months ago 
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.  */
>
Re: [Qemu-devel] [PATCH] ohci: don't die on ED_LINK_LIMIT overflow
Posted by Laurent Vivier 6 years, 10 months ago 
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.  */
>>
Re: [Qemu-devel] [PATCH] ohci: don't die on ED_LINK_LIMIT overflow
Posted by BALATON Zoltan 6 years, 10 months ago 
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.? */
>>>
>
>
Re: [Qemu-devel] [PATCH] ohci: don't die on ED_LINK_LIMIT overflow
Posted by Gerd Hoffmann 6 years, 10 months ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.