Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cpu/cortexm_common: split interrupt vectors into parts #3540

Closed
wants to merge 14 commits into from
Closed

cpu/cortexm_common: split interrupt vectors into parts #3540

wants to merge 14 commits into from

Conversation

sgso
Copy link
Contributor

@sgso sgso commented Aug 3, 2015

Proposal that splits the vectors on cortexm_common into a 'common' and a 'vendor specific' part which allows cpu implementations to just specify the vendor entries.

  • First commit contains the linker script changes with two sections (.vectors.common / .vectors.vendor) now instead of one and automatic initial stack pointer filling.
  • Second commit changes the names of the stubs from *_fault_default to *_fault and makes SVC, pendSV, NMI (renamed to isr_nmi) and SysTick calls to the shared dummy handler.
  • Benefits:
    • Things are where they belong.
    • 20 lines of code less per cpu.
    • consistent names of ISRs enforced by code instead of conventions.
    • makes the ifdef'ing in platforms (sam3) less painful to watch.
  • Third commit contains example for samd21.
  • Might affect/ease cpu/cortexm_common: additional information on hardfault #3333 (why not just make it a module?)
  • 4 Bytes overhead. I cut some strings for that.

sgso added 3 commits August 3, 2015 06:44
- lets the linker fill the initial stack pointer

- splits .vectors into .vectors.common & .vectors.vendor

- adds assertions to make sure the layout is correct
- defines the first segment of the NVIC vector tables through
  vector_common[].

- provides weak defined stubs for *_fault-type exceptions.

- provides weak aliases to a dummy handler for isr_*-type exceptions.

- changes names from *_fault_default() to *_fault() (nmi_default became
  isr_nmi)
- only the cpu/vendor specific entries must be specified.

- using ISR_VECTORS with an array of exceptions > SysTick will do the
  right thing.

- inserting the initial stack pointer is not necessary anymore.
@sgso sgso changed the title cpu/cortexm_common: split vectors into common and vendor specific parts cpu/cortexm_common: split interrupt vectors into parts Aug 3, 2015
@PeterKietzmann PeterKietzmann added Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: new feature The issue requests / The PR implemements a new feature for RIOT labels Aug 3, 2015
@@ -88,6 +96,15 @@ SECTIONS
_efixed = .; /* End of text section */
} > rom

/* Cortex-M expects the initial stack pointer at 0x0. */
ASSERT(_stack_base == ORIGIN(rom),
"ld script assert failed, check cortexm_base.ld for explanation.")
Copy link
Member

Choose a reason for hiding this comment

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

could you make these assert messages a bit more specific?

@jnohlgard
Copy link
Member

👍 for this change in general. It is always good to consolidate things to avoid repeating ourselves.

@haukepetersen
Copy link
Contributor

looks good to me

@sgso
Copy link
Contributor Author

sgso commented Aug 4, 2015

Adressed comments.

Can I start adapting the other cpus then?

@jnohlgard
Copy link
Member

@sgso yes, go ahead.

@sgso
Copy link
Contributor Author

sgso commented Aug 5, 2015

I took out the assertion completely because it screws up the output of objdump. Anybody knows how to fix it, so that objdump keeps showing the data as .word?

For most cpus it was straight-forward. For kinetis I only changed the handler names because it uses its own linker scripts.

Somebody with a cc2538 or experience with it should test the new linker script. I don't think the separate linker script is necessary.

@haukepetersen
Copy link
Contributor

155 additions and 538 deletions.

looking better and better :-)

@jnohlgard
Copy link
Member

@sgso I don't understand why the asserts would make objdump misbehave. What do you mean by screws up the output?

@sgso
Copy link
Contributor Author

sgso commented Aug 6, 2015

@gebart I don't know why this is happening:

objdump -d without assertion:

08000000 <_vectors_common>:
 8000000:       20000b68        .word   0x20000b68
 8000004:       08000465        .word   0x08000465
 8000008:       08000451        .word   0x08000451
 800000c:       08000441        .word   0x08000441
        ...
 800002c:       08000415        .word   0x08000415
        ...
 8000038:       080003f5        .word   0x080003f5
 800003c:       08000451        .word   0x08000451

08000040 <_vectors_vendor>:
 8000040:       0800015d        .word   0x0800015d
 8000044:       0800015d        .word   0x0800015d

Same output with ASSERT(ABSOLUTE(_vectors_vendor) == ORIGIN(rom) + 0x40, "...") added:

08000000 <_vectors_common>:                                                                    
 8000000:       20000b68        .word   0x20000b68                                             
 8000004:       08000465        .word   0x08000465                                             
 8000008:       08000451        .word   0x08000451                                             
 800000c:       08000441        .word   0x08000441                                             
        ...                                                                                    
 800002c:       08000415        .word   0x08000415                                             
        ...                                                                                    
 8000038:       080003f5        .word   0x080003f5                                             
 800003c:       08000451        .word   0x08000451                                            

08000040 <interrupt_vector>:                                                             
 8000040:       0800015d 0800015d 0800015d 0800015d     ]...]...]...]...                       
 8000050:       0800015d 0800015d 0800015d 0800015d     ]...]...]...]...                       
 8000060:       0800015d 0800015d 0800015d 0800015d     ]...]...]...]...                       
 8000070:       0800015d 0800015d 0800015d 08001015     ]...]...].......                       
 8000080:       0800015d 0800015d 0800015d 0800015d     ]...]...]...]...                       
 8000090:       0800015d 0800015d 0800015d 0800015d     ]...]...]...]...                       
 80000a0:       0800015d 0800015d 0800015d 08000ebd     ]...]...].......                       
 80000b0:       08000ecd 0800015d 0800015d 00000000     ....]...].......  

@jnohlgard
Copy link
Member

I'm clueless as to the reason of the objdump difference.

However, after looking over the linker script again I think it might be better to put the vectors in their own section, like the kinetis scripts do it. It makes it easier to examine the binary with the usual tools, for example objdump -s -j .vector_table

@sgso
Copy link
Contributor Author

sgso commented Aug 6, 2015

I'm almost sure they will be scrambled like that as well when put in their own sections but there might be a way to mark them as .word-data.

I initially opted against this because it does not show up without extra arguments in objdump and might confuse people ("Who stole my vectors?"). I'd like a second opinion because I don't care that much, @haukepetersen?

@sgso
Copy link
Contributor Author

sgso commented Aug 6, 2015

Just tried, creating two sections .vector_table_common / .vector_table_vendor allows assertions on the size of the common part and does not scramble output.

@haukepetersen
Copy link
Contributor

I am actually a fan of not putting the vectors in its own section, so it shows up in the default make objdump. But this is not a strong opinion - so whatever the majority votes for...

What I would love to see is a unification of linkerscripts between kinetis, cc2538 and the rest of the boards - as said, in either way... But this is not subject of this PR...

Regarding the scrambled object dump output: I have no idea...

@OlegHahm OlegHahm modified the milestone: Release NEXT MAJOR Sep 2, 2015
@OlegHahm OlegHahm modified the milestones: Release 2015.12, Release 2016.03 Dec 8, 2015
@sgso sgso closed this Jan 28, 2016
@OlegHahm
Copy link
Member

Please state a reason for closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants