| Message ID | 20190411115623.5749-3-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/9] riscv: use asm-generic/extable.h | expand |
Στις 2019-04-11 14:56, Christoph Hellwig έγραψε: > RISC-V is always little endian. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > arch/riscv/include/asm/uaccess.h | 9 +-------- > 1 file changed, 1 insertion(+), 8 deletions(-) > > diff --git a/arch/riscv/include/asm/uaccess.h > b/arch/riscv/include/asm/uaccess.h > index cc5b253d4c57..5c50ed6f8c93 100644 > --- a/arch/riscv/include/asm/uaccess.h > +++ b/arch/riscv/include/asm/uaccess.h > @@ -99,15 +99,8 @@ static inline int __access_ok(unsigned long addr, > unsigned long size) > * on our cache or tlb entries. > */ > > -#if defined(__LITTLE_ENDIAN) > -#define __MSW 1 > #define __LSW 0 > -#elif defined(__BIG_ENDIAN) > -#define __MSW 0 > -#define __LSW 1 > -#else > -#error "Unknown endianness" > -#endif > +#define __MSW 1 > > /* > * The "__xxx" versions of the user access functions do not verify the > address From the ISA manual: "We chose little-endian byte ordering for the RISC-V memory system because little-endian systems are currently dominant commercially (all x86 systems; iOS, Android, and Windows for ARM). A minor point is that we have also found little-endian memory systems to be more natural for hardware designers. However, certain application areas, such as IP networking, operate on big-endian data structures, and certain legacy code bases have been built assuming big-endian processors, so we expect that future specifications will describe big-endian or bi-endian variants of RISC-V." I don't think we can definitely say that RISC-V will always have a little-endian, memory system only that it is little-endian for now. Also this code acts as a check, I'd prefer if you put an error (something like "unsupported endianess, check your toolchain") in case __BIG_ENDIAN was defined instead of completely removing the check. Regards, Nick
On Thu, Apr 11, 2019 at 06:40:07PM +0300, Nick Kossifidis wrote: > I don't think we can definitely say that RISC-V will always have a > little-endian, > memory system only that it is little-endian for now. Also this code acts as > a check, And we don't know if Linux will be around if that ever changes. The point is: a) the current RISC-V spec is LE only b) the current linux port is LE only except for this little bit There is no point in leaving just this bitrotting code around. It just confuses developers, (very very slightly) slows down compiles and will bitrot. It also won't be any significant help to a future developer down the road doing a hypothetical BE RISC-V Linux port.
Στις 2019-04-11 18:47, Christoph Hellwig έγραψε: > On Thu, Apr 11, 2019 at 06:40:07PM +0300, Nick Kossifidis wrote: >> I don't think we can definitely say that RISC-V will always have a >> little-endian, >> memory system only that it is little-endian for now. Also this code >> acts as >> a check, > > And we don't know if Linux will be around if that ever changes. > > The point is: > > a) the current RISC-V spec is LE only > b) the current linux port is LE only except for this little bit > > There is no point in leaving just this bitrotting code around. It > just confuses developers, (very very slightly) slows down compiles > and will bitrot. It also won't be any significant help to a future > developer down the road doing a hypothetical BE RISC-V Linux port. I think we should at least warn the user about unsupported endianess, also I suggest you also clean up include/asm/elf.h.
On Thu, Apr 11, 2019 at 07:08:30PM +0300, Nick Kossifidis wrote: > I think we should at least warn the user about unsupported endianess, The endianess is selected by arch/riscv/include/uapi/asm/byteorder.h including include/uapi/linux/byteorder/little_endian.h, which then defines __LITTLE_ENDIAN. So we'd really need to check one part of the kernel port agrees with others, which is a little odd. And in which place do we check that and in which one not? It isn't like other ports like x86 are full of such just because checks.. > also I suggest you also clean up include/asm/elf.h. Agreed.
Στις 2019-04-11 19:31, Christoph Hellwig έγραψε: > On Thu, Apr 11, 2019 at 07:08:30PM +0300, Nick Kossifidis wrote: >> I think we should at least warn the user about unsupported endianess, > > The endianess is selected by arch/riscv/include/uapi/asm/byteorder.h > including include/uapi/linux/byteorder/little_endian.h, which then > defines __LITTLE_ENDIAN. So we'd really need to check one part of > the kernel port agrees with others, which is a little odd. And in > which place do we check that and in which one not? It isn't like > other ports like x86 are full of such just because checks.. > You got a point there, how about something like: #if __BYTE_ORDER__ != __ORDER_LITTLE_ENDIAN__ #error "Unsupported endianess, check your toolchain" #endif on arch/riscv/include/uapi/asm/byteorder.h ?
On Thu, Apr 11, 2019 at 07:47:20PM +0300, Nick Kossifidis wrote: >> The endianess is selected by arch/riscv/include/uapi/asm/byteorder.h >> including include/uapi/linux/byteorder/little_endian.h, which then >> defines __LITTLE_ENDIAN. So we'd really need to check one part of >> the kernel port agrees with others, which is a little odd. And in >> which place do we check that and in which one not? It isn't like >> other ports like x86 are full of such just because checks.. >> > > You got a point there, how about something like: > > #if __BYTE_ORDER__ != __ORDER_LITTLE_ENDIAN__ > #error "Unsupported endianess, check your toolchain" > #endif > > on arch/riscv/include/uapi/asm/byteorder.h ? I'm not really sold on it. But I can do it as a separate patch and we'll see if anyone likes it.. In fact it might make sense to just add this to the generic byte order headers so that RISC-V doesn't stand out too much, which would seem a little more sensible.
diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h index cc5b253d4c57..5c50ed6f8c93 100644 --- a/arch/riscv/include/asm/uaccess.h +++ b/arch/riscv/include/asm/uaccess.h @@ -99,15 +99,8 @@ static inline int __access_ok(unsigned long addr, unsigned long size) * on our cache or tlb entries. */ -#if defined(__LITTLE_ENDIAN) -#define __MSW 1 #define __LSW 0 -#elif defined(__BIG_ENDIAN) -#define __MSW 0 -#define __LSW 1 -#else -#error "Unknown endianness" -#endif +#define __MSW 1 /* * The "__xxx" versions of the user access functions do not verify the address
RISC-V is always little endian. Signed-off-by: Christoph Hellwig <hch@lst.de> --- arch/riscv/include/asm/uaccess.h | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)