| Message ID | 20210218123058.748882-1-mpe@ellerman.id.au (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | powerpc/4xx: Fix build errors from mfdcr() | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (626a6c3d2e20da80aaa710104f34ea6037b28b33) |
| snowpatch_ozlabs/build-ppc64le | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64be | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64e | success | Build succeeded |
| snowpatch_ozlabs/build-pmac32 | success | Build succeeded |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 20 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
On Thu, Feb 18, 2021 at 11:30:58PM +1100, Michael Ellerman wrote: > lkp reported a build error in fsp2.o: > > CC arch/powerpc/platforms/44x/fsp2.o > {standard input}:577: Error: unsupported relocation against base > > Which comes from: > > pr_err("GESR0: 0x%08x\n", mfdcr(base + PLB4OPB_GESR0)); > > Where our mfdcr() macro is stringifying "base + PLB4OPB_GESR0", and > passing that to the assembler, which obviously doesn't work. > > The mfdcr() macro already checks that the argument is constant using > __builtin_constant_p(), and if not calls the out-of-line version of > mfdcr(). But in this case GCC is smart enough to notice that "base + > PLB4OPB_GESR0" will be constant, even though it's not something we can > immediately stringify into a register number. > > Segher pointed out that passing the register number to the inline asm > as a constant would be better, and in fact it fixes the build error, > presumably because it gives GCC a chance to resolve the value. > > While we're at it, change mtdcr() similarly. > > Reported-by: kernel test robot <lkp@intel.com> > Suggested-by: Segher Boessenkool <segher@kernel.crashing.org> > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> Acked-by: Feng Tang <feng.tang@intel.com> Thanks! > --- > arch/powerpc/include/asm/dcr-native.h | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/arch/powerpc/include/asm/dcr-native.h b/arch/powerpc/include/asm/dcr-native.h > index 7141ccea8c94..a92059964579 100644 > --- a/arch/powerpc/include/asm/dcr-native.h > +++ b/arch/powerpc/include/asm/dcr-native.h > @@ -53,8 +53,8 @@ static inline void mtdcrx(unsigned int reg, unsigned int val) > #define mfdcr(rn) \ > ({unsigned int rval; \ > if (__builtin_constant_p(rn) && rn < 1024) \ > - asm volatile("mfdcr %0," __stringify(rn) \ > - : "=r" (rval)); \ > + asm volatile("mfdcr %0, %1" : "=r" (rval) \ > + : "n" (rn)); \ > else if (likely(cpu_has_feature(CPU_FTR_INDEXED_DCR))) \ > rval = mfdcrx(rn); \ > else \ > @@ -64,8 +64,8 @@ static inline void mtdcrx(unsigned int reg, unsigned int val) > #define mtdcr(rn, v) \ > do { \ > if (__builtin_constant_p(rn) && rn < 1024) \ > - asm volatile("mtdcr " __stringify(rn) ",%0" \ > - : : "r" (v)); \ > + asm volatile("mtdcr %0, %1" \ > + : : "n" (rn), "r" (v)); \ > else if (likely(cpu_has_feature(CPU_FTR_INDEXED_DCR))) \ > mtdcrx(rn, v); \ > else \ > -- > 2.25.1
On Thu, 18 Feb 2021 23:30:58 +1100, Michael Ellerman wrote: > lkp reported a build error in fsp2.o: > > CC arch/powerpc/platforms/44x/fsp2.o > {standard input}:577: Error: unsupported relocation against base > > Which comes from: > > [...] Applied to powerpc/fixes. [1/1] powerpc/4xx: Fix build errors from mfdcr() https://git.kernel.org/powerpc/c/eead089311f4d935ab5d1d8fbb0c42ad44699ada cheers
diff --git a/arch/powerpc/include/asm/dcr-native.h b/arch/powerpc/include/asm/dcr-native.h index 7141ccea8c94..a92059964579 100644 --- a/arch/powerpc/include/asm/dcr-native.h +++ b/arch/powerpc/include/asm/dcr-native.h @@ -53,8 +53,8 @@ static inline void mtdcrx(unsigned int reg, unsigned int val) #define mfdcr(rn) \ ({unsigned int rval; \ if (__builtin_constant_p(rn) && rn < 1024) \ - asm volatile("mfdcr %0," __stringify(rn) \ - : "=r" (rval)); \ + asm volatile("mfdcr %0, %1" : "=r" (rval) \ + : "n" (rn)); \ else if (likely(cpu_has_feature(CPU_FTR_INDEXED_DCR))) \ rval = mfdcrx(rn); \ else \ @@ -64,8 +64,8 @@ static inline void mtdcrx(unsigned int reg, unsigned int val) #define mtdcr(rn, v) \ do { \ if (__builtin_constant_p(rn) && rn < 1024) \ - asm volatile("mtdcr " __stringify(rn) ",%0" \ - : : "r" (v)); \ + asm volatile("mtdcr %0, %1" \ + : : "n" (rn), "r" (v)); \ else if (likely(cpu_has_feature(CPU_FTR_INDEXED_DCR))) \ mtdcrx(rn, v); \ else \
lkp reported a build error in fsp2.o: CC arch/powerpc/platforms/44x/fsp2.o {standard input}:577: Error: unsupported relocation against base Which comes from: pr_err("GESR0: 0x%08x\n", mfdcr(base + PLB4OPB_GESR0)); Where our mfdcr() macro is stringifying "base + PLB4OPB_GESR0", and passing that to the assembler, which obviously doesn't work. The mfdcr() macro already checks that the argument is constant using __builtin_constant_p(), and if not calls the out-of-line version of mfdcr(). But in this case GCC is smart enough to notice that "base + PLB4OPB_GESR0" will be constant, even though it's not something we can immediately stringify into a register number. Segher pointed out that passing the register number to the inline asm as a constant would be better, and in fact it fixes the build error, presumably because it gives GCC a chance to resolve the value. While we're at it, change mtdcr() similarly. Reported-by: kernel test robot <lkp@intel.com> Suggested-by: Segher Boessenkool <segher@kernel.crashing.org> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> --- arch/powerpc/include/asm/dcr-native.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)