lowRISC / opentitan

OpenTitan: Open source silicon root of trust
https://www.opentitan.org
Apache License 2.0
2.5k stars 745 forks source link

Replace srec_cat with objcopy #1107

Open imphil opened 4 years ago

imphil commented 4 years ago

We currently use srec_cat from srecord to create vmem files. objcopy does have similar functionality, but https://sourceware.org/bugzilla/show_bug.cgi?id=19921 was needed first. That patch landed, and binutils 2.33 should be the first release with a fixed objcopy. Once our toolchain has that version of binutils, try it and remove the srecord dependency.

towoe commented 4 years ago

For reference: reported by @olofk https://sourceware.org/bugzilla/show_bug.cgi?id=25202

GregAC commented 4 years ago

I had a go at fixing this while fixing #1986 and indeed the issue @olofk reported meant I couldn't work out a way to use objcopy instead of srec.

Currently the verilog target outputs the bytes in the wrong order. This can be fixed with --reverse-bytes but this only works when your input data size is a multiple of the reverse-bytes setting.

So for the 64-bit vmem in particular (that #1986 wants) --reverse-bytes=8 fails because the binary size isn't a multiple of 8 bytes.

We could fix this with the software build by instructing the linker script to pad all sections to an 8-byte boundary but I'm not sure we should be adding such things to the linker script purely to allows us to use objcopy.

imphil commented 4 years ago

Yeah, I think the patch for binutils got merged in the initial form, which I tried some time ago and it wasn't sufficient. So I'd simply stay with srec_cat for now, it's available widely enough.

jwnrt commented 1 year ago

Binutils 2.40 was just released with this patch to fix the byte order with --verilog-data-width. Does this look like enough for objcopy to work for our use case?

msfschaffner commented 11 months ago

@cfrantz @timothytrippel @a-will we should check whether we should get rid of this dependency or not.

timothytrippel commented 11 months ago

we could, but I don't think this is critical at the moment, so I would be in favor of closing or backlogging this.