fpga_compress: pick the LZ4 block size by consumer, not by file count

The 1 MB block branch exists for the ARM .data section: start.c's
uncompress_data_section() reads one 4-byte length and does one
LZ4_decompress_safe(), so .data has to arrive as a single block. It was
selected by 'num_infiles == 1', which is not what tells the two callers apart.

A build that skips LF, FeliCa and ISO15693 leaves FPGA_BITSTREAMS holding just
fpga_pm3_hf.bit, so the bitstream took that same branch and was packed as one
42 kB block. get_from_fpga_combined_stream() decompresses into a
FPGA_RING_BUFFER_BYTES buffer, 16 kB since 83c3f81b1:

  [#] inflate returned: -13247
  [#] reset_fpga_stream failed

Before 83c3f81b1 the copy was clamped with MIN(FPGA_RING_BUFFER_BYTES, ...)
whatever buffer_size said, so the blocks came out at 30 kB and the 30 kB ring
buffer still took them. That is why the commit looks like the cause - it only
removed the clamp that was covering for the wrong branch.

Add -s for the single block case and let the FPGA path always chop at
FPGA_RING_BUFFER_BYTES, however many bitstreams went in:

  4 bitstreams   169344 in -> 106933 out, byte identical to before
  1 bitstream     42172 in ->  28718 out, 3 blocks 13265/13206/2247,
                                          was 1 block of 27627
  .data  (-s)     14944 in ->   8786 out, byte identical to the
                                          obj/fullimage.data.bin.z in tree

Also hand the ring buffer back when reset_fpga_stream() fails. The early
return left it allocated for the rest of the session, which is the reporter's

  [#]   BigBuf_size............. 48116
  [#]   Available memory........ 31732

48116 - 31732 is 16384, exactly FPGA_RING_BUFFER_BYTES.

No CAPABILITIES_VERSION bump: fpga_all.bit.z is objcopy'd into the same
fullimage as the decompressor that reads it, so nothing here is client facing.

Reported and correctly diagnosed by @ewangsoft.

Fixes #3599

Co-Authored-By: Claude Opus 5 (1M context)
This commit is contained in:
iceman1001
2026-09-13 08:14:43 +02:00
co-authored by Claude Opus 5 (1M context)
parent 3b078f2c57
commit 43fe6c3eb8
5 changed files with 57 additions and 23 deletions
+2
View File
@@ -3,6 +3,8 @@ All notable changes to this project will be documented in this file.
This project uses the changelog in accordance with [keepchangelog](http://keepachangelog.com/). Please use this to write notable changes, which is not the same as git commit log...
## [unreleased][unreleased]
- Fixed `fpga_compress` - a build with only one FPGA bitstream (`SKIP_LF`, `SKIP_FELICA`, `SKIP_ISO15693`) packed it as a single 1 MB block the ARM could not decompress, `inflate returned: -13247` (@iceman1001)
- Fixed `FpgaDownloadAndGo` - a failed FPGA decompress leaked the ring buffer, leaving BigBuf 16 kB short for the rest of the session (@iceman1001)
- Changed `tools/pm3_online_tests.sh desfire_value` - the plain/mac assertion pairs sent byte identical APDUs (@iceman1001)
- Fixed `smart raw --t1` - a card runs the protocol its ATR names until a PPS changes it (@iceman1001)
- Fixed `tools/pm3_online_tests.sh smartcard` - the T=0 checks used the non existent `-0` flag (@iceman1001)
+2 -2
View File
@@ -319,9 +319,9 @@ $(OBJDIR)/fullimage.data.bin: $(OBJDIR)/fullimage.stage1.elf
$(OBJDIR)/fullimage.data.bin.z: $(OBJDIR)/fullimage.data.bin $(FPGA_COMPRESSOR)
$(info [-] GEN $@)
ifeq ($(Q),@)
@$(FPGA_COMPRESSOR) $(filter %.bin,$^) $@ >/dev/null
@$(FPGA_COMPRESSOR) -s $(filter %.bin,$^) $@ >/dev/null
else
$(FPGA_COMPRESSOR) $(filter %.bin,$^) $@
$(FPGA_COMPRESSOR) -s $(filter %.bin,$^) $@
endif
$(OBJDIR)/fullimage.elf: $(OBJDIR)/fullimage.stage1.elf $(OBJDIR)/fullimage.data.bin.z
+9 -1
View File
@@ -364,10 +364,18 @@ static void FpgaDownloadAndGoEx(int bitstream_target, bool keep_em) {
if (reset_fpga_stream(bitstream_target, &compressed_fpga_stream, output_buffer) == false) {
Dbprintf(_RED_("reset_fpga_stream failed"));
// hand the ring buffer back. Bailing out with it still allocated leaves BigBuf
// short by FPGA_RING_BUFFER_BYTES for the rest of the session, which shows up in
// "hw status" as available memory lower than BigBuf_size for no visible reason.
if (keep_em) {
BigBuf_free_keep_EM();
} else {
BigBuf_free();
}
return;
}
uint32_t bitstream_length;
uint32_t bitstream_length = 0;
if (bitparse_find_section(bitstream_target, 'e', &bitstream_length, &compressed_fpga_stream, output_buffer)) {
DownloadFPGA(bitstream_target, bitstream_length, &compressed_fpga_stream, output_buffer);
downloaded_bitstream = bitstream_target;
+5 -4
View File
@@ -36,13 +36,14 @@
// fpga_compress chops the interleaved bitstreams into blocks of at most this
// size, and the ARM decompresses them one at a time while DownloadFPGA() shifts
// the bytes out to the FPGA. It is allocated out of BigBuf, which on AT91 is
// only ~31-33 kB, so 30 kB left nothing for the caller.
// Smaller window = slightly worse compression
// Measured on flash:
// only ~31-33 kB, so 30 kB left nothing for the caller.
// Smaller window = slightly worse compression
// Measured on flash:
// 30 kB -> 102555 bytes
// 16 kB -> 107121 bytes
// This does NOT bound the firmware's .data section
// fpga_compress gives the single-input case its own 1 MB block, because start.c only decompresses one.
// fpga_compress -s gives the .data section its own 1 MB block, because start.c only decompresses one.
// Every FPGA bitstream, even a lone one in a HF only build, is chopped at this size.
#define FPGA_RING_BUFFER_BYTES (1024 * 16)
#define FPGA_TRACE_SIZE 3072
+39 -16
View File
@@ -32,6 +32,8 @@ int fileno(FILE *);
static void usage(void) {
fprintf(stdout, "Usage: fpga_compress <infile1> <infile2> ... <infile_n> <outfile>\n");
fprintf(stdout, " Combine n FPGA bitstream files and compress them into one.\n\n");
fprintf(stdout, " fpga_compress -s <infile> <outfile>\n");
fprintf(stdout, " Compress <infile> into ONE single LZ4 block. Used for the ARM .data section\n\n");
fprintf(stdout, " fpga_compress -v <infile1> <infile2> ... <infile_n> <outfile>\n");
fprintf(stdout, " Extract Version Information from FPGA bitstream files and write it to <outfile>\n\n");
fprintf(stdout, " fpga_compress -d <infile> <outfile(s)>\n");
@@ -47,7 +49,7 @@ static bool all_feof(FILE *infile[], uint8_t num_infiles) {
return true;
}
static int zlib_compress(FILE *infile[], uint8_t num_infiles, FILE *outfile) {
static int zlib_compress(FILE *infile[], uint8_t num_infiles, FILE *outfile, bool single_block) {
uint8_t *fpga_config = calloc(num_infiles * FPGA_CONFIG_SIZE, sizeof(uint8_t));
if (fpga_config == NULL) {
@@ -83,22 +85,25 @@ static int zlib_compress(FILE *infile[], uint8_t num_infiles, FILE *outfile) {
} while (all_feof(infile, num_infiles) == false);
// Block size for the LZ4 stream. Two very different consumers:
// Block size for the LZ4 stream. Two very different consumers, told apart by the
// caller with "-s", never by counting input files:
//
// - many infiles: the interleaved FPGA bitstreams. armsrc decompresses them
// one block at a time into a FPGA_RING_BUFFER_BYTES buffer taken from BigBuf
// - default: the interleaved FPGA bitstreams. armsrc decompresses them one
// block at a time into a FPGA_RING_BUFFER_BYTES buffer taken from BigBuf
// (see get_from_fpga_combined_stream()), so the block size must match.
// This holds for a single bitstream too. A HF-only build ( SKIP_LF,
// SKIP_FELICA, SKIP_ISO15693 ) hands us just fpga_pm3_hf.bit and it still
// has to come out in ring buffer sized blocks.
//
// - one infile: the firmware's .data section. start.c's
// uncompress_data_section() reads ONE 4-byte length and does ONE
// LZ4_decompress_safe(), so this must come out as a SINGLE block - it has no
// loop over blocks, and a short result is not treated as an error, so a
// second block would silently leave the tail of .data uninitialized.
// That is why it gets its own, much larger, block size and must not be
// tied to FPGA_RING_BUFFER_BYTES.
// - "-s": the firmware's .data section. start.c's uncompress_data_section()
// reads ONE 4-byte length and does ONE LZ4_decompress_safe(), so this must
// come out as a SINGLE block - it has no loop over blocks, and a short
// result is not treated as an error, so a second block would silently leave
// the tail of .data uninitialized. That is why it gets its own, much
// larger, block size and must not be tied to FPGA_RING_BUFFER_BYTES.
uint32_t buffer_size = FPGA_RING_BUFFER_BYTES;
if (num_infiles == 1) {
if (single_block) {
// 1M bytes for now
buffer_size = 1024 * 1024;
}
@@ -123,11 +128,11 @@ static int zlib_compress(FILE *infile[], uint8_t num_infiles, FILE *outfile) {
LZ4_streamHC_t *lz4_streamhc = LZ4_createStreamHC();
LZ4_resetStreamHC_fast(lz4_streamhc, LZ4HC_CLEVEL_MAX);
if (num_infiles == 1 && total_size > buffer_size) {
if (single_block && total_size > buffer_size) {
fprintf(stderr, "error: %u bytes does not fit in a single %u byte block, and start.c only decompresses one\n"
, total_size
, buffer_size
);
);
free(ring_buffer);
free(outbuf);
free(fpga_config);
@@ -547,14 +552,32 @@ int main(int argc, char **argv) {
} else { // Compress or generate version info
bool generate_version_file = false;
bool single_block = false;
uint8_t num_input_files = 0;
uint8_t first_input_file = 1;
if (!strcmp(argv[1], "-v")) { // generate version info
generate_version_file = true;
first_input_file = 2;
num_input_files = argc - 3;
} else if (!strcmp(argv[1], "-s")) { // compress one file into a single block
single_block = true;
first_input_file = 2;
num_input_files = argc - 3;
} else { // compress 1..n fpga files
num_input_files = argc - 2;
}
if (num_input_files == 0) {
usage();
return (EXIT_FAILURE);
}
if (single_block && num_input_files != 1) {
fprintf(stderr, "Error. -s takes exactly one input file\n\n");
return (EXIT_FAILURE);
}
FILE **infiles = calloc(num_input_files, sizeof(FILE *));
char **infile_names = calloc(num_input_files, sizeof(char *));
if (infiles == NULL || infile_names == NULL) {
@@ -564,7 +587,7 @@ int main(int argc, char **argv) {
return (EXIT_FAILURE);
}
for (uint8_t i = 0; i < num_input_files; i++) {
infile_names[i] = argv[i + (generate_version_file ? 2 : 1)];
infile_names[i] = argv[i + first_input_file];
infiles[i] = fopen(infile_names[i], "rb");
if (infiles[i] == NULL) {
fprintf(stderr, "Error. Cannot open input file %s\n\n", infile_names[i]);
@@ -592,7 +615,7 @@ int main(int argc, char **argv) {
if (generate_version_file) {
ret = generate_fpga_version_info(infiles, infile_names, num_input_files, outfile);
} else {
ret = zlib_compress(infiles, num_input_files, outfile);
ret = zlib_compress(infiles, num_input_files, outfile, single_block);
}
// close file handlers