diff mbox

arm64: mm: support instruction SETEND

Message ID 1420609930-1689-1-git-send-email-leo.yan@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Leo Yan Jan. 7, 2015, 5:52 a.m. UTC
Currently kernel has set the bit SCTLR_EL1.SED, so the SETEND
instruction will be treated as UNALLOCATED; this error can be
reproduced when ARMv8 cpu runs with EL1/aarch64 and EL0/aarch32
mode, finally kernel will trap the exception if the userspace
libs use SETEND instruction.

So this patch clears bit SCTLR_EL1.SED to support SETEND instruction.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
Signed-off-by: Xiaolong Ye <yexl@marvell.com>
---
 arch/arm64/mm/proc.S | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Suzuki K Poulose Jan. 7, 2015, 10:10 a.m. UTC | #1
On 07/01/15 05:52, Leo Yan wrote:
> Currently kernel has set the bit SCTLR_EL1.SED, so the SETEND
> instruction will be treated as UNALLOCATED; this error can be
> reproduced when ARMv8 cpu runs with EL1/aarch64 and EL0/aarch32
> mode, finally kernel will trap the exception if the userspace
> libs use SETEND instruction.
>
> So this patch clears bit SCTLR_EL1.SED to support SETEND instruction.
>
The best way to do this, is via the instruction emulation framework 
added by Punit, which handles the armv8 deprecated/obsoleted 
instructions. This is now queued for 3.19.
I have a patchset which adds the 'SETEND' emulation support to the 
framework. This will enable better handling of the feature, including 
finding out the users of the deprecated instruction (when we switch to 
the emulation mode).

Btw, there is one open question that I am seeking answer for.

What should be the endianness of the signal handlers ? Should we leave 
it to the application ? Or restore the 'default' endianness for the 
signal handler ?


Thanks
Suzuki


> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> Signed-off-by: Xiaolong Ye <yexl@marvell.com>
> ---
>   arch/arm64/mm/proc.S | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> index 4e778b1..66a7363 100644
> --- a/arch/arm64/mm/proc.S
> +++ b/arch/arm64/mm/proc.S
> @@ -249,9 +249,9 @@ ENDPROC(__cpu_setup)
>   	 *       CE0      XWHW CZ     ME TEEA S
>   	 * .... .IEE .... NEAI TE.I ..AD DEN0 ACAM
>   	 * 0011 0... 1101 ..0. ..0. 10.. .... .... < hardware reserved
> -	 * .... .1.. .... 01.1 11.1 ..01 0001 1101 < software settings
> +	 * .... .1.. .... 01.1 11.1 ..00 0001 1101 < software settings
>   	 */
>   	.type	crval, #object
>   crval:
> -	.word	0x000802e2			// clear
> -	.word	0x0405d11d			// set
> +	.word	0x000803e2			// clear
> +	.word	0x0405d01d			// set
>
Will Deacon Jan. 7, 2015, 10:25 a.m. UTC | #2
On Wed, Jan 07, 2015 at 10:10:34AM +0000, Suzuki K. Poulose wrote:
> On 07/01/15 05:52, Leo Yan wrote:
> > Currently kernel has set the bit SCTLR_EL1.SED, so the SETEND
> > instruction will be treated as UNALLOCATED; this error can be
> > reproduced when ARMv8 cpu runs with EL1/aarch64 and EL0/aarch32
> > mode, finally kernel will trap the exception if the userspace
> > libs use SETEND instruction.
> >
> > So this patch clears bit SCTLR_EL1.SED to support SETEND instruction.
> >
> The best way to do this, is via the instruction emulation framework 
> added by Punit, which handles the armv8 deprecated/obsoleted 
> instructions. This is now queued for 3.19.
> I have a patchset which adds the 'SETEND' emulation support to the 
> framework. This will enable better handling of the feature, including 
> finding out the users of the deprecated instruction (when we switch to 
> the emulation mode).
> 
> Btw, there is one open question that I am seeking answer for.
> 
> What should be the endianness of the signal handlers ? Should we leave 
> it to the application ? Or restore the 'default' endianness for the 
> signal handler ?

I think we should restore the default endianness, otherwise you're
essentially forcing signal handlers to do a setend as their first
instruction to get into a consistent state. That also matches the endianness
of the sigframe that we push onto the stack, right?

setjmp/longjmp could be fun, but I think that an application would need
to take care not to make endianness assumptions across those anyway.

Will
Leo Yan Jan. 7, 2015, 10:58 a.m. UTC | #3
On Wed, Jan 07, 2015 at 10:10:34AM +0000, Suzuki K. Poulose wrote:
> On 07/01/15 05:52, Leo Yan wrote:
> >Currently kernel has set the bit SCTLR_EL1.SED, so the SETEND
> >instruction will be treated as UNALLOCATED; this error can be
> >reproduced when ARMv8 cpu runs with EL1/aarch64 and EL0/aarch32
> >mode, finally kernel will trap the exception if the userspace
> >libs use SETEND instruction.
> >
> >So this patch clears bit SCTLR_EL1.SED to support SETEND instruction.
> >
> The best way to do this, is via the instruction emulation framework
> added by Punit, which handles the armv8 deprecated/obsoleted
> instructions. This is now queued for 3.19.
> I have a patchset which adds the 'SETEND' emulation support to the
> framework. This will enable better handling of the feature,
> including finding out the users of the deprecated instruction (when
> we switch to the emulation mode).
> 

i'm a little confuse for this point:

if the deprecated instructions cannot be supported by CPU, then only
can use emulation; on the other hand, if CPU can natively support the
deprecated instruction, why we cannot directly enable this h/w feature?
if use the emulation mode, suppose here will have performance penalty.

how about u think for this? :-)

> >Signed-off-by: Leo Yan <leo.yan@linaro.org>
> >Signed-off-by: Xiaolong Ye <yexl@marvell.com>
> >---
> >  arch/arm64/mm/proc.S | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> >diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> >index 4e778b1..66a7363 100644
> >--- a/arch/arm64/mm/proc.S
> >+++ b/arch/arm64/mm/proc.S
> >@@ -249,9 +249,9 @@ ENDPROC(__cpu_setup)
> >  	 *       CE0      XWHW CZ     ME TEEA S
> >  	 * .... .IEE .... NEAI TE.I ..AD DEN0 ACAM
> >  	 * 0011 0... 1101 ..0. ..0. 10.. .... .... < hardware reserved
> >-	 * .... .1.. .... 01.1 11.1 ..01 0001 1101 < software settings
> >+	 * .... .1.. .... 01.1 11.1 ..00 0001 1101 < software settings
> >  	 */
> >  	.type	crval, #object
> >  crval:
> >-	.word	0x000802e2			// clear
> >-	.word	0x0405d11d			// set
> >+	.word	0x000803e2			// clear
> >+	.word	0x0405d01d			// set
> >
> 
>
Will Deacon Jan. 7, 2015, 11:11 a.m. UTC | #4
On Wed, Jan 07, 2015 at 10:58:24AM +0000, Leo Yan wrote:
> On Wed, Jan 07, 2015 at 10:10:34AM +0000, Suzuki K. Poulose wrote:
> > On 07/01/15 05:52, Leo Yan wrote:
> > >Currently kernel has set the bit SCTLR_EL1.SED, so the SETEND
> > >instruction will be treated as UNALLOCATED; this error can be
> > >reproduced when ARMv8 cpu runs with EL1/aarch64 and EL0/aarch32
> > >mode, finally kernel will trap the exception if the userspace
> > >libs use SETEND instruction.
> > >
> > >So this patch clears bit SCTLR_EL1.SED to support SETEND instruction.
> > >
> > The best way to do this, is via the instruction emulation framework
> > added by Punit, which handles the armv8 deprecated/obsoleted
> > instructions. This is now queued for 3.19.
> > I have a patchset which adds the 'SETEND' emulation support to the
> > framework. This will enable better handling of the feature,
> > including finding out the users of the deprecated instruction (when
> > we switch to the emulation mode).
> > 
> 
> i'm a little confuse for this point:
> 
> if the deprecated instructions cannot be supported by CPU, then only
> can use emulation; on the other hand, if CPU can natively support the
> deprecated instruction, why we cannot directly enable this h/w feature?
> if use the emulation mode, suppose here will have performance penalty.
> 
> how about u think for this? :-)

A *huge* advantage of emulation is that we can print a diagnostic to dmesg
warning the user that they are making use of a CPU feature that is likely to
disappear from future revisions of the hardware. We've not been great at
doing this in the past, which led to all the fun around SWP emulation that
you can find in the list archives.

Furthermore, I think the emulation framework does allow the hardware support
to be enabled, it just doesn't make that the default behaviour.

In other words; use the emulation to find out where SETEND is being used
and fix those applications wherever you can. In the cases where you're
stuck with a legacy binary, you can enable CPU support if it is available
but that's not a longterm solution.

Will
Leo Yan Jan. 7, 2015, 2:06 p.m. UTC | #5
On Wed, Jan 07, 2015 at 11:11:48AM +0000, Will Deacon wrote:
> On Wed, Jan 07, 2015 at 10:58:24AM +0000, Leo Yan wrote:
> > On Wed, Jan 07, 2015 at 10:10:34AM +0000, Suzuki K. Poulose wrote:
> > > On 07/01/15 05:52, Leo Yan wrote:
> > > >Currently kernel has set the bit SCTLR_EL1.SED, so the SETEND
> > > >instruction will be treated as UNALLOCATED; this error can be
> > > >reproduced when ARMv8 cpu runs with EL1/aarch64 and EL0/aarch32
> > > >mode, finally kernel will trap the exception if the userspace
> > > >libs use SETEND instruction.
> > > >
> > > >So this patch clears bit SCTLR_EL1.SED to support SETEND instruction.
> > > >
> > > The best way to do this, is via the instruction emulation framework
> > > added by Punit, which handles the armv8 deprecated/obsoleted
> > > instructions. This is now queued for 3.19.
> > > I have a patchset which adds the 'SETEND' emulation support to the
> > > framework. This will enable better handling of the feature,
> > > including finding out the users of the deprecated instruction (when
> > > we switch to the emulation mode).
> > > 
> > 
> > i'm a little confuse for this point:
> > 
> > if the deprecated instructions cannot be supported by CPU, then only
> > can use emulation; on the other hand, if CPU can natively support the
> > deprecated instruction, why we cannot directly enable this h/w feature?
> > if use the emulation mode, suppose here will have performance penalty.
> > 
> > how about u think for this? :-)
> 
> A *huge* advantage of emulation is that we can print a diagnostic to dmesg
> warning the user that they are making use of a CPU feature that is likely to
> disappear from future revisions of the hardware. We've not been great at
> doing this in the past, which led to all the fun around SWP emulation that
> you can find in the list archives.
> 
> Furthermore, I think the emulation framework does allow the hardware support
> to be enabled, it just doesn't make that the default behaviour.
> 
> In other words; use the emulation to find out where SETEND is being used
> and fix those applications wherever you can. In the cases where you're
> stuck with a legacy binary, you can enable CPU support if it is available
> but that's not a longterm solution.
> 

Thanks for clarification; Let's use emulation framework as formal method.

Thanks,
Leo Yan
Catalin Marinas Jan. 7, 2015, 2:25 p.m. UTC | #6
On Wed, Jan 07, 2015 at 10:25:43AM +0000, Will Deacon wrote:
> On Wed, Jan 07, 2015 at 10:10:34AM +0000, Suzuki K. Poulose wrote:
> > On 07/01/15 05:52, Leo Yan wrote:
> > > Currently kernel has set the bit SCTLR_EL1.SED, so the SETEND
> > > instruction will be treated as UNALLOCATED; this error can be
> > > reproduced when ARMv8 cpu runs with EL1/aarch64 and EL0/aarch32
> > > mode, finally kernel will trap the exception if the userspace
> > > libs use SETEND instruction.
> > >
> > > So this patch clears bit SCTLR_EL1.SED to support SETEND instruction.
> > >
> > The best way to do this, is via the instruction emulation framework 
> > added by Punit, which handles the armv8 deprecated/obsoleted 
> > instructions. This is now queued for 3.19.
> > I have a patchset which adds the 'SETEND' emulation support to the 
> > framework. This will enable better handling of the feature, including 
> > finding out the users of the deprecated instruction (when we switch to 
> > the emulation mode).
> > 
> > Btw, there is one open question that I am seeking answer for.
> > 
> > What should be the endianness of the signal handlers ? Should we leave 
> > it to the application ? Or restore the 'default' endianness for the 
> > signal handler ?
> 
> I think we should restore the default endianness, otherwise you're
> essentially forcing signal handlers to do a setend as their first
> instruction to get into a consistent state. That also matches the endianness
> of the sigframe that we push onto the stack, right?

It looks like this is what setup_return() already does on arm32
(switching to the default endianness).

Now that we plan to allow SETEND in AArch32 applications, we need
something similar in the arm64 compat_setup_return().
diff mbox

Patch

diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
index 4e778b1..66a7363 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -249,9 +249,9 @@  ENDPROC(__cpu_setup)
 	 *       CE0      XWHW CZ     ME TEEA S
 	 * .... .IEE .... NEAI TE.I ..AD DEN0 ACAM
 	 * 0011 0... 1101 ..0. ..0. 10.. .... .... < hardware reserved
-	 * .... .1.. .... 01.1 11.1 ..01 0001 1101 < software settings
+	 * .... .1.. .... 01.1 11.1 ..00 0001 1101 < software settings
 	 */
 	.type	crval, #object
 crval:
-	.word	0x000802e2			// clear
-	.word	0x0405d11d			// set
+	.word	0x000803e2			// clear
+	.word	0x0405d01d			// set