Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1649398 > unrolled thread

CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)

Back to article view | Back to linux.kernel

Contents

  CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc:  Define config parameter CPU_BIG_ENDIAN) Geert Uytterhoeven <geert@linux-m68k.org> - 2017-05-24 12:00 +0200
    Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7]  arch/sparc: Define config parameter CPU_BIG_ENDIAN) Arnd Bergmann <arnd@arndb.de> - 2017-05-24 12:20 +0200
      Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7]  arch/sparc: Define config parameter CPU_BIG_ENDIAN) Babu Moger <babu.moger@oracle.com> - 2017-05-24 16:50 +0200
        Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7]  arch/sparc: Define config parameter CPU_BIG_ENDIAN) Arnd Bergmann <arnd@arndb.de> - 2017-05-24 17:10 +0200
          Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7]  arch/sparc: Define config parameter CPU_BIG_ENDIAN) Babu Moger <babu.moger@oracle.com> - 2017-05-24 19:10 +0200

#1649398 — CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)

On Tue, May 23, 2017 at 11:45 PM, Babu Moger <babu.moger@oracle.com> wrote:
> Found this problem while enabling queued rwlock on SPARC.
> The parameter CONFIG_CPU_BIG_ENDIAN is used to clear the
> specific byte in qrwlock structure. Without this parameter,
> we clear the wrong byte. Here is the code.
>
> static inline u8 *__qrwlock_write_byte(struct qrwlock *lock)
>  {
>         return (u8 *)lock + 3 * IS_BUILTIN(CONFIG_CPU_BIG_ENDIAN);
>  }
>
> Define CPU_BIG_ENDIAN for SPARC to fix it.

> --- a/arch/sparc/Kconfig
> +++ b/arch/sparc/Kconfig
> @@ -92,6 +92,10 @@ config ARCH_DEFCONFIG
>  config ARCH_PROC_KCORE_TEXT
>         def_bool y
>
> +config CPU_BIG_ENDIAN
> +       bool
> +       default y if SPARC

Nice catch!

Traditionally, CPU_BIG_ENDIAN and CPU_LITTLE_ENDIAN were defined only on
architectures that may support both.  And it was checked in platform code
and drivers only.
Hence the symbol is lacking from most architectures. Heck, even
architectures that support both may default to one endiannes, and declare
only the symbol for the other endianness:

--- arch/alpha ---
--- arch/arc ---
arch/arc/Kconfig:config CPU_BIG_ENDIAN
--- arch/arm ---
arch/arm/mm/Kconfig:config CPU_BIG_ENDIAN
--- arch/arm64 ---
arch/arm64/Kconfig:config CPU_BIG_ENDIAN
--- arch/blackfin ---
--- arch/c6x ---
arch/c6x/Kconfig:config CPU_BIG_ENDIAN
--- arch/cris ---
--- arch/frv ---
--- arch/h8300 ---
--- arch/hexagon ---
--- arch/ia64 ---
--- arch/Kconfig ---
--- arch/m32r ---
arch/m32r/Kconfig:config CPU_LITTLE_ENDIAN
--- arch/m68k ---
--- arch/metag ---
--- arch/microblaze ---
--- arch/mips ---
arch/mips/Kconfig:config CPU_BIG_ENDIAN
arch/mips/Kconfig:config CPU_LITTLE_ENDIAN
--- arch/mn10300 ---
--- arch/nios2 ---
--- arch/openrisc ---
--- arch/parisc ---
--- arch/powerpc ---
arch/powerpc/platforms/Kconfig.cputype:config CPU_BIG_ENDIAN
arch/powerpc/platforms/Kconfig.cputype:config CPU_LITTLE_ENDIAN
--- arch/s390 ---
arch/s390/Kconfig:config CPU_BIG_ENDIAN
--- arch/score ---
--- arch/sh ---
arch/sh/Kconfig.cpu:config CPU_LITTLE_ENDIAN
arch/sh/Kconfig.cpu:config CPU_BIG_ENDIAN
--- arch/sparc ---
--- arch/tile ---
--- arch/um ---
--- arch/unicore32 ---
--- arch/x86 ---
--- arch/xtensa ---

However, there are already a few users in generic code, which are thus
broken on many platforms:

    drivers/of/base.c
    drivers/of/fdt.c
    drivers/tty/serial/earlycon.c
    drivers/tty/serial/serial_core.c

include/asm-generic/qrwlock.h is also generic, but depends on the
architecture to select ARCH_USE_QUEUED_RWLOCKS, which only very few do
(x86, and now sparc).

I guess the time is ripe for adding (both) symbols to all architectures?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

[toc] | [next] | [standalone]

#1649414 — Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)

On Wed, May 24, 2017 at 11:59 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Tue, May 23, 2017 at 11:45 PM, Babu Moger <babu.moger@oracle.com> wrote:
>> Found this problem while enabling queued rwlock on SPARC.
>> The parameter CONFIG_CPU_BIG_ENDIAN is used to clear the
>> specific byte in qrwlock structure. Without this parameter,
>> we clear the wrong byte. Here is the code.
>>
>> static inline u8 *__qrwlock_write_byte(struct qrwlock *lock)
>>  {
>>         return (u8 *)lock + 3 * IS_BUILTIN(CONFIG_CPU_BIG_ENDIAN);
>>  }
>>
>> Define CPU_BIG_ENDIAN for SPARC to fix it.
>
>> --- a/arch/sparc/Kconfig
>> +++ b/arch/sparc/Kconfig
>> @@ -92,6 +92,10 @@ config ARCH_DEFCONFIG
>>  config ARCH_PROC_KCORE_TEXT
>>         def_bool y
>>
>> +config CPU_BIG_ENDIAN
>> +       bool
>> +       default y if SPARC
>
> Nice catch!
>
> Traditionally, CPU_BIG_ENDIAN and CPU_LITTLE_ENDIAN were defined only on
> architectures that may support both.  And it was checked in platform code
> and drivers only.
> Hence the symbol is lacking from most architectures. Heck, even
> architectures that support both may default to one endiannes, and declare
> only the symbol for the other endianness:
>
> --- arch/alpha ---
> --- arch/arc ---
> arch/arc/Kconfig:config CPU_BIG_ENDIAN
> --- arch/arm ---
> arch/arm/mm/Kconfig:config CPU_BIG_ENDIAN
> --- arch/arm64 ---
> arch/arm64/Kconfig:config CPU_BIG_ENDIAN
> --- arch/blackfin ---
> --- arch/c6x ---
> arch/c6x/Kconfig:config CPU_BIG_ENDIAN
> --- arch/cris ---
> --- arch/frv ---
> --- arch/h8300 ---
> --- arch/hexagon ---
> --- arch/ia64 ---
> --- arch/Kconfig ---
> --- arch/m32r ---
> arch/m32r/Kconfig:config CPU_LITTLE_ENDIAN
> --- arch/m68k ---
> --- arch/metag ---
> --- arch/microblaze ---
> --- arch/mips ---
> arch/mips/Kconfig:config CPU_BIG_ENDIAN
> arch/mips/Kconfig:config CPU_LITTLE_ENDIAN
> --- arch/mn10300 ---
> --- arch/nios2 ---
> --- arch/openrisc ---
> --- arch/parisc ---
> --- arch/powerpc ---
> arch/powerpc/platforms/Kconfig.cputype:config CPU_BIG_ENDIAN
> arch/powerpc/platforms/Kconfig.cputype:config CPU_LITTLE_ENDIAN
> --- arch/s390 ---
> arch/s390/Kconfig:config CPU_BIG_ENDIAN
> --- arch/score ---
> --- arch/sh ---
> arch/sh/Kconfig.cpu:config CPU_LITTLE_ENDIAN
> arch/sh/Kconfig.cpu:config CPU_BIG_ENDIAN
> --- arch/sparc ---
> --- arch/tile ---
> --- arch/um ---
> --- arch/unicore32 ---
> --- arch/x86 ---
> --- arch/xtensa ---
>
> However, there are already a few users in generic code, which are thus
> broken on many platforms:
>
>     drivers/of/base.c
>     drivers/of/fdt.c
>     drivers/tty/serial/earlycon.c
>     drivers/tty/serial/serial_core.c
>
> include/asm-generic/qrwlock.h is also generic, but depends on the
> architecture to select ARCH_USE_QUEUED_RWLOCKS, which only very few do
> (x86, and now sparc).
>
> I guess the time is ripe for adding (both) symbols to all architectures?

Good idea. I think we can do most of this by adding a few lines to
arch/Kconfig:

config CPU_BIG_ENDIAN
        bool

config CPU_LITTLE_ENDIAN
       def_bool !CPU_BIG_ENDIAN

This way, we only need to add 'select CPU_BIG_ENDIAN' to the
architectures that are always big-endian, and we don't need to
change anything for the ones that have a single 'CPU_BIG_ENDIAN'
option.

The three architectures that have a 'choice' statement (mips, ppc and
sh) will have to convert, and m32r will have to replace the
option with the opposite one, which could break 'make oldconfig',
but nobody really cares about m32r any more.

       Arnd

[toc] | [prev] | [next] | [standalone]

#1649677 — Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)

Arnd,


On 5/24/2017 5:18 AM, Arnd Bergmann wrote:
> On Wed, May 24, 2017 at 11:59 AM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Tue, May 23, 2017 at 11:45 PM, Babu Moger <babu.moger@oracle.com> wrote:
>>> Found this problem while enabling queued rwlock on SPARC.
>>> The parameter CONFIG_CPU_BIG_ENDIAN is used to clear the
>>> specific byte in qrwlock structure. Without this parameter,
>>> we clear the wrong byte. Here is the code.
>>>
>>> static inline u8 *__qrwlock_write_byte(struct qrwlock *lock)
>>>   {
>>>          return (u8 *)lock + 3 * IS_BUILTIN(CONFIG_CPU_BIG_ENDIAN);
>>>   }
>>>
>>> Define CPU_BIG_ENDIAN for SPARC to fix it.
>>> --- a/arch/sparc/Kconfig
>>> +++ b/arch/sparc/Kconfig
>>> @@ -92,6 +92,10 @@ config ARCH_DEFCONFIG
>>>   config ARCH_PROC_KCORE_TEXT
>>>          def_bool y
>>>
>>> +config CPU_BIG_ENDIAN
>>> +       bool
>>> +       default y if SPARC
>> Nice catch!
>>
>> Traditionally, CPU_BIG_ENDIAN and CPU_LITTLE_ENDIAN were defined only on
>> architectures that may support both.  And it was checked in platform code
>> and drivers only.
>> Hence the symbol is lacking from most architectures. Heck, even
>> architectures that support both may default to one endiannes, and declare
>> only the symbol for the other endianness:
>>
>> --- arch/alpha ---
>> --- arch/arc ---
>> arch/arc/Kconfig:config CPU_BIG_ENDIAN
>> --- arch/arm ---
>> arch/arm/mm/Kconfig:config CPU_BIG_ENDIAN
>> --- arch/arm64 ---
>> arch/arm64/Kconfig:config CPU_BIG_ENDIAN
>> --- arch/blackfin ---
>> --- arch/c6x ---
>> arch/c6x/Kconfig:config CPU_BIG_ENDIAN
>> --- arch/cris ---
>> --- arch/frv ---
>> --- arch/h8300 ---
>> --- arch/hexagon ---
>> --- arch/ia64 ---
>> --- arch/Kconfig ---
>> --- arch/m32r ---
>> arch/m32r/Kconfig:config CPU_LITTLE_ENDIAN
>> --- arch/m68k ---
>> --- arch/metag ---
>> --- arch/microblaze ---
>> --- arch/mips ---
>> arch/mips/Kconfig:config CPU_BIG_ENDIAN
>> arch/mips/Kconfig:config CPU_LITTLE_ENDIAN
>> --- arch/mn10300 ---
>> --- arch/nios2 ---
>> --- arch/openrisc ---
>> --- arch/parisc ---
>> --- arch/powerpc ---
>> arch/powerpc/platforms/Kconfig.cputype:config CPU_BIG_ENDIAN
>> arch/powerpc/platforms/Kconfig.cputype:config CPU_LITTLE_ENDIAN
>> --- arch/s390 ---
>> arch/s390/Kconfig:config CPU_BIG_ENDIAN
>> --- arch/score ---
>> --- arch/sh ---
>> arch/sh/Kconfig.cpu:config CPU_LITTLE_ENDIAN
>> arch/sh/Kconfig.cpu:config CPU_BIG_ENDIAN
>> --- arch/sparc ---
>> --- arch/tile ---
>> --- arch/um ---
>> --- arch/unicore32 ---
>> --- arch/x86 ---
>> --- arch/xtensa ---
>>
>> However, there are already a few users in generic code, which are thus
>> broken on many platforms:
>>
>>      drivers/of/base.c
>>      drivers/of/fdt.c
>>      drivers/tty/serial/earlycon.c
>>      drivers/tty/serial/serial_core.c
>>
>> include/asm-generic/qrwlock.h is also generic, but depends on the
>> architecture to select ARCH_USE_QUEUED_RWLOCKS, which only very few do
>> (x86, and now sparc).
>>
>> I guess the time is ripe for adding (both) symbols to all architectures?
> Good idea. I think we can do most of this by adding a few lines to
> arch/Kconfig:
>
> config CPU_BIG_ENDIAN
>          bool
>
> config CPU_LITTLE_ENDIAN
>         def_bool !CPU_BIG_ENDIAN
I noticed that even x86 does not define CPU_LITTLE_ENDIAN.  Strange.
With this code all the architecture will default to 
CONFIG_CPU_LITTLE_ENDIAN.
I can make it as a separate patch. But I can only test SPARC and little 
bit of x86.
Is that ok?

>
> This way, we only need to add 'select CPU_BIG_ENDIAN' to the
> architectures that are always big-endian, and we don't need to
> change anything for the ones that have a single 'CPU_BIG_ENDIAN'
> option.
>
> The three architectures that have a 'choice' statement (mips, ppc and
> sh) will have to convert, and m32r will have to replace the
> option with the opposite one, which could break 'make oldconfig',
> but nobody really cares about m32r any more.
>
>         Arnd

[toc] | [prev] | [next] | [standalone]

#1649697 — Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)

On Wed, May 24, 2017 at 4:45 PM, Babu Moger <babu.moger@oracle.com> wrote:
> On 5/24/2017 5:18 AM, Arnd Bergmann wrote:
>> On Wed, May 24, 2017 at 11:59 AM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>>> On Tue, May 23, 2017 at 11:45 PM, Babu Moger <babu.moger@oracle.com>
>>> wrote:
>>> include/asm-generic/qrwlock.h is also generic, but depends on the
>>> architecture to select ARCH_USE_QUEUED_RWLOCKS, which only very few do
>>> (x86, and now sparc).
>>>
>>> I guess the time is ripe for adding (both) symbols to all architectures?
>>
>> Good idea. I think we can do most of this by adding a few lines to
>> arch/Kconfig:
>>
>> config CPU_BIG_ENDIAN
>>          bool
>>
>> config CPU_LITTLE_ENDIAN
>>         def_bool !CPU_BIG_ENDIAN
>
> I noticed that even x86 does not define CPU_LITTLE_ENDIAN.  Strange.

There is no architecture-independent code that tests for
CONFIG_CPU_LITTLE_ENDIAN, unlike CONFIG_CPU_BIG_ENDIAN,
so that's not very suprising.

> With this code all the architecture will default to
> CONFIG_CPU_LITTLE_ENDIAN.

What I meant is that we have to 'select CPU_BIG_ENDIAN' on all architectures
that actually are big-endian:

These are all configurable:
$ git grep -l linux/byteorder/big_endian.h | xargs grep -l
linux/byteorder/little_endian.h
arch/arc/include/uapi/asm/byteorder.h
arch/arm/include/uapi/asm/byteorder.h
arch/arm64/include/uapi/asm/byteorder.h
arch/c6x/include/uapi/asm/byteorder.h
arch/m32r/include/uapi/asm/byteorder.h
arch/microblaze/include/uapi/asm/byteorder.h
arch/mips/include/uapi/asm/byteorder.h
arch/powerpc/include/uapi/asm/byteorder.h
arch/sh/include/uapi/asm/byteorder.h
arch/tile/include/uapi/asm/byteorder.h

These are always big-endian:
$ git grep -l linux/byteorder/big_endian.h | xargs grep -L
linux/byteorder/little_endian.h
arch/avr32/include/uapi/asm/byteorder.h
arch/frv/include/uapi/asm/byteorder.h
arch/m68k/include/uapi/asm/byteorder.h
arch/openrisc/include/uapi/asm/byteorder.h
arch/parisc/include/uapi/asm/byteorder.h
arch/s390/include/uapi/asm/byteorder.h
arch/sparc/include/uapi/asm/byteorder.h

And these are always little-endian:
arch/alpha/include/uapi/asm/byteorder.h
arch/blackfin/include/uapi/asm/byteorder.h
arch/cris/include/uapi/asm/byteorder.h
arch/hexagon/include/uapi/asm/byteorder.h
arch/ia64/include/uapi/asm/byteorder.h
arch/metag/include/uapi/asm/byteorder.h
arch/mn10300/include/uapi/asm/byteorder.h
arch/score/include/uapi/asm/byteorder.h
arch/unicore32/include/uapi/asm/byteorder.h
arch/x86/include/uapi/asm/byteorder.h

So if we 'select CPU_BIG_ENDIAN' from avr32, frv, m68k, openrisc, parisc,
s390 and sparc, this covers all the fixed-endian architectures, and the
other ones are those that already have either CPU_BIG_ENDIAN
as a 'bool' option, or both as a 'choice'.

> I can make it as a separate patch. But I can only test SPARC and little bit
> of x86. Is that ok?

I think that's ok.

        Arnd

[toc] | [prev] | [next] | [standalone]

#1649780 — Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)

On 5/24/2017 10:09 AM, Arnd Bergmann wrote:
> On Wed, May 24, 2017 at 4:45 PM, Babu Moger <babu.moger@oracle.com> wrote:
>> On 5/24/2017 5:18 AM, Arnd Bergmann wrote:
>>> On Wed, May 24, 2017 at 11:59 AM, Geert Uytterhoeven
>>> <geert@linux-m68k.org> wrote:
>>>> On Tue, May 23, 2017 at 11:45 PM, Babu Moger <babu.moger@oracle.com>
>>>> wrote:
>>>> include/asm-generic/qrwlock.h is also generic, but depends on the
>>>> architecture to select ARCH_USE_QUEUED_RWLOCKS, which only very few do
>>>> (x86, and now sparc).
>>>>
>>>> I guess the time is ripe for adding (both) symbols to all architectures?
>>> Good idea. I think we can do most of this by adding a few lines to
>>> arch/Kconfig:
>>>
>>> config CPU_BIG_ENDIAN
>>>           bool
>>>
>>> config CPU_LITTLE_ENDIAN
>>>          def_bool !CPU_BIG_ENDIAN
>> I noticed that even x86 does not define CPU_LITTLE_ENDIAN.  Strange.
> There is no architecture-independent code that tests for
> CONFIG_CPU_LITTLE_ENDIAN, unlike CONFIG_CPU_BIG_ENDIAN,
> so that's not very suprising.

Ok. Thanks
>> With this code all the architecture will default to
>> CONFIG_CPU_LITTLE_ENDIAN.
> What I meant is that we have to 'select CPU_BIG_ENDIAN' on all architectures
> that actually are big-endian:
Ok. Sure.
>
> These are all configurable:
> $ git grep -l linux/byteorder/big_endian.h | xargs grep -l
> linux/byteorder/little_endian.h
> arch/arc/include/uapi/asm/byteorder.h
> arch/arm/include/uapi/asm/byteorder.h
> arch/arm64/include/uapi/asm/byteorder.h
> arch/c6x/include/uapi/asm/byteorder.h
> arch/m32r/include/uapi/asm/byteorder.h
> arch/microblaze/include/uapi/asm/byteorder.h
> arch/mips/include/uapi/asm/byteorder.h
> arch/powerpc/include/uapi/asm/byteorder.h
> arch/sh/include/uapi/asm/byteorder.h
> arch/tile/include/uapi/asm/byteorder.h
>
> These are always big-endian:
> $ git grep -l linux/byteorder/big_endian.h | xargs grep -L
> linux/byteorder/little_endian.h
> arch/avr32/include/uapi/asm/byteorder.h
> arch/frv/include/uapi/asm/byteorder.h
> arch/m68k/include/uapi/asm/byteorder.h
> arch/openrisc/include/uapi/asm/byteorder.h
> arch/parisc/include/uapi/asm/byteorder.h
> arch/s390/include/uapi/asm/byteorder.h
> arch/sparc/include/uapi/asm/byteorder.h
>
> And these are always little-endian:
> arch/alpha/include/uapi/asm/byteorder.h
> arch/blackfin/include/uapi/asm/byteorder.h
> arch/cris/include/uapi/asm/byteorder.h
> arch/hexagon/include/uapi/asm/byteorder.h
> arch/ia64/include/uapi/asm/byteorder.h
> arch/metag/include/uapi/asm/byteorder.h
> arch/mn10300/include/uapi/asm/byteorder.h
> arch/score/include/uapi/asm/byteorder.h
> arch/unicore32/include/uapi/asm/byteorder.h
> arch/x86/include/uapi/asm/byteorder.h
>
> So if we 'select CPU_BIG_ENDIAN' from avr32, frv, m68k, openrisc, parisc,
> s390 and sparc, this covers all the fixed-endian architectures, and the
> other ones are those that already have either CPU_BIG_ENDIAN
> as a 'bool' option, or both as a 'choice'.
Ok.  Great details. I think I have all the details required for the 
first version.  Will post it soon. Thanks

>> I can make it as a separate patch. But I can only test SPARC and little bit
>> of x86. Is that ok?
> I think that's ok.
>
>          Arnd

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web