Re: [Qemu-devel] [PATCH v2 1/2] ohci: delay first SOF interrupt

[Thomas Huth]

On 06.01.2016 20:45, Laurent Vivier wrote:
> On overcommitted CPU, kernel can be so slow that an interrupt can
> be triggered by the device whereas the driver is not ready to receive
> it. This drives us into an infinite loop.
> 
> This does not happen on real hardware because real hardware never send
> interrupt immediately after the controller has been moved to OPERATION state.
> 
> This patch tries to delay the first SOF interrupt to let driver exits from
> the critical section (which is not protected against interrupts...)
> 
> Some details:
> 
> - ohci_irq(): the OHCI interrupt handler, acknowledges the SOF IRQ
>   only if the state of the driver (rh_state) is OHCI_STATE_RUNNING.
>   So if this interrupt happens and the driver is not in this state,
>   the function is called again and again, moving the system to a
>   CPU starvation.
> 
> - ohci_rh_resume(): the driver re-enables operation with OHCI_USB_OPER.
>   In QEMU this start the SOF timer and QEMU starts to send IRQs. As
>   the driver is not in OHCI_STATE_RUNNING and not protected against IRQ,
>   the ohci_irq() can be called and the driver never moved to
>   OHCI_STATE_RUNNING.
> 
> Suggested-by: Gerd Hoffmann <address@hidden>
> Signed-off-by: Laurent Vivier <address@hidden>
> ---
>  hw/usb/hcd-ohci.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
> index 7d65818..a44fab2 100644
> --- a/hw/usb/hcd-ohci.c
> +++ b/hw/usb/hcd-ohci.c
> @@ -1231,11 +1231,16 @@ static int ohci_service_ed_list(OHCIState *ohci, 
> uint32_t head, int completion)
>      return active;
>  }
>  
> -/* Generate a SOF event, and set a timer for EOF */
> -static void ohci_sof(OHCIState *ohci)
> +/* set a timer for EOF */
> +static void ohci_eof_timer(OHCIState *ohci)
>  {
>      ohci->sof_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>      timer_mod(ohci->eof_timer, ohci->sof_time + usb_frame_time);
> +}
> +/* Set a timer for EOF and generate a SOF event */
> +static void ohci_sof(OHCIState *ohci)
> +{
> +    ohci_eof_timer(ohci);
>      ohci_set_interrupt(ohci, OHCI_INTR_SF);
>  }
>  
> @@ -1343,7 +1348,12 @@ static int ohci_bus_start(OHCIState *ohci)
>  
>      trace_usb_ohci_start(ohci->name);
>  
> -    ohci_sof(ohci);
> +    /* Delay the first SOF event by one frame time as
> +     * linux driver is not ready to receive it and
> +     * can meet some race conditions
> +     */
> +
> +    ohci_eof_timer(ohci);
>  
>      return 1;
>  }

Reviewed-by: Thomas Huth <address@hidden>