Re: [PATCH] s390: Fix build failure in __cpu_up()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, 2008-06-07 at 11:32 +0200, Segher Boessenkool wrote:
> The first argument to __ctl_store() should be the array to store
> stuff in, not just the first element of that array.  With the
> current code in __cpu_up(), mainline GCC dies with an internal
> compiler error.  I didn't diagnose that further, but just fixed
> the kernel bug.
> 
> Signed-off-by: Segher Boessenkool <segher@xxxxxxxxxxxxxxxxxxx>
> Cc: Martin Schwidefsky <schwidefsky@xxxxxxxxxx>
> Cc: Heiko Carstens <heiko.carstens@xxxxxxxxxx>
> ---
>  arch/s390/kernel/smp.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/s390/kernel/smp.c b/arch/s390/kernel/smp.c
> index 42b1d12..5d4fa4b 100644
> --- a/arch/s390/kernel/smp.c
> +++ b/arch/s390/kernel/smp.c
> @@ -711,7 +711,7 @@ int __cpuinit __cpu_up(unsigned int cpu)
>  	memset(sf, 0, sizeof(struct stack_frame));
>  	sf->gprs[9] = (unsigned long) sf;
>  	cpu_lowcore->save_area[15] = (unsigned long) sf;
> -	__ctl_store(cpu_lowcore->cregs_save_area[0], 0, 15);
> +	__ctl_store(cpu_lowcore->cregs_save_area, 0, 15);
>  	asm volatile(
>  		"	stam	0,15,0(%0)"
>  		: : "a" (&cpu_lowcore->access_regs_save_area) : "memory");

Definitly a bug. The __ctl_store stores 16 registers and passing only
the first element of the array will make the addrtype in the __ctl_store
macro too small:

#define __ctl_store(array, low, high) ({                        \
        typedef struct { char _[sizeof(array)]; } addrtype;     \
        asm volatile(                                           \
                "       stctg   %2,%3,0(%1)\n"                  \
                : "=m" (*(addrtype *)(array))                   \
                : "a" (&array), "i" (low), "i" (high));         \
        })

We've used __ctl_store with an "unsigned long" as first argument at a
couple of places. If __cpu_up dies with an ICE then these other
occurences will probably be broken as well.
"=m" (*(addrtype *)(<unsigned long>)) doesn't look healthy since the
unsigned long is interpreted as an address. Not good ..

I'll add this patch to the queue for 2.6.26 and have to think about some
more fixes.

-- 
blue skies,
  Martin.

"Reality continues to ruin my life." - Calvin.


--
To unsubscribe from this list: send the line "unsubscribe linux-s390" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Kernel Development]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Info]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Linux Media]     [Device Mapper]

  Powered by Linux