| 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
| ||
|
Message-ID: <BYAPR12MB3399A285F4DAF09F2AB18DACA7849@BYAPR12MB3399.namprd12.prod.outlook.com>
Date: Fri, 24 Mar 2023 05:09:10 +0000
From: Minas Harutyunyan <Minas.Harutyunyan@...opsys.com>
To: Fabrice Gasnier <fabrice.gasnier@...s.st.com>,
"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
"quentin.schulz@...obroma-systems.com"
<quentin.schulz@...obroma-systems.com>
CC: "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-stm32@...md-mailman.stormreply.com"
<linux-stm32@...md-mailman.stormreply.com>,
"amelie.delaunay@...s.st.com" <amelie.delaunay@...s.st.com>,
"alexandre.torgue@...s.st.com" <alexandre.torgue@...s.st.com>
Subject: RE: [PATCH] usb: dwc2: fix a race, don't power off/on phy for
dual-role mode
Hi Fabrice,
>On 3/15/2023 6:45 PM, Fabrice Gasnier <fabrice.gasnier@...s.st.com> wrote:
>From: Fabrice Gasnier <fabrice.gasnier@...s.st.com>
>Sent: Wednesday, March 15, 2023 6:45 PM
>To: Minas Harutyunyan <hminas@...opsys.com>; gregkh@...uxfoundation.org;
>quentin.schulz@...obroma-systems.com
>Cc: linux-usb@...r.kernel.org; linux-kernel@...r.kernel.org; linux-stm32@st-
>md-mailman.stormreply.com; amelie.delaunay@...s.st.com;
>alexandre.torgue@...s.st.com; fabrice.gasnier@...s.st.com
>Subject: [PATCH] usb: dwc2: fix a race, don't power off/on phy for dual-role
>mode
>
>When in dual role mode (dr_mode == USB_DR_MODE_OTG), platform probe
>successively basically calls:
>- dwc2_gadget_init()
>- dwc2_hcd_init()
>- dwc2_lowlevel_hw_disable() since recent change [1]
>- usb_add_gadget_udc()
>
>The PHYs (and so the clocks it may provide) shouldn't be disabled for all
>SoCs, in OTG mode, as the HCD part has been initialized.
>
>On STM32 this creates some weird race condition upon boot, when:
>- initially attached as a device, to a HOST
>- and there is a gadget script invoked to setup the device part.
>Below issue becomes systematic, as long as the gadget script isn't started
>by userland: the hardware PHYs (and so the clocks provided by the
>PHYs) remains disabled.
>It ends up in having an endless interrupt storm, before the watchdog resets
>the platform.
>
>[ 16.924163] dwc2 49000000.usb-otg: EPs: 9, dedicated fifos, 952 entries
>in SPRAM
>[ 16.962704] dwc2 49000000.usb-otg: DWC OTG Controller
>[ 16.966488] dwc2 49000000.usb-otg: new USB bus registered, assigned bus
>number 2
>[ 16.974051] dwc2 49000000.usb-otg: irq 77, io mem 0x49000000
>[ 17.032170] hub 2-0:1.0: USB hub found
>[ 17.042299] hub 2-0:1.0: 1 port detected
>[ 17.175408] dwc2 49000000.usb-otg: Mode Mismatch Interrupt: currently in
>Host mode
>[ 17.181741] dwc2 49000000.usb-otg: Mode Mismatch Interrupt: currently in
>Host mode
>[ 17.189303] dwc2 49000000.usb-otg: Mode Mismatch Interrupt: currently in
>Host mode
>...
>
>The host part is also not functional, until the gadget part is configured.
>
>The HW may only be disabled for peripheral mode (original init), e.g.
>dr_mode == USB_DR_MODE_PERIPHERAL, until the gadget driver initializes.
>
>But when in USB_DR_MODE_OTG, the HW should remain enabled, as the HCD part
>is able to run, while the gadget part isn't necessarily configured.
>
>I don't fully get the of purpose the original change, that claims disabling
>the hardware is missing. It creates conditions on SOCs using the PHY
>initialization to be completely non working in OTG mode. Original change [1]
>should be reworked to be platform specific.
>
>[1] https://urldefense.com/v3/__https://lore.kernel.org/r/20221206-dwc2-
>gadget-dual-role-v1-2-36515e1092cd@...obroma-
>systems.com__;!!A4F2R9G_pg!Y21e8pRbIOVyLQTRP5HjdeDUHpSjbtiRQVFGVCOBBDu9yH32W
>tdppqmP-8TLyGrBjyOBG5iI4qw6XMFEdfRJDhZ8HVc$
>
>Fixes: ade23d7b7ec5 ("usb: dwc2: power on/off phy for peripheral mode in
>dual-role mode")
>Signed-off-by: Fabrice Gasnier <fabrice.gasnier@...s.st.com>
Acked-by: Minas Harutyunyan <hminas@...opsys.com>
>---
> drivers/usb/dwc2/gadget.c | 6 ++----
> drivers/usb/dwc2/platform.c | 3 +--
> 2 files changed, 3 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index
>62fa6378d2d7..8b15742d9e8a 100644
>--- a/drivers/usb/dwc2/gadget.c
>+++ b/drivers/usb/dwc2/gadget.c
>@@ -4549,8 +4549,7 @@ static int dwc2_hsotg_udc_start(struct usb_gadget
>*gadget,
> hsotg->gadget.dev.of_node = hsotg->dev->of_node;
> hsotg->gadget.speed = USB_SPEED_UNKNOWN;
>
>- if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL ||
>- (hsotg->dr_mode == USB_DR_MODE_OTG && dwc2_is_device_mode(hsotg)))
>{
>+ if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL) {
> ret = dwc2_lowlevel_hw_enable(hsotg);
> if (ret)
> goto err;
>@@ -4612,8 +4611,7 @@ static int dwc2_hsotg_udc_stop(struct usb_gadget
>*gadget)
> if (!IS_ERR_OR_NULL(hsotg->uphy))
> otg_set_peripheral(hsotg->uphy->otg, NULL);
>
>- if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL ||
>- (hsotg->dr_mode == USB_DR_MODE_OTG && dwc2_is_device_mode(hsotg)))
>+ if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL)
> dwc2_lowlevel_hw_disable(hsotg);
>
> return 0;
>diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c index
>23ef75996823..262c13b6362a 100644
>--- a/drivers/usb/dwc2/platform.c
>+++ b/drivers/usb/dwc2/platform.c
>@@ -576,8 +576,7 @@ static int dwc2_driver_probe(struct platform_device
>*dev)
> dwc2_debugfs_init(hsotg);
>
> /* Gadget code manages lowlevel hw on its own */
>- if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL ||
>- (hsotg->dr_mode == USB_DR_MODE_OTG && dwc2_is_device_mode(hsotg)))
>+ if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL)
> dwc2_lowlevel_hw_disable(hsotg);
>
> #if IS_ENABLED(CONFIG_USB_DWC2_PERIPHERAL) || \
>--
>2.25.1
Powered by blists - more mailing lists