* [PATCH] powerpc: Make set_endian() return EINVAL when not supporting little endian
@ 2021-08-25 13:43 ` Christophe Leroy
0 siblings, 0 replies; 8+ messages in thread
From: Christophe Leroy @ 2021-08-25 13:43 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linux-kernel, linuxppc-dev
There is no point in modifying MSR_LE bit on CPUs not supporting
little endian.
Just like done for get_endian(), make set_endian() return
EINVAL in that case.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/kernel/process.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 185beb290580..b2b9919795a2 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1995,6 +1995,10 @@ int set_endian(struct task_struct *tsk, unsigned int val)
{
struct pt_regs *regs = tsk->thread.regs;
+ if (!cpu_has_feature(CPU_FTR_PPC_LE) &&
+ !cpu_has_feature(CPU_FTR_REAL_LE))
+ return -EINVAL;
+
if ((val == PR_ENDIAN_LITTLE && !cpu_has_feature(CPU_FTR_REAL_LE)) ||
(val == PR_ENDIAN_PPC_LITTLE && !cpu_has_feature(CPU_FTR_PPC_LE)))
return -EINVAL;
--
2.25.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH] powerpc: Make set_endian() return EINVAL when not supporting little endian
@ 2021-08-25 13:43 ` Christophe Leroy
0 siblings, 0 replies; 8+ messages in thread
From: Christophe Leroy @ 2021-08-25 13:43 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
There is no point in modifying MSR_LE bit on CPUs not supporting
little endian.
Just like done for get_endian(), make set_endian() return
EINVAL in that case.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/kernel/process.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 185beb290580..b2b9919795a2 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1995,6 +1995,10 @@ int set_endian(struct task_struct *tsk, unsigned int val)
{
struct pt_regs *regs = tsk->thread.regs;
+ if (!cpu_has_feature(CPU_FTR_PPC_LE) &&
+ !cpu_has_feature(CPU_FTR_REAL_LE))
+ return -EINVAL;
+
if ((val == PR_ENDIAN_LITTLE && !cpu_has_feature(CPU_FTR_REAL_LE)) ||
(val == PR_ENDIAN_PPC_LITTLE && !cpu_has_feature(CPU_FTR_PPC_LE)))
return -EINVAL;
--
2.25.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] powerpc: Make set_endian() return EINVAL when not supporting little endian
2021-08-25 13:43 ` Christophe Leroy
@ 2021-08-26 3:41 ` Michael Ellerman
-1 siblings, 0 replies; 8+ messages in thread
From: Michael Ellerman @ 2021-08-26 3:41 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
Cc: linux-kernel, linuxppc-dev
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> There is no point in modifying MSR_LE bit on CPUs not supporting
> little endian.
Isn't that an ABI break?
set_endian(PR_ENDIAN_BIG) should work on a big endian CPU, even if it
does nothing useful.
cheers
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 185beb290580..b2b9919795a2 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -1995,6 +1995,10 @@ int set_endian(struct task_struct *tsk, unsigned int val)
> {
> struct pt_regs *regs = tsk->thread.regs;
>
> + if (!cpu_has_feature(CPU_FTR_PPC_LE) &&
> + !cpu_has_feature(CPU_FTR_REAL_LE))
> + return -EINVAL;
> +
> if ((val == PR_ENDIAN_LITTLE && !cpu_has_feature(CPU_FTR_REAL_LE)) ||
> (val == PR_ENDIAN_PPC_LITTLE && !cpu_has_feature(CPU_FTR_PPC_LE)))
> return -EINVAL;
> --
> 2.25.0
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] powerpc: Make set_endian() return EINVAL when not supporting little endian
@ 2021-08-26 3:41 ` Michael Ellerman
0 siblings, 0 replies; 8+ messages in thread
From: Michael Ellerman @ 2021-08-26 3:41 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> There is no point in modifying MSR_LE bit on CPUs not supporting
> little endian.
Isn't that an ABI break?
set_endian(PR_ENDIAN_BIG) should work on a big endian CPU, even if it
does nothing useful.
cheers
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 185beb290580..b2b9919795a2 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -1995,6 +1995,10 @@ int set_endian(struct task_struct *tsk, unsigned int val)
> {
> struct pt_regs *regs = tsk->thread.regs;
>
> + if (!cpu_has_feature(CPU_FTR_PPC_LE) &&
> + !cpu_has_feature(CPU_FTR_REAL_LE))
> + return -EINVAL;
> +
> if ((val == PR_ENDIAN_LITTLE && !cpu_has_feature(CPU_FTR_REAL_LE)) ||
> (val == PR_ENDIAN_PPC_LITTLE && !cpu_has_feature(CPU_FTR_PPC_LE)))
> return -EINVAL;
> --
> 2.25.0
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] powerpc: Make set_endian() return EINVAL when not supporting little endian
2021-08-26 3:41 ` Michael Ellerman
@ 2021-08-26 4:55 ` Christophe Leroy
-1 siblings, 0 replies; 8+ messages in thread
From: Christophe Leroy @ 2021-08-26 4:55 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras
Cc: linux-kernel, linuxppc-dev
Le 26/08/2021 à 05:41, Michael Ellerman a écrit :
> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>> There is no point in modifying MSR_LE bit on CPUs not supporting
>> little endian.
>
> Isn't that an ABI break?
Or an ABI fix ? I don't know.
My first thought was that all other 32 bits architectures were returning -EINVAL, but looking at the
man page of prctl, it is explicit that this is powerpc only.
>
> set_endian(PR_ENDIAN_BIG) should work on a big endian CPU, even if it
> does nothing useful.
Fair enough. But shouldn't in that case get_endian() return PR_ENDIAN_BIG instead of returning EINVAL ?
We can do one or the other, but I think it should at least be consistant between them, shouldn't it ?
Christophe
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] powerpc: Make set_endian() return EINVAL when not supporting little endian
@ 2021-08-26 4:55 ` Christophe Leroy
0 siblings, 0 replies; 8+ messages in thread
From: Christophe Leroy @ 2021-08-26 4:55 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
Le 26/08/2021 à 05:41, Michael Ellerman a écrit :
> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>> There is no point in modifying MSR_LE bit on CPUs not supporting
>> little endian.
>
> Isn't that an ABI break?
Or an ABI fix ? I don't know.
My first thought was that all other 32 bits architectures were returning -EINVAL, but looking at the
man page of prctl, it is explicit that this is powerpc only.
>
> set_endian(PR_ENDIAN_BIG) should work on a big endian CPU, even if it
> does nothing useful.
Fair enough. But shouldn't in that case get_endian() return PR_ENDIAN_BIG instead of returning EINVAL ?
We can do one or the other, but I think it should at least be consistant between them, shouldn't it ?
Christophe
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] powerpc: Make set_endian() return EINVAL when not supporting little endian
2021-08-26 4:55 ` Christophe Leroy
@ 2021-08-26 14:23 ` Michael Ellerman
-1 siblings, 0 replies; 8+ messages in thread
From: Michael Ellerman @ 2021-08-26 14:23 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
Cc: linux-kernel, linuxppc-dev
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 26/08/2021 à 05:41, Michael Ellerman a écrit :
>> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>>> There is no point in modifying MSR_LE bit on CPUs not supporting
>>> little endian.
>>
>> Isn't that an ABI break?
>
> Or an ABI fix ? I don't know.
It could break existing applications, even if the new semantics make
more sense. So that's a break IMHO :)
> My first thought was that all other 32 bits architectures were returning -EINVAL, but looking at the
> man page of prctl, it is explicit that this is powerpc only.
It could be generic, but yeah seems we're the only arch that implements
it.
>> set_endian(PR_ENDIAN_BIG) should work on a big endian CPU, even if it
>> does nothing useful.
>
> Fair enough. But shouldn't in that case get_endian() return PR_ENDIAN_BIG instead of returning EINVAL ?
> We can do one or the other, but I think it should at least be consistant between them, shouldn't it ?
It should be consistent, but it isn't, and if we change it now we
potentially break existing userspace, which is bad.
I don't think it's widely used, and the risk of breakage would be
minimal, but it's not zero.
So I'm not sure it's worth changing it just for the sake of consistency.
cheers
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] powerpc: Make set_endian() return EINVAL when not supporting little endian
@ 2021-08-26 14:23 ` Michael Ellerman
0 siblings, 0 replies; 8+ messages in thread
From: Michael Ellerman @ 2021-08-26 14:23 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 26/08/2021 à 05:41, Michael Ellerman a écrit :
>> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>>> There is no point in modifying MSR_LE bit on CPUs not supporting
>>> little endian.
>>
>> Isn't that an ABI break?
>
> Or an ABI fix ? I don't know.
It could break existing applications, even if the new semantics make
more sense. So that's a break IMHO :)
> My first thought was that all other 32 bits architectures were returning -EINVAL, but looking at the
> man page of prctl, it is explicit that this is powerpc only.
It could be generic, but yeah seems we're the only arch that implements
it.
>> set_endian(PR_ENDIAN_BIG) should work on a big endian CPU, even if it
>> does nothing useful.
>
> Fair enough. But shouldn't in that case get_endian() return PR_ENDIAN_BIG instead of returning EINVAL ?
> We can do one or the other, but I think it should at least be consistant between them, shouldn't it ?
It should be consistent, but it isn't, and if we change it now we
potentially break existing userspace, which is bad.
I don't think it's widely used, and the risk of breakage would be
minimal, but it's not zero.
So I'm not sure it's worth changing it just for the sake of consistency.
cheers
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-08-26 14:24 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-25 13:43 [PATCH] powerpc: Make set_endian() return EINVAL when not supporting little endian Christophe Leroy
2021-08-25 13:43 ` Christophe Leroy
2021-08-26 3:41 ` Michael Ellerman
2021-08-26 3:41 ` Michael Ellerman
2021-08-26 4:55 ` Christophe Leroy
2021-08-26 4:55 ` Christophe Leroy
2021-08-26 14:23 ` Michael Ellerman
2021-08-26 14:23 ` Michael Ellerman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.