genodelabs / genode

Genode OS Framework
https://genode.org/
Other
1.08k stars 255 forks source link

qemu-usb: prevent URB processing within usb_packet_complete #5389

Open skalk opened 1 day ago

skalk commented 1 day ago

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.

cnuke commented 1 day ago

@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 :-/.