-
Notifications
You must be signed in to change notification settings - Fork 318
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.
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
Global UUID registry, cleanup, simplification #9261
Changes from all commits
7d2f95b
9042bca
2b5a8ef
d92966c
5c8dadc
190a771
7702f68
de345d9
eb4ff70
1c5e0af
a665561
a0e0cef
1c25b2a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
# | ||
# UUID registry generation | ||
# | ||
|
||
# Simple target. FOUR (really 4.5, as LIBRARY builds use the same | ||
# CMakeLists.txt but differ significantly in how it executes) | ||
# different cmake environments into which it needs to build. | ||
is_zephyr(zephyr_is) | ||
if(zephyr_is) | ||
set(TOPDIR ${sof_top_dir}) | ||
set(UUID_REG_H ${PROJECT_BINARY_DIR}/include/generated/uuid-registry.h) | ||
set(DEP_TARGET zephyr_interface) | ||
elseif(${PROJECT_NAME} STREQUAL "SOF_TOOLS") | ||
set(TOPDIR "${PROJECT_SOURCE_DIR}/..") | ||
set(UUID_REG_H "${CMAKE_CURRENT_BINARY_DIR}/uuid-registry.h") | ||
set(DEP_TARGET sof-logger) | ||
elseif(${PROJECT_NAME} STREQUAL "SOF_TPLG_PARSER") | ||
set(TOPDIR "${PROJECT_SOURCE_DIR}/../..") | ||
set(UUID_REG_H "${PROJECT_BINARY_DIR}/include/uuid-registry.h") | ||
set(DEP_TARGET sof_tplg_parser) | ||
else() | ||
# Legacy SOF, or CONFIG_LIBRARY | ||
set(TOPDIR ${PROJECT_SOURCE_DIR}) | ||
set(UUID_REG_H ${PROJECT_BINARY_DIR}/generated/include/uuid-registry.h) | ||
set(DEP_TARGET sof_public_headers) | ||
endif() | ||
|
||
add_custom_command( | ||
OUTPUT ${UUID_REG_H} | ||
COMMAND | ||
${PYTHON_EXECUTABLE} ${TOPDIR}/scripts/gen-uuid-reg.py | ||
${TOPDIR}/uuid-registry.txt | ||
${UUID_REG_H}) | ||
|
||
add_custom_target(uuid_reg_h DEPENDS ${UUID_REG_H}) | ||
|
||
add_dependencies(${DEP_TARGET} uuid_reg_h) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
#!/usr/bin/env python3 | ||
# SPDX-License-Identifier: BSD-3-Clause | ||
import re | ||
import sys | ||
|
||
# Very simple UUID registry validator and C header generator. Parses | ||
# uuid-registry.txt (passed as the first command line argument) and | ||
# writes a C header (named in the second argument) containing | ||
# definitions to be used at build time. Fails via assertion if the | ||
# any element in the registry is invalid. | ||
|
||
header = """/* | ||
* GENERATED CODE. DO NOT EDIT. | ||
* | ||
* Generated UUID records (initializers for struct sof_uuid) | ||
* See scripts/gen-uuid-reg.py | ||
*/ | ||
#ifndef _UUID_REGISTRY_H | ||
#define _UUID_REGISTRY_H | ||
""" | ||
|
||
all_syms = set() | ||
all_uuids = set() | ||
out_recs = [] | ||
|
||
def emit_uuid_rec(uu, sym): | ||
recs = uu.split('-') | ||
brec = recs[3] | ||
wrecs = [ "0x" + r for r in recs[0:3] ] | ||
byts = [ "0x" + brec[ 2*i : 2*i+2 ] for i in range(int(len(brec) / 2)) ] | ||
uuidinit = "{ " + ", ".join(wrecs) + ", { " + ", ".join(byts) + " } }" | ||
out_recs.append(f"#define _UUIDREG_{sym} {uuidinit}") | ||
|
||
def main(): | ||
with open(sys.argv[1]) as f: | ||
for line in f.readlines(): | ||
line = re.sub(r'\s*#.*', '', line) # trim comments | ||
line = re.sub(r'^\s*', '', line) # trim leading ws | ||
line = re.sub(r'\s*$', '', line) # trim trailing ws | ||
if line == "": | ||
continue | ||
m = re.match(r'(.*)\s+(.*)', line) | ||
assert m | ||
(uu, sym) = (m.group(1).lower(), m.group(2)) | ||
assert re.match(r'[0-9a-f]{8}(?:-[0-9a-f]{4}){2}-[0-9a-f]{16}', uu) | ||
assert re.match(r'[a-zA-Z_][a-zA-Z0-9_]*', sym) | ||
assert len(sym) < 32 | ||
assert uu not in all_uuids | ||
assert sym not in all_syms | ||
all_uuids.add(uu) | ||
all_syms.add(sym) | ||
emit_uuid_rec(uu, sym) | ||
|
||
with open(sys.argv[2], "w") as f: | ||
f.write(header) | ||
for l in out_recs: | ||
f.write(l + "\n") | ||
f.write("#endif /* _UUID_REGISTRY_H */\n") | ||
|
||
if __name__ == "__main__": | ||
main() |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -877,16 +877,6 @@ def build_platforms(): | |
else: # unknown failure | ||
raise cpe | ||
|
||
smex_executable = pathlib.Path(west_top, platform_build_dir_name, "zephyr", "smex_ep", | ||
"build", "smex") | ||
fw_ldc_file = pathlib.Path(sof_platform_output_dir, f"sof-{platform}.ldc") | ||
input_elf_file = pathlib.Path(west_top, platform_build_dir_name, "zephyr", "zephyr.elf") | ||
# Extract metadata | ||
execute_command([str(smex_executable), "-l", str(fw_ldc_file), str(input_elf_file)]) | ||
|
||
for p_alias in platform_configs[platform].aliases: | ||
symlink_or_copy(sof_platform_output_dir, f"sof-{platform}.ldc", sof_platform_output_dir, f"sof-{p_alias}.ldc") | ||
|
||
# reproducible-zephyr.ri is less useful now that show_installed_files() shows | ||
# checksums too. However: - it's still useful when only the .ri file is | ||
# available (no build logs for the other image), - it makes sure sof_ri_info.py | ||
|
@@ -900,15 +890,6 @@ def build_platforms(): | |
src_dest_list = [] | ||
tools_output_dir = pathlib.Path(STAGING_DIR, "tools") | ||
|
||
# Install sof-logger from the last platform built (requires building at least one platform). | ||
if platform_build_dir_name: | ||
sof_logger_dir = pathlib.Path(west_top, platform_build_dir_name, "zephyr", | ||
"sof-logger_ep", "build", "logger") | ||
sof_logger_executable_to_copy = pathlib.Path(shutil.which("sof-logger", path=sof_logger_dir)) | ||
sof_logger_installed_file = pathlib.Path(tools_output_dir, sof_logger_executable_to_copy.name).resolve() | ||
|
||
src_dest_list += [(sof_logger_executable_to_copy, sof_logger_installed_file)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is this hunk removed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Honestly I'm hoping someone chimes in to clarify this. AFAICT sof-logger, smex, .ldc files and old-style trace context usage are all degenerate in Zephyr builds. The window that used to store the dictionary logging output has been repurposed to transmit plaintext Zephyr logs, and almost nothing is actually emitting the trace output anyway as all the {tr,comp}_{err,warn,info}() has been redirected to Zephyr LOG_xxx() macros which ignore the context struct. Basically this is me ripping the bandaid off to see what breaks. For sure, after this PR merges none of that stuff will work anymore as the addresses of the sof_uuid structs won't appear within the static_uuids section and sof-logger won't see any ldc entries. If we absolutely still need this, let me know and I'll see what needs to be ported. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @andyross @lyakh Let's have some from @thesofproject/nxp check this. In the transition to Zephyr, we did use sof-logger early with Zephyr so the smex/ldc/soflogger stuff was also used with Zephyr, but indeed for Intel platforms we've now moved on to Zephyr logging subsystem completely. But has NXP made the move? There is certainly a maintainance cost in having these around (see #8822 ), so in general I'm happy to drop these. PS The UUID rework looks great. Once we have the v2.10 release out, I'll do a more proper review. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @thesofproject/mediatek should also check to see if they still depend on this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW: mt8xxx builds are fine in-tree as non-Zephyr builds are unchanged and will continue to work. The question here is whether or not any Zephyr configurations are still in the deprecated/only-partially-working trace context world. And I'm pretty sure[1] the answer is no, because if you tried you'd discover that it doesn't actually have all the info. When Anyway, the point being: this PR moves the UUIDs to a different location in the binary (an iterable array and not a special [1] I mean, actually I'm more than pretty sure. But software is complicated and edge cases abound, thus the questions. [2] Zephyr logging is plaintext, ignores the trace context and UUID, and doesn't require .ldc files nor smex/sof_logger. There actually is a Zephyr dictionary logging feature @dcpleung did long ago, but it's not enabled for SOF and to be honest I don't know much about how its tooling works. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh i totally misread this as C code, not python, then yes ignore my previous comment since this is zephyr specific There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately we still have old and very complex build and CI dependencies here because (as usual) the logging transitions to Zephyr and IPC4 were after-thoughts and rushed on the validation side. Some in Intel CI are not even open-source, which means you couldn't fix them even if you volunteered to. So unless this literally stops building, I highly recommend you don't touch this if you want the rest merged in a reasonable time. Feel free to print additional warnings. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @andyross As for Zephyr logging being plaintext, we have enabled both Zephyr dictionary and mipi syst and even have an overlay in upstream SOF -> work/sof/app/logging_mipisystcat_overlay.conf . Enablement is pending changes to CI infra which has a lot of dependencies to the current plaintext logs, but we have full capability to employ dictionaries. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @andyross I'm good to go ahead without CI being ready - can you wrap any transition a a Kconfig which we can enable as CI catches up. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CI should be fine, almost all of it was just waiting for the missing Zephyr patch mentioned above, which is in now that #9278 has merged.[1] Should have it up today, July-4th-distractions willing. [1] Also there was a tweak needed for testbench, which builds the SOF code in a third (!) cmake environment to make a shared library for tplg_parser which early-exits (!!) from the legacy CMakeLists.txt, so I needed to move where the generation was included. |
||
|
||
src_dest_list += [(pathlib.Path(SOF_TOP) / | ||
"tools" / "mtrace"/ "mtrace-reader.py", | ||
tools_output_dir / "mtrace-reader.py")] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,11 +39,9 @@ static const struct comp_driver comp_dai; | |
|
||
LOG_MODULE_REGISTER(dai_comp, CONFIG_SOF_LOG_LEVEL); | ||
|
||
/* c2b00d27-ffbc-4150-a51a-245c79c5e54b */ | ||
DECLARE_SOF_RT_UUID("dai", dai_comp_uuid, 0xc2b00d27, 0xffbc, 0x4150, | ||
0xa5, 0x1a, 0x24, 0x5c, 0x79, 0xc5, 0xe5, 0x4b); | ||
SOF_DEFINE_REG_UUID(dai); | ||
|
||
DECLARE_TR_CTX(dai_comp_tr, SOF_UUID(dai_comp_uuid), LOG_LEVEL_INFO); | ||
DECLARE_TR_CTX(dai_comp_tr, SOF_UUID(dai_uuid), LOG_LEVEL_INFO); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this part of the unification needed for some automation in one of the following commits? If not - maybe we could keep different names, makes grepping the code easier There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually this is sort of a fakeout. The names in-tree are both "dai", and they were renamed by an early patch in this series to disambiguate them. But then it turns out that the value of the UUID is the same in both cases, which is unfortunate but technically unresolvable (I mean... not really, but I'm trying very hard not to have to renumber these things and being really clear where I do). So this moves back to the original usage where the two drivers share the same UUID, which is OK in this case as they can't be built into the same image anyway. I should fold them together to reduce the churn, though that's a mildly hard rebase. |
||
|
||
#if CONFIG_COMP_DAI_GROUP | ||
|
||
|
@@ -1112,7 +1110,7 @@ static uint64_t dai_get_processed_data(struct comp_dev *dev, uint32_t stream_no, | |
|
||
static const struct comp_driver comp_dai = { | ||
.type = SOF_COMP_DAI, | ||
.uid = SOF_RT_UUID(dai_comp_uuid), | ||
.uid = SOF_RT_UUID(dai_uuid), | ||
.tctx = &dai_comp_tr, | ||
.ops = { | ||
.create = dai_new, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bytes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"bytes()" is a python builtin, alas.