Re: [PATCH v9] usb: dwc3: debugfs: Prevent any register access when devices is runtime suspended

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, May 05, 2023, Udipto Goswami wrote:
> When the dwc3 device is runtime suspended, various required clocks would
> get disabled and it is not guaranteed that access to any registers would
> work. Depending on the SoC glue, a register read could be as benign as
> returning 0 or be fatal enough to hang the system.
> 
> In order to prevent such scenarios of fatal errors, make sure to resume
> dwc3 then allow the function to proceed.
> 
> Fixes: 62ba09d6bb63 ("usb: dwc3: debugfs: Dump internal LSP and ep registers")

This fix goes before the above change.

This also touches on many places and is more than 100 lines. While this
is a fix, I'm not sure if Cc stable is needed. Perhaps others can
comment.

Thanks,
Thinh

> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Udipto Goswami <quic_ugoswami@xxxxxxxxxxx>
> ---
> v9: Fixed function dwc3_rx_fifo_size_show & return values in function
> 	dwc3_link_state_write along with minor changes for code symmetry.
> v8: Replace pm_runtime_get_sync with pm_runtime_resume_and get.
> v7: Replaced pm_runtime_put with pm_runtime_put_sync & returned proper values.
> v6: Added changes to handle get_dync failure appropriately.
> v5: Reworked the patch to resume dwc3 while accessing the registers.
> v4: Introduced pm_runtime_get_if_in_use in order to make sure dwc3 isn't
> 	suspended while accessing the registers.
> v3: Replace pr_err to dev_err. 
> v2: Replaced return 0 with -EINVAL & seq_puts with pr_err.
> 
>  drivers/usb/dwc3/debugfs.c | 109 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 109 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
> index e4a2560b9dc0..ebf03468fac4 100644
> --- a/drivers/usb/dwc3/debugfs.c
> +++ b/drivers/usb/dwc3/debugfs.c
> @@ -332,6 +332,11 @@ static int dwc3_lsp_show(struct seq_file *s, void *unused)
>  	unsigned int		current_mode;
>  	unsigned long		flags;
>  	u32			reg;
> +	int			ret;
> +
> +	ret = pm_runtime_resume_and_get(dwc->dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	spin_lock_irqsave(&dwc->lock, flags);
>  	reg = dwc3_readl(dwc->regs, DWC3_GSTS);
> @@ -350,6 +355,8 @@ static int dwc3_lsp_show(struct seq_file *s, void *unused)
>  	}
>  	spin_unlock_irqrestore(&dwc->lock, flags);
>  
> +	pm_runtime_put_sync(dwc->dev);
> +
>  	return 0;
>  }
>  
> @@ -395,6 +402,11 @@ static int dwc3_mode_show(struct seq_file *s, void *unused)
>  	struct dwc3		*dwc = s->private;
>  	unsigned long		flags;
>  	u32			reg;
> +	int			ret;
> +
> +	ret = pm_runtime_resume_and_get(dwc->dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	spin_lock_irqsave(&dwc->lock, flags);
>  	reg = dwc3_readl(dwc->regs, DWC3_GCTL);
> @@ -414,6 +426,8 @@ static int dwc3_mode_show(struct seq_file *s, void *unused)
>  		seq_printf(s, "UNKNOWN %08x\n", DWC3_GCTL_PRTCAP(reg));
>  	}
>  
> +	pm_runtime_put_sync(dwc->dev);
> +
>  	return 0;
>  }
>  
> @@ -463,6 +477,11 @@ static int dwc3_testmode_show(struct seq_file *s, void *unused)
>  	struct dwc3		*dwc = s->private;
>  	unsigned long		flags;
>  	u32			reg;
> +	int			ret;
> +
> +	ret = pm_runtime_resume_and_get(dwc->dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	spin_lock_irqsave(&dwc->lock, flags);
>  	reg = dwc3_readl(dwc->regs, DWC3_DCTL);
> @@ -493,6 +512,8 @@ static int dwc3_testmode_show(struct seq_file *s, void *unused)
>  		seq_printf(s, "UNKNOWN %d\n", reg);
>  	}
>  
> +	pm_runtime_put_sync(dwc->dev);
> +
>  	return 0;
>  }
>  
> @@ -509,6 +530,7 @@ static ssize_t dwc3_testmode_write(struct file *file,
>  	unsigned long		flags;
>  	u32			testmode = 0;
>  	char			buf[32];
> +	int			ret;
>  
>  	if (copy_from_user(&buf, ubuf, min_t(size_t, sizeof(buf) - 1, count)))
>  		return -EFAULT;
> @@ -526,10 +548,16 @@ static ssize_t dwc3_testmode_write(struct file *file,
>  	else
>  		testmode = 0;
>  
> +	ret = pm_runtime_resume_and_get(dwc->dev);
> +	if (ret < 0)
> +		return ret;
> +
>  	spin_lock_irqsave(&dwc->lock, flags);
>  	dwc3_gadget_set_test_mode(dwc, testmode);
>  	spin_unlock_irqrestore(&dwc->lock, flags);
>  
> +	pm_runtime_put_sync(dwc->dev);
> +
>  	return count;
>  }
>  
> @@ -548,12 +576,18 @@ static int dwc3_link_state_show(struct seq_file *s, void *unused)
>  	enum dwc3_link_state	state;
>  	u32			reg;
>  	u8			speed;
> +	int			ret;
> +
> +	ret = pm_runtime_resume_and_get(dwc->dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	spin_lock_irqsave(&dwc->lock, flags);
>  	reg = dwc3_readl(dwc->regs, DWC3_GSTS);
>  	if (DWC3_GSTS_CURMOD(reg) != DWC3_GSTS_CURMOD_DEVICE) {
>  		seq_puts(s, "Not available\n");
>  		spin_unlock_irqrestore(&dwc->lock, flags);
> +		pm_runtime_put_sync(dwc->dev);
>  		return 0;
>  	}
>  
> @@ -566,6 +600,8 @@ static int dwc3_link_state_show(struct seq_file *s, void *unused)
>  		   dwc3_gadget_hs_link_string(state));
>  	spin_unlock_irqrestore(&dwc->lock, flags);
>  
> +	pm_runtime_put_sync(dwc->dev);
> +
>  	return 0;
>  }
>  
> @@ -584,6 +620,7 @@ static ssize_t dwc3_link_state_write(struct file *file,
>  	char			buf[32];
>  	u32			reg;
>  	u8			speed;
> +	int			ret;
>  
>  	if (copy_from_user(&buf, ubuf, min_t(size_t, sizeof(buf) - 1, count)))
>  		return -EFAULT;
> @@ -603,10 +640,15 @@ static ssize_t dwc3_link_state_write(struct file *file,
>  	else
>  		return -EINVAL;
>  
> +	ret = pm_runtime_resume_and_get(dwc->dev);
> +	if (ret < 0)
> +		return ret;
> +
>  	spin_lock_irqsave(&dwc->lock, flags);
>  	reg = dwc3_readl(dwc->regs, DWC3_GSTS);
>  	if (DWC3_GSTS_CURMOD(reg) != DWC3_GSTS_CURMOD_DEVICE) {
>  		spin_unlock_irqrestore(&dwc->lock, flags);
> +		pm_runtime_put_sync(dwc->dev);
>  		return -EINVAL;
>  	}
>  
> @@ -616,12 +658,15 @@ static ssize_t dwc3_link_state_write(struct file *file,
>  	if (speed < DWC3_DSTS_SUPERSPEED &&
>  	    state != DWC3_LINK_STATE_RECOV) {
>  		spin_unlock_irqrestore(&dwc->lock, flags);
> +		pm_runtime_put_sync(dwc->dev);
>  		return -EINVAL;
>  	}
>  
>  	dwc3_gadget_set_link_state(dwc, state);
>  	spin_unlock_irqrestore(&dwc->lock, flags);
>  
> +	pm_runtime_put_sync(dwc->dev);
> +
>  	return count;
>  }
>  
> @@ -645,6 +690,11 @@ static int dwc3_tx_fifo_size_show(struct seq_file *s, void *unused)
>  	unsigned long		flags;
>  	u32			mdwidth;
>  	u32			val;
> +	int			ret;
> +
> +	ret = pm_runtime_resume_and_get(dwc->dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	spin_lock_irqsave(&dwc->lock, flags);
>  	val = dwc3_core_fifo_space(dep, DWC3_TXFIFO);
> @@ -657,6 +707,8 @@ static int dwc3_tx_fifo_size_show(struct seq_file *s, void *unused)
>  	seq_printf(s, "%u\n", val);
>  	spin_unlock_irqrestore(&dwc->lock, flags);
>  
> +	pm_runtime_put_sync(dwc->dev);
> +
>  	return 0;
>  }
>  
> @@ -667,6 +719,11 @@ static int dwc3_rx_fifo_size_show(struct seq_file *s, void *unused)
>  	unsigned long		flags;
>  	u32			mdwidth;
>  	u32			val;
> +	int			ret;
> +
> +	ret = pm_runtime_resume_and_get(dwc->dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	spin_lock_irqsave(&dwc->lock, flags);
>  	val = dwc3_core_fifo_space(dep, DWC3_RXFIFO);
> @@ -679,6 +736,8 @@ static int dwc3_rx_fifo_size_show(struct seq_file *s, void *unused)
>  	seq_printf(s, "%u\n", val);
>  	spin_unlock_irqrestore(&dwc->lock, flags);
>  
> +	pm_runtime_put_sync(dwc->dev);
> +
>  	return 0;
>  }
>  
> @@ -688,12 +747,19 @@ static int dwc3_tx_request_queue_show(struct seq_file *s, void *unused)
>  	struct dwc3		*dwc = dep->dwc;
>  	unsigned long		flags;
>  	u32			val;
> +	int			ret;
> +
> +	ret = pm_runtime_resume_and_get(dwc->dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	spin_lock_irqsave(&dwc->lock, flags);
>  	val = dwc3_core_fifo_space(dep, DWC3_TXREQQ);
>  	seq_printf(s, "%u\n", val);
>  	spin_unlock_irqrestore(&dwc->lock, flags);
>  
> +	pm_runtime_put_sync(dwc->dev);
> +
>  	return 0;
>  }
>  
> @@ -703,12 +769,19 @@ static int dwc3_rx_request_queue_show(struct seq_file *s, void *unused)
>  	struct dwc3		*dwc = dep->dwc;
>  	unsigned long		flags;
>  	u32			val;
> +	int			ret;
> +
> +	ret = pm_runtime_resume_and_get(dwc->dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	spin_lock_irqsave(&dwc->lock, flags);
>  	val = dwc3_core_fifo_space(dep, DWC3_RXREQQ);
>  	seq_printf(s, "%u\n", val);
>  	spin_unlock_irqrestore(&dwc->lock, flags);
>  
> +	pm_runtime_put_sync(dwc->dev);
> +
>  	return 0;
>  }
>  
> @@ -718,12 +791,19 @@ static int dwc3_rx_info_queue_show(struct seq_file *s, void *unused)
>  	struct dwc3		*dwc = dep->dwc;
>  	unsigned long		flags;
>  	u32			val;
> +	int			ret;
> +
> +	ret = pm_runtime_resume_and_get(dwc->dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	spin_lock_irqsave(&dwc->lock, flags);
>  	val = dwc3_core_fifo_space(dep, DWC3_RXINFOQ);
>  	seq_printf(s, "%u\n", val);
>  	spin_unlock_irqrestore(&dwc->lock, flags);
>  
> +	pm_runtime_put_sync(dwc->dev);
> +
>  	return 0;
>  }
>  
> @@ -733,12 +813,19 @@ static int dwc3_descriptor_fetch_queue_show(struct seq_file *s, void *unused)
>  	struct dwc3		*dwc = dep->dwc;
>  	unsigned long		flags;
>  	u32			val;
> +	int			ret;
> +
> +	ret = pm_runtime_resume_and_get(dwc->dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	spin_lock_irqsave(&dwc->lock, flags);
>  	val = dwc3_core_fifo_space(dep, DWC3_DESCFETCHQ);
>  	seq_printf(s, "%u\n", val);
>  	spin_unlock_irqrestore(&dwc->lock, flags);
>  
> +	pm_runtime_put_sync(dwc->dev);
> +
>  	return 0;
>  }
>  
> @@ -748,12 +835,19 @@ static int dwc3_event_queue_show(struct seq_file *s, void *unused)
>  	struct dwc3		*dwc = dep->dwc;
>  	unsigned long		flags;
>  	u32			val;
> +	int			ret;
> +
> +	ret = pm_runtime_resume_and_get(dwc->dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	spin_lock_irqsave(&dwc->lock, flags);
>  	val = dwc3_core_fifo_space(dep, DWC3_EVENTQ);
>  	seq_printf(s, "%u\n", val);
>  	spin_unlock_irqrestore(&dwc->lock, flags);
>  
> +	pm_runtime_put_sync(dwc->dev);
> +
>  	return 0;
>  }
>  
> @@ -798,6 +892,11 @@ static int dwc3_trb_ring_show(struct seq_file *s, void *unused)
>  	struct dwc3		*dwc = dep->dwc;
>  	unsigned long		flags;
>  	int			i;
> +	int			ret;
> +
> +	ret = pm_runtime_resume_and_get(dwc->dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	spin_lock_irqsave(&dwc->lock, flags);
>  	if (dep->number <= 1) {
> @@ -827,6 +926,8 @@ static int dwc3_trb_ring_show(struct seq_file *s, void *unused)
>  out:
>  	spin_unlock_irqrestore(&dwc->lock, flags);
>  
> +	pm_runtime_put_sync(dwc->dev);
> +
>  	return 0;
>  }
>  
> @@ -839,6 +940,11 @@ static int dwc3_ep_info_register_show(struct seq_file *s, void *unused)
>  	u32			lower_32_bits;
>  	u32			upper_32_bits;
>  	u32			reg;
> +	int			ret;
> +
> +	ret = pm_runtime_resume_and_get(dwc->dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	spin_lock_irqsave(&dwc->lock, flags);
>  	reg = DWC3_GDBGLSPMUX_EPSELECT(dep->number);
> @@ -851,6 +957,8 @@ static int dwc3_ep_info_register_show(struct seq_file *s, void *unused)
>  	seq_printf(s, "0x%016llx\n", ep_info);
>  	spin_unlock_irqrestore(&dwc->lock, flags);
>  
> +	pm_runtime_put_sync(dwc->dev);
> +
>  	return 0;
>  }
>  
> @@ -910,6 +1018,7 @@ void dwc3_debugfs_init(struct dwc3 *dwc)
>  	dwc->regset->regs = dwc3_regs;
>  	dwc->regset->nregs = ARRAY_SIZE(dwc3_regs);
>  	dwc->regset->base = dwc->regs - DWC3_GLOBALS_REGS_START;
> +	dwc->regset->dev = dwc->dev;
>  
>  	root = debugfs_create_dir(dev_name(dwc->dev), usb_debug_root);
>  	dwc->debug_root = root;
> -- 
> 2.17.1
> 




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux