On Mon, 2008-06-09 at 13:19 +0200, Segher Boessenkool wrote:
> > 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:
>
> Yes, exactly.
>
> > #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.
>
> Only one, actually: in __udelay() (it does two __ctl_load()s as well).
> Oh, and paging_init() has some __ctl_load() with an unsigned long.
And system.h and sclp.c and cio.c and .. there are a few.
> > 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 ..
>
> Yeah. What is this whole addrtype thing supposed to accomplish?
> Can't you just write "=m" (array) ?
The addrtype thing is a type that covers memory of some length. The
"m"/"=m" constraints are there to tell the compiler exactly which pieces
of memory are needed / modified. We had cases where we created some
local stack variables and passed them to an inline assembly without
telling the compiler that we need the content of the variable, we only
passed the address. gcc outsmarted us and removed the instructions to
initialize the stack variable..
> [/me tries it out]
>
> That works fine for __ctl_store(), but doing the analogue thing to
> __ctl_load() gives
>
> arch/s390/kernel/smp.c:341: error: memory input 3 is not directly
> addressable
Exactly .. with the typedef and the cast we get around this.
--
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
