Why does second stage bootloader not set VTOR and SP? #6

Closed
opened 2021-01-23 13:47:47 -06:00 by thejpster · 28 comments
thejpster commented 2021-01-23 13:47:47 -06:00 (Migrated from github.com)

Having to make 0x100 in flash a function as opposed to a vector table complicates the use of other programming languages and SDKs, which assume flash starts with a vector table.

Is there a reason why the second stage bootloader doesn't set VTOR to 0x1000_0100, SP to [0x1000_0100] and then jump to [0x1000_0104]?

Having to make 0x100 in flash a function as opposed to a vector table complicates the use of other programming languages and SDKs, which assume flash starts with a vector table. Is there a reason why the second stage bootloader doesn't set VTOR to 0x1000_0100, SP to [0x1000_0100] and then jump to [0x1000_0104]?
kilograham commented 2021-01-23 14:31:25 -06:00 (Migrated from github.com)

Having to make 0x100 in flash a function as opposed to a vector table complicates the use of other programming languages and SDKs, which assume flash starts with a vector table.

I'm not sure that is an assumption they should make (unless flash is at 0x00000000), Still if they don't reference the stack pointer you could replace it with something like

0x47184b00, // ldr r3, [pc, #0]; bx r3

to jump to the reset vector, and actually put a stack pointer in there (though all your bootstrap code need to be in the first 256 bytes of flash somewhere)

> Having to make 0x100 in flash a function as opposed to a vector table complicates the use of other programming languages and SDKs, which assume flash starts with a vector table. I'm not sure that is an assumption they should make (unless flash is at 0x00000000), Still if they don't reference the stack pointer you could replace it with something like 0x47184b00, // ldr r3, [pc, #0]; bx r3 to jump to the reset vector, and actually put a stack pointer in there (though all your bootstrap code need to be in the first 256 bytes of flash somewhere)
Wren6991 commented 2021-01-23 14:32:03 -06:00 (Migrated from github.com)

complicates the use of other programming languages and SDKs, which assume flash starts with a vector table.

Seems like the second stage bootloader already breaks this assumption, so I guess the other issue is the alignment requirement of VTOR, and not wanting to hoist the reset handler over the vector table to fill the alignment hole?

> complicates the use of other programming languages and SDKs, which assume flash starts with a vector table. Seems like the second stage bootloader already breaks this assumption, so I guess the other issue is the alignment requirement of VTOR, and not wanting to hoist the reset handler over the vector table to fill the alignment hole?
kilograham commented 2021-01-23 14:33:37 -06:00 (Migrated from github.com)

P.S. yay, rust i'm guessing - i wanted to give it a try myself, but i had zero time, and my Rust is not very idiomatic yet!

P.S. yay, rust i'm guessing - i wanted to give it a try myself, but i had zero time, and my Rust is not very idiomatic yet!
Wren6991 commented 2021-01-23 14:35:34 -06:00 (Migrated from github.com)

Is there a reason why the second stage bootloader doesn't set VTOR to 0x1000_0100, SP to [0x1000_0100] and then jump to [0x1000_0104]?

Purely a size consideration, in case we needed to squeeze more in there at a later time (as you can see it is already quite tight), and want to keep the interface stable. No strong objections to that mechanism but it looks like it would add a few bytes. Then again I guess now would be the time to change it, if ever.

> Is there a reason why the second stage bootloader doesn't set VTOR to 0x1000_0100, SP to [0x1000_0100] and then jump to [0x1000_0104]? Purely a size consideration, in case we needed to squeeze more in there at a later time (as you can see it is already quite tight), and want to keep the interface stable. No strong objections to that mechanism but it looks like it would add a few bytes. Then again I guess now would be the time to change it, if ever.
Wren6991 commented 2021-01-23 14:36:27 -06:00 (Migrated from github.com)

(I'm guessing one of the unstated priors here is that you would rather not ship your own second stages with those SDKs)

(I'm guessing one of the unstated priors here is that you would rather not ship your own second stages with those SDKs)
Wren6991 commented 2021-01-23 14:43:51 -06:00 (Migrated from github.com)

So I understand, you're suggesting a patch like this? (edit: and also setting VTOR as well as vectoring through the table)

--- a/src/rp2_common/boot_stage2/boot2_w25q080.S
+++ b/src/rp2_common/boot_stage2/boot2_w25q080.S
@@ -277,7 +277,10 @@ soft_reset:
     pop {r0}
     cmp r0, #0
     bne 1f
-    ldr r0, =(XIP_BASE + 0x101)
+    ldr r0, =(XIP_BASE + 0x100)
+    ldr r1, [r0]
+    msr msp, r1
+    ldr r0, [r0, #4]
 1:
     bx r0
So I understand, you're suggesting a patch like this? (edit: and also setting VTOR as well as vectoring through the table) ``` --- a/src/rp2_common/boot_stage2/boot2_w25q080.S +++ b/src/rp2_common/boot_stage2/boot2_w25q080.S @@ -277,7 +277,10 @@ soft_reset: pop {r0} cmp r0, #0 bne 1f - ldr r0, =(XIP_BASE + 0x101) + ldr r0, =(XIP_BASE + 0x100) + ldr r1, [r0] + msr msp, r1 + ldr r0, [r0, #4] 1: bx r0 ```
kilograham commented 2021-01-23 14:46:12 -06:00 (Migrated from github.com)

oh i had entirely misread the original topic - ignore everything i said!

oh i had entirely misread the original topic - ignore everything i said!
Wren6991 commented 2021-01-23 15:29:41 -06:00 (Migrated from github.com)

Thought about this a bit longer, and it makes sense to put vector table at the start in flash images, but we also have a NO_FLASH build flag, where binaries are built to be loaded into RAM. The issue is that when loaded over UF2->USB, the bootrom enters these images in the same way that boot2 currently enters flash crt0.

This means we'll have vector table first on flash builds (which by default we then copy out and relocate into RAM anyway), and second on RAM-only builds. I can't see this causing any particular trouble other than making people's eyes bulge, so I'll raise a PR to make these changes.

Thought about this a bit longer, and it makes sense to put vector table at the start in flash images, but we also have a NO_FLASH build flag, where binaries are built to be loaded into RAM. The issue is that when loaded over UF2->USB, the bootrom enters these images in the same way that boot2 currently enters flash crt0. This means we'll have vector table first on flash builds (which by default we then copy out and relocate into RAM anyway), and second on RAM-only builds. I can't see this causing any particular trouble other than making people's eyes bulge, so I'll raise a PR to make these changes.
thejpster commented 2021-01-23 15:36:44 -06:00 (Migrated from github.com)

Thanks!

https://GitHub.com/thejpster/pico-blink-rs contains my terrible hacks.

Thanks! https://GitHub.com/thejpster/pico-blink-rs contains my terrible hacks.
thejpster commented 2021-01-23 15:39:33 -06:00 (Migrated from github.com)

And to be clear, yes I would prefer not to ship either ASM or blobs with a Rust application. ASM is preferable, but it involves pulling in big dependencies to kick off gcc / as automatically.

And to be clear, yes I would prefer not to ship either ASM or blobs with a Rust application. ASM is preferable, but it involves pulling in big dependencies to kick off gcc / as automatically.
jonas-schievink commented 2021-01-23 15:51:17 -06:00 (Migrated from github.com)

Shipping ASM shims is much easier if you take the approach that I've used in the cortex-m crate, it works without any external dependencies: https://github.com/rust-embedded/cortex-m/blob/master/asm/lib.rs

Shipping ASM shims is much easier if you take the approach that I've used in the cortex-m crate, it works without any external dependencies: https://github.com/rust-embedded/cortex-m/blob/master/asm/lib.rs
Wren6991 commented 2021-01-23 15:51:43 -06:00 (Migrated from github.com)

Ok, so the ideal situation from a dependency point of view is to reimplement boot2 in rust, if you are able? In other words, you would be free to change the handover between that second stage and the reset handler of the rust program?

I think it still makes sense to make this change to the boot2 files shipped with the SDK.

Ok, so the ideal situation from a dependency point of view is to reimplement boot2 in rust, if you are able? In other words, you would be free to change the handover between that second stage and the reset handler of the rust program? I think it still makes sense to make this change to the boot2 files shipped with the SDK.
thejpster commented 2021-01-23 16:34:21 -06:00 (Migrated from github.com)

I think the change would help with supporting other OSes too.

We'll look at doing boot2 in Rust. It'll be an interesting challenge!

I think the change would help with supporting other OSes too. We'll look at doing boot2 in Rust. It'll be an interesting challenge!
kilograham commented 2021-01-23 17:41:17 -06:00 (Migrated from github.com)

Note the other thing we have in the first 256 bytes is the root of our machine walkable "binary info" tree (used by picotool)
This is a bunch of (semi) self-describing information embedded in the binary; we have a 20 byte header that is only looked for in 10000100 to 10000200 in flash binaries (though we can obviously change that if it is going to be an issue for other languages, so goot to get your opinion)

Our hope is to have other languages include other information that might be useful to users (you can run this against a BIN/UF2/ELF file or against a device to see what is on it!)

The information is designed to be compact, but if nothing else included the end address of the binary is good because it allows you to do picotool save to save your program off your RP2040 device into a UF2/BIN

e.g.

$ picotool info -a firmware.elf
File firmware.elf:

Program Information
 name:            MicroPython
 version:         v1.13-289-g1196871a0
 features:        USB REPL
                  thread support
 frozen modules:  _boot, rp2, ds18x20, onewire, uasyncio, uasyncio/core, uasyncio/event,
                  uasyncio/funcs, uasyncio/lock, uasyncio/stream
 binary start:    0x10000000
 binary end:      0x10038ac0
 embedded drive:  0x100a0000-0x10200000 (1408K): MicroPython

Fixed Pin Information
 none

Build Information
 sdk version:       1.0.0
 pico_board:        pico
 build date:        Jan 20 2021
 build attributes:  MinSizeRel

or

$ picotool info -a ./scanvideo/sprite_demo/sprite_demo.elf
File ./scanvideo/sprite_demo/sprite_demo.elf:

Program Information
 name:          sprite_demo
 features:      UART stdin / stdout
 binary start:  0x10000000
 binary end:    0x100179ec

Fixed Pin Information
 0-4:    Red 0-4
 6-10:   Green 0-4
 11-15:  Blue 0-4
 16:     HSync
 17:     VSync
 18:     Display Enable
 19:     Pixel Clock
 20:     UART1 TX
 21:     UART1 RX

Build Information
 sdk version:       1.0.0
 pico_board:        vgaboard
 build date:        Jan 19 2021
 build attributes:  Release
Note the other thing we have in the first 256 bytes is the root of our machine walkable "binary info" tree (used by picotool) This is a bunch of (semi) self-describing information embedded in the binary; we have a 20 byte header that is only looked for in 10000100 to 10000200 in flash binaries (though we can obviously change that if it is going to be an issue for other languages, so goot to get your opinion) Our hope is to have other languages include other information that might be useful to users (you can run this against a BIN/UF2/ELF file or against a device to see what is on it!) The information is designed to be compact, but if nothing else included the end address of the binary is good because it allows you to do `picotool save` to save your program off your RP2040 device into a UF2/BIN e.g. ``` $ picotool info -a firmware.elf File firmware.elf: Program Information name: MicroPython version: v1.13-289-g1196871a0 features: USB REPL thread support frozen modules: _boot, rp2, ds18x20, onewire, uasyncio, uasyncio/core, uasyncio/event, uasyncio/funcs, uasyncio/lock, uasyncio/stream binary start: 0x10000000 binary end: 0x10038ac0 embedded drive: 0x100a0000-0x10200000 (1408K): MicroPython Fixed Pin Information none Build Information sdk version: 1.0.0 pico_board: pico build date: Jan 20 2021 build attributes: MinSizeRel ``` or ``` $ picotool info -a ./scanvideo/sprite_demo/sprite_demo.elf File ./scanvideo/sprite_demo/sprite_demo.elf: Program Information name: sprite_demo features: UART stdin / stdout binary start: 0x10000000 binary end: 0x100179ec Fixed Pin Information 0-4: Red 0-4 6-10: Green 0-4 11-15: Blue 0-4 16: HSync 17: VSync 18: Display Enable 19: Pixel Clock 20: UART1 TX 21: UART1 RX Build Information sdk version: 1.0.0 pico_board: vgaboard build date: Jan 19 2021 build attributes: Release ```
thejpster commented 2021-01-24 03:53:38 -06:00 (Migrated from github.com)

Off the top of my head, 256 bytes minus 20 bytes leaves room for 57 interrupt vectors, which is more than enough I think. Configuring an SDK to put a special section after the vector table is usually much easier than trying to put it in front of the vector table.

Off the top of my head, 256 bytes minus 20 bytes leaves room for 57 interrupt vectors, which is more than enough I think. Configuring an SDK to put a special section after the vector table is usually much easier than trying to put it in front of the vector table.
Dirbaio commented 2021-01-24 12:07:19 -06:00 (Migrated from github.com)

I'm not that sure moving the vectors to 0x10000100 is that advantageous. If we're going to copy the vectors to RAM anyway, they can be anywhere in flash really. They could even be first thing in .data, so they'll be initialized by the .data copy.

I'm not that sure moving the vectors to 0x10000100 is that advantageous. If we're going to copy the vectors to RAM anyway, they can be anywhere in flash really. They could even be first thing in .data, so they'll be initialized by the .data copy.
Wren6991 commented 2021-01-24 13:04:01 -06:00 (Migrated from github.com)

I'm not that sure moving the vectors to 0x10000100 is that advantageous

I'm not aware of the build system or language context here. It makes sense to me that existing projects might assume their image starts with a vector table, as they may expect the hardware to be doing the vectoring. It seems a bit cleaner for boot2 (whose contents ought to be language-agnostic, and whose size is fixed) to be the only thing bumped on the front of that.

> I'm not that sure moving the vectors to 0x10000100 is that advantageous I'm not aware of the build system or language context here. It makes sense to me that existing projects might assume their image starts with a vector table, as they may expect the hardware to be doing the vectoring. It seems a bit cleaner for boot2 (whose contents ought to be language-agnostic, and whose size is fixed) to be the only thing bumped on the front of that.
kilograham commented 2021-01-24 13:19:44 -06:00 (Migrated from github.com)

Yes FYI the original reason for this was that we support the very non standard idea of RAM UF2s, and the bootrom has no idea what the entry point is in the binary, so we go with the lowest address. Also RAM binaries means that you can't just expect to always download code in debugger, then reboot, you should also support "file" and "run".

Anyway I think we support all that with the new layout (which I agree is more sensible) without needing to change the bootrom :-)

Yes FYI the original reason for this was that we support the very non standard idea of RAM UF2s, and the bootrom has no idea what the entry point is in the binary, so we go with the lowest address. Also RAM binaries means that you can't just expect to always download code in debugger, then reboot, you should also support "file" and "run". Anyway I think we support all that with the new layout (which I agree is more sensible) without needing to change the bootrom :-)
kilograham commented 2021-01-24 13:50:16 -06:00 (Migrated from github.com)

I'm not that sure moving the vectors to 0x10000100 is that advantageous. If we're going to copy the vectors to RAM anyway, they can be anywhere in flash really. They could even be first thing in .data, so they'll be initialized by the .data copy.

Yeah definitely worth considering the RAM binary use case. (RP2040 with no flash, running something on the device without disturbing the flash) use cases, since you do need code at the beginning for that (although clearly you could just trade out the boot stage 2 for a 256 byte indirect thru boot vector at 0x100)

> I'm not that sure moving the vectors to 0x10000100 is that advantageous. If we're going to copy the vectors to RAM anyway, they can be anywhere in flash really. They could even be first thing in .data, so they'll be initialized by the .data copy. Yeah definitely worth considering the RAM binary use case. (RP2040 with no flash, running something on the device without disturbing the flash) use cases, since you do need code at the beginning for that (although clearly you could just trade out the boot stage 2 for a 256 byte indirect thru boot vector at 0x100)
kilograham commented 2021-01-24 15:09:33 -06:00 (Migrated from github.com)

Off the top of my head, 256 bytes minus 20 bytes leaves room for 57 interrupt vectors, which is more than enough I think. Configuring an SDK to put a special section after the vector table is usually much easier than trying to put it in front of the vector table.

Agreed; more of an FYI than a concern over space

> Off the top of my head, 256 bytes minus 20 bytes leaves room for 57 interrupt vectors, which is more than enough I think. Configuring an SDK to put a special section after the vector table is usually much easier than trying to put it in front of the vector table. Agreed; more of an FYI than a concern over space
conkerkh commented 2021-01-26 10:46:49 -06:00 (Migrated from github.com)

Traditionally the main app will relocate VTOR if necessary, which is why it doesn't make sense to do it in boot stage2. You can have a look at SystemInit function in system_XXX.c files in CMSIS libraries. This concept makes sense, you can have multiple implementations of system_XXXX.c, one which will copy VTOR to RAM and relocate address there, another while will use FLASH for this purpose. I don't think that that it's necessary to do it in boot_stage2.

On the other hand, I think that you could use C directly writing boot_stage2 code. It's more readable for most people and the size of the output is pretty much the same, or very close (it fits 252bytes). I've verified this by rewriting this in C. I'm porting this ASM to C and the results look ok. I think I should get my pico tomorrow and will check if everything works. If it's ok then I will post a like to my repo.

Traditionally the main app will relocate VTOR if necessary, which is why it doesn't make sense to do it in boot stage2. You can have a look at SystemInit function in system_XXX.c files in CMSIS libraries. This concept makes sense, you can have multiple implementations of system_XXXX.c, one which will copy VTOR to RAM and relocate address there, another while will use FLASH for this purpose. I don't think that that it's necessary to do it in boot_stage2. On the other hand, I think that you could use C directly writing boot_stage2 code. It's more readable for most people and the size of the output is pretty much the same, or very close (it fits 252bytes). I've verified this by rewriting this in C. I'm porting this ASM to C and the results look ok. I think I should get my pico tomorrow and will check if everything works. If it's ok then I will post a like to my repo.
kilograham commented 2021-01-26 10:59:03 -06:00 (Migrated from github.com)

Yeah, the question here is not about installing the VTOR, but whether to allow the real binary to have its vector table at the beginning of the binary (which we currently don't as we jump there). The change would just be to pull initial PC,SP from 0x100-0x108 instead of jumping to 0x100

Yeah, the question here is not about installing the VTOR, but whether to allow the real binary to have its vector table at the beginning of the binary (which we currently don't as we jump there). The change would just be to pull initial PC,SP from 0x100-0x108 instead of jumping to 0x100
conkerkh commented 2021-01-26 11:48:49 -06:00 (Migrated from github.com)

Oh damn, yeah missed the point :D I've looked at the linker file again, I get what you mean. That looks really weird indeed... What was the initial idea behind placing reset section in front of vector?

Oh damn, yeah missed the point :D I've looked at the linker file again, I get what you mean. That looks really weird indeed... What was the initial idea behind placing reset section in front of vector?
Wren6991 commented 2021-01-26 14:04:03 -06:00 (Migrated from github.com)

That looks really weird indeed... What was the initial idea behind placing reset section in front of vector

Keeping the handover between boot2 and crt0 absolutely minimal in case there was any critical hardware setup we needed to use that space for. We've released now, things seem to be basically working, so it makes sense to spend those bytes to bring our binary layout in line with other codebases' assumptions

I've verified this by rewriting this in C. I'm porting this ASM to C and the results look ok. I think I should get my pico tomorrow and will check if everything works. If it's ok then I will post a like to my repo.

Please do share -- right now I'm keen on just getting the binary layout change merged before people go too much further with bare metal language bringup stuff, but it would be interesting to look at rewriting in C in the future (and might let us have a single, more configurable boot2)

> That looks really weird indeed... What was the initial idea behind placing reset section in front of vector Keeping the handover between boot2 and crt0 absolutely minimal in case there was any critical hardware setup we needed to use that space for. We've released now, things seem to be basically working, so it makes sense to spend those bytes to bring our binary layout in line with other codebases' assumptions > I've verified this by rewriting this in C. I'm porting this ASM to C and the results look ok. I think I should get my pico tomorrow and will check if everything works. If it's ok then I will post a like to my repo. Please do share -- right now I'm keen on just getting the binary layout change merged before people go too much further with bare metal language bringup stuff, but it would be interesting to look at rewriting in C in the future (and might let us have a single, more configurable boot2)
kilograham commented 2021-01-26 16:14:33 -06:00 (Migrated from github.com)

I added a comment to the PR, but I don't think we should actually install the VTOR. we should just redirect thru the first 8 bytes, and leave it to the language runtime to decide how they setup vector table. Thoughts?

I added a comment to the PR, but I don't think we should actually install the VTOR. we should just redirect thru the first 8 bytes, and leave it to the language runtime to decide how they setup vector table. Thoughts?
Wren6991 commented 2021-01-26 16:37:23 -06:00 (Migrated from github.com)

I think the issue with not installing VTOR is that a lot of existing runtime will just turn on IRQs without modifying VTOR (as VTOR is not modifiable on some systems anyway), under the assumption that VTOR points at the vector table that you came into the image through, which is true on almost every system apart from ours.

If we didn't install VTOR, this would send all such code through the bootrom vector table.

This doesn't affect our runtime anyway because we relocate VTOR before enabling IRQs, so it wouldn't affect our code, and would give other crt0s an experience that's more consistent with other Cortex-M platforms.

I think the issue with not installing VTOR is that a lot of existing runtime will just turn on IRQs without modifying VTOR (as VTOR is not modifiable on some systems anyway), under the assumption that VTOR points at the vector table that you came into the image through, which is true on almost every system apart from ours. If we didn't install VTOR, this would send all such code through the bootrom vector table. This doesn't affect our runtime anyway because _we_ relocate VTOR before enabling IRQs, so it wouldn't affect our code, and would give other crt0s an experience that's more consistent with other Cortex-M platforms.
kilograham commented 2021-01-26 17:49:47 -06:00 (Migrated from github.com)

Yes, you are right, my brain was out to lunch

Yes, you are right, my brain was out to lunch
Wren6991 commented 2021-01-28 08:13:02 -06:00 (Migrated from github.com)

The answer to the question in this thread's title is now "it does" :)

The answer to the question in this thread's title is now "it does" :)
Sign in to join this conversation.
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-raspberrypi/pico-sdk#6
No description provided.