Re: [PATCH v3] MIPS: Avoid warnings from use of dla in 32 bit kernels

[Paul Burton <paul.burton@xxxxxxxxxx>]

Hi Maciej,

On Friday, 31 March 2017 04:18:40 PDT Maciej W. Rozycki wrote:
> On Thu, 30 Mar 2017, Paul Burton wrote:
> > One seemingly straightforward fix would be to make use of the PTR_LA
> > macro to emit the appropriate pseudo-instruction, however this would
> > involve including asm.h which is intended solely for inclusion in
> > assembly code. When included by C code its definition of various generic
> > & non-namespaced macros such as LONG, PTR, CAT etc cause numerous build
> > failures.
> 
>  This is however exactly what we do in several places, and I would
> recommend here as well.  Can you point me at the earlier review of your
> proposal?

The first 2 revisions of the patch, which did include asm/asm.h & use PTR_LA, 
can be found in patchwork:

  https://patchwork.linux-mips.org/patch/12436/
  https://patchwork.linux-mips.org/patch/12443/

> > Instead fix this by adding a ".set gp=64" directive to inform the
> > assembler that general purpose registers are 64 bit for the dla
> > instruction. This is a lie, but no more so than using the dla
> > instruction to begin with.
> 
>  I agree using DLA unconditionally is wrong, so if using <asm/asm.h> and
> its PTR_LA turns out infeasible indeed, then please define a local macro
> that expands to LA or DLA as appropriate and does not cause a namespace
> issue, and use it in `instruction_hazard' (all instances) rather than this
> horrible hack.

Horrible though the use of dla may be at first glance, I do wonder if it 
actually makes sense to just keep it. Presumably Ralf thought it made sense 
when he added it in 7043ad4f4c81 ("MIPS: R2: Try to bulletproof 
instruction_hazard against miss-compilation."). The presumption is simply that 
the dla pseudo-instruction won't result in any MIPS64 instructions being 
emitted when used on a 32 bit canonical address, which might not be "nice" but 
then neither is the #ifdef solution.

I had lost track of what the build failures were when including asm/asm.h, but 
reverting to v2 of my patch & building all defconfigs shows a number of 
issues.

For example, from mtx1_defconfig:

  CC [M]  drivers/net/ethernet/fealnx.o
In file included from ./arch/mips/include/asm/hazards.h:15:0,
                 from ./arch/mips/include/asm/mipsregs.h:18,
                 from ./arch/mips/include/asm/mach-generic/spaces.h:15,
                 from ./arch/mips/include/asm/addrspace.h:13,
                 from ./arch/mips/include/asm/barrier.h:11,
                 from ./arch/mips/include/asm/bitops.h:18,
                 from ./include/linux/bitops.h:36,
                 from ./include/linux/kernel.h:10,
                 from ./include/linux/list.h:8,
                 from ./include/linux/module.h:9,
                 from drivers/net/ethernet/fealnx.c:71:
./arch/mips/include/asm/asm.h:318:15: error: expected identifier before ‘.’ 
token
 #define LONG  .word
               ^
drivers/net/ethernet/fealnx.c:261:2: note: in expansion of macro ‘LONG’
  LONG = 0x20,  /* long packet received */
  ^

Or from rm200_defconfig:

  CC      arch/mips/sni/setup.o
In file included from ./arch/mips/include/asm/hazards.h:15:0,
                 from ./arch/mips/include/asm/mipsregs.h:18,
                 from ./arch/mips/include/asm/mach-generic/spaces.h:15,
                 from ./arch/mips/include/asm/addrspace.h:13,
                 from ./arch/mips/include/asm/barrier.h:11,
                 from ./arch/mips/include/asm/bitops.h:18,
                 from ./include/linux/bitops.h:36,
                 from ./include/linux/kernel.h:10,
                 from ./include/linux/list.h:8,
                 from ./include/linux/kobject.h:20,
                 from ./include/linux/device.h:17,
                 from ./include/linux/eisa.h:5,
                 from arch/mips/sni/setup.c:11:
./arch/mips/include/asm/asm.h:318:15: error: expected identifier or ‘(’ before 
‘.’ token
 #define LONG  .word
               ^
./arch/mips/include/asm/fw/arc/types.h:18:15: note: in expansion of macro 
‘LONG’
 typedef long  LONG __attribute__ ((__mode__ (__SI__)));

Other builds that include asm/fw/arc/types.h of course fail similarly, for 
example ip22_defconfig, ip27_defconfig, ip28_defconfig, ip32_defconfig & 
jazz_defconfig all fail to build when asm/hazards.h includes asm/asm.h.

Your point above that we already include asm/asm.h in various C files is a 
good one, though given how generic some of the names of its preprocessor 
definitions are it wouldn't surprise me to see that cause other build breakage 
in the future.

Thanks,
    Paul

Attachment: signature.asc
Description: This is a digitally signed message part.