mirror of
https://github.com/RfidResearchGroup/proxmark3.git
synced 2026-10-12 02:09:28 +00:00
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 since83c3f81b1: [#] inflate returned: -13247 [#] reset_fpga_stream failed Before83c3f81b1the 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:
co-authored by
Claude Opus 5 (1M context)
parent
3b078f2c57
commit
43fe6c3eb8
@@ -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
@@ -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
|
||||
|
||||
@@ -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
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user