linux-kernel - Re: [PATCH v5] usb: ohci: disable start-of-frame interrupt in ohci_rh

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c26e86a7-a24d-42f5-3d39-71c34f8c5e31@loongson.cn>
Date:   Tue, 12 Oct 2021 20:11:45 +0800
From:   zhuyinbo <zhuyinbo@...ngson.cn>
To:     Alan Stern <stern@...land.harvard.edu>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        Greg Kroah-Hartman <greg@...ah.com>,
        Patchwork Bot <patchwork-bot@...nel.org>
Subject: Re: [PATCH v5] usb: ohci: disable start-of-frame interrupt in
 ohci_rh_suspend


在 2021/10/12 下午4:30, Yinbo Zhu 写道:
> The usb states of ohci controller include UsbOperational, UsbReset,
> UsbSuspend and UsbResume. Among them, only the UsbOperational state
> supports launching the start-of-frame for host controller according
> the ohci protocol spec, but in S3 and S4 (suspend to memory/suspend
> to disk) press test procedure, it may happen that the start-of-
> frame was launched in UsbSuspend status and cause ohci works failed
> that the phenomenon was hc will allways reproduce the SoF interrupt
> and consider that hc doesn't deal with the ed/td/done list in non-
> UsbOperational, and this patch was to disable SoF interrupt in ohci
> _rh_suspend so that it can fix ohci SoF abnormally interrupt issue.
>
> Signed-off-by: Yinbo Zhu <zhuyinbo@...ngson.cn>
> ---
> Change in v5:
> 		Move the key code change into ohci_rh_suspend.
> 		Rework the commit log information.
>
>
>   drivers/usb/host/ohci-hub.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/drivers/usb/host/ohci-hub.c b/drivers/usb/host/ohci-hub.c
> index f474f2f..80a0094 100644
> --- a/drivers/usb/host/ohci-hub.c
> +++ b/drivers/usb/host/ohci-hub.c
> @@ -88,6 +88,8 @@ static int ohci_rh_suspend (struct ohci_hcd *ohci, int autostop)
>   		msleep (8);
>   		spin_lock_irq (&ohci->lock);
>   	}
> +	/* All ED unlinks should be finished, no need for SOF interrupts */
> +	ohci_writel(ohci, OHCI_INTR_SF, &ohci->regs->intrdisable);
>   	update_done_list(ohci);
>   	ohci_work(ohci);

Hi Alan Stern,

     I consider that autostop sometimes was '1' then and HC doesn't  
stop deal with the ed/td/done list , then ohci_wok will called 
start_ed_unlink and

     start_ed_unlink will enable INTR_SF as follows, so I think add 
above two line code after ohci_work it is more appropriate, Do you think so?

       static void start_ed_unlink (struct ohci_hcd *ohci, struct ed *ed)
        {

              ...

              494         /* enable SOF interrupt */
              495         ohci_writel (ohci, OHCI_INTR_SF, 
&ohci->regs->intrstatus);
              496         ohci_writel (ohci, OHCI_INTR_SF, 
&ohci->regs->intrenable);

               ...

         }


BRs,

Yinbo Zhu.

>