diff mbox series

selftests/powerpc: Fix compilation issue due to asm label

Message ID 20181031171813.8970-1-naveen.n.rao@linux.vnet.ibm.com (mailing list archive)
State Accepted
Commit 1936f094e164cc13ebf17aba1d6b34e033e64188
Headers show
Series selftests/powerpc: Fix compilation issue due to asm label | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success next/apply_patch Successfully applied
snowpatch_ozlabs/checkpatch success Test checkpatch on branch next
snowpatch_ozlabs/build-ppc64le success Test build-ppc64le on branch next
snowpatch_ozlabs/build-ppc64be success Test build-ppc64be on branch next
snowpatch_ozlabs/build-ppc64e success Test build-ppc64e on branch next
snowpatch_ozlabs/build-ppc32 success Test build-ppc32 on branch next

Commit Message

Naveen N. Rao Oct. 31, 2018, 5:18 p.m. UTC
We are using 'dscr_insn' as a label in inline asm to identify if a
SIGILL was generated by the mtspr instruction at that point. However,
with inline assembly, the compiler is still free to duplicate the asm
statement for optimization purposes, which results in the label being
defined twice with the error:
	/tmp/ccerQCql.s:874: Error: symbol `dscr_insn' is already defined

With different compiler versions, we may also see:
	/tmp/ccJzLDlN.o:(.toc+0x0): undefined reference to `dscr_insn'

Remove the use of the label in the inline assembly. Instead, just look
for the offending instruction in the signal handler.

Reported-by: Breno Leitao <leitao@debian.org>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 tools/testing/selftests/powerpc/utils.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Naveen N. Rao Oct. 31, 2018, 5:24 p.m. UTC | #1
Naveen N. Rao wrote:
> We are using 'dscr_insn' as a label in inline asm to identify if a
> SIGILL was generated by the mtspr instruction at that point. However,
> with inline assembly, the compiler is still free to duplicate the asm
> statement for optimization purposes, which results in the label being
> defined twice with the error:
> 	/tmp/ccerQCql.s:874: Error: symbol `dscr_insn' is already defined
> 
> With different compiler versions, we may also see:
> 	/tmp/ccJzLDlN.o:(.toc+0x0): undefined reference to `dscr_insn'
> 
> Remove the use of the label in the inline assembly. Instead, just look
> for the offending instruction in the signal handler.
> 
> Reported-by: Breno Leitao <leitao@debian.org>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---

Missed adding:
Fixes: d2bf793237b3aa9c ("selftests/powerpc: Add test to verify rfi flush across a system call")

- Naveen
Breno Leitao Oct. 31, 2018, 7:05 p.m. UTC | #2
hi Naveen,

On 10/31/18 2:24 PM, Naveen N. Rao wrote:
> Naveen N. Rao wrote:
>> We are using 'dscr_insn' as a label in inline asm to identify if a
>> SIGILL was generated by the mtspr instruction at that point. However,
>> with inline assembly, the compiler is still free to duplicate the asm
>> statement for optimization purposes, which results in the label being
>> defined twice with the error:
>>     /tmp/ccerQCql.s:874: Error: symbol `dscr_insn' is already defined
>>
>> With different compiler versions, we may also see:
>>     /tmp/ccJzLDlN.o:(.toc+0x0): undefined reference to `dscr_insn'
>>
>> Remove the use of the label in the inline assembly. Instead, just look
>> for the offending instruction in the signal handler.

Cool, I've tested this patch on my mainline tree and selftests are being
able to build again. I also tested the rfi_flush_test selftest as other
tests (ptrace/ and tm/) and everything seems to be normal again.

Thank you!

>> Reported-by: Breno Leitao <leitao@debian.org>
>> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>

Tested-by: Breno Leitao <leitao@debian.org>
Michael Ellerman Nov. 1, 2018, 12:46 p.m. UTC | #3
On Wed, 2018-10-31 at 17:18:13 UTC, "Naveen N. Rao" wrote:
> We are using 'dscr_insn' as a label in inline asm to identify if a
> SIGILL was generated by the mtspr instruction at that point. However,
> with inline assembly, the compiler is still free to duplicate the asm
> statement for optimization purposes, which results in the label being
> defined twice with the error:
> 	/tmp/ccerQCql.s:874: Error: symbol `dscr_insn' is already defined
> 
> With different compiler versions, we may also see:
> 	/tmp/ccJzLDlN.o:(.toc+0x0): undefined reference to `dscr_insn'
> 
> Remove the use of the label in the inline assembly. Instead, just look
> for the offending instruction in the signal handler.
> 
> Reported-by: Breno Leitao <leitao@debian.org>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> Tested-by: Breno Leitao <leitao@debian.org>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/1936f094e164cc13ebf17aba1d6b34

cheers
diff mbox series

Patch

diff --git a/tools/testing/selftests/powerpc/utils.c b/tools/testing/selftests/powerpc/utils.c
index 43c342845be0..ed62f4153d3e 100644
--- a/tools/testing/selftests/powerpc/utils.c
+++ b/tools/testing/selftests/powerpc/utils.c
@@ -25,7 +25,6 @@ 
 #include "utils.h"
 
 static char auxv[4096];
-extern unsigned int dscr_insn[];
 
 int read_auxv(char *buf, ssize_t buf_size)
 {
@@ -247,7 +246,8 @@  static void sigill_handler(int signr, siginfo_t *info, void *unused)
 	ucontext_t *ctx = (ucontext_t *)unused;
 	unsigned long *pc = &UCONTEXT_NIA(ctx);
 
-	if (*pc == (unsigned long)&dscr_insn) {
+	/* mtspr 3,RS to check for move to DSCR below */
+	if ((*((unsigned int *)*pc) & 0xfc1fffff) == 0x7c0303a6) {
 		if (!warned++)
 			printf("WARNING: Skipping over dscr setup. Consider running 'ppc64_cpu --dscr=1' manually.\n");
 		*pc += 4;
@@ -271,5 +271,5 @@  void set_dscr(unsigned long val)
 		init = 1;
 	}
 
-	asm volatile("dscr_insn: mtspr %1,%0" : : "r" (val), "i" (SPRN_DSCR));
+	asm volatile("mtspr %1,%0" : : "r" (val), "i" (SPRN_DSCR));
 }