| 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: <20230407015833.3x6fdw2rwemsul7t@synopsys.com>
Date: Fri, 7 Apr 2023 01:58:48 +0000
From: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To: Johan Hovold <johan+linaro@...nel.org>
CC: Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 05/11] USB: dwc3: drop dead hibernation code
On Tue, Apr 04, 2023, Johan Hovold wrote:
> The hibernation code is broken and has never been enabled in mainline
> and should thus be dropped.
Hibernation is not implemented in the kernel mainline. We only have some
small stub/placeholder code for it. It's fine to remove these.
>
> Specifically, the scratch buffer DMA mapping would have been leaked on
> every suspend cycle since commit 51f5d49ad6f0 ("usb: dwc3: core:
> simplify suspend/resume operations") if this feature was ever enabled.
>
> The related error handling was also broken and could have resulted in
> attempts to unmap never mapped buffers, etc.
>
> This effectively revert commit 0ffcaf3798bf ("usb: dwc3: core: allocate
> scratch buffers").
>
> Signed-off-by: Johan Hovold <johan+linaro@...nel.org>
> ---
> drivers/usb/dwc3/core.c | 103 +---------------------------------------
> drivers/usb/dwc3/core.h | 8 ----
> 2 files changed, 1 insertion(+), 110 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 5b362ed43e7e..d837ab511686 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -534,90 +534,6 @@ void dwc3_event_buffers_cleanup(struct dwc3 *dwc)
> dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), 0);
> }
>
> -static int dwc3_alloc_scratch_buffers(struct dwc3 *dwc)
> -{
> - if (!dwc->has_hibernation)
> - return 0;
> -
> - if (!dwc->nr_scratch)
> - return 0;
> -
> - dwc->scratchbuf = kmalloc_array(dwc->nr_scratch,
> - DWC3_SCRATCHBUF_SIZE, GFP_KERNEL);
> - if (!dwc->scratchbuf)
> - return -ENOMEM;
> -
> - return 0;
> -}
> -
> -static int dwc3_setup_scratch_buffers(struct dwc3 *dwc)
> -{
> - dma_addr_t scratch_addr;
> - u32 param;
> - int ret;
> -
> - if (!dwc->has_hibernation)
> - return 0;
> -
> - if (!dwc->nr_scratch)
> - return 0;
> -
> - /* should never fall here */
> - if (!WARN_ON(dwc->scratchbuf))
> - return 0;
> -
> - scratch_addr = dma_map_single(dwc->sysdev, dwc->scratchbuf,
> - dwc->nr_scratch * DWC3_SCRATCHBUF_SIZE,
> - DMA_BIDIRECTIONAL);
> - if (dma_mapping_error(dwc->sysdev, scratch_addr)) {
> - dev_err(dwc->sysdev, "failed to map scratch buffer\n");
> - ret = -EFAULT;
> - goto err0;
> - }
> -
> - dwc->scratch_addr = scratch_addr;
> -
> - param = lower_32_bits(scratch_addr);
> -
> - ret = dwc3_send_gadget_generic_command(dwc,
> - DWC3_DGCMD_SET_SCRATCHPAD_ADDR_LO, param);
> - if (ret < 0)
> - goto err1;
> -
> - param = upper_32_bits(scratch_addr);
> -
> - ret = dwc3_send_gadget_generic_command(dwc,
> - DWC3_DGCMD_SET_SCRATCHPAD_ADDR_HI, param);
> - if (ret < 0)
> - goto err1;
> -
> - return 0;
> -
> -err1:
> - dma_unmap_single(dwc->sysdev, dwc->scratch_addr, dwc->nr_scratch *
> - DWC3_SCRATCHBUF_SIZE, DMA_BIDIRECTIONAL);
> -
> -err0:
> - return ret;
> -}
> -
> -static void dwc3_free_scratch_buffers(struct dwc3 *dwc)
> -{
> - if (!dwc->has_hibernation)
> - return;
> -
> - if (!dwc->nr_scratch)
> - return;
> -
> - /* should never fall here */
> - if (!WARN_ON(dwc->scratchbuf))
> - return;
> -
> - dma_unmap_single(dwc->sysdev, dwc->scratch_addr, dwc->nr_scratch *
> - DWC3_SCRATCHBUF_SIZE, DMA_BIDIRECTIONAL);
> - kfree(dwc->scratchbuf);
> -}
> -
> static void dwc3_core_num_eps(struct dwc3 *dwc)
> {
> struct dwc3_hwparams *parms = &dwc->hwparams;
> @@ -877,7 +793,6 @@ static bool dwc3_core_is_valid(struct dwc3 *dwc)
>
> static void dwc3_core_setup_global_control(struct dwc3 *dwc)
> {
> - u32 hwparams4 = dwc->hwparams.hwparams4;
> u32 reg;
>
> reg = dwc3_readl(dwc->regs, DWC3_GCTL);
> @@ -905,9 +820,6 @@ static void dwc3_core_setup_global_control(struct dwc3 *dwc)
> reg &= ~DWC3_GCTL_DSBLCLKGTNG;
> break;
> case DWC3_GHWPARAMS1_EN_PWROPT_HIB:
> - /* enable hibernation here */
> - dwc->nr_scratch = DWC3_GHWPARAMS4_HIBER_SCRATCHBUFS(hwparams4);
> -
> /*
> * REVISIT Enabling this bit so that host-mode hibernation
> * will work. Device-mode hibernation is not yet implemented.
> @@ -1151,10 +1063,6 @@ static int dwc3_core_init(struct dwc3 *dwc)
> dwc3_core_setup_global_control(dwc);
> dwc3_core_num_eps(dwc);
>
> - ret = dwc3_setup_scratch_buffers(dwc);
> - if (ret)
> - goto err1;
> -
> /* Set power down scale of suspend_clk */
> dwc3_set_power_down_clk_scale(dwc);
>
> @@ -1908,14 +1816,10 @@ static int dwc3_probe(struct platform_device *pdev)
> if (ret)
> goto err3;
>
> - ret = dwc3_alloc_scratch_buffers(dwc);
> - if (ret)
> - goto err3;
> -
> ret = dwc3_core_init(dwc);
> if (ret) {
> dev_err_probe(dev, ret, "failed to initialize core\n");
> - goto err4;
> + goto err3;
> }
>
> dwc3_check_params(dwc);
> @@ -1944,10 +1848,6 @@ static int dwc3_probe(struct platform_device *pdev)
> phy_exit(dwc->usb3_generic_phy);
>
> dwc3_ulpi_exit(dwc);
> -
> -err4:
> - dwc3_free_scratch_buffers(dwc);
> -
> err3:
> dwc3_free_event_buffers(dwc);
>
> @@ -1987,7 +1887,6 @@ static int dwc3_remove(struct platform_device *pdev)
> pm_runtime_set_suspended(&pdev->dev);
>
> dwc3_free_event_buffers(dwc);
> - dwc3_free_scratch_buffers(dwc);
>
> if (dwc->usb_psy)
> power_supply_put(dwc->usb_psy);
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 4743e918dcaf..fbbc565d3b34 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -969,12 +969,10 @@ struct dwc3_scratchpad_array {
> * @drd_work: workqueue used for role swapping
> * @ep0_trb: trb which is used for the ctrl_req
> * @bounce: address of bounce buffer
> - * @scratchbuf: address of scratch buffer
> * @setup_buf: used while precessing STD USB requests
> * @ep0_trb_addr: dma address of @ep0_trb
> * @bounce_addr: dma address of @bounce
> * @ep0_usb_req: dummy req used while handling STD USB requests
> - * @scratch_addr: dma address of scratchbuf
> * @ep0_in_setup: one control transfer is completed and enter setup phase
> * @lock: for synchronizing
> * @mutex: for mode switching
> @@ -999,7 +997,6 @@ struct dwc3_scratchpad_array {
> * @current_otg_role: current role of operation while using the OTG block
> * @desired_otg_role: desired role of operation while using the OTG block
> * @otg_restart_host: flag that OTG controller needs to restart host
> - * @nr_scratch: number of scratch buffers
> * @u1u2: only used on revisions <1.83a for workaround
> * @maximum_speed: maximum speed requested (mainly for testing purposes)
> * @max_ssp_rate: SuperSpeed Plus maximum signaling rate and lane count
> @@ -1056,7 +1053,6 @@ struct dwc3_scratchpad_array {
> * @delayed_status: true when gadget driver asks for delayed status
> * @ep0_bounced: true when we used bounce buffer
> * @ep0_expect_in: true when we expect a DATA IN transfer
> - * @has_hibernation: true when dwc3 was configured with Hibernation
> * @sysdev_is_parent: true when dwc3 device has a parent driver
> * @has_lpm_erratum: true when core was configured with LPM Erratum. Note that
> * there's now way for software to detect this in runtime.
> @@ -1123,11 +1119,9 @@ struct dwc3 {
> struct work_struct drd_work;
> struct dwc3_trb *ep0_trb;
> void *bounce;
> - void *scratchbuf;
> u8 *setup_buf;
> dma_addr_t ep0_trb_addr;
> dma_addr_t bounce_addr;
> - dma_addr_t scratch_addr;
> struct dwc3_request ep0_usb_req;
> struct completion ep0_in_setup;
>
> @@ -1187,7 +1181,6 @@ struct dwc3 {
> u32 current_otg_role;
> u32 desired_otg_role;
> bool otg_restart_host;
> - u32 nr_scratch;
> u32 u1u2;
> u32 maximum_speed;
> u32 gadget_max_speed;
> @@ -1284,7 +1277,6 @@ struct dwc3 {
> unsigned delayed_status:1;
> unsigned ep0_bounced:1;
> unsigned ep0_expect_in:1;
> - unsigned has_hibernation:1;
> unsigned sysdev_is_parent:1;
> unsigned has_lpm_erratum:1;
> unsigned is_utmi_l1_suspend:1;
> --
> 2.39.2
>
Acked-by: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
Thanks,
Thinh
Powered by blists - more mailing lists