Skip to content

sof-logger: add checksums to -d(ump_file) option #5110

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.

Sign up for GitHub

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

Merged
lgirdwood merged 3 commits into from
Dec 22, 2021
Merged

sof-logger: add checksums to -d(ump_file) option #5110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f7d58b4
logger: rename verify_fw_ver() to verify_ldc_checksum(), fix -h help
marc-hb Dec 18, 2021
b176dc4
logger: add dictionary checksum in output of dump_ldc option
marc-hb Dec 18, 2021
3c870ca
logger: when FW is loaded, show the expected checksum in -d output
marc-hb Dec 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 25 additions & 10 deletions tools/logger/convert.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -950,15 +950,15 @@ static int logger_read(void)
return ret;
}

/* fw verification */
static int verify_fw_ver(void)
/** Compare the dictionary checksum in the firmware image (through
* /sys/kernel/debug/sof/fw_version) with the checksum in the .ldc file.
* @return 0 when the checksums match
*/
static int verify_ldc_checksum(const uint32_t ldc_sum)
{
struct sof_ipc_fw_version ver;
int count;

if (!global_config->version_fd)
return 0;

/* here fw verification should be exploited */
count = fread(&ver, sizeof(ver), 1, global_config->version_fd);
if (!count) {
Expand All @@ -967,9 +967,9 @@ static int verify_fw_ver(void)
}

/* compare source hash value from version file and ldc file */
if (ver.src_hash != global_config->logs_header->version.src_hash) {
if (ver.src_hash != ldc_sum) {
log_err("src hash value from version file (0x%x) differ from src hash version saved in dictionary (0x%x).\n",
ver.src_hash, global_config->logs_header->version.src_hash);
ver.src_hash, ldc_sum);
return -EINVAL;
}
return 0;
Expand All @@ -989,10 +989,22 @@ static int dump_ldc_info(void)
SOF_ABI_VERSION_MAJOR(SOF_ABI_DBG_VERSION),
SOF_ABI_VERSION_MINOR(SOF_ABI_DBG_VERSION),
SOF_ABI_VERSION_PATCH(SOF_ABI_DBG_VERSION));

fprintf(out_fd, "ldc_file ABI Version is\t%d:%d:%d\n",
SOF_ABI_VERSION_MAJOR(global_config->logs_header->version.abi_version),
SOF_ABI_VERSION_MINOR(global_config->logs_header->version.abi_version),
SOF_ABI_VERSION_PATCH(global_config->logs_header->version.abi_version));
fprintf(out_fd, "ldc_file src checksum\t\t0x%08x\n",
global_config->logs_header->version.src_hash);

if (global_config->version_fd) {
struct sof_ipc_fw_version ver;

if (fread(&ver, sizeof(ver), 1, global_config->version_fd))
fprintf(out_fd, "Loaded FW expects checksum\t0x%08x\n",
ver.src_hash);
}

fprintf(out_fd, "\n");
fprintf(out_fd, "Components uuid dictionary size:\t%zd bytes\n",
remaining);
Expand Down Expand Up @@ -1043,9 +1055,12 @@ int convert(struct convert_config *config)
return -EINVAL;
}

ret = verify_fw_ver();
if (ret)
return ret;
if (global_config->version_fw && /* -n option */
!global_config->dump_ldc) {
ret = verify_ldc_checksum(global_config->logs_header->version.src_hash);
if (ret)
return ret;
}

/* default logger and ldc_file abi verification */
if (SOF_ABI_VERSION_INCOMPATIBLE(SOF_ABI_DBG_VERSION,
Expand Down
9 changes: 4 additions & 5 deletions tools/logger/logger.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ static void usage(void)
fprintf(stdout, "%s:\t -l *.ldc_file\t\t.ldc input file generated by smex\n",
APP_NAME);
fprintf(stdout, "%s:\t -p \t\t\tInput from stdin\n", APP_NAME);
fprintf(stdout, "%s:\t -v ver_file\t\tEnable checking firmware version "
"with ver_file file\n", APP_NAME);
fprintf(stdout, "%s:\t -n\t\t\tDisable checking firmware version\n",
fprintf(stdout, "%s:\t -v ver_file\t\tUse ver_file instead of "
"/sys/kernel/debug/sof/fw_version\n", APP_NAME);
fprintf(stdout, "%s:\t -n\t\t\tDo not compare dictionary checksums\n",
APP_NAME);
fprintf(stdout, "%s:\t -c clock\t\tSet timestamp clock in MHz, %.2f MHz by default\n",
APP_NAME, DEFAULT_CLOCK_MHZ);
Expand Down Expand Up @@ -270,7 +270,6 @@ int main(int argc, char *argv[])
usage();
}
config.dump_ldc = 1;
config.version_fw = 0;
Copy link
Member

@ktrzcinx ktrzcinx Dec 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What motivation stands behind this change ?

Copy link
Collaborator Author

@marc-hb marc-hb Dec 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ktrzcinx for reviewing this PR, really appreciated.

This is what allows the -d ldc_file option to ALSO show the checksum expected by the firmware - when and only when a firmware is running. See sample output in the description. I suggest looking at each commit independently if you have looked only at the entire diff for now.

Copy link
Member

@ktrzcinx ktrzcinx Dec 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I've got it!

config.ldc_file = optarg;
break;
case 'F':
Expand Down Expand Up @@ -307,7 +306,7 @@ int main(int argc, char *argv[])

if (config.version_fw) {
config.version_fd = fopen(config.version_file, "rb");
if (!config.version_fd) {
if (!config.version_fd && !config.dump_ldc) {
ret = errno;
fprintf(stderr,
"error: Unable to open version file %s: %s\n",
Expand Down