Re: [peterz-queue:perf/core 10/10] arch/x86/events/intel/uncore_snb.c:1111:36: warning: left shift count >= width of type

[Peter Zijlstra <peterz@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 2500 bytes --]

On Sun, Feb 09, 2020 at 11:20:38AM -0500, Liang, Kan wrote:
> 
> 
> On 2/7/2020 5:50 PM, kbuild test robot wrote:
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git perf/core
> > head:   60291b1620ac0b5d07624290a0ce909a0b09863e
> > commit: 60291b1620ac0b5d07624290a0ce909a0b09863e [10/10] perf/x86: Add Intel Tiger Lake uncore support
> > config: i386-allyesconfig (attached as .config)
> > compiler: gcc-7 (Debian 7.5.0-3) 7.5.0
> > reproduce:
> >          git checkout 60291b1620ac0b5d07624290a0ce909a0b09863e
> >          # save the attached .config to linux build tree
> >          make ARCH=i386
> > 
> > If you fix the issue, kindly add following tag
> > Reported-by: kbuild test robot <lkp@intel.com>
> > 
> > All warnings (new ones prefixed by >>):
> > 
> >     arch/x86/events/intel/uncore_snb.c: In function 'tgl_uncore_imc_freerunning_init_box':
> > > > arch/x86/events/intel/uncore_snb.c:1111:36: warning: left shift count >= width of type [-Wshift-count-overflow]
> >       addr |= ((resource_size_t)mch_bar << 32);
> >                                         ^~
> > 
> 
> The patch as below should fix the warning. But I didn't do any test with 32
> bit OS. I'm not sure if we still support 32 bit.

Of course we do. The compiler just told you.

Now, personally I'd _love_ it if we were to just delete the entire 32bit
x86 code, but last time we proposed that Linus wasn't convinced to put
it mildly.

Now, if you can get an official Intel statement that Intel no longer
cares about 32bit, that's be awesome. But last I checked we were
actually still selling 32bit only parts :-/

> Peter, could you please let me know if I should submit a separate patch to
> fix the warning?
> 
> diff --git a/arch/x86/events/intel/uncore_snb.c
> b/arch/x86/events/intel/uncore_snb.c
> index ab67b23b2106..3de1065eefc4 100644
> --- a/arch/x86/events/intel/uncore_snb.c
> +++ b/arch/x86/events/intel/uncore_snb.c
> @@ -1107,8 +1107,10 @@ static void
> tgl_uncore_imc_freerunning_init_box(struct intel_uncore_box *box)
>  	mch_bar &= ~BIT(0);
>  	addr = (resource_size_t)(mch_bar + TGL_UNCORE_MMIO_IMC_MEM_OFFSET *
> pmu->pmu_idx);
> 
> +#ifdef CONFIG_PHYS_ADDR_T_64BIT
>  	pci_read_config_dword(pdev, SNB_UNCORE_PCI_IMC_BAR_OFFSET + 4, &mch_bar);
>  	addr |= ((resource_size_t)mch_bar << 32);
> +#endif
> 
>  	box->io_addr = ioremap(addr, SNB_UNCORE_PCI_IMC_MAP_SIZE);
>  }

Let me frob that in and see what it does.