hardcoded rjmp to start of new firmware

[mattvenn]

When I compile my firmware (fork of avr keyscanner: https://github.com/Dygmalab/avr-keyscanner/tree/arm-flasher) the interrupt table is longer:

00000156 <__ctors_end>:

so the rjmp hardcoded here https://github.com/keyboardio/attiny_i2c_bootloader/blob/5b14e6ed124770c6de17157b63fb360d59ffac06/TWI_Slave/twi_slave.c#L201

Doesn't work.

[mattvenn]

Could we not just jmp (not rjmp) to the start of the interrupt table, and then that would rjmp us to the correct place for any firmware?

[obra]

@swenson I'd love to get your take on this.

[mattvenn]

I'm also stuck on figuring out 2 things:

1/- the bootloader protects the interrupt table so that on reset it always gets run. But then how does the uploaded program then get to handle interrupts? For example our firmware uses SPI, timer and I2C interrupts and all work as expected.

2/- how is the value for the rjmp calculated? I can see that 0x1bc8 + 0x0038 = 0x1c00. Where 0x1c00 is the start of the bootloader. But what has that got to do with the new program's starting point? Why wouldn't it be __vectors + 0x0038 (I checked and this doesn't work).

[obra]

@mattvenn - I think I'm now seeing the issue you were. It sure as heck -looks- like the issue is related to a change in GCC 6 or GCC7 and how it handles the vector table. Can you confirm that you were hitting this with a GCC of that vintage or newer?

Historically, we always built with GCC4.9 or 5.4, because that was what was in the Arduino toolchain.

[obra]

Actually, I think you were probably using GCC 4.9 or GCC 5.

With GCC 5: this is your vector table and the bit that comes after it:

Disassembly of section .text:

00000000 <__vectors>: 0: b2 c0 rjmp .+356 ; 0x166 <__ctors_end> 2: cc c0 rjmp .+408 ; 0x19c <__bad_interrupt> 4: cb c0 rjmp .+406 ; 0x19c <__bad_interrupt> 6: ca c0 rjmp .+404 ; 0x19c <__bad_interrupt> 8: c9 c0 rjmp .+402 ; 0x19c <__bad_interrupt> a: c8 c0 rjmp .+400 ; 0x19c <__bad_interrupt> c: c7 c0 rjmp .+398 ; 0x19c <__bad_interrupt> e: c6 c0 rjmp .+396 ; 0x19c <__bad_interrupt> 10: c5 c0 rjmp .+394 ; 0x19c <__bad_interrupt> 12: ae c2 rjmp .+1372 ; 0x570 <__vector_9> 14: c3 c0 rjmp .+390 ; 0x19c <__bad_interrupt> 16: c2 c0 rjmp .+388 ; 0x19c <__bad_interrupt> 18: c1 c0 rjmp .+386 ; 0x19c <__bad_interrupt> 1a: c0 c0 rjmp .+384 ; 0x19c <__bad_interrupt> 1c: bf c0 rjmp .+382 ; 0x19c <__bad_interrupt> 1e: 6c c3 rjmp .+1752 ; 0x6f8 <__vector_15> 20: bd c0 rjmp .+378 ; 0x19c <__bad_interrupt> 22: bc c0 rjmp .+376 ; 0x19c <__bad_interrupt> 24: bb c0 rjmp .+374 ; 0x19c <__bad_interrupt> 26: b3 c2 rjmp .+1382 ; 0x58e <__vector_19> 28: 22 c1 rjmp .+580 ; 0x26e <twi_data_requested+0x30> 2a: 43 c1 rjmp .+646 ; 0x2b2 <twi_data_requested+0x74> 2c: 4e c1 rjmp .+668 ; 0x2ca <twi_data_requested+0x8c> 2e: 76 c1 rjmp .+748 ; 0x31c <stack+0x1d> 30: 75 c1 rjmp .+746 ; 0x31c <stack+0x1d> 32: 74 c1 rjmp .+744 ; 0x31c <stack+0x1d> 34: 4f c1 rjmp .+670 ; 0x2d4 <twi_data_requested+0x96> 36: 72 c1 rjmp .+740 ; 0x31c <stack+0x1d> 38: 4f c1 rjmp .+670 ; 0x2d8 <twi_data_requested+0x9a> 3a: 54 c1 rjmp .+680 ; 0x2e4 <twi_data_requested+0xa6> 3c: 57 c1 rjmp .+686 ; 0x2ec <twi_data_requested+0xae> 3e: 66 c1 rjmp .+716 ; 0x30c <stack+0xd> 40: 5f c1 rjmp .+702 ; 0x300 <stack+0x1> 42: 4d c1 rjmp .+666 ; 0x2de <twi_data_requested+0xa0> 44: 6b c1 rjmp .+726 ; 0x31c <stack+0x1d> 46: 5f c1 rjmp .+702 ; 0x306 <stack+0x7> 48: 24 c2 rjmp .+1096 ; 0x492 <LOCK_REGION_LENGTH+0x92> 4a: db c1 rjmp .+950 ; 0x402 <LOCK_REGION_LENGTH+0x2> 4c: f7 c1 rjmp .+1006 ; 0x43c <LOCK_REGION_LENGTH+0x3c> 4e: 0a c2 rjmp .+1044 ; 0x464 <LOCK_REGION_LENGTH+0x64> 50: 84 c2 rjmp .+1288 ; 0x55a <LOCK_REGION_LENGTH+0x15a> 52: 83 c2 rjmp .+1286 ; 0x55a <LOCK_REGION_LENGTH+0x15a> 54: 82 c2 rjmp .+1284 ; 0x55a <LOCK_REGION_LENGTH+0x15a> 56: 21 c2 rjmp .+1090 ; 0x49a <LOCK_REGION_LENGTH+0x9a> 58: 22 c2 rjmp .+1092 ; 0x49e <LOCK_REGION_LENGTH+0x9e> 5a: 23 c2 rjmp .+1094 ; 0x4a2 <LOCK_REGION_LENGTH+0xa2> 5c: 18 c2 rjmp .+1072 ; 0x48e <LOCK_REGION_LENGTH+0x8e> 5e: 1b c2 rjmp .+1078 ; 0x496 <LOCK_REGION_LENGTH+0x96> 60: db c1 rjmp .+950 ; 0x418 <LOCK_REGION_LENGTH+0x18> 62: 23 c2 rjmp .+1094 ; 0x4aa <LOCK_REGION_LENGTH+0xaa> 64: 12 c2 rjmp .+1060 ; 0x48a <LOCK_REGION_LENGTH+0x8a>

00000066 <__trampolines_end>: 66: ff ff .word 0xffff ; ???? 68: 00 03 mulsu r16, r16 6a: 06 09 sbc r16, r6 6c: 0c 0f add r16, r28

With GCC9:

Disassembly of section .text:

00000000 <__vectors>: 0: 93 c0 rjmp .+294 ; 0x128 <__ctors_end> 2: ad c0 rjmp .+346 ; 0x15e <__bad_interrupt> 4: ac c0 rjmp .+344 ; 0x15e <__bad_interrupt> 6: ab c0 rjmp .+342 ; 0x15e <__bad_interrupt> 8: aa c0 rjmp .+340 ; 0x15e <__bad_interrupt> a: a9 c0 rjmp .+338 ; 0x15e <__bad_interrupt> c: a8 c0 rjmp .+336 ; 0x15e <__bad_interrupt> e: a7 c0 rjmp .+334 ; 0x15e <__bad_interrupt> 10: a6 c0 rjmp .+332 ; 0x15e <__bad_interrupt> 12: 8f c2 rjmp .+1310 ; 0x532 <__vector_9> 14: a4 c0 rjmp .+328 ; 0x15e <__bad_interrupt> 16: a3 c0 rjmp .+326 ; 0x15e <__bad_interrupt> 18: a2 c0 rjmp .+324 ; 0x15e <__bad_interrupt> 1a: a1 c0 rjmp .+322 ; 0x15e <__bad_interrupt> 1c: a0 c0 rjmp .+320 ; 0x15e <__bad_interrupt> 1e: 39 c3 rjmp .+1650 ; 0x692 <__vector_15> 20: 9e c0 rjmp .+316 ; 0x15e <__bad_interrupt> 22: 9d c0 rjmp .+314 ; 0x15e <__bad_interrupt> 24: 9c c0 rjmp .+312 ; 0x15e <__bad_interrupt> 26: 8b c2 rjmp .+1302 ; 0x53e <__vector_19>

00000028 <__trampolines_end>: 28: ff ff .word 0xffff ; ????

[mattvenn]

Correct, I was using a manually installed avr-gcc, version 4.9.2

[obra]

Ok. It's, as far as I can tell, absolutely critical that you only ever use GCC 5 or older to build keyscanner firmware going forward for that bootloader. ᐧ

On Thu, Jan 16, 2020 at 4:58 AM matt venn notifications@github.com wrote:

Correct, I was using a manually installed avr-gcc, version 4.9.2

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/keyboardio/attiny_i2c_bootloader/issues/2?email_source=notifications&email_token=AAALC2FQKCZOQNNF5GRIWD3Q6BKYZA5CNFSM4EZR6J6KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJD7ARQ#issuecomment-575139910, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAALC2CCJHJXR4CFR52N5CLQ6BKYZANCNFSM4EZR6J6A .

[tlyu]

Having looked at a related problem recently, I think it's a change in how GCC compiles jump tables for switch statements, etc.

I have some rough ideas about how to have the bootloader patch into an unused interrupt vector to preserve the uploaded application reset vector, which should make this more robust. (An alternative is to reserve part of the bootloader flash for this vector, but that makes accidentally overwriting or corrupting the bootloader more likely.)

[obra]

With GCC 7+, how likely do you think we are to see this fluctuate further?

On Fri, May 20, 2022 at 10:43 AM Taylor Yu @.***> wrote:

Having looked at a related problem recently, I think it's a change in how GCC compiles jump tables for switch statements, etc.

I have some rough ideas about how to have the bootloader patch into an unused interrupt vector to preserve the uploaded application reset vector, which should make this more robust. (An alternative is to reserve part of the bootloader flash for this vector, but that makes accidentally overwriting or corrupting the bootloader more likely.)

— Reply to this email directly, view it on GitHub https://github.com/keyboardio/attiny_i2c_bootloader/issues/2#issuecomment-1133160749, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAALC2GTXEVVCIB2NN4NPZTVK7FNHANCNFSM4EZR6J6A . You are receiving this because you commented.Message ID: @.***>

[tlyu]

With GCC 7+, how likely do you think we are to see this fluctuate further?

I'm not sure. GCC developers seem to err on the side of faster instead of smaller code. For example, I'm having some trouble with buffer_reset_vector being aggressively unrolled, even with explicit flags like -fno-unroll-loops -Os, etc., which pushes the Model 01 ATtiny bootloader over the 1k threshold with my toolchain.

I wouldn't be surprised if they defaulted to jump tables again in the future (thus moving application start addresses around again).

[tlyu]

In terms of design, I think we can choose to either commit to stealing an interrupt vector slot to stash the application reset vector, or we can reserve a flash page near the bootloader (but not within its executable space, to make self-programming less risky). I think we're already committed to losing the INT0 vector, because the current bootloader tries to preserve it (along with the reset vector) when flashing new application code.

On the other hand, it looks like the factory programming procedure actually doesn't use the bootloader, so maybe it isn't that important?

Any preferences?

(If I get really inspired, I might even work on a bootloader upgrader, because the ATtiny88 doesn't have hardware bootloader section protection.)

[obra]

Indeed, the factory flashes the whole thing at once over ICSP. I don't actually feel like I have the depth to make design choice on this one, so I'm not going to opine on what the right way is. -j

On Sun, May 22, 2022 at 6:46 PM Taylor Yu @.***> wrote:

In terms of design, I think we can choose to either commit to stealing an interrupt vector slot to stash the application reset vector, or we can reserve a flash page near the bootloader (but not within its executable space, to make self-programming less risky). I think we're already committed to losing the INT0 vector, because the current bootloader tries to preserve it (along with the reset vector) when flashing new application code.

On the other hand, it looks like the factory programming procedure actually doesn't use the bootloader, so maybe it isn't that important?

Any preferences?

(If I get really inspired, I might even work on a bootloader upgrader, because the ATtiny88 doesn't have hardware bootloader section protection.)

— Reply to this email directly, view it on GitHub https://github.com/keyboardio/attiny_i2c_bootloader/issues/2#issuecomment-1134077994, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAALC2GET6GGGCWCH6VSUSDVLLPPBANCNFSM4EZR6J6A . You are receiving this because you commented.Message ID: @.***>