-
Notifications
You must be signed in to change notification settings - Fork 6
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
Support bootloader version 3 in command offset 0x8E #55
Support bootloader version 3 in command offset 0x8E #55
Conversation
Bootloader v1/v2/v3, start of flash: What if: Can it be a better strategy to start by comparing if 1FFE is 8A? If not, then check if 1E00 is 8A? (as this machine is little endian, and the instructions are 16 bit wide, the most significant byte of the opcode is the second byte. Thus, using the first byte to check for "8A" gives a bit random results, if the address also can be used to store an instruction. Rjmp, first instruction of bootloader v3 and newer, is 0xCxxx", which in memory is "xx Cx"). |
That said. We should probably ban instructions in the form "8A xx" as the first bootloader instruction, as older smc versions will then report some random bootloader version. Thus, with this convention, we can keep this PR as is |
Banning the bootloader to begin with 0x8a is a good idea. I don't think the AVRA assembler supports that, but we can at least let one of the scripts run by the Github action look for this. This is one possible solution: https://github.com/stefan-b-jakobsson/x16-smc-bootloader/tree/20241013-prohibit-0x8a |
Sounds good. Note that, due to AVR having a little endian architecture, the banned instructions are the "xx8A" instructions. Which can be C08A (rjmp .+276). The first instruction will likely always be a rjmp instruction ("Cxxx"). Thus, we can use all rjmp instructions, except +276 (+ N * 256). If this actually turns out to be a problem, we can always jump to some other address inside the bootloader which jumps to the correct address, I assume we will always find room for 2 extra bytes. |
Changing the order of the if statements is also a good idea. I did that as well. |
Well, I kind of changed my mind when realizing that older SMC versions will then report inconsistent versions compared to newer SMC versions IF newer bootloaders have a first instruction which is a "8A xx" instruction. But, given that we can choose to ban this instruction from now on (and can get pretty sure there will not be made any "rogue" bootloaders (well, we actually have some experience here.....)), I guess the order should not really matter. For best backward compatibility, in general, I guess it is best to start with the old stuff, and then move to the new, if the old indicate that this should be done. Up to you. |
I did some thinking in terms of this PR: Version rules: All instructions in the form "8A xx" must be banned as the first instruction. This means that proper bootloaders will only have 8a in address 1E00 or 1FFE. This means that the order of checks is not important for "proper" bootloaders. A rogue (improper) bootloader will behave like this: If 1E00 is checked first: If 1FFE is checked first: Due to how rogue bootloaders will be handled, I think reading 1E00 first, may be the best and most consistent option. In other words, I suggest to revert the latest commit. (Although rogue bootloaders should off course never happen....) |
A third alternative would be: If 0x1e00 = 0x8a & 0x1e01 = 0x01 or 0x02: Read bootloader version from 0x1e01 |
Sure, if we can afford keeping the test so strict. This will also add some extra bytes to the code size. But, note that, if there is a test for "if 1e00 = 8a & 1e01 = 1 or 2", it may suggest that other 8axx can be used. Which will give inconsistent results based on smc version. Note that bootloader v3 already will give inconsistent version based on smc version, but, that is due to this test have not been implemented yet. Option 1 (1e00 before 1ffe): Pros: Straight forward, incremental, backward compatible for old bootloaders, minimal code size Cons: Rogue bootloaders (8axx code at 1e00) shows 1e01 as version (but, version is bad anyway) Option 2 (1ffe before 1e00): Note: Rogue bootloaders shows 1fff as version, while for old smc version it shows 1e01. Pros: Intended rogue version is revealed Cons: Version is rogue and may look like a good version. Version is inconsistent on old vs new SMC (random 1e01 data vs claimed version). But, all newer bootloaders will be inconsistent based on SMC version anyway (FF vs claimed version) Option 3: Strict range check, 1e00 before 1ffe: Note: Rogue bootloaders will most likely show intended version Pros: Intent is more clear in the code. Bigger chance that intended bootloader version is shown Cons: Bigger code size, may suggest that bootloader can start with 8axx One factor is how "rogue versions" is handled (first instruxtion is 8a xx). This should not be used by proper versions, but, we know that bad versions can suddenly be present. If this is the case, I would argue that it is not that important to show the intended version. The best option could perhaps be to have a sanity check and report e.g. FE which indicates "version inconsistency". Yet, this occupies bytes in flash which we maybe do not have. And, to confirm a rogue (and hence, "bad" bootloader), we have learnt that the best tool is to read the entire bootloader and compare the checksum against known bootloaders. In fact: SMC does not really need to have this sanity check. SMC can be simple and stupid. The programming tools can have the sanity check. If current SMC version is 47.2.3 or newer, this can be the programming tool logic:
|
Sorry for the long, previous comment. To summarize: I suggest to keep the SMC bootloader version test, in SMC firmware, simple and dumb, to save space, but add sanity checks for corrupt versions in the programming tools, by reading all bytes of the bootloader. Thus, I suggest any of option 1 and 2. |
OK, I reverted the code to checking 0x1e00 first, and 0x1ffe second. The update program doesn't currently verify the bootloader by calculating and comparing its CRC. I guess that's a good improvement. |
Maybe I should get a new hobby 😄 |
I forgot: This is a change of the machine code, and behavior, and thus, the version number should probably have been bumped as well. @stefan-b-jakobsson |
Command offset 0x8E returns the bootloader version.
This adds support, so that the version is returned if you have bootloader 3 installed. Currently that doesn't work.