Build regression introduced by 31cdd0c39c7544ced79da53aa0b7e989f3a39582

6 messages, 3 authors, 2016-09-10 · open the first message on its own page

Build regression introduced by 31cdd0c39c7544ced79da53aa0b7e989f3a39582

From: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Date: 2016-09-06 21:08:23

Hi Paul!

I'm referring to your change [1]:

"powerpc/xmon: Fix SPR read/write commands and add command to dump SPRs"

which introduced assembly code in the new file arch/powerpc/xmon/spr_access.S.

Unfortunately, this code contains assembly instructions which are not available on
all ppc targets. In particular, this change breaks the build on PPC e500v2 targets,
see the corresponding Debian bug report [2].

Could you have a look and possibly add some guarding #ifdefs for the ppc targets
where "ld" is not a supported instruction?

Thanks,
Adrian
[1] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=31cdd0c39c7544ced79da53aa0b7e989f3a39582
[2] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=836741
-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: Build regression introduced by 31cdd0c39c7544ced79da53aa0b7e989f3a39582

From: Paul Mackerras <hidden>
Date: 2016-09-07 05:07:21

On Tue, Sep 06, 2016 at 11:01:02PM +0200, John Paul Adrian Glaubitz wrote:
Hi Paul!

I'm referring to your change [1]:

"powerpc/xmon: Fix SPR read/write commands and add command to dump SPRs"

which introduced assembly code in the new file arch/powerpc/xmon/spr_access.S.

Unfortunately, this code contains assembly instructions which are not available on
all ppc targets. In particular, this change breaks the build on PPC e500v2 targets,
see the corresponding Debian bug report [2].

Could you have a look and possibly add some guarding #ifdefs for the ppc targets
where "ld" is not a supported instruction?
I blame Michael. :)  My original version of the patch didn't have the
ld instructions, and then Michael did a v2 that was different.  He
didn't update the patch description so I don't know exactly why he
changed that.  I'll discuss it with him.

Paul.

Re: Build regression introduced by 31cdd0c39c7544ced79da53aa0b7e989f3a39582

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2016-09-09 01:29:18

Paul Mackerras [off-list ref] writes:
On Tue, Sep 06, 2016 at 11:01:02PM +0200, John Paul Adrian Glaubitz wrote:
quoted
Hi Paul!

I'm referring to your change [1]:

"powerpc/xmon: Fix SPR read/write commands and add command to dump SPRs"

which introduced assembly code in the new file arch/powerpc/xmon/spr_access.S.

Unfortunately, this code contains assembly instructions which are not available on
all ppc targets. In particular, this change breaks the build on PPC e500v2 targets,
see the corresponding Debian bug report [2].

Could you have a look and possibly add some guarding #ifdefs for the ppc targets
where "ld" is not a supported instruction?
I blame Michael. :)  My original version of the patch didn't have the
ld instructions, and then Michael did a v2 that was different.  He
didn't update the patch description so I don't know exactly why he
changed that.  I'll discuss it with him.
Yep my fault. I changed the code to load the address of the SPR table
from the TOC, and just used ld without thinking.

Interestingly it builds fine for me, even for 32-bit configs, I assume
because my toolchains are 32/64-bit they are able to cope with it. I'll
try and built a 32-bit only toolchain to catch these problems in future.

cheers

Re: Build regression introduced by 31cdd0c39c7544ced79da53aa0b7e989f3a39582

From: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Date: 2016-09-09 09:52:28

On 09/09/2016 03:29 AM, Michael Ellerman wrote:
Interestingly it builds fine for me, even for 32-bit configs, I assume
because my toolchains are 32/64-bit they are able to cope with it. I'll
try and built a 32-bit only toolchain to catch these problems in future.
Yeah, I was just asking myself the same question after seeing your patch
on the list as the kernel package builds fine on Debian's powerpc port [1].

However, the powerpc Debian package always builds a 64-bit kernel image
as well in order to be able to use a 32-bit userland on a 64-bit kernel
when the hardware is capable of 64-bit. So, your guess above might actually
be true, but I haven't tested it.

What I do know is that "ld" is definitely not available on PowerPC e500
while "lwz" is available, see 3-68 in the e500 reference manual [2].

Thanks for the quick fix!

Adrian
[1] https://buildd.debian.org/status/fetch.php?pkg=linux&arch=powerpc&ver=4.7.2-1&stamp=1472464832
[2] http://www.nxp.com/files/32bit/doc/ref_manual/E500CORERM.pdf
-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: Build regression introduced by 31cdd0c39c7544ced79da53aa0b7e989f3a39582

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2016-09-10 09:53:14

John Paul Adrian Glaubitz [off-list ref] writes:
On 09/09/2016 03:29 AM, Michael Ellerman wrote:
quoted
Interestingly it builds fine for me, even for 32-bit configs, I assume
because my toolchains are 32/64-bit they are able to cope with it. I'll
try and built a 32-bit only toolchain to catch these problems in future.
Yeah, I was just asking myself the same question after seeing your patch
on the list as the kernel package builds fine on Debian's powerpc port [1].

However, the powerpc Debian package always builds a 64-bit kernel image
as well in order to be able to use a 32-bit userland on a 64-bit kernel
when the hardware is capable of 64-bit. So, your guess above might actually
be true, but I haven't tested it.
I built a 32-bit only toolchain, ie. one that doesn't accept -m64. But
it still happily assembles a .S with "ld" in it. So not sure what's
going on with that.
What I do know is that "ld" is definitely not available on PowerPC e500
while "lwz" is available, see 3-68 in the e500 reference manual [2].
Yep.
Thanks for the quick fix!
I assume by that it worked for you? I'll add a Tested-by: for you?

cheers

Re: Build regression introduced by 31cdd0c39c7544ced79da53aa0b7e989f3a39582

From: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Date: 2016-09-10 10:04:02

On 09/10/2016 11:53 AM, Michael Ellerman wrote:
quoted
Thanks for the quick fix!
I assume by that it worked for you? I'll add a Tested-by: for you?
Not yet. I just got my second PowerPC e500 evaluation board yesterday and the
other board is running 24/7 as an automatic build machine. I will need a few
days to be able to assemble the second machine since I need to order some parts
(SSD, power supply, memory and so on) first.

Cheers,
Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help