Re: [PATCH v4] cpukit/aarch64: Add ESR register decoding

2021-03-23 Thread Gedare Bloom
On Tue, Mar 23, 2021 at 9:57 AM Joel Sherrill  wrote:
>
>
>
> On Tue, Mar 23, 2021 at 10:49 AM Sebastian Huber 
>  wrote:
>>
>> Hello Alex,
>>
>> this file has little in common with the RTEMS coding style. This is
>> quite understandable since the style is very exotic. I don't think we
>> will have a formatting tool which is able to produce the RTEMS style in
>> the near future. This gives raise to some general questions.
>>
>> 1. Do we care about the coding style?
>
>
> Yes.
>>
>>
>> 2. Who wants to enforce the coding style?
>
>
> I hope we all catch these when we spot them.
>
> Alex will reformat it.
>
>>
>>
>> 3. Should new files use a new style understood by a well known tool and
>> a well known coding style?
>
>
> That would be preferable but like the license change and SPDX markers, this
> is down the list.
>
> You have something that is close. If you want to add it to rtems-tools as a 
> starting
> point that is accessible, we can start to use it and see how it goes.
>
>>
>>
>> I see a lot of
>>
>> type var = initial_value;
>>
>> Maybe we should drop the stone age variable declarations at block begin
>> if we choose a new style and declare and initialize local variables upon
>> first use.
>
>
> I don't know when this was started but it was never part of the original
> RTEMS style.
>
> Personally I have never liked it.
>

It is a holdover from K and the gcc -ansi flag would complain if you
didn't do it. (For older compiler writers, it made it a lot easier to
parse and generate the code if you could pass over all the stack
allocations in one group.)

> --joel
>
>>
>>
>> --
>> embedded brains GmbH
>> Herr Sebastian HUBER
>> Dornierstr. 4
>> 82178 Puchheim
>> Germany
>> email: sebastian.hu...@embedded-brains.de
>> phone: +49-89-18 94 741 - 16
>> fax:   +49-89-18 94 741 - 08
>>
>> Registergericht: Amtsgericht München
>> Registernummer: HRB 157899
>> Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
>> Unsere Datenschutzerklärung finden Sie hier:
>> https://embedded-brains.de/datenschutzerklaerung/
>>
>> ___
>> devel mailing list
>> devel@rtems.org
>> http://lists.rtems.org/mailman/listinfo/devel
>
> ___
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH v4] cpukit/aarch64: Add ESR register decoding

2021-03-23 Thread Joel Sherrill
On Tue, Mar 23, 2021 at 10:49 AM Sebastian Huber <
sebastian.hu...@embedded-brains.de> wrote:

> Hello Alex,
>
> this file has little in common with the RTEMS coding style. This is
> quite understandable since the style is very exotic. I don't think we
> will have a formatting tool which is able to produce the RTEMS style in
> the near future. This gives raise to some general questions.
>
> 1. Do we care about the coding style?
>

Yes.

>
> 2. Who wants to enforce the coding style?
>

I hope we all catch these when we spot them.

Alex will reformat it.


>
> 3. Should new files use a new style understood by a well known tool and
> a well known coding style?
>

That would be preferable but like the license change and SPDX markers, this
is down the list.

You have something that is close. If you want to add it to rtems-tools as a
starting
point that is accessible, we can start to use it and see how it goes.


>
> I see a lot of
>
> type var = initial_value;
>
> Maybe we should drop the stone age variable declarations at block begin
> if we choose a new style and declare and initialize local variables upon
> first use.
>

I don't know when this was started but it was never part of the original
RTEMS style.

Personally I have never liked it.

--joel


>
> --
> embedded brains GmbH
> Herr Sebastian HUBER
> Dornierstr. 4
> 82178 Puchheim
> Germany
> email: sebastian.hu...@embedded-brains.de
> phone: +49-89-18 94 741 - 16
> fax:   +49-89-18 94 741 - 08
>
> Registergericht: Amtsgericht München
> Registernummer: HRB 157899
> Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
> Unsere Datenschutzerklärung finden Sie hier:
> https://embedded-brains.de/datenschutzerklaerung/
>
> ___
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH v4] cpukit/aarch64: Add ESR register decoding

2021-03-23 Thread Sebastian Huber

Hello Alex,

this file has little in common with the RTEMS coding style. This is 
quite understandable since the style is very exotic. I don't think we 
will have a formatting tool which is able to produce the RTEMS style in 
the near future. This gives raise to some general questions.


1. Do we care about the coding style?

2. Who wants to enforce the coding style?

3. Should new files use a new style understood by a well known tool and 
a well known coding style?


I see a lot of

type var = initial_value;

Maybe we should drop the stone age variable declarations at block begin 
if we choose a new style and declare and initialize local variables upon 
first use.


--
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.hu...@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH v4] cpukit/aarch64: Add ESR register decoding

2021-03-23 Thread Gedare Bloom
On Mon, Mar 22, 2021 at 3:32 PM Alex White  wrote:
>
> ---
>  .../aarch64/aarch64-exception-frame-print.c   | 105 +-
>  1 file changed, 101 insertions(+), 4 deletions(-)
>
> diff --git a/cpukit/score/cpu/aarch64/aarch64-exception-frame-print.c 
> b/cpukit/score/cpu/aarch64/aarch64-exception-frame-print.c
> index 59b5d06032..348151a468 100644
> --- a/cpukit/score/cpu/aarch64/aarch64-exception-frame-print.c
> +++ b/cpukit/score/cpu/aarch64/aarch64-exception-frame-print.c
> @@ -45,8 +45,90 @@
>  #include 
>
>  #include 
> +#include 
>  #include 
>
> +static const char* _exception_class_to_string( uint16_t exception_class );

static symbol shouldn't need an _. An _ generally is reserved. I don't
know what our rules say about this, but anyway it is not needed here.

> +
> +static void _binary_sprintf(
ditto.

> +  char *s,
> +  size_t maxlen,
> +  uint32_t num_bits,
> +  uint32_t value
> +);
> +
> +static const char* _exception_class_to_string( uint16_t exception_class )
> +{
> +  switch (exception_class)
> +  {
> +  case 0x01: return "Trapped WFI or WFE instruction";
indent two spaces, put the return on a newline, to adhere to score style.

> +  case 0x03: return "Trapped MCR or MRC access with (coproc==0b)";
> +  case 0x04: return "Trapped MCRR or MRRC access with (coproc==0b)";
> +  case 0x05: return "Trapped MCR or MRC access with (coproc==0b1110)";
> +  case 0x06: return "Trapped LDC or STC access";
> +  case 0x0c: return "Trapped MRRC access with (coproc==0b1110)";
> +  case 0x0e: return "Illegal Execution state";
> +  case 0x18: return "Trapped MSR, MRS, or System instruction";
> +  case 0x20: return "Instruction Abort from a lower Exception level";
> +  case 0x21: return "Instruction Abort taken without a change in Exception "
> +"level";
> +  case 0x22: return "PC alignment fault";
> +  case 0x24: return "Data Abort from a lower Exception level";
> +  case 0x25: return "Data Abort taken without a change in Exception level";
> +  case 0x26: return "SP alignment fault";
> +  case 0x30: return "Breakpoint exception from a lower Exception level";
> +  case 0x31: return "Breakpoint exception taken without a change in "
> +"Exception level";
> +  case 0x32: return "Software Step exception from a lower Exception level";
> +  case 0x33: return "Software Step exception taken without a change in "
> +"Exception level";
> +  case 0x34: return "Watchpoint exception from a lower Exception level";
> +  case 0x35: return "Watchpoint exception taken without a change in "
> +"Exception level";
> +  case 0x38: return "BKPT instruction execution in AArch32 state";
> +  case 0x3c: return "BRK instruction execution in AArch64 state";
> +  default: return "Unknown";
> +  }
> +}
> +
> +typedef struct {
> +   char *s;
> +   size_t n;
> +} string_context;
> +
> +static void put_char(int c, void *arg)
> +{
> +  string_context *sctx = arg;
> +  size_t n = sctx->n;
> +
> +  if (n > 0) {
> +char *s = sctx->s;
> +*s = (char)c;
> +sctx->s = s + 1;
> +sctx->n = n - 1;
> +  }
> +}
> +
> +static void _binary_sprintf(
> +  char *s,
> +  size_t maxlen,
> +  uint32_t num_bits,
> +  uint32_t value
> +)
> +{
> +  string_context sctx = {
> +.s = s,
> +.n = maxlen
> +  };
> +  uint32_t mask = 1<<(num_bits-1);
> +  int cx = 0;
> +
> +  while ( mask != 0 ) {
> +cx += _IO_Printf(put_char, , "%d", (value & mask ? 1 : 0));
> +mask >>= 1;
> +  }
> +}
> +
>  void _CPU_Exception_frame_print( const CPU_Exception_frame *frame )
>  {
>printk(
> @@ -68,8 +150,7 @@ void _CPU_Exception_frame_print( const CPU_Exception_frame 
> *frame )
>  "X14  = 0x%016" PRIx64  " SP   = 0x%016" PRIxPTR "\n"
>  "X15  = 0x%016" PRIx64  " PC   = 0x%016" PRIxPTR "\n"
>  "X16  = 0x%016" PRIx64  " DAIF = 0x%016" PRIx64 "\n"
> -"VEC  = 0x%016" PRIxPTR " CPSR = 0x%016" PRIx64 "\n"
> -"ESR  = 0x%016" PRIx64  " FAR  = 0x%016" PRIx64 "\n",
> +"VEC  = 0x%016" PRIxPTR " CPSR = 0x%016" PRIx64 "\n",
>  frame->register_x0, frame->register_x17,
>  frame->register_x1, frame->register_x18,
>  frame->register_x2, frame->register_x19,
> @@ -87,10 +168,26 @@ void _CPU_Exception_frame_print( const 
> CPU_Exception_frame *frame )
>  frame->register_x14, (intptr_t)frame->register_sp,
>  frame->register_x15, (intptr_t)frame->register_pc,
>  frame->register_x16, frame->register_daif,
> -(intptr_t) frame->vector, frame->register_cpsr,
> -frame->register_syndrome, frame->register_fault_address
> +(intptr_t) frame->vector, frame->register_cpsr
>);
>
> +  uint32_t ec = (frame->register_syndrome >> 26) & 0x3f;
> +  uint32_t il = (frame->register_syndrome >> 25) & 0x1;
> +  uint32_t iss = frame->register_syndrome & 0x1ff;
> +  char ec_str[7] = {0};
> +  char iss_str[26] = {0};
> +
> +  _binary_sprintf(ec_str, 7, 6, ec);
> +  _binary_sprintf(iss_str, 26, 25, iss);
> +
> +  printk(
> +"ESR 

[PATCH v4] cpukit/aarch64: Add ESR register decoding

2021-03-22 Thread Alex White
---
 .../aarch64/aarch64-exception-frame-print.c   | 105 +-
 1 file changed, 101 insertions(+), 4 deletions(-)

diff --git a/cpukit/score/cpu/aarch64/aarch64-exception-frame-print.c 
b/cpukit/score/cpu/aarch64/aarch64-exception-frame-print.c
index 59b5d06032..348151a468 100644
--- a/cpukit/score/cpu/aarch64/aarch64-exception-frame-print.c
+++ b/cpukit/score/cpu/aarch64/aarch64-exception-frame-print.c
@@ -45,8 +45,90 @@
 #include 
 
 #include 
+#include 
 #include 
 
+static const char* _exception_class_to_string( uint16_t exception_class );
+
+static void _binary_sprintf(
+  char *s,
+  size_t maxlen,
+  uint32_t num_bits,
+  uint32_t value
+);
+
+static const char* _exception_class_to_string( uint16_t exception_class )
+{
+  switch (exception_class)
+  {
+  case 0x01: return "Trapped WFI or WFE instruction";
+  case 0x03: return "Trapped MCR or MRC access with (coproc==0b)";
+  case 0x04: return "Trapped MCRR or MRRC access with (coproc==0b)";
+  case 0x05: return "Trapped MCR or MRC access with (coproc==0b1110)";
+  case 0x06: return "Trapped LDC or STC access";
+  case 0x0c: return "Trapped MRRC access with (coproc==0b1110)";
+  case 0x0e: return "Illegal Execution state";
+  case 0x18: return "Trapped MSR, MRS, or System instruction";
+  case 0x20: return "Instruction Abort from a lower Exception level";
+  case 0x21: return "Instruction Abort taken without a change in Exception "
+"level";
+  case 0x22: return "PC alignment fault";
+  case 0x24: return "Data Abort from a lower Exception level";
+  case 0x25: return "Data Abort taken without a change in Exception level";
+  case 0x26: return "SP alignment fault";
+  case 0x30: return "Breakpoint exception from a lower Exception level";
+  case 0x31: return "Breakpoint exception taken without a change in "
+"Exception level";
+  case 0x32: return "Software Step exception from a lower Exception level";
+  case 0x33: return "Software Step exception taken without a change in "
+"Exception level";
+  case 0x34: return "Watchpoint exception from a lower Exception level";
+  case 0x35: return "Watchpoint exception taken without a change in "
+"Exception level";
+  case 0x38: return "BKPT instruction execution in AArch32 state";
+  case 0x3c: return "BRK instruction execution in AArch64 state";
+  default: return "Unknown";
+  }
+}
+
+typedef struct {
+   char *s;
+   size_t n;
+} string_context;
+
+static void put_char(int c, void *arg)
+{
+  string_context *sctx = arg;
+  size_t n = sctx->n;
+
+  if (n > 0) {
+char *s = sctx->s;
+*s = (char)c;
+sctx->s = s + 1;
+sctx->n = n - 1;
+  }
+}
+
+static void _binary_sprintf(
+  char *s,
+  size_t maxlen,
+  uint32_t num_bits,
+  uint32_t value
+)
+{
+  string_context sctx = {
+.s = s,
+.n = maxlen
+  };
+  uint32_t mask = 1<<(num_bits-1);
+  int cx = 0;
+
+  while ( mask != 0 ) {
+cx += _IO_Printf(put_char, , "%d", (value & mask ? 1 : 0));
+mask >>= 1;
+  }
+}
+
 void _CPU_Exception_frame_print( const CPU_Exception_frame *frame )
 {
   printk(
@@ -68,8 +150,7 @@ void _CPU_Exception_frame_print( const CPU_Exception_frame 
*frame )
 "X14  = 0x%016" PRIx64  " SP   = 0x%016" PRIxPTR "\n"
 "X15  = 0x%016" PRIx64  " PC   = 0x%016" PRIxPTR "\n"
 "X16  = 0x%016" PRIx64  " DAIF = 0x%016" PRIx64 "\n"
-"VEC  = 0x%016" PRIxPTR " CPSR = 0x%016" PRIx64 "\n"
-"ESR  = 0x%016" PRIx64  " FAR  = 0x%016" PRIx64 "\n",
+"VEC  = 0x%016" PRIxPTR " CPSR = 0x%016" PRIx64 "\n",
 frame->register_x0, frame->register_x17,
 frame->register_x1, frame->register_x18,
 frame->register_x2, frame->register_x19,
@@ -87,10 +168,26 @@ void _CPU_Exception_frame_print( const CPU_Exception_frame 
*frame )
 frame->register_x14, (intptr_t)frame->register_sp,
 frame->register_x15, (intptr_t)frame->register_pc,
 frame->register_x16, frame->register_daif,
-(intptr_t) frame->vector, frame->register_cpsr,
-frame->register_syndrome, frame->register_fault_address
+(intptr_t) frame->vector, frame->register_cpsr
   );
 
+  uint32_t ec = (frame->register_syndrome >> 26) & 0x3f;
+  uint32_t il = (frame->register_syndrome >> 25) & 0x1;
+  uint32_t iss = frame->register_syndrome & 0x1ff;
+  char ec_str[7] = {0};
+  char iss_str[26] = {0};
+
+  _binary_sprintf(ec_str, 7, 6, ec);
+  _binary_sprintf(iss_str, 26, 25, iss);
+
+  printk(
+"ESR  = EC: 0b%s" " IL: 0b%d" " ISS: 0b%s" "\n"
+"   %s\n",
+ec_str, il, iss_str, _exception_class_to_string(ec)
+  );
+
+  printk( "FAR  = 0x%016" PRIx64 "\n", frame->register_fault_address );
+
   const uint128_t *qx = >register_q0;
   int i;
 
-- 
2.27.0

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel