Skip to content

ASoC: SOF: Introduce extended manifest#1818

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
kv2019i merged 6 commits into from
Mar 17, 2020
Merged

ASoC: SOF: Introduce extended manifest #1818

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
27451c3
ASoC: SOF: Mark get_ext* function ext_hdr arguments as const
ktrzcinx Mar 4, 2020
a5b175b
ASoC: SOF: Introduce offset in firmware data
ktrzcinx Mar 5, 2020
bea790a
ASoC: SOF: Introduce extended manifest
ktrzcinx Mar 2, 2020
a1d10c2
ASoC: SOF: ext_manifest: parse firmware version
ktrzcinx Mar 2, 2020
c0ba626
ASoC: SOF: ext_manifest: parse windows
ktrzcinx Mar 3, 2020
8fb90fe
ASoC: SOF: ext_manifest: parse compiler version
ktrzcinx Mar 3, 2020
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
3 changes: 3 additions & 0 deletions include/sound/sof.h
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -27,6 +27,9 @@ struct snd_sof_pdata {

struct device *dev;

/* indicate how many first bytes shouldn't be loaded into DSP memory. */
Comment thread
ktrzcinx marked this conversation as resolved.
Comment thread
ktrzcinx marked this conversation as resolved.
size_t fw_offset;

/*
* notification callback used if the hardware initialization
* can take time or is handled in a workqueue. This callback
Expand Down
91 changes: 91 additions & 0 deletions include/uapi/sound/sof/ext_manifest.h
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */
Comment thread
ranj063 marked this conversation as resolved.
/*
* This file is provided under a dual BSD/GPLv2 license. When using or
* redistributing this file, you may do so under either license.
*
* Copyright(c) 2020 Intel Corporation. All rights reserved.
*/

/*
* Extended manifest is a place to store metadata about firmware, known during
* compilation time - for example firmware version or used compiler.
* Given information are read on host side before firmware startup.
* This part of output binary is not signed.
*/

#ifndef __SOF_FIRMWARE_EXT_MANIFEST_H__
#define __SOF_FIRMWARE_EXT_MANIFEST_H__

#include <linux/const.h>

@lyakhlyakhMar 17, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

you also need <linux/types.h> and <linux/bits.h>. Strictly speaking also <linux/compiler.h> but it looks like nobody does that, that will be pulled in for you somewhere along the way.

@ktrzcinxktrzcinxMar 17, 2020

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I will keep it in mind and make a proper fix.

#include <sound/sof/info.h>

/* In ASCII `XMan` */
#define SOF_EXT_MAN_MAGIC_NUMBER 0x6e614d58

/* Build u32 number in format MMmmmppp */
#define SOF_EXT_MAN_BUILD_VERSION(MAJOR, MINOR, PATH) ((uint32_t)( \
((MAJOR) << 24) | \
((MINOR) << 12) | \
(PATH)))

/* check extended manifest version consistency */
#define SOF_EXT_MAN_VERSION_INCOMPATIBLE(host_ver, cli_ver) ( \
((host_ver) & GENMASK(31, 24)) != \
((cli_ver) & GENMASK(31, 24)))

/* used extended manifest header version */
#define SOF_EXT_MAN_VERSION SOF_EXT_MAN_BUILD_VERSION(1, 0, 0)

/* extended manifest header, deleting any field breaks backward compatibility */
struct sof_ext_man_header {
uint32_t magic; /*< identification number, */
/*< EXT_MAN_MAGIC_NUMBER */
uint32_t full_size; /*< [bytes] full size of ext_man, */
/*< (header + content + padding) */
uint32_t header_size; /*< [bytes] makes header extensionable, */
/*< after append new field to ext_man header */
/*< then backward compatible won't be lost */
uint32_t header_version; /*< value of EXT_MAN_VERSION */
/*< not related with following content */
uint8_t elements[]; /*< list of ext_man_elem_* elements */
} __packed;

/* Now define extended manifest elements */

/* Extended manifest elements types */
enum sof_ext_man_elem_type {
SOF_EXT_MAN_ELEM_FW_VERSION = 0,
SOF_EXT_MAN_ELEM_WINDOW = SOF_IPC_EXT_WINDOW,
SOF_EXT_MAN_ELEM_CC_VERSION = SOF_IPC_EXT_CC_INFO,
};

/* extended manifest element header */
struct sof_ext_man_elem_header {
uint32_t type; /*< SOF_EXT_MAN_ELEM_ */
uint32_t size; /*< in bytes, including header size */
uint8_t blob[]; /*< type dependent content */
} __packed;

/* FW version */
struct sof_ext_man_fw_version {
struct sof_ext_man_elem_header hdr;
/* use sof_ipc struct because of code re-use */
struct sof_ipc_fw_version version;
uint32_t flags;
} __packed;

/* extended data memory windows for IPC, trace and debug */
struct sof_ext_man_window {
struct sof_ext_man_elem_header hdr;
/* use sof_ipc struct because of code re-use */
struct sof_ipc_window ipc_window;
} __packed;

/* Used C compiler description */
struct sof_ext_man_cc_version {
struct sof_ext_man_elem_header hdr;
/* use sof_ipc struct because of code re-use */
struct sof_ipc_cc_version cc_version;
} __packed;

#endif /* __SOF_FIRMWARE_EXT_MANIFEST_H__ */
9 changes: 7 additions & 2 deletions sound/soc/sof/intel/hda-loader.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -296,8 +296,13 @@ int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev)

chip_info = desc->chip_info;

stripped_firmware.data = plat_data->fw->data;
stripped_firmware.size = plat_data->fw->size;
if (plat_data->fw->size < plat_data->fw_offset) {

@lyakhlyakhMar 17, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

it should ne <= probably?

@kv2019ikv2019iMar 17, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

At least "<" guarantees the wrap around, that's the purpose of this check.

@lyakhlyakhMar 17, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@kv2019i sorry, don't understand. The difference between <= and < is the == case. With <=== would also trigger the error path. And indeed, that would fit the error message ("must be larger") and 0 size after the offset doesn't indeed seem to make much sense. As for wrapping around 4GiB - wouldn't you have to catch that earlier - when performing calculations, that potentially can wrap?

dev_err(sdev->dev, "error: firmware size must be greater than firmware offset\n");
return -EINVAL;
}

stripped_firmware.data = plat_data->fw->data + plat_data->fw_offset;
stripped_firmware.size = plat_data->fw->size - plat_data->fw_offset;
Comment thread
ktrzcinx marked this conversation as resolved.

/* init for booting wait */
init_waitqueue_head(&sdev->boot_wait);
Expand Down
Loading