[sw,otbn] Fix earlgrey otbn patch offset

[sasdf]

The older OTBN boot binary in the Earlgrey-PROD-A2-M6-ROM-RC1 ROM release is different from the one when PR #27679 was authored.

The instruction to be patched in the binary built on Earlgrey-PROD-A2-M6-ROM-RC1 tag is 0x888:

git checkout --detach Earlgrey-PROD-A2-M6-ROM-RC1
./bazelisk.sh build //sw/otbn/crypto:boot
ar x --output=/tmp/ bazel-bin/sw/otbn/crypto/boot.rv32embed.a
llvm-objcopy-19 -O binary /tmp/boot.rv32embed.o /tmp/boot.bin

xxd /tmp/boot.bin | grep '7b38 fac1'
00000880: 7bb9 0fc1 7b3a f841 7b38 fac1 0b77 2000  {...{:.A{8...w .

[nasahlpa]

Thanks @sasdf!

I had a look into the disassembly of the Earlgrey-PROD-A2-M6-ROM-RC1 tag as well and I can confirm that the address is 0x888.

[cfrantz]

Does the newer OTBN binary even need the patch?

Changing this will break future immutable sections for existing chips. In order for this to function correctly, we need to look at chip_info (or maybe its called build_info now) and decided whether to apply the patch and to what offset to apply it.

[sasdf]

Does the newer OTBN binary even need the patch?

This is a patch to OTBN binary in earlgrey ROM, which won't have newer OTBN binary. Though, I can't speak whether this patch is necessary.

Changing this will break future immutable sections for existing chips.

RomExt for chips with immutability enforced should be linked to their prebuilt sections (in private repo), which won't be affected by this PR. In fact, we already have 4 commits since the last imm_section release that changed its hash.

[cfrantz]

This is a patch to OTBN binary in earlgrey ROM, which won't have newer OTBN binary. Though, I can't speak whether this patch is necessary.

Correct, but any recent (post tapeout) FPGA bitstream builds will pick up the new OTBN binary which will then get this patch applied when imm_section_start runs (whether in ROM_EXT or in a simulated immutable section during a test provisioning run).

This is patching an error in the original OTBN boot code loaded by ROM. If we're updating that boot code, we ought to be fixing the error that necessitated this patch.

Changing this will break future immutable sections for existing chips.

RomExt for chips with immutability enforced should be linked to their prebuilt sections (in private repo), which won't be affected by this PR. In fact, we already have 4 commits since the last imm_section release that changed its hash.

What I mean is that if we ever need to build a new immutable section because we're updating a SKU that builds on existing silicon, then the change to this patching logic will create an immutable section that wont work on A2 silicon.

IOW, the patch code should only be relevant to A2 silicon. It is not inconceivable that we'd find a bug in the immutable section and decide that we need to fix the imm_section and update SKUs (which would continue to manufacture with A2 silicon). If we change this patching code to accomodate the changed OTBN boot services program, it won't work anymore for A2.

[sasdf]

If we change this patching code to accomodate the changed OTBN boot services program, it won't work anymore for A2.

I'm a bit confused. This PR is trying to make the patch working with the A2 silicon's OTBN binary. That patch branch was actually skipped on real A2 silicon previously.

On current top-of-tree OTBN builds / bitstreams, this branch will be skipped as expected.

[cfrantz]

If we change this patching code to accomodate the changed OTBN boot services program, it won't work anymore for A2.

I'm a bit confused. This PR is trying to make the patch working with the A2 silicon's OTBN binary. That patch branch was actually skipped on real A2 silicon previously.

On current top-of-tree OTBN builds / bitstreams, this branch will be skipped as expected.

Oh, nvm. It was I who was confused. I thought this was dealing with the changes introduced by #28625. I see that the patching code checks that the instruction is the erroneous instruction before applying the patch.

I'm sorry for the confusion. I had #28625 on my mind.