search

rust-embedded / msp430-rt

Minimal startup / runtime for MSP430 microcontrollers
Apache License 2.0
15 stars 4 forks source link

bring up to par with cortex-m-rt v0.6.x #4

Closed japaric closed 4 years ago

japaric commented 5 years ago

I have a few questions / comments related to some of the changes I made

  1. I have assumed that the vector table is always 32 bytes in size. That seems to the maximum vector table for MSP430 CPUs AFAICT (MSP430X CPUs support more interrupts). Is this table size correct? Or should it be bigger to accommodate some device?

  2. Like I said the vector table is fixed size. This means that if your device has less than 15 interrupts (plus Reset) the table will still be 32 bytes in size so you won't be able to use the free space for your program. In other words, your .text + .rodata end address can not exceed 0xFFE0 even if the vector table starts at e.g. 0xFFF0.

  3. Just a comment: because the vector table is now fixed size the VECTORS memory region is no longer used and doesn't need to be included in memory.x. Instead the ROM memory region should always have end address equal to 0x1_0000 because the .vector_table section is placed at the end of ROM. (I should document this requirement better)

  4. Is there any interrupt handler that should be forbidden from returning? In Cortex-M land the HardFault handler fires on invalid memory access, execution of illegal / undefined instructions, etc. Returning from this exception would resume a program that's in an invalid state and would result in UB. For this reason we have the exception attribute enforce that the HardFault handler is a divergent function (fn() -> !). Is there anything like that in MSP430 land?

  5. A heads up: This is a breaking change that will require regenerating ~device~ PAC crates with svd2rust v0.14.0 (I need to land some MSP430 changes in svd2rust before making the v0.14.0 release)

r? @cr1901 cc @awygle

japaric commented 5 years ago
  1. It seems that the NMI (interrupt at 0xFFFC) is not device specific. Should this interrupt be overridable via the interrupt attribute when the "device" feature is disabled? I think this would imply that SVD files must not include NMI in their list of interrupts.
cr1901 commented 5 years ago

Some preliminary answers/responses to your q's/comments:

1./2. The MSP430F55{1,2}x family is an example of an MSP430 (not X) core with 23 interrupts, and up to 64 are reserved (page 53). Assuming you make the vector table a fixed 64 entries, that's (64-16)*2= 96 bytes lost to the vector table on small devices like the MSP430G2211 I use for AT2XT, or about 5% overhead.

  1. Ack. Trying to apply this breaks my own codebase (ERROR(msp430-rt): .vector_table is shorter than expected.), but we can talk about this on IRC.

  2. Flash memory access violation might fall under this (this can occur when writing new data (!) to the flash)?

  3. Ack.

  4. Having not kept up w/ Cortex-M land, I don't understand what you mean by "overrideable". Is the idea that if you're not targeting a specific device, you only have access to NMI b/c it's truly "generic" across all devices? (Having only access to NMI for device-independent code seems quite limiting.)

cr1901 commented 5 years ago

@Disasm Per agreement w/ @japaric, we are waiting a few months (post-thesis) to discuss merging this.

@japaric By agreement, svd2rust 0.14.0 came without the msp430 changes. When you get a free moment- could you elaborate on what msp430-specific changes you needed to add to svd2rust as a "record" of what still needs to be done? That way, we can pick up from where we left off.

cr1901 commented 4 years ago

@japaric I've started looking into bring msp430-rt up to par earnestly. I want to use this PR as a base, but there are some modifications I want to make. In particular (will add to this list):

  1. It appears that cortex-m-rtfm grew the ability to have a variable-sized vector table. I think I want that functionality here as well.
cr1901 commented 4 years ago

@japaric There are some problems I need to resolve locally (along with updating documentation), but I have confirmed your changes work and I can access peripherals with the take API using a device crate I generated with svd2rust v0.16.1 plus local changes. So all in all, I'm very happy w/ the PR, even though it took me forever to review- good work :D!

Re: 1., 2. and 3. I have gone back to the old style of having the user specify a VECTORS memory region manually for now. The start address of the vector table can be extracted from TI's headers (or at least @pftbest's msp430_svd can extract the info), but I'm not sure how to best incorporate that into svd2rust at present.

I wish to punt on 4. and 6. for now and come back to them later.

Re: 5.

By agreement, svd2rust 0.14.0 came without the msp430 changes.

I'm taking care of the required changes on my end- at least, the ones required to play nice with this crate's breaking changes :).

My additions to this PR can be found here: https://github.com/cr1901/msp430-rt/tree/rt-up