Logger: added tool for parsing new trace format by bkokoszx · Pull Request #77 · thesofproject/soft

bkokoszx · 2018-09-26T10:55:19Z

I've added tool for parsing new trace format.
As input tool gets two files:

extracted .static_log_entries section generated by rimage in file *ldc (sof\src\arch\xtensa*ldc) - this rimage feature is added in #418 PR in sof repository;
binary file containing catch trace from dma.
Tool throws parsed logs to stdout.

lgirdwood

@bardliao please work with @bkokoszx here so that teh kernel output will align with the new tool. Can you also provide some kernel output once you have the ASCII HEX part working.

lgirdwood · 2018-09-26T11:10:19Z

+	uint32_t *params;
+}; 
+
+struct dma_log_header {


Is this a FW struct, if so we should include it from a header in uapi/

@bardliao is also making changes to the driver to use the kernel trace framework to output the tarce/log data. This means the format output will be the structure in ASCII HEX format.

@lgirdwood Ok, but in what way can I include uapi headers existing in different repo? (sof)

@bkokoszx the should be installed by "make install" rule from FW.

I've included from "/usr/local/include/sof/uapi/logging.h"

lgirdwood · 2018-09-26T11:11:21Z

+		entry.file_name,
+		entry.header.line_idx);
+
+	switch(entry.header.params_num){


switch ( needs a space

lgirdwood · 2018-09-26T11:11:49Z

+	switch(entry.header.params_num){
+	case 0:
+		fprintf(stdout, "%s", entry.text);
+	break;


break should be indented.

lgirdwood · 2018-09-26T11:12:15Z

+	uint32_t base_address,
+	uint32_t data_offset,
+	struct dma_log dma_log) {
+


why the newlines within the variable declaration block ?

lgirdwood · 2018-09-26T11:12:34Z

+
+	/* fetching elf header params */
+	ret = fread(&entry.header, sizeof(entry.header), 1, f_ldc);
+


no need for newline

lgirdwood · 2018-09-26T11:17:18Z

+	/* fetching text */
+	entry.text = (char *) malloc(entry.text_len);
+	if (entry.text == NULL) {
+		printf("Memory error.");


fprintf(stderr, "error: can't alocate %d bytes for XXX\n"); and for similar errors.

lgirdwood · 2018-09-26T11:18:47Z

+	return ret;
+}
+
+int main(int argc, char *argv[]) {


{ on next line

lgirdwood · 2018-09-26T11:19:32Z

+	int ret;
+
+	if (argc < 3) {
+		printf("Error: Too few input arguments. \n");


fprintf(stderr, etc...
this should also print the usage of logger including cmd line args.

lgirdwood · 2018-09-26T11:21:10Z

+		return -EINVAL;
+	}
+
+	const char *LDC_DIR = argv[1];


This should be in one block above and lowercase

lgirdwood · 2018-09-26T11:25:11Z

@@ -0,0 +1,4 @@
+bin_PROGRAMS = logger


sof-logger as this will avoid namespace conflicts.

bkokoszx · 2018-09-27T10:21:29Z

travis-ci was expected to fail. Sof should be installed ("make install") before building soft.

xiulipan · 2018-09-28T03:36:34Z

@bkokoszx
Why we need sof install for soft?
We want SOFT to be install independently. Thus why we move rimage into sof to avoid this situation.

bkokoszx · 2018-09-28T06:59:52Z

@xiulipan
I have to exploit struct existing in uapi header in FW. I can use use it since I install FW (make install copy headers to: /usr/local/include/sof/uapi/). It was change due Liam suggestion (#77 (comment)).
@lgirdwood
Did I misunderstand something?

xiulipan · 2018-09-28T07:51:16Z

@lgirdwood
So now we need some dependency on SOF again?
What should this work with trace event from @bardliao then? And the trace level from @RanderWang

singalsu · 2018-09-28T08:18:01Z

@bkokoszx @lgirdwood Is it possible for you to remove the offending code from SOF if this work is not mature enough to be merged to kernel and SOFT.

lgirdwood · 2018-09-28T08:54:57Z

@xiulipan soft tools can depend on sof. rimage was moved as dependency was the other way around. The tools need the uapi headers to use the ABI.

@bkokoszx can you resend new PR today and ignore the ASCII HEX readback part, this wont be ready for at least a week since national holidays in China.

bkokoszx · 2018-09-28T09:10:23Z

@lgirdwood
Can not it be merge from this PR? I've made changes due to your suggestions

lgirdwood · 2018-09-28T09:26:01Z

@bkokoszx have you re-pushed the new updates. I can still see some changes related to errno that are missing. i.e. you are returning EINVAL when you should be returning errno.

bkokoszx · 2018-09-28T09:41:56Z

@lgirdwood
I have return errno only to handle fopen functions. I've read that fread function doesn't set errno, so in that case I return EINVAL. Am I wrong?

xiulipan · 2018-09-28T09:56:32Z

@lgirdwood
So how do we handler the CI issues?
If we need to git clone SOF first?

@bkokoszx
And how about the old rmbox? should this logger to replace the rmbox or rmbox will still alive?
Maybe you can just rewrite the rmbox with the rmbox help functions.

xiulipan · 2018-09-28T09:59:23Z

+	int ret;
+
+	const char *ldc_dir = argv[1];
+	const char *dma_dump = argv[2];


Can you take rmbox as reference to have use getopt for the function.
Or just replace the old rmbox.

while ((opt = getopt(argc, argv, "ho:i:s:m:c:t")) != -1) { switch (opt) { case 'o': out_file = optarg; break; case 'i': in_file = optarg; break; case 't': trace = 1; break; case 'c': clk = atof(optarg); break; case 's': return snapshot(optarg); case 'h': default: /* '?' */ usage(argv[0]); } }

@bkokoszx yes pls use getopt, no need to replace rmbox atm as we need it for legacy. I want to merge this PR today.

bkokoszx · 2018-09-28T11:40:49Z

To handle fread I use following code:
ret = fread(..., file);
if (!ret) {
ret = -ferror(file);
goto out;
}
I will update getops

Signed-off-by: Bartosz Kokoszko <bartoszx.kokoszko@linux.intel.com>

bkokoszx · 2018-09-28T12:30:36Z

@lgirdwood
I've updated PR using getops function

lgirdwood · 2018-09-28T12:46:26Z

@bkokoszx one more change that can be done incrementally is to use relative path for uapi/logger.h i.e. include <sof/uapi/logger.h>

cujomalainey · 2018-10-01T19:29:36Z

Synchronization through version matching in the ABI?

lgirdwood · 2018-10-01T19:32:23Z

Yes, we would need some version data. @bkokoszx can you add versioning support here and we can duplicate the header.

mwierzbix · 2018-10-10T10:06:30Z

@lgirdwood @bkokoszx regarding header duplication in developement.
We could do this via git-submodule.
Make soft repo an optional submodule for sof repo, like ${sof_repo_root}/soft
This way, if one wants to checkout only soft he still can do this (it is separate repository still), and then can either replicate the directory tree and place the header in ${soft_repo_root}/../src/include/uapi/logging.h, or symlink it (even from a different repo).

And when you checkout both sof and soft inside of it, then you can freely change between branches/commits of both sof and soft.

If you want to check if logger from soft revision A works with sof revision B, then you just

cd "${sof_repo_root}"
git checkout B
cd soft
git checkout A
./logger (...)

Or we could do this the other way around.
Treat sof as a "library" for soft, and then make logger look for the header file in ${soft_repo_root}/sof, which would be a submodule populated via git-submodule update or just containing a header file / symlink to header file.

If you want to check if logger from soft revision A works with sof revision B, then you just

cd "${soft_repo_root}"
git checkout A
cd sof
git checkout B
cd ..
./logger (...)

And without populating submodule, only copying file

cd "${soft_repo_root}"
echo "sof/" >> .gitignore
mkdir -p sof/src/include/uapi
cp "${header_path}" sof/src/include/uapi/logging.h

or symlinking file
ln -s "${header_path}" sof/src/include/uapi/logging.h
from elsewhere.

git-submodule is an elegant way of forcing fixed directory structure between multiple git repositories.
What BTW we already have (it is, fixed directory structure, not git-submodule) with sof and sof-docs.

Regading how to make git-submodule optional: https://stackoverflow.com/questions/44366417/what-is-the-point-of-git-submodule-init

dabekjakub · 2018-10-10T10:32:42Z

@mwierzbix @lgirdwood @bkokoszx
In my experience having duplicate headers doesn't work very good. Tool repository headers will get updated only when the tool stops working and most of the users won't even be aware of the dependency. Which in worst case can lead to incorrect translation of the logs with no other symptoms.

Another solution to this could be moving logger itself to the FW repository (As was done with rimage). Since then we will have a complete package after building (FW and application to translate logs).

lgirdwood · 2018-10-10T15:45:40Z

@mmaka1 any objections to merging soft -> sof repo ? This would make CI easier too for working topologies since they would be aligned with FW. @cujomalainey would merging repos be OK for you ?

cujomalainey · 2018-10-10T16:37:53Z

Would it be just the tools migrated or the topologies as well? Either should be fine, I don't see any reason why that would be an issue over here. We will just need to update the scripts and docker system once the change is done.

lgirdwood · 2018-10-10T20:24:18Z

@cujomalainey everything over, tools and topologies.

mmaka1 · 2018-10-11T13:54:05Z

@lgirdwood I am Ok with this simplification.

bkokoszx · 2018-10-19T13:43:46Z

@lgirdwood What is your opinion about using git-submodule mentioned by @mwierzbix?

mwierzbix · 2018-10-19T13:51:42Z

@bkokoszx @lgirdwood Some pretty solid articles in the subject:
https://medium.com/@porteneuve/mastering-git-submodules-34c65e940407
https://medium.com/@porteneuve/mastering-git-subtrees-943d29a798ec

cujomalainey · 2018-10-19T14:21:27Z

Just make sure everyone understands to run git submodule update --init --recursive otherwise there will be a lot of confusing build failures

mwierzbix · 2018-10-19T14:22:45Z

@cujomalainey Hmm, you suggest README.md or maybe get-sof.sh?

cujomalainey · 2018-10-19T14:26:59Z

I would do a readme personally. Git submodules are pretty easy once you get over the short learning curve. The script would add to the confusion in my opinion.

lgirdwood suggested changes Sep 26, 2018

View reviewed changes

bkokoszx force-pushed the logger branch 3 times, most recently from 174554d to 957ee93 Compare September 27, 2018 09:35

xiulipan suggested changes Sep 28, 2018

View reviewed changes

bkokoszx force-pushed the logger branch from 957ee93 to 3503c21 Compare September 28, 2018 11:38

bkokoszx added 2 commits September 28, 2018 14:23

logger: added tool for parsing new trace format

6157e89

Signed-off-by: Bartosz Kokoszko <bartoszx.kokoszko@linux.intel.com>

logger: added and updated logger makefiles

437de79

Signed-off-by: Bartosz Kokoszko <bartoszx.kokoszko@linux.intel.com>

bkokoszx force-pushed the logger branch from 3503c21 to 437de79 Compare September 28, 2018 12:27

lgirdwood merged commit 918e401 into thesofproject:master Sep 28, 2018

mengdonglin mentioned this pull request Sep 28, 2018

Need rmbox update for debugability update in SOF #75

Closed

xiulipan mentioned this pull request Sep 29, 2018

Traris CI failed as #77 merged #85

Closed

bkokoszx deleted the logger branch October 16, 2018 07:51


		/* fetching elf header params */
		ret = fread(&entry.header, sizeof(entry.header), 1, f_ldc);

Conversation

bkokoszx commented Sep 26, 2018

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bkokoszx commented Sep 27, 2018

Uh oh!

xiulipan commented Sep 28, 2018

Uh oh!

bkokoszx commented Sep 28, 2018

Uh oh!

xiulipan commented Sep 28, 2018

Uh oh!

singalsu commented Sep 28, 2018

Uh oh!

lgirdwood commented Sep 28, 2018

Uh oh!

bkokoszx commented Sep 28, 2018

Uh oh!

lgirdwood commented Sep 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bkokoszx commented Sep 28, 2018

Uh oh!

xiulipan commented Sep 28, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bkokoszx commented Sep 28, 2018

Uh oh!

bkokoszx commented Sep 28, 2018

Uh oh!

lgirdwood commented Sep 28, 2018

Uh oh!

cujomalainey commented Oct 1, 2018

Uh oh!

lgirdwood commented Oct 1, 2018

Uh oh!

mwierzbix commented Oct 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dabekjakub commented Oct 10, 2018

Uh oh!

lgirdwood commented Oct 10, 2018

Uh oh!

cujomalainey commented Oct 10, 2018

Uh oh!

lgirdwood commented Oct 10, 2018

Uh oh!

mmaka1 commented Oct 11, 2018

Uh oh!

lgirdwood commented Sep 28, 2018 •

edited

Loading

mwierzbix commented Oct 10, 2018 •

edited

Loading