[AArch32] Add support for HLT to Mixed Mode models

Message ID 20190207160553.GA2205@arm.com
State New
Headers show
Series
  • [AArch32] Add support for HLT to Mixed Mode models
Related show

Commit Message

Tamar Christina Feb. 7, 2019, 4:05 p.m.
Hi All,

The Semihosting v2 protocol requires us to output the Armv8-a HLT instruction
when in mixed mode (SEMIHOST_V2_MIXED_MODE), however it also requires this to
be done for Armv7-a and earlier architectures.

The HLT instruction is defined in the undefined encoding space for older
architectures but simulators such as QEMU already trap on it [1] for all
architectures and is a requirement for semihosting v2 [2]

Unfortunately the GAS restricts the use of HLT to Armv8-a which requires us to
use the instruction encodings we want directly in crt0.

This patch does this, I have not updated newlib/libc/* as that is quite out of
date already.  A proper sync is needed in order to get things back in sync.

A different patch for this would be best.

[1] https://github.com/qemu/qemu/commit/19a6e31c9d2701ef648b70ddcfc3bf64cec8c37e
[2] https://developer.arm.com/docs/100863/latest/the-semihosting-interface

Regtested on arm-none-eabi and no issues.

Ok for trunk?

Thanks,
Tamar

libgloss/ChangeLog:

2019-02-07  Tamar Christina  <tamar.christina@arm.com>

	PR libgloss/24070
	* arm/crt0.S: Convert macros to function Macros.
	* arm/swi.h (AngelSWI_ARM, AngelSWI): Use raw insn encoding for
	SEMIHOST_V2_MIXED_MODE case.
	(AngelSWIAsm): Make function.
	(AngelSWIInsn, AngelSWIAsm): Use .inst directive as instruction.

--

Comments

Corinna Vinschen Feb. 7, 2019, 6:26 p.m. | #1
On Feb  7 16:05, Tamar Christina wrote:
> Hi All,

> 

> The Semihosting v2 protocol requires us to output the Armv8-a HLT instruction

> when in mixed mode (SEMIHOST_V2_MIXED_MODE), however it also requires this to

> be done for Armv7-a and earlier architectures.

> 

> The HLT instruction is defined in the undefined encoding space for older

> architectures but simulators such as QEMU already trap on it [1] for all

> architectures and is a requirement for semihosting v2 [2]

> 

> Unfortunately the GAS restricts the use of HLT to Armv8-a which requires us to

> use the instruction encodings we want directly in crt0.

> 

> This patch does this, I have not updated newlib/libc/* as that is quite out of

> date already.  A proper sync is needed in order to get things back in sync.

> 

> A different patch for this would be best.

> 

> [1] https://github.com/qemu/qemu/commit/19a6e31c9d2701ef648b70ddcfc3bf64cec8c37e

> [2] https://developer.arm.com/docs/100863/latest/the-semihosting-interface

> 

> Regtested on arm-none-eabi and no issues.

> 

> Ok for trunk?


Sorry, no.

> Thanks,

> Tamar

> 

> libgloss/ChangeLog:

> 

> 2019-02-07  Tamar Christina  <tamar.christina@arm.com>

> 

> 	PR libgloss/24070

> 	* arm/crt0.S: Convert macros to function Macros.

> 	* arm/swi.h (AngelSWI_ARM, AngelSWI): Use raw insn encoding for

> 	SEMIHOST_V2_MIXED_MODE case.

> 	(AngelSWIAsm): Make function.

> 	(AngelSWIInsn, AngelSWIAsm): Use .inst directive as instruction.


Did you have a look into libgloss/ChangeLog lately?  We're using git for
three years now.  Please provide patches in git format-patch format and
provide a normal git log entry explaining what the patch does and why.
We don't do CVS ChangeLogs anymore.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat
Tamar Christina Feb. 8, 2019, 10:21 a.m. | #2
Hi Corinna,

So sorry, I had been busy sending patches all day and forgot about the different
format for newlib.

I have updated my scripts to generate git format patches for newlib so it
shouldn't happen again!

Ok for master?

Thanks,
Tamar

The 02/07/2019 19:26, Corinna Vinschen wrote:
> On Feb  7 16:05, Tamar Christina wrote:

> > Hi All,

> > 

> > The Semihosting v2 protocol requires us to output the Armv8-a HLT instruction

> > when in mixed mode (SEMIHOST_V2_MIXED_MODE), however it also requires this to

> > be done for Armv7-a and earlier architectures.

> > 

> > The HLT instruction is defined in the undefined encoding space for older

> > architectures but simulators such as QEMU already trap on it [1] for all

> > architectures and is a requirement for semihosting v2 [2]

> > 

> > Unfortunately the GAS restricts the use of HLT to Armv8-a which requires us to

> > use the instruction encodings we want directly in crt0.

> > 

> > This patch does this, I have not updated newlib/libc/* as that is quite out of

> > date already.  A proper sync is needed in order to get things back in sync.

> > 

> > A different patch for this would be best.

> > 

> > [1] https://github.com/qemu/qemu/commit/19a6e31c9d2701ef648b70ddcfc3bf64cec8c37e

> > [2] https://developer.arm.com/docs/100863/latest/the-semihosting-interface

> > 

> > Regtested on arm-none-eabi and no issues.

> > 

> > Ok for trunk?

> 

> Sorry, no.

> 

> > Thanks,

> > Tamar

> > 

> > libgloss/ChangeLog:

> > 

> > 2019-02-07  Tamar Christina  <tamar.christina@arm.com>

> > 

> > 	PR libgloss/24070

> > 	* arm/crt0.S: Convert macros to function Macros.

> > 	* arm/swi.h (AngelSWI_ARM, AngelSWI): Use raw insn encoding for

> > 	SEMIHOST_V2_MIXED_MODE case.

> > 	(AngelSWIAsm): Make function.

> > 	(AngelSWIInsn, AngelSWIAsm): Use .inst directive as instruction.

> 

> Did you have a look into libgloss/ChangeLog lately?  We're using git for

> three years now.  Please provide patches in git format-patch format and

> provide a normal git log entry explaining what the patch does and why.

> We don't do CVS ChangeLogs anymore.

> 

> 

> Thanks,

> Corinna

> 

> -- 

> Corinna Vinschen

> Cygwin Maintainer

> Red Hat




--
From 2f69cf1de05cdd124195574df5b7bef4a43481e3 Mon Sep 17 00:00:00 2001
From: Tamar Christina <tamar.christina@arm.com>

Date: Wed, 6 Feb 2019 11:27:12 +0000
Subject: [PATCH] AArch32: Add support for HLT to Mixed Mode models

The Semihosting v2 protocol requires us to output the Armv8-a HLT instruction
when in mixed mode (SEMIHOST_V2_MIXED_MODE), however it also requires this to
be done for Armv7-a and earlier architectures.

The HLT instruction is defined in the undefined encoding space for older
architectures but simulators such as QEMU already trap on it [1] for all
architectures and is a requirement for semihosting v2 [2].

Unfortunately the GAS restricts the use of HLT to Armv8-a which requires us to
use the instruction encodings we want directly in crt0.

This patch does this, I have not updated newlib/libc/* as that is quite out of
date already.  A proper sync is needed in order to get things back in sync.

A different patch for this would be best.

[1] https://github.com/qemu/qemu/commit/19a6e31c9d2701ef648b70ddcfc3bf64cec8c37e
[2] https://developer.arm.com/docs/100863/latest/the-semihosting-interface
---
 libgloss/arm/crt0.S |  6 +++---
 libgloss/arm/swi.h  | 14 ++++++++++----
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/libgloss/arm/crt0.S b/libgloss/arm/crt0.S
index 48f3d6b1db..c708f63d83 100644
--- a/libgloss/arm/crt0.S
+++ b/libgloss/arm/crt0.S
@@ -116,10 +116,10 @@
 	bkpt	AngelSWI
 #elif defined(__thumb2__)
 	/*  We are in thumb mode for startup on armv7 architectures. */
-	AngelSWIAsm	AngelSWI
+	AngelSWIAsm (AngelSWI)
 #else
 	/*  We are always in ARM mode for startup on pre armv7 archs. */
-	AngelSWIAsm	AngelSWI_ARM
+	AngelSWIAsm (AngelSWI_ARM)
 #endif
 	ldr	r0, .LC0	/*  point at values read */
 
@@ -297,7 +297,7 @@ __change_mode:
 #else
 	movs	r0, #AngelSWI_Reason_GetCmdLine
 	ldr	r1, .LC30	/*  Space for command line */
-	AngelSWIAsm	AngelSWI
+	AngelSWIAsm (AngelSWI)
 	ldr	r1, .LC30
 	ldr	r1, [r1]
 #endif
diff --git a/libgloss/arm/swi.h b/libgloss/arm/swi.h
index 67eb36b3fa..8f50ee7d9b 100644
--- a/libgloss/arm/swi.h
+++ b/libgloss/arm/swi.h
@@ -31,9 +31,9 @@
 
 /* Now the SWI numbers and reason codes for RDI (Angel) monitors.  */
 #if defined (SEMIHOST_V2) && defined (SEMIHOST_V2_MIXED_MODE)
-  #define AngelSWI_ARM			0xF000 /* HLT A32.  */
+  #define AngelSWI_ARM			0xE10F0070 /* HLT #0xF000 A32.  */
   #ifdef __thumb__
-    #define AngelSWI			0x3C /* HLT T32.  */
+    #define AngelSWI			0xBABC /* HLT #0x3c T32.  */
   #else /* __thumb__.  */
     #define AngelSWI			AngelSWI_ARM
   #endif /* __thumb__.  */
@@ -49,10 +49,16 @@
 /* For thumb only architectures use the BKPT instruction instead of SWI.  */
 #ifdef THUMB_VXM
   #define AngelSWIInsn			"bkpt"
-  #define AngelSWIAsm			bkpt
+  #define AngelSWIAsm(IMM)		bkpt IMM
+#elif defined (SEMIHOST_V2) && defined (SEMIHOST_V2_MIXED_MODE)
+  /* This is actually encoding the HLT instruction, however we don't have
+     support for this in older assemblers.  So we have to encode the
+     instruction manually.  */
+  #define AngelSWIInsn			".inst"
+  #define AngelSWIAsm(IMM)		.inst IMM
 #else
   #define AngelSWIInsn			"swi"
-  #define AngelSWIAsm			swi
+  #define AngelSWIAsm(IMM)		swi IMM
 #endif
 
 /* The reason codes:  */
-- 
2.20.1
Corinna Vinschen Feb. 8, 2019, 11:38 a.m. | #3
On Feb  8 10:21, Tamar Christina wrote:
> Hi Corinna,

> 

> So sorry, I had been busy sending patches all day and forgot about the different

> format for newlib.

> 

> I have updated my scripts to generate git format patches for newlib so it

> shouldn't happen again!

> 

> Ok for master?


Yes, pushed.


Thank you,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat
Alexander Fedotov Feb. 8, 2019, 2:02 p.m. | #4
Hi Tarmar

I tried to build latest master and got the following error:
arm-none-eabi-gcc
-B/home/af/arm-none-eabi/obj_newlib/arm-none-eabi/newlib/ -isystem
/home/af/arm-none-eabi/obj_newlib/arm-none-eabi/newlib/targ-include
-isystem /home/af/arm-none-eabi/src_newlib/newlib/libc/include
-B/home/af/arm-none-eabi/obj_newlib/arm-none-eabi/libgloss/arm
-L/home/af/arm-none-eabi/obj_newlib/arm-none-eabi/libgloss/libnosys
-L/home/af/arm-none-eabi/src_newlib/libgloss/arm    -Os -g
-ffunction-sections -fdata-sections -DSEMIHOST_V2
-DSEMIHOST_V2_MIXED_MODE -Os -g -ffunction-sections -fdata-sections
-mthumb -march=armv6s-m -I.
-I../../../../../../../src_newlib/libgloss/arm/.. `if [ -d
../../../newlib ]; then echo
-I../../../../../../../src_newlib/libgloss/arm/../../newlib/libc/machine/arm;
fi` -DARM_RDI_MONITOR -o rdimon-crt0-v2m.o -c
../../../../../../../src_newlib/libgloss/arm/crt0.S
../../../../../../../src_newlib/libgloss/arm/crt0.S: Assembler messages:
../../../../../../../src_newlib/libgloss/arm/crt0.S:116: Error:
immediate value out of range -- `bkpt 0xBABC'
../../../../../../../src_newlib/libgloss/arm/crt0.S:300: Error:
immediate value out of range -- `bkpt 0xBABC'
Makefile:170: recipe for target 'rdimon-crt0-v2m.o' failed

Alex

On Fri, Feb 8, 2019 at 2:38 PM Corinna Vinschen <vinschen@redhat.com> wrote:
>

> On Feb  8 10:21, Tamar Christina wrote:

> > Hi Corinna,

> >

> > So sorry, I had been busy sending patches all day and forgot about the different

> > format for newlib.

> >

> > I have updated my scripts to generate git format patches for newlib so it

> > shouldn't happen again!

> >

> > Ok for master?

>

> Yes, pushed.

>

>

> Thank you,

> Corinna

>

> --

> Corinna Vinschen

> Cygwin Maintainer

> Red Hat




-- 
Best regards,
AF
Tamar Christina Feb. 8, 2019, 4:22 p.m. | #5
Hi Alexander,

Ah apologies, I only tested v5t, 7 and 8.

I'll write up a patch.

Thanks,
Tamar

> -----Original Message-----

> From: Alexander Fedotov <alfedotov@gmail.com>

> Sent: Friday, February 8, 2019 14:03

> To: Tamar Christina <Tamar.Christina@arm.com>; Newlib

> <newlib@sourceware.org>

> Cc: nd <nd@arm.com>; Richard Earnshaw <Richard.Earnshaw@arm.com>

> Subject: Re: [PATCH][Newlib][AArch32] Add support for HLT to Mixed Mode

> models

> 

> Hi Tarmar

> 

> I tried to build latest master and got the following error:

> arm-none-eabi-gcc

> -B/home/af/arm-none-eabi/obj_newlib/arm-none-eabi/newlib/ -isystem

> /home/af/arm-none-eabi/obj_newlib/arm-none-eabi/newlib/targ-include

> -isystem /home/af/arm-none-eabi/src_newlib/newlib/libc/include

> -B/home/af/arm-none-eabi/obj_newlib/arm-none-eabi/libgloss/arm

> -L/home/af/arm-none-eabi/obj_newlib/arm-none-eabi/libgloss/libnosys

> -L/home/af/arm-none-eabi/src_newlib/libgloss/arm    -Os -g

> -ffunction-sections -fdata-sections -DSEMIHOST_V2 -

> DSEMIHOST_V2_MIXED_MODE -Os -g -ffunction-sections -fdata-sections -

> mthumb -march=armv6s-m -I.

> -I../../../../../../../src_newlib/libgloss/arm/.. `if [ -d ../../../newlib ]; then echo

> -I../../../../../../../src_newlib/libgloss/arm/../../newlib/libc/machine/arm;

> fi` -DARM_RDI_MONITOR -o rdimon-crt0-v2m.o -

> c ../../../../../../../src_newlib/libgloss/arm/crt0.S

> ../../../../../../../src_newlib/libgloss/arm/crt0.S: Assembler messages:

> ../../../../../../../src_newlib/libgloss/arm/crt0.S:116: Error:

> immediate value out of range -- `bkpt 0xBABC'

> ../../../../../../../src_newlib/libgloss/arm/crt0.S:300: Error:

> immediate value out of range -- `bkpt 0xBABC'

> Makefile:170: recipe for target 'rdimon-crt0-v2m.o' failed

> 

> Alex

> 

> On Fri, Feb 8, 2019 at 2:38 PM Corinna Vinschen <vinschen@redhat.com>

> wrote:

> >

> > On Feb  8 10:21, Tamar Christina wrote:

> > > Hi Corinna,

> > >

> > > So sorry, I had been busy sending patches all day and forgot about

> > > the different format for newlib.

> > >

> > > I have updated my scripts to generate git format patches for newlib

> > > so it shouldn't happen again!

> > >

> > > Ok for master?

> >

> > Yes, pushed.

> >

> >

> > Thank you,

> > Corinna

> >

> > --

> > Corinna Vinschen

> > Cygwin Maintainer

> > Red Hat

> 

> 

> 

> --

> Best regards,

> AF

Patch

diff --git a/libgloss/arm/crt0.S b/libgloss/arm/crt0.S
index 48f3d6b1db9d82d542966752c367239b4e796b9d..c708f63d8361a8862880a40687f3cdd979762a15 100644
--- a/libgloss/arm/crt0.S
+++ b/libgloss/arm/crt0.S
@@ -116,10 +116,10 @@ 
 	bkpt	AngelSWI
 #elif defined(__thumb2__)
 	/*  We are in thumb mode for startup on armv7 architectures. */
-	AngelSWIAsm	AngelSWI
+	AngelSWIAsm (AngelSWI)
 #else
 	/*  We are always in ARM mode for startup on pre armv7 archs. */
-	AngelSWIAsm	AngelSWI_ARM
+	AngelSWIAsm (AngelSWI_ARM)
 #endif
 	ldr	r0, .LC0	/*  point at values read */
 
@@ -297,7 +297,7 @@  __change_mode:
 #else
 	movs	r0, #AngelSWI_Reason_GetCmdLine
 	ldr	r1, .LC30	/*  Space for command line */
-	AngelSWIAsm	AngelSWI
+	AngelSWIAsm (AngelSWI)
 	ldr	r1, .LC30
 	ldr	r1, [r1]
 #endif
diff --git a/libgloss/arm/swi.h b/libgloss/arm/swi.h
index 67eb36b3fa1ad5fb3367e25d3b626da89d496e52..8f50ee7d9b58984537dc60675f6ace954f682341 100644
--- a/libgloss/arm/swi.h
+++ b/libgloss/arm/swi.h
@@ -31,9 +31,9 @@ 
 
 /* Now the SWI numbers and reason codes for RDI (Angel) monitors.  */
 #if defined (SEMIHOST_V2) && defined (SEMIHOST_V2_MIXED_MODE)
-  #define AngelSWI_ARM			0xF000 /* HLT A32.  */
+  #define AngelSWI_ARM			0xE10F0070 /* HLT #0xF000 A32.  */
   #ifdef __thumb__
-    #define AngelSWI			0x3C /* HLT T32.  */
+    #define AngelSWI			0xBABC /* HLT #0x3c T32.  */
   #else /* __thumb__.  */
     #define AngelSWI			AngelSWI_ARM
   #endif /* __thumb__.  */
@@ -49,10 +49,16 @@ 
 /* For thumb only architectures use the BKPT instruction instead of SWI.  */
 #ifdef THUMB_VXM
   #define AngelSWIInsn			"bkpt"
-  #define AngelSWIAsm			bkpt
+  #define AngelSWIAsm(IMM)		bkpt IMM
+#elif defined (SEMIHOST_V2) && defined (SEMIHOST_V2_MIXED_MODE)
+  /* This is actually encoding the HLT instruction, however we don't have
+     support for this in older assemblers.  So we have to encode the
+     instruction manually.  */
+  #define AngelSWIInsn			".inst"
+  #define AngelSWIAsm(IMM)		.inst IMM
 #else
   #define AngelSWIInsn			"swi"
-  #define AngelSWIAsm			swi
+  #define AngelSWIAsm(IMM)		swi IMM
 #endif
 
 /* The reason codes:  */