Skip to content

Conversation

@mmaka1
Copy link

@mmaka1 mmaka1 commented Feb 17, 2020

The sof-logger and potentially other debug API clients perform ABI
compatibility check using the single FW ABI version. The same one is used
by the primary FW client which is the kernel driver. If there is a change
made to the debug API, the main ABI has to be updated to protect integrity
of the debug tools while such a change may not affect the kernel driver
at all.

This patch introduces new debug ABI version to be increased when changing
user space debug interfaces while the the main ABI is not affected.
Recompilation and installation of the new driver every time the tunneled
debug protocol is upgraded may be avoided.

Signed-off-by: Marcin Maka marcin.maka@linux.intel.com

@mmaka1
Copy link
Author

mmaka1 commented Feb 17, 2020

Implements #2374

Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like the separating out of this function can be its own patch.

Copy link
Contributor

Choose a reason for hiding this comment

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

If it were just refactor, then yea. But here you'd have 1 diff to have it changed in another.

@lgirdwood
Copy link
Member

Do we need corresponding kernel PR for this one ?

@mmaka1
Copy link
Author

mmaka1 commented Feb 17, 2020

Do we need corresponding kernel PR for this one ?

@lgirdwood I tested with unchanged kernel and seems to work fine. Another fw_ready extension does not break compatibility, therefore I increased 'minor' component only in the main ABI before aligning initial debug ABI version with the main one.

@lgirdwood
Copy link
Member

Do we need corresponding kernel PR for this one ?

@lgirdwood I tested with unchanged kernel and seems to work fine. Another fw_ready extension does not break compatibility, therefore I increased 'minor' component only in the main ABI before aligning initial debug ABI version with the main one.

Ok great, @plbossart good for you ? or would you need a patch that printk() the ABI version for userspace debug ?

@lgirdwood
Copy link
Member

SOFCI TEST

@lgirdwood
Copy link
Member

@zrombel @xiulipan CI is showing a pass afaict but we are seeing pending/failed ?

@zrombel
Copy link

zrombel commented Feb 19, 2020

Quickbuild changes are causing some problems, sorry for that. When it will be fixed I will rebuild this PR.

@zrombel
Copy link

zrombel commented Feb 19, 2020

QB is back on track. Build and tests passed.

Comment on lines +21 to +23
Copy link
Contributor

Choose a reason for hiding this comment

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

Is 'debug abi' the best name for this? We have folders that are named by-abi-user (kernelspace or userspace), but ABI version symbols are named by-feature (generic vs debug).

Copy link
Contributor

Choose a reason for hiding this comment

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

Nvm, I see it's going to be more granular for user, versions are in sof_ipc_user_abi_version

@jajanusz
Copy link
Contributor

sof-ci/jenkins/pr-tools-build Pending — Tools Building shows as pending, but it's CI reporting error, cos results there are skipped/ok

@lgirdwood
Copy link
Member

lgirdwood commented Feb 19, 2020

@mmaka1 could you copy the rimage patches to the external rimage repo to as another PR.
@fredoh9 @xiulipan any idea why tools build is timing out ? Server OK ?

@mmaka1 mmaka1 mentioned this pull request Feb 19, 2020
@jajanusz
Copy link
Contributor

@mmaka1 can u fix checkpatch issue?

@xiulipan
Copy link
Contributor

@lgirdwood As we are trying to update the GitHub behind a proyx, there is always some chance that the status update is missed. Will try to add some more strict check to see if we can use retry or other method to avoid such issue.

The sof-logger and potentially other debug API clients perform ABI
compatibility check using the single FW ABI version. The same one is used
by the primary FW client which is the kernel driver. If there is a change
made to the debug API, the main ABI has to be updated to protect integrity
of the debug tools while such a change may not affect the kernel driver
at all.

This patch introduces new debug ABI version to be increased when changing
user space debug interfaces while the the main ABI is not affected.
Recompilation and installation of the new driver every time the tunneled
debug protocol is upgraded may be avoided.

Signed-off-by: Marcin Maka <marcin.maka@linux.intel.com>
@mmaka1
Copy link
Author

mmaka1 commented Feb 20, 2020

@jajanusz I fixed the alignment issue. Another one be fixed is the memcpy issue however this function is used in many places in that source file, so there is a general replacement, done as another patch, required.

@lgirdwood
Copy link
Member

@zrombel we have a DMIC CI failure on ICL, but it should be unrelated to this patch ?

@wwittbrx
Copy link
Contributor

@lgirdwood Aside from SSP KD tests failing due to regression on master (should be fixed by #2440) this looks good to be merged.

@lgirdwood
Copy link
Member

SOFCI TEST

@lgirdwood
Copy link
Member

@wwittbrx running CI again now that Jenkins has had some fixes too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ABI ABI change involved

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants