Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

qemu-usb: prevent URB processing within usb_packet_complete #5389

Open
skalk opened this issue Nov 29, 2024 · 1 comment
Open

qemu-usb: prevent URB processing within usb_packet_complete #5389

skalk opened this issue Nov 29, 2024 · 1 comment
Labels

Comments

@skalk
Copy link
Member

skalk commented Nov 29, 2024

Within the Qemu ported code of our XHCI device model in Virtualbox there is the function usb_packet_complete, which has to be called when an asynchronously handled packet (literally all of our USB requests) got processed. The Qemu code in this function takes out other packets out of a queue and is doing the following:

  usb_packet_check_state(p, USB_PACKET_QUEUED);
  usb_process_one(p);
  if (p->status == USB_RET_ASYNC) {
    usb_packet_set_state(p, USB_PACKET_ASYNC);
    break;
  }

When the formerly queued packet gets processed, it is turned into an USB request of our USB API and update_urb on the corresponding USB Interface is called. Unfortunately, if the USB stack (driver, hardware) finishs to process this request before the context of update_urb was left (e.g. high CPU workload, long time of not being scheduled again), the completion of this packet is called while it is still marked queued and not as being asynchronously processed. This will lead to an assertion of the qemu code, and thereby to a non functional VM.

Apart from detecting the assertion before it gets triggered and circumventing it, it is best to avoid calling update_urbs within the complete function at all. Better signal the local I/O handler that work needs to be done.

skalk added a commit to skalk/genode that referenced this issue Nov 29, 2024
Instead of directly process URBs whenever a USBPacket arrives from
the Qemu ported XHCI layer, send a local signal to the I/O handler,
which will process the requests after leaving certain sensible
code pathes like usb_packet_complete. Otherwise, it might happen
that a packet, which was still marked as being queued gets already
completed, which leads to an assertion and hang of the library.

Fix genodelabs#5389
@skalk skalk added the fixed label Nov 29, 2024
@cnuke
Copy link
Member

cnuke commented Nov 29, 2024

@ssumpf please try 0bc4d92 as well. I tested it on the Fuji5b to check if it leads to side effects with isochronous transfers and after more than 10 minutes the high pitched noise problem did not yet occur.

EDIT: nevermind, finally it did occur - it just took a much longer time :-/.

skalk added a commit to skalk/genode that referenced this issue Dec 2, 2024
Instead of directly process URBs whenever a USBPacket arrives from
the Qemu ported XHCI layer, send a local signal to the I/O handler,
which will process the requests after leaving certain sensible
code pathes like usb_packet_complete. Otherwise, it might happen
that a packet, which was still marked as being queued gets already
completed, which leads to an assertion and hang of the library.

Fix genodelabs#5389
chelmuth pushed a commit that referenced this issue Dec 2, 2024
Instead of directly process URBs whenever a USBPacket arrives from
the Qemu ported XHCI layer, send a local signal to the I/O handler,
which will process the requests after leaving certain sensible
code pathes like usb_packet_complete. Otherwise, it might happen
that a packet, which was still marked as being queued gets already
completed, which leads to an assertion and hang of the library.

Fix #5389
chelmuth pushed a commit to chelmuth/genode that referenced this issue Dec 2, 2024
Instead of directly process URBs whenever a USBPacket arrives from
the Qemu ported XHCI layer, send a local signal to the I/O handler,
which will process the requests after leaving certain sensible
code pathes like usb_packet_complete. Otherwise, it might happen
that a packet, which was still marked as being queued gets already
completed, which leads to an assertion and hang of the library.

Fix genodelabs#5389
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants