[Pending mbed-os 5.6] Print peripheral on ACL violation from debug blinky example #54

theamirocohen · 2017-09-10T08:58:20Z

Print the name of a peripheral when faulting on an address belonging
to a perhipheral.
The memory maps of K64F and STM32F4 were added.

theamirocohen · 2017-09-10T12:45:18Z

@danny4478
@romkuz01
please review

danny4478 · 2017-09-10T14:44:18Z

source/mem_map/mem_map.h

+#define __MEM_MAP_H__
+
+typedef struct mem_map {
+    const char *name;


Space after *

danny4478 · 2017-09-10T14:44:53Z

source/mem_map/mem_map.h

+    uint32_t end;
+} MemMap;
+
+const MemMap* memory_map_name(uint32_t addr);


Space before *

danny4478 · 2017-09-10T14:50:27Z

source/mem_map/mem_map.cpp

+
+    /* find system memory region */
+    map = g_mem_map;
+    for(i = 0; i < UVISOR_ARRAY_COUNT(g_mem_map); i++)


I would always use " { ... }", even wen not needed by compiler.
It is safer.

danny4478 · 2017-09-10T14:51:32Z

source/main.cpp

+
+static void example_halt_error(THaltError reason, const THaltInfo *halt_info) {
+
+	const MemMap *map;


Space after *

danny4478 · 2017-09-10T14:59:28Z

source/main.cpp

+static void example_halt_error(THaltError reason, const THaltInfo *halt_info) {
+
+	const MemMap *map;
+	map = memory_map_name(halt_info->bfar);


Are we sure here that bfar is valid?

alzix · 2017-09-10T17:24:13Z

source/mem_map/mem_map.cpp

+    map = g_mem_map;
+    for(i = 0; i < UVISOR_ARRAY_COUNT(g_mem_map); i++)
+        if((addr >= map->base) && (addr <= map->end))
+            return map;


same goes for if statements - braces ARE mandatory for all block statements - also for single line cases

Patater · 2017-09-12T09:21:19Z

"Print peripheral on ACL violation from debug fault example"

Shouldn't this PR be against the debug fault example?

theamirocohen · 2017-09-12T14:30:44Z

Hi,

@alzix instructed me to use the basic uVisor example from uvisor repo

Patater · 2017-09-12T15:33:46Z

Please update the commit message to reflect this, then.

Thanks

alzix · 2017-09-13T05:14:50Z

Hi @Patater ,
our debug-fault example faults on arbitrary address - thus this addition would not be helpful at all.
On the other hand mbed-os-example-uvisor is mentioned in our porting guide - thus porting engineers most likely will start with it and at first stages run in to some faults.
We are open for suggestions.

Patater · 2017-09-13T08:11:28Z

@alzix Makes sense to me. Not sure how valuable the separate debug fault example is with this change, though.

mikisch81 · 2017-09-13T08:17:18Z

@Patater I think the debug fault example is still useful as it's an example which explicitly shows a fault scenario.
The mbed-os-example-uvisor normally should not fault, BUT if it does faults (usually because of missing ACL entry) we need to show what is the expected output.

Patater · 2017-09-13T09:26:42Z

@mikisch81 @alzix
Our old uVisor example, where pressing the button made the system try to access something it shouldn't and fault, would be a good place for a debug box example. But, the memory map isn't really needed there, as the example touches another box's SRAM and not a peripheral.

It'd probably be best to modify our debug fault example to poke some unowned peripheral instead of 0xFFFFFFFF, that way we could use a memory map like this. The IRQ example could stay simple as well without the memory map, although I agree with the porting benefits of having the memory map here too.

theamirocohen · 2017-09-17T16:04:00Z

pending for mbed-os : mbed-os-5.6.0-rc1
depends on THaltInfo struct

alzix · 2017-10-02T10:35:08Z

retest uvisor

alzix · 2017-10-22T18:57:43Z

/morph uvisor-test

alzix · 2017-10-22T19:17:51Z

@theamirocohen , please update mbed-os hash to point after ARMmbed/mbed-os#5275 merge

theamirocohen · 2017-10-23T06:28:12Z

@alzix, I've updated mbed-os hash to point after ARMmbed/mbed-os#5275 merge

alzix · 2017-10-23T07:14:31Z

/morph uvisor-test

alzix · 2017-10-23T07:16:56Z

/morph uvisor-test

Print the name of a peripheral when faulting on an address belonging to a perhipheral. The memory maps of K64F and STM32F4 were added.

danny4478 reviewed Sep 10, 2017

View reviewed changes

source/mem_map/mem_map.h Outdated

#define __MEM_MAP_H__

typedef struct mem_map {

const char *name;

Copy link

danny4478 Sep 10, 2017 •

edited

Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Space after *

danny4478 reviewed Sep 10, 2017

View reviewed changes

source/main.cpp Outdated

static void example_halt_error(THaltError reason, const THaltInfo *halt_info) {

const MemMap *map;

Copy link

danny4478 Sep 10, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Space after *

danny4478 reviewed Sep 10, 2017

View reviewed changes

alzix suggested changes Sep 10, 2017

View reviewed changes

theamirocohen force-pushed the print_peri branch from e2e98e7 to 9788239 Compare September 13, 2017 04:32

theamirocohen changed the title ~~Print peripheral on ACL violation from debug fault example~~ Print peripheral on ACL violation from debug blinky example Sep 13, 2017

theamirocohen changed the title ~~Print peripheral on ACL violation from debug blinky example~~ [Pending mbed-os 5.6] Print peripheral on ACL violation from debug blinky example Sep 17, 2017

alzix added the run-CI label Oct 16, 2017

alzix previously approved these changes Oct 22, 2017

View reviewed changes

theamirocohen dismissed alzix’s stale review via 6fd2838 October 23, 2017 06:25

theamirocohen force-pushed the print_peri branch from f6c5947 to 6fd2838 Compare October 23, 2017 06:25

theamirocohen force-pushed the print_peri branch 2 times, most recently from e223d20 to 8993dcf Compare October 23, 2017 07:08

theamirocohen force-pushed the print_peri branch from f00c526 to 578522b Compare October 23, 2017 10:36

Print peripheral on ACL violation from debug blinky example

a9927f7

Print the name of a peripheral when faulting on an address belonging to a perhipheral. The memory maps of K64F and STM32F4 were added.

theamirocohen force-pushed the print_peri branch from 578522b to a9927f7 Compare October 23, 2017 10:37

Merge branch 'master' into print_peri

1775155

alzix merged commit 57bdf06 into ARMmbed:master Oct 23, 2017


		static void example_halt_error(THaltError reason, const THaltInfo *halt_info) {

		const MemMap *map;

[Pending mbed-os 5.6] Print peripheral on ACL violation from debug blinky example #54

[Pending mbed-os 5.6] Print peripheral on ACL violation from debug blinky example #54

Uh oh!

Conversation

theamirocohen commented Sep 10, 2017

Uh oh!

theamirocohen commented Sep 10, 2017

Uh oh!

danny4478 Sep 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danny4478 Sep 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danny4478 Sep 10, 2017

Choose a reason for hiding this comment

Uh oh!

danny4478 Sep 10, 2017

Choose a reason for hiding this comment

Uh oh!

danny4478 Sep 10, 2017

Choose a reason for hiding this comment

Uh oh!

alzix Sep 10, 2017

Choose a reason for hiding this comment

Uh oh!

Patater commented Sep 12, 2017

Uh oh!

theamirocohen commented Sep 12, 2017

Uh oh!

Patater commented Sep 12, 2017

Uh oh!

alzix commented Sep 13, 2017

Uh oh!

Patater commented Sep 13, 2017

Uh oh!

mikisch81 commented Sep 13, 2017

Uh oh!

Patater commented Sep 13, 2017

Uh oh!

theamirocohen commented Sep 17, 2017

Uh oh!

alzix commented Oct 2, 2017

Uh oh!

alzix commented Oct 22, 2017

Uh oh!

alzix commented Oct 22, 2017

Uh oh!

theamirocohen commented Oct 23, 2017

Uh oh!

alzix commented Oct 23, 2017

Uh oh!

alzix commented Oct 23, 2017

Uh oh!

Uh oh!

danny4478 Sep 10, 2017 •

edited

Loading

danny4478 Sep 10, 2017 •

edited

Loading