* [PATCH] mailbox: pcc: Handle all PCC subtypes correctly in pcc_mbox_irq
@ 2021-12-09 9:21 Sudeep Holla
2021-12-22 13:40 ` Sudeep Holla
0 siblings, 1 reply; 3+ messages in thread
From: Sudeep Holla @ 2021-12-09 9:21 UTC (permalink / raw)
To: linux-kernel; +Cc: Sudeep Holla, linux-acpi, Jassi Brar, Justin He
Commit c45ded7e1135 ("mailbox: pcc: Add support for PCCT extended PCC
subspaces(type 3/4)") enabled the type3/4 of PCCT, but the change in
pcc_mbox_irq breaks the other PCC subtypes.
The kernel reports a warning on an Ampere eMag server
-->8
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.16.0-rc4 #127
Hardware name: MiTAC RAPTOR EV-883832-X3-0001/RAPTOR, BIOS 0.14 02/22/2019
Call trace:
dump_backtrace+0x0/0x200
show_stack+0x20/0x30
dump_stack_lvl+0x68/0x84
dump_stack+0x18/0x34
__report_bad_irq+0x54/0x17c
note_interrupt+0x330/0x428
handle_irq_event_percpu+0x90/0x98
handle_irq_event+0x4c/0x148
handle_fasteoi_irq+0xc4/0x188
generic_handle_domain_irq+0x44/0x68
gic_handle_irq+0x84/0x2ec
call_on_irq_stack+0x28/0x34
do_interrupt_handler+0x88/0x90
el1_interrupt+0x48/0xb0
el1h_64_irq_handler+0x18/0x28
el1h_64_irq+0x7c/0x80
---
The main reason for that is the command complete register is read as 0
if the GAS register doesn't exist for the same which is the case for
PCC subtypes 0-2. Fix it by checking for non-zero value before masking
with the status flag and checking for command completion.
Fixes: c45ded7e1135 ("mailbox: pcc: Add support for PCCT extended PCC subspaces(type 3/4)")
Cc: Jassi Brar <jassisinghbrar@gmail.com>
Reported-by: Justin He <justin.he@arm.com>
Tested-by: Justin He <justin.he@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/mailbox/pcc.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index e0a1ab3861f0..ed18936b8ce6 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -241,9 +241,11 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
if (ret)
return IRQ_NONE;
- val &= pchan->cmd_complete.status_mask;
- if (!val)
- return IRQ_NONE;
+ if (val) { /* Ensure GAS exists and value is non-zero */
+ val &= pchan->cmd_complete.status_mask;
+ if (!val)
+ return IRQ_NONE;
+ }
ret = pcc_chan_reg_read(&pchan->error, &val);
if (ret)
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] mailbox: pcc: Handle all PCC subtypes correctly in pcc_mbox_irq
2021-12-09 9:21 [PATCH] mailbox: pcc: Handle all PCC subtypes correctly in pcc_mbox_irq Sudeep Holla
@ 2021-12-22 13:40 ` Sudeep Holla
0 siblings, 0 replies; 3+ messages in thread
From: Sudeep Holla @ 2021-12-22 13:40 UTC (permalink / raw)
To: linux-kernel, Jassi Brar; +Cc: linux-acpi, Justin He, Sudeep Holla
Hi Jassi,
On Thu, Dec 09, 2021 at 09:21:46AM +0000, Sudeep Holla wrote:
> Commit c45ded7e1135 ("mailbox: pcc: Add support for PCCT extended PCC
> subspaces(type 3/4)") enabled the type3/4 of PCCT, but the change in
> pcc_mbox_irq breaks the other PCC subtypes.
>
> The kernel reports a warning on an Ampere eMag server
>
> -->8
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.16.0-rc4 #127
> Hardware name: MiTAC RAPTOR EV-883832-X3-0001/RAPTOR, BIOS 0.14 02/22/2019
> Call trace:
> dump_backtrace+0x0/0x200
> show_stack+0x20/0x30
> dump_stack_lvl+0x68/0x84
> dump_stack+0x18/0x34
> __report_bad_irq+0x54/0x17c
> note_interrupt+0x330/0x428
> handle_irq_event_percpu+0x90/0x98
> handle_irq_event+0x4c/0x148
> handle_fasteoi_irq+0xc4/0x188
> generic_handle_domain_irq+0x44/0x68
> gic_handle_irq+0x84/0x2ec
> call_on_irq_stack+0x28/0x34
> do_interrupt_handler+0x88/0x90
> el1_interrupt+0x48/0xb0
> el1h_64_irq_handler+0x18/0x28
> el1h_64_irq+0x7c/0x80
> ---
>
> The main reason for that is the command complete register is read as 0
> if the GAS register doesn't exist for the same which is the case for
> PCC subtypes 0-2. Fix it by checking for non-zero value before masking
> with the status flag and checking for command completion.
>
> Fixes: c45ded7e1135 ("mailbox: pcc: Add support for PCCT extended PCC subspaces(type 3/4)")
Can you take this patch and [1] for v5.17 ? This one is a bug fix and good
to get it merged ASAP.
--
Regards,
Sudeep
[1] https://lore.kernel.org/r/20211209082143.619354-1-sudeep.holla@arm.com
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] mailbox: pcc: Handle all PCC subtypes correctly in pcc_mbox_irq
@ 2021-12-11 1:32 kernel test robot
0 siblings, 0 replies; 3+ messages in thread
From: kernel test robot @ 2021-12-11 1:32 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 15916 bytes --]
CC: llvm(a)lists.linux.dev
CC: kbuild-all(a)lists.01.org
In-Reply-To: <20211209092146.620024-1-sudeep.holla@arm.com>
References: <20211209092146.620024-1-sudeep.holla@arm.com>
TO: Sudeep Holla <sudeep.holla@arm.com>
TO: linux-kernel(a)vger.kernel.org
CC: Sudeep Holla <sudeep.holla@arm.com>
CC: linux-acpi(a)vger.kernel.org
CC: Jassi Brar <jassisinghbrar@gmail.com>
CC: Justin He <justin.he@arm.com>
Hi Sudeep,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linux/master]
[also build test WARNING on fujitsu-integration/mailbox-for-next linus/master v5.16-rc4 next-20211208]
[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]
url: https://github.com/0day-ci/linux/commits/Sudeep-Holla/mailbox-pcc-Handle-all-PCC-subtypes-correctly-in-pcc_mbox_irq/20211209-172331
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 136057256686de39cc3a07c2e39ef6bc43003ff6
:::::: branch date: 2 days ago
:::::: commit date: 2 days ago
config: x86_64-randconfig-c007-20211209 (https://download.01.org/0day-ci/archive/20211211/202112110925.83I7jqKe-lkp(a)intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 097a1cb1d5ebb3a0ec4bcaed8ba3ff6a8e33c00a)
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/0day-ci/linux/commit/a5c9987836ae9937053ca154bc04e6f178cf2f94
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Sudeep-Holla/mailbox-pcc-Handle-all-PCC-subtypes-correctly-in-pcc_mbox_irq/20211209-172331
git checkout a5c9987836ae9937053ca154bc04e6f178cf2f94
# save the config file to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 clang-analyzer
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
clang-analyzer warnings: (new ones prefixed by >>)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/reada.c:122:6: note: 'err' is -5
if (err)
^~~
fs/btrfs/reada.c:122:2: note: Taking true branch
if (err)
^
fs/btrfs/reada.c:123:3: note: Control jumps to line 181
goto cleanup;
^
fs/btrfs/reada.c:181:2: note: Loop condition is true. Entering loop body
while (!list_empty(&list)) {
^
fs/btrfs/reada.c:191:7: note: Assuming the condition is false
if (atomic_dec_and_test(&rc->elems)) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/reada.c:191:3: note: Taking false branch
if (atomic_dec_and_test(&rc->elems)) {
^
fs/btrfs/reada.c:181:2: note: Loop condition is true. Entering loop body
while (!list_empty(&list)) {
^
fs/btrfs/reada.c:188:3: note: Memory is released
kfree(rec);
^~~~~~~~~~
fs/btrfs/reada.c:191:7: note: Assuming the condition is false
if (atomic_dec_and_test(&rc->elems)) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/reada.c:191:3: note: Taking false branch
if (atomic_dec_and_test(&rc->elems)) {
^
fs/btrfs/reada.c:181:2: note: Loop condition is true. Entering loop body
while (!list_empty(&list)) {
^
fs/btrfs/reada.c:186:3: note: Calling 'list_del'
list_del(&rec->list);
^~~~~~~~~~~~~~~~~~~~
include/linux/list.h:149:14: note: Use of memory after it is freed
entry->next = LIST_POISON1;
~~~~~~~~~~~ ^
Suppressed 7 warnings (7 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
4 warnings generated.
Suppressed 4 warnings (4 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
8 warnings generated.
drivers/mailbox/pcc.c:194:6: warning: The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage [clang-analyzer-core.uninitialized.Assign]
val &= reg->preserve_mask;
^
drivers/mailbox/pcc.c:373:8: note: Calling 'pcc_chan_reg_read_modify_write'
ret = pcc_chan_reg_read_modify_write(&pchan->cmd_update);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/mailbox/pcc.c:188:2: note: 'val' declared without an initial value
u64 val;
^~~~~~~
drivers/mailbox/pcc.c:190:8: note: Calling 'pcc_chan_reg_read'
ret = pcc_chan_reg_read(reg, &val);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/mailbox/pcc.c:157:6: note: Assuming field 'gas' is non-null, which participates in a condition later
if (!reg->gas) {
^~~~~~~~~
drivers/mailbox/pcc.c:157:2: note: Taking false branch
if (!reg->gas) {
^
drivers/mailbox/pcc.c:162:6: note: Assuming field 'vaddr' is non-null
if (reg->vaddr)
^~~~~~~~~~
drivers/mailbox/pcc.c:162:2: note: Taking true branch
if (reg->vaddr)
^
drivers/mailbox/pcc.c:163:3: note: Calling 'read_register'
read_register(reg->vaddr, val, reg->gas->bit_width);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/mailbox/pcc.c:119:2: note: 'Default' branch taken. Execution continues on line 119
switch (bit_width) {
^
drivers/mailbox/pcc.c:133:1: note: Returning without writing to '*val'
}
^
drivers/mailbox/pcc.c:163:3: note: Returning from 'read_register'
read_register(reg->vaddr, val, reg->gas->bit_width);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/mailbox/pcc.c:167:2: note: Returning without writing to '*val'
return ret;
^
drivers/mailbox/pcc.c:167:2: note: Returning zero (loaded from 'ret'), which participates in a condition later
return ret;
^~~~~~~~~~
drivers/mailbox/pcc.c:190:8: note: Returning from 'pcc_chan_reg_read'
ret = pcc_chan_reg_read(reg, &val);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/mailbox/pcc.c:191:6: note: 'ret' is 0
if (ret)
^~~
drivers/mailbox/pcc.c:191:2: note: Taking false branch
if (ret)
^
drivers/mailbox/pcc.c:194:6: note: The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage
val &= reg->preserve_mask;
~~~ ^
>> drivers/mailbox/pcc.c:244:6: warning: Branch condition evaluates to a garbage value [clang-analyzer-core.uninitialized.Branch]
if (val) { /* Ensure GAS exists and value is non-zero */
^~~
drivers/mailbox/pcc.c:235:2: note: 'val' declared without an initial value
u64 val;
^~~~~~~
drivers/mailbox/pcc.c:240:8: note: Calling 'pcc_chan_reg_read'
ret = pcc_chan_reg_read(&pchan->cmd_complete, &val);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/mailbox/pcc.c:157:6: note: Assuming field 'gas' is non-null, which participates in a condition later
if (!reg->gas) {
^~~~~~~~~
drivers/mailbox/pcc.c:157:2: note: Taking false branch
if (!reg->gas) {
^
drivers/mailbox/pcc.c:162:6: note: Assuming field 'vaddr' is non-null
if (reg->vaddr)
^~~~~~~~~~
drivers/mailbox/pcc.c:162:2: note: Taking true branch
if (reg->vaddr)
^
drivers/mailbox/pcc.c:163:3: note: Calling 'read_register'
read_register(reg->vaddr, val, reg->gas->bit_width);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/mailbox/pcc.c:119:2: note: 'Default' branch taken. Execution continues on line 119
switch (bit_width) {
^
drivers/mailbox/pcc.c:133:1: note: Returning without writing to '*val'
}
^
drivers/mailbox/pcc.c:163:3: note: Returning from 'read_register'
read_register(reg->vaddr, val, reg->gas->bit_width);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/mailbox/pcc.c:167:2: note: Returning without writing to '*val'
return ret;
^
drivers/mailbox/pcc.c:167:2: note: Returning zero (loaded from 'ret'), which participates in a condition later
return ret;
^~~~~~~~~~
drivers/mailbox/pcc.c:240:8: note: Returning from 'pcc_chan_reg_read'
ret = pcc_chan_reg_read(&pchan->cmd_complete, &val);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/mailbox/pcc.c:241:6: note: 'ret' is 0
if (ret)
^~~
drivers/mailbox/pcc.c:241:2: note: Taking false branch
if (ret)
^
drivers/mailbox/pcc.c:244:6: note: Branch condition evaluates to a garbage value
if (val) { /* Ensure GAS exists and value is non-zero */
^~~
drivers/mailbox/pcc.c:294:3: warning: 1st function call argument is an uninitialized value [clang-analyzer-core.CallAndMessage]
dev_err(dev, "Channel not found for idx: %d\n", subspace_id);
^
include/linux/dev_printk.h:144:24: note: expanded from macro 'dev_err'
dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
^ ~~~
include/linux/dev_printk.h:110:3: note: expanded from macro 'dev_printk_index_wrap'
_p_func(dev, fmt, ##__VA_ARGS__); \
^ ~~~
drivers/mailbox/pcc.c:285:2: note: 'dev' declared without an initial value
struct device *dev;
^~~~~~~~~~~~~~~~~~
drivers/mailbox/pcc.c:288:6: note: Assuming 'subspace_id' is >= 0
if (subspace_id < 0 || subspace_id >= pcc_chan_count)
^~~~~~~~~~~~~~~
drivers/mailbox/pcc.c:288:6: note: Left side of '||' is false
drivers/mailbox/pcc.c:288:25: note: Assuming 'subspace_id' is < 'pcc_chan_count'
if (subspace_id < 0 || subspace_id >= pcc_chan_count)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/mailbox/pcc.c:288:2: note: Taking false branch
if (subspace_id < 0 || subspace_id >= pcc_chan_count)
^
drivers/mailbox/pcc.c:293:6: note: Calling 'IS_ERR'
if (IS_ERR(chan) || chan->cl) {
^~~~~~~~~~~~
include/linux/err.h:36:9: note: Assuming the condition is true
return IS_ERR_VALUE((unsigned long)ptr);
^
include/linux/err.h:22:34: note: expanded from macro 'IS_ERR_VALUE'
#define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned long)-MAX_ERRNO)
~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/compiler.h:78:42: note: expanded from macro 'unlikely'
# define unlikely(x) __builtin_expect(!!(x), 0)
^
include/linux/err.h:36:2: note: Returning the value 1, which participates in a condition later
return IS_ERR_VALUE((unsigned long)ptr);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/mailbox/pcc.c:293:6: note: Returning from 'IS_ERR'
if (IS_ERR(chan) || chan->cl) {
^~~~~~~~~~~~
drivers/mailbox/pcc.c:293:19: note: Left side of '||' is true
if (IS_ERR(chan) || chan->cl) {
^
drivers/mailbox/pcc.c:294:3: note: Loop condition is false. Exiting loop
dev_err(dev, "Channel not found for idx: %d\n", subspace_id);
^
include/linux/dev_printk.h:144:2: note: expanded from macro 'dev_err'
dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
^
include/linux/dev_printk.h:109:3: note: expanded from macro 'dev_printk_index_wrap'
vim +244 drivers/mailbox/pcc.c
aca314efb17727 hotran 2016-08-15 223
aca314efb17727 hotran 2016-08-15 224 /**
aca314efb17727 hotran 2016-08-15 225 * pcc_mbox_irq - PCC mailbox interrupt handler
10dcc2d66292f9 Sudeep Holla 2021-09-17 226 * @irq: interrupt number
10dcc2d66292f9 Sudeep Holla 2021-09-17 227 * @p: data/cookie passed from the caller to identify the channel
10dcc2d66292f9 Sudeep Holla 2021-09-17 228 *
10dcc2d66292f9 Sudeep Holla 2021-09-17 229 * Returns: IRQ_HANDLED if interrupt is handled or IRQ_NONE if not
aca314efb17727 hotran 2016-08-15 230 */
aca314efb17727 hotran 2016-08-15 231 static irqreturn_t pcc_mbox_irq(int irq, void *p)
aca314efb17727 hotran 2016-08-15 232 {
80b2bdde002c52 Sudeep Holla 2021-09-17 233 struct pcc_chan_info *pchan;
aca314efb17727 hotran 2016-08-15 234 struct mbox_chan *chan = p;
c45ded7e11352d Sudeep Holla 2021-09-17 235 u64 val;
c45ded7e11352d Sudeep Holla 2021-09-17 236 int ret;
aca314efb17727 hotran 2016-08-15 237
bf18123e78f4d1 Sudeep Holla 2021-09-17 238 pchan = chan->con_priv;
aca314efb17727 hotran 2016-08-15 239
c45ded7e11352d Sudeep Holla 2021-09-17 240 ret = pcc_chan_reg_read(&pchan->cmd_complete, &val);
c45ded7e11352d Sudeep Holla 2021-09-17 241 if (ret)
c45ded7e11352d Sudeep Holla 2021-09-17 242 return IRQ_NONE;
c45ded7e11352d Sudeep Holla 2021-09-17 243
a5c9987836ae99 Sudeep Holla 2021-12-09 @244 if (val) { /* Ensure GAS exists and value is non-zero */
c45ded7e11352d Sudeep Holla 2021-09-17 245 val &= pchan->cmd_complete.status_mask;
c45ded7e11352d Sudeep Holla 2021-09-17 246 if (!val)
c45ded7e11352d Sudeep Holla 2021-09-17 247 return IRQ_NONE;
a5c9987836ae99 Sudeep Holla 2021-12-09 248 }
c45ded7e11352d Sudeep Holla 2021-09-17 249
c45ded7e11352d Sudeep Holla 2021-09-17 250 ret = pcc_chan_reg_read(&pchan->error, &val);
c45ded7e11352d Sudeep Holla 2021-09-17 251 if (ret)
c45ded7e11352d Sudeep Holla 2021-09-17 252 return IRQ_NONE;
c45ded7e11352d Sudeep Holla 2021-09-17 253 val &= pchan->error.status_mask;
c45ded7e11352d Sudeep Holla 2021-09-17 254 if (val) {
c45ded7e11352d Sudeep Holla 2021-09-17 255 val &= ~pchan->error.status_mask;
c45ded7e11352d Sudeep Holla 2021-09-17 256 pcc_chan_reg_write(&pchan->error, val);
c45ded7e11352d Sudeep Holla 2021-09-17 257 return IRQ_NONE;
c45ded7e11352d Sudeep Holla 2021-09-17 258 }
c45ded7e11352d Sudeep Holla 2021-09-17 259
bf18123e78f4d1 Sudeep Holla 2021-09-17 260 if (pcc_chan_reg_read_modify_write(&pchan->plat_irq_ack))
aca314efb17727 hotran 2016-08-15 261 return IRQ_NONE;
aca314efb17727 hotran 2016-08-15 262
bf18123e78f4d1 Sudeep Holla 2021-09-17 263 mbox_chan_received_data(chan, NULL);
aca314efb17727 hotran 2016-08-15 264
aca314efb17727 hotran 2016-08-15 265 return IRQ_HANDLED;
aca314efb17727 hotran 2016-08-15 266 }
aca314efb17727 hotran 2016-08-15 267
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-12-22 13:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-09 9:21 [PATCH] mailbox: pcc: Handle all PCC subtypes correctly in pcc_mbox_irq Sudeep Holla
2021-12-22 13:40 ` Sudeep Holla
2021-12-11 1:32 kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.