* [PATCH v8] usb: dwc3: debugfs: Prevent any register access when devices is runtime suspended
@ 2023-05-04 4:50 Udipto Goswami
2023-05-04 7:28 ` kernel test robot
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Udipto Goswami @ 2023-05-04 4:50 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman
Cc: Pratham Pratap, Jack Pham, Johan Hovold, Oliver Neukum, linux-usb,
Udipto Goswami
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.
Signed-off-by: Udipto Goswami <quic_ugoswami@quicinc.com>
---
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 | 99 +++++++++++++++++++++++++++++++++++++-
1 file changed, 97 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
index e4a2560b9dc0..a996e3580150 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);
@@ -349,6 +354,7 @@ static int dwc3_lsp_show(struct seq_file *s, void *unused)
break;
}
spin_unlock_irqrestore(&dwc->lock, flags);
+ pm_runtime_put_sync(dwc->dev);
return 0;
}
@@ -395,6 +401,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 +425,7 @@ 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 +475,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 +510,7 @@ 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 +527,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 +545,15 @@ 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 +572,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 +596,7 @@ 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 +615,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,11 +635,16 @@ 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);
- return -EINVAL;
+ pm_runtime_put_sync(dwc->dev);
+ return ret;
}
reg = dwc3_readl(dwc->regs, DWC3_DSTS);
@@ -616,12 +653,14 @@ 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);
- return -EINVAL;
+ pm_runtime_put_sync(dwc->dev);
+ return ret;
}
dwc3_gadget_set_link_state(dwc, state);
spin_unlock_irqrestore(&dwc->lock, flags);
+ pm_runtime_put_sync(dwc->dev);
return count;
}
@@ -645,6 +684,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 +701,7 @@ 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 +712,12 @@ 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)
+ pm_runtime_put_sync(dwc->dev);
+ return ret;
spin_lock_irqsave(&dwc->lock, flags);
val = dwc3_core_fifo_space(dep, DWC3_RXFIFO);
@@ -679,6 +730,7 @@ 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 +740,18 @@ 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 +761,18 @@ 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 +782,18 @@ 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 +803,18 @@ 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 +824,18 @@ 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 +880,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 +914,7 @@ 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 +927,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 +944,7 @@ 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 +1004,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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v8] usb: dwc3: debugfs: Prevent any register access when devices is runtime suspended
2023-05-04 4:50 [PATCH v8] usb: dwc3: debugfs: Prevent any register access when devices is runtime suspended Udipto Goswami
@ 2023-05-04 7:28 ` kernel test robot
2023-05-04 8:16 ` Johan Hovold
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2023-05-04 7:28 UTC (permalink / raw)
To: Udipto Goswami, Thinh Nguyen, Greg Kroah-Hartman
Cc: oe-kbuild-all, Pratham Pratap, Jack Pham, Johan Hovold,
Oliver Neukum, linux-usb, Udipto Goswami
Hi Udipto,
kernel test robot noticed the following build warnings:
[auto build test WARNING on usb/usb-testing]
[also build test WARNING on usb/usb-next usb/usb-linus driver-core/driver-core-testing driver-core/driver-core-next driver-core/driver-core-linus johan-usb-serial/usb-next johan-usb-serial/usb-linus linus/master v6.3 next-20230428]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Udipto-Goswami/usb-dwc3-debugfs-Prevent-any-register-access-when-devices-is-runtime-suspended/20230504-125225
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link: https://lore.kernel.org/r/20230504045052.22347-1-quic_ugoswami%40quicinc.com
patch subject: [PATCH v8] usb: dwc3: debugfs: Prevent any register access when devices is runtime suspended
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230504/202305041526.hBiM1g2W-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/1c53edaeee33380f0fc3e0d262829ffaa66f45e1
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Udipto-Goswami/usb-dwc3-debugfs-Prevent-any-register-access-when-devices-is-runtime-suspended/20230504-125225
git checkout 1c53edaeee33380f0fc3e0d262829ffaa66f45e1
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/usb/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202305041526.hBiM1g2W-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/usb/dwc3/debugfs.c: In function 'dwc3_rx_fifo_size_show':
>> drivers/usb/dwc3/debugfs.c:718:9: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
718 | if (ret < 0)
| ^~
drivers/usb/dwc3/debugfs.c:720:17: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
720 | return ret;
| ^~~~~~
vim +/if +718 drivers/usb/dwc3/debugfs.c
707
708 static int dwc3_rx_fifo_size_show(struct seq_file *s, void *unused)
709 {
710 struct dwc3_ep *dep = s->private;
711 struct dwc3 *dwc = dep->dwc;
712 unsigned long flags;
713 u32 mdwidth;
714 u32 val;
715 int ret;
716
717 ret = pm_runtime_resume_and_get(dwc->dev);
> 718 if (ret < 0)
719 pm_runtime_put_sync(dwc->dev);
720 return ret;
721
722 spin_lock_irqsave(&dwc->lock, flags);
723 val = dwc3_core_fifo_space(dep, DWC3_RXFIFO);
724
725 /* Convert to bytes */
726 mdwidth = dwc3_mdwidth(dwc);
727
728 val *= mdwidth;
729 val >>= 3;
730 seq_printf(s, "%u\n", val);
731 spin_unlock_irqrestore(&dwc->lock, flags);
732
733 pm_runtime_put_sync(dwc->dev);
734 return 0;
735 }
736
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v8] usb: dwc3: debugfs: Prevent any register access when devices is runtime suspended
2023-05-04 4:50 [PATCH v8] usb: dwc3: debugfs: Prevent any register access when devices is runtime suspended Udipto Goswami
2023-05-04 7:28 ` kernel test robot
@ 2023-05-04 8:16 ` Johan Hovold
2023-05-09 22:04 ` kernel test robot
2023-05-10 7:32 ` Dan Carpenter
3 siblings, 0 replies; 6+ messages in thread
From: Johan Hovold @ 2023-05-04 8:16 UTC (permalink / raw)
To: Udipto Goswami
Cc: Thinh Nguyen, Greg Kroah-Hartman, Pratham Pratap, Jack Pham,
Johan Hovold, Oliver Neukum, linux-usb
Please use a more succinct Subject. The current one is over 80 chars
which generally too long. Please also check the grammar (e.g. "devices
is") and make sure that the subject reflects what your patch does (and
not what it did in v1).
On Thu, May 04, 2023 at 10:20:52AM +0530, 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.
As this fixes a crash/hang you should also add a Fixes and CC-stable
tag.
> Signed-off-by: Udipto Goswami <quic_ugoswami@quicinc.com>
> ---
> 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 | 99 +++++++++++++++++++++++++++++++++++++-
> 1 file changed, 97 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
> index e4a2560b9dc0..a996e3580150 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);
> @@ -349,6 +354,7 @@ static int dwc3_lsp_show(struct seq_file *s, void *unused)
> break;
> }
> spin_unlock_irqrestore(&dwc->lock, flags);
Add a newline here for symmetry.
> + pm_runtime_put_sync(dwc->dev);
>
> return 0;
> }
> @@ -395,6 +401,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 +425,7 @@ 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);
For consistency and readability, add a newline here before the return
statement. Same below.
> return 0;
> }
>
> @@ -584,6 +615,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,11 +635,16 @@ 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);
> - return -EINVAL;
> + pm_runtime_put_sync(dwc->dev);
> + return ret;
Why are you changing the return value here? That's simply wrong.
> }
>
> reg = dwc3_readl(dwc->regs, DWC3_DSTS);
> @@ -616,12 +653,14 @@ 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);
> - return -EINVAL;
> + pm_runtime_put_sync(dwc->dev);
> + return ret;
Same here.
> }
>
> dwc3_gadget_set_link_state(dwc, state);
> spin_unlock_irqrestore(&dwc->lock, flags);
>
> + pm_runtime_put_sync(dwc->dev);
> return count;
> }
>
> @@ -667,6 +712,12 @@ 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)
> + pm_runtime_put_sync(dwc->dev);
> + return ret;
As the build bot reported, you forgot to remove pm_runtime_put_sync()
here which means that you just broke this function which now always
returns some random stack data.
>
> spin_lock_irqsave(&dwc->lock, flags);
> val = dwc3_core_fifo_space(dep, DWC3_RXFIFO);
> @@ -679,6 +730,7 @@ 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;
> }
Johan
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v8] usb: dwc3: debugfs: Prevent any register access when devices is runtime suspended
2023-05-04 4:50 [PATCH v8] usb: dwc3: debugfs: Prevent any register access when devices is runtime suspended Udipto Goswami
2023-05-04 7:28 ` kernel test robot
2023-05-04 8:16 ` Johan Hovold
@ 2023-05-09 22:04 ` kernel test robot
2023-05-10 7:32 ` Dan Carpenter
3 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2023-05-09 22:04 UTC (permalink / raw)
To: Udipto Goswami, Thinh Nguyen, Greg Kroah-Hartman
Cc: llvm, oe-kbuild-all, Pratham Pratap, Jack Pham, Johan Hovold,
Oliver Neukum, linux-usb, Udipto Goswami
Hi Udipto,
kernel test robot noticed the following build warnings:
[auto build test WARNING on usb/usb-testing]
[also build test WARNING on usb/usb-next usb/usb-linus driver-core/driver-core-testing driver-core/driver-core-next driver-core/driver-core-linus johan-usb-serial/usb-next johan-usb-serial/usb-linus linus/master v6.4-rc1 next-20230509]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Udipto-Goswami/usb-dwc3-debugfs-Prevent-any-register-access-when-devices-is-runtime-suspended/20230504-125225
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link: https://lore.kernel.org/r/20230504045052.22347-1-quic_ugoswami%40quicinc.com
patch subject: [PATCH v8] usb: dwc3: debugfs: Prevent any register access when devices is runtime suspended
config: hexagon-randconfig-r024-20230509 (https://download.01.org/0day-ci/archive/20230510/202305100533.3s1xdH64-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project b0fb98227c90adf2536c9ad644a74d5e92961111)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/1c53edaeee33380f0fc3e0d262829ffaa66f45e1
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Udipto-Goswami/usb-dwc3-debugfs-Prevent-any-register-access-when-devices-is-runtime-suspended/20230504-125225
git checkout 1c53edaeee33380f0fc3e0d262829ffaa66f45e1
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/usb/dwc3/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202305100533.3s1xdH64-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/usb/dwc3/debugfs.c:23:
In file included from drivers/usb/dwc3/core.h:20:
In file included from include/linux/dma-mapping.h:10:
In file included from include/linux/scatterlist.h:9:
In file included from arch/hexagon/include/asm/io.h:334:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __raw_readb(PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
#define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
^
In file included from drivers/usb/dwc3/debugfs.c:23:
In file included from drivers/usb/dwc3/core.h:20:
In file included from include/linux/dma-mapping.h:10:
In file included from include/linux/scatterlist.h:9:
In file included from arch/hexagon/include/asm/io.h:334:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
^
In file included from drivers/usb/dwc3/debugfs.c:23:
In file included from drivers/usb/dwc3/core.h:20:
In file included from include/linux/dma-mapping.h:10:
In file included from include/linux/scatterlist.h:9:
In file included from arch/hexagon/include/asm/io.h:334:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
>> drivers/usb/dwc3/debugfs.c:720:3: warning: misleading indentation; statement is not part of the previous 'if' [-Wmisleading-indentation]
return ret;
^
drivers/usb/dwc3/debugfs.c:718:2: note: previous statement is here
if (ret < 0)
^
7 warnings generated.
vim +/if +720 drivers/usb/dwc3/debugfs.c
707
708 static int dwc3_rx_fifo_size_show(struct seq_file *s, void *unused)
709 {
710 struct dwc3_ep *dep = s->private;
711 struct dwc3 *dwc = dep->dwc;
712 unsigned long flags;
713 u32 mdwidth;
714 u32 val;
715 int ret;
716
717 ret = pm_runtime_resume_and_get(dwc->dev);
718 if (ret < 0)
719 pm_runtime_put_sync(dwc->dev);
> 720 return ret;
721
722 spin_lock_irqsave(&dwc->lock, flags);
723 val = dwc3_core_fifo_space(dep, DWC3_RXFIFO);
724
725 /* Convert to bytes */
726 mdwidth = dwc3_mdwidth(dwc);
727
728 val *= mdwidth;
729 val >>= 3;
730 seq_printf(s, "%u\n", val);
731 spin_unlock_irqrestore(&dwc->lock, flags);
732
733 pm_runtime_put_sync(dwc->dev);
734 return 0;
735 }
736
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v8] usb: dwc3: debugfs: Prevent any register access when devices is runtime suspended
2023-05-04 4:50 [PATCH v8] usb: dwc3: debugfs: Prevent any register access when devices is runtime suspended Udipto Goswami
` (2 preceding siblings ...)
2023-05-09 22:04 ` kernel test robot
@ 2023-05-10 7:32 ` Dan Carpenter
2023-05-10 7:50 ` Johan Hovold
3 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2023-05-10 7:32 UTC (permalink / raw)
To: oe-kbuild, Udipto Goswami, Thinh Nguyen, Greg Kroah-Hartman
Cc: lkp, oe-kbuild-all, Pratham Pratap, Jack Pham, Johan Hovold,
Oliver Neukum, linux-usb, Udipto Goswami
Hi Udipto,
kernel test robot noticed the following build warnings:
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Udipto-Goswami/usb-dwc3-debugfs-Prevent-any-register-access-when-devices-is-runtime-suspended/20230504-125225
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link: https://lore.kernel.org/r/20230504045052.22347-1-quic_ugoswami%40quicinc.com
patch subject: [PATCH v8] usb: dwc3: debugfs: Prevent any register access when devices is runtime suspended
config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20230510/202305101451.V2D0cM4S-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Link: https://lore.kernel.org/r/202305101451.V2D0cM4S-lkp@intel.com/
New smatch warnings:
drivers/usb/dwc3/debugfs.c:647 dwc3_link_state_write() warn: missing error code? 'ret'
drivers/usb/dwc3/debugfs.c:720 dwc3_rx_fifo_size_show() warn: curly braces intended?
drivers/usb/dwc3/debugfs.c:722 dwc3_rx_fifo_size_show() warn: inconsistent indenting
drivers/usb/dwc3/debugfs.c:722 dwc3_rx_fifo_size_show() warn: ignoring unreachable code.
Old smatch warnings:
drivers/usb/dwc3/debugfs.c:657 dwc3_link_state_write() warn: missing error code? 'ret'
vim +/ret +647 drivers/usb/dwc3/debugfs.c
1c53edaeee3338 Udipto Goswami 2023-05-04 638 ret = pm_runtime_resume_and_get(dwc->dev);
1c53edaeee3338 Udipto Goswami 2023-05-04 639 if (ret < 0)
1c53edaeee3338 Udipto Goswami 2023-05-04 640 return ret;
1c53edaeee3338 Udipto Goswami 2023-05-04 641
138801aaa566ec Felipe Balbi 2012-01-02 642 spin_lock_irqsave(&dwc->lock, flags);
d102444cac1564 Thinh Nguyen 2018-11-07 643 reg = dwc3_readl(dwc->regs, DWC3_GSTS);
d102444cac1564 Thinh Nguyen 2018-11-07 644 if (DWC3_GSTS_CURMOD(reg) != DWC3_GSTS_CURMOD_DEVICE) {
d102444cac1564 Thinh Nguyen 2018-11-07 645 spin_unlock_irqrestore(&dwc->lock, flags);
1c53edaeee3338 Udipto Goswami 2023-05-04 646 pm_runtime_put_sync(dwc->dev);
1c53edaeee3338 Udipto Goswami 2023-05-04 @647 return ret;
ret is not necessarily an error code.
d102444cac1564 Thinh Nguyen 2018-11-07 648 }
d102444cac1564 Thinh Nguyen 2018-11-07 649
0d36dede457873 Thinh Nguyen 2018-11-07 650 reg = dwc3_readl(dwc->regs, DWC3_DSTS);
0d36dede457873 Thinh Nguyen 2018-11-07 651 speed = reg & DWC3_DSTS_CONNECTSPD;
0d36dede457873 Thinh Nguyen 2018-11-07 652
0d36dede457873 Thinh Nguyen 2018-11-07 653 if (speed < DWC3_DSTS_SUPERSPEED &&
0d36dede457873 Thinh Nguyen 2018-11-07 654 state != DWC3_LINK_STATE_RECOV) {
0d36dede457873 Thinh Nguyen 2018-11-07 655 spin_unlock_irqrestore(&dwc->lock, flags);
1c53edaeee3338 Udipto Goswami 2023-05-04 656 pm_runtime_put_sync(dwc->dev);
1c53edaeee3338 Udipto Goswami 2023-05-04 657 return ret;
0d36dede457873 Thinh Nguyen 2018-11-07 658 }
[ snip ]
2c85a1817e4ba0 Thinh Nguyen 2018-11-07 708 static int dwc3_rx_fifo_size_show(struct seq_file *s, void *unused)
818ec3aba883f5 Felipe Balbi 2016-04-14 709 {
818ec3aba883f5 Felipe Balbi 2016-04-14 710 struct dwc3_ep *dep = s->private;
818ec3aba883f5 Felipe Balbi 2016-04-14 711 struct dwc3 *dwc = dep->dwc;
818ec3aba883f5 Felipe Balbi 2016-04-14 712 unsigned long flags;
d00be779cc5016 Thinh Nguyen 2021-03-27 713 u32 mdwidth;
818ec3aba883f5 Felipe Balbi 2016-04-14 714 u32 val;
1c53edaeee3338 Udipto Goswami 2023-05-04 715 int ret;
1c53edaeee3338 Udipto Goswami 2023-05-04 716
1c53edaeee3338 Udipto Goswami 2023-05-04 717 ret = pm_runtime_resume_and_get(dwc->dev);
1c53edaeee3338 Udipto Goswami 2023-05-04 718 if (ret < 0)
1c53edaeee3338 Udipto Goswami 2023-05-04 719 pm_runtime_put_sync(dwc->dev);
1c53edaeee3338 Udipto Goswami 2023-05-04 @720 return ret;
Needs curly braces.
818ec3aba883f5 Felipe Balbi 2016-04-14 721
818ec3aba883f5 Felipe Balbi 2016-04-14 @722 spin_lock_irqsave(&dwc->lock, flags);
2c85a1817e4ba0 Thinh Nguyen 2018-11-07 723 val = dwc3_core_fifo_space(dep, DWC3_RXFIFO);
0f874f79dc81ae Thinh Nguyen 2018-11-07 724
0f874f79dc81ae Thinh Nguyen 2018-11-07 725 /* Convert to bytes */
d00be779cc5016 Thinh Nguyen 2021-03-27 726 mdwidth = dwc3_mdwidth(dwc);
4244ba02edb850 Thinh Nguyen 2020-04-11 727
4244ba02edb850 Thinh Nguyen 2020-04-11 728 val *= mdwidth;
0f874f79dc81ae Thinh Nguyen 2018-11-07 729 val >>= 3;
818ec3aba883f5 Felipe Balbi 2016-04-14 730 seq_printf(s, "%u\n", val);
818ec3aba883f5 Felipe Balbi 2016-04-14 731 spin_unlock_irqrestore(&dwc->lock, flags);
818ec3aba883f5 Felipe Balbi 2016-04-14 732
1c53edaeee3338 Udipto Goswami 2023-05-04 733 pm_runtime_put_sync(dwc->dev);
818ec3aba883f5 Felipe Balbi 2016-04-14 734 return 0;
818ec3aba883f5 Felipe Balbi 2016-04-14 735 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v8] usb: dwc3: debugfs: Prevent any register access when devices is runtime suspended
2023-05-10 7:32 ` Dan Carpenter
@ 2023-05-10 7:50 ` Johan Hovold
0 siblings, 0 replies; 6+ messages in thread
From: Johan Hovold @ 2023-05-10 7:50 UTC (permalink / raw)
To: Dan Carpenter
Cc: oe-kbuild, Udipto Goswami, Thinh Nguyen, Greg Kroah-Hartman, lkp,
oe-kbuild-all, Pratham Pratap, Jack Pham, Johan Hovold,
Oliver Neukum, linux-usb
On Wed, May 10, 2023 at 10:32:54AM +0300, Dan Carpenter wrote:
> Hi Udipto,
>
> kernel test robot noticed the following build warnings:
>
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Udipto-Goswami/usb-dwc3-debugfs-Prevent-any-register-access-when-devices-is-runtime-suspended/20230504-125225
> base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
> patch link: https://lore.kernel.org/r/20230504045052.22347-1-quic_ugoswami%40quicinc.com
> patch subject: [PATCH v8] usb: dwc3: debugfs: Prevent any register access when devices is runtime suspended
> config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20230510/202305101451.V2D0cM4S-lkp@intel.com/config)
> compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <error27@gmail.com>
> | Link: https://lore.kernel.org/r/202305101451.V2D0cM4S-lkp@intel.com/
>
> New smatch warnings:
> drivers/usb/dwc3/debugfs.c:647 dwc3_link_state_write() warn: missing error code? 'ret'
> drivers/usb/dwc3/debugfs.c:720 dwc3_rx_fifo_size_show() warn: curly braces intended?
> drivers/usb/dwc3/debugfs.c:722 dwc3_rx_fifo_size_show() warn: inconsistent indenting
> drivers/usb/dwc3/debugfs.c:722 dwc3_rx_fifo_size_show() warn: ignoring unreachable code.
>
> Old smatch warnings:
> drivers/usb/dwc3/debugfs.c:657 dwc3_link_state_write() warn: missing error code? 'ret'
>
> vim +/ret +647 drivers/usb/dwc3/debugfs.c
>
> 1c53edaeee3338 Udipto Goswami 2023-05-04 638 ret = pm_runtime_resume_and_get(dwc->dev);
> 1c53edaeee3338 Udipto Goswami 2023-05-04 639 if (ret < 0)
> 1c53edaeee3338 Udipto Goswami 2023-05-04 640 return ret;
> 1c53edaeee3338 Udipto Goswami 2023-05-04 641
> 138801aaa566ec Felipe Balbi 2012-01-02 642 spin_lock_irqsave(&dwc->lock, flags);
> d102444cac1564 Thinh Nguyen 2018-11-07 643 reg = dwc3_readl(dwc->regs, DWC3_GSTS);
> d102444cac1564 Thinh Nguyen 2018-11-07 644 if (DWC3_GSTS_CURMOD(reg) != DWC3_GSTS_CURMOD_DEVICE) {
> d102444cac1564 Thinh Nguyen 2018-11-07 645 spin_unlock_irqrestore(&dwc->lock, flags);
> 1c53edaeee3338 Udipto Goswami 2023-05-04 646 pm_runtime_put_sync(dwc->dev);
> 1c53edaeee3338 Udipto Goswami 2023-05-04 @647 return ret;
>
> ret is not necessarily an error code.
>
> d102444cac1564 Thinh Nguyen 2018-11-07 648 }
> d102444cac1564 Thinh Nguyen 2018-11-07 649
> 0d36dede457873 Thinh Nguyen 2018-11-07 650 reg = dwc3_readl(dwc->regs, DWC3_DSTS);
> 0d36dede457873 Thinh Nguyen 2018-11-07 651 speed = reg & DWC3_DSTS_CONNECTSPD;
> 0d36dede457873 Thinh Nguyen 2018-11-07 652
> 0d36dede457873 Thinh Nguyen 2018-11-07 653 if (speed < DWC3_DSTS_SUPERSPEED &&
> 0d36dede457873 Thinh Nguyen 2018-11-07 654 state != DWC3_LINK_STATE_RECOV) {
> 0d36dede457873 Thinh Nguyen 2018-11-07 655 spin_unlock_irqrestore(&dwc->lock, flags);
> 1c53edaeee3338 Udipto Goswami 2023-05-04 656 pm_runtime_put_sync(dwc->dev);
> 1c53edaeee3338 Udipto Goswami 2023-05-04 657 return ret;
> 0d36dede457873 Thinh Nguyen 2018-11-07 658 }
>
> [ snip ]
>
> 2c85a1817e4ba0 Thinh Nguyen 2018-11-07 708 static int dwc3_rx_fifo_size_show(struct seq_file *s, void *unused)
> 818ec3aba883f5 Felipe Balbi 2016-04-14 709 {
> 818ec3aba883f5 Felipe Balbi 2016-04-14 710 struct dwc3_ep *dep = s->private;
> 818ec3aba883f5 Felipe Balbi 2016-04-14 711 struct dwc3 *dwc = dep->dwc;
> 818ec3aba883f5 Felipe Balbi 2016-04-14 712 unsigned long flags;
> d00be779cc5016 Thinh Nguyen 2021-03-27 713 u32 mdwidth;
> 818ec3aba883f5 Felipe Balbi 2016-04-14 714 u32 val;
> 1c53edaeee3338 Udipto Goswami 2023-05-04 715 int ret;
> 1c53edaeee3338 Udipto Goswami 2023-05-04 716
> 1c53edaeee3338 Udipto Goswami 2023-05-04 717 ret = pm_runtime_resume_and_get(dwc->dev);
> 1c53edaeee3338 Udipto Goswami 2023-05-04 718 if (ret < 0)
> 1c53edaeee3338 Udipto Goswami 2023-05-04 719 pm_runtime_put_sync(dwc->dev);
> 1c53edaeee3338 Udipto Goswami 2023-05-04 @720 return ret;
>
> Needs curly braces.
I believe this was all fixed in v9/v10.
Johan
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-05-10 7:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-04 4:50 [PATCH v8] usb: dwc3: debugfs: Prevent any register access when devices is runtime suspended Udipto Goswami
2023-05-04 7:28 ` kernel test robot
2023-05-04 8:16 ` Johan Hovold
2023-05-09 22:04 ` kernel test robot
2023-05-10 7:32 ` Dan Carpenter
2023-05-10 7:50 ` Johan Hovold
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox