Re: OpenRISC: Module loader broken

[Geert Uytterhoeven]

On Fri, Mar 2, 2012 at 07:11, Jonas Bonn <jonas@xxxxxxxxxxxx> wrote:
>> It explodes somewhere in apply_relocate_add().
>> This function is anyway fishy:
>> Â Â Â Â Â Â Â Â case R_OR32_CONST:
>> Â Â Â Â Â Â Â Â Â Â Â Â location = (uint16_t *)location + 1;
>> Â Â Â Â Â Â Â Â Â Â Â Â *((uint16_t *)location) = (uint16_t) (value);
>> Â Â Â Â Â Â Â Â Â Â Â Â break;
>> Â Â Â Â Â Â Â Â case R_OR32_CONSTH:
>> Â Â Â Â Â Â Â Â Â Â Â Â location = (uint16_t *)location + 1;
>> Â Â Â Â Â Â Â Â Â Â Â Â *((uint16_t *)location) = (uint16_t) (value >> 16);
>> Â Â Â Â Â Â Â Â Â Â Â Â break;
>>
>> Why the cast to (uint16_t *)?
>
> This is for loading a 32 bit const, which is done in two steps:
>
> l.movhi rX,hi(const)
> l.ori  rX,rX,lo(const)
>
> The immediate value 'const' here is encoded in the low 16 bits of those
> instructions.

So OpenRISC is big endian, which matches
arch/openrisc/include/asm/byteorder.h:#include <linux/byteorder/big_endian.h>

However, there are some remainings of little endian support:
arch/openrisc/include/asm/unaligned.h:#if defined(__LITTLE_ENDIAN)
arch/openrisc/include/asm/unaligned.h:#elif defined(__BIG_ENDIAN)
arch/openrisc/include/asm/unaligned.h:# error need to define endianess

> Since 'location' points at the 32-bit instruction in question, that cast
> is supposed to be saying: Â'move 16 bit value into low 16 bits of
> instruction'.

It may be more readable and safer (w.r.t. changing endianness) to do
32-bit loads and stores and explicitly modify the 16 LSB.

Gr{oetje,eeting}s,

            Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
             Â Â -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/