* [pci:for-linus] BUILD SUCCESS WITH WARNING 15ac366c3d20ce1e08173f1de393a8ce95a1facf
@ 2021-06-18 10:34 kernel test robot
2021-06-18 13:13 ` Bjorn Helgaas
0 siblings, 1 reply; 7+ messages in thread
From: kernel test robot @ 2021-06-18 10:34 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: linux-pci
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git for-linus
branch HEAD: 15ac366c3d20ce1e08173f1de393a8ce95a1facf PCI: aardvark: Fix kernel panic during PIO transfer
possible Warning in current branch:
drivers/pci/controller/dwc/pcie-tegra194.c:1829:23: warning: Shifting signed 32-bit value by 31 bits is undefined behaviour. See condition at line 1826. [shiftTooManyBitsSigned]
Warning ids grouped by kconfigs:
gcc_recent_errors
`-- m68k-randconfig-p001-20210617
`-- drivers-pci-controller-dwc-pcie-tegra194.c:warning:Shifting-signed-bit-value-by-bits-is-undefined-behaviour.-See-condition-at-line-.-shiftTooManyBitsSigned
elapsed time: 723m
configs tested: 127
configs skipped: 2
gcc tested configs:
arm defconfig
arm64 allyesconfig
arm64 defconfig
arm allyesconfig
arm allmodconfig
arm davinci_all_defconfig
sh se7750_defconfig
mips loongson1c_defconfig
arm lpc32xx_defconfig
mips ath79_defconfig
arm lpd270_defconfig
arm64 alldefconfig
arm keystone_defconfig
riscv rv32_defconfig
arm trizeps4_defconfig
sh rts7751r2d1_defconfig
arm omap2plus_defconfig
s390 debug_defconfig
mips mpc30x_defconfig
powerpc bluestone_defconfig
nios2 alldefconfig
powerpc adder875_defconfig
powerpc mpc834x_itxgp_defconfig
arm ep93xx_defconfig
mips loongson1b_defconfig
sh sh03_defconfig
powerpc pseries_defconfig
riscv nommu_k210_sdcard_defconfig
arm pxa910_defconfig
sh lboxre2_defconfig
arm at91_dt_defconfig
arm tct_hammer_defconfig
arm pxa255-idp_defconfig
arm multi_v5_defconfig
sh se7722_defconfig
powerpc klondike_defconfig
powerpc eiger_defconfig
ia64 zx1_defconfig
sh kfr2r09-romimage_defconfig
powerpc pcm030_defconfig
sh r7785rp_defconfig
xtensa smp_lx200_defconfig
m68k sun3x_defconfig
arm cerfcube_defconfig
powerpc tqm8xx_defconfig
m68k q40_defconfig
powerpc mpc834x_mds_defconfig
powerpc ppc44x_defconfig
x86_64 allnoconfig
ia64 allmodconfig
ia64 defconfig
ia64 allyesconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
arc allyesconfig
nds32 allnoconfig
nds32 defconfig
nios2 allyesconfig
csky defconfig
alpha defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
arc defconfig
sh allmodconfig
parisc defconfig
s390 allyesconfig
s390 allmodconfig
parisc allyesconfig
s390 defconfig
i386 allyesconfig
sparc allyesconfig
sparc defconfig
i386 defconfig
mips allyesconfig
mips allmodconfig
powerpc allyesconfig
powerpc allmodconfig
powerpc allnoconfig
i386 randconfig-a002-20210618
i386 randconfig-a006-20210618
i386 randconfig-a004-20210618
i386 randconfig-a001-20210618
i386 randconfig-a005-20210618
i386 randconfig-a003-20210618
x86_64 randconfig-a015-20210618
x86_64 randconfig-a011-20210618
x86_64 randconfig-a012-20210618
x86_64 randconfig-a014-20210618
x86_64 randconfig-a016-20210618
x86_64 randconfig-a013-20210618
i386 randconfig-a015-20210618
i386 randconfig-a016-20210618
i386 randconfig-a013-20210618
i386 randconfig-a014-20210618
i386 randconfig-a012-20210618
i386 randconfig-a011-20210618
i386 randconfig-a015-20210617
i386 randconfig-a013-20210617
i386 randconfig-a016-20210617
i386 randconfig-a012-20210617
i386 randconfig-a014-20210617
i386 randconfig-a011-20210617
riscv nommu_k210_defconfig
riscv allyesconfig
riscv nommu_virt_defconfig
riscv allnoconfig
riscv defconfig
riscv allmodconfig
x86_64 rhel-8.3-kselftests
um x86_64_defconfig
um i386_defconfig
um kunit_defconfig
x86_64 allyesconfig
x86_64 defconfig
x86_64 rhel-8.3
x86_64 rhel-8.3-kbuiltin
x86_64 kexec
clang tested configs:
x86_64 randconfig-b001-20210618
x86_64 randconfig-a002-20210618
x86_64 randconfig-a001-20210618
x86_64 randconfig-a004-20210618
x86_64 randconfig-a003-20210618
x86_64 randconfig-a006-20210618
x86_64 randconfig-a005-20210618
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pci:for-linus] BUILD SUCCESS WITH WARNING 15ac366c3d20ce1e08173f1de393a8ce95a1facf
2021-06-18 10:34 [pci:for-linus] BUILD SUCCESS WITH WARNING 15ac366c3d20ce1e08173f1de393a8ce95a1facf kernel test robot
@ 2021-06-18 13:13 ` Bjorn Helgaas
2021-06-18 14:03 ` Jon Hunter
0 siblings, 1 reply; 7+ messages in thread
From: Bjorn Helgaas @ 2021-06-18 13:13 UTC (permalink / raw)
To: kernel test robot, Jon Hunter; +Cc: linux-pci, Thierry Reding
[+to Jon, +cc Thierry]
On Fri, Jun 18, 2021 at 06:34:20PM +0800, kernel test robot wrote:
> tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git for-linus
> branch HEAD: 15ac366c3d20ce1e08173f1de393a8ce95a1facf PCI: aardvark: Fix kernel panic during PIO transfer
>
> possible Warning in current branch:
>
> drivers/pci/controller/dwc/pcie-tegra194.c:1829:23: warning: Shifting signed 32-bit value by 31 bits is undefined behaviour. See condition at line 1826. [shiftTooManyBitsSigned]
This looks like a legit warning, but I think the only commit that
could be related is this one:
https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=for-linus&id=99ab5996278379a02d5a84c4a7ac33a2ebfdb29e
which doesn't touch that code.
It does *move* some code, so maybe this was an existing warning that
moved enough that the robot thought it was new?
Or is this a new compiler version or were more warnings turned on?
How can we reproduce this to make sure we fix it?
> Warning ids grouped by kconfigs:
>
> gcc_recent_errors
> `-- m68k-randconfig-p001-20210617
> `-- drivers-pci-controller-dwc-pcie-tegra194.c:warning:Shifting-signed-bit-value-by-bits-is-undefined-behaviour.-See-condition-at-line-.-shiftTooManyBitsSigned
>
> elapsed time: 723m
>
> configs tested: 127
> configs skipped: 2
>
> gcc tested configs:
> arm defconfig
> arm64 allyesconfig
> arm64 defconfig
> arm allyesconfig
> arm allmodconfig
> arm davinci_all_defconfig
> sh se7750_defconfig
> mips loongson1c_defconfig
> arm lpc32xx_defconfig
> mips ath79_defconfig
> arm lpd270_defconfig
> arm64 alldefconfig
> arm keystone_defconfig
> riscv rv32_defconfig
> arm trizeps4_defconfig
> sh rts7751r2d1_defconfig
> arm omap2plus_defconfig
> s390 debug_defconfig
> mips mpc30x_defconfig
> powerpc bluestone_defconfig
> nios2 alldefconfig
> powerpc adder875_defconfig
> powerpc mpc834x_itxgp_defconfig
> arm ep93xx_defconfig
> mips loongson1b_defconfig
> sh sh03_defconfig
> powerpc pseries_defconfig
> riscv nommu_k210_sdcard_defconfig
> arm pxa910_defconfig
> sh lboxre2_defconfig
> arm at91_dt_defconfig
> arm tct_hammer_defconfig
> arm pxa255-idp_defconfig
> arm multi_v5_defconfig
> sh se7722_defconfig
> powerpc klondike_defconfig
> powerpc eiger_defconfig
> ia64 zx1_defconfig
> sh kfr2r09-romimage_defconfig
> powerpc pcm030_defconfig
> sh r7785rp_defconfig
> xtensa smp_lx200_defconfig
> m68k sun3x_defconfig
> arm cerfcube_defconfig
> powerpc tqm8xx_defconfig
> m68k q40_defconfig
> powerpc mpc834x_mds_defconfig
> powerpc ppc44x_defconfig
> x86_64 allnoconfig
> ia64 allmodconfig
> ia64 defconfig
> ia64 allyesconfig
> m68k allmodconfig
> m68k defconfig
> m68k allyesconfig
> nios2 defconfig
> arc allyesconfig
> nds32 allnoconfig
> nds32 defconfig
> nios2 allyesconfig
> csky defconfig
> alpha defconfig
> alpha allyesconfig
> xtensa allyesconfig
> h8300 allyesconfig
> arc defconfig
> sh allmodconfig
> parisc defconfig
> s390 allyesconfig
> s390 allmodconfig
> parisc allyesconfig
> s390 defconfig
> i386 allyesconfig
> sparc allyesconfig
> sparc defconfig
> i386 defconfig
> mips allyesconfig
> mips allmodconfig
> powerpc allyesconfig
> powerpc allmodconfig
> powerpc allnoconfig
> i386 randconfig-a002-20210618
> i386 randconfig-a006-20210618
> i386 randconfig-a004-20210618
> i386 randconfig-a001-20210618
> i386 randconfig-a005-20210618
> i386 randconfig-a003-20210618
> x86_64 randconfig-a015-20210618
> x86_64 randconfig-a011-20210618
> x86_64 randconfig-a012-20210618
> x86_64 randconfig-a014-20210618
> x86_64 randconfig-a016-20210618
> x86_64 randconfig-a013-20210618
> i386 randconfig-a015-20210618
> i386 randconfig-a016-20210618
> i386 randconfig-a013-20210618
> i386 randconfig-a014-20210618
> i386 randconfig-a012-20210618
> i386 randconfig-a011-20210618
> i386 randconfig-a015-20210617
> i386 randconfig-a013-20210617
> i386 randconfig-a016-20210617
> i386 randconfig-a012-20210617
> i386 randconfig-a014-20210617
> i386 randconfig-a011-20210617
> riscv nommu_k210_defconfig
> riscv allyesconfig
> riscv nommu_virt_defconfig
> riscv allnoconfig
> riscv defconfig
> riscv allmodconfig
> x86_64 rhel-8.3-kselftests
> um x86_64_defconfig
> um i386_defconfig
> um kunit_defconfig
> x86_64 allyesconfig
> x86_64 defconfig
> x86_64 rhel-8.3
> x86_64 rhel-8.3-kbuiltin
> x86_64 kexec
>
> clang tested configs:
> x86_64 randconfig-b001-20210618
> x86_64 randconfig-a002-20210618
> x86_64 randconfig-a001-20210618
> x86_64 randconfig-a004-20210618
> x86_64 randconfig-a003-20210618
> x86_64 randconfig-a006-20210618
> x86_64 randconfig-a005-20210618
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pci:for-linus] BUILD SUCCESS WITH WARNING 15ac366c3d20ce1e08173f1de393a8ce95a1facf
2021-06-18 13:13 ` Bjorn Helgaas
@ 2021-06-18 14:03 ` Jon Hunter
2021-06-18 14:48 ` Thierry Reding
0 siblings, 1 reply; 7+ messages in thread
From: Jon Hunter @ 2021-06-18 14:03 UTC (permalink / raw)
To: Bjorn Helgaas, kernel test robot, Vidya Sagar; +Cc: linux-pci, Thierry Reding
Hi Bjorn,
On 18/06/2021 14:13, Bjorn Helgaas wrote:
> [+to Jon, +cc Thierry]
>
> On Fri, Jun 18, 2021 at 06:34:20PM +0800, kernel test robot wrote:
>> tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git for-linus
>> branch HEAD: 15ac366c3d20ce1e08173f1de393a8ce95a1facf PCI: aardvark: Fix kernel panic during PIO transfer
>>
>> possible Warning in current branch:
>>
>> drivers/pci/controller/dwc/pcie-tegra194.c:1829:23: warning: Shifting signed 32-bit value by 31 bits is undefined behaviour. See condition at line 1826. [shiftTooManyBitsSigned]
>
> This looks like a legit warning, but I think the only commit that
> could be related is this one:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=for-linus&id=99ab5996278379a02d5a84c4a7ac33a2ebfdb29e
>
> which doesn't touch that code.
>
> It does *move* some code, so maybe this was an existing warning that
> moved enough that the robot thought it was new?
I guessing that this is now happening because we are now compiling the
bulk of the code in the driver. However, yes looks like it has been
there for a while.
I wonder if the '(1 << irq)' is being treated as a signed type.
> How can we reproduce this to make sure we fix it?
I was able to reproduce it by ...
$ cppcheck --force --enable=all drivers/pci/controller/dwc/pcie-tegra194.c
Checking drivers/pci/controller/dwc/pcie-tegra194.c ...
drivers/pci/controller/dwc/pcie-tegra194.c:1829:23: portability: Shifting signed 32-bit value by 31 bits is implementation-defined behaviour. See condition at line 1826. [shiftTooManyBitsSigned]
appl_writel(pcie, (1 << irq), APPL_MSI_CTRL_1);
^
drivers/pci/controller/dwc/pcie-tegra194.c:1826:19: note: Assuming that condition 'irq>31' is not redundant
if (unlikely(irq > 31))
^
drivers/pci/controller/dwc/pcie-tegra194.c:1829:23: note: Shift
appl_writel(pcie, (1 << irq), APPL_MSI_CTRL_1);
I was able to fix this by ...
diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index 8fc08336f76e..05d6a8da190b 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -1826,7 +1826,7 @@ static int tegra_pcie_ep_raise_msi_irq(struct tegra_pcie_dw *pcie, u16 irq)
if (unlikely(irq > 31))
return -EINVAL;
- appl_writel(pcie, (1 << irq), APPL_MSI_CTRL_1);
+ appl_writel(pcie, (BIT(1) << irq), APPL_MSI_CTRL_1);
return 0;
}
I can send this as a patch.
Cheers
Jon
--
nvpublic
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [pci:for-linus] BUILD SUCCESS WITH WARNING 15ac366c3d20ce1e08173f1de393a8ce95a1facf
2021-06-18 14:03 ` Jon Hunter
@ 2021-06-18 14:48 ` Thierry Reding
2021-06-18 14:49 ` Jon Hunter
0 siblings, 1 reply; 7+ messages in thread
From: Thierry Reding @ 2021-06-18 14:48 UTC (permalink / raw)
To: Jon Hunter; +Cc: Bjorn Helgaas, kernel test robot, Vidya Sagar, linux-pci
[-- Attachment #1: Type: text/plain, Size: 3292 bytes --]
On Fri, Jun 18, 2021 at 03:03:24PM +0100, Jon Hunter wrote:
> Hi Bjorn,
>
> On 18/06/2021 14:13, Bjorn Helgaas wrote:
> > [+to Jon, +cc Thierry]
> >
> > On Fri, Jun 18, 2021 at 06:34:20PM +0800, kernel test robot wrote:
> >> tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git for-linus
> >> branch HEAD: 15ac366c3d20ce1e08173f1de393a8ce95a1facf PCI: aardvark: Fix kernel panic during PIO transfer
> >>
> >> possible Warning in current branch:
> >>
> >> drivers/pci/controller/dwc/pcie-tegra194.c:1829:23: warning: Shifting signed 32-bit value by 31 bits is undefined behaviour. See condition at line 1826. [shiftTooManyBitsSigned]
> >
> > This looks like a legit warning, but I think the only commit that
> > could be related is this one:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=for-linus&id=99ab5996278379a02d5a84c4a7ac33a2ebfdb29e
> >
> > which doesn't touch that code.
> >
> > It does *move* some code, so maybe this was an existing warning that
> > moved enough that the robot thought it was new?
>
>
> I guessing that this is now happening because we are now compiling the
> bulk of the code in the driver. However, yes looks like it has been
> there for a while.
>
> I wonder if the '(1 << irq)' is being treated as a signed type.
>
> > How can we reproduce this to make sure we fix it?
>
> I was able to reproduce it by ...
>
> $ cppcheck --force --enable=all drivers/pci/controller/dwc/pcie-tegra194.c
> Checking drivers/pci/controller/dwc/pcie-tegra194.c ...
>
> drivers/pci/controller/dwc/pcie-tegra194.c:1829:23: portability: Shifting signed 32-bit value by 31 bits is implementation-defined behaviour. See condition at line 1826. [shiftTooManyBitsSigned]
> appl_writel(pcie, (1 << irq), APPL_MSI_CTRL_1);
> ^
> drivers/pci/controller/dwc/pcie-tegra194.c:1826:19: note: Assuming that condition 'irq>31' is not redundant
> if (unlikely(irq > 31))
> ^
> drivers/pci/controller/dwc/pcie-tegra194.c:1829:23: note: Shift
> appl_writel(pcie, (1 << irq), APPL_MSI_CTRL_1);
>
>
> I was able to fix this by ...
>
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index 8fc08336f76e..05d6a8da190b 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -1826,7 +1826,7 @@ static int tegra_pcie_ep_raise_msi_irq(struct tegra_pcie_dw *pcie, u16 irq)
> if (unlikely(irq > 31))
> return -EINVAL;
>
> - appl_writel(pcie, (1 << irq), APPL_MSI_CTRL_1);
> + appl_writel(pcie, (BIT(1) << irq), APPL_MSI_CTRL_1);
>
> return 0;
> }
>
> I can send this as a patch.
I think that's not the same anymore. The equivalent would rather be:
appl_writel(pcie, (BIT(0) << irq), APPL_MSI_CTRL_1);
But I think this can be achieved more easily by doing this:
appl_writel(pcie, 1UL << irq, APPL_MSI_CTRL_1);
Which is what BIT(0) would effectively end up doing. Actually, this
sounds like it should really have been this all along:
appl_writel(pcie, BIT(irq), APPL_MSI_CTRL_1);
Which should also get rid of that warning.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pci:for-linus] BUILD SUCCESS WITH WARNING 15ac366c3d20ce1e08173f1de393a8ce95a1facf
2021-06-18 14:48 ` Thierry Reding
@ 2021-06-18 14:49 ` Jon Hunter
2021-06-18 15:38 ` Bjorn Helgaas
0 siblings, 1 reply; 7+ messages in thread
From: Jon Hunter @ 2021-06-18 14:49 UTC (permalink / raw)
To: Thierry Reding; +Cc: Bjorn Helgaas, kernel test robot, Vidya Sagar, linux-pci
On 18/06/2021 15:48, Thierry Reding wrote:
> On Fri, Jun 18, 2021 at 03:03:24PM +0100, Jon Hunter wrote:
>> Hi Bjorn,
>>
>> On 18/06/2021 14:13, Bjorn Helgaas wrote:
>>> [+to Jon, +cc Thierry]
>>>
>>> On Fri, Jun 18, 2021 at 06:34:20PM +0800, kernel test robot wrote:
>>>> tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git for-linus
>>>> branch HEAD: 15ac366c3d20ce1e08173f1de393a8ce95a1facf PCI: aardvark: Fix kernel panic during PIO transfer
>>>>
>>>> possible Warning in current branch:
>>>>
>>>> drivers/pci/controller/dwc/pcie-tegra194.c:1829:23: warning: Shifting signed 32-bit value by 31 bits is undefined behaviour. See condition at line 1826. [shiftTooManyBitsSigned]
>>>
>>> This looks like a legit warning, but I think the only commit that
>>> could be related is this one:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=for-linus&id=99ab5996278379a02d5a84c4a7ac33a2ebfdb29e
>>>
>>> which doesn't touch that code.
>>>
>>> It does *move* some code, so maybe this was an existing warning that
>>> moved enough that the robot thought it was new?
>>
>>
>> I guessing that this is now happening because we are now compiling the
>> bulk of the code in the driver. However, yes looks like it has been
>> there for a while.
>>
>> I wonder if the '(1 << irq)' is being treated as a signed type.
>>
>>> How can we reproduce this to make sure we fix it?
>>
>> I was able to reproduce it by ...
>>
>> $ cppcheck --force --enable=all drivers/pci/controller/dwc/pcie-tegra194.c
>> Checking drivers/pci/controller/dwc/pcie-tegra194.c ...
>>
>> drivers/pci/controller/dwc/pcie-tegra194.c:1829:23: portability: Shifting signed 32-bit value by 31 bits is implementation-defined behaviour. See condition at line 1826. [shiftTooManyBitsSigned]
>> appl_writel(pcie, (1 << irq), APPL_MSI_CTRL_1);
>> ^
>> drivers/pci/controller/dwc/pcie-tegra194.c:1826:19: note: Assuming that condition 'irq>31' is not redundant
>> if (unlikely(irq > 31))
>> ^
>> drivers/pci/controller/dwc/pcie-tegra194.c:1829:23: note: Shift
>> appl_writel(pcie, (1 << irq), APPL_MSI_CTRL_1);
>>
>>
>> I was able to fix this by ...
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
>> index 8fc08336f76e..05d6a8da190b 100644
>> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
>> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
>> @@ -1826,7 +1826,7 @@ static int tegra_pcie_ep_raise_msi_irq(struct tegra_pcie_dw *pcie, u16 irq)
>> if (unlikely(irq > 31))
>> return -EINVAL;
>>
>> - appl_writel(pcie, (1 << irq), APPL_MSI_CTRL_1);
>> + appl_writel(pcie, (BIT(1) << irq), APPL_MSI_CTRL_1);
>>
>> return 0;
>> }
>>
>> I can send this as a patch.
>
> I think that's not the same anymore. The equivalent would rather be:
>
> appl_writel(pcie, (BIT(0) << irq), APPL_MSI_CTRL_1);
Yes indeed!
> But I think this can be achieved more easily by doing this:
>
> appl_writel(pcie, 1UL << irq, APPL_MSI_CTRL_1);
>
> Which is what BIT(0) would effectively end up doing. Actually, this
> sounds like it should really have been this all along:
>
> appl_writel(pcie, BIT(irq), APPL_MSI_CTRL_1);
>
> Which should also get rid of that warning.
Yes it does. I will send a patch with the above.
Thanks
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pci:for-linus] BUILD SUCCESS WITH WARNING 15ac366c3d20ce1e08173f1de393a8ce95a1facf
2021-06-18 14:49 ` Jon Hunter
@ 2021-06-18 15:38 ` Bjorn Helgaas
2021-06-18 16:04 ` Jon Hunter
0 siblings, 1 reply; 7+ messages in thread
From: Bjorn Helgaas @ 2021-06-18 15:38 UTC (permalink / raw)
To: Jon Hunter; +Cc: Thierry Reding, kernel test robot, Vidya Sagar, linux-pci
On Fri, Jun 18, 2021 at 03:49:39PM +0100, Jon Hunter wrote:
>
> On 18/06/2021 15:48, Thierry Reding wrote:
> > On Fri, Jun 18, 2021 at 03:03:24PM +0100, Jon Hunter wrote:
> >> Hi Bjorn,
> >>
> >> On 18/06/2021 14:13, Bjorn Helgaas wrote:
> >>> [+to Jon, +cc Thierry]
> >>>
> >>> On Fri, Jun 18, 2021 at 06:34:20PM +0800, kernel test robot wrote:
> >>>> tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git for-linus
> >>>> branch HEAD: 15ac366c3d20ce1e08173f1de393a8ce95a1facf PCI: aardvark: Fix kernel panic during PIO transfer
> >>>>
> >>>> possible Warning in current branch:
> >>>>
> >>>> drivers/pci/controller/dwc/pcie-tegra194.c:1829:23: warning: Shifting signed 32-bit value by 31 bits is undefined behaviour. See condition at line 1826. [shiftTooManyBitsSigned]
> >>>
> >>> This looks like a legit warning, but I think the only commit that
> >>> could be related is this one:
> >>>
> >>> https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=for-linus&id=99ab5996278379a02d5a84c4a7ac33a2ebfdb29e
> >>>
> >>> which doesn't touch that code.
> >>>
> >>> It does *move* some code, so maybe this was an existing warning that
> >>> moved enough that the robot thought it was new?
> >>
> >>
> >> I guessing that this is now happening because we are now compiling the
> >> bulk of the code in the driver. However, yes looks like it has been
> >> there for a while.
> >>
> >> I wonder if the '(1 << irq)' is being treated as a signed type.
> >>
> >>> How can we reproduce this to make sure we fix it?
> >>
> >> I was able to reproduce it by ...
> >>
> >> $ cppcheck --force --enable=all drivers/pci/controller/dwc/pcie-tegra194.c
> >> Checking drivers/pci/controller/dwc/pcie-tegra194.c ...
> >>
> >> drivers/pci/controller/dwc/pcie-tegra194.c:1829:23: portability: Shifting signed 32-bit value by 31 bits is implementation-defined behaviour. See condition at line 1826. [shiftTooManyBitsSigned]
> >> appl_writel(pcie, (1 << irq), APPL_MSI_CTRL_1);
> >> ^
> >> drivers/pci/controller/dwc/pcie-tegra194.c:1826:19: note: Assuming that condition 'irq>31' is not redundant
> >> if (unlikely(irq > 31))
> >> ^
> >> drivers/pci/controller/dwc/pcie-tegra194.c:1829:23: note: Shift
> >> appl_writel(pcie, (1 << irq), APPL_MSI_CTRL_1);
> >>
> >>
> >> I was able to fix this by ...
> >>
> >> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> >> index 8fc08336f76e..05d6a8da190b 100644
> >> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> >> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> >> @@ -1826,7 +1826,7 @@ static int tegra_pcie_ep_raise_msi_irq(struct tegra_pcie_dw *pcie, u16 irq)
> >> if (unlikely(irq > 31))
> >> return -EINVAL;
> >>
> >> - appl_writel(pcie, (1 << irq), APPL_MSI_CTRL_1);
> >> + appl_writel(pcie, (BIT(1) << irq), APPL_MSI_CTRL_1);
> >>
> >> return 0;
> >> }
> >>
> >> I can send this as a patch.
> >
> > I think that's not the same anymore. The equivalent would rather be:
> >
> > appl_writel(pcie, (BIT(0) << irq), APPL_MSI_CTRL_1);
>
> Yes indeed!
>
> > But I think this can be achieved more easily by doing this:
> >
> > appl_writel(pcie, 1UL << irq, APPL_MSI_CTRL_1);
> >
> > Which is what BIT(0) would effectively end up doing. Actually, this
> > sounds like it should really have been this all along:
> >
> > appl_writel(pcie, BIT(irq), APPL_MSI_CTRL_1);
> >
> > Which should also get rid of that warning.
>
> Yes it does. I will send a patch with the above.
Please also let me know what changed that made this show up now.
We need to figure out whether this fix should be squashed into
"PCI: tegra194: Fix MCFG quirk build regressions", applied before it
on for-linus for v5.13, or applied for v5.14.
Bjorn
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pci:for-linus] BUILD SUCCESS WITH WARNING 15ac366c3d20ce1e08173f1de393a8ce95a1facf
2021-06-18 15:38 ` Bjorn Helgaas
@ 2021-06-18 16:04 ` Jon Hunter
0 siblings, 0 replies; 7+ messages in thread
From: Jon Hunter @ 2021-06-18 16:04 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Thierry Reding, kernel test robot, Vidya Sagar, linux-pci
On 18/06/2021 16:38, Bjorn Helgaas wrote:
...
> Please also let me know what changed that made this show up now.
>
> We need to figure out whether this fix should be squashed into
> "PCI: tegra194: Fix MCFG quirk build regressions", applied before it
> on for-linus for v5.13, or applied for v5.14.
I see the issue even without the above patch and so I don't believe it
is related. I am not sure why it is only being flagged now. It looks
like commit c57247f940e8 ("PCI: tegra: Add support for PCIe endpoint
mode in Tegra194") introduced this.
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-06-18 16:04 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-18 10:34 [pci:for-linus] BUILD SUCCESS WITH WARNING 15ac366c3d20ce1e08173f1de393a8ce95a1facf kernel test robot
2021-06-18 13:13 ` Bjorn Helgaas
2021-06-18 14:03 ` Jon Hunter
2021-06-18 14:48 ` Thierry Reding
2021-06-18 14:49 ` Jon Hunter
2021-06-18 15:38 ` Bjorn Helgaas
2021-06-18 16:04 ` Jon Hunter
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.